From 7a736905260fcb8cc2097f62a565bd7c879c6ed4 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Wed, 16 Oct 2024 15:38:38 -0700 Subject: [PATCH] Fix or suppress CheckReturnValue errors PiperOrigin-RevId: 686665984 --- .../errorprone/apply/ImportStatements.java | 5 +++ .../google/errorprone/util/Reachability.java | 4 +++ .../errorprone/fixes/AppliedFixTest.java | 2 +- .../errorprone/util/ASTHelpersTest.java | 4 +-- .../bugpatterns/StringSplitter.java | 14 ++++----- .../bugpatterns/time/TimeUnitMismatch.java | 4 +++ .../google/errorprone/refaster/Bindings.java | 1 + .../google/errorprone/refaster/Template.java | 15 ++++----- .../google/errorprone/refaster/UMatches.java | 2 +- .../errorprone/refaster/UTemplater.java | 4 +-- .../google/errorprone/refaster/Unifier.java | 2 ++ .../ErrorProneCompilerIntegrationTest.java | 31 ++++++++++--------- .../ErrorProneJavaCompilerTest.java | 4 +-- .../bugpatterns/ObjectToStringTest.java | 2 +- .../errorprone/BugPatternFileGenerator.java | 2 ++ 15 files changed, 59 insertions(+), 37 deletions(-) diff --git a/check_api/src/main/java/com/google/errorprone/apply/ImportStatements.java b/check_api/src/main/java/com/google/errorprone/apply/ImportStatements.java index fc457e3abf5..7849e096233 100644 --- a/check_api/src/main/java/com/google/errorprone/apply/ImportStatements.java +++ b/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 importsToAdd) { return importStrings.addAll(importsToAdd); } @@ -145,6 +148,7 @@ public boolean addAll(Collection 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 importsToRemove) { return importStrings.removeAll(importsToRemove); } diff --git a/check_api/src/main/java/com/google/errorprone/util/Reachability.java b/check_api/src/main/java/com/google/errorprone/util/Reachability.java index 67d31bf7d9c..12e644fa8ac 100644 --- a/check_api/src/main/java/com/google/errorprone/util/Reachability.java +++ b/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 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); } diff --git a/check_api/src/test/java/com/google/errorprone/fixes/AppliedFixTest.java b/check_api/src/test/java/com/google/errorprone/fixes/AppliedFixTest.java index b68f8a12ce0..b8c0277f62d 100644 --- a/check_api/src/test/java/com/google/errorprone/fixes/AppliedFixTest.java +++ b/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 diff --git a/check_api/src/test/java/com/google/errorprone/util/ASTHelpersTest.java b/check_api/src/test/java/com/google/errorprone/util/ASTHelpersTest.java index 6b3a968c8e5..ff4dd6c2aad 100644 --- a/check_api/src/test/java/com/google/errorprone/util/ASTHelpersTest.java +++ b/check_api/src/test/java/com/google/errorprone/util/ASTHelpersTest.java @@ -1834,7 +1834,7 @@ public void typesWithModifiersTree() { private static void assertGetModifiersInvoked( Class clazz, Function 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 void assertGetAnnotationsInvoked( Class clazz, Function> getter) { T tree = mock(clazz); - ASTHelpers.getAnnotations(tree); + var unused = ASTHelpers.getAnnotations(tree); // This effectively means the same as {@code verify(tree).getAnnotations()}. List ignored = getter.apply(verify(tree)); } diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/StringSplitter.java b/core/src/main/java/com/google/errorprone/bugpatterns/StringSplitter.java index 69b3897c596..69a00ea0e80 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/StringSplitter.java +++ b/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").addImport("java.util.List"); } - replaceWithSplitter(fix, tree, arg, state, "splitToList", needsMutableList[0]); - } else { - if (!isImplicitlyTyped) { - fix.replace(varType, "Iterable"); - } - 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"); } - return Optional.of(fix.build()); + return Optional.of( + replaceWithSplitter(fix, tree, arg, state, "split", needsMutableList[0]).build()); } private static String getMethodAndArgument( diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/time/TimeUnitMismatch.java b/core/src/main/java/com/google/errorprone/bugpatterns/time/TimeUnitMismatch.java index 9ed7fa74608..90360371895 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/time/TimeUnitMismatch.java +++ b/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 formals, List 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 diff --git a/core/src/main/java/com/google/errorprone/refaster/Bindings.java b/core/src/main/java/com/google/errorprone/refaster/Bindings.java index 1d99777ce2b..bfe2bfee082 100644 --- a/core/src/main/java/com/google/errorprone/refaster/Bindings.java +++ b/core/src/main/java/com/google/errorprone/refaster/Bindings.java @@ -124,6 +124,7 @@ public V getBinding(Key key) { } @SuppressWarnings("unchecked") + @CanIgnoreReturnValue public V putBinding(Key key, V value) { checkNotNull(value); return (V) super.put(key, value); diff --git a/core/src/main/java/com/google/errorprone/refaster/Template.java b/core/src/main/java/com/google/errorprone/refaster/Template.java index 4c226065741..6175c60cd29 100644 --- a/core/src/main/java/com/google/errorprone/refaster/Template.java +++ b/core/src/main/java/com/google/errorprone/refaster/Template.java @@ -200,13 +200,14 @@ protected Optional typecheck( List actualTypes) { try { ImmutableList freeTypeVars = freeTypeVars(unifier); - infer( - warner, - inliner, - inliner.inlineList(freeTypeVars), - expectedTypes, - inliner.symtab().voidType, - actualTypes); + var unused = + infer( + warner, + inliner, + inliner.inlineList(freeTypeVars), + expectedTypes, + inliner.symtab().voidType, + actualTypes); for (UTypeVar var : freeTypeVars) { Type instantiationForVar = diff --git a/core/src/main/java/com/google/errorprone/refaster/UMatches.java b/core/src/main/java/com/google/errorprone/refaster/UMatches.java index f6c07a3a5d9..ce70b4b5af4 100644 --- a/core/src/main/java/com/google/errorprone/refaster/UMatches.java +++ b/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); } diff --git a/core/src/main/java/com/google/errorprone/refaster/UTemplater.java b/core/src/main/java/com/google/errorprone/refaster/UTemplater.java index d33b3493791..02c738557d3 100644 --- a/core/src/main/java/com/google/errorprone/refaster/UTemplater.java +++ b/core/src/main/java/com/google/errorprone/refaster/UTemplater.java @@ -641,7 +641,7 @@ public UExpression visitIdentifier(IdentifierTree tree, Void v) { static Class> 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> getValue(Matches matches static Class> getValue(NotMatches matches) { String name; try { - matches.value(); + var unused = matches.value(); throw new RuntimeException("unreachable"); } catch (MirroredTypeException e) { DeclaredType type = (DeclaredType) e.getTypeMirror(); diff --git a/core/src/main/java/com/google/errorprone/refaster/Unifier.java b/core/src/main/java/com/google/errorprone/refaster/Unifier.java index 7220bac1ea4..afad0a9939b 100644 --- a/core/src/main/java/com/google/errorprone/refaster/Unifier.java +++ b/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 putBinding(Bindings.Key key, V value) { checkArgument(!bindings.containsKey(key), "Cannot bind %s more than once", key); return bindings.putBinding(key, value); diff --git a/core/src/test/java/com/google/errorprone/ErrorProneCompilerIntegrationTest.java b/core/src/test/java/com/google/errorprone/ErrorProneCompilerIntegrationTest.java index 4f5dc18ec60..fa3740d18d5 100644 --- a/core/src/test/java/com/google/errorprone/ErrorProneCompilerIntegrationTest.java +++ b/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"); } diff --git a/core/src/test/java/com/google/errorprone/ErrorProneJavaCompilerTest.java b/core/src/test/java/com/google/errorprone/ErrorProneJavaCompilerTest.java index a6b81b3b89a..fb73c80a010 100644 --- a/core/src/test/java/com/google/errorprone/ErrorProneJavaCompilerTest.java +++ b/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); } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/ObjectToStringTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/ObjectToStringTest.java index a980a493305..bd0b9b31e6b 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/ObjectToStringTest.java +++ b/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; } } diff --git a/docgen/src/main/java/com/google/errorprone/BugPatternFileGenerator.java b/docgen/src/main/java/com/google/errorprone/BugPatternFileGenerator.java index 51286af9273..22ce16ea3eb 100644 --- a/docgen/src/main/java/com/google/errorprone/BugPatternFileGenerator.java +++ b/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);