diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/DoubleCheckedLocking.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/DoubleCheckedLocking.java index cdea3cfabe9..0fbf92741ee 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/DoubleCheckedLocking.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/DoubleCheckedLocking.java @@ -40,6 +40,7 @@ import com.sun.source.tree.SynchronizedTree; import com.sun.source.tree.Tree; import com.sun.source.tree.Tree.Kind; +import com.sun.source.tree.UnaryTree; import com.sun.source.util.SimpleTreeVisitor; import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Symbol; @@ -233,21 +234,35 @@ static DclInfo create( /** * Matches comparisons to null (e.g. {@code foo == null}) and returns the expression being tested. + * + *
Also matches the logically equivalent form {@code !(foo != null)}, including with + * parentheses around either the whole condition or the negated comparison. */ private static @Nullable ExpressionTree getNullCheckedExpression(ExpressionTree condition) { condition = stripParentheses(condition); + // !(x != null) is equivalent to x == null + if (condition.getKind() == Kind.LOGICAL_COMPLEMENT) { + condition = stripParentheses(((UnaryTree) condition).getExpression()); + if (!(condition instanceof BinaryTree bin) || bin.getKind() != Kind.NOT_EQUAL_TO) { + return null; + } + return expressionComparedToNull(bin); + } if (!(condition instanceof BinaryTree bin)) { return null; } - ExpressionTree other; + return expressionComparedToNull(bin); + } + + /** Returns the non-null operand of a comparison against {@code null}, or null if none. */ + private static @Nullable ExpressionTree expressionComparedToNull(BinaryTree bin) { if (bin.getLeftOperand().getKind() == Kind.NULL_LITERAL) { - other = bin.getRightOperand(); - } else if (bin.getRightOperand().getKind() == Kind.NULL_LITERAL) { - other = bin.getLeftOperand(); - } else { - return null; + return bin.getRightOperand(); } - return other; + if (bin.getRightOperand().getKind() == Kind.NULL_LITERAL) { + return bin.getLeftOperand(); + } + return null; } /** diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/DoubleCheckedLockingTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/DoubleCheckedLockingTest.java index ec0c7de1d13..e02d71a113f 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/DoubleCheckedLockingTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/DoubleCheckedLockingTest.java @@ -53,6 +53,33 @@ void m() { .doTest(); } + // https://github.com/google/error-prone/issues/5963 + @Test + public void positiveNegatedNotEqualsNull() { + compilationHelper + .addSourceLines( + "threadsafety/Test.java", + """ + package threadsafety; + + class Test { + public Object x; + + void m() { + // BUG: Diagnostic contains: public volatile Object x + if (!(x != null)) { + synchronized (this) { + if (x == null) { + x = new Object(); + } + } + } + } + } + """) + .doTest(); + } + @Test public void positiveNoFix() { compilationHelper