diff --git a/java/piranha/src/main/java/com/uber/piranha/XPFlagCleaner.java b/java/piranha/src/main/java/com/uber/piranha/XPFlagCleaner.java index 508836fbb..620b7109b 100644 --- a/java/piranha/src/main/java/com/uber/piranha/XPFlagCleaner.java +++ b/java/piranha/src/main/java/com/uber/piranha/XPFlagCleaner.java @@ -66,6 +66,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.regex.Pattern; import javax.annotation.Nullable; import javax.lang.model.element.ElementKind; @@ -808,8 +809,30 @@ public Description matchVariable(VariableTree tree, VisitorState state) { // trailing "," if present on the parent. String enumAsStr = state.getSourceForNode(state.getPath().getParentPath().getLeaf()); String varAsStrWithComma = tree.getName().toString() + ","; - if (enumAsStr.contains(varAsStrWithComma)) { + String varAsStrWithSemiColon = tree.getName().toString() + ";"; + if (Pattern.compile("\\b" + varAsStrWithComma).matcher(enumAsStr).find()) { return buildDescription(tree).addFix(SuggestedFix.replace(tree, "", 0, 1)).build(); + } else if (Pattern.compile("\\b" + varAsStrWithSemiColon).matcher(enumAsStr).find()) { + // Complicated calculation to remove backwards from this enum field, pushing the ';' back on + // the previous enum field, if any. Otherwise, delete the full statement. + final int varTreeStartIdx = enumAsStr.indexOf(varAsStrWithSemiColon); + int deleteStartIdx = varTreeStartIdx; + char c = enumAsStr.charAt(deleteStartIdx); + while (c != ',' && c != ';' && c != '{') { + deleteStartIdx -= 1; + c = enumAsStr.charAt(deleteStartIdx); + } + // negative offset up to and including the comma, bracket or semicolon + int offset = deleteStartIdx - varTreeStartIdx; + if (c == ';' || c == '{') { + // Don't delete the previous bracket or semicolon, but do delete the trailing semicolon + return buildDescription(tree) + .addFix(SuggestedFix.replace(tree, "", offset + 1, 1)) + .build(); + } else { + // Previous separator is a comma, erase it and push back the `;` + return buildDescription(tree).addFix(SuggestedFix.replace(tree, "", offset, 0)).build(); + } } else { // Fallback for single/last enum variable detection return buildDescription(tree).addFix(SuggestedFix.delete(tree)).build(); diff --git a/java/piranha/src/test/java/com/uber/piranha/CorePiranhaTest.java b/java/piranha/src/test/java/com/uber/piranha/CorePiranhaTest.java index f63e2bbc3..3d9f6f84b 100644 --- a/java/piranha/src/test/java/com/uber/piranha/CorePiranhaTest.java +++ b/java/piranha/src/test/java/com/uber/piranha/CorePiranhaTest.java @@ -499,4 +499,36 @@ public void testIgnoresPrefixMatchFlag() throws IOException { "class TestClassPartialMatch { }") .doTest(); } + + @Test + public void testEnumWithTrailingSemicolon() throws IOException { + + ErrorProneFlags.Builder b = ErrorProneFlags.builder(); + b.putFlag("Piranha:FlagName", "STALE_FLAG"); + b.putFlag("Piranha:IsTreated", "false"); + b.putFlag("Piranha:Config", "config/properties.json"); + + BugCheckerRefactoringTestHelper bcr = + BugCheckerRefactoringTestHelper.newInstance(new XPFlagCleaner(b.build()), getClass()); + + bcr = bcr.setArgs("-d", temporaryFolder.getRoot().getAbsolutePath()); + + bcr = PiranhaTestingHelpers.addHelperClasses(bcr); + bcr.addInputLines( + "TestExperimentName.java", + "package com.uber.piranha;", + "public enum TestExperimentName {", + " OTHER_FLAG,", + " STALE_FLAG;", + " public void foo() {}", + "}") + .addOutputLines( + "TestExperimentName.java", + "package com.uber.piranha;", + "public enum TestExperimentName {", + " OTHER_FLAG;", + " public void foo() {}", + "}") + .doTest(); + } }