From 403f8f338404da81126dbfb71c6f489c3a84aaec Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Sat, 18 May 2024 20:09:12 -0700 Subject: [PATCH 1/9] add NullUnmarked checks --- .../main/java/com/uber/nullaway/NullAway.java | 6 ++++- .../nullaway/generics/GenericsChecks.java | 4 ++- .../uber/nullaway/jspecify/GenericsTests.java | 27 +++++++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index e5de3a9e1b..1363645e56 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -738,9 +738,13 @@ public Description matchParameterizedType(ParameterizedTypeTree tree, VisitorSta if (!withinAnnotatedCode(state)) { return Description.NO_MATCH; } + + Symbol calledClass = ASTHelpers.getSymbol(tree); + boolean isNullUnmarked = codeAnnotationInfo.isSymbolUnannotated(calledClass, config, handler); + if (config.isJSpecifyMode()) { GenericsChecks.checkInstantiationForParameterizedTypedTree( - tree, state, this, config, handler); + isNullUnmarked, tree, state, this, config, handler); } return Description.NO_MATCH; } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index ffe45190ea..bdac7832ba 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -63,6 +63,7 @@ private GenericsChecks() {} * @param handler the handler instance */ public static void checkInstantiationForParameterizedTypedTree( + boolean isNullUnmarked, ParameterizedTypeTree tree, VisitorState state, NullAway analysis, @@ -105,7 +106,8 @@ public static void checkInstantiationForParameterizedTypedTree( upperBound.getAnnotationMirrors(); boolean hasNullableAnnotation = Nullness.hasNullableAnnotation(annotationMirrors.stream(), config) - || handler.onOverrideTypeParameterUpperBound(baseType.tsym.toString(), i); + || handler.onOverrideTypeParameterUpperBound(baseType.tsym.toString(), i) + || isNullUnmarked; // if base type argument does not have @Nullable annotation then the instantiation is // invalid if (!hasNullableAnnotation) { diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java index 1fac4d03a0..d5dd7cbe1a 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -1737,6 +1737,33 @@ public void testRawTypeReceiverCast() { .doTest(); } + @Test + public void testUseOfUnannotatedCode() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.NullMarked;", + "import org.jspecify.annotations.NullUnmarked;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " @NullUnmarked", + " static class nullUnmarkedClass {", + " static void m1(T t) {}", + " }", + " @NullMarked", + " static class markedClass {", + " static void testInstantiation() {", + " new nullUnmarkedClass<@Nullable String>();", + " }", + " static void testAssignment() {", + " nullUnmarkedClass<@Nullable Integer> var = null;", + " }", + " }", + "}") + .doTest(); + } + public void boxInteger() { makeHelper() .addSourceLines( From bec4bffe1db77676efb5bad44809d529f7cf1633 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Tue, 21 May 2024 13:57:39 -0700 Subject: [PATCH 2/9] formatting for CI jobs --- .../uber/nullaway/jspecify/GenericsTests.java | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java index d5dd7cbe1a..501ee99747 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -1740,28 +1740,28 @@ public void testRawTypeReceiverCast() { @Test public void testUseOfUnannotatedCode() { makeHelper() - .addSourceLines( - "Test.java", - "package com.uber;", - "import org.jspecify.annotations.NullMarked;", - "import org.jspecify.annotations.NullUnmarked;", - "import org.jspecify.annotations.Nullable;", - "class Test {", - " @NullUnmarked", - " static class nullUnmarkedClass {", - " static void m1(T t) {}", - " }", - " @NullMarked", - " static class markedClass {", - " static void testInstantiation() {", - " new nullUnmarkedClass<@Nullable String>();", - " }", - " static void testAssignment() {", - " nullUnmarkedClass<@Nullable Integer> var = null;", - " }", - " }", - "}") - .doTest(); + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.NullMarked;", + "import org.jspecify.annotations.NullUnmarked;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " @NullUnmarked", + " static class nullUnmarkedClass {", + " static void m1(T t) {}", + " }", + " @NullMarked", + " static class markedClass {", + " static void testInstantiation() {", + " new nullUnmarkedClass<@Nullable String>();", + " }", + " static void testAssignment() {", + " nullUnmarkedClass<@Nullable Integer> var = null;", + " }", + " }", + "}") + .doTest(); } public void boxInteger() { From 765e1ec58e215b9631e4deb8570e33a8f833010e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 24 May 2024 17:20:54 -0700 Subject: [PATCH 3/9] add castToNonNull --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 1363645e56..02d2061c51 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -739,7 +739,7 @@ public Description matchParameterizedType(ParameterizedTypeTree tree, VisitorSta return Description.NO_MATCH; } - Symbol calledClass = ASTHelpers.getSymbol(tree); + Symbol calledClass = castToNonNull(ASTHelpers.getSymbol(tree)); boolean isNullUnmarked = codeAnnotationInfo.isSymbolUnannotated(calledClass, config, handler); if (config.isJSpecifyMode()) { From a9bf295cbd283e45490f448f4628c73fdcbb6bf1 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Mon, 10 Jun 2024 01:22:40 -0700 Subject: [PATCH 4/9] code order change --- .../src/main/java/com/uber/nullaway/NullAway.java | 12 +++++++----- .../com/uber/nullaway/generics/GenericsChecks.java | 4 +--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 02d2061c51..082ae02ee6 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -739,13 +739,15 @@ public Description matchParameterizedType(ParameterizedTypeTree tree, VisitorSta return Description.NO_MATCH; } - Symbol calledClass = castToNonNull(ASTHelpers.getSymbol(tree)); - boolean isNullUnmarked = codeAnnotationInfo.isSymbolUnannotated(calledClass, config, handler); - if (config.isJSpecifyMode()) { - GenericsChecks.checkInstantiationForParameterizedTypedTree( - isNullUnmarked, tree, state, this, config, handler); + Symbol calledClass = ASTHelpers.getSymbol(tree); + boolean isNullUnmarked = codeAnnotationInfo.isSymbolUnannotated(calledClass, config, handler); + if(!isNullUnmarked) { + GenericsChecks.checkInstantiationForParameterizedTypedTree( + tree, state, this, config, handler); + } } + return Description.NO_MATCH; } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index bdac7832ba..ffe45190ea 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -63,7 +63,6 @@ private GenericsChecks() {} * @param handler the handler instance */ public static void checkInstantiationForParameterizedTypedTree( - boolean isNullUnmarked, ParameterizedTypeTree tree, VisitorState state, NullAway analysis, @@ -106,8 +105,7 @@ public static void checkInstantiationForParameterizedTypedTree( upperBound.getAnnotationMirrors(); boolean hasNullableAnnotation = Nullness.hasNullableAnnotation(annotationMirrors.stream(), config) - || handler.onOverrideTypeParameterUpperBound(baseType.tsym.toString(), i) - || isNullUnmarked; + || handler.onOverrideTypeParameterUpperBound(baseType.tsym.toString(), i); // if base type argument does not have @Nullable annotation then the instantiation is // invalid if (!hasNullableAnnotation) { From e623404f25000eab33f14b2b636fe379b5022064 Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Wed, 12 Jun 2024 15:45:49 -0700 Subject: [PATCH 5/9] formating --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 082ae02ee6..bb9dbe8015 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -742,9 +742,9 @@ public Description matchParameterizedType(ParameterizedTypeTree tree, VisitorSta if (config.isJSpecifyMode()) { Symbol calledClass = ASTHelpers.getSymbol(tree); boolean isNullUnmarked = codeAnnotationInfo.isSymbolUnannotated(calledClass, config, handler); - if(!isNullUnmarked) { + if (!isNullUnmarked) { GenericsChecks.checkInstantiationForParameterizedTypedTree( - tree, state, this, config, handler); + tree, state, this, config, handler); } } From 14cf1779fd2cd5a475005d343f6ba95a2e7c65fc Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Wed, 12 Jun 2024 17:35:27 -0700 Subject: [PATCH 6/9] cleanup and changing names --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 6 ++---- .../java/com/uber/nullaway/jspecify/GenericsTests.java | 9 +++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index bb9dbe8015..8fd6f8d843 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -738,16 +738,14 @@ public Description matchParameterizedType(ParameterizedTypeTree tree, VisitorSta if (!withinAnnotatedCode(state)) { return Description.NO_MATCH; } - if (config.isJSpecifyMode()) { - Symbol calledClass = ASTHelpers.getSymbol(tree); - boolean isNullUnmarked = codeAnnotationInfo.isSymbolUnannotated(calledClass, config, handler); + Symbol baseClass = ASTHelpers.getSymbol(tree); + boolean isNullUnmarked = codeAnnotationInfo.isSymbolUnannotated(baseClass, config, handler); if (!isNullUnmarked) { GenericsChecks.checkInstantiationForParameterizedTypedTree( tree, state, this, config, handler); } } - return Description.NO_MATCH; } diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java index 501ee99747..a1c87e68f4 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -1748,16 +1748,17 @@ public void testUseOfUnannotatedCode() { "import org.jspecify.annotations.Nullable;", "class Test {", " @NullUnmarked", - " static class nullUnmarkedClass {", + " static class NullUnmarkedClass {", " static void m1(T t) {}", " }", " @NullMarked", - " static class markedClass {", + " static class MarkedClass {", " static void testInstantiation() {", - " new nullUnmarkedClass<@Nullable String>();", + " // NullUnmarkedClass is marked @NullUnmarked, so we should not have an error even though the variable is marked @Nullable", + " new NullUnmarkedClass<@Nullable String>();", " }", " static void testAssignment() {", - " nullUnmarkedClass<@Nullable Integer> var = null;", + " NullUnmarkedClass<@Nullable Integer> var = null;", " }", " }", "}") From fce9c3f20fca7dee2f4857c98288fe6aef60d68b Mon Sep 17 00:00:00 2001 From: Yeahn Kim Date: Fri, 14 Jun 2024 16:32:19 -0700 Subject: [PATCH 7/9] debuged :nullaway:buildWithNullAway --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 03ee5112e4..1ed251427e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -713,7 +713,10 @@ public Description matchParameterizedType(ParameterizedTypeTree tree, VisitorSta } if (config.isJSpecifyMode()) { Symbol baseClass = ASTHelpers.getSymbol(tree); - boolean isNullUnmarked = codeAnnotationInfo.isSymbolUnannotated(baseClass, config, handler); + boolean isNullUnmarked = false; + if (baseClass != null) { + isNullUnmarked = codeAnnotationInfo.isSymbolUnannotated(baseClass, config, handler); + } if (!isNullUnmarked) { GenericsChecks.checkInstantiationForParameterizedTypedTree( tree, state, this, config, handler); From 055b25036a418fe3ae7ed6894ba09b3ccb50893d Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 14 Jun 2024 18:42:16 -0700 Subject: [PATCH 8/9] simplify --- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 1ed251427e..67c070dd7f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -713,10 +713,8 @@ public Description matchParameterizedType(ParameterizedTypeTree tree, VisitorSta } if (config.isJSpecifyMode()) { Symbol baseClass = ASTHelpers.getSymbol(tree); - boolean isNullUnmarked = false; - if (baseClass != null) { - isNullUnmarked = codeAnnotationInfo.isSymbolUnannotated(baseClass, config, handler); - } + boolean isNullUnmarked = + baseClass != null && codeAnnotationInfo.isSymbolUnannotated(baseClass, config, handler); if (!isNullUnmarked) { GenericsChecks.checkInstantiationForParameterizedTypedTree( tree, state, this, config, handler); From 06c01c1bc6f22e2c24f5caa8aaddeca48989ebf4 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 14 Jun 2024 18:44:20 -0700 Subject: [PATCH 9/9] tweaks --- .../java/com/uber/nullaway/jspecify/GenericsTests.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java index a1c87e68f4..e1294b460f 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -1748,13 +1748,13 @@ public void testUseOfUnannotatedCode() { "import org.jspecify.annotations.Nullable;", "class Test {", " @NullUnmarked", - " static class NullUnmarkedClass {", - " static void m1(T t) {}", - " }", + " static class NullUnmarkedClass {", + " }", " @NullMarked", " static class MarkedClass {", " static void testInstantiation() {", - " // NullUnmarkedClass is marked @NullUnmarked, so we should not have an error even though the variable is marked @Nullable", + " // NullUnmarkedClass is marked @NullUnmarked, so we get no error", + " // even though the type variable does not have a @Nullable upper bound", " new NullUnmarkedClass<@Nullable String>();", " }", " static void testAssignment() {",