Merge branch 'master' into 2024-12-21-jarinfer-wildcards

uber · Dec 22, 2024 · d27f267 · d27f267
2 parents 510cfaa + 2754c45
commit d27f267
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 1 deletion.
diff --git a/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/ModuleInfoTests.java b/jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/ModuleInfoTests.java
@@ -35,6 +35,7 @@ public void testModuleInfo() {
     defaultCompilationHelper
         .addSourceLines(
             "module-info.java",
+            "@SuppressWarnings(\"some-warning-name\")",
             "module com.uber.mymodule {",
             "  // Important: two-level deep module tests matching of identifier `java` as base expression;",
             "  // see further discussion at https://github.com/uber/NullAway/pull/544#discussion_r780829467",

diff --git a/jmh/src/test/java/com/uber/nullaway/jmh/BenchmarkCompilationTest.java b/jmh/src/test/java/com/uber/nullaway/jmh/BenchmarkCompilationTest.java
@@ -3,6 +3,7 @@
 import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
+import org.junit.Ignore;
 import org.junit.Test;
 
 /** Tests that all our JMH benchmarks compile successfully */
@@ -14,6 +15,7 @@ public void testAutodispose() throws IOException {
   }
 
   @Test
+  @Ignore("Need to update caffeine version; https://github.com/uber/NullAway/issues/1108")
   public void testCaffeine() throws IOException {
     assertTrue(new CaffeineCompiler().compile());
   }

diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
@@ -82,6 +82,7 @@
 import com.sun.source.tree.ReturnTree;
 import com.sun.source.tree.StatementTree;
 import com.sun.source.tree.SwitchTree;
+import com.sun.source.tree.SynchronizedTree;
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.TryTree;
 import com.sun.source.tree.TypeCastTree;
@@ -186,7 +187,8 @@ public class NullAway extends BugChecker
         BugChecker.CompoundAssignmentTreeMatcher,
         BugChecker.SwitchTreeMatcher,
         BugChecker.TypeCastTreeMatcher,
-        BugChecker.ParameterizedTypeTreeMatcher {
+        BugChecker.ParameterizedTypeTreeMatcher,
+        BugChecker.SynchronizedTreeMatcher {
 
   static final String INITIALIZATION_CHECK_NAME = "NullAway.Init";
   static final String OPTIONAL_CHECK_NAME = "NullAway.Optional";
@@ -486,6 +488,12 @@ public Description matchAssignment(AssignmentTree tree, VisitorState state) {
       doUnboxingCheck(state, tree.getExpression());
     }
     Symbol assigned = ASTHelpers.getSymbol(tree.getVariable());
+    if (assigned instanceof Symbol.MethodSymbol) {
+      // javac generates an AssignmentTree for setting an annotation attribute value.  E.g., for
+      // `@SuppressWarnings("foo")`, javac generates an AssignmentTree of the form `value() =
+      // "foo"`, where the LHS is a MethodSymbol.  We don't want to analyze these.
+      return Description.NO_MATCH;
+    }
     if (assigned != null && codeAnnotationInfo.isSymbolUnannotated(assigned, config, handler)) {
       // assigning to symbol that is unannotated
       return Description.NO_MATCH;
@@ -1762,6 +1770,27 @@ public Description matchEnhancedForLoop(EnhancedForLoopTree tree, VisitorState s
     return Description.NO_MATCH;
   }
 
+  @Override
+  public Description matchSynchronized(SynchronizedTree tree, VisitorState state) {
+    ExpressionTree lockExpr = tree.getExpression();
+    // For a synchronized block `synchronized (e) { ... }`, javac returns `(e)` as the expression.
+    // We strip the outermost parentheses for a nicer-looking error message.
+    if (lockExpr instanceof ParenthesizedTree) {
+      lockExpr = ((ParenthesizedTree) lockExpr).getExpression();
+    }
+    if (mayBeNullExpr(state, lockExpr)) {
+      ErrorMessage errorMessage =
+          new ErrorMessage(
+              MessageTypes.DEREFERENCE_NULLABLE,
+              "synchronized block expression \""
+                  + state.getSourceForNode(lockExpr)
+                  + "\" is @Nullable");
+      return errorBuilder.createErrorDescription(
+          errorMessage, buildDescription(lockExpr), state, null);
+    }
+    return Description.NO_MATCH;
+  }
+
   /**
    * Checks that all given expressions cannot be null, and for those that are {@code @Nullable},
    * reports an unboxing error.

diff --git a/nullaway/src/test/java/com/uber/nullaway/CoreTests.java b/nullaway/src/test/java/com/uber/nullaway/CoreTests.java
@@ -1071,4 +1071,23 @@ public void ternaryBothCasesNull() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void synchronizedDeref() {
+    defaultCompilationHelper
+        .addSourceLines(
+            "TestCase.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "public class TestCase {",
+            "  public static void testPositive(@Nullable Object lock) {",
+            "    // BUG: Diagnostic contains: synchronized block expression \"lock\" is @Nullable",
+            "    synchronized (lock) {}",
+            "  }",
+            "  public static void testNegative(Object lock) {",
+            "    synchronized (lock) {}",
+            "  }",
+            "}")
+        .doTest();
+  }
 }