-
Notifications
You must be signed in to change notification settings - Fork 39
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Introduce
Slf4jLogDeclaration
to canonicalize slf4j's variable's de…
…claration
- Loading branch information
mohamedsamehsalah
committed
Sep 8, 2023
1 parent
87f79da
commit fc4d717
Showing
2 changed files
with
220 additions
and
0 deletions.
There are no files selected for viewing
129 changes: 129 additions & 0 deletions
129
...r-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/Slf4jLogDeclaration.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
package tech.picnic.errorprone.bugpatterns; | ||
|
||
import static com.google.errorprone.BugPattern.LinkType.CUSTOM; | ||
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; | ||
import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR; | ||
import static com.google.errorprone.matchers.Matchers.allOf; | ||
import static com.google.errorprone.matchers.Matchers.hasIdentifier; | ||
import static com.google.errorprone.matchers.Matchers.isSubtypeOf; | ||
import static com.google.errorprone.matchers.Matchers.staticMethod; | ||
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL; | ||
|
||
import com.google.auto.service.AutoService; | ||
import com.google.errorprone.BugPattern; | ||
import com.google.errorprone.ErrorProneFlags; | ||
import com.google.errorprone.VisitorState; | ||
import com.google.errorprone.bugpatterns.BugChecker; | ||
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher; | ||
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; | ||
import com.google.errorprone.fixes.SuggestedFix; | ||
import com.google.errorprone.fixes.SuggestedFixes; | ||
import com.google.errorprone.matchers.Description; | ||
import com.google.errorprone.matchers.Matcher; | ||
import com.sun.source.tree.ClassTree; | ||
import com.sun.source.tree.ExpressionTree; | ||
import com.sun.source.tree.MemberSelectTree; | ||
import com.sun.source.tree.MethodInvocationTree; | ||
import com.sun.source.tree.Tree; | ||
import com.sun.source.tree.Tree.Kind; | ||
import com.sun.source.tree.VariableTree; | ||
import javax.inject.Inject; | ||
import javax.lang.model.element.Modifier; | ||
import javax.lang.model.element.Name; | ||
import javax.tools.JavaFileObject; | ||
import tech.picnic.errorprone.bugpatterns.util.SourceCode; | ||
|
||
/** | ||
* A {@link BugChecker} that warns when SLF4J declarations are not canonicalized across the project. | ||
* | ||
* @apiNote The default canonicalized logger name can be overriden through {@link ErrorProneFlags | ||
* flag arguments}. | ||
*/ | ||
@AutoService(BugChecker.class) | ||
@BugPattern( | ||
summary = "Make sure SLF4J log declarations follow the SLF4J conventions inside classes.", | ||
link = BUG_PATTERNS_BASE_URL + "Slf4jLogDeclaration", | ||
linkType = CUSTOM, | ||
severity = WARNING, | ||
tags = LIKELY_ERROR) | ||
public final class Slf4jLogDeclaration extends BugChecker | ||
implements ClassTreeMatcher, MethodInvocationTreeMatcher { | ||
private static final long serialVersionUID = 1L; | ||
private static final Matcher<ClassTree> TEST_CLASS_WITH_LOGGER = | ||
allOf(hasIdentifier(isSubtypeOf("org.slf4j.Logger"))); | ||
private static final Matcher<Tree> LOGGER = isSubtypeOf("org.slf4j.Logger"); | ||
private static final String CANONICALIZED_LOGGER_NAME_FLAG = | ||
"Slf4jLogDeclaration:CanonicalizedLoggerName"; | ||
private static final String DEFAULT_CANONICALIZED_LOGGER_NAME = "LOG"; | ||
private static final Matcher<ExpressionTree> GET_LOGGER_METHOD = | ||
staticMethod().onDescendantOf("org.slf4j.LoggerFactory").named("getLogger"); | ||
|
||
private final String canonicalizedLoggerName; | ||
|
||
/** Instantiates a default {@link Slf4jLogDeclaration} instance. */ | ||
public Slf4jLogDeclaration() { | ||
this(ErrorProneFlags.empty()); | ||
} | ||
|
||
/** | ||
* Instantiates a customized {@link Slf4jLogDeclaration}. | ||
* | ||
* @param flags Any provided command line flags. | ||
*/ | ||
@Inject | ||
Slf4jLogDeclaration(ErrorProneFlags flags) { | ||
canonicalizedLoggerName = getCanonicalizedLoggerName(flags); | ||
} | ||
|
||
@Override | ||
public Description matchClass(ClassTree tree, VisitorState state) { | ||
if (tree.getKind() == Kind.INTERFACE || !TEST_CLASS_WITH_LOGGER.matches(tree, state)) { | ||
return Description.NO_MATCH; | ||
} | ||
|
||
SuggestedFix.Builder fixBuilder = SuggestedFix.builder(); | ||
for (Tree member : tree.getMembers()) { | ||
if (LOGGER.matches(member, state)) { | ||
VariableTree variable = (VariableTree) member; | ||
SuggestedFixes.addModifiers( | ||
member, state, Modifier.PRIVATE, Modifier.STATIC, Modifier.FINAL) | ||
.ifPresent(fixBuilder::merge); | ||
if (!variable.getName().toString().startsWith(canonicalizedLoggerName)) { | ||
fixBuilder.merge(SuggestedFixes.renameVariable(variable, canonicalizedLoggerName, state)); | ||
} | ||
} | ||
} | ||
|
||
return describeMatch(tree, fixBuilder.build()); | ||
} | ||
|
||
@Override | ||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { | ||
if (!GET_LOGGER_METHOD.matches(tree, state)) { | ||
return Description.NO_MATCH; | ||
} | ||
SuggestedFix.Builder fixBuilder = SuggestedFix.builder(); | ||
|
||
for (ExpressionTree arg : tree.getArguments()) { | ||
MemberSelectTree memberArgument = (MemberSelectTree) arg; | ||
|
||
for (Tree typeDeclaration : state.getPath().getCompilationUnit().getTypeDecls()) { | ||
if (typeDeclaration instanceof ClassTree) { | ||
Name className = ((ClassTree) typeDeclaration).getSimpleName(); | ||
String argumentName = SourceCode.treeToString(memberArgument.getExpression(), state); | ||
|
||
if (!className.contentEquals(argumentName)) { | ||
fixBuilder.merge( | ||
SuggestedFix.replace(arg, className + JavaFileObject.Kind.CLASS.extension)); | ||
} | ||
} | ||
} | ||
} | ||
|
||
return describeMatch(tree, fixBuilder.build()); | ||
} | ||
|
||
private static String getCanonicalizedLoggerName(ErrorProneFlags flags) { | ||
return flags.get(CANONICALIZED_LOGGER_NAME_FLAG).orElse(DEFAULT_CANONICALIZED_LOGGER_NAME); | ||
} | ||
} |
91 changes: 91 additions & 0 deletions
91
...one-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/Slf4jLogDeclarationTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
package tech.picnic.errorprone.bugpatterns; | ||
|
||
import com.google.errorprone.BugCheckerRefactoringTestHelper; | ||
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; | ||
import org.junit.jupiter.api.Test; | ||
|
||
final class Slf4jLogDeclarationTest { | ||
@Test | ||
void replacement() { | ||
BugCheckerRefactoringTestHelper.newInstance(Slf4jLogDeclaration.class, getClass()) | ||
.addInputLines( | ||
"A.java", | ||
"import org.slf4j.Logger;", | ||
"import org.slf4j.LoggerFactory;", | ||
"", | ||
"class A {", | ||
" Logger LOG1 = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" private Logger LOG2 = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" private static Logger LOG3 = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" static final Logger LOG4 = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" private final Logger LOG5 = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" private static final Logger LOG6 = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" private static final Logger NOT_PROPER_LOGGER_NAME = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" private static final Logger LOGGER_WITH_WRONG_CLASS_AS_ARGUMENT = LoggerFactory.getLogger(B.class);", | ||
"", | ||
" class B {}", | ||
"}") | ||
.addOutputLines( | ||
"A.java", | ||
"import org.slf4j.Logger;", | ||
"import org.slf4j.LoggerFactory;", | ||
"", | ||
"class A {", | ||
" private static final Logger LOG1 = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" private static final Logger LOG2 = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" private static final Logger LOG3 = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" private static final Logger LOG4 = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" private static final Logger LOG5 = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" private static final Logger LOG6 = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" private static final Logger LOG = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" private static final Logger LOGGER_WITH_WRONG_CLASS_AS_ARGUMENT = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" class B {}", | ||
"}") | ||
.doTest(TestMode.TEXT_MATCH); | ||
} | ||
|
||
@Test | ||
void doNotAddModifiersToDeclarationsInsideInterfaces() { | ||
BugCheckerRefactoringTestHelper.newInstance(Slf4jLogDeclaration.class, getClass()) | ||
.addInputLines( | ||
"A.java", | ||
"import org.slf4j.Logger;", | ||
"import org.slf4j.LoggerFactory;", | ||
"", | ||
"interface A {", | ||
" Logger LOG = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" Logger LOG1 = LoggerFactory.getLogger(B.class);", | ||
"", | ||
" class B {}", | ||
"}") | ||
.addOutputLines( | ||
"A.java", | ||
"import org.slf4j.Logger;", | ||
"import org.slf4j.LoggerFactory;", | ||
"", | ||
"interface A {", | ||
" Logger LOG = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" Logger LOG1 = LoggerFactory.getLogger(A.class);", | ||
"", | ||
" class B {}", | ||
"}") | ||
.doTest(TestMode.TEXT_MATCH); | ||
} | ||
} |