From d4ad0a186bc2ff699797982537d30ddd791d53dc Mon Sep 17 00:00:00 2001 From: Sumit Bhagwani Date: Thu, 23 Jul 2020 13:58:11 -0700 Subject: [PATCH] Variable created by ConstantPatternCompile should be private. Also fix the spacing issue. Fixes https://github.com/google/error-prone/issues/1655 flume hits : http://unknown commit RELNOTES : Variable created by ConstantPatternCompile should be private. Also fix the spacing issue. PiperOrigin-RevId: 322860215 --- .../bugpatterns/ConstantPatternCompile.java | 7 +-- .../ConstantPatternCompileTest.java | 44 +++++++++++++++---- 2 files changed, 40 insertions(+), 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 index d6353d9b493b..1c991451e7f3 100644 --- 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 index ea22d4cf0dba..09baa1463799 100644 --- 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();