Address comments: Add test case for bug checker's flags

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/Slf4jLogDeclaration.java

@@ -5,7 +5,8 @@
 import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
 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 static javax.tools.JavaFileObject.Kind.CLASS;
+import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;
 
 import com.google.auto.service.AutoService;
 import com.google.errorprone.BugPattern;
@@ -14,6 +15,7 @@
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
 import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.fixes.SuggestedFix.Builder;
 import com.google.errorprone.fixes.SuggestedFixes;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
@@ -27,14 +29,13 @@
 import javax.inject.Inject;
 import javax.lang.model.element.Modifier;
 import javax.lang.model.element.Name;
-import javax.tools.JavaFileObject;
 import org.jspecify.annotations.Nullable;
-import tech.picnic.errorprone.bugpatterns.util.SourceCode;
+import tech.picnic.errorprone.utils.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
+ * @apiNote The default canonicalized logger name can be overridden through {@link ErrorProneFlags
  *     flag arguments}.
  */
 @AutoService(BugChecker.class)
@@ -82,7 +83,7 @@ public Description matchClass(ClassTree tree, VisitorState state) {
         canonicalizeLoggerVariable(member, state, fixBuilder);
       }
     }
-    fixLoggerVariableDeclaration(state, fixBuilder);
+    fixLoggerVariableDeclaration(tree, state, fixBuilder);
 
     return fixBuilder.isEmpty() ? Description.NO_MATCH : describeMatch(tree, fixBuilder.build());
   }
@@ -96,38 +97,33 @@ private static void fixLoggerVariableModifiers(
   private void canonicalizeLoggerVariable(
       Tree member, VisitorState state, SuggestedFix.Builder fixBuilder) {
     VariableTree variable = (VariableTree) member;
-    if (!variable.getName().toString().equals(canonicalizedLoggerName)) {
+    if (!variable.getName().contentEquals(canonicalizedLoggerName)) {
       fixBuilder.merge(SuggestedFixes.renameVariable(variable, canonicalizedLoggerName, state));
     }
   }
 
   private static void fixLoggerVariableDeclaration(
-      VisitorState state, SuggestedFix.Builder fixBuilder) {
-    for (Tree typeDeclaration : state.getPath().getCompilationUnit().getTypeDecls()) {
-      if (typeDeclaration instanceof ClassTree) {
-        new TreeScanner<@Nullable Void, Name>() {
-          @Override
-          public @Nullable Void visitClass(ClassTree classTree, Name className) {
-            return super.visitClass(classTree, classTree.getSimpleName());
-          }
+      ClassTree tree, VisitorState state, Builder fixBuilder) {
+    new TreeScanner<@Nullable Void, Name>() {
+      @Override
+      public @Nullable Void visitClass(ClassTree classTree, Name className) {
+        return super.visitClass(classTree, classTree.getSimpleName());
+      }
 
-          @Override
-          public @Nullable Void visitMethodInvocation(
-              MethodInvocationTree methodTree, Name className) {
-            if (GET_LOGGER_METHOD.matches(methodTree, state)) {
-              ExpressionTree arg = methodTree.getArguments().get(0);
-              String argumentName = SourceCode.treeToString(arg, state);
+      @Override
+      public @Nullable Void visitMethodInvocation(MethodInvocationTree methodTree, Name className) {
+        if (GET_LOGGER_METHOD.matches(methodTree, state)) {
+          ExpressionTree arg = methodTree.getArguments().get(0);
+          String argumentName = SourceCode.treeToString(arg, state);
 
-              if (!className.contentEquals(argumentName)) {
-                fixBuilder.merge(
-                    SuggestedFix.replace(arg, className + JavaFileObject.Kind.CLASS.extension));
-              }
-            }
-            return super.visitMethodInvocation(methodTree, className);
+          if (!className.contentEquals(
+              argumentName.substring(0, argumentName.indexOf(CLASS.extension)))) {
+            fixBuilder.merge(SuggestedFix.replace(arg, className + CLASS.extension));
           }
-        }.scan(typeDeclaration, null);
+        }
+        return super.visitMethodInvocation(methodTree, className);
       }
-    }
+    }.scan(tree, tree.getSimpleName());
   }
 
   private static String getCanonicalizedLoggerName(ErrorProneFlags flags) {

error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/Slf4jLogDeclarationTest.java

@@ -1,98 +1,110 @@
 package tech.picnic.errorprone.bugpatterns;
 
+import com.google.common.collect.ImmutableList;
 import com.google.errorprone.BugCheckerRefactoringTestHelper;
-import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
+import com.google.errorprone.CompilationTestHelper;
 import org.junit.jupiter.api.Test;
 
 final class Slf4jLogDeclarationTest {
   @Test
-  void replacement() {
-    BugCheckerRefactoringTestHelper.newInstance(Slf4jLogDeclaration.class, getClass())
-        .addInputLines(
+  void identification() {
+    CompilationTestHelper.newInstance(Slf4jLogDeclaration.class, getClass())
+        .addSourceLines(
             "A.java",
             "import org.slf4j.Logger;",
             "import org.slf4j.LoggerFactory;",
             "",
+            "// BUG: Diagnostic contains: SLF4J",
             "class A {",
-            "  Logger NOT_PROPER_LOGGER_NAME = LoggerFactory.getLogger(A.class);",
+            "  Logger FOO = LoggerFactory.getLogger(A.class);",
             "",
+            "  // BUG: Diagnostic contains: SLF4J",
             "  static class B {",
-            "    private Logger NOT_PROPER_LOGGER_NAME = LoggerFactory.getLogger(B.class);",
+            "    private Logger BAR = LoggerFactory.getLogger(B.class);",
             "  }",
             "",
+            "  // BUG: Diagnostic contains: SLF4J",
             "  static class C {",
-            "    private static Logger NOT_PROPER_LOGGER_NAME = LoggerFactory.getLogger(C.class);",
+            "    private static Logger BAZ = LoggerFactory.getLogger(C.class);",
             "  }",
             "",
+            "  // BUG: Diagnostic contains: SLF4J",
             "  static class D {",
-            "    static final Logger NOT_PROPER_LOGGER_NAME = LoggerFactory.getLogger(D.class);",
+            "    static final Logger QUX = LoggerFactory.getLogger(D.class);",
             "  }",
             "",
+            "  // BUG: Diagnostic contains: SLF4J",
             "  static class E {",
-            "    private final Logger NOT_PROPER_LOGGER_NAME = LoggerFactory.getLogger(E.class);",
+            "    private final Logger QUUX = LoggerFactory.getLogger(E.class);",
             "  }",
             "",
+            "  // BUG: Diagnostic contains: SLF4J",
             "  static class F {",
-            "    private static final Logger NOT_PROPER_LOGGER_NAME = LoggerFactory.getLogger(F.class);",
+            "    private static final Logger CORGE = LoggerFactory.getLogger(F.class);",
             "  }",
             "",
+            "  // BUG: Diagnostic contains: SLF4J",
             "  static class G {",
-            "    private static final Logger NOT_PROPER_LOGGER_NAME = LoggerFactory.getLogger(G.class);",
+            "    private static final Logger GRAPLY = LoggerFactory.getLogger(G.class);",
             "  }",
             "",
+            "  // BUG: Diagnostic contains: SLF4J",
             "  static class H {",
-            "    private static final Logger LOGGER_WITH_WRONG_CLASS_AS_ARGUMENT =",
-            "        LoggerFactory.getLogger(J.class);",
+            "    private static final Logger WALDO = LoggerFactory.getLogger(J.class);",
             "  }",
             "",
             "  class J {}",
             "",
+            "  // BUG: Diagnostic contains: SLF4J",
             "  interface K {",
-            "    Logger NOT_PROPER_LOGGER_NAME = LoggerFactory.getLogger(A.class);",
+            "    Logger FRED = LoggerFactory.getLogger(A.class);",
             "  }",
             "}")
+        .doTest();
+  }
+
+  @Test
+  void replacementWithDefaultCanonicalizedLoggerName() {
+    BugCheckerRefactoringTestHelper.newInstance(Slf4jLogDeclaration.class, getClass())
+        .addInputLines(
+            "A.java",
+            "import org.slf4j.Logger;",
+            "import org.slf4j.LoggerFactory;",
+            "",
+            "class A {",
+            "  Logger FOO = LoggerFactory.getLogger(A.class);",
+            "}")
         .addOutputLines(
             "A.java",
             "import org.slf4j.Logger;",
             "import org.slf4j.LoggerFactory;",
             "",
             "class A {",
             "  private static final Logger LOG = LoggerFactory.getLogger(A.class);",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  void replacementWithOverriddenCanonicalizedLoggerName() {
+    BugCheckerRefactoringTestHelper.newInstance(Slf4jLogDeclaration.class, getClass())
+        .setArgs(ImmutableList.of("-XepOpt:Slf4jLogDeclaration:CanonicalizedLoggerName=BAR"))
+        .addInputLines(
+            "A.java",
+            "import org.slf4j.Logger;",
+            "import org.slf4j.LoggerFactory;",
             "",
-            "  static class B {",
-            "    private static final Logger LOG = LoggerFactory.getLogger(B.class);",
-            "  }",
-            "",
-            "  static class C {",
-            "    private static final Logger LOG = LoggerFactory.getLogger(C.class);",
-            "  }",
-            "",
-            "  static class D {",
-            "    private static final Logger LOG = LoggerFactory.getLogger(D.class);",
-            "  }",
-            "",
-            "  static class E {",
-            "    private static final Logger LOG = LoggerFactory.getLogger(E.class);",
-            "  }",
-            "",
-            "  static class F {",
-            "    private static final Logger LOG = LoggerFactory.getLogger(F.class);",
-            "  }",
-            "",
-            "  static class G {",
-            "    private static final Logger LOG = LoggerFactory.getLogger(G.class);",
-            "  }",
-            "",
-            "  static class H {",
-            "    private static final Logger LOG = LoggerFactory.getLogger(H.class);",
-            "  }",
-            "",
-            "  class J {}",
+            "class A {",
+            "  Logger FOO = LoggerFactory.getLogger(A.class);",
+            "}")
+        .addOutputLines(
+            "A.java",
+            "import org.slf4j.Logger;",
+            "import org.slf4j.LoggerFactory;",
             "",
-            "  interface K {",
-            "    Logger LOG = LoggerFactory.getLogger(K.class);",
-            "  }",
+            "class A {",
+            "  private static final Logger BAR = LoggerFactory.getLogger(A.class);",
             "}")
-        .doTest(TestMode.TEXT_MATCH);
+        .doTest();
   }
 }