Skip to content

Commit

Permalink
Introduce Slf4jLogDeclaration to canonicalize slf4j's variable's de…
Browse files Browse the repository at this point in the history
…claration
  • Loading branch information
mohamedsamehsalah committed Sep 8, 2023
1 parent 87f79da commit 536811a
Show file tree
Hide file tree
Showing 2 changed files with 220 additions and 0 deletions.
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);
}
}
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_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_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);
}
}

0 comments on commit 536811a

Please sign in to comment.