Merge branch 'generic-method-lambda-arg-wildcard' of https://github.com/dhruv-agr/NullAway into generic-method-lambda-arg-wildcard

dhruv-agr · dhruv-agr · commit b2f848088dbf · 2026-01-27T13:16:24.000-08:00
diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml
@@ -61,6 +61,8 @@ jobs:
             url: https://github.com/ben-manes/caffeine.git
           - project: spring-framework
             url: https://github.com/spring-projects/spring-framework.git
+          - project: junit-framework
+            url: https://github.com/junit-team/junit-framework.git
     permissions:
       contents: read
     runs-on: ubuntu-latest
diff --git a/AGENTS.md b/AGENTS.md
@@ -8,3 +8,11 @@ test class or method within that module, you can use the `--tests` flag. For exa
 ```bash
 ./gradlew :nullaway:test --tests "com.uber.nullaway.NullAwayTest"
 ```
+
+# Changelog
+
+Our `CHANGELOG.md` file should be formatted as follows:
+
+* Link to PRs just via their number, e.g. `#1234`, not a full GitHub URL
+* Don't credit `@msridhar` in changelog entries, but credit all other contributors
+* Under maintenance, list sub-bullets with a `-` rather than a `*`
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,15 @@
 Changelog
 =========
 
+Version 0.13.1
+--------------
+
+* Improve verification of !null -> !null contracts (#1441)
+* Substitute inferred `@NonNull` types for generic method inference (#1445)
+* Better support for some contracts with boolean argument constraints (#1447)
+* Maintenance
+  - Add junit-framework as another integration test (#1446)
+
 Version 0.13.0
 --------------
 
diff --git a/gradle.properties b/gradle.properties
@@ -12,7 +12,7 @@ org.gradle.caching=true
 org.gradle.jvmargs=-Xmx2g -XX:MaxMetaspaceSize=512m
 
 GROUP=com.uber.nullaway
-VERSION_NAME=0.13.1-SNAPSHOT
+VERSION_NAME=0.13.2-SNAPSHOT
 
 POM_DESCRIPTION=A fast annotation-based null checker for Java
 
diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
@@ -211,6 +211,13 @@ public static boolean hasAnyAnnotationMatching(
         return true;
       }
     }
+    // we need this loop over the type's annotation mirrors in cases like explicitly-annotated
+    // lambda parameters, possibly due to bugs in javac
+    for (AnnotationMirror annotationMirror : symbol.type.getAnnotationMirrors()) {
+      if (predicate.test(annotationMirror.getAnnotationType().toString())) {
+        return true;
+      }
+    }
     // to handle bytecodes, also check direct type-use annotations stored in attributes
     Symbol typeAnnotationOwner =
         symbol.getKind().equals(ElementKind.PARAMETER) ? symbol.owner : symbol;
diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java
@@ -89,6 +89,13 @@ public String visitArrayType(Type.ArrayType t, @Nullable Void unused) {
     return sb.append(" []").toString();
   }
 
+  @Override
+  public String visitErrorType(Type.ErrorType t, @Nullable Void unused) {
+    // this arises for our synthetic @Nullable and @NonNull annotations; we just return the simple
+    // name
+    return t.tsym.getSimpleName().toString();
+  }
+
   @Override
   public String visitType(Type t, @Nullable Void s) {
     return t.toString();
diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java b/nullaway/src/main/java/com/uber/nullaway/generics/TypeSubstitutionUtils.java
@@ -224,18 +224,20 @@ private static Type substituteInferredNullabilityForTypeVariables(
     ListBuffer<Type> inferredTypes = new ListBuffer<>();
     for (Map.Entry<Element, ConstraintSolver.InferredNullability> entry :
         typeVarNullability.entrySet()) {
-      if (entry.getValue() == NULLABLE) {
-        // find all TypeVars occurring in targetType with the same symbol and substitute for those.
-        // we can have multiple such TypeVars due to previous substitutions that modified the type
-        // in some way, e.g., by changing its bounds
-        Element symbol = entry.getKey();
-        TypeVarWithSymbolCollector tvc = new TypeVarWithSymbolCollector(symbol);
-        targetType.accept(tvc, null);
-        for (Type.TypeVar tv : tvc.getMatches()) {
-          typeVars.append(tv);
-          inferredTypes.append(
-              typeWithAnnot(tv, GenericsChecks.getSyntheticNullableAnnotType(state)));
-        }
+      // find all TypeVars occurring in targetType with the same symbol and substitute for those.
+      // we can have multiple such TypeVars due to previous substitutions that modified the type
+      // in some way, e.g., by changing its bounds
+      Element symbol = entry.getKey();
+      TypeVarWithSymbolCollector tvc = new TypeVarWithSymbolCollector(symbol);
+      targetType.accept(tvc, null);
+      for (Type.TypeVar tv : tvc.getMatches()) {
+        typeVars.append(tv);
+        inferredTypes.append(
+            typeWithAnnot(
+                tv,
+                entry.getValue() == NULLABLE
+                    ? GenericsChecks.getSyntheticNullableAnnotType(state)
+                    : GenericsChecks.getSyntheticNonNullAnnotType(state)));
       }
     }
     List<Type> typeVarsToReplace = typeVars.toList();
diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractHandler.java
@@ -30,6 +30,7 @@
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.tools.javac.code.Symbol;
 import com.uber.nullaway.Config;
@@ -209,6 +210,18 @@ public NullnessHint onDataflowVisitMethodInvocation(
         if (valueConstraint.equals("_")) {
           // do nothing
         } else if (valueConstraint.equals("false") || valueConstraint.equals("true")) {
+          ExpressionTree argumentTree = tree.getArguments().get(i);
+          Object constValue = ASTHelpers.constValue(argumentTree);
+          if (constValue instanceof Boolean booleanValue) {
+            boolean booleanConstraintValue = valueConstraint.equals("true");
+            if (booleanValue == booleanConstraintValue) {
+              // Antecedent is satisfied by a compile-time boolean constant
+              continue;
+            }
+            // constant passed is opposite of antecedent
+            supported = false;
+            break;
+          }
           // We handle boolean constraints in the case that the boolean argument is the result
           // of a null or not-null check. For example,
           // '@Contract("true -> true") boolean func(boolean v)'
diff --git a/nullaway/src/test/java/com/uber/nullaway/ContractsTests.java b/nullaway/src/test/java/com/uber/nullaway/ContractsTests.java
@@ -732,6 +732,149 @@ String test(Object required) {
         .doTest();
   }
 
+  @Test
+  public void booleanToNotNullContract() {
+    makeTestHelperWithArgs(
+            withJSpecifyModeArgs(
+                Arrays.asList(
+                    "-d",
+                    temporaryFolder.getRoot().getAbsolutePath(),
+                    "-XepOpt:NullAway:OnlyNullMarked=true")))
+        .addSourceLines(
+            "Test.java",
+            """
+            import org.jspecify.annotations.NullMarked;
+            import org.jspecify.annotations.Nullable;
+            import org.jetbrains.annotations.Contract;
+            @NullMarked
+            class Test {
+              @Contract("false -> !null")
+              @Nullable String nonNullWhenPassedFalse(boolean returnNull) {
+                  if (returnNull) {
+                      return null;
+                  }
+                  return "foo";
+              }
+              @Contract("true -> !null")
+              @Nullable String nonNullWhenPassedTrue(boolean dontReturnNull) {
+                  if (dontReturnNull) {
+                      return "foo";
+                  }
+                  return null;
+              }
+              void testNegative() {
+                  nonNullWhenPassedFalse(false).hashCode();
+                  nonNullWhenPassedTrue(true).hashCode();
+              }
+              void testPositive(boolean b) {
+                  // BUG: Diagnostic contains: dereferenced expression
+                  nonNullWhenPassedFalse(true).hashCode();
+                  // false positive expected here since we do not do boolean reasoning
+                  // BUG: Diagnostic contains: dereferenced expression
+                  nonNullWhenPassedFalse(b && !b).hashCode();
+                  // BUG: Diagnostic contains: dereferenced expression
+                  nonNullWhenPassedTrue(false).hashCode();
+                  // false positive expected here since we do not do boolean reasoning
+                  // BUG: Diagnostic contains: dereferenced expression
+                  nonNullWhenPassedTrue(b || !b).hashCode();
+              }
+            }
+            """)
+        .doTest();
+  }
+
+  @Test
+  public void multiArgBooleanToNotNullContract() {
+    makeTestHelperWithArgs(
+            withJSpecifyModeArgs(
+                Arrays.asList(
+                    "-d",
+                    temporaryFolder.getRoot().getAbsolutePath(),
+                    "-XepOpt:NullAway:OnlyNullMarked=true")))
+        .addSourceLines(
+            "Test.java",
+            """
+            import org.jspecify.annotations.NullMarked;
+            import org.jspecify.annotations.Nullable;
+            import org.jetbrains.annotations.Contract;
+            @NullMarked
+            class Test {
+              @Contract("false, _ -> !null")
+              @Nullable String nonNullWhenFirstFalse(boolean returnNull, @Nullable String value) {
+                  if (returnNull) {
+                      return null;
+                  }
+                  return "foo";
+              }
+              @Contract("false, true -> !null")
+              @Nullable String nonNullWhenFalseTrue(boolean returnNull, boolean forceNonNull) {
+                  if (returnNull || !forceNonNull) {
+                      return null;
+                  }
+                  return "baz";
+              }
+              @Contract("_, true -> !null")
+              @Nullable String nonNullWhenSecondTrue(@Nullable String value, boolean dontReturnNull) {
+                  if (dontReturnNull) {
+                      return "bar";
+                  }
+                  return null;
+              }
+              void testNegative() {
+                  nonNullWhenFirstFalse(false, null).hashCode();
+                  nonNullWhenFalseTrue(false, true).hashCode();
+                  nonNullWhenSecondTrue(null, true).hashCode();
+              }
+              void testPositive(boolean b) {
+                  // BUG: Diagnostic contains: dereferenced expression
+                  nonNullWhenFirstFalse(true, null).hashCode();
+                  // BUG: Diagnostic contains: dereferenced expression
+                  nonNullWhenFalseTrue(false, false).hashCode();
+                  // BUG: Diagnostic contains: dereferenced expression
+                  nonNullWhenSecondTrue(null, false).hashCode();
+              }
+            }
+            """)
+        .doTest();
+  }
+
+  @Test
+  public void multiArgBooleanAndNullToNotNullContract() {
+    makeTestHelperWithArgs(
+            withJSpecifyModeArgs(
+                Arrays.asList(
+                    "-d",
+                    temporaryFolder.getRoot().getAbsolutePath(),
+                    "-XepOpt:NullAway:OnlyNullMarked=true")))
+        .addSourceLines(
+            "Test.java",
+            """
+            import org.jspecify.annotations.NullMarked;
+            import org.jspecify.annotations.Nullable;
+            import org.jetbrains.annotations.Contract;
+            @NullMarked
+            class Test {
+              @Contract("false, !null -> !null")
+              @Nullable String nonNullWhenFalseAndNonNull(boolean returnNull, @Nullable Object obj) {
+                  if (returnNull || obj == null) {
+                      return null;
+                  }
+                  return "ok";
+              }
+              void testNegative() {
+                  nonNullWhenFalseAndNonNull(false, new Object()).hashCode();
+              }
+              void testPositive(boolean b, @Nullable Object obj) {
+                  // BUG: Diagnostic contains: dereferenced expression
+                  nonNullWhenFalseAndNonNull(true, new Object()).hashCode();
+                  // BUG: Diagnostic contains: dereferenced expression
+                  nonNullWhenFalseAndNonNull(false, null).hashCode();
+              }
+            }
+            """)
+        .doTest();
+  }
+
   @Test
   public void checkNotNullToNotNullContract() {
     makeTestHelperWithArgs(
diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericLambdaTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericLambdaTests.java
@@ -133,6 +133,31 @@ void testNegative(Foo<String> foo) {
         .doTest();
   }
 
+  @Test
+  public void issue1448() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            """
+            package org.example;
+
+            import org.jspecify.annotations.NullMarked;
+            import org.jspecify.annotations.Nullable;
+
+            @NullMarked
+            class Test {
+                interface Func<T> {
+                    void apply( @Nullable T t );
+                }
+
+                class Bar {
+                    Func<Integer> baz = ( @Nullable Integer i ) -> {};
+                }
+            }
+            """)
+        .doTest();
+  }
+
   private CompilationTestHelper makeHelper() {
     return makeTestHelperWithArgs(
         JSpecifyJavacConfig.withJSpecifyModeArgs(
diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java
@@ -425,7 +425,7 @@ abstract class TestCase {
                 static class Bar<T extends @Nullable Object> {}
                 abstract <U> Bar<U> make(Bar<U> other);
                 void test(Bar<Bar<String>> other) {
-                    // BUG: Diagnostic contains: incompatible types: Bar<Bar<String>> cannot be converted to Bar<Bar<@Nullable String>>
+                    // BUG: Diagnostic contains: incompatible types: Bar<@NonNull Bar<String>> cannot be converted to Bar<Bar<@Nullable String>>
                     Bar<Bar<@Nullable String>> unused = make(other);
                 }
             }
@@ -1520,6 +1520,36 @@ void test(String s1, String s2, List<Object> l) {
         .doTest();
   }
 
+  @Test
+  public void issue1444() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            """
+            import org.jspecify.annotations.NullMarked;
+            import org.jspecify.annotations.Nullable;
+            import java.util.stream.Stream;
+            @NullMarked
+            public class Test {
+                static class DefaultConfiguration {
+                    final @Nullable Stream<? extends String> children;
+
+                    DefaultConfiguration(Stream<? extends String> children) {
+                        this.children = children;
+                    }
+                }
+                final Stream<? extends String> children;
+                Test(DefaultConfiguration configuration) {
+                    this.children = notNull(configuration.children, "children must not be null");
+                }
+                public static <T> T notNull(@Nullable T object, String message) {
+                    throw new RuntimeException("todo");
+                }
+            }
+            """)
+        .doTest();
+  }
+
   private CompilationTestHelper makeHelper() {
     return makeTestHelperWithArgs(
         JSpecifyJavacConfig.withJSpecifyModeArgs(

Original file line number	Diff line number	Diff line change
`@@ -211,6 +211,13 @@ public static boolean hasAnyAnnotationMatching(`
`211`	`211`	`return true;`
`212`	`212`	`}`
`213`	`213`	`}`
	`214`	`+ // we need this loop over the type's annotation mirrors in cases like explicitly-annotated`
	`215`	`+ // lambda parameters, possibly due to bugs in javac`
	`216`	`+ for (AnnotationMirror annotationMirror : symbol.type.getAnnotationMirrors()) {`
	`217`	`+ if (predicate.test(annotationMirror.getAnnotationType().toString())) {`
	`218`	`+ return true;`
	`219`	`+ }`
	`220`	`+ }`
`214`	`221`	`// to handle bytecodes, also check direct type-use annotations stored in attributes`
`215`	`222`	`Symbol typeAnnotationOwner =`
`216`	`223`	`symbol.getKind().equals(ElementKind.PARAMETER) ? symbol.owner : symbol;`