Fix or suppress CheckReturnValue errors

PiperOrigin-RevId: 686665984

check_api/src/main/java/com/google/errorprone/apply/ImportStatements.java

@@ -20,6 +20,7 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.sun.tools.javac.tree.EndPosTable;
 import com.sun.tools.javac.tree.JCTree;
 import com.sun.tools.javac.tree.JCTree.JCCompilationUnit;
@@ -123,6 +124,7 @@ public int getEndPos() {
    * @param importToAdd a string representation of the import to add
    * @return true if the import was added
    */
+  @CanIgnoreReturnValue
   public boolean add(String importToAdd) {
     return importStrings.add(importToAdd);
   }
@@ -134,6 +136,7 @@ public boolean add(String importToAdd) {
    * @param importsToAdd a collection of imports to add
    * @return true if any imports were added to the list
    */
+  @CanIgnoreReturnValue
   public boolean addAll(Collection<String> importsToAdd) {
     return importStrings.addAll(importsToAdd);
   }
@@ -145,6 +148,7 @@ public boolean addAll(Collection<String> importsToAdd) {
    * @param importToRemove a string representation of the import to remove
    * @return true if the import was removed
    */
+  @CanIgnoreReturnValue
   public boolean remove(String importToRemove) {
     return importStrings.remove(importToRemove);
   }
@@ -156,6 +160,7 @@ public boolean remove(String importToRemove) {
    * @param importsToRemove a collection of imports to remove
    * @return true if any imports were removed from the list
    */
+  @CanIgnoreReturnValue
   public boolean removeAll(Collection<String> importsToRemove) {
     return importStrings.removeAll(importsToRemove);
   }

check_api/src/main/java/com/google/errorprone/util/Reachability.java

@@ -21,6 +21,7 @@
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
 import static java.util.Objects.requireNonNull;
 
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.sun.source.tree.AssertTree;
 import com.sun.source.tree.BlockTree;
 import com.sun.source.tree.BreakTree;
@@ -99,6 +100,9 @@ boolean scan(List<? extends StatementTree> trees) {
       return completes;
     }
 
+    // The result can be ignored to scan for label definitions in blocks that
+    // don't otherwise affect the result of the reachability analysis.
+    @CanIgnoreReturnValue
     private boolean scan(Tree tree) {
       return tree.accept(this, null);
     }

check_api/src/test/java/com/google/errorprone/fixes/AppliedFixTest.java

@@ -163,7 +163,7 @@ public void shouldApplyFixesInReverseOrder() {
 
     // If the fixes had been applied in the wrong order, this would fail.
     // But it succeeds, so they were applied in the right order.
-    AppliedFix.fromSource(" ", endPositions).apply(mockFix);
+    var unused = AppliedFix.fromSource(" ", endPositions).apply(mockFix);
   }
 
   @Test

check_api/src/test/java/com/google/errorprone/util/ASTHelpersTest.java

@@ -1834,7 +1834,7 @@ public void typesWithModifiersTree() {
   private static <T extends Tree> void assertGetModifiersInvoked(
       Class<T> clazz, Function<T, ModifiersTree> getter) {
     T tree = mock(clazz);
-    ASTHelpers.getModifiers(tree);
+    var unused = ASTHelpers.getModifiers(tree);
 
     // This effectively means the same as {@code verify(tree).getModifiers()}.
     ModifiersTree ignored = getter.apply(verify(tree));
@@ -1871,7 +1871,7 @@ public void typesWithGetAnnotations() {
   private static <T extends Tree> void assertGetAnnotationsInvoked(
       Class<T> clazz, Function<T, List<? extends AnnotationTree>> getter) {
     T tree = mock(clazz);
-    ASTHelpers.getAnnotations(tree);
+    var unused = ASTHelpers.getAnnotations(tree);
     // This effectively means the same as {@code verify(tree).getAnnotations()}.
     List<? extends AnnotationTree> ignored = getter.apply(verify(tree));
   }

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

@@ -216,14 +216,14 @@ public Boolean visitMemberSelect(MemberSelectTree tree, Void unused) {
       if (!isImplicitlyTyped) {
         fix.replace(varType, "List<String>").addImport("java.util.List");
       }
-      replaceWithSplitter(fix, tree, arg, state, "splitToList", needsMutableList[0]);
-    } else {
-      if (!isImplicitlyTyped) {
-        fix.replace(varType, "Iterable<String>");
-      }
-      replaceWithSplitter(fix, tree, arg, state, "split", needsMutableList[0]);
+      return Optional.of(
+          replaceWithSplitter(fix, tree, arg, state, "splitToList", needsMutableList[0]).build());
+    }
+    if (!isImplicitlyTyped) {
+      fix.replace(varType, "Iterable<String>");
     }
-    return Optional.of(fix.build());
+    return Optional.of(
+        replaceWithSplitter(fix, tree, arg, state, "split", needsMutableList[0]).build());
   }
 
   private static String getMethodAndArgument(

core/src/main/java/com/google/errorprone/bugpatterns/time/TimeUnitMismatch.java

@@ -45,6 +45,7 @@
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.AssignmentTreeMatcher;
 import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
@@ -165,6 +166,7 @@ private boolean checkSetterStyleMethod(
    * Checks whether this call is a call to {@code TimeUnit.to*} and, if so, whether the units of its
    * parameter and its receiver disagree.
    */
+  @CanIgnoreReturnValue
   private boolean checkTimeUnitToUnit(
       MethodInvocationTree tree, MethodSymbol methodSymbol, VisitorState state) {
     if (tree.getMethodSelect().getKind() != MEMBER_SELECT) {
@@ -202,6 +204,7 @@ private static boolean isTimeUnit(Symbol receiverSymbol, VisitorState state) {
           .put(DAYS, "toDays")
           .buildOrThrow();
 
+  @CanIgnoreReturnValue
   private boolean checkAll(
       List<VarSymbol> formals, List<? extends ExpressionTree> actuals, VisitorState state) {
     if (formals.size() != actuals.size()) {
@@ -221,6 +224,7 @@ private boolean checkAll(
     return hasFinding;
   }
 
+  @CanIgnoreReturnValue
   private boolean check(String formalName, ExpressionTree actualTree, VisitorState state) {
     /*
      * Sometimes people name a Duration parameter something like "durationMs." Then we falsely

core/src/main/java/com/google/errorprone/refaster/Bindings.java

@@ -124,6 +124,7 @@ public <V> V getBinding(Key<V> key) {
   }
 
   @SuppressWarnings("unchecked")
+  @CanIgnoreReturnValue
   public <V> V putBinding(Key<V> key, V value) {
     checkNotNull(value);
     return (V) super.put(key, value);

core/src/main/java/com/google/errorprone/refaster/Template.java

@@ -200,13 +200,14 @@ protected Optional<Unifier> typecheck(
       List<Type> actualTypes) {
     try {
       ImmutableList<UTypeVar> freeTypeVars = freeTypeVars(unifier);
-      infer(
-          warner,
-          inliner,
-          inliner.<Type>inlineList(freeTypeVars),
-          expectedTypes,
-          inliner.symtab().voidType,
-          actualTypes);
+      var unused =
+          infer(
+              warner,
+              inliner,
+              inliner.<Type>inlineList(freeTypeVars),
+              expectedTypes,
+              inliner.symtab().voidType,
+              actualTypes);
 
       for (UTypeVar var : freeTypeVars) {
         Type instantiationForVar =

core/src/main/java/com/google/errorprone/refaster/UMatches.java

@@ -40,7 +40,7 @@ public static UMatches create(
       boolean positive,
       UExpression expression) {
     // Verify that we can instantiate the Matcher
-    makeMatcher(matcherClass);
+    var unused = makeMatcher(matcherClass);
 
     return new AutoValue_UMatches(positive, matcherClass, expression);
   }

core/src/main/java/com/google/errorprone/refaster/UTemplater.java

@@ -641,7 +641,7 @@ public UExpression visitIdentifier(IdentifierTree tree, Void v) {
   static Class<? extends Matcher<? super ExpressionTree>> getValue(Matches matches) {
     String name;
     try {
-      matches.value();
+      var unused = matches.value();
       throw new RuntimeException("unreachable");
     } catch (MirroredTypeException e) {
       DeclaredType type = (DeclaredType) e.getTypeMirror();
@@ -662,7 +662,7 @@ static Class<? extends Matcher<? super ExpressionTree>> getValue(Matches matches
   static Class<? extends Matcher<? super ExpressionTree>> getValue(NotMatches matches) {
     String name;
     try {
-      matches.value();
+      var unused = matches.value();
       throw new RuntimeException("unreachable");
     } catch (MirroredTypeException e) {
       DeclaredType type = (DeclaredType) e.getTypeMirror();

core/src/main/java/com/google/errorprone/refaster/Unifier.java

@@ -20,6 +20,7 @@
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
 import com.google.errorprone.SubContext;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Types;
 import com.sun.tools.javac.tree.JCTree;
@@ -75,6 +76,7 @@ public Inliner createInliner() {
     return bindings.getBinding(key);
   }
 
+  @CanIgnoreReturnValue
   public <V> V putBinding(Bindings.Key<V> key, V value) {
     checkArgument(!bindings.containsKey(key), "Cannot bind %s more than once", key);
     return bindings.putBinding(key, value);

core/src/test/java/com/google/errorprone/ErrorProneCompilerIntegrationTest.java

@@ -579,20 +579,22 @@ public Description matchReturn(ReturnTree tree, VisitorState state) {
   public void compilationWithError() {
     compilerBuilder.report(ScannerSupplier.fromBugCheckerClasses(CPSChecker.class));
     compiler = compilerBuilder.build();
-    compiler.compile(
-        new String[] {
-          "-XDshouldStopPolicyIfError=LOWER",
-        },
-        Arrays.asList(
-            forSourceLines(
-                "Test.java",
-                """
-                package test;
-                public class Test {
-                  Object f() { return new NoSuch(); }
-                }
-                """)));
+    Result exitCode =
+        compiler.compile(
+            new String[] {
+              "-XDshouldStopPolicyIfError=LOWER",
+            },
+            Arrays.asList(
+                forSourceLines(
+                    "Test.java",
+                    """
+                    package test;
+                    public class Test {
+                      Object f() { return new NoSuch(); }
+                    }
+                    """)));
     outputStream.flush();
+    assertWithMessage(outputStream.toString()).that(exitCode).isEqualTo(Result.ERROR);
     String output = diagnosticHelper.getDiagnostics().toString();
     assertThat(output).contains("error: cannot find symbol");
     assertThat(output).doesNotContain("Using 'return' is considered harmful");
@@ -640,8 +642,9 @@ public class Test {
 
     compilerBuilder.report(ScannerSupplier.fromBugCheckerClasses(ForbiddenString.class));
     compiler = compilerBuilder.build();
-    compiler.compile(args, sources);
+    Result exitCode = compiler.compile(args, sources);
     outputStream.flush();
+    assertWithMessage(outputStream.toString()).that(exitCode).isEqualTo(Result.ERROR);
     String output = diagnosticHelper.getDiagnostics().toString();
     assertThat(output).contains("Please don't return this const value");
   }

core/src/test/java/com/google/errorprone/ErrorProneJavaCompilerTest.java

@@ -97,7 +97,7 @@ public void getStandardJavaFileManager() {
     JavaFileObjectDiagnosticListener listener = mock(JavaFileObjectDiagnosticListener.class);
     Locale locale = Locale.CANADA;
 
-    compiler.getStandardFileManager(listener, locale, null);
+    var unused = compiler.getStandardFileManager(listener, locale, null);
     verify(mockCompiler).getStandardFileManager(listener, locale, null);
   }
 
@@ -111,7 +111,7 @@ public void run() {
     OutputStream err = mock(OutputStream.class);
     String[] arguments = {"-source", "8", "-target", "8"};
 
-    compiler.run(in, out, err, arguments);
+    var unused = compiler.run(in, out, err, arguments);
     verify(mockCompiler).run(in, out, err, arguments);
   }
 

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

@@ -176,7 +176,7 @@ public String toString() {
   public static class CompletionChecker extends BugChecker implements ClassTreeMatcher {
     @Override
     public Description matchClass(ClassTree tree, VisitorState state) {
-      state.getSymbolFromString(One.class.getName());
+      var unused = state.getSymbolFromString(One.class.getName());
       return Description.NO_MATCH;
     }
   }

docgen/src/main/java/com/google/errorprone/BugPatternFileGenerator.java

@@ -25,6 +25,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.io.LineProcessor;
 import com.google.errorprone.BugPattern.SeverityLevel;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.gson.Gson;
 import java.io.IOException;
 import java.io.StringWriter;
@@ -78,6 +79,7 @@ public BugPatternFileGenerator(
     result = new ArrayList<>();
   }
 
+  @CanIgnoreReturnValue
   @Override
   public boolean processLine(String line) throws IOException {
     BugPatternInstance pattern = new Gson().fromJson(line, BugPatternInstance.class);