From f050cba3272915ae96b9860a29d8c058b546b281 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 6 Aug 2024 17:21:49 -0700 Subject: [PATCH 1/7] test case and fix --- .../CheckIdenticalNullabilityVisitor.java | 17 +++++++++++++++++ .../uber/nullaway/jspecify/GenericsTests.java | 16 ++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java index dda48e44ca..f6d9561a03 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java @@ -5,6 +5,7 @@ import com.sun.tools.javac.code.Types; import java.util.List; import javax.lang.model.type.NullType; +import javax.lang.model.type.TypeMirror; /** * Visitor that checks for identical nullability annotations at all nesting levels within two types. @@ -23,6 +24,9 @@ public Boolean visitClassType(Type.ClassType lhsType, Type rhsType) { if (rhsType instanceof NullType || rhsType.isPrimitive()) { return true; } + if (lhsType.isIntersection()) { + return handleIntersectionType((Type.IntersectionClassType) lhsType, rhsType); + } Types types = state.getTypes(); // The base type of rhsType may be a subtype of lhsType's base type. In such cases, we must // compare lhsType against the supertype of rhsType with a matching base type. @@ -59,6 +63,19 @@ public Boolean visitClassType(Type.ClassType lhsType, Type rhsType) { return lhsType.getEnclosingType().accept(this, rhsType.getEnclosingType()); } + /** Check identical nullability for every type in the intersection */ + private Boolean handleIntersectionType( + Type.IntersectionClassType intersectionType, Type rhsType) { + List bounds = intersectionType.getBounds(); + for (int i = 0; i < bounds.size(); i++) { + Type bound = (Type) bounds.get(i); + if (!bound.accept(this, rhsType)) { + return false; + } + } + return true; + } + @Override public Boolean visitArrayType(Type.ArrayType lhsType, Type rhsType) { if (rhsType instanceof NullType) { 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 2a662b55a5..1a2b7d1d70 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -1856,6 +1856,22 @@ public void issue1008() { .doTest(); } + @Test + public void issue1013() { + makeHelper() + .addSourceLines( + "ServiceExtraInfo.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "public class ServiceExtraInfo {", + " private java.util.@Nullable List relativeServices;", + " private String getStr() {", + " return (relativeServices == null ? \"relativeServices == null\" : relativeServices.size()) + \"\";", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From 2ffb971a7b8a039aa9044c1883817a4528e4961e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 6 Aug 2024 17:24:21 -0700 Subject: [PATCH 2/7] rename test --- .../src/test/java/com/uber/nullaway/jspecify/GenericsTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1a2b7d1d70..680e391876 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -1857,7 +1857,7 @@ public void issue1008() { } @Test - public void issue1013() { + public void intersectionTypeFromConditionalExprInStringConcat() { makeHelper() .addSourceLines( "ServiceExtraInfo.java", From aee032cdb48b36e1d4080a2ace8724c86a3f46ea Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 6 Aug 2024 17:40:57 -0700 Subject: [PATCH 3/7] another test --- .../test/java/com/uber/nullaway/jspecify/GenericsTests.java | 3 +++ 1 file changed, 3 insertions(+) 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 680e391876..f2529fd2b8 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -1868,6 +1868,9 @@ public void intersectionTypeFromConditionalExprInStringConcat() { " private String getStr() {", " return (relativeServices == null ? \"relativeServices == null\" : relativeServices.size()) + \"\";", " }", + " private String getStr2(boolean b, java.util.List l) {", + " return (b ? (b ? l.size() : \"hello\") : 3) + \"\";", + " }", "}") .doTest(); } From 90e61dc3739155761691b52631b523a6062cc2b1 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 6 Aug 2024 17:42:56 -0700 Subject: [PATCH 4/7] todo --- .../src/test/java/com/uber/nullaway/jspecify/GenericsTests.java | 2 ++ 1 file changed, 2 insertions(+) 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 f2529fd2b8..e0c7a3ec08 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -1875,6 +1875,8 @@ public void intersectionTypeFromConditionalExprInStringConcat() { .doTest(); } + // TODO write an intersection test where nullability of type parameters does not match + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From 06ccd500e23d5a0aa2f9554fdf9681579868191b Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 6 Aug 2024 20:43:02 -0700 Subject: [PATCH 5/7] more --- .../CheckIdenticalNullabilityVisitor.java | 11 ++------ .../GenericTypePrettyPrintingVisitor.java | 9 +++++++ .../uber/nullaway/jspecify/GenericsTests.java | 27 ++++++++++++++++++- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java index f6d9561a03..7bf0b44e50 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/CheckIdenticalNullabilityVisitor.java @@ -5,7 +5,6 @@ import com.sun.tools.javac.code.Types; import java.util.List; import javax.lang.model.type.NullType; -import javax.lang.model.type.TypeMirror; /** * Visitor that checks for identical nullability annotations at all nesting levels within two types. @@ -66,14 +65,8 @@ public Boolean visitClassType(Type.ClassType lhsType, Type rhsType) { /** Check identical nullability for every type in the intersection */ private Boolean handleIntersectionType( Type.IntersectionClassType intersectionType, Type rhsType) { - List bounds = intersectionType.getBounds(); - for (int i = 0; i < bounds.size(); i++) { - Type bound = (Type) bounds.get(i); - if (!bound.accept(this, rhsType)) { - return false; - } - } - return true; + return intersectionType.getBounds().stream() + .allMatch(type -> ((Type) type).accept(this, rhsType)); } @Override diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java index b6c2f9c991..f0f5f5b4cc 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java @@ -37,6 +37,9 @@ public String visitWildcardType(Type.WildcardType t, Void unused) { @Override public String visitClassType(Type.ClassType t, Void s) { + if (t.isIntersection()) { + return prettyIntersectionType((Type.IntersectionClassType) t); + } StringBuilder sb = new StringBuilder(); Type enclosingType = t.getEnclosingType(); if (!ASTHelpers.isSameType(enclosingType, Type.noType, state)) { @@ -57,6 +60,12 @@ public String visitClassType(Type.ClassType t, Void s) { return sb.toString(); } + private String prettyIntersectionType(Type.IntersectionClassType t) { + return t.getBounds().stream() + .map(type -> ((Type) type).accept(this, null)) + .collect(joining(" & ")); + } + @Override public String visitCapturedType(Type.CapturedType t, Void s) { return t.wildcard.accept(this, null); 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 e0c7a3ec08..385168551b 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -1875,7 +1875,32 @@ public void intersectionTypeFromConditionalExprInStringConcat() { .doTest(); } - // TODO write an intersection test where nullability of type parameters does not match + @Test + public void intersectionTypeInvalidAssign() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import java.io.Serializable;", + "public class Test {", + " interface A {}", + " static class B implements A<@Nullable String>, Serializable {}", + " static void test1(Object o) {", + " var x = (A & Serializable) o;", + " // BUG: Diagnostic contains: Cannot assign from type B to type A & Serializable", + " x = new B();", + " }", + " static class C implements A, Serializable {}", + " static void test2(Object o) {", + " var x = (A<@Nullable String> & Serializable) o;", + // TODO: this assignment should be an error but we do not compute annotations in the + // type of x correctly + " x = new C();", + " }", + "}") + .doTest(); + } private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( From 564b1ba2fbecc792270c9d7960dfbe6a8e3b41c5 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 19 Aug 2024 10:16:01 -0700 Subject: [PATCH 6/7] add @Test annotation --- .../src/test/java/com/uber/nullaway/jspecify/GenericsTests.java | 1 + 1 file changed, 1 insertion(+) 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 67ce7c811c..d3683cb302 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -1902,6 +1902,7 @@ public void intersectionTypeInvalidAssign() { .doTest(); } + @Test public void issue1014() { makeHelper() .addSourceLines( From 5a8bdd6393f0d6b2c54801b420dccdb46bd72b17 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 19 Aug 2024 10:22:40 -0700 Subject: [PATCH 7/7] add tests and issue link --- .../java/com/uber/nullaway/jspecify/GenericsTests.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 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 d3683cb302..869f736790 100644 --- a/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java @@ -1886,16 +1886,20 @@ public void intersectionTypeInvalidAssign() { "public class Test {", " interface A {}", " static class B implements A<@Nullable String>, Serializable {}", + " static class C implements A, Serializable {}", " static void test1(Object o) {", " var x = (A & Serializable) o;", " // BUG: Diagnostic contains: Cannot assign from type B to type A & Serializable", " x = new B();", + " // ok", + " x = new C();", " }", - " static class C implements A, Serializable {}", " static void test2(Object o) {", " var x = (A<@Nullable String> & Serializable) o;", - // TODO: this assignment should be an error but we do not compute annotations in the - // type of x correctly + // TODO: should _not_ be an error, see https://github.com/uber/NullAway/issues/1022 + " // BUG: Diagnostic contains: Cannot assign from type B to type A & Serializable", + " x = new B();", + // TODO: _should_ be an error, see https://github.com/uber/NullAway/issues/1022 " x = new C();", " }", "}")