Suggest inlining trivial constants like EMPTY_STRING

PiperOrigin-RevId: 539687077

core/src/main/java/com/google/errorprone/bugpatterns/InlineTrivialConstant.java

@@ -0,0 +1,133 @@
+/*
+ * Copyright 2023 The Error Prone Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.errorprone.bugpatterns;
+
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.util.ASTHelpers.getSymbol;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ListMultimap;
+import com.google.common.collect.MultimapBuilder;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker.CompilationUnitTreeMatcher;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.CompilationUnitTree;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.IdentifierTree;
+import com.sun.source.tree.LiteralTree;
+import com.sun.source.tree.MemberSelectTree;
+import com.sun.source.tree.Tree;
+import com.sun.source.tree.VariableTree;
+import com.sun.source.util.TreePathScanner;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symbol.VarSymbol;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import javax.lang.model.element.ElementKind;
+import javax.lang.model.element.Modifier;
+
+/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
+@BugPattern(summary = "Consider inlining this constant", severity = WARNING)
+public class InlineTrivialConstant extends BugChecker implements CompilationUnitTreeMatcher {
+
+  @AutoValue
+  abstract static class TrivialConstant {
+    abstract VariableTree tree();
+
+    abstract String replacement();
+
+    static TrivialConstant create(VariableTree tree, String replacement) {
+      return new AutoValue_InlineTrivialConstant_TrivialConstant(tree, replacement);
+    }
+  }
+
+  @Override
+  public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) {
+    Map<VarSymbol, TrivialConstant> fields = new HashMap<>();
+    new SuppressibleTreePathScanner<Void, Void>(state) {
+      @Override
+      public Void visitVariable(VariableTree tree, Void unused) {
+        VarSymbol sym = getSymbol(tree);
+        isTrivialConstant(tree, sym, state)
+            .ifPresent(r -> fields.put(sym, TrivialConstant.create(tree, r)));
+        return super.visitVariable(tree, null);
+      }
+    }.scan(state.getPath(), null);
+    ListMultimap<Symbol, Tree> uses = MultimapBuilder.hashKeys().arrayListValues().build();
+    new TreePathScanner<Void, Void>() {
+      @Override
+      public Void visitMemberSelect(MemberSelectTree tree, Void unused) {
+        handle(tree);
+        return super.visitMemberSelect(tree, null);
+      }
+
+      @Override
+      public Void visitIdentifier(IdentifierTree tree, Void unused) {
+        handle(tree);
+        return super.visitIdentifier(tree, null);
+      }
+
+      private void handle(Tree tree) {
+        Symbol sym = getSymbol(tree);
+        if (sym == null) {
+          return;
+        }
+        if (fields.containsKey(sym)) {
+          uses.put(sym, tree);
+        }
+      }
+    }.scan(state.getPath(), null);
+    for (Map.Entry<VarSymbol, TrivialConstant> e : fields.entrySet()) {
+      SuggestedFix.Builder fix = SuggestedFix.builder();
+      TrivialConstant value = e.getValue();
+      fix.delete(value.tree());
+      uses.get(e.getKey()).forEach(x -> fix.replace(x, value.replacement()));
+      state.reportMatch(describeMatch(value.tree(), fix.build()));
+    }
+    return NO_MATCH;
+  }
+
+  private static Optional<String> isTrivialConstant(
+      VariableTree tree, VarSymbol sym, VisitorState state) {
+    if (!(sym.getKind().equals(ElementKind.FIELD)
+        && sym.getModifiers().contains(Modifier.PRIVATE)
+        && sym.isStatic()
+        && sym.getModifiers().contains(Modifier.FINAL))) {
+      return Optional.empty();
+    }
+    if (!tree.getName().contentEquals("EMPTY_STRING")) {
+      return Optional.empty();
+    }
+    if (!ASTHelpers.isSameType(sym.asType(), state.getSymtab().stringType, state)) {
+      return Optional.empty();
+    }
+    ExpressionTree initializer = tree.getInitializer();
+    if (initializer.getKind().equals(Tree.Kind.STRING_LITERAL)) {
+      String value = (String) ((LiteralTree) initializer).getValue();
+      if (Objects.equals(value, "")) {
+        return Optional.of("\"\"");
+      }
+    }
+    return Optional.empty();
+  }
+}

core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java

@@ -172,6 +172,7 @@
 import com.google.errorprone.bugpatterns.InfiniteRecursion;
 import com.google.errorprone.bugpatterns.InitializeInline;
 import com.google.errorprone.bugpatterns.InjectOnBugCheckers;
+import com.google.errorprone.bugpatterns.InlineTrivialConstant;
 import com.google.errorprone.bugpatterns.InputStreamSlowMultibyteRead;
 import com.google.errorprone.bugpatterns.InsecureCipherMode;
 import com.google.errorprone.bugpatterns.InstanceOfAndCastMatchWrongType;
@@ -890,6 +891,7 @@ public static ScannerSupplier errorChecks() {
           InjectOnConstructorOfAbstractClass.class,
           InjectedConstructorAnnotations.class,
           InlineFormatString.class,
+          InlineTrivialConstant.class,
           Inliner.class,
           InputStreamSlowMultibyteRead.class,
           InstanceOfAndCastMatchWrongType.class,

core/src/test/java/com/google/errorprone/bugpatterns/InlineTrivialConstantTest.java

@@ -0,0 +1,75 @@
+/*
+ * Copyright 2023 The Error Prone Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.errorprone.bugpatterns;
+
+import com.google.errorprone.BugCheckerRefactoringTestHelper;
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class InlineTrivialConstantTest {
+  @Test
+  public void positive() {
+    BugCheckerRefactoringTestHelper.newInstance(InlineTrivialConstant.class, getClass())
+        .addInputLines(
+            "Test.java",
+            "class Test {",
+            "  private static final String EMPTY_STRING = \"\";",
+            "  void f() {",
+            "    System.err.println(EMPTY_STRING);",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "class Test {",
+            "  void f() {",
+            "    System.err.println(\"\");",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void negative() {
+    CompilationTestHelper.newInstance(InlineTrivialConstant.class, getClass())
+        .addSourceLines(
+            "Test.java",
+            "class Test {",
+            "  static class NonPrivate {",
+            "    static final String EMPTY_STRING = \"\";",
+            "  }",
+            "  static class NonStatic {",
+            "    private final String EMPTY_STRING = \"\";",
+            "  }",
+            "  static class NonFinal {",
+            "    private static String EMPTY_STRING = \"\";",
+            "  }",
+            "  static class NonString {",
+            "    private static final Object EMPTY_STRING = \"\";",
+            "  }",
+            "  static class WrongName {",
+            "    private static final String LAUNCH_CODES = \"\";",
+            "  }",
+            "  static class WrongValue {",
+            "    private static final String EMPTY_STRING = \"hello\";",
+            "  }",
+            "}")
+        .doTest();
+  }
+}

docs/bugpattern/InlineTrivialConstant.md

@@ -0,0 +1,20 @@
+Constants should be given names that emphasize the semantic meaning of the
+value. If the name of the constant doesn't convey any information that isn't
+clear from the value, consider inlining it.
+
+For example, prefer this:
+
+```java
+System.err.println(1);
+System.err.println("");
+```
+
+to this:
+
+```java
+private static final int ONE = 1;
+private static final String EMPTY_STRING = "";
+...
+System.err.println(ONE);
+System.err.println(EMPTY_STRING);
+```