cla
* Only method trees that belong to the {@code classTree} are returned, so methods declared in
* nested classes are not going to be considered.
*/
- private ImmutableList getClassTreeMethods(ClassTree classTree) {
+ private ImmutableList getClassTreeMethods(ClassTree classTree, VisitorState state) {
List members = classTree.getMembers();
return members.stream()
.filter(MethodTree.class::isInstance)
.map(MethodTree.class::cast)
- .filter(m -> !isSuppressed(m))
+ .filter(m -> !isSuppressed(m, state))
.collect(toImmutableList());
}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ConstantExpressions.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ConstantExpressions.java
index 399cb91a0cc..d1fb518615a 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ConstantExpressions.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ConstantExpressions.java
@@ -320,6 +320,7 @@ public final String toString() {
return receiver + symbol().getSimpleName();
}
return receiver
+ + (symbol().isStatic() ? symbol().owner.getSimpleName() + "." : "")
+ symbol().getSimpleName()
+ arguments().stream().map(Object::toString).collect(joining(", ", "(", ")"));
}
@@ -350,8 +351,9 @@ public Optional symbolizeImmutableExpression(
? getReceiver(tree)
: null;
+ Symbol symbol = getSymbol(tree);
Optional receiverConstant;
- if (receiver == null) {
+ if (receiver == null || (symbol != null && symbol.isStatic())) {
receiverConstant = Optional.empty();
} else {
receiverConstant = constantExpression(receiver, state);
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByChecker.java
index 1e00315c028..cbed17f6986 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByChecker.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByChecker.java
@@ -85,7 +85,7 @@ private void analyze(VisitorState state) {
state,
(ExpressionTree tree, GuardedByExpression guard, HeldLockSet live) ->
report(GuardedByChecker.this.checkGuardedAccess(tree, guard, live, state), state),
- this::isSuppressed,
+ tree1 -> isSuppressed(tree1, state),
flags,
reportMissingGuards);
}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnalysis.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnalysis.java
index 9357562fef1..62c6107a9ce 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnalysis.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableAnalysis.java
@@ -257,14 +257,14 @@ Violation areFieldsImmutable(
}
/** Check a single field for immutability. */
- private Violation isFieldImmutable(
+ Violation isFieldImmutable(
Optional tree,
ImmutableSet immutableTyParams,
ClassSymbol classSym,
ClassType classType,
VarSymbol var,
ViolationReporter reporter) {
- if (bugChecker.isSuppressed(var)) {
+ if (bugChecker.isSuppressed(var, state)) {
return Violation.absent();
}
if (!var.getModifiers().contains(Modifier.FINAL)
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableChecker.java
index 67d9e2cd2cf..d3fdcf1dcf6 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableChecker.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableChecker.java
@@ -19,9 +19,17 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.util.ASTHelpers.getReceiver;
+import static com.google.errorprone.util.ASTHelpers.getSymbol;
+import static com.google.errorprone.util.ASTHelpers.getType;
+import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
+import static java.lang.String.format;
+import static java.util.stream.Collectors.joining;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.LinkedHashMultimap;
+import com.google.common.collect.SetMultimap;
import com.google.common.collect.Sets;
import com.google.common.collect.Sets.SetView;
import com.google.errorprone.BugPattern;
@@ -30,6 +38,7 @@
import com.google.errorprone.annotations.Immutable;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
+import com.google.errorprone.bugpatterns.BugChecker.LambdaExpressionTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.MemberReferenceTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
@@ -41,22 +50,34 @@
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.IdentifierTree;
+import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.MemberReferenceTree;
+import com.sun.source.tree.MemberSelectTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.NewClassTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.TypeParameterTree;
+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.ClassSymbol;
+import com.sun.tools.javac.code.Symbol.MethodSymbol;
+import com.sun.tools.javac.code.Symbol.TypeSymbol;
import com.sun.tools.javac.code.Symbol.TypeVariableSymbol;
+import com.sun.tools.javac.code.Symbol.VarSymbol;
import com.sun.tools.javac.code.Type;
+import com.sun.tools.javac.code.Type.ClassType;
import com.sun.tools.javac.tree.JCTree.JCMemberReference;
import com.sun.tools.javac.tree.JCTree.JCNewClass;
import java.util.Collection;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
+import javax.lang.model.element.ElementKind;
/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
@BugPattern(
@@ -66,6 +87,7 @@
documentSuppression = false)
public class ImmutableChecker extends BugChecker
implements ClassTreeMatcher,
+ LambdaExpressionTreeMatcher,
NewClassTreeMatcher,
MethodInvocationTreeMatcher,
MethodTreeMatcher,
@@ -73,6 +95,7 @@ public class ImmutableChecker extends BugChecker
private final WellKnownMutability wellKnownMutability;
private final ImmutableSet immutableAnnotations;
+ private final boolean matchLambdas;
ImmutableChecker(ImmutableSet immutableAnnotations) {
this(ErrorProneFlags.empty(), immutableAnnotations);
@@ -85,34 +108,150 @@ public ImmutableChecker(ErrorProneFlags flags) {
private ImmutableChecker(ErrorProneFlags flags, ImmutableSet immutableAnnotations) {
this.wellKnownMutability = WellKnownMutability.fromFlags(flags);
this.immutableAnnotations = immutableAnnotations;
+ this.matchLambdas = flags.getBoolean("ImmutableChecker:MatchLambdas").orElse(true);
+ }
+
+ @Override
+ public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
+ if (!matchLambdas) {
+ return NO_MATCH;
+ }
+ TypeSymbol lambdaType = getType(tree).tsym;
+ if (!hasImmutableAnnotation(lambdaType, state)) {
+ return NO_MATCH;
+ }
+ Set variablesClosed = new HashSet<>();
+ SetMultimap typesClosed = LinkedHashMultimap.create();
+ Set variablesOwnedByLambda = new HashSet<>();
+
+ new TreePathScanner() {
+ @Override
+ public Void visitVariable(VariableTree tree, Void unused) {
+ var symbol = getSymbol(tree);
+ variablesOwnedByLambda.add(symbol);
+ return super.visitVariable(tree, null);
+ }
+
+ @Override
+ public Void visitMethodInvocation(MethodInvocationTree tree, Void unused) {
+ if (getReceiver(tree) == null) {
+ var symbol = getSymbol(tree);
+ if (!symbol.isStatic()) {
+ // TODO(b/77333859): This isn't precise. What we really want is the type of `this`, if
+ // the method call were qualified with it.
+ typesClosed.put((ClassSymbol) symbol.owner, symbol);
+ }
+ }
+ return super.visitMethodInvocation(tree, null);
+ }
+
+ @Override
+ public Void visitMemberSelect(MemberSelectTree tree, Void unused) {
+ // Special case the access of fields to allow accessing fields which would pass an immutable
+ // check.
+ if (tree.getExpression() instanceof IdentifierTree
+ && getSymbol(tree) instanceof VarSymbol) {
+ handleIdentifier(getSymbol(tree));
+ // If we're only seeing a field access, don't complain about the fact we closed around
+ // `this`.
+ if (tree.getExpression() instanceof IdentifierTree
+ && ((IdentifierTree) tree.getExpression()).getName().contentEquals("this")) {
+ return null;
+ }
+ }
+ return super.visitMemberSelect(tree, null);
+ }
+
+ @Override
+ public Void visitIdentifier(IdentifierTree tree, Void unused) {
+ handleIdentifier(getSymbol(tree));
+ return super.visitIdentifier(tree, null);
+ }
+
+ private void handleIdentifier(Symbol symbol) {
+ if (symbol instanceof VarSymbol && !variablesOwnedByLambda.contains(symbol)) {
+ variablesClosed.add((VarSymbol) symbol);
+ }
+ }
+ }.scan(state.getPath(), null);
+
+ ImmutableAnalysis analysis = createImmutableAnalysis(state);
+ ImmutableSet typarams =
+ immutableTypeParametersInScope(getSymbol(tree), state, analysis);
+ variablesClosed.stream()
+ .map(closedVariable -> checkClosedLambdaVariable(closedVariable, tree, typarams, analysis))
+ .filter(Violation::isPresent)
+ .forEachOrdered(
+ v -> {
+ String message = formLambdaReason(lambdaType) + ", but " + v.message();
+ state.reportMatch(buildDescription(tree).setMessage(message).build());
+ });
+ for (var entry : typesClosed.asMap().entrySet()) {
+ var classSymbol = entry.getKey();
+ var methods = entry.getValue();
+ if (!hasImmutableAnnotation(classSymbol.type.tsym, state)) {
+ String message =
+ format(
+ "%s, but accesses instance method(s) '%s' on '%s' which is not @Immutable.",
+ formLambdaReason(lambdaType),
+ methods.stream().map(Symbol::getSimpleName).collect(joining(", ")),
+ classSymbol.getSimpleName());
+ state.reportMatch(buildDescription(tree).setMessage(message).build());
+ }
+ }
+
+ return NO_MATCH;
+ }
+
+ private Violation checkClosedLambdaVariable(
+ VarSymbol closedVariable,
+ LambdaExpressionTree tree,
+ ImmutableSet typarams,
+ ImmutableAnalysis analysis) {
+ if (!closedVariable.getKind().equals(ElementKind.FIELD)) {
+ return analysis.isThreadSafeType(false, typarams, closedVariable.type);
+ }
+ return analysis.isFieldImmutable(
+ Optional.empty(),
+ typarams,
+ (ClassSymbol) closedVariable.owner,
+ (ClassType) closedVariable.owner.type,
+ closedVariable,
+ (t, v) -> buildDescription(tree));
+ }
+
+ private static String formLambdaReason(TypeSymbol typeSymbol) {
+ return "This lambda implements @Immutable interface '" + typeSymbol.getSimpleName() + "'";
+ }
+
+ private boolean hasImmutableAnnotation(TypeSymbol tsym, VisitorState state) {
+ return immutableAnnotations.stream()
+ .anyMatch(annotation -> hasAnnotation(tsym, annotation, state));
}
// check instantiations of `@ImmutableTypeParameter`s in method references
@Override
public Description matchMemberReference(MemberReferenceTree tree, VisitorState state) {
- checkInvocation(
- tree, ((JCMemberReference) tree).referentType, state, ASTHelpers.getSymbol(tree));
+ checkInvocation(tree, getSymbol(tree), ((JCMemberReference) tree).referentType, state);
return NO_MATCH;
}
// check instantiations of `@ImmutableTypeParameter`s in method invocations
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
- checkInvocation(
- tree, ASTHelpers.getType(tree.getMethodSelect()), state, ASTHelpers.getSymbol(tree));
+ checkInvocation(tree, getSymbol(tree), ASTHelpers.getType(tree.getMethodSelect()), state);
return NO_MATCH;
}
@Override
public Description matchNewClass(NewClassTree tree, VisitorState state) {
// check instantiations of `@ImmutableTypeParameter`s in generic constructor invocations
- checkInvocation(
- tree, ((JCNewClass) tree).constructorType, state, ((JCNewClass) tree).constructor);
+ checkInvocation(tree, getSymbol(tree), ((JCNewClass) tree).constructorType, state);
// check instantiations of `@ImmutableTypeParameter`s in class constructor invocations
checkInstantiation(
tree,
state,
- ASTHelpers.getSymbol(tree.getIdentifier()).getTypeParameters(),
+ getSymbol(tree.getIdentifier()).getTypeParameters(),
ASTHelpers.getType(tree).getTypeArguments());
ClassTree classBody = tree.getClassBody();
@@ -132,7 +271,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
checkInstantiation(
tree,
state,
- ASTHelpers.getSymbol(tree).getTypeParameters(),
+ getSymbol(tree).getTypeParameters(),
ASTHelpers.getType(tree).getTypeArguments());
return NO_MATCH;
}
@@ -141,7 +280,8 @@ private ImmutableAnalysis createImmutableAnalysis(VisitorState state) {
return new ImmutableAnalysis(this, state, wellKnownMutability, immutableAnnotations);
}
- private void checkInvocation(Tree tree, Type methodType, VisitorState state, Symbol symbol) {
+ private void checkInvocation(
+ Tree tree, MethodSymbol symbol, Type methodType, VisitorState state) {
ImmutableAnalysis analysis = createImmutableAnalysis(state);
Violation info = analysis.checkInvocation(methodType, symbol);
if (info.isPresent()) {
@@ -197,14 +337,13 @@ public Description matchClass(ClassTree tree, VisitorState state) {
// Check that the types in containerOf actually exist
Map typarams = new HashMap<>();
for (TypeParameterTree typaram : tree.getTypeParameters()) {
- typarams.put(
- typaram.getName().toString(), (TypeVariableSymbol) ASTHelpers.getSymbol(typaram));
+ typarams.put(typaram.getName().toString(), (TypeVariableSymbol) getSymbol(typaram));
}
SetView difference = Sets.difference(annotation.containerOf(), typarams.keySet());
if (!difference.isEmpty()) {
return buildDescription(tree)
.setMessage(
- String.format(
+ format(
"could not find type(s) referenced by containerOf: %s",
Joiner.on("', '").join(difference)))
.build();
@@ -220,7 +359,7 @@ public Description matchClass(ClassTree tree, VisitorState state) {
if (!immutableAndContainer.isEmpty()) {
return buildDescription(tree)
.setMessage(
- String.format(
+ format(
"using both @ImmutableTypeParameter and containerOf is redundant: %s",
Joiner.on("', '").join(immutableAndContainer)))
.build();
@@ -231,12 +370,12 @@ public Description matchClass(ClassTree tree, VisitorState state) {
// Check that the fields (including inherited fields) are immutable, and
// validate the type hierarchy superclass.
- ClassSymbol sym = ASTHelpers.getSymbol(tree);
+ ClassSymbol sym = getSymbol(tree);
Violation info =
analysis.checkForImmutability(
Optional.of(tree),
- immutableTypeParametersInScope(ASTHelpers.getSymbol(tree), state, analysis),
+ immutableTypeParametersInScope(getSymbol(tree), state, analysis),
ASTHelpers.getType(tree),
(Tree matched, Violation violation) ->
describeClass(matched, sym, annotation, violation));
@@ -255,7 +394,7 @@ private void checkClassTreeInstantiation(
tree,
state,
analysis,
- ASTHelpers.getSymbol(implementTree).getTypeParameters(),
+ getSymbol(implementTree).getTypeParameters(),
ASTHelpers.getType(implementTree).getTypeArguments());
}
@@ -265,7 +404,7 @@ private void checkClassTreeInstantiation(
tree,
state,
analysis,
- ASTHelpers.getSymbol(extendsClause).getTypeParameters(),
+ getSymbol(extendsClause).getTypeParameters(),
ASTHelpers.getType(extendsClause).getTypeArguments());
}
}
@@ -277,7 +416,7 @@ private Description.Builder describeClass(
message = "type annotated with @Immutable could not be proven immutable: " + info.message();
} else {
message =
- String.format(
+ format(
"Class extends @Immutable type %s, but is not immutable: %s",
annotation.typeName(), info.message());
}
@@ -289,7 +428,7 @@ private Description.Builder describeClass(
/** Check anonymous implementations of {@code @Immutable} types. */
private Description handleAnonymousClass(
ClassTree tree, VisitorState state, ImmutableAnalysis analysis) {
- ClassSymbol sym = ASTHelpers.getSymbol(tree);
+ ClassSymbol sym = getSymbol(tree);
Type superType = immutableSupertype(sym, state);
if (superType == null) {
return NO_MATCH;
@@ -324,7 +463,7 @@ public Description.Builder describe(Tree tree, Violation info) {
private Description.Builder describeAnonymous(Tree tree, Type superType, Violation info) {
String message =
- String.format(
+ format(
"Class extends @Immutable type %s, but is not immutable: %s",
superType, info.message());
return buildDescription(tree).setMessage(message);
@@ -334,14 +473,13 @@ private Description.Builder describeAnonymous(Tree tree, Type superType, Violati
/** Check for classes without {@code @Immutable} that have immutable supertypes. */
private Description checkSubtype(ClassTree tree, VisitorState state) {
- ClassSymbol sym = ASTHelpers.getSymbol(tree);
+ ClassSymbol sym = getSymbol(tree);
Type superType = immutableSupertype(sym, state);
if (superType == null) {
return NO_MATCH;
}
String message =
- String.format(
- "Class extends @Immutable type %s, but is not annotated as immutable", superType);
+ format("Class extends @Immutable type %s, but is not annotated as immutable", superType);
Fix fix =
SuggestedFix.builder()
.prefixWith(tree, "@Immutable ")
@@ -361,8 +499,7 @@ private Type immutableSupertype(Symbol sym, VisitorState state) {
}
// Don't use getImmutableAnnotation here: subtypes of trusted types are
// also trusted, only check for explicitly annotated supertypes.
- if (immutableAnnotations.stream()
- .anyMatch(annotation -> ASTHelpers.hasAnnotation(superType.tsym, annotation, state))) {
+ if (hasImmutableAnnotation(superType.tsym, state)) {
return superType;
}
// We currently trust that @interface annotations are immutable, but don't enforce that
diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
index 45d260cc62e..8f41a09b039 100644
--- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
+++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
@@ -143,7 +143,6 @@
import com.google.errorprone.bugpatterns.GetClassOnAnnotation;
import com.google.errorprone.bugpatterns.GetClassOnClass;
import com.google.errorprone.bugpatterns.GetClassOnEnum;
-import com.google.errorprone.bugpatterns.HashCodeToString;
import com.google.errorprone.bugpatterns.HashtableContains;
import com.google.errorprone.bugpatterns.HidingField;
import com.google.errorprone.bugpatterns.IdentityBinaryExpression;
@@ -233,6 +232,7 @@
import com.google.errorprone.bugpatterns.MustBeClosedChecker;
import com.google.errorprone.bugpatterns.MutablePublicArray;
import com.google.errorprone.bugpatterns.NCopiesOfChar;
+import com.google.errorprone.bugpatterns.NarrowCalculation;
import com.google.errorprone.bugpatterns.NarrowingCompoundAssignment;
import com.google.errorprone.bugpatterns.NegativeCharLiteral;
import com.google.errorprone.bugpatterns.NestedInstanceOfConditions;
@@ -284,7 +284,6 @@
import com.google.errorprone.bugpatterns.ProtocolBufferOrdinal;
import com.google.errorprone.bugpatterns.ProtosAsKeyOfSetOrMap;
import com.google.errorprone.bugpatterns.PublicApiNamedStreamShouldReturnStream;
-import com.google.errorprone.bugpatterns.PublicConstructorForAbstractClass;
import com.google.errorprone.bugpatterns.RandomCast;
import com.google.errorprone.bugpatterns.RandomModInteger;
import com.google.errorprone.bugpatterns.ReachabilityFenceUsage;
@@ -901,6 +900,7 @@ public static ScannerSupplier errorChecks() {
MultipleParallelOrSequentialCalls.class,
MultipleUnaryOperatorsInMethodCall.class,
MutablePublicArray.class,
+ NarrowCalculation.class,
NarrowingCompoundAssignment.class,
NegativeCharLiteral.class,
NestedInstanceOfConditions.class,
@@ -1037,7 +1037,6 @@ public static ScannerSupplier errorChecks() {
FunctionalInterfaceClash.class,
FuzzyEqualsShouldNotBeUsedInEqualsMethod.class,
HardCodedSdCardPath.class,
- HashCodeToString.class,
ImmutableMemberCollection.class,
ImmutableRefactoring.class,
ImmutableSetForContains.class,
@@ -1073,7 +1072,6 @@ public static ScannerSupplier errorChecks() {
PrivateConstructorForUtilityClass.class,
ProtosAsKeyOfSetOrMap.class,
PublicApiNamedStreamShouldReturnStream.class,
- PublicConstructorForAbstractClass.class,
QualifierWithTypeUse.class,
RedundantOverride.class,
RedundantThrows.class,
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/AlreadyCheckedTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/AlreadyCheckedTest.java
index 3cd8203fb6f..d0acacfe3f5 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/AlreadyCheckedTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/AlreadyCheckedTest.java
@@ -378,6 +378,27 @@ public void durationsComparedUsingFactoryMethods() {
.doTest();
}
+ @Test
+ public void durationsComparedUsingFactoryMethods_withDifferentImport() {
+ helper
+ .addSourceLines(
+ "Test.java",
+ "import static java.time.Duration.ofSeconds;",
+ "import java.time.Duration;",
+ "class Test {",
+ " public void test(Duration a, Duration b) {",
+ " if (a.equals(Duration.ofSeconds(1))) {",
+ " if (a.equals(Duration.ofSeconds(2))) {}",
+ " // BUG: Diagnostic contains:",
+ " if (a.equals(ofSeconds(1))) {}",
+ " // BUG: Diagnostic contains:",
+ " if (a.equals(java.time.Duration.ofSeconds(1))) {}",
+ " }",
+ " }",
+ "}")
+ .doTest();
+ }
+
@Test
public void autoValues() {
helper
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/CheckReturnValueTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/CheckReturnValueTest.java
index 6d6d8b5cf60..65f5f6cf2b6 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/CheckReturnValueTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/CheckReturnValueTest.java
@@ -16,6 +16,9 @@
package com.google.errorprone.bugpatterns;
+import com.google.auto.value.processor.AutoBuilderProcessor;
+import com.google.auto.value.processor.AutoValueProcessor;
+import com.google.common.collect.ImmutableList;
import com.google.errorprone.CompilationTestHelper;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -705,6 +708,89 @@ public void constructor_superCall() {
.doTest();
}
+ @Test
+ public void constructor_anonymousClassInheritsCIRV() {
+ compilationHelperLookingAtAllConstructors()
+ .addSourceLines(
+ "Test.java",
+ "class Test {",
+ " @com.google.errorprone.annotations.CanIgnoreReturnValue",
+ " public Test() {}",
+ " public static void foo() {",
+ " new Test() {};",
+ " new Test() {{ System.out.println(\"Lookie, instance initializer\"); }};",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void constructor_anonymousClassInheritsCRV() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "class Test {",
+ " @com.google.errorprone.annotations.CheckReturnValue",
+ " public Test() {}",
+ " public static void foo() {",
+ " // BUG: Diagnostic contains: ",
+ " new Test() {};",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void constructor_hasOuterInstance() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "class Test {",
+ " class Inner {",
+ " @com.google.errorprone.annotations.CheckReturnValue",
+ " public Inner() {}",
+ " }",
+ " public static void foo() {",
+ " // BUG: Diagnostic contains: ",
+ " new Test().new Inner() {};",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void constructor_anonymousClassInheritsCRV_syntheticConstructor() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "class Test {",
+ " @com.google.errorprone.annotations.CheckReturnValue",
+ " static class Nested {}",
+ " public static void foo() {",
+ " // BUG: Diagnostic contains: ",
+ " new Nested() {};", // The "called" constructor is synthetic, but within @CRV Nested
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void constructor_inheritsFromCrvInterface() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "class Test {",
+ " @com.google.errorprone.annotations.CheckReturnValue",
+ " static interface IFace {}",
+ " public static void foo() {",
+ // TODO(b/226203690): It's arguable that this might need to be @CRV?
+ // The superclass of the anonymous class is Object, not IFace, but /shrug
+ " new IFace() {};",
+ " }",
+ "}")
+ .doTest();
+ }
+
@Test
public void constructor_throwingContexts() {
compilationHelper
@@ -789,6 +875,130 @@ public void usingElementInTestExpected() {
.doTest();
}
+ @Test
+ public void testAutoValueBuilderSetterMethods() {
+ compilationHelper
+ .addSourceLines(
+ "Animal.java",
+ "package com.google.frobber;",
+ "import com.google.auto.value.AutoValue;",
+ "import com.google.errorprone.annotations.CheckReturnValue;",
+ "@AutoValue",
+ "@CheckReturnValue",
+ "abstract class Animal {",
+ " abstract String name();",
+ " abstract int numberOfLegs();",
+ " static Builder builder() {",
+ " return new AutoValue_Animal.Builder();",
+ " }",
+ " @AutoValue.Builder",
+ " abstract static class Builder {",
+ " abstract Builder setName(String value);",
+ " abstract Builder setNumberOfLegs(int value);",
+ " abstract Animal build();",
+ " }",
+ "}")
+ .addSourceLines(
+ "AnimalCaller.java",
+ "package com.google.frobber;",
+ "public final class AnimalCaller {",
+ " static void testAnimal() {",
+ " Animal.Builder builder = Animal.builder();",
+ " builder.setNumberOfLegs(4);", // AutoValue.Builder setters are implicitly @CIRV
+ " // BUG: Diagnostic contains: Ignored return value of 'build'",
+ " builder.build();",
+ " }",
+ "}")
+ .setArgs(ImmutableList.of("-processor", AutoValueProcessor.class.getName()))
+ .doTest();
+ }
+
+ @Test
+ public void testAutoBuilderSetterMethods() {
+ compilationHelper
+ .addSourceLines(
+ "Person.java",
+ "package com.google.frobber;",
+ "public final class Person {",
+ " public Person(String name, int id) {}",
+ "}")
+ .addSourceLines(
+ "PersonBuilder.java",
+ "package com.google.frobber;",
+ "import com.google.auto.value.AutoBuilder;",
+ "import com.google.errorprone.annotations.CheckReturnValue;",
+ "@CheckReturnValue",
+ "@AutoBuilder(ofClass = Person.class)",
+ "abstract class PersonBuilder {",
+ " static PersonBuilder personBuilder() {",
+ " return new AutoBuilder_PersonBuilder();",
+ " }",
+ " abstract PersonBuilder setName(String name);",
+ " abstract PersonBuilder setId(int id);",
+ " abstract Person build();",
+ "}")
+ .addSourceLines(
+ "PersonCaller.java",
+ "package com.google.frobber;",
+ "public final class PersonCaller {",
+ " static void testPersonBuilder() {",
+ " // BUG: Diagnostic contains: Ignored return value of 'personBuilder'",
+ " PersonBuilder.personBuilder();",
+ " PersonBuilder builder = PersonBuilder.personBuilder();",
+ " builder.setName(\"kurt\");", // AutoBuilder setters are implicitly @CIRV
+ " builder.setId(42);", // AutoBuilder setters are implicitly @CIRV
+ " // BUG: Diagnostic contains: Ignored return value of 'build'",
+ " builder.build();",
+ " }",
+ "}")
+ .setArgs(ImmutableList.of("-processor", AutoBuilderProcessor.class.getName()))
+ .doTest();
+ }
+
+ @Test
+ public void testAutoBuilderSetterMethods_withInterface() {
+ compilationHelper
+ .addSourceLines(
+ "LogUtil.java",
+ "package com.google.frobber;",
+ "import java.util.logging.Level;",
+ "public class LogUtil {",
+ " public static void log(Level severity, String message) {}",
+ "}")
+ .addSourceLines(
+ "Caller.java",
+ "package com.google.frobber;",
+ "import com.google.auto.value.AutoBuilder;",
+ "import java.util.logging.Level;",
+ "import com.google.errorprone.annotations.CheckReturnValue;",
+ "@CheckReturnValue",
+ "@AutoBuilder(callMethod = \"log\", ofClass = LogUtil.class)",
+ "public interface Caller {",
+ " static Caller logCaller() {",
+ " return new AutoBuilder_Caller();",
+ " }",
+ " Caller setSeverity(Level level);",
+ " Caller setMessage(String message);",
+ " void call(); // calls: LogUtil.log(severity, message)",
+ "}")
+ .addSourceLines(
+ "LogCaller.java",
+ "package com.google.frobber;",
+ "import java.util.logging.Level;",
+ "public final class LogCaller {",
+ " static void testLogCaller() {",
+ " // BUG: Diagnostic contains: Ignored return value of 'logCaller'",
+ " Caller.logCaller();",
+ " Caller caller = Caller.logCaller();",
+ " caller.setMessage(\"hi\");", // AutoBuilder setters are implicitly @CIRV
+ " caller.setSeverity(Level.FINE);", // AutoBuilder setters are implicitly @CIRV
+ " caller.call();",
+ " }",
+ "}")
+ .setArgs(ImmutableList.of("-processor", AutoBuilderProcessor.class.getName()))
+ .doTest();
+ }
+
private CompilationTestHelper compilationHelperLookingAtAllConstructors() {
return compilationHelper.setArgs(
"-XepOpt:" + CheckReturnValue.CHECK_ALL_CONSTRUCTORS + "=true");
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/HashCodeToStringTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/HashCodeToStringTest.java
deleted file mode 100644
index ce99ea14100..00000000000
--- a/core/src/test/java/com/google/errorprone/bugpatterns/HashCodeToStringTest.java
+++ /dev/null
@@ -1,190 +0,0 @@
-/*
- * Copyright 2020 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.CompilationTestHelper;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-/** {@link HashCodeToString}Test */
-@RunWith(JUnit4.class)
-public class HashCodeToStringTest {
-
- private final CompilationTestHelper compilationHelper =
- CompilationTestHelper.newInstance(HashCodeToString.class, getClass());
-
- @Test
- public void testPositiveCase() {
- compilationHelper
- .addSourceLines(
- "HashCodeOnly.java",
- "public class HashCodeOnly {",
- " // BUG: Diagnostic contains: HashCodeToString",
- " public int hashCode() {",
- " return 0;",
- " }",
- "}")
- .doTest();
- }
-
- @Test
- public void negative_bothHashCodeAndToString() {
- compilationHelper
- .addSourceLines(
- "HashCodeAndToString.java",
- "public class HashCodeAndToString {",
- " public int hashCode() {",
- " return 0;",
- " }",
- " public String toString() {",
- " return \"42\";",
- " }",
- "}")
- .doTest();
- }
-
- @Test
- public void negative_toStringOnly() {
- compilationHelper
- .addSourceLines(
- "ToStringOnly.java",
- "public class ToStringOnly {",
- " public String toString() {",
- " return \"42\";",
- " }",
- "}")
- .doTest();
- }
-
- @Test
- public void negative_neitherHashCodeAndToString() {
- compilationHelper
- .addSourceLines(
- "NeitherHashCodeAndToString.java", //
- "public class NeitherHashCodeAndToString {",
- "}")
- .doTest();
- }
-
- @Test
- public void superClassWithoutToString() {
- compilationHelper
- .addSourceLines("Super.java", "abstract class Super {}")
- .addSourceLines(
- "Test.java",
- "class Test extends Super {",
- " // BUG: Diagnostic contains: HashCodeToString",
- " public int hashCode() { return 0; }",
- "}")
- .doTest();
- }
-
- @Test
- public void inherited() {
- compilationHelper
- .addSourceLines(
- "Super.java",
- "class Super {",
- " public String toString() {",
- " return \"42\";",
- " }",
- "}")
- .addSourceLines(
- "Test.java", //
- "class Test extends Super {",
- " public int hashCode() { return 0; }",
- "}")
- .doTest();
- }
-
- @Test
- public void interfaceHashCode() {
- compilationHelper
- .addSourceLines(
- "I.java", //
- "interface I {",
- " int hashCode();",
- "}")
- .doTest();
- }
-
- @Test
- public void abstractToString() {
- compilationHelper
- .addSourceLines(
- "Super.java",
- "abstract class Super {",
- " public abstract int hashCode();",
- " public abstract String toString();",
- "}")
- .doTest();
- }
-
- @Test
- public void abstractNoToString() {
- compilationHelper
- .addSourceLines(
- "Super.java",
- "abstract class Super {",
- " // BUG: Diagnostic contains:",
- " public abstract int hashCode();",
- "}")
- .doTest();
- }
-
- @Test
- public void suppressOnHashCode() {
- compilationHelper
- .addSourceLines(
- "Test.java",
- "class Test {",
- " @SuppressWarnings(\"HashCodeToString\")",
- " public int hashCode() { return 0; }",
- "}")
- .doTest();
- }
-
- @Test
- public void nopHashCode() {
- compilationHelper
- .addSourceLines(
- "Test.java",
- "class Test {",
- " public int hashCode() {",
- " return super.hashCode();",
- " }",
- "}")
- .doTest();
- }
-
- @Test
- public void dontFlagAutoValue() {
- compilationHelper
- .addSourceLines(
- "Test.java",
- "import com.google.auto.value.AutoValue;",
- "@AutoValue",
- "abstract class Test {",
- " @Override",
- " public int hashCode() {",
- " return 1;",
- " }",
- "}")
- .doTest();
- }
-}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/IgnoredPureGetterTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/IgnoredPureGetterTest.java
index 0b8021114d1..20eaaf2e943 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/IgnoredPureGetterTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/IgnoredPureGetterTest.java
@@ -142,6 +142,94 @@ public void autoValue_secondFix() {
.doTest();
}
+ @Test
+ public void autoBuilder_getters() {
+ helper
+ .addSourceLines(
+ "Named.java",
+ "import com.google.auto.value.AutoBuilder;",
+ "import java.util.Optional;",
+ "public class Named {",
+ " Named(String name, String nickname) {}",
+ " @AutoBuilder",
+ " public abstract static class Builder {",
+ " public abstract Builder setName(String x);",
+ " public abstract Builder setNickname(String x);",
+ " abstract String getName();",
+ " abstract Optional getNickname();",
+ " abstract Named autoBuild();",
+ " public Named build() {",
+ " if (!getNickname().isPresent()) {",
+ " setNickname(getName());",
+ " }",
+ " return autoBuild();",
+ " }",
+ " }",
+ "}")
+ .addSourceLines(
+ "B.java",
+ "class B {",
+ " void test(Named.Builder builder) {",
+ // The setters are OK
+ " builder.setName(\"Stumpy\");",
+ " builder.setNickname(\"Stumps\");",
+ " // BUG: Diagnostic contains: IgnoredPureGetter",
+ " builder.getName();",
+ " // BUG: Diagnostic contains: IgnoredPureGetter",
+ " builder.getNickname();",
+ " // BUG: Diagnostic contains: IgnoredPureGetter",
+ " builder.autoBuild();",
+ // build() isn't covered since it's non-abstract
+ " builder.build();",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void autoBuilder_buildIsntCrv() {
+ helper
+ .addSourceLines(
+ "Named.java",
+ "import com.google.auto.value.AutoBuilder;",
+ "import java.time.LocalTime;",
+ "@AutoBuilder(callMethod = \"of\", ofClass = LocalTime.class)",
+ "interface LocalTimeBuilder {",
+ " LocalTimeBuilder setHour(int hour);",
+ " LocalTimeBuilder setMinute(int minute);",
+ " LocalTimeBuilder setSecond(int second);",
+ " LocalTimeBuilder setNanoOfSecond(int nanoOfSecond);",
+ " int getHour();",
+ " int getMinute();",
+ " int getSecond();",
+ " int getNanoOfSecond();",
+ " LocalTime build();", // calls: LocalTime.of(...)
+ "}")
+ .addSourceLines(
+ "B.java",
+ "class B {",
+ " void test(LocalTimeBuilder builder) {",
+ // the setters are treated as CIRV:
+ " builder.setHour(12);",
+ " builder.setMinute(12);",
+ " builder.setSecond(12);",
+ // the getters are treated as CRV:
+ " // BUG: Diagnostic contains: IgnoredPureGetter",
+ " builder.getHour();",
+ " // BUG: Diagnostic contains: IgnoredPureGetter",
+ " builder.getMinute();",
+ " // BUG: Diagnostic contains: IgnoredPureGetter",
+ " builder.getSecond();",
+ " // BUG: Diagnostic contains: IgnoredPureGetter",
+ " builder.getNanoOfSecond();",
+ // TODO(b/229107551): this shouldn't be treated as CRV
+ " // BUG: Diagnostic contains: IgnoredPureGetter",
+ " builder.build();",
+ " }",
+ "}")
+ .doTest();
+ }
+
@Test
public void refactoringHelper() {
refactoringHelper
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/NarrowCalculationTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/NarrowCalculationTest.java
new file mode 100644
index 00000000000..ad397944780
--- /dev/null
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/NarrowCalculationTest.java
@@ -0,0 +1,136 @@
+/*
+ * Copyright 2022 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;
+
+/** Tests for {@link NarrowCalculation}. */
+@RunWith(JUnit4.class)
+public final class NarrowCalculationTest {
+ private final CompilationTestHelper helper =
+ CompilationTestHelper.newInstance(NarrowCalculation.class, getClass());
+
+ private final BugCheckerRefactoringTestHelper refactoring =
+ BugCheckerRefactoringTestHelper.newInstance(NarrowCalculation.class, getClass());
+
+ @Test
+ public void integerDivision() {
+ helper
+ .addSourceLines(
+ "Test.java", //
+ "class Test {",
+ " // BUG: Diagnostic contains:",
+ " final float a = 1 / 2;",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void integerDivision_actuallyInteger() {
+ helper
+ .addSourceLines(
+ "Test.java", //
+ "class Test {",
+ " final float a = 8 / 2;",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void integerDivision_fix() {
+ refactoring
+ .addInputLines(
+ "Test.java", //
+ "class Test {",
+ " final float a = 1 / 2;",
+ "}")
+ .addOutputLines(
+ "Test.java", //
+ "class Test {",
+ " final float a = 1 / 2f;",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void longDivision_fix() {
+ refactoring
+ .addInputLines(
+ "Test.java", //
+ "class Test {",
+ " final double a = 1 / 2L;",
+ "}")
+ .addOutputLines(
+ "Test.java", //
+ "class Test {",
+ " final double a = 1 / 2.0;",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void targetTypeInteger_noFinding() {
+ helper
+ .addSourceLines(
+ "Test.java", //
+ "class Test {",
+ " final int a = 1 / 2;",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void multiplication_doesNotOverflow() {
+ helper
+ .addSourceLines(
+ "Test.java", //
+ "class Test {",
+ " final long a = 2 * 100;",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void multiplication_wouldOverflow() {
+ helper
+ .addSourceLines(
+ "Test.java", //
+ "class Test {",
+ " // BUG: Diagnostic contains:",
+ " final long a = 1_000_000_000 * 1_000_000_000;",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void multiplication_couldOverflow() {
+ helper
+ .addSourceLines(
+ "Test.java", //
+ "class Test {",
+ " void t(int a) {",
+ " // BUG: Diagnostic contains: 2L * a",
+ " long b = 2 * a;",
+ " }",
+ "}")
+ .doTest();
+ }
+}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/PublicConstructorForAbstractClassTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/PublicConstructorForAbstractClassTest.java
deleted file mode 100644
index a5a66160f14..00000000000
--- a/core/src/test/java/com/google/errorprone/bugpatterns/PublicConstructorForAbstractClassTest.java
+++ /dev/null
@@ -1,79 +0,0 @@
-/*
- * Copyright 2020 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 java.io.IOException;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-/** {@link PublicConstructorForAbstractClass}Test */
-@RunWith(JUnit4.class)
-public class PublicConstructorForAbstractClassTest {
-
- @Test
- public void basicRefactoringTest() throws IOException {
- BugCheckerRefactoringTestHelper testHelper =
- BugCheckerRefactoringTestHelper.newInstance(
- PublicConstructorForAbstractClass.class, getClass());
- testHelper
- .addInputLines(
- "in/Test.java", //
- "public abstract class Test {",
- " public Test() {",
- " }",
- "}")
- .addOutputLines(
- "out/Test.java", //
- "public abstract class Test {",
- " protected Test() {",
- " }",
- "}")
- .doTest();
- }
-
- @Test
- public void basicUsageTest() {
- CompilationTestHelper testHelper =
- CompilationTestHelper.newInstance(PublicConstructorForAbstractClass.class, getClass());
- testHelper
- .addSourceLines(
- "in/Test.java",
- "public abstract class Test {",
- " // BUG: Diagnostic contains:",
- " public Test() {",
- " }",
- "}")
- .doTest();
- }
-
- @Test
- public void basicNegativeTest() {
- CompilationTestHelper testHelper =
- CompilationTestHelper.newInstance(PublicConstructorForAbstractClass.class, getClass());
- testHelper
- .addSourceLines(
- "in/Test.java", //
- "public class Test {",
- " public Test() {",
- " }",
- "}")
- .doTest();
- }
-}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryBoxedVariableTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryBoxedVariableTest.java
index e7dddff7a49..26629b25957 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryBoxedVariableTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryBoxedVariableTest.java
@@ -17,6 +17,7 @@
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;
@@ -28,6 +29,8 @@
public class UnnecessaryBoxedVariableTest {
private final BugCheckerRefactoringTestHelper helper =
BugCheckerRefactoringTestHelper.newInstance(UnnecessaryBoxedVariable.class, getClass());
+ private final CompilationTestHelper compilationTestHelper =
+ CompilationTestHelper.newInstance(UnnecessaryBoxedVariable.class, getClass());
@Test
public void testCases() {
@@ -36,4 +39,34 @@ public void testCases() {
.addOutput("testdata/UnnecessaryBoxedVariableCases_expected.java")
.doTest();
}
+
+ @Test
+ public void suppression() {
+ helper
+ .addInputLines(
+ "Test.java",
+ "class Test {",
+ " @SuppressWarnings(\"UnnecessaryBoxedVariable\")",
+ " private int a(Integer o) {",
+ " return o;",
+ " }",
+ "}")
+ .expectUnchanged()
+ .doTest();
+ }
+
+ @Test
+ public void lambdas() {
+ compilationTestHelper
+ .addSourceLines(
+ "Test.java",
+ "class Test {",
+ " interface Boxed { void a(O b); }",
+ " void boxed(Boxed b) {}",
+ " private void test() {",
+ " boxed((Double a) -> { double b = a + 1; });",
+ " }",
+ "}")
+ .doTest();
+ }
}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryLambdaTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryLambdaTest.java
index 6279b0e2197..ad7253ecfe7 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryLambdaTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryLambdaTest.java
@@ -60,6 +60,42 @@ public void method() {
.doTest();
}
+ @Test
+ public void method_effectivelyPrivate() {
+ testHelper
+ .addInputLines(
+ "Test.java",
+ "import java.util.function.Function;",
+ "class Test {",
+ " private class Inner {",
+ " Function f() {",
+ " return x -> {",
+ " return \"hello \" + x;",
+ " };",
+ " }",
+ " void g() {",
+ " Function f = f();",
+ " System.err.println(f().apply(\"world\"));",
+ " }",
+ " }",
+ "}")
+ .addOutputLines(
+ "Test.java",
+ "import java.util.function.Function;",
+ "class Test {",
+ " private class Inner {",
+ " String f(String x) {",
+ " return \"hello \" + x;",
+ " }",
+ " void g() {",
+ " Function f = this::f;",
+ " System.err.println(f(\"world\"));",
+ " }",
+ " }",
+ "}")
+ .doTest();
+ }
+
@Test
public void method_static() {
testHelper
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/CheckReturnValuePositiveCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/CheckReturnValuePositiveCases.java
index 1f5f95f662b..e51e83c9e32 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/CheckReturnValuePositiveCases.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/CheckReturnValuePositiveCases.java
@@ -77,22 +77,7 @@ public void testBeforeAndAfterRule() {
}
public void constructor() {
- /*
- * We may or may not want to treat this as a bug. On the one hand, the
- * subclass might be "using" the superclass, so it might not be being
- * "ignored." (Plus, it would be a pain to produce a valid suggested fix
- * that incorporates any subclass constructor body, which might even contain
- * calls to methods in the class.) On the other hand, the more likely
- * scenario may be a class like IteratorTester, which requires (a) that the
- * user subclass it to implement a method and (b) that the user call test()
- * on the constructed object. There, it would be nice if IteratorTester
- * could be annotated with @CheckReturnValue to mean "anyone who creates an
- * anonymous subclasses of this should still do something with that
- * subclass." But perhaps that's an abuse of @CheckReturnValue.
- *
- * Anyway, these tests are here to ensure that subclasses don't don't crash
- * the compiler.
- */
+ // BUG: Diagnostic contains: Ignored return value
new MyObject() {};
class MySubObject1 extends MyObject {}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableCheckerTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableCheckerTest.java
index 83f3274581b..c09afccb0c7 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableCheckerTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableCheckerTest.java
@@ -2486,4 +2486,159 @@ private CompilationTestHelper withImmutableTypeParameterGeneric() {
"import com.google.errorprone.annotations.ImmutableTypeParameter;",
"class GenericWithImmutableParam<@ImmutableTypeParameter T> {}");
}
+
+ @Test
+ public void lambda_cannotCloseAroundMutableField() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "import com.google.errorprone.annotations.Immutable;",
+ "import java.util.ArrayList;",
+ "import java.util.List;",
+ "class Test {",
+ " @Immutable interface ImmutableFunction { A apply(B b); }",
+ " private int a = 0;",
+ " void test(ImmutableFunction f) {",
+ " // BUG: Diagnostic contains:",
+ " test(x -> ++a);",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void lambda_canCloseAroundImmutableField() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "import com.google.errorprone.annotations.Immutable;",
+ "import java.util.ArrayList;",
+ "import java.util.List;",
+ "class Test {",
+ " @Immutable interface ImmutableFunction { A apply(B b); }",
+ " private final int b = 1;",
+ " void test(ImmutableFunction f) {",
+ " test(x -> b);",
+ " test(x -> this.b);",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void lambda_cannotCloseAroundMutableLocal() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "import com.google.errorprone.annotations.Immutable;",
+ "import java.util.List;",
+ "import java.util.ArrayList;",
+ "class Test {",
+ " @Immutable interface ImmutableFunction { A apply(B b); }",
+ " void test(ImmutableFunction f) {",
+ " List xs = new ArrayList<>();",
+ " // BUG: Diagnostic contains:",
+ " test(x -> xs.get(x));",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void notImmutableAnnotatedLambda_noFinding() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "import java.util.ArrayList;",
+ "import java.util.List;",
+ "import java.util.function.Function;",
+ "class Test {",
+ " void test(Function f) {",
+ " List xs = new ArrayList<>();",
+ " test(x -> xs.get(x));",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void lambda_canHaveMutableVariablesWithin() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "import com.google.errorprone.annotations.Immutable;",
+ "import java.util.ArrayList;",
+ "import java.util.List;",
+ "class Test {",
+ " @Immutable interface ImmutableFunction { A apply(B b); }",
+ " void test(ImmutableFunction f) {",
+ " test(x -> { List xs = new ArrayList<>(); return xs.get(x); });",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void lambda_cannotCallMethodOnMutableClass() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "import com.google.errorprone.annotations.Immutable;",
+ "abstract class Test {",
+ " @Immutable interface ImmutableFunction { A apply(B b); }",
+ " abstract int mutable(int a);",
+ " void test(ImmutableFunction f) {",
+ " // BUG: Diagnostic contains: This lambda implements @Immutable interface"
+ + " 'ImmutableFunction', but accesses instance method(s) 'mutable' on 'Test' which"
+ + " is not @Immutable",
+ " test(x -> mutable(x));",
+ " // BUG: Diagnostic contains: This lambda implements @Immutable interface"
+ + " 'ImmutableFunction', but 'Test' has field 'this' of type 'Test', the"
+ + " declaration of type 'Test' is not annotated with"
+ + " @com.google.errorprone.annotations.Immutable",
+ " test(x -> this.mutable(x));",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void lambda_canCallMethodOnImmutableClass() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "import com.google.errorprone.annotations.Immutable;",
+ "@Immutable",
+ "abstract class Test {",
+ " @Immutable interface ImmutableFunction { A apply(B b); }",
+ " abstract int mutable(int a);",
+ " void test(ImmutableFunction f) {",
+ " test(x -> mutable(x));",
+ " test(x -> this.mutable(x));",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void subclassesOfMutableType() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "import com.google.errorprone.annotations.Immutable;",
+ "import java.util.function.Function;",
+ "@Immutable",
+ "abstract class Test {",
+ " @Immutable interface ImmutableFunction extends Function {",
+ " default ImmutableFunction andThen(ImmutableFunction fn) {",
+ // TODO(ghm): this one is sad, we're really accessing an immutable class's method here,
+ // but the owner of the method is not @Immutable. Look for a better heuristic to find
+ // the receiver type.
+ " // BUG: Diagnostic contains:",
+ " return x -> fn.apply(apply(x));",
+ " }",
+ " }",
+ "}")
+ .doTest();
+ }
}
diff --git a/docgen/pom.xml b/docgen/pom.xml
index a57b821986f..1a220ab90cc 100644
--- a/docgen/pom.xml
+++ b/docgen/pom.xml
@@ -21,7 +21,7 @@
com.google.errorprone
error_prone_parent
- HEAD-SNAPSHOT
+ 2.13.0
Documentation tool for generating Error Prone bugpattern documentation
diff --git a/docgen_processor/pom.xml b/docgen_processor/pom.xml
index 42915547130..601dda4395f 100644
--- a/docgen_processor/pom.xml
+++ b/docgen_processor/pom.xml
@@ -21,7 +21,7 @@
com.google.errorprone
error_prone_parent
- HEAD-SNAPSHOT
+ 2.13.0
JSR-269 annotation processor for @BugPattern annotation
diff --git a/docs/bugpattern/HashCodeToString.md b/docs/bugpattern/HashCodeToString.md
deleted file mode 100644
index 63ea5201c6d..00000000000
--- a/docs/bugpattern/HashCodeToString.md
+++ /dev/null
@@ -1,11 +0,0 @@
-Classes that override `hashCode()` should consider also overriding `toString()`
-with details to aid debugging and diagnostics, instead of relying on the default
-`Object.toString()` implementation.
-
-`Object.toString()` returns a string consisting of the class' name and the
-instances' hash code. When `hashCode()` is overridden this can be misleading, as
-users typically expect this default `toString()` to be (semi)unique
-per-instance, especially when debugging.
-
-See also
-[`MoreObjects.toStringHelper()`](https://guava.dev/releases/snapshot/api/docs/com/google/common/base/MoreObjects.html#toStringHelper-java.lang.Object-)
diff --git a/docs/bugpattern/NarrowCalculation.md b/docs/bugpattern/NarrowCalculation.md
new file mode 100644
index 00000000000..0d98656be2a
--- /dev/null
+++ b/docs/bugpattern/NarrowCalculation.md
@@ -0,0 +1,40 @@
+Integer division is suspicious when the target type of the expression is a
+float. For example:
+
+```java
+private static final double ONE_HALF = 1 / 2; // Actually 0.
+```
+
+If you specifically want the integer result, consider pulling out variable to
+make it clear what's happening. For example, instead of:
+
+```java
+// Deduct 10% from the grade for every week the assignment is late:
+float adjustedGrade = grade - (days / 7) * .1;
+```
+
+Prefer:
+
+```java
+// Deduct 10% from the grade for every week the assignment is late:
+int fullWeeks = days / 7;
+float adjustedGrade = grade - fullWeeks * .1;
+```
+
+Similarly, multiplication of two `int` values which are then cast to a `long` is
+problematic, as the `int` multiplication could overflow. It's better to perform
+the multiplication using `long` arithmetic.
+
+```java
+long secondsToNanos(int seconds) {
+ return seconds * 1_000_000_000; // Oops; starts overflowing around 2.15 seconds.
+}
+```
+
+Instead, prefer:
+
+```java
+long secondsToNanos(int seconds) {
+ return seconds * 1_000_000_000L; // Or ((long) seconds) * 1_000_000_000.
+}
+```
diff --git a/docs/bugpattern/StaticAssignmentOfThrowable.md b/docs/bugpattern/StaticAssignmentOfThrowable.md
index d25bd9e337d..f1894d2cdf8 100644
--- a/docs/bugpattern/StaticAssignmentOfThrowable.md
+++ b/docs/bugpattern/StaticAssignmentOfThrowable.md
@@ -6,7 +6,7 @@ necessary: it often isn't. Could a Throwable simply be instantiated when needed?
``` {.bad}
// this always has the same stack trace
-static final MY_EXCEPTION = new MyException("something terrible has happened!");
+static final MyException MY_EXCEPTION = new MyException("something terrible has happened!");
```
``` {.good}
diff --git a/pom.xml b/pom.xml
index f2161ec1eba..62734880590 100644
--- a/pom.xml
+++ b/pom.xml
@@ -21,7 +21,7 @@
Error Prone parent POM
com.google.errorprone
error_prone_parent
- HEAD-SNAPSHOT
+ 2.13.0
pom
Error Prone is a static analysis tool for Java that catches common programming mistakes at compile-time.
@@ -248,7 +248,6 @@
ossrh
https://oss.sonatype.org/
- true
diff --git a/refaster/pom.xml b/refaster/pom.xml
index d7951f0e492..ac35a4d8322 100644
--- a/refaster/pom.xml
+++ b/refaster/pom.xml
@@ -19,7 +19,7 @@
error_prone_parent
com.google.errorprone
- HEAD-SNAPSHOT
+ 2.13.0
4.0.0
diff --git a/test_helpers/pom.xml b/test_helpers/pom.xml
index 6d798ac870c..883d2ac323f 100644
--- a/test_helpers/pom.xml
+++ b/test_helpers/pom.xml
@@ -21,7 +21,7 @@
com.google.errorprone
error_prone_parent
- HEAD-SNAPSHOT
+ 2.13.0
error-prone test helpers
diff --git a/test_helpers/src/test/java/com/google/errorprone/CompilationTestHelperTest.java b/test_helpers/src/test/java/com/google/errorprone/CompilationTestHelperTest.java
index bd7bdf04bde..0fede355ec4 100644
--- a/test_helpers/src/test/java/com/google/errorprone/CompilationTestHelperTest.java
+++ b/test_helpers/src/test/java/com/google/errorprone/CompilationTestHelperTest.java
@@ -549,4 +549,26 @@ public void onlyCallDoTestOnce() {
assertThrows(IllegalStateException.class, () -> compilationHelper.doTest());
assertThat(expected).hasMessageThat().contains("doTest");
}
+
+ @Test
+ public void assertionErrors_causeTestFailures() {
+ var compilationTestHelper =
+ CompilationTestHelper.newInstance(AssertionFailingChecker.class, getClass())
+ .addSourceLines("test/Test.java", "package test;", "public class Test {}");
+ AssertionError expected =
+ assertThrows(AssertionError.class, () -> compilationTestHelper.doTest());
+ assertThat(expected)
+ .hasMessageThat()
+ .contains("An unhandled exception was thrown by the Error Prone static analysis plugin");
+ }
+
+ /** A BugPattern that always throws. */
+ @BugPattern(summary = "A really broken checker.", severity = ERROR)
+ public static class AssertionFailingChecker extends BugChecker
+ implements CompilationUnitTreeMatcher {
+ @Override
+ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) {
+ throw new AssertionError();
+ }
+ }
}
diff --git a/type_annotations/pom.xml b/type_annotations/pom.xml
index b2495a2167f..03cd271a6d8 100644
--- a/type_annotations/pom.xml
+++ b/type_annotations/pom.xml
@@ -21,7 +21,7 @@
com.google.errorprone
error_prone_parent
- HEAD-SNAPSHOT
+ 2.13.0
error-prone type annotations