From 72668849bd408e3264f1aeeaa413af7f161897d6 Mon Sep 17 00:00:00 2001 From: Lazaro Clapp Date: Wed, 18 Oct 2023 20:50:47 -0700 Subject: [PATCH 1/3] Prepare for release 0.10.15. --- CHANGELOG.md | 18 ++++++++++++++++++ gradle.properties | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26c65e8f08..f7b48b656e 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ Changelog ========= +Version 0.10.15 +--------------- +* [IMPORTANT] Update minimum Error Prone version and Guava version (#843) + NullAway now requires Error Prone 2.10.0 or later +* Add Spring mock/testing annotations to excluded field annotation list (#757) +* Update to Checker Framework 3.39.0 (#839) [Support for JDK 21 constructs] +* Support for JSpecify's 0.3.0 annotation [experimental] + - Properly check generic method overriding in explicitly-typed anonymous classes (#808) + - JSpecify: handle incorrect method parameter nullability for method reference (#845) + - JSpecify: initial handling of generic enclosing types for inner classes (#837) +* Build / CI tooling for NullAway itself: + - Update Gradle and a couple of plugin versions (#832) + - Run recent JDK tests on JDK 21 (#834) + - Fix which JDKs are installed on CI (#835) + - Update to Error Prone 2.22.0 (#833) + - Ignore code coverage for method executed non-deterministically in tests (#838 and #844) + - Build NullAway with JSpecify mode enabled (#841) + Version 0.10.14 --------------- IMPORTANT: This version introduces EXPERIMENTAL JDK21 support. diff --git a/gradle.properties b/gradle.properties index 452472526a..64ed79f3ca 100644 --- 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.10.15-SNAPSHOT +VERSION_NAME=0.10.15 POM_DESCRIPTION=A fast annotation-based null checker for Java From 099a7a581f24a59733c6acb38dc9a666547587fa Mon Sep 17 00:00:00 2001 From: Lazaro Clapp Date: Wed, 18 Oct 2023 21:14:34 -0700 Subject: [PATCH 2/3] Prepare next development version. --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 64ed79f3ca..86ddb10413 100644 --- 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.10.15 +VERSION_NAME=0.10.16-SNAPSHOT POM_DESCRIPTION=A fast annotation-based null checker for Java From 8f270e2c824176a1169dbf213340033f3f8a59b4 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Thu, 19 Oct 2023 10:07:20 -0700 Subject: [PATCH 3/3] JSpecify: handle return types of method references in Java Generics (#847) Adding new test case and the logic to handle the negative scenario of the test case where we were getting a false positive earlier. ```Java class Test { interface A { T1 function(Object o); } static @Nullable String foo(Object o) { return o.toString(); } static void testPositive() { // BUG: Diagnostic contains: referenced method returns @Nullable A p = Test::foo; } static void testNegative() { // We were getting a false positive error on this line as the Nullability of the overriden method was not identified A<@Nullable String> p = Test::foo; } } ``` All test cases in NullAwayJSpecifyGenericTests.java passed for these changes. --------- Co-authored-by: Manu Sridharan --- .../com/uber/nullaway/GenericsChecks.java | 14 +-- .../main/java/com/uber/nullaway/NullAway.java | 25 +++-- .../NullAwayJSpecifyGenericsTests.java | 100 ++++++++++++++++++ 3 files changed, 126 insertions(+), 13 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index cfba752fb2..af79f42486 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -702,11 +702,6 @@ public static void checkTypeParameterNullnessForMethodOverriding( public static Nullness getGenericMethodReturnTypeNullness( Symbol.MethodSymbol method, Symbol enclosingSymbol, VisitorState state, Config config) { Type enclosingType = getTypeForSymbol(enclosingSymbol, state); - if (enclosingType == null) { - // we have no additional information from generics, so return NONNULL (presence of a @Nullable - // annotation should have been handled by the caller) - return Nullness.NONNULL; - } return getGenericMethodReturnTypeNullness(method, enclosingType, state, config); } @@ -738,8 +733,13 @@ private static Type getTypeForSymbol(Symbol symbol, VisitorState state) { } } - private static Nullness getGenericMethodReturnTypeNullness( - Symbol.MethodSymbol method, Type enclosingType, VisitorState state, Config config) { + static Nullness getGenericMethodReturnTypeNullness( + Symbol.MethodSymbol method, @Nullable Type enclosingType, VisitorState state, Config config) { + if (enclosingType == null) { + // we have no additional information from generics, so return NONNULL (presence of a @Nullable + // annotation should have been handled by the caller) + return Nullness.NONNULL; + } Type overriddenMethodType = state.getTypes().memberType(enclosingType, method); verify( overriddenMethodType instanceof ExecutableType, diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 3a1f328cd5..5d8dc8304e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -957,7 +957,8 @@ private Description checkOverriding( // if the super method returns nonnull, overriding method better not return nullable // Note that, for the overriding method, the permissive default is non-null, // but it's nullable for the overridden one. - if (overriddenMethodReturnsNonNull(overriddenMethod, overridingMethod.owner, state) + if (overriddenMethodReturnsNonNull( + overriddenMethod, overridingMethod.owner, memberReferenceTree, state) && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL) .equals(Nullness.NULLABLE) && (memberReferenceTree == null @@ -996,18 +997,30 @@ && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL) } private boolean overriddenMethodReturnsNonNull( - Symbol.MethodSymbol overriddenMethod, Symbol enclosingSymbol, VisitorState state) { + Symbol.MethodSymbol overriddenMethod, + Symbol enclosingSymbol, + @Nullable MemberReferenceTree memberReferenceTree, + VisitorState state) { Nullness methodReturnNullness = getMethodReturnNullness(overriddenMethod, state, Nullness.NULLABLE); if (!methodReturnNullness.equals(Nullness.NONNULL)) { return false; } // In JSpecify mode, for generic methods, we additionally need to check the return nullness - // using the type parameters from the type enclosing the overriding method + // using the type arguments from the type enclosing the overriding method if (config.isJSpecifyMode()) { - return GenericsChecks.getGenericMethodReturnTypeNullness( - overriddenMethod, enclosingSymbol, state, config) - .equals(Nullness.NONNULL); + if (memberReferenceTree != null) { + // For a method reference, we get generic type arguments from javac's inferred type for the + // tree, which properly preserves type-use annotations + return GenericsChecks.getGenericMethodReturnTypeNullness( + overriddenMethod, ASTHelpers.getType(memberReferenceTree), state, config) + .equals(Nullness.NONNULL); + } else { + // Use the enclosing class of the overriding method to find generic type arguments + return GenericsChecks.getGenericMethodReturnTypeNullness( + overriddenMethod, enclosingSymbol, state, config) + .equals(Nullness.NONNULL); + } } return true; } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index b4d9ff1137..dd70683380 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -484,6 +484,106 @@ public void testForMethodReferenceInAnAssignment() { .doTest(); } + @Test + public void testForMethodReferenceForClassFieldAssignment() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface A {", + " T1 function(Object o);", + " }", + " static @Nullable String foo(Object o) {", + " return o.toString();", + " }", + " // BUG: Diagnostic contains: referenced method returns @Nullable", + " A positiveField = Test::foo;", + " A<@Nullable String> negativeField = Test::foo;", + "}") + .doTest(); + } + + @Test + public void testForMethodReferenceReturnTypeInAnAssignment() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface A {", + " T1 function(Object o);", + " }", + " static @Nullable String foo(Object o) {", + " return o.toString();", + " }", + " static void testPositive() {", + " // BUG: Diagnostic contains: referenced method returns @Nullable", + " A p = Test::foo;", + " }", + " static void testNegative() {", + " A<@Nullable String> p = Test::foo;", + " }", + "}") + .doTest(); + } + + @Test + public void testForMethodReferenceWhenReturned() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface A {", + " T1 function(Object o);", + " }", + " static @Nullable String foo(Object o) {", + " return o.toString();", + " }", + " static A testPositive() {", + " // BUG: Diagnostic contains: referenced method returns @Nullable", + " return Test::foo;", + " }", + " static A<@Nullable String> testNegative() {", + " return Test::foo;", + " }", + "}") + .doTest(); + } + + @Test + public void testForMethodReferenceAsMethodParameter() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface A {", + " T1 function(Object o);", + " }", + " static @Nullable String foo(Object o) {", + " return o.toString();", + " }", + " static void fooPositive(A a) {", + " }", + " static void fooNegative(A<@Nullable String> a) {", + " }", + " static void testPositive() {", + " // BUG: Diagnostic contains: referenced method returns @Nullable", + " fooPositive(Test::foo);", + " }", + " static void testNegative() {", + " fooNegative(Test::foo);", + " }", + "}") + .doTest(); + } + @Test public void testForLambdasInAnAssignment() { makeHelper()