diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringEscape.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringEscape.java new file mode 100644 index 00000000..f699c3d6 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringEscape.java @@ -0,0 +1,95 @@ +package tech.picnic.errorprone.bugpatterns; + +import static com.google.errorprone.BugPattern.LinkType.CUSTOM; +import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; +import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION; +import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.LiteralTreeMatcher; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.LiteralTree; +import tech.picnic.errorprone.utils.SourceCode; + +/** A {@link BugChecker} that flags string constants with extraneous escaping. */ +// XXX: Also cover `\"` sequences inside text blocks. Note that this requires a more subtle +// approach, as double-quote characters will need to remain escaped if removing the backslash would +// create a new sequence of three or more double-quotes. (TBD whether we'd like to enforce a +// "preferred" approach to escaping, e.g. by always escaping the last of a triplet, such that the +// over-all number of escaped characters is minimized.) +// XXX: Also flag `'\"'` char literals. +@AutoService(BugChecker.class) +@BugPattern( + summary = "Inside string expressions single quotes do not need to be escaped", + link = BUG_PATTERNS_BASE_URL + "RedundantStringEscape", + linkType = CUSTOM, + severity = SUGGESTION, + tags = SIMPLIFICATION) +public final class RedundantStringEscape extends BugChecker implements LiteralTreeMatcher { + private static final long serialVersionUID = 1L; + + /** Instantiates a new {@link RedundantStringEscape} instance. */ + public RedundantStringEscape() {} + + @Override + public Description matchLiteral(LiteralTree tree, VisitorState state) { + String constant = ASTHelpers.constValue(tree, String.class); + if (constant == null || constant.indexOf('\'') < 0) { + /* Fast path: this isn't a string constant with a single quote. */ + return Description.NO_MATCH; + } + + String source = SourceCode.treeToString(tree, state); + if (!containsBannedEscapeSequence(source)) { + /* Semi-fast path: this expression doesn't contain an escaped single quote. */ + return Description.NO_MATCH; + } + + /* Slow path: suggest dropping the escape characters. */ + return describeMatch(tree, SuggestedFix.replace(tree, dropRedundantEscapeSequences(source))); + } + + /** + * Tells whether the given string constant source expression contains an escaped single quote. + * + * @implNote As the input is a literal Java string expression, it will start and end with a double + * quote; as such any found backslash will not be the string's final character. + */ + private static boolean containsBannedEscapeSequence(String source) { + for (int p = source.indexOf('\\'); p != -1; p = source.indexOf('\\', p + 2)) { + if (source.charAt(p + 1) == '\'') { + return true; + } + } + + return false; + } + + /** + * Simplifies the given string constant source expression by dropping the backslash preceding an + * escaped single quote. + * + * @implNote Note that this method does not delegate to {@link + * SourceCode#toStringConstantExpression}, as that operation may replace other Unicode + * characters with their associated escape sequence. + * @implNote As the input is a literal Java string expression, it will start and end with a double + * quote; as such any found backslash will not be the string's final character. + */ + private static String dropRedundantEscapeSequences(String source) { + StringBuilder result = new StringBuilder(); + + for (int p = 0; p < source.length(); p++) { + char c = source.charAt(p); + if (c != '\\' || source.charAt(p + 1) != '\'') { + result.append(c); + } + } + + return result.toString(); + } +} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/Slf4jLogStatement.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/Slf4jLogStatement.java index ccf73b82..7836961f 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/Slf4jLogStatement.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/Slf4jLogStatement.java @@ -41,7 +41,7 @@ import tech.picnic.errorprone.utils.SourceCode; tags = LIKELY_ERROR) public final class Slf4jLogStatement extends BugChecker implements MethodInvocationTreeMatcher { private static final long serialVersionUID = 1L; - private static final Matcher MARKER = isSubtypeOf("org.slf4j.Marker"); + private static final Matcher SLF4J_MARKER = isSubtypeOf("org.slf4j.Marker"); private static final Matcher THROWABLE = isSubtypeOf(Throwable.class); private static final Matcher SLF4J_LOGGER_INVOCATION = instanceMethod() @@ -71,7 +71,7 @@ public final class Slf4jLogStatement extends BugChecker implements MethodInvocat * SLF4J log statements may accept a "marker" as a first argument, before the format string. * We ignore such markers. */ - int lTrim = MARKER.matches(args.get(0), state) ? 1 : 0; + int lTrim = SLF4J_MARKER.matches(args.get(0), state) ? 1 : 0; /* * SLF4J treats the final argument to a log statement specially if it is a `Throwabe`: it * will always choose to render the associated stacktrace, even if the argument has a diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StringJoin.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StringJoin.java index 4ab098b3..99b3380d 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StringJoin.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StringJoin.java @@ -23,7 +23,6 @@ import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.tools.javac.code.Type; -import com.sun.tools.javac.util.Constants; import java.util.Formattable; import java.util.Iterator; import java.util.List; @@ -150,7 +149,7 @@ public final class StringJoin extends BugChecker implements MethodInvocationTree SuggestedFix.Builder fix = SuggestedFix.builder() .replace(tree.getMethodSelect(), "String.join") - .replace(arguments.next(), Constants.format(separator)); + .replace(arguments.next(), SourceCode.toStringConstantExpression(separator, state)); while (arguments.hasNext()) { ExpressionTree argument = arguments.next(); diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java index 9ce8236f..76c3dcf0 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/BugCheckerRules.java @@ -11,6 +11,7 @@ import com.sun.tools.javac.util.Constants; import com.sun.tools.javac.util.Convert; import javax.lang.model.element.Name; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; +import tech.picnic.errorprone.utils.SourceCode; /** Refaster rules related to {@link com.google.errorprone.bugpatterns.BugChecker} classes. */ @OnlineDocumentation @@ -56,16 +57,26 @@ final class BugCheckerRules { } } - /** Prefer using the {@link Constants} API over more verbose alternatives. */ + /** + * Prefer {@link SourceCode#toStringConstantExpression(Object, + * com.google.errorprone.VisitorState)} over alternatives that unnecessarily escape single quote + * characters. + */ static final class ConstantsFormat { + @BeforeTemplate + String before(CharSequence value) { + return Constants.format(value); + } + @BeforeTemplate String before(String value) { return String.format("\"%s\"", Convert.quote(value)); } @AfterTemplate - String after(String value) { - return Constants.format(value); + String after(CharSequence value) { + return SourceCode.toStringConstantExpression( + value, Refaster.emitCommentBefore("REPLACEME", null)); } } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RedundantStringEscapeTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RedundantStringEscapeTest.java new file mode 100644 index 00000000..c3f21511 --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RedundantStringEscapeTest.java @@ -0,0 +1,90 @@ +package tech.picnic.errorprone.bugpatterns; + +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; +import com.google.errorprone.CompilationTestHelper; +import org.junit.jupiter.api.Test; + +final class RedundantStringEscapeTest { + @Test + void identification() { + CompilationTestHelper.newInstance(RedundantStringEscape.class, getClass()) + .addSourceLines( + "A.java", + "import java.util.Arrays;", + "import java.util.List;", + "", + "class A {", + " List m() {", + " return Arrays.asList(", + " \"foo\",", + " \"ß\",", + " \"'\",", + " \"\\\"\",", + " \"\\\\\",", + " \"\\\\'\",", + " \"'\\\\\",", + " // BUG: Diagnostic contains:", + " \"\\\\\\'\",", + " // BUG: Diagnostic contains:", + " \"\\'\\\\\",", + " // BUG: Diagnostic contains:", + " \"\\'\",", + " // BUG: Diagnostic contains:", + " \"'\\'\",", + " // BUG: Diagnostic contains:", + " \"\\''\",", + " // BUG: Diagnostic contains:", + " \"\\'\\'\",", + " (", + " // BUG: Diagnostic contains:", + " /* Leading comment. */ \"\\'\" /* Trailing comment. */),", + " // BUG: Diagnostic contains:", + " \"\\'foo\\\"bar\\'baz\\\"qux\\'\");", + " }", + "}") + .doTest(); + } + + @Test + void replacement() { + BugCheckerRefactoringTestHelper.newInstance(RedundantStringEscape.class, getClass()) + .addInputLines( + "A.java", + "import java.util.Arrays;", + "import java.util.List;", + "", + "class A {", + " List m() {", + " return Arrays.asList(", + " \"\\'\",", + " \"'\\'\",", + " \"\\''\",", + " \"\\'\\'\",", + " \"\\'ß\\'\",", + " (", + " /* Leading comment. */ \"\\'\" /* Trailing comment. */),", + " \"\\'foo\\\"bar\\'baz\\\"qux\\'\");", + " }", + "}") + .addOutputLines( + "A.java", + "import java.util.Arrays;", + "import java.util.List;", + "", + "class A {", + " List m() {", + " return Arrays.asList(", + " \"'\",", + " \"''\",", + " \"''\",", + " \"''\",", + " \"'ß'\",", + " (", + " /* Leading comment. */ \"'\" /* Trailing comment. */),", + " \"'foo\\\"bar'baz\\\"qux'\");", + " }", + "}") + .doTest(TestMode.TEXT_MATCH); + } +} diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestInput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestInput.java index 6d1ac1db..38166665 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestInput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestInput.java @@ -4,6 +4,7 @@ import com.google.common.collect.ImmutableSet; import com.google.errorprone.BugCheckerRefactoringTestHelper; import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; import com.google.errorprone.bugpatterns.BugChecker; +import com.sun.tools.javac.util.Constants; import com.sun.tools.javac.util.Convert; import javax.lang.model.element.Name; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; @@ -11,7 +12,7 @@ import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; final class BugCheckerRulesTest implements RefasterRuleCollectionTestCase { @Override public ImmutableSet elidedTypesAndStaticImports() { - return ImmutableSet.of(Convert.class, FixChoosers.class); + return ImmutableSet.of(Constants.class, Convert.class, FixChoosers.class); } ImmutableSet testBugCheckerRefactoringTestHelperIdentity() { @@ -29,8 +30,8 @@ final class BugCheckerRulesTest implements RefasterRuleCollectionTestCase { .addOutputLines("A.java", "class A {}"); } - String testConstantsFormat() { - return String.format("\"%s\"", Convert.quote("foo")); + ImmutableSet testConstantsFormat() { + return ImmutableSet.of(Constants.format("foo"), String.format("\"%s\"", Convert.quote("bar"))); } ImmutableSet testNameContentEquals() { diff --git a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestOutput.java b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestOutput.java index 3227c420..524cf36f 100644 --- a/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestOutput.java +++ b/error-prone-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/BugCheckerRulesTestOutput.java @@ -8,11 +8,12 @@ import com.sun.tools.javac.util.Constants; import com.sun.tools.javac.util.Convert; import javax.lang.model.element.Name; import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; +import tech.picnic.errorprone.utils.SourceCode; final class BugCheckerRulesTest implements RefasterRuleCollectionTestCase { @Override public ImmutableSet elidedTypesAndStaticImports() { - return ImmutableSet.of(Convert.class, FixChoosers.class); + return ImmutableSet.of(Constants.class, Convert.class, FixChoosers.class); } ImmutableSet testBugCheckerRefactoringTestHelperIdentity() { @@ -28,8 +29,10 @@ final class BugCheckerRulesTest implements RefasterRuleCollectionTestCase { .expectUnchanged(); } - String testConstantsFormat() { - return Constants.format("foo"); + ImmutableSet testConstantsFormat() { + return ImmutableSet.of( + SourceCode.toStringConstantExpression("foo", /* REPLACEME */ null), + SourceCode.toStringConstantExpression("bar", /* REPLACEME */ null)); } ImmutableSet testNameContentEquals() { diff --git a/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/BugPatternLink.java b/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/BugPatternLink.java index 3d050b2e..b6274bd0 100644 --- a/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/BugPatternLink.java +++ b/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/BugPatternLink.java @@ -30,8 +30,8 @@ import com.sun.source.tree.BinaryTree; import com.sun.source.tree.ClassTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.Tree.Kind; -import com.sun.tools.javac.util.Constants; import javax.lang.model.element.Name; +import tech.picnic.errorprone.utils.SourceCode; /** * A {@link BugChecker} that flags {@link BugChecker} declarations inside {@code @@ -126,7 +126,9 @@ public final class BugPatternLink extends BugChecker implements ClassTreeMatcher state, "link", ImmutableList.of( - linkPrefix + " + " + Constants.format(tree.getSimpleName().toString())))); + linkPrefix + + " + " + + SourceCode.toStringConstantExpression(tree.getSimpleName(), state)))); String linkType = SuggestedFixes.qualifyStaticImport( diff --git a/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/ErrorProneRuntimeClasspath.java b/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/ErrorProneRuntimeClasspath.java index 1a13aba9..fb19c4dd 100644 --- a/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/ErrorProneRuntimeClasspath.java +++ b/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/ErrorProneRuntimeClasspath.java @@ -26,8 +26,8 @@ import com.sun.source.tree.LiteralTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Symbol.ClassSymbol; -import com.sun.tools.javac.util.Constants; import java.util.regex.Pattern; +import tech.picnic.errorprone.utils.SourceCode; import tech.picnic.errorprone.utils.ThirdPartyLibrary; /** @@ -123,7 +123,8 @@ public final class ErrorProneRuntimeClasspath extends BugChecker .setMessage("This type may not be on the runtime classpath; use a string literal instead") .addFix( SuggestedFix.replace( - tree, Constants.format(receiver.owner.getQualifiedName().toString()))) + tree, + SourceCode.toStringConstantExpression(receiver.owner.getQualifiedName(), state))) .build(); } @@ -150,7 +151,9 @@ public final class ErrorProneRuntimeClasspath extends BugChecker original, identifier + ".class.getCanonicalName()" - + (suffix.isEmpty() ? "" : (" + " + Constants.format(suffix)))) + + (suffix.isEmpty() + ? "" + : (" + " + SourceCode.toStringConstantExpression(suffix, state)))) .build(); } diff --git a/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/ExhaustiveRefasterTypeMigration.java b/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/ExhaustiveRefasterTypeMigration.java index 9874b0f4..ada6b19c 100644 --- a/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/ExhaustiveRefasterTypeMigration.java +++ b/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/ExhaustiveRefasterTypeMigration.java @@ -38,7 +38,6 @@ import com.sun.tools.javac.code.Attribute; import com.sun.tools.javac.code.Symbol.ClassSymbol; import com.sun.tools.javac.code.Symbol.MethodSymbol; import com.sun.tools.javac.code.Symbol.TypeSymbol; -import com.sun.tools.javac.util.Constants; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; @@ -49,6 +48,7 @@ import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.AnnotationValue; import javax.lang.model.element.Modifier; import org.jspecify.annotations.Nullable; +import tech.picnic.errorprone.utils.SourceCode; /** * A {@link BugChecker} that validates the claim made by {@link @@ -129,7 +129,9 @@ public final class ExhaustiveRefasterTypeMigration extends BugChecker implements migrationAnnotation, state, TYPE_MIGRATION_UNMIGRATED_METHODS_ELEMENT, - unmigratedMethods.stream().map(Constants::format).collect(toImmutableList())) + unmigratedMethods.stream() + .map(m -> SourceCode.toStringConstantExpression(m, state)) + .collect(toImmutableList())) .build()); } diff --git a/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java b/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java index 824b8649..6d29e961 100644 --- a/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java +++ b/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java @@ -42,6 +42,24 @@ public final class SourceCode { return src != null ? src : tree.toString(); } + /** + * Returns a Java string constant expression (i.e., a quoted string) representing the given input. + * + * @param value The value of interest. + * @param state A {@link VisitorState} describing the context in which the given {@link Tree} is + * found. + * @return A non-{@code null} string. + * @apiNote This method differs from {@link com.sun.tools.javac.util.Constants#format(Object)} in + * that it does not superfluously escape single quote characters. It is different from {@link + * VisitorState#getConstantExpression(Object)} in that it is more performant and accepts any + * {@link CharSequence} instance. + */ + // XXX: Drop this method if https://github.com/google/error-prone/pull/4586 is merged and released + // with the proposed `CharSequence` compatibility change. + public static String toStringConstantExpression(Object value, VisitorState state) { + return state.getConstantExpression(value instanceof CharSequence ? value.toString() : value); + } + /** * Creates a {@link SuggestedFix} for the deletion of the given {@link Tree}, including any * whitespace that follows it. diff --git a/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/SourceCodeTest.java b/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/SourceCodeTest.java index 0b61a054..ed4b73d9 100644 --- a/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/SourceCodeTest.java +++ b/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/SourceCodeTest.java @@ -8,18 +8,49 @@ import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker; import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher; +import com.google.errorprone.bugpatterns.BugChecker.LiteralTreeMatcher; import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher; +import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher; +import com.google.errorprone.fixes.SuggestedFix; import com.google.errorprone.matchers.Description; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.AnnotationTree; +import com.sun.source.tree.LiteralTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; import com.sun.source.tree.Tree; +import com.sun.source.tree.VariableTree; +import java.util.Optional; import javax.lang.model.element.Name; import org.junit.jupiter.api.Test; final class SourceCodeTest { + @Test + void toStringConstantExpression() { + BugCheckerRefactoringTestHelper.newInstance( + ToStringConstantExpressionTestChecker.class, getClass()) + .addInputLines( + "A.java", + "class A {", + " String m() {", + " char a = 'c';", + " char b = '\\'';", + " return \"foo\\\"bar\\'baz\\bqux\";", + " }", + "}") + .addOutputLines( + "A.java", + "class A {", + " String m() {", + " char a = 'c' /* 'c' */; /* \"a\" */", + " char b = '\\'' /* '\\'' */; /* \"b\" */", + " return \"foo\\\"bar\\'baz\\bqux\" /* \"foo\\\"bar'baz\\bqux\" */;", + " }", + "}") + .doTest(TestMode.TEXT_MATCH); + } + @Test void deleteWithTrailingWhitespaceAnnotations() { BugCheckerRefactoringTestHelper.newInstance( @@ -228,6 +259,41 @@ final class SourceCodeTest { .doTest(TestMode.TEXT_MATCH); } + /** + * A {@link BugChecker} that applies {@link SourceCode#toStringConstantExpression(Object, + * VisitorState)} to string literals. + */ + @BugPattern(severity = ERROR, summary = "Interacts with `SourceCode` for testing purposes") + public static final class ToStringConstantExpressionTestChecker extends BugChecker + implements LiteralTreeMatcher, VariableTreeMatcher { + private static final long serialVersionUID = 1L; + + @Override + public Description matchLiteral(LiteralTree tree, VisitorState state) { + // XXX: The character conversion is a workaround for the fact that `ASTHelpers#constValue` + // returns an `Integer` value for `char` constants. + return Optional.ofNullable(ASTHelpers.constValue(tree)) + .map( + constant -> + ASTHelpers.isSubtype(ASTHelpers.getType(tree), state.getSymtab().charType, state) + ? (char) (int) constant + : constant) + .map(constant -> describeMatch(tree, addComment(tree, constant, state))) + .orElse(Description.NO_MATCH); + } + + @Override + public Description matchVariable(VariableTree tree, VisitorState state) { + return describeMatch( + tree, addComment(tree, ASTHelpers.getSymbol(tree).getSimpleName(), state)); + } + + private static SuggestedFix addComment(Tree tree, Object value, VisitorState state) { + return SuggestedFix.postfixWith( + tree, "/* %s */".formatted(SourceCode.toStringConstantExpression(value, state))); + } + } + /** * A {@link BugChecker} that uses {@link SourceCode#deleteWithTrailingWhitespace(Tree, * VisitorState)} to suggest the deletion of annotations and methods with a name containing