Variable created by ConstantPatternCompile should be private.

Also fix the spacing issue. Fixes #1655 flume hits : http://unknown commit RELNOTES : Variable created by ConstantPatternCompile should be private. Also fix the spacing issue. PiperOrigin-RevId: 322860215
google · Jul 23, 2020 · d4ad0a1 · d4ad0a1
1 parent e625571
commit d4ad0a1
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 11 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ConstantPatternCompile.java b/core/src/main/java/com/google/errorprone/bugpatterns/ConstantPatternCompile.java
@@ -107,12 +107,13 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
             .replace(pos, pos + varName.length(), upperUnderscoreVarName)
             .toString();
 
-    String modifiers = canUseStatic ? "static final " : "final ";
-    String variableTreeString = modifiers + variableReplacedString;
+    String modifiers = "private " + (canUseStatic ? "static " : "") + "final ";
+    String variableTreeString = "\n" + modifiers + variableReplacedString + "\n";
+
     SuggestedFix fix =
         SuggestedFix.builder()
             .merge(renameVariableOccurrences(tree, upperUnderscoreVarName, state))
-            .prefixWith(outerMethodTree, variableTreeString)
+            .postfixWith(outerMethodTree, variableTreeString)
             .delete(tree)
             .build();
     return describeMatch(tree, fix);

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/ConstantPatternCompileTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/ConstantPatternCompileTest.java
@@ -70,8 +70,8 @@ public void testFixGenerationStatic() {
             "class Test {",
             "  private static final String MY_COOL_PATTERN = \"a+\";",
             "  public static void myPopularStaticMethod() {",
-            "    Pattern attern = Pattern.compile(MY_COOL_PATTERN);",
-            "    Matcher m = attern.matcher(\"aaaaab\");",
+            "    Pattern somePattern = Pattern.compile(MY_COOL_PATTERN);",
+            "    Matcher m = somePattern.matcher(\"aaaaab\");",
             "  }",
             "}")
         .addOutputLines(
@@ -80,10 +80,38 @@ public void testFixGenerationStatic() {
             "import java.util.regex.Pattern;",
             "class Test {",
             "  private static final String MY_COOL_PATTERN = \"a+\";",
-            "  static final Pattern ATTERN = Pattern.compile(MY_COOL_PATTERN);",
             "  public static void myPopularStaticMethod() {",
-            "    Matcher m = ATTERN.matcher(\"aaaaab\");",
+            "    Matcher m = SOME_PATTERN.matcher(\"aaaaab\");",
             "  }",
+            "  private static final Pattern SOME_PATTERN = Pattern.compile(MY_COOL_PATTERN);",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void testFixGenerationWithJavadoc() {
+    testHelper
+        .addInputLines(
+            "in/Test.java",
+            "import java.util.regex.Matcher;",
+            "import java.util.regex.Pattern;",
+            "class Test {",
+            "  /** This is a javadoc. **/ ",
+            "  public static void myPopularStaticMethod() {",
+            "    Pattern myPattern = Pattern.compile(\"a+\");",
+            "    Matcher m = myPattern.matcher(\"aaaaab\");",
+            "  }",
+            "}")
+        .addOutputLines(
+            "in/Test.java",
+            "import java.util.regex.Matcher;",
+            "import java.util.regex.Pattern;",
+            "class Test {",
+            "  /** This is a javadoc. **/ ",
+            "  public static void myPopularStaticMethod() {",
+            "    Matcher m = MY_PATTERN.matcher(\"aaaaab\");",
+            "  }",
+            "  private static final Pattern MY_PATTERN = Pattern.compile(\"a+\");",
             "}")
         .doTest();
   }
@@ -99,8 +127,8 @@ public void testFixGeneration_nonStaticInnerClass() {
             "  private static final String MY_COOL_PATTERN = \"a+\";",
             "  class Inner {",
             "  public void myPopularStaticMethod() {",
-            "      Pattern attern = Pattern.compile(MY_COOL_PATTERN);",
-            "      Matcher m = attern.matcher(\"aaaaab\");",
+            "      Pattern myPattern = Pattern.compile(MY_COOL_PATTERN);",
+            "      Matcher m = myPattern.matcher(\"aaaaab\");",
             "    }",
             "  }",
             "}")
@@ -111,10 +139,10 @@ public void testFixGeneration_nonStaticInnerClass() {
             "class Test {",
             "  private static final String MY_COOL_PATTERN = \"a+\";",
             "  class Inner {",
-            "  final Pattern ATTERN = Pattern.compile(MY_COOL_PATTERN);",
             "  public void myPopularStaticMethod() {",
-            "      Matcher m = ATTERN.matcher(\"aaaaab\");",
+            "      Matcher m = MY_PATTERN.matcher(\"aaaaab\");",
             "    }",
+            "  private final Pattern MY_PATTERN = Pattern.compile(MY_COOL_PATTERN);",
             "  }",
             "}")
         .doTest();