Address comments: Conditionally include public constant fields

PicnicSupermarket · Sep 23, 2023 · 42a7575 · 42a7575
1 parent f6bb469
commit 42a7575
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 12 deletions.
diff --git a/...-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantFieldName.java b/...-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantFieldName.java
@@ -39,15 +39,20 @@
  *
  * <ul>
  *   <li>private static final int number = 1;
- *   <li>static final int otherNumber = 2;
  * </ul>
  *
  * <p>To the following:
  *
  * <ul>
  *   <li>private static final int NUMBER = 1;
- *   <li>static final int OTHER_NUMBER = 2;
  * </ul>
+ *
+ * @apiNote This {@link BugChecker checker} has two optional flags:
+ *     <ul>
+ *       <li>ExcludedConstantFliedNames: A list of field names to exclude from this check.
+ *       <li>IncludePublicConstantFields: Whether to include public constants when running this
+ *           check.
+ *     </ul>
  */
 @AutoService(BugChecker.class)
 @BugPattern(
@@ -61,13 +66,16 @@ public final class CanonicalConstantFieldName extends BugChecker implements Vari
   private static final long serialVersionUID = 1L;
   private static final Matcher<Tree> IS_CONSTANT =
       allOf(hasModifier(Modifier.STATIC), hasModifier(Modifier.FINAL));
+  private static final Matcher<Tree> IS_PRIVATE = hasModifier(Modifier.PRIVATE);
   private static final Pattern TO_SNAKE_CASE = Pattern.compile("([a-z])([A-Z])");
   private static final ImmutableSet<String> DEFAULT_EXCLUDED_CONSTANT_FIELD_NAMES =
       ImmutableSet.of("serialVersionUID");
   private static final String EXCLUDED_CONSTANT_FIELD_NAMES =
       "CanonicalConstantFieldName:ExcludedConstantFliedNames";
-
+  private static final String IS_INCLUDE_PUBLIC_CONSTANT_FIELDS =
+      "CanonicalConstantFieldName:IncludePublicConstantFields";
   private final ImmutableList<String> optionalExcludedConstantFliedNames;
+  private final boolean includePublicConstantFieldNames;
 
   /** Instantiates a default {@link CanonicalConstantFieldName} instance. */
   public CanonicalConstantFieldName() {
@@ -82,11 +90,12 @@ public CanonicalConstantFieldName() {
   @Inject
   CanonicalConstantFieldName(ErrorProneFlags flags) {
     optionalExcludedConstantFliedNames = getCanonicalizedLoggerName(flags);
+    includePublicConstantFieldNames = getIncludePrivateConstantFieldNames(flags);
   }
 
   @Override
   public Description matchVariable(VariableTree tree, VisitorState state) {
-    if (!IS_CONSTANT.matches(tree, state)) {
+    if (!(IS_CONSTANT.matches(tree, state) && isFieldAccessModifierApplicable(tree, state))) {
       return Description.NO_MATCH;
     }
     SuggestedFix.Builder fixBuilder = SuggestedFix.builder();
@@ -101,6 +110,10 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
     return fixBuilder.isEmpty() ? Description.NO_MATCH : describeMatch(tree, fixBuilder.build());
   }
 
+  private boolean isFieldAccessModifierApplicable(VariableTree tree, VisitorState state) {
+    return includePublicConstantFieldNames || IS_PRIVATE.matches(tree, state);
+  }
+
   private static boolean isVariableUpperSnakeCase(String variableName) {
     return variableName.equals(toUpperSnakeCase(variableName));
   }
@@ -117,4 +130,8 @@ private static String toUpperSnakeCase(String variableName) {
   private static ImmutableList<String> getCanonicalizedLoggerName(ErrorProneFlags flags) {
     return Flags.getList(flags, EXCLUDED_CONSTANT_FIELD_NAMES);
   }
+
+  private static boolean getIncludePrivateConstantFieldNames(ErrorProneFlags flags) {
+    return flags.getBoolean(IS_INCLUDE_PUBLIC_CONSTANT_FIELDS).orElse(false);
+  }
 }
diff --git a/...trib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantFieldNameTest.java b/...trib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantFieldNameTest.java
@@ -13,10 +13,6 @@ void replacement() {
             "class A {",
             "  private static final int number = 1;",
             "",
-            "  static final int otherNumber = 2;",
-            "",
-            "  static final int ANOTHER_NUMBER = 3;",
-            "",
             "  static int getNumber() {",
             "    return number;",
             "  }",
@@ -26,10 +22,6 @@ void replacement() {
             "class A {",
             "  private static final int NUMBER = 1;",
             "",
-            "  static final int OTHER_NUMBER = 2;",
-            "",
-            "  static final int ANOTHER_NUMBER = 3;",
-            "",
             "  static int getNumber() {",
             "    return NUMBER;",
             "  }",