From 7c8ac63ab855c9bd84a843a9e7feb92f044d62fd Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 21 Jan 2023 16:04:14 -0800 Subject: [PATCH] WIP --- build.gradle | 2 +- gradle/dependencies.gradle | 1 + nullaway/build.gradle | 13 + .../nullaway/NullAwayAccessPathsTests.java | 191 +++-- ...cknowledgeRestrictiveAnnotationsTests.java | 267 ++++--- .../nullaway/NullAwayAssertionLibsTests.java | 106 +-- .../NullAwayAutoSuggestNoCastTest.java | 159 +++- .../nullaway/NullAwayAutoSuggestTest.java | 39 +- .../NullAwayBytecodeInteractionsTests.java | 15 +- .../NullAwayContractsBooleanTests.java | 169 +++-- .../uber/nullaway/NullAwayContractsTests.java | 138 +++- .../com/uber/nullaway/NullAwayCoreTests.java | 712 +++++++++++------- .../NullAwayCustomLibraryModelsTests.java | 56 +- .../nullaway/NullAwayEnsuresNonNullTests.java | 123 ++- .../uber/nullaway/NullAwayFrameworkTests.java | 150 ++-- .../NullAwayGuavaAssertionsTests.java | 82 +- .../nullaway/NullAwayInitializationTests.java | 30 +- .../NullAwayJSpecifyGenericsTests.java | 204 +++-- .../uber/nullaway/NullAwayJSpecifyTests.java | 216 +++++- .../nullaway/NullAwayKeySetIteratorTests.java | 58 +- .../NullAwayOptionalEmptinessTests.java | 383 ++++++---- .../uber/nullaway/NullAwayThriftTests.java | 93 ++- .../NullAwayTypeUseAnnotationTests.java | 18 +- .../NullAwayTypeUseAnnotationsTests.java | 73 +- .../nullaway/NullAwayUnannotatedTests.java | 73 +- .../nullaway/NullAwayUnsoundnessTests.java | 2 + .../uber/nullaway/NullAwayVarargsTests.java | 38 +- .../thirdpartylibs/NullAwayGrpcTest.java | 74 +- 28 files changed, 2324 insertions(+), 1161 deletions(-) diff --git a/build.gradle b/build.gradle index 33d5ff664f..cf2bdb7a7a 100644 --- a/build.gradle +++ b/build.gradle @@ -62,7 +62,7 @@ subprojects { project -> "-Xlint:deprecation", "-Xlint:rawtypes", "-Xlint:unchecked", - "-Werror" + //"-Werror" ] options.errorprone { // disable warnings in generated code; AutoValue code fails UnnecessaryParentheses check diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index ef6765cbf5..c1c6c0066b 100755 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -62,6 +62,7 @@ def apt = [ autoServiceAnnot : "com.google.auto.service:auto-service-annotations:${versions.autoService}", jakartaInject : "jakarta.inject:jakarta.inject-api:2.0.0", javaxInject : "javax.inject:javax.inject:1", + errorProneSupport: "tech.picnic.error-prone-support:error-prone-contrib:0.7.0" ] def build = [ diff --git a/nullaway/build.gradle b/nullaway/build.gradle index d6c4463dca..9b36adec4a 100644 --- a/nullaway/build.gradle +++ b/nullaway/build.gradle @@ -30,6 +30,7 @@ dependencies { annotationProcessor deps.apt.autoValue compileOnly deps.apt.autoServiceAnnot annotationProcessor deps.apt.autoService + errorprone deps.apt.errorProneSupport compileOnly deps.build.jsr305Annotations compileOnly deps.test.jetbrainsAnnotations @@ -38,6 +39,8 @@ dependencies { implementation deps.build.checkerDataflow implementation deps.build.guava + //testAnnotationProcessor deps.apt.errorProneSupport + testImplementation project(":annotations") testImplementation deps.test.junit4 testImplementation(deps.build.errorProneTestHelpers) { @@ -69,6 +72,16 @@ dependencies { nullawayJar "com.uber.nullaway:nullaway:$VERSION_NAME" } +tasks.withType(JavaCompile) { + options.errorprone { + check("StaticImport", CheckSeverity.OFF) + check("CanonicalAnnotationSyntax", CheckSeverity.OFF) + check("FormatStringConcatenation", CheckSeverity.OFF) + check("CollectorMutability", CheckSeverity.OFF) + check("IdentityConversion", CheckSeverity.OFF) + errorproneArgs.addAll("-XepPatchChecks:ErrorProneTestHelperSourceFormat", "-XepPatchLocation:IN_PLACE") + } +} javadoc { failOnError = false } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAccessPathsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayAccessPathsTests.java index f3f0c07b2f..8eda668247 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayAccessPathsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayAccessPathsTests.java @@ -10,25 +10,30 @@ public void testConstantsInAccessPathsNegative() { .addSourceLines( "NullableContainer.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public interface NullableContainer {", - " @Nullable public V get(K k);", + " @Nullable", + " public V get(K k);", "}") .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "public class Test {", " public void testSingleStringCheck(NullableContainer c) {", " if (c.get(\"KEY_STR\") != null) {", " c.get(\"KEY_STR\").toString(); // is safe", " }", " }", + "", " public void testSingleIntCheck(NullableContainer c) {", " if (c.get(42) != null) {", " c.get(42).toString(); // is safe", " }", " }", + "", " public void testMultipleChecks(NullableContainer> c) {", " if (c.get(\"KEY_STR\") != null && c.get(\"KEY_STR\").get(42) != null) {", " c.get(\"KEY_STR\").get(42).toString(); // is safe", @@ -44,14 +49,17 @@ public void testConstantsInAccessPathsPositive() { .addSourceLines( "NullableContainer.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public interface NullableContainer {", - " @Nullable public V get(K k);", + " @Nullable", + " public V get(K k);", "}") .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "public class Test {", " public void testEnhancedFor(NullableContainer> c) {", " if (c.get(\"KEY_STR\") != null && c.get(\"KEY_STR\").get(0) != null) {", @@ -69,21 +77,25 @@ public void testVariablesInAccessPathsNegative() { .addSourceLines( "NullableContainer.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public interface NullableContainer {", - " @Nullable public V get(K k);", + " @Nullable", + " public V get(K k);", "}") .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "public class Test {", - " private static final int INT_KEY = 42;", // Guaranteed constant! + " private static final int INT_KEY = 42;", + "", " public void testEnhancedFor(NullableContainer> c) {", " if (c.get(\"KEY_STR\") != null && c.get(\"KEY_STR\").get(INT_KEY) != null) {", " c.get(\"KEY_STR\").get(INT_KEY).toString();", " c.get(\"KEY_STR\").get(Test.INT_KEY).toString();", - " c.get(\"KEY_STR\").get(42).toString();", // Extra magic! + " c.get(\"KEY_STR\").get(42).toString();", " }", " }", "}") @@ -96,16 +108,20 @@ public void testVariablesInAccessPathsPositive() { .addSourceLines( "NullableContainer.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public interface NullableContainer {", - " @Nullable public V get(K k);", + " @Nullable", + " public V get(K k);", "}") .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "public class Test {", - " private Integer intKey = 42;", // No guarantee it's a constant + " private Integer intKey = 42;", + "", " public void testEnhancedFor(NullableContainer> c) {", " if (c.get(\"KEY_STR\") != null && c.get(\"KEY_STR\").get(this.intKey) != null) {", " // BUG: Diagnostic contains: dereferenced expression c.get(\"KEY_STR\").get", @@ -122,15 +138,16 @@ public void testField() { .addSourceLines( "Foo.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public class Foo {", - " @Nullable public Object o;", + " @Nullable public Object o;", "}") .addSourceLines( "Test.java", "package com.uber;", - "import java.util.ArrayList;", - "import javax.annotation.Nullable;", + "", "public class Test {", " public String testFieldCheck(Foo foo) {", " if (foo.o == null) {", @@ -148,16 +165,19 @@ public void testArrayListField() { .addSourceLines( "Foo.java", "package com.uber;", + "", "import java.util.List;", "import javax.annotation.Nullable;", + "", "public class Foo {", - " @Nullable public List list;", + " @Nullable public List list;", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.ArrayList;", - "import javax.annotation.Nullable;", + "", "public class Test {", " public Object testFieldCheck(Foo foo) {", " if (foo.list == null) {", @@ -175,15 +195,16 @@ public void testFieldWithoutValidAccessPath() { .addSourceLines( "Foo.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public class Foo {", - " @Nullable public Object o;", + " @Nullable public Object o;", "}") .addSourceLines( "Test.java", "package com.uber;", - "import java.util.ArrayList;", - "import javax.annotation.Nullable;", + "", "public class Test {", " public String testFieldCheck(Foo foo) {", " if (foo.o == null) {", @@ -202,18 +223,18 @@ public void testFieldWithoutValidAccessPathLongChain() { .addSourceLines( "Foo.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "public class Foo {", - " public Foo nonNull;", - " public Foo() {", - " nonNull = this;", - " }", + " public Foo nonNull;", + "", + " public Foo() {", + " nonNull = this;", + " }", "}") .addSourceLines( "Test.java", "package com.uber;", - "import java.util.ArrayList;", - "import javax.annotation.Nullable;", + "", "public class Test {", " public String testFieldCheck(Foo foo) {", " // Just checking that NullAway doesn't crash on a long but ultimately rootless access path", @@ -229,61 +250,74 @@ public void testFinalFieldNullabilityPropagatesToInnerContexts() { .addSourceLines( "Foo.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public class Foo {", - " @Nullable public final Bar bar;", - " @Nullable public Bar mutableBar;", - " public Foo(@Nullable Bar bar) {", - " this.bar = bar;", - " this.mutableBar = bar;", - " }", + " @Nullable public final Bar bar;", + " @Nullable public Bar mutableBar;", + "", + " public Foo(@Nullable Bar bar) {", + " this.bar = bar;", + " this.mutableBar = bar;", + " }", "}") .addSourceLines( "Bar.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public class Bar {", - " @Nullable public final Foo foo;", - " @Nullable public Foo mutableFoo;", - " public Bar(@Nullable Foo foo) {", - " this.foo = foo;", - " this.mutableFoo = foo;", - " }", + " @Nullable public final Foo foo;", + " @Nullable public Foo mutableFoo;", + "", + " public Bar(@Nullable Foo foo) {", + " this.foo = foo;", + " this.mutableFoo = foo;", + " }", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "import com.google.common.base.Preconditions;", - "import java.util.ArrayList;", - "import java.util.function.Predicate;", "import java.util.function.Function;", + "import java.util.function.Predicate;", "import javax.annotation.Nullable;", + "", "public class Test {", " @Nullable private final Foo foo;", " @Nullable private Foo mutableFoo;", + "", " public Test(@Nullable Foo foo) {", " this.foo = foo;", " this.mutableFoo = foo;", " }", + "", " public Predicate testReadFinalFromLambdaNoCheck() {", " // BUG: Diagnostic contains: dereferenced expression foo is @Nullable", " return (s -> foo.toString().equals(s));", " }", + "", " public Predicate testReadFinalFromLambdaAfterCheck() {", " Preconditions.checkNotNull(foo);", " // safe!", " return (s -> foo.toString().equals(s));", " }", + "", " public Predicate testReadMutableFromLambdaAfterCheck() {", " Preconditions.checkNotNull(mutableFoo);", " // BUG: Diagnostic contains: dereferenced expression mutableFoo is @Nullable", " return (s -> mutableFoo.toString().equals(s));", " }", + "", " public Function> testReadFinalFromLambdaAfterCheckDeepContext() {", " Preconditions.checkNotNull(foo);", " // safe!", " return (s1 -> (s2 -> foo.toString().equals(s1 + s2)));", " }", + "", " public Predicate testReadFinalFromLambdaAfterCheckDeepAP() {", " Preconditions.checkNotNull(foo);", " Preconditions.checkNotNull(foo.bar);", @@ -291,12 +325,14 @@ public void testFinalFieldNullabilityPropagatesToInnerContexts() { " // safe!", " return (s -> foo.bar.foo.toString().equals(s));", " }", + "", " public Predicate testReadFinalFromLambdaAfterCheckDeepAPIncomplete() {", " Preconditions.checkNotNull(foo);", " Preconditions.checkNotNull(foo.bar);", " // BUG: Diagnostic contains: dereferenced expression foo.bar.foo is @Nullable", " return (s -> foo.bar.foo.toString().equals(s));", " }", + "", " public Predicate testReadMutableFromLambdaAfterCheckDeepAP1() {", " Preconditions.checkNotNull(foo);", " Preconditions.checkNotNull(foo.mutableBar);", @@ -304,6 +340,7 @@ public void testFinalFieldNullabilityPropagatesToInnerContexts() { " // BUG: Diagnostic contains: dereferenced expression foo.mutableBar is @Nullable", " return (s -> foo.mutableBar.foo.toString().equals(s));", " }", + "", " public Predicate testReadMutableFromLambdaAfterCheckDeepAP2() {", " Preconditions.checkNotNull(foo);", " Preconditions.checkNotNull(foo.bar);", @@ -311,64 +348,78 @@ public void testFinalFieldNullabilityPropagatesToInnerContexts() { " // BUG: Diagnostic contains: dereferenced expression foo.bar.mutableFoo is @Nullable", " return (s -> foo.bar.mutableFoo.toString().equals(s));", " }", + "", " public boolean testReadFinalFromLambdaAfterCheckLocalClass(String s) {", " Preconditions.checkNotNull(foo);", " // safe!", " class Inner {", - " public Inner() { }", - " public boolean doTest(String s) { return foo.toString().equals(s); }", + " public Inner() {}", + "", + " public boolean doTest(String s) {", + " return foo.toString().equals(s);", + " }", " }", " return (new Inner()).doTest(s);", " }", + "", " public boolean testReadFinalFromLambdaCheckBeforeUseLocalClass(String s) {", " class Inner {", - " public Inner() { }", - " // At the time of declaring this, foo is not known to be non-null!", - " public boolean doTest(String s) {", - " // BUG: Diagnostic contains: dereferenced expression foo is @Nullable", - " return foo.toString().equals(s);", - " }", + " public Inner() {}", + " // At the time of declaring this, foo is not known to be non-null!", + " public boolean doTest(String s) {", + " // BUG: Diagnostic contains: dereferenced expression foo is @Nullable", + " return foo.toString().equals(s);", + " }", " }", " // Technically safe, but hard to reason about: needs to be aware of *when* doTest() can be", " // called which is generally _beyond_ NullAway.", " Preconditions.checkNotNull(foo);", " return (new Inner()).doTest(s);", " }", + "", " public boolean testReadMutableFromLambdaAfterCheckLocalClass(String s) {", " Preconditions.checkNotNull(mutableFoo);", " class Inner {", - " public Inner() { }", - " public boolean doTest(String s) {", - " // BUG: Diagnostic contains: dereferenced expression mutableFoo is @Nullable", - " return mutableFoo.toString().equals(s);", - " }", - " public boolean doTestSafe(String s) {", - " Preconditions.checkNotNull(mutableFoo);", - " return mutableFoo.toString().equals(s);", - " }", + " public Inner() {}", + "", + " public boolean doTest(String s) {", + " // BUG: Diagnostic contains: dereferenced expression mutableFoo is @Nullable", + " return mutableFoo.toString().equals(s);", + " }", + "", + " public boolean doTestSafe(String s) {", + " Preconditions.checkNotNull(mutableFoo);", + " return mutableFoo.toString().equals(s);", + " }", " }", " if (s.length() % 2 == 0) {", " return (new Inner()).doTest(s);", " } else {", - " // safe!", + " // safe!", " return (new Inner()).doTestSafe(s);", " }", " }", + "", " public boolean testReadFinalFromLambdaAfterCheckLocalClassWithNameCollision(String s) {", " Preconditions.checkNotNull(foo);", " class Inner {", - " @Nullable private Foo foo;", - " public Inner() { this.foo = null; }", - " public boolean doTest(String s) {", - " // BUG: Diagnostic contains: dereferenced expression foo is @Nullable", - " return foo.toString().equals(s);", - " }", - " public boolean doTestSafe(String s) {", - " // TODO: Technically safe, but it would need recognizing Test.this.[...] as the same AP as", - " // that from the closure.", - " // BUG: Diagnostic contains: dereferenced expression Test.this.foo is @Nullable", - " return Test.this.foo.toString().equals(s);", - " }", + " @Nullable private Foo foo;", + "", + " public Inner() {", + " this.foo = null;", + " }", + "", + " public boolean doTest(String s) {", + " // BUG: Diagnostic contains: dereferenced expression foo is @Nullable", + " return foo.toString().equals(s);", + " }", + "", + " public boolean doTestSafe(String s) {", + " // TODO: Technically safe, but it would need recognizing Test.this.[...] as the same AP as", + " // that from the closure.", + " // BUG: Diagnostic contains: dereferenced expression Test.this.foo is @Nullable", + " return Test.this.foo.toString().equals(s);", + " }", " }", " if (s.length() % 2 == 0) {", " return (new Inner()).doTest(s);", diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAcknowledgeRestrictiveAnnotationsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayAcknowledgeRestrictiveAnnotationsTests.java index e84016c37f..894934c09f 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayAcknowledgeRestrictiveAnnotationsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayAcknowledgeRestrictiveAnnotationsTests.java @@ -31,8 +31,11 @@ public void generatedAsUnannotatedPlusRestrictive() { .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", - " void foo() { (new Generated()).retNull().toString(); }", + " void foo() {", + " (new Generated()).retNull().toString();", + " }", "}") .doTest(); } @@ -49,8 +52,9 @@ public void defaultPermissiveOnUnannotated() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.lib.unannotated.RestrictivelyAnnotatedClass;", + "", "class Test {", " Object test() {", " // Assume methods take @Nullable, even if annotated otherwise", @@ -75,8 +79,9 @@ public void acknowledgeRestrictiveAnnotationsWhenFlagSet() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.lib.unannotated.RestrictivelyAnnotatedClass;", + "", "class Test {", " Object test() {", " RestrictivelyAnnotatedClass.consumesObjectUnannotated(null);", @@ -104,8 +109,9 @@ public void defaultPermissiveOnRecently() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.lib.unannotated.AndroidRecentlyAnnotatedClass;", + "", "class Test {", " Object test() {", " // Assume methods take @Nullable, even if annotated otherwise", @@ -131,8 +137,9 @@ public void acknowledgeRecentlyAnnotationsWhenFlagSet() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.lib.unannotated.AndroidRecentlyAnnotatedClass;", + "", "class Test {", " Object test() {", " AndroidRecentlyAnnotatedClass.consumesObjectUnannotated(null);", @@ -157,8 +164,9 @@ public void restrictivelyAnnotatedMethodsWorkWithNullnessFromDataflow() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.lib.unannotated.RestrictivelyAnnotatedClass;", + "", "class Test {", " Object test1(RestrictivelyAnnotatedClass instance) {", " if (instance.getField() != null) {", @@ -166,6 +174,7 @@ public void restrictivelyAnnotatedMethodsWorkWithNullnessFromDataflow() { " }", " throw new Error();", " }", + "", " Object test2(RestrictivelyAnnotatedClass instance) {", " if (instance.field != null) {", " return instance.field;", @@ -188,8 +197,9 @@ public void restrictivelyAnnotatedMethodsWorkWithNullnessFromDataflow2() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.lib.unannotated.RestrictivelyAnnotatedGenericContainer;", + "", "class Test {", " String test(RestrictivelyAnnotatedGenericContainer instance) {", " if (instance.getField() == null) {", @@ -213,30 +223,56 @@ public void overridingRestrictivelyAnnotatedMethod() { .addSourceLines( "TestNegativeCases.java", "package com.uber;", + "", "import com.uber.lib.unannotated.RestrictivelyAnnotatedClass;", - "import org.checkerframework.checker.nullness.qual.NonNull;", "import javax.annotation.Nullable;", + "", "public class TestNegativeCases extends RestrictivelyAnnotatedClass {", - " TestNegativeCases(){ super(new Object()); }", - " @Override public void acceptsNonNull(@Nullable Object o) { }", - " @Override public void acceptsNonNull2(Object o) { }", - " @Override public void acceptsNullable2(@Nullable Object o) { }", - " @Override public Object returnsNonNull() { return new Object(); }", - " @Override public Object returnsNullable() { return new Object(); }", - " @Override public @Nullable Object returnsNullable2() { return new Object();}", + " TestNegativeCases() {", + " super(new Object());", + " }", + "", + " @Override", + " public void acceptsNonNull(@Nullable Object o) {}", + "", + " @Override", + " public void acceptsNonNull2(Object o) {}", + "", + " @Override", + " public void acceptsNullable2(@Nullable Object o) {}", + "", + " @Override", + " public Object returnsNonNull() {", + " return new Object();", + " }", + "", + " @Override", + " public Object returnsNullable() {", + " return new Object();", + " }", + "", + " @Override", + " public @Nullable Object returnsNullable2() {", + " return new Object();", + " }", "}") .addSourceLines( "TestPositiveCases.java", "package com.uber;", + "", "import com.uber.lib.unannotated.RestrictivelyAnnotatedClass;", - "import org.checkerframework.checker.nullness.qual.NonNull;", "import javax.annotation.Nullable;", + "", "public class TestPositiveCases extends RestrictivelyAnnotatedClass {", - " TestPositiveCases(){ super(new Object()); }", - " // BUG: Diagnostic contains: parameter o is @NonNull", - " public void acceptsNullable(Object o) { }", - " // BUG: Diagnostic contains: method returns @Nullable", - " public @Nullable Object returnsNonNull2() { return new Object(); }", + " TestPositiveCases() {", + " super(new Object());", + " }", + " // BUG: Diagnostic contains: parameter o is @NonNull", + " public void acceptsNullable(Object o) {}", + " // BUG: Diagnostic contains: method returns @Nullable", + " public @Nullable Object returnsNonNull2() {", + " return new Object();", + " }", "}") .doTest(); } @@ -253,15 +289,17 @@ public void lambdaPlusRestrictivePositive() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.uber.lib.unannotated.RestrictivelyAnnotatedFI;", - "import javax.annotation.Nullable;", + "", "public class Test {", " void foo() {", - " RestrictivelyAnnotatedFI func = (x) -> {", - " // BUG: Diagnostic contains: dereferenced expression x is @Nullable", - " x.toString();", - " return new Object();", - " };", + " RestrictivelyAnnotatedFI func =", + " (x) -> {", + " // BUG: Diagnostic contains: dereferenced expression x is @Nullable", + " x.toString();", + " return new Object();", + " };", " }", "}") .doTest(); @@ -278,15 +316,17 @@ public void lambdaPlusRestrictiveNegative() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.uber.lib.unannotated.RestrictivelyAnnotatedFI;", - "import javax.annotation.Nullable;", + "", "public class Test {", " void foo() {", - " RestrictivelyAnnotatedFI func = (x) -> {", - " // no error since AcknowledgeRestrictiveAnnotations disabled", - " x.toString();", - " return new Object();", - " };", + " RestrictivelyAnnotatedFI func =", + " (x) -> {", + " // no error since AcknowledgeRestrictiveAnnotations disabled", + " x.toString();", + " return new Object();", + " };", " }", "}") .doTest(); @@ -305,57 +345,67 @@ public void annotatedVsUnannotatedMethodRefOverrideChecks() { .addSourceLines( "AnnotatedStringIDFunctions.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class AnnotatedStringIDFunctions {", - " public static String idRetNonNull(String s) {", - " return s;", - " }", - " @Nullable", - " public static String idRetNullable(String s) {", - " return s;", - " }", + " public static String idRetNonNull(String s) {", + " return s;", + " }", + "", + " @Nullable", + " public static String idRetNullable(String s) {", + " return s;", + " }", "}") .addSourceLines( "UnannotatedStringIDFunctions.java", "package com.uber.nullaway.lib.unannotated;", + "", "import javax.annotation.Nullable;", + "", "public class UnannotatedStringIDFunctions {", - " public static String idRetNonNull(String s) {", - " return s;", - " }", - " @Nullable", - " public static String idRetNullable(String s) {", - " return s;", - " }", + " public static String idRetNonNull(String s) {", + " return s;", + " }", + "", + " @Nullable", + " public static String idRetNullable(String s) {", + " return s;", + " }", "}") .addSourceLines( "Test.java", "package com.uber;", - "import com.google.common.base.Function;", // is Function from model + "", "import com.google.common.collect.Maps;", "import com.uber.nullaway.lib.unannotated.UnannotatedStringIDFunctions;", "import java.util.List;", "import java.util.Map;", - "import javax.annotation.Nullable;", + "", "class Test {", - " public static Map testFunctionOverrideMethodRef1(List keys) {", - " return Maps.toMap(keys,", - " /* is Function */ AnnotatedStringIDFunctions::idRetNonNull);", - " }", - " public static Map testFunctionOverrideMethodRef2(List keys) {", - " return Maps.toMap(keys,", - " // BUG: Diagnostic contains: method returns @Nullable, but functional interface", - " /* is Function */ AnnotatedStringIDFunctions::idRetNullable);", - " }", - " public static Map testFunctionOverrideMethodRef3(List keys) {", - " return Maps.toMap(keys,", - " /* is Function */ UnannotatedStringIDFunctions::idRetNonNull);", - " }", - " public static Map testFunctionOverrideMethodRef4(List keys) {", - " // No report, since idRetNullable() is unannotated and restrictive annotations are off", - " return Maps.toMap(keys,", - " /* is Function */ UnannotatedStringIDFunctions::idRetNullable);", - " }", + " public static Map testFunctionOverrideMethodRef1(List keys) {", + " return Maps.toMap(", + " keys, /* is Function */ AnnotatedStringIDFunctions::idRetNonNull);", + " }", + "", + " public static Map testFunctionOverrideMethodRef2(List keys) {", + " return Maps.toMap(", + " keys,", + " // BUG: Diagnostic contains: method returns @Nullable, but functional interface", + " /* is Function */ AnnotatedStringIDFunctions::idRetNullable);", + " }", + "", + " public static Map testFunctionOverrideMethodRef3(List keys) {", + " return Maps.toMap(", + " keys, /* is Function */ UnannotatedStringIDFunctions::idRetNonNull);", + " }", + "", + " public static Map testFunctionOverrideMethodRef4(List keys) {", + " // No report, since idRetNullable() is unannotated and restrictive annotations are off", + " return Maps.toMap(", + " keys, /* is Function */ UnannotatedStringIDFunctions::idRetNullable);", + " }", "}") .doTest(); } @@ -372,59 +422,70 @@ public void annotatedVsUnannotatedMethodRefOverrideWithRestrictiveAnnotations() .addSourceLines( "AnnotatedStringIDFunctions.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class AnnotatedStringIDFunctions {", - " public static String idRetNonNull(String s) {", - " return s;", - " }", - " @Nullable", - " public static String idRetNullable(String s) {", - " return s;", - " }", + " public static String idRetNonNull(String s) {", + " return s;", + " }", + "", + " @Nullable", + " public static String idRetNullable(String s) {", + " return s;", + " }", "}") .addSourceLines( "UnannotatedStringIDFunctions.java", "package com.uber.nullaway.lib.unannotated;", + "", "import javax.annotation.Nullable;", + "", "public class UnannotatedStringIDFunctions {", - " public static String idRetNonNull(String s) {", - " return s;", - " }", - " @Nullable", - " public static String idRetNullable(String s) {", - " return s;", - " }", + " public static String idRetNonNull(String s) {", + " return s;", + " }", + "", + " @Nullable", + " public static String idRetNullable(String s) {", + " return s;", + " }", "}") .addSourceLines( "Test.java", "package com.uber;", - "import com.google.common.base.Function;", // is Function from model + "", "import com.google.common.collect.Maps;", "import com.uber.nullaway.lib.unannotated.UnannotatedStringIDFunctions;", "import java.util.List;", "import java.util.Map;", - "import javax.annotation.Nullable;", + "", "class Test {", - " public static Map testFunctionOverrideMethodRef1(List keys) {", - " return Maps.toMap(keys,", - " /* is Function */ AnnotatedStringIDFunctions::idRetNonNull);", - " }", - " public static Map testFunctionOverrideMethodRef2(List keys) {", - " return Maps.toMap(keys,", - " // BUG: Diagnostic contains: method returns @Nullable, but functional interface", - " /* is Function */ AnnotatedStringIDFunctions::idRetNullable);", - " }", - " public static Map testFunctionOverrideMethodRef3(List keys) {", - " return Maps.toMap(keys,", - " /* is Function */ UnannotatedStringIDFunctions::idRetNonNull);", - " }", - " public static Map testFunctionOverrideMethodRef4(List keys) {", - " // Note: doesn't matter that the method ref is unannotated, since restrictive annotations", - " // are on.", - " return Maps.toMap(keys,", - " // BUG: Diagnostic contains: method returns @Nullable, but functional interface", - " /* is Function */ UnannotatedStringIDFunctions::idRetNullable);", - " }", + " public static Map testFunctionOverrideMethodRef1(List keys) {", + " return Maps.toMap(", + " keys, /* is Function */ AnnotatedStringIDFunctions::idRetNonNull);", + " }", + "", + " public static Map testFunctionOverrideMethodRef2(List keys) {", + " return Maps.toMap(", + " keys,", + " // BUG: Diagnostic contains: method returns @Nullable, but functional interface", + " /* is Function */ AnnotatedStringIDFunctions::idRetNullable);", + " }", + "", + " public static Map testFunctionOverrideMethodRef3(List keys) {", + " return Maps.toMap(", + " keys, /* is Function */ UnannotatedStringIDFunctions::idRetNonNull);", + " }", + "", + " public static Map testFunctionOverrideMethodRef4(List keys) {", + " // Note: doesn't matter that the method ref is unannotated, since restrictive annotations", + " // are on.", + " return Maps.toMap(", + " keys,", + " // BUG: Diagnostic contains: method returns @Nullable, but functional interface", + " /* is Function */ UnannotatedStringIDFunctions::idRetNullable);", + " }", "}") .doTest(); } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAssertionLibsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayAssertionLibsTests.java index 4bf39b471f..0f4cbdce86 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayAssertionLibsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayAssertionLibsTests.java @@ -16,10 +16,11 @@ public void supportTruthAssertThatIsNotNull_Object() { .addSourceLines( "Test.java", "package com.uber;", - "import java.lang.Object;", - "import java.util.Objects;", - "import javax.annotation.Nullable;", + "", "import static com.google.common.truth.Truth.assertThat;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object o) {", " assertThat(o).isNotNull();", @@ -40,9 +41,11 @@ public void supportTruthAssertThatIsNotNull_String() { .addSourceLines( "Test.java", "package com.uber;", - "import java.util.Objects;", - "import javax.annotation.Nullable;", + "", "import static com.google.common.truth.Truth.assertThat;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable String s) {", " assertThat(s).isNotNull();", @@ -63,11 +66,13 @@ public void supportTruthAssertThatIsNotNull_MapKey() { .addSourceLines( "Test.java", "package com.uber;", - "import java.util.Map;", - "import javax.annotation.Nullable;", + "", "import static com.google.common.truth.Truth.assertThat;", + "", + "import java.util.Map;", + "", "class Test {", - " private void foo(Map m) {", + " private void foo(Map m) {", " assertThat(m.get(\"foo\")).isNotNull();", " m.get(\"foo\").toString();", " }", @@ -86,10 +91,11 @@ public void doNotSupportTruthAssertThatWhenDisabled() { .addSourceLines( "Test.java", "package com.uber;", - "import java.lang.Object;", - "import java.util.Objects;", - "import javax.annotation.Nullable;", + "", "import static com.google.common.truth.Truth.assertThat;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " assertThat(a).isNotNull();", @@ -111,11 +117,12 @@ public void supportHamcrestAssertThatMatchersIsNotNull() { .addSourceLines( "Test.java", "package com.uber;", - "import java.lang.Object;", - "import java.util.Objects;", - "import javax.annotation.Nullable;", + "", "import static org.hamcrest.MatcherAssert.assertThat;", "import static org.hamcrest.Matchers.*;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a, @Nullable Object b) {", " assertThat(a, is(notNullValue()));", @@ -138,11 +145,12 @@ public void doNotSupportHamcrestAssertThatWhenDisabled() { .addSourceLines( "Test.java", "package com.uber;", - "import java.lang.Object;", - "import java.util.Objects;", - "import javax.annotation.Nullable;", + "", "import static org.hamcrest.MatcherAssert.assertThat;", "import static org.hamcrest.Matchers.*;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " assertThat(a, is(notNullValue()));", @@ -164,11 +172,12 @@ public void supportHamcrestAssertThatCoreMatchersIsNotNull() { .addSourceLines( "Test.java", "package com.uber;", - "import java.lang.Object;", - "import java.util.Objects;", - "import javax.annotation.Nullable;", - "import static org.hamcrest.MatcherAssert.assertThat;", + "", "import static org.hamcrest.CoreMatchers.*;", + "import static org.hamcrest.MatcherAssert.assertThat;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a, @Nullable Object b) {", " assertThat(a, is(notNullValue()));", @@ -191,12 +200,13 @@ public void supportHamcrestAssertThatCoreIsNotNull() { .addSourceLines( "Test.java", "package com.uber;", - "import java.lang.Object;", - "import java.util.Objects;", - "import javax.annotation.Nullable;", - "import static org.hamcrest.MatcherAssert.assertThat;", + "", "import static org.hamcrest.CoreMatchers.*;", + "import static org.hamcrest.MatcherAssert.assertThat;", + "", + "import javax.annotation.Nullable;", "import org.hamcrest.core.IsNull;", + "", "class Test {", " private void foo(@Nullable Object a, @Nullable Object b) {", " assertThat(a, is(IsNull.notNullValue()));", @@ -219,11 +229,12 @@ public void supportJunitAssertThatIsNotNull_Object() { .addSourceLines( "Test.java", "package com.uber;", - "import java.lang.Object;", - "import java.util.Objects;", - "import javax.annotation.Nullable;", - "import static org.junit.Assert.assertThat;", + "", "import static org.hamcrest.Matchers.*;", + "import static org.junit.Assert.assertThat;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a, @Nullable Object b) {", " assertThat(a, is(notNullValue()));", @@ -246,11 +257,12 @@ public void doNotSupportJunitAssertThatWhenDisabled() { .addSourceLines( "Test.java", "package com.uber;", - "import java.lang.Object;", - "import java.util.Objects;", - "import javax.annotation.Nullable;", - "import static org.junit.Assert.assertThat;", + "", "import static org.hamcrest.Matchers.*;", + "import static org.junit.Assert.assertThat;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " assertThat(a, is(notNullValue()));", @@ -272,10 +284,11 @@ public void supportAssertJAssertThatIsNotNull_Object() { .addSourceLines( "Test.java", "package com.uber;", - "import java.lang.Object;", - "import java.util.Objects;", - "import javax.annotation.Nullable;", + "", "import static org.assertj.core.api.Assertions.assertThat;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object o) {", " assertThat(o).isNotNull();", @@ -296,9 +309,11 @@ public void supportAssertJAssertThatIsNotNull_String() { .addSourceLines( "Test.java", "package com.uber;", - "import java.util.Objects;", - "import javax.annotation.Nullable;", + "", "import static org.assertj.core.api.Assertions.assertThat;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable String s) {", " assertThat(s).isNotNull();", @@ -319,11 +334,13 @@ public void supportAssertJAssertThatIsNotNull_MapKey() { .addSourceLines( "Test.java", "package com.uber;", - "import java.util.Map;", - "import javax.annotation.Nullable;", + "", "import static org.assertj.core.api.Assertions.assertThat;", + "", + "import java.util.Map;", + "", "class Test {", - " private void foo(Map m) {", + " private void foo(Map m) {", " assertThat(m.get(\"foo\")).isNotNull();", " m.get(\"foo\").toString();", " }", @@ -342,10 +359,11 @@ public void doNotSupportAssertJAssertThatWhenDisabled() { .addSourceLines( "Test.java", "package com.uber;", - "import java.lang.Object;", - "import java.util.Objects;", - "import javax.annotation.Nullable;", + "", "import static org.assertj.core.api.Assertions.assertThat;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " assertThat(a).isNotNull();", diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestNoCastTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestNoCastTest.java index 4ed879279f..e918797640 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestNoCastTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestNoCastTest.java @@ -94,6 +94,7 @@ public void suggestSuppressionWithComment() { .addInputLines( "Test.java", "package com.uber;", + "", "class Test {", " Object test1() {", " return null;", @@ -102,8 +103,10 @@ public void suggestSuppressionWithComment() { .addOutputLines( "out/Test.java", "package com.uber;", - "class Test {", // Can we actually check comments? - " @SuppressWarnings(\"NullAway\") /* PR #000000 */ Object test1() {", + "", + "class Test {", + " @SuppressWarnings(\"NullAway\") /* PR #000000 */", + " Object test1() {", " return null;", " }", "}") @@ -116,6 +119,7 @@ public void suggestSuppressionWithoutComment() { .addInputLines( "Test.java", "package com.uber;", + "", "class Test {", " Object test1() {", " return null;", @@ -124,8 +128,10 @@ public void suggestSuppressionWithoutComment() { .addOutputLines( "out/Test.java", "package com.uber;", + "", "class Test {", - " @SuppressWarnings(\"NullAway\") Object test1() {", + " @SuppressWarnings(\"NullAway\")", + " Object test1() {", " return null;", " }", "}") @@ -138,25 +144,30 @@ public void suggestSuppressionFieldLambdaDeref() { .addInputLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " @Nullable private Object foo;", " private final Runnable runnable =", - " () -> {", - " foo.toString();", - " };", + " () -> {", + " foo.toString();", + " };", "}") .addOutputLines( "out/Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " @Nullable private Object foo;", + "", " @SuppressWarnings(\"NullAway\")", " private final Runnable runnable =", - " () -> {", - " foo.toString();", - " };", + " () -> {", + " foo.toString();", + " };", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -167,27 +178,39 @@ public void suggestSuppressionFieldLambdaUnbox() { .addInputLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " @Nullable private Integer foo;", - " static int id(int x) { return x; }", + "", + " static int id(int x) {", + " return x;", + " }", + "", " private final Runnable runnable =", - " () -> {", - " id(foo);", - " };", + " () -> {", + " id(foo);", + " };", "}") .addOutputLines( "out/Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " @Nullable private Integer foo;", - " static int id(int x) { return x; }", + "", + " static int id(int x) {", + " return x;", + " }", + "", " @SuppressWarnings(\"NullAway\")", " private final Runnable runnable =", - " () -> {", - " id(foo);", - " };", + " () -> {", + " id(foo);", + " };", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -198,27 +221,39 @@ public void suggestSuppressionFieldLambdaAssignment() { .addInputLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " @Nullable private Integer foo;", - " static int id(int x) { return x; }", + "", + " static int id(int x) {", + " return x;", + " }", + "", " private final Runnable runnable =", - " () -> {", - " int x = foo + 1;", - " };", + " () -> {", + " int x = foo + 1;", + " };", "}") .addOutputLines( "out/Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " @Nullable private Integer foo;", - " static int id(int x) { return x; }", + "", + " static int id(int x) {", + " return x;", + " }", + "", " private final Runnable runnable =", - " () -> {", - " @SuppressWarnings(\"NullAway\")", - " int x = foo + 1;", - " };", + " () -> {", + " @SuppressWarnings(\"NullAway\")", + " int x = foo + 1;", + " };", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -229,31 +264,51 @@ public void suggestLambdaAssignInMethod() { .addInputLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " @Nullable private Integer foo;", " @Nullable private java.util.function.Function f;", + "", " void m1() {", - " f = (x) -> { return foo + 1; };", + " f =", + " (x) -> {", + " return foo + 1;", + " };", " }", + "", " void m2() {", - " java.util.function.Function g = (x) -> { return foo + 1; };", + " java.util.function.Function g =", + " (x) -> {", + " return foo + 1;", + " };", " }", "}") .addOutputLines( "out/Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " @Nullable private Integer foo;", " @Nullable private java.util.function.Function f;", + "", " @SuppressWarnings(\"NullAway\")", " void m1() {", - " f = (x) -> { return foo + 1; };", + " f =", + " (x) -> {", + " return foo + 1;", + " };", " }", + "", " void m2() {", " @SuppressWarnings(\"NullAway\")", - " java.util.function.Function g = (x) -> { return foo + 1; };", + " java.util.function.Function g =", + " (x) -> {", + " return foo + 1;", + " };", " }", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); @@ -265,13 +320,20 @@ public void suppressMethodRefOverrideParam() { .addInputLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " static interface I {", " public void foo(@Nullable Object o);", " }", + "", " static void biz(Object p) {}", - " static void callFoo(I i) { i.foo(null); }", + "", + " static void callFoo(I i) {", + " i.foo(null);", + " }", + "", " static void bar() {", " callFoo(Test::biz);", " }", @@ -279,13 +341,20 @@ public void suppressMethodRefOverrideParam() { .addOutputLines( "out/Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " static interface I {", " public void foo(@Nullable Object o);", " }", + "", " static void biz(Object p) {}", - " static void callFoo(I i) { i.foo(null); }", + "", + " static void callFoo(I i) {", + " i.foo(null);", + " }", + "", " @SuppressWarnings(\"NullAway\")", " static void bar() {", " callFoo(Test::biz);", @@ -300,14 +369,23 @@ public void suppressMethodRefOverrideReturn() { .addInputLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " static interface I {", " public Object foo();", " }", + "", " @Nullable", - " static Object biz() { return null; }", - " static void callFoo(I i) { i.foo(); }", + " static Object biz() {", + " return null;", + " }", + "", + " static void callFoo(I i) {", + " i.foo();", + " }", + "", " static void bar() {", " callFoo(Test::biz);", " }", @@ -315,14 +393,23 @@ public void suppressMethodRefOverrideReturn() { .addOutputLines( "out/Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " static interface I {", " public Object foo();", " }", + "", " @Nullable", - " static Object biz() { return null; }", - " static void callFoo(I i) { i.foo(); }", + " static Object biz() {", + " return null;", + " }", + "", + " static void callFoo(I i) {", + " i.foo();", + " }", + "", " @SuppressWarnings(\"NullAway\")", " static void bar() {", " callFoo(Test::biz);", diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestTest.java index 5aec8370a2..077614fe1d 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayAutoSuggestTest.java @@ -70,8 +70,11 @@ public void correctCastToNonNull() throws IOException { .addInputLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import static com.uber.nullaway.testdata.Util.castToNonNull;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " Object test1(@Nullable Object o) {", " return castToNonNull(o);", @@ -87,7 +90,9 @@ public void suggestCastToNonNull() throws IOException { .addInputLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " Object test1(@Nullable Object o) {", " return o;", @@ -96,8 +101,11 @@ public void suggestCastToNonNull() throws IOException { .addOutputLines( "out/Test.java", "package com.uber;", + "", "import static com.uber.nullaway.testdata.Util.castToNonNull;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " Object test1(@Nullable Object o) {", " return castToNonNull(o);", @@ -112,8 +120,9 @@ public void removeUnnecessaryCastToNonNull() throws IOException { .addInputLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import static com.uber.nullaway.testdata.Util.castToNonNull;", + "", "class Test {", " Object test1(Object o) {", " return castToNonNull(o);", @@ -122,8 +131,11 @@ public void removeUnnecessaryCastToNonNull() throws IOException { .addOutputLines( "out/Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import static com.uber.nullaway.testdata.Util.castToNonNull;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " Object test1(Object o) {", " return o;", @@ -138,17 +150,22 @@ public void suggestSuppressionOnMethodRef() throws IOException { .addInputLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " public @Nullable Object doReturnNullable() {", " return null;", " }", - " public static void takesNonNull(Object o) { ", + "", + " public static void takesNonNull(Object o) {", " System.out.println(o.toString());", " }", - " public R execute(io.reactivex.functions.Function f) throws Exception {", + "", + " public R execute(io.reactivex.functions.Function f) throws Exception {", " return f.apply(this);", " }", + "", " void test() throws Exception {", " takesNonNull(execute(Test::doReturnNullable));", " }", @@ -156,18 +173,24 @@ public void suggestSuppressionOnMethodRef() throws IOException { .addOutputLines( "out/Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " public @Nullable Object doReturnNullable() {", " return null;", " }", - " public static void takesNonNull(Object o) { ", + "", + " public static void takesNonNull(Object o) {", " System.out.println(o.toString());", " }", - " public R execute(io.reactivex.functions.Function f) throws Exception {", + "", + " public R execute(io.reactivex.functions.Function f) throws Exception {", " return f.apply(this);", " }", - " @SuppressWarnings(\"NullAway\") void test() throws Exception {", + "", + " @SuppressWarnings(\"NullAway\")", + " void test() throws Exception {", " takesNonNull(execute(Test::doReturnNullable));", " }", "}") diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayBytecodeInteractionsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayBytecodeInteractionsTests.java index 338bc721c2..33904d8146 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayBytecodeInteractionsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayBytecodeInteractionsTests.java @@ -9,7 +9,9 @@ public void typeUseJarReturn() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.uber.lib.*;", + "", "class Test {", " void foo(CFNullableStuff.NullableReturn r) {", " // BUG: Diagnostic contains: dereferenced expression", @@ -25,7 +27,9 @@ public void typeUseJarParam() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.uber.lib.*;", + "", "class Test {", " void foo(CFNullableStuff.NullableParam p) {", " p.doSomething(null);", @@ -43,7 +47,9 @@ public void typeUseJarField() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.uber.lib.*;", + "", "class Test {", " void foo(CFNullableStuff c) {", " // BUG: Diagnostic contains: dereferenced expression c.f", @@ -59,20 +65,27 @@ public void typeUseJarOverride() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.uber.lib.*;", "import org.checkerframework.checker.nullness.qual.Nullable;", + "", "class Test {", " class Test1 implements CFNullableStuff.NullableReturn {", - " public @Nullable Object get() { return null; }", + " public @Nullable Object get() {", + " return null;", + " }", " }", + "", " class Test2 implements CFNullableStuff.NullableParam {", " // BUG: Diagnostic contains: parameter o is @NonNull", " public void doSomething(Object o) {}", " // BUG: Diagnostic contains: parameter p is @NonNull", " public void doSomething2(Object o, Object p) {}", " }", + "", " class Test3 implements CFNullableStuff.NullableParam {", " public void doSomething(@Nullable Object o) {}", + "", " public void doSomething2(Object o, @Nullable Object p) {}", " }", "}") diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayContractsBooleanTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayContractsBooleanTests.java index b04189a34e..f391304ef1 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayContractsBooleanTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayContractsBooleanTests.java @@ -12,20 +12,26 @@ public void nonNullCheckIsTrueIsNotNullable() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import java.util.Map;", + "import javax.annotation.Nullable;", + "", "class Test {", " String test1(@Nullable Object o1) {", " Validation.checkTrue(o1 != null);", " return o1.toString();", " }", + "", " String test2(Map map) {", " Validation.checkTrue(map.get(\"key\") != null);", " return map.get(\"key\").toString();", " }", + "", " interface HasNullableGetter {", - " @Nullable Object get();", + " @Nullable", + " Object get();", " }", + "", " String test3(HasNullableGetter obj) {", " Validation.checkTrue(obj.get() != null);", " return obj.get().toString();", @@ -40,20 +46,26 @@ public void nonNullCheckIsTrueIsNotNullableReversed() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import java.util.Map;", + "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o1) {", " Validation.checkTrue(null != o1);", " return o1.toString();", " }", + "", " String test2(Map map) {", " Validation.checkTrue(null != map.get(\"key\"));", " return map.get(\"key\").toString();", " }", + "", " interface HasNullableGetter {", - " @Nullable Object get();", + " @Nullable", + " Object get();", " }", + "", " String test3(HasNullableGetter obj) {", " Validation.checkTrue(null != obj.get());", " return obj.get().toString();", @@ -68,7 +80,9 @@ public void nullCheckIsFalseIsNotNullable() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o1) {", " Validation.checkFalse(o1 == null);", @@ -84,7 +98,9 @@ public void nullCheckIsFalseIsNotNullableReversed() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o1) {", " Validation.checkFalse(null == o1);", @@ -100,7 +116,9 @@ public void nullCheckIsTrueIsNull() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o1) {", " Validation.checkTrue(o1 == null);", @@ -117,7 +135,9 @@ public void nullCheckIsTrueIsNullReversed() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o1) {", " Validation.checkTrue(null == o1);", @@ -134,7 +154,9 @@ public void nonNullCheckIsFalseIsNull() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o1) {", " Validation.checkFalse(o1 != null);", @@ -151,7 +173,9 @@ public void nonNullCheckIsFalseIsNullReversed() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o1) {", " Validation.checkFalse(null != o1);", @@ -168,19 +192,25 @@ public void compositeNullCheckAndStringEquality() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import java.util.Map;", + "import javax.annotation.Nullable;", + "", "class Test {", " String test1(@Nullable Object o1) {", " Validation.checkTrue(o1 != null && o1.toString().equals(\"\"));", " return o1.toString();", " }", + "", " String test2(@Nullable Object o1) {", - " Validation.checkTrue(o1 != null ||", - " // BUG: Diagnostic contains: dereferenced expression", - " o1.toString().equals(\"\"));", + " Validation.checkTrue(", + " o1 != null", + " ||", + " // BUG: Diagnostic contains: dereferenced expression", + " o1.toString().equals(\"\"));", " return o1.toString();", " }", + "", " String test3(Map map) {", " Validation.checkTrue(map.get(\"key\") != null && map.get(\"key\").toString().equals(\"\"));", " return map.get(\"key\").toString();", @@ -195,7 +225,9 @@ public void compositeNullCheckMultipleNonNull() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o1, @Nullable Object o2) {", " Validation.checkTrue(o1 != null && o2 != null);", @@ -211,7 +243,9 @@ public void compositeNullCheckFalseAndStringEquality() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o1) {", " Validation.checkFalse(o1 == null || o1.toString().equals(\"\"));", @@ -227,18 +261,21 @@ public void compositeNullCheckFalseMultipleNonNull() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test1(@Nullable Object o1, @Nullable Object o2) {", " Validation.checkFalse(o1 == null || o2 == null);", " return o1.toString() + o2.toString();", " }", + "", " String test2(@Nullable Object o1, @Nullable Object o2) {", " Validation.checkFalse(o1 == null && o2 == null);", " // BUG: Diagnostic contains: dereferenced expression", " return o1.toString()", - " // BUG: Diagnostic contains: dereferenced expression", - " + o2.toString();", + " // BUG: Diagnostic contains: dereferenced expression", + " + o2.toString();", " }", "}") .doTest(); @@ -250,7 +287,9 @@ public void identityNotNull() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o1) {", " if (Validation.identity(null != o1)) {", @@ -270,7 +309,9 @@ public void complexIdentityNotNull() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o1, Object o2) {", " if (Validation.identity(null != o1, o2)) {", @@ -290,7 +331,9 @@ public void identityIsNull() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o1) {", " if (Validation.identity(null == o1)) {", @@ -310,7 +353,9 @@ public void complexContractIdentityIsNull() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o1, Object o2) {", " if (Validation.identity(null == o1, o2)) {", @@ -330,8 +375,10 @@ public void checkAndReturn() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", "import org.jetbrains.annotations.Contract;", + "", "class Test {", " @Contract(\"false -> fail\")", " static boolean checkAndReturn(boolean value) {", @@ -340,22 +387,26 @@ public void checkAndReturn() { " }", " return true;", " }", + "", " String test1(@Nullable Object o1, @Nullable Object o2) {", " if (checkAndReturn(o1 != null) && o2 != null) {", " return o1.toString() + o2.toString();", " } else {", - " return o1.toString() + ", - " // BUG: Diagnostic contains: dereferenced expression", - " o2.toString();", + " return o1.toString()", + " +", + " // BUG: Diagnostic contains: dereferenced expression", + " o2.toString();", " }", " }", + "", " boolean test2(@Nullable Object o1, @Nullable Object o2) {", " return checkAndReturn(o1 != null) && o1.toString().isEmpty();", " }", + "", " boolean test3(@Nullable Object o1, @Nullable Object o2) {", - " return checkAndReturn(o1 == null) ", - " // BUG: Diagnostic contains: dereferenced expression", - " && o1.toString().isEmpty();", + " return checkAndReturn(o1 == null)", + " // BUG: Diagnostic contains: dereferenced expression", + " && o1.toString().isEmpty();", " }", "}") .doTest(); @@ -367,8 +418,10 @@ public void complexCheckAndReturn() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", "import org.jetbrains.annotations.Contract;", + "", "class Test {", " @Contract(\"false, _ -> fail\")", " static boolean checkAndReturn(boolean value, Object other) {", @@ -377,22 +430,26 @@ public void complexCheckAndReturn() { " }", " return true;", " }", + "", " String test1(@Nullable Object o1, @Nullable Object o2, Object other) {", " if (checkAndReturn(o1 != null, other) && o2 != null) {", " return o1.toString() + o2.toString();", " } else {", - " return o1.toString() + ", - " // BUG: Diagnostic contains: dereferenced expression", - " o2.toString();", + " return o1.toString()", + " +", + " // BUG: Diagnostic contains: dereferenced expression", + " o2.toString();", " }", " }", + "", " boolean test2(@Nullable Object o1, @Nullable Object o2, Object other) {", " return checkAndReturn(o1 != null, other) && o1.toString().isEmpty();", " }", + "", " boolean test3(@Nullable Object o1, @Nullable Object o2, Object other) {", - " return checkAndReturn(o1 == null, other) ", - " // BUG: Diagnostic contains: dereferenced expression", - " && o1.toString().isEmpty();", + " return checkAndReturn(o1 == null, other)", + " // BUG: Diagnostic contains: dereferenced expression", + " && o1.toString().isEmpty();", " }", "}") .doTest(); @@ -404,11 +461,10 @@ public void contractUnreachablePath() { .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", " String test(Object required) {", - " return Validation.identity(required == null)", - " ? required.toString()", - " : required.toString();", + " return Validation.identity(required == null) ? required.toString() : required.toString();", " }", "}") .doTest(); @@ -420,11 +476,10 @@ public void complexContractUnreachablePath() { .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", " String test(Object required, Object other) {", - " return Validation.identity(required == null, other)", - " ? required.toString()", - " : required.toString();", + " return Validation.identity(required == null, other) ? required.toString() : required.toString();", " }", "}") .doTest(); @@ -436,18 +491,16 @@ public void contractUnreachablePathAfterFailure() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o) {", " Validation.checkTrue(o == null);", " return Validation.identity(o == null)", - " // BUG: Diagnostic contains: dereferenced expression", - " ? o.toString()", - // This path is unreachable because o is guaranteed to be null - // after checkTrue(o == null). No failures should be reported. - // Note that we're not doing general reachability analysis, - // rather ensuring that we don't incorrectly produce errors. - " : o.toString();", + " // BUG: Diagnostic contains: dereferenced expression", + " ? o.toString()", + " : o.toString();", " }", "}") .doTest(); @@ -459,17 +512,17 @@ public void contractNestedBooleanNullness() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o) {", " return Validation.identity(o == null)", - " ? (Validation.identity(o != null)", - " ? o.toString()", - " // BUG: Diagnostic contains: dereferenced expression", - " : o.toString())", - " : (Validation.identity(o != null)", - " ? o.toString()", - " : o.toString());", + " ? (Validation.identity(o != null)", + " ? o.toString()", + " // BUG: Diagnostic contains: dereferenced expression", + " : o.toString())", + " : (Validation.identity(o != null) ? o.toString() : o.toString());", " }", "}") .doTest(); @@ -481,17 +534,17 @@ public void complexContractNestedBooleanNullness() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test(@Nullable Object o, Object other) {", " return Validation.identity(o == null, other)", - " ? (Validation.identity(o != null, other)", - " ? o.toString()", - " // BUG: Diagnostic contains: dereferenced expression", - " : o.toString())", - " : (Validation.identity(o != null, other)", - " ? o.toString()", - " : o.toString());", + " ? (Validation.identity(o != null, other)", + " ? o.toString()", + " // BUG: Diagnostic contains: dereferenced expression", + " : o.toString())", + " : (Validation.identity(o != null, other) ? o.toString() : o.toString());", " }", "}") .doTest(); @@ -503,8 +556,10 @@ public void nonNullBooleanDoubleContractTest() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", "import org.jetbrains.annotations.Contract;", + "", "class Test {", " @Contract(\"false, false -> fail\")", " static void falseFalseFail(boolean b1, boolean b2) {", @@ -512,24 +567,21 @@ public void nonNullBooleanDoubleContractTest() { " throw new RuntimeException();", " }", " }", + "", " String test1(@Nullable Object maybe, Object required) {", - // 'required == null' is known to be false, so if we go past this line, - // we know 'maybe != null' evaluates to true, hence both args are @NonNull. " falseFalseFail(maybe != null, required == null);", " return maybe.toString() + required.toString();", " }", + "", " String test2(@Nullable Object maybe) {", " String ref = null;", - // 'ref != null' is known to be false, so if we go past this line, - // we know 'maybe != null' evaluates to true. " falseFalseFail(maybe != null, ref != null);", " return maybe.toString();", " }", + "", " String test3(@Nullable Object maybe) {", " String ref = \"\";", " ref = null;", - // 'ref != null' is known to be false, so if we go past this line, - // we know 'maybe != null' evaluates to true. " falseFalseFail(maybe != null, ref != null);", " return maybe.toString();", " }", @@ -546,21 +598,26 @@ private CompilationTestHelper helper() { .addSourceLines( "Validation.java", "package com.uber;", - "import org.jetbrains.annotations.Contract;", + "", "import javax.annotation.Nullable;", + "import org.jetbrains.annotations.Contract;", + "", "public final class Validation {", " @Contract(\"false -> fail\")", " static void checkTrue(boolean value) {", " if (!value) throw new RuntimeException();", " }", + "", " @Contract(\"true -> fail\")", " static void checkFalse(boolean value) {", " if (value) throw new RuntimeException();", " }", + "", " @Contract(\"true -> true; false -> false\")", " static boolean identity(boolean value) {", " return value;", " }", + "", " @Contract(\"true, _ -> true; false, _ -> false\")", " static boolean identity(boolean value, @Nullable Object other) {", " return value;", diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayContractsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayContractsTests.java index d3849b041d..a0a6611d54 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayContractsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayContractsTests.java @@ -39,32 +39,47 @@ public void basicContractAnnotation() { .addSourceLines( "NullnessChecker.java", "package com.uber;", + "", "import javax.annotation.Nullable;", "import org.jetbrains.annotations.Contract;", + "", "public class NullnessChecker {", " @Contract(\"_, null -> true\")", - " static boolean isNull(boolean flag, @Nullable Object o) { return o == null; }", + " static boolean isNull(boolean flag, @Nullable Object o) {", + " return o == null;", + " }", + "", " @Contract(\"null -> false\")", - " static boolean isNonNull(@Nullable Object o) { return o != null; }", + " static boolean isNonNull(@Nullable Object o) {", + " return o != null;", + " }", + "", " @Contract(\"null -> fail\")", - " static void assertNonNull(@Nullable Object o) { if (o != null) throw new Error(); }", + " static void assertNonNull(@Nullable Object o) {", + " if (o != null) throw new Error();", + " }", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test1(@Nullable Object o) {", " return NullnessChecker.isNonNull(o) ? o.toString() : \"null\";", " }", + "", " String test2(@Nullable Object o) {", " return NullnessChecker.isNull(false, o) ? \"null\" : o.toString();", " }", + "", " String test3(@Nullable Object o) {", " NullnessChecker.assertNonNull(o);", " return o.toString();", " }", - " String test4(java.util.Map m) {", + "", + " String test4(java.util.Map m) {", " NullnessChecker.assertNonNull(m.get(\"foo\"));", " return m.get(\"foo\").toString();", " }", @@ -82,24 +97,32 @@ public void impliesNonNullContractAnnotation() { .addSourceLines( "NullnessChecker.java", "package com.uber;", + "", "import javax.annotation.Nullable;", "import org.jetbrains.annotations.Contract;", + "", "public class NullnessChecker {", " @Contract(\"!null -> !null\")", - " static @Nullable Object noOp(@Nullable Object o) { return o; }", + " static @Nullable Object noOp(@Nullable Object o) {", + " return o;", + " }", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test1(@Nullable Object o1) {", " // BUG: Diagnostic contains: dereferenced expression", " return NullnessChecker.noOp(o1).toString();", " }", + "", " String test2(Object o2) {", " return NullnessChecker.noOp(o2).toString();", " }", + "", " Object test3(@Nullable Object o1) {", " // BUG: Diagnostic contains: returning @Nullable expression", " return NullnessChecker.noOp(o1);", @@ -118,26 +141,41 @@ public void malformedContractAnnotations() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", "import org.jetbrains.annotations.Contract;", + "", "class Test {", " @Contract(\"!null -> -> !null\")", - " static @Nullable Object foo(@Nullable Object o) { return o; }", + " static @Nullable Object foo(@Nullable Object o) {", + " return o;", + " }", + "", " @Contract(\"!null -> !null\")", - " static @Nullable Object bar(@Nullable Object o, String s) { return o; }", + " static @Nullable Object bar(@Nullable Object o, String s) {", + " return o;", + " }", + "", " @Contract(\"jabberwocky -> !null\")", - " static @Nullable Object baz(@Nullable Object o) { return o; }", - // We don't care as long as nobody calls the method: + " static @Nullable Object baz(@Nullable Object o) {", + " return o;", + " }", + "", " @Contract(\"!null -> -> !null\")", - " static @Nullable Object dontcare(@Nullable Object o) { return o; }", + " static @Nullable Object dontcare(@Nullable Object o) {", + " return o;", + " }", + "", " static Object test1() {", " // BUG: Diagnostic contains: Invalid @Contract annotation", " return foo(null);", " }", + "", " static Object test2() {", " // BUG: Diagnostic contains: Invalid @Contract annotation", " return bar(null, \"\");", " }", + "", " static Object test3() {", " // BUG: Diagnostic contains: Invalid @Contract annotation", " return baz(null);", @@ -156,16 +194,20 @@ public void contractNonVarArg() { .addSourceLines( "NullnessChecker.java", "package com.uber;", + "", "import javax.annotation.Nullable;", "import org.jetbrains.annotations.Contract;", + "", "public class NullnessChecker {", " @Contract(\"null -> fail\")", - " static void assertNonNull(@Nullable Object o) { if (o != null) throw new Error(); }", + " static void assertNonNull(@Nullable Object o) {", + " if (o != null) throw new Error();", + " }", "}") .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "class Test {", " void test(java.util.function.Function fun) {", " NullnessChecker.assertNonNull(fun.apply(new Object()));", @@ -184,11 +226,11 @@ public void contractPureOnlyIgnored() { .addSourceLines( "PureLibrary.java", "package com.example.library;", + "", "import org.jetbrains.annotations.Contract;", + "", "public class PureLibrary {", - " @Contract(", - " pure = true", - " )", + " @Contract(pure = true)", " public static String pi() {", " // Meh, close enough...", " return Double.toString(3.14);", @@ -197,15 +239,16 @@ public void contractPureOnlyIgnored() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.library.PureLibrary;", - "import javax.annotation.Nullable;", + "", "class Test {", " String piValue() {", " String pi = PureLibrary.pi();", " // Note: we must trigger dataflow to ensure that", " // ContractHandler.onDataflowVisitMethodInvocation is called", " if (pi != null) {", - " return pi;", + " return pi;", " }", " return Integer.toString(3);", " }", @@ -225,8 +268,11 @@ public void customContractAnnotation() { .addSourceLines( "CustomContract.java", "package com.example.library;", + "", "import static java.lang.annotation.RetentionPolicy.CLASS;", + "", "import java.lang.annotation.Retention;", + "", "@Retention(CLASS)", "public @interface CustomContract {", " String value();", @@ -234,8 +280,10 @@ public void customContractAnnotation() { .addSourceLines( "NullnessChecker.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.example.library.CustomContract;", + "import javax.annotation.Nullable;", + "", "public class NullnessChecker {", " @CustomContract(\"_, !null -> !null\")", " @Nullable", @@ -259,11 +307,12 @@ public void customContractAnnotation() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "class Test {", " String test1() {", " return NullnessChecker.good(\"bar\", \"foo\").toString();", " }", + "", " String test2() {", " // BUG: Diagnostic contains: dereferenced expression", " return NullnessChecker.good(\"bar\", null).toString();", @@ -282,8 +331,10 @@ public void contractDeclaringBothNotNull() { .addSourceLines( "NullnessChecker.java", "package com.uber;", + "", "import javax.annotation.Nullable;", "import org.jetbrains.annotations.Contract;", + "", "public class NullnessChecker {", " @Contract(\"null, _ -> false; _, null -> false\")", " static boolean bothNotNull(@Nullable Object o1, @Nullable Object o2) {", @@ -302,19 +353,22 @@ public void contractDeclaringBothNotNull() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test1(@Nullable Object o) {", " return NullnessChecker.bothNotNull(\"\", o)", - " ? o.toString()", - " // BUG: Diagnostic contains: dereferenced expression", - " : o.toString();", + " ? o.toString()", + " // BUG: Diagnostic contains: dereferenced expression", + " : o.toString();", " }", + "", " String test2(@Nullable Object o) {", " return NullnessChecker.bothNotNull(o, \"\")", - " ? o.toString()", - " // BUG: Diagnostic contains: dereferenced expression", - " : o.toString();", + " ? o.toString()", + " // BUG: Diagnostic contains: dereferenced expression", + " : o.toString();", " }", "}") .doTest(); @@ -330,8 +384,10 @@ public void contractDeclaringEitherNull() { .addSourceLines( "NullnessChecker.java", "package com.uber;", + "", "import javax.annotation.Nullable;", "import org.jetbrains.annotations.Contract;", + "", "public class NullnessChecker {", " @Contract(\"null, _ -> true; _, null -> true\")", " static boolean eitherIsNullOrRandom(@Nullable Object o1, @Nullable Object o2) {", @@ -350,19 +406,22 @@ public void contractDeclaringEitherNull() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test1(@Nullable Object o) {", " return NullnessChecker.eitherIsNullOrRandom(\"\", o)", - " // BUG: Diagnostic contains: dereferenced expression", - " ? o.toString()", - " : o.toString();", + " // BUG: Diagnostic contains: dereferenced expression", + " ? o.toString()", + " : o.toString();", " }", + "", " String test2(@Nullable Object o) {", " return NullnessChecker.eitherIsNullOrRandom(o, \"\")", - " // BUG: Diagnostic contains: dereferenced expression", - " ? o.toString()", - " : o.toString();", + " // BUG: Diagnostic contains: dereferenced expression", + " ? o.toString()", + " : o.toString();", " }", "}") .doTest(); @@ -378,8 +437,10 @@ public void contractDeclaringNullOrRandomFalse() { .addSourceLines( "NullnessChecker.java", "package com.uber;", + "", "import javax.annotation.Nullable;", "import org.jetbrains.annotations.Contract;", + "", "public class NullnessChecker {", " @Contract(\"null -> false\")", " static boolean isHashCodeZero(@Nullable Object o) {", @@ -394,13 +455,15 @@ public void contractDeclaringNullOrRandomFalse() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " String test1(@Nullable Object o) {", " return NullnessChecker.isHashCodeZero(o)", - " ? o.toString()", - " // BUG: Diagnostic contains: dereferenced expression", - " : o.toString();", + " ? o.toString()", + " // BUG: Diagnostic contains: dereferenced expression", + " : o.toString();", " }", "}") .doTest(); @@ -416,8 +479,10 @@ public void contractUnreachablePath() { .addSourceLines( "NullnessChecker.java", "package com.uber;", + "", "import javax.annotation.Nullable;", "import org.jetbrains.annotations.Contract;", + "", "public class NullnessChecker {", " @Contract(\"!null -> false\")", " static boolean isNull(@Nullable Object o) {", @@ -431,11 +496,10 @@ public void contractUnreachablePath() { .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", " String test(Object required) {", - " return NullnessChecker.isNull(required)", - " ? required.toString()", - " : required.toString();", + " return NullnessChecker.isNull(required) ? required.toString() : required.toString();", " }", "}") .doTest(); diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java index f8f3ed622c..c3e54d1556 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayCoreTests.java @@ -74,8 +74,10 @@ public void testGenericAnonymousInner() { .addSourceLines( "GenericSuper.java", "package com.uber;", + "", "class GenericSuper {", " T x;", + "", " GenericSuper(T y) {", " this.x = y;", " }", @@ -83,12 +85,15 @@ public void testGenericAnonymousInner() { .addSourceLines( "AnonSub.java", "package com.uber;", + "", "import java.util.List;", "import javax.annotation.Nullable;", + "", "class AnonSub {", " static GenericSuper> makeSuper(List list) {", " return new GenericSuper>(list) {};", " }", + "", " static GenericSuper> makeSuperBad(@Nullable List list) {", " // BUG: Diagnostic contains: passing @Nullable parameter 'list' where @NonNull", " return new GenericSuper>(list) {};", @@ -104,7 +109,9 @@ public void erasedIterator() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.*;", + "", "class Test {", " static class Foo implements Iterable {", " public Iterator iterator() {", @@ -113,6 +120,7 @@ public void erasedIterator() { " public boolean hasNext() {", " return false;", " }", + "", " @Override", " public Iterator next() {", " throw new NoSuchElementException();", @@ -120,6 +128,7 @@ public void erasedIterator() { " };", " }", " }", + "", " static void testErasedIterator(Foo foo) {", " for (Object x : foo) {", " x.hashCode();", @@ -135,14 +144,20 @@ public void compoundAssignment() { .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", " static void assignments() {", - " String x = null; x += \"hello\";", + " String x = null;", + " x += \"hello\";", " // BUG: Diagnostic contains: unboxing of a @Nullable value", - " Integer y = null; y += 3;", + " Integer y = null;", + " y += 3;", " // BUG: Diagnostic contains: unboxing of a @Nullable value", - " boolean b = false; Boolean c = null; b |= c;", + " boolean b = false;", + " Boolean c = null;", + " b |= c;", " }", + "", " static Integer returnCompound() {", " Integer z = 7;", " return (z += 10);", @@ -157,9 +172,11 @@ public void arrayIndexUnbox() { .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", " static void indexUnbox() {", - " Integer x = null; int[] fizz = { 0, 1 };", + " Integer x = null;", + " int[] fizz = {0, 1};", " // BUG: Diagnostic contains: unboxing of a @Nullable value", " int y = fizz[x];", " }", @@ -173,8 +190,10 @@ public void cfNullableArrayField() { .addSourceLines( "CFNullable.java", "package com.uber;", - "import org.checkerframework.checker.nullness.qual.Nullable;", + "", "import java.util.List;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "", "abstract class CFNullable {", " List @Nullable [] table;", "}") @@ -187,45 +206,59 @@ public void supportSwitchExpression() { .addSourceLines( "TestPositive.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "enum Level {", - " HIGH, MEDIUM, LOW }", + " HIGH,", + " MEDIUM,", + " LOW", + "}", + "", "class TestPositive {", - " void foo(@Nullable Integer s) {", + " void foo(@Nullable Integer s) {", " // BUG: Diagnostic contains: switch expression s is @Nullable", - " switch(s) {", - " case 5: break;", + " switch (s) {", + " case 5:", + " break;", " }", " String x = null;", " // BUG: Diagnostic contains: switch expression x is @Nullable", - " switch(x) {", - " default: break;", + " switch (x) {", + " default:", + " break;", " }", " Level level = null;", " // BUG: Diagnostic contains: switch expression level is @Nullable", " switch (level) {", - " default: break; }", + " default:", + " break;", " }", + " }", "}") .addSourceLines( "TestNegative.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "class TestNegative {", - " void foo(Integer s, short y) {", - " switch(s) {", - " case 5: break;", + " void foo(Integer s, short y) {", + " switch (s) {", + " case 5:", + " break;", " }", " String x = \"irrelevant\";", - " switch(x) {", - " default: break;", + " switch (x) {", + " default:", + " break;", " }", - " switch(y) {", - " default: break;", + " switch (y) {", + " default:", + " break;", " }", " Level level = Level.HIGH;", " switch (level) {", - " default: break;", + " default:", + " break;", " }", " }", "}") @@ -239,12 +272,16 @@ public void testCastToNonNull() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import static com.uber.nullaway.testdata.Util.castToNonNull;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " Object test1(@Nullable Object o) {", " return castToNonNull(o);", " }", + "", " Object test2(Object o) {", " // BUG: Diagnostic contains: passing known @NonNull parameter 'o' to CastToNonNullMethod", " return castToNonNull(o);", @@ -260,21 +297,27 @@ public void testCastToNonNullExtraArgsWarning() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import static com.uber.nullaway.testdata.Util.castToNonNull;", + "", + "import javax.annotation.Nullable;", + "", "class Test {", " Object test1(Object o) {", " // BUG: Diagnostic contains: passing known @NonNull parameter 'o' to CastToNonNullMethod", " return castToNonNull(o, \"o should be @Nullable but never actually null\");", " }", + "", " Object test2(Object o) {", " // BUG: Diagnostic contains: passing known @NonNull parameter 'o' to CastToNonNullMethod", " return castToNonNull(\"o should be @Nullable but never actually null\", o, 0);", " }", + "", " Object test3(@Nullable Object o) {", " // Expected use of cast", " return castToNonNull(o, \"o should be @Nullable but never actually null\");", " }", + "", " Object test4(@Nullable Object o) {", " // Expected use of cast", " return castToNonNull(o, \"o should be @Nullable but never actually null\");", @@ -289,11 +332,12 @@ public void testReadStaticInConstructor() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "class Test {", " // BUG: Diagnostic contains: @NonNull static field o not initialized", " static Object o;", " Object f, g;", + "", " public Test() {", " f = new String(\"hi\");", " o = new Object();", @@ -314,10 +358,12 @@ public void customErrorURL() { .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", " static void foo() {", " // BUG: Diagnostic contains: mydomain.com", - " Object x = null; x.toString();", + " Object x = null;", + " x.toString();", " }", "}") .doTest(); @@ -333,10 +379,12 @@ public void defaultURL() { .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", " static void foo() {", " // BUG: Diagnostic contains: t.uber.com/nullaway", - " Object x = null; x.toString();", + " Object x = null;", + " x.toString();", " }", "}") .doTest(); @@ -348,8 +396,10 @@ public void invokeNativeFromInitializer() { .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", " Object f;", + "", " private native void foo();", " // BUG: Diagnostic contains: initializer method does not guarantee @NonNull field f", " Test() {", @@ -370,12 +420,14 @@ public void testEnhancedFor() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import java.util.List;", + "import javax.annotation.Nullable;", + "", "public class Test {", " public void testEnhancedFor(@Nullable List l) {", " // BUG: Diagnostic contains: enhanced-for expression l is @Nullable", - " for (String x: l) {}", + " for (String x : l) {}", " }", "}") .doTest(); @@ -387,41 +439,48 @@ public void testMapWithCustomPut() { // See https://github.com/uber/NullAway/iss .addSourceLines( "Item.java", "package com.uber.lib.unannotated.collections;", - "public class Item {", - " public final K key;", - " public final V value;", - " public Item(K k, V v) {", - " this.key = k;", - " this.value = v;", - " }", + "", + "public class Item {", + " public final K key;", + " public final V value;", + "", + " public Item(K k, V v) {", + " this.key = k;", + " this.value = v;", + " }", "}") .addSourceLines( "MapLike.java", "package com.uber.lib.unannotated.collections;", + "", "import java.util.HashMap;", "// Too much work to implement java.util.Map from scratch", - "public class MapLike extends HashMap {", - " public MapLike() {", - " super();", - " }", - " public void put(Item item) {", - " put(item.key, item.value);", - " }", + "", + "public class MapLike extends HashMap {", + " public MapLike() {", + " super();", + " }", + "", + " public void put(Item item) {", + " put(item.key, item.value);", + " }", "}") .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.lib.unannotated.collections.Item;", "import com.uber.lib.unannotated.collections.MapLike;", + "import javax.annotation.Nullable;", + "", "public class Test {", - " public static MapLike test_389(@Nullable Item item) {", - " MapLike map = new MapLike();", - " if (item != null) {", // Required to trigger dataflow analysis - " map.put(item);", + " public static MapLike test_389(@Nullable Item item) {", + " MapLike map = new MapLike();", + " if (item != null) {", + " map.put(item);", + " }", + " return map;", " }", - " return map;", - " }", "}") .doTest(); } @@ -432,6 +491,7 @@ public void derefNullableTernary() { .addSourceLines( "Test.java", "package com.uber;", + "", "public class Test {", " public void derefTernary(boolean b) {", " Object o1 = null, o2 = new Object();", @@ -454,21 +514,26 @@ public void testCustomNullableAnnotation() { temporaryFolder.getRoot().getAbsolutePath(), "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:CustomNullableAnnotations=qual.Null")) - .addSourceLines("qual/Null.java", "package qual;", "public @interface Null {", "}") + .addSourceLines("qual/Null.java", "package qual;", "", "public @interface Null {}") .addSourceLines( "Test.java", "package com.uber;", + "", "import qual.Null;", + "", "class Test {", - " @Null Object foo;", // No error, should detect @Null - " @Null Object baz(){", - " bar(foo);", - " return null;", // No error, should detect @Null - " }", - " String bar(@Null Object item){", - " // BUG: Diagnostic contains: dereferenced expression item is @Nullable", - " return item.toString();", - " }", + " @Null Object foo;", + "", + " @Null", + " Object baz() {", + " bar(foo);", + " return null;", + " }", + "", + " String bar(@Null Object item) {", + " // BUG: Diagnostic contains: dereferenced expression item is @Nullable", + " return item.toString();", + " }", "}") .doTest(); } @@ -483,23 +548,27 @@ public void testCustomNonnullAnnotation() { "-XepOpt:NullAway:UnannotatedClasses=com.uber.Other", "-XepOpt:NullAway:CustomNonnullAnnotations=qual.NoNull", "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true")) - .addSourceLines("qual/NoNull.java", "package qual;", "public @interface NoNull {", "}") + .addSourceLines("qual/NoNull.java", "package qual;", "", "public @interface NoNull {}") .addSourceLines( "Other.java", "package com.uber;", + "", "import qual.NoNull;", + "", "public class Other {", - " void bar(@NoNull Object item) { }", + " void bar(@NoNull Object item) {}", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", - " Other other = new Other();", - " void foo(){", - " // BUG: Diagnostic contains: passing @Nullable parameter 'null'", - " other.bar(null);", - " }", + " Other other = new Other();", + "", + " void foo() {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null'", + " other.bar(null);", + " }", "}") .doTest(); } @@ -510,28 +579,33 @@ public void testMapGetChainWithCast() { .addSourceLines( "Constants.java", "package com.uber;", + "", "public class Constants {", - " public static final String KEY_1 = \"key1\";", - " public static final String KEY_2 = \"key2\";", - " public static final String KEY_3 = \"key3\";", + " public static final String KEY_1 = \"key1\";", + " public static final String KEY_2 = \"key2\";", + " public static final String KEY_3 = \"key3\";", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.Map;", + "", "class Test {", - " boolean withoutCast(Map>> topLevelMap){", - " return topLevelMap.get(Constants.KEY_1) == null ", - " || topLevelMap.get(Constants.KEY_1).get(Constants.KEY_2) == null", - " || topLevelMap.get(Constants.KEY_1).get(Constants.KEY_2).get(Constants.KEY_3) == null;", - " }", - " boolean withCast(Map topLevelMap){", - " return topLevelMap.get(Constants.KEY_1) == null ", - " || ((Map) topLevelMap.get(Constants.KEY_1)).get(Constants.KEY_2) == null", - " || ((Map) ", - " ((Map) topLevelMap.get(Constants.KEY_1)).get(Constants.KEY_2))", - " .get(Constants.KEY_3) == null;", - " }", + " boolean withoutCast(Map>> topLevelMap) {", + " return topLevelMap.get(Constants.KEY_1) == null", + " || topLevelMap.get(Constants.KEY_1).get(Constants.KEY_2) == null", + " || topLevelMap.get(Constants.KEY_1).get(Constants.KEY_2).get(Constants.KEY_3) == null;", + " }", + "", + " boolean withCast(Map topLevelMap) {", + " return topLevelMap.get(Constants.KEY_1) == null", + " || ((Map) topLevelMap.get(Constants.KEY_1)).get(Constants.KEY_2) == null", + " || ((Map)", + " ((Map) topLevelMap.get(Constants.KEY_1)).get(Constants.KEY_2))", + " .get(Constants.KEY_3)", + " == null;", + " }", "}") .doTest(); } @@ -542,16 +616,19 @@ public void testMapPutAndPutIfAbsent() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.Map;", + "", "class Test {", - " Object testPut(String key, Object o, Map m){", - " m.put(key, o);", - " return m.get(key);", - " }", - " Object testPutIfAbsent(String key, Object o, Map m){", - " m.putIfAbsent(key, o);", - " return m.get(key);", - " }", + " Object testPut(String key, Object o, Map m) {", + " m.put(key, o);", + " return m.get(key);", + " }", + "", + " Object testPutIfAbsent(String key, Object o, Map m) {", + " m.putIfAbsent(key, o);", + " return m.get(key);", + " }", "}") .doTest(); } @@ -562,29 +639,34 @@ public void testMapComputeIfAbsent() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.Map;", "import java.util.function.Function;", - // Need JSpecify (vs javax) for annotating generics "import org.jspecify.annotations.Nullable;", + "", "class Test {", - " Object testComputeIfAbsent(String key, Function f, Map m){", - " m.computeIfAbsent(key, f);", - " return m.get(key);", - " }", - " Object testComputeIfAbsentLambda(String key, Map m){", - " m.computeIfAbsent(key, k -> k);", - " return m.get(key);", - " }", - " Object testComputeIfAbsentNull(String key, Function f, Map m){", - " m.computeIfAbsent(key, f);", - " // BUG: Diagnostic contains: returning @Nullable expression from method with @NonNull return type", - " return m.get(key);", - " }", - " // ToDo: should error somewhere, but doesn't, due to limited checking of generics", - " Object testComputeIfAbsentNullLambda(String key, Map m){", - " m.computeIfAbsent(key, k -> null);", - " return m.get(key);", - " }", + " Object testComputeIfAbsent(String key, Function f, Map m) {", + " m.computeIfAbsent(key, f);", + " return m.get(key);", + " }", + "", + " Object testComputeIfAbsentLambda(String key, Map m) {", + " m.computeIfAbsent(key, k -> k);", + " return m.get(key);", + " }", + "", + " Object testComputeIfAbsentNull(", + " String key, Function f, Map m) {", + " m.computeIfAbsent(key, f);", + " // BUG: Diagnostic contains: returning @Nullable expression from method with @NonNull return", + " // type", + " return m.get(key);", + " }", + " // ToDo: should error somewhere, but doesn't, due to limited checking of generics", + " Object testComputeIfAbsentNullLambda(String key, Map m) {", + " m.computeIfAbsent(key, k -> null);", + " return m.get(key);", + " }", "}") .doTest(); } @@ -595,17 +677,18 @@ public void testMapWithMapGetKey() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.Map;", - "import java.util.function.Function;", + "", "class Test {", - " String testMapWithMapGetKey(Map m1, Map m2) {", - " if (m1.containsKey(\"s1\")) {", - " if (m2.containsKey(m1.get(\"s1\"))) {", - " return m2.get(m1.get(\"s1\")).toString();", - " }", - " }", - " return \"no\";", - " }", + " String testMapWithMapGetKey(Map m1, Map m2) {", + " if (m1.containsKey(\"s1\")) {", + " if (m2.containsKey(m1.get(\"s1\"))) {", + " return m2.get(m1.get(\"s1\")).toString();", + " }", + " }", + " return \"no\";", + " }", "}") .doTest(); } @@ -621,10 +704,12 @@ public void tryWithResourcesSupport() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import java.io.BufferedReader;", "import java.io.FileReader;", "import java.io.IOException;", + "import javax.annotation.Nullable;", + "", "class Test {", " String foo(String path, @Nullable String s, @Nullable Object o) throws IOException {", " try (BufferedReader br = new BufferedReader(new FileReader(path))) {", @@ -645,13 +730,15 @@ public void tryWithResourcesSupportInit() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import java.io.BufferedReader;", "import java.io.FileReader;", "import java.io.IOException;", + "", "class Test {", " private String path;", " private String f;", + "", " Test(String p) throws IOException {", " path = p;", " try (BufferedReader br = new BufferedReader(new FileReader(path))) {", @@ -668,13 +755,15 @@ public void tryFinallySupportInit() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import java.io.BufferedReader;", "import java.io.FileReader;", "import java.io.IOException;", + "", "class Test {", " private String path;", " private String f;", + "", " Test(String p) throws IOException {", " path = p;", " try {", @@ -694,13 +783,17 @@ public void nullableOnJavaLangVoid() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " Void foo1() {", " // temporarily, we treat a Void return type as if it was @Nullable Void", " return null;", " }", - " @Nullable Void foo2() {", + "", + " @Nullable", + " Void foo2() {", " return null;", " }", "}") @@ -713,25 +806,29 @@ public void nullableOnJavaLangVoidWithCast() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", " // Currently unhandled. In fact, it *should* produce an error. This entire test case", " // needs to be rethought once we properly support generics, such that it works on T v", " // when T == @Nullable Void, but not when T == Void. Without generics, though, this is the", " // best we can do.", " @SuppressWarnings(\"NullAway\")", - " private Void v = (Void)null;", + " private Void v = (Void) null;", + "", " Void foo1() {", " // temporarily, we treat a Void return type as if it was @Nullable Void", - " return (Void)null;", + " return (Void) null;", " }", " // Temporarily, we treat any Void formal as if it were @Nullable Void", - " void consumeVoid(Void v) {", - " }", - " @Nullable Void foo2() {", + " void consumeVoid(Void v) {}", + "", + " @Nullable", + " Void foo2() {", " consumeVoid(null); // See comment on consumeVoid for why this is allowed", - " consumeVoid((Void)null);", - " return (Void)null;", + " consumeVoid((Void) null);", + " return (Void) null;", " }", "}") .doTest(); @@ -743,9 +840,15 @@ public void staticCallZeroArgsNullCheck() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", - " @Nullable static Object nullableReturn() { return new Object(); }", + " @Nullable", + " static Object nullableReturn() {", + " return new Object();", + " }", + "", " void foo() {", " if (nullableReturn() != null) {", " nullableReturn().toString();", @@ -753,6 +856,7 @@ public void staticCallZeroArgsNullCheck() { " // BUG: Diagnostic contains: dereferenced expression", " nullableReturn().toString();", " }", + "", " void foo2() {", " if (Test.nullableReturn() != null) {", " nullableReturn().toString();", @@ -770,23 +874,25 @@ public void primitiveCasts() { .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", - " static void foo(int i) { }", - " static void m() {", - " Integer i = null;", - " // BUG: Diagnostic contains: unboxing", - " int i2 = (int) i;", - " // this is fine", - " int i3 = (int) Integer.valueOf(3);", - " // BUG: Diagnostic contains: unboxing", - " int i4 = ((int) i) + 1;", - " // BUG: Diagnostic contains: unboxing", - " foo((int) i);", - " // try another type", - " Double d = null;", - " // BUG: Diagnostic contains: unboxing", - " double d2 = (double) d;", - " }", + " static void foo(int i) {}", + "", + " static void m() {", + " Integer i = null;", + " // BUG: Diagnostic contains: unboxing", + " int i2 = (int) i;", + " // this is fine", + " int i3 = (int) Integer.valueOf(3);", + " // BUG: Diagnostic contains: unboxing", + " int i4 = ((int) i) + 1;", + " // BUG: Diagnostic contains: unboxing", + " foo((int) i);", + " // try another type", + " Double d = null;", + " // BUG: Diagnostic contains: unboxing", + " double d2 = (double) d;", + " }", "}") .doTest(); } @@ -797,98 +903,116 @@ public void unboxingInBinaryTrees() { .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", - " static void m1() {", - " Integer i = null;", - " Integer j = null;", - " // BUG: Diagnostic contains: unboxing", - " int i2 = i + j;", - " }", - " static void m2() {", - " Integer i = null;", - " // this is fine", - " String s = i + \"hi\";", - " }", - " static void m3() {", - " Integer i = null;", - " Integer j = null;", - " // BUG: Diagnostic contains: unboxing", - " int i3 = i - j;", - " }", - " static void m4() {", - " Integer i = null;", - " Integer j = null;", - " // BUG: Diagnostic contains: unboxing", - " int i4 = i * j;", - " }", - " static void m5() {", - " Integer i = null;", - " // BUG: Diagnostic contains: unboxing", - " int i5 = i << 2;", - " }", - " static void m6() {", - " Integer i = null;", - " Integer j = null;", - " // BUG: Diagnostic contains: unboxing", - " boolean b1 = i <= j;", - " }", - " static void m7() {", - " Boolean x = null;", - " Boolean y = null;", - " // BUG: Diagnostic contains: unboxing", - " boolean b2 = x && y;", - " }", - " static void m8() {", - " Integer i = null;", - " Integer j = null;", - " // this is fine", - " boolean b = i == j;", - " }", - " static void m9() {", - " Integer i = null;", - " // BUG: Diagnostic contains: unboxing", - " boolean b = i != 0;", - " }", - " static void m10() {", - " Integer i = null;", - " // BUG: Diagnostic contains: unboxing", - " int j = 3 - i;", - " }", - " static void m11() {", - " Integer i = null;", - " Integer j = null;", + " static void m1() {", + " Integer i = null;", + " Integer j = null;", + " // BUG: Diagnostic contains: unboxing", + " int i2 = i + j;", + " }", + "", + " static void m2() {", + " Integer i = null;", + " // this is fine", + " String s = i + \"hi\";", + " }", + "", + " static void m3() {", + " Integer i = null;", + " Integer j = null;", + " // BUG: Diagnostic contains: unboxing", + " int i3 = i - j;", + " }", + "", + " static void m4() {", + " Integer i = null;", + " Integer j = null;", + " // BUG: Diagnostic contains: unboxing", + " int i4 = i * j;", + " }", + "", + " static void m5() {", + " Integer i = null;", + " // BUG: Diagnostic contains: unboxing", + " int i5 = i << 2;", + " }", + "", + " static void m6() {", + " Integer i = null;", + " Integer j = null;", + " // BUG: Diagnostic contains: unboxing", + " boolean b1 = i <= j;", + " }", + "", + " static void m7() {", + " Boolean x = null;", + " Boolean y = null;", + " // BUG: Diagnostic contains: unboxing", + " boolean b2 = x && y;", + " }", + "", + " static void m8() {", + " Integer i = null;", + " Integer j = null;", + " // this is fine", + " boolean b = i == j;", + " }", + "", + " static void m9() {", + " Integer i = null;", + " // BUG: Diagnostic contains: unboxing", + " boolean b = i != 0;", + " }", + "", + " static void m10() {", + " Integer i = null;", + " // BUG: Diagnostic contains: unboxing", + " int j = 3 - i;", + " }", + "", + " static void m11() {", + " Integer i = null;", + " Integer j = null;", + " // BUG: Diagnostic contains: unboxing", + " int i2 =", + " i", + " +", + " // BUG: Diagnostic contains: unboxing", + " j;", + " }", + "", + " static void m12() {", + " Integer i = null;", + " // BUG: Diagnostic contains: unboxing", + " int i2 =", + " i", + " +", + " // no error here, due to previous unbox of i", + " i;", + " }", + "", + " static void m13() {", + " int[] arr = null;", + " Integer i = null;", + " // BUG: Diagnostic contains: dereferenced", + " int i2 =", + " arr[", + " // BUG: Diagnostic contains: unboxing", + " i];", + " }", + "", + " static void primitiveArgs(int x, int y) {}", + "", + " static void m14() {", + " Integer i = null;", + " Integer j = null;", + " primitiveArgs(", " // BUG: Diagnostic contains: unboxing", - " int i2 = i", - " +", - " // BUG: Diagnostic contains: unboxing", - " j;", - " }", - " static void m12() {", - " Integer i = null;", + " i,", " // BUG: Diagnostic contains: unboxing", - " int i2 = i", - " +", - " // no error here, due to previous unbox of i", - " i;", - " }", - " static void m13() {", - " int[] arr = null;", - " Integer i = null;", - " // BUG: Diagnostic contains: dereferenced", - " int i2 = arr[", - " // BUG: Diagnostic contains: unboxing", - " i];", - " }", - " static void primitiveArgs(int x, int y) {}", - " static void m14() {", - " Integer i = null;", - " Integer j = null;", - " primitiveArgs(", - " // BUG: Diagnostic contains: unboxing", - " i,", - " // BUG: Diagnostic contains: unboxing", - " j);", - " }", + " j);", + " }", "}") .doTest(); } @@ -899,64 +1023,78 @@ public void primitiveCastsRememberNullChecks() { .addSourceLines( "Test.java", "package com.uber;", + "", + "import com.google.common.base.Preconditions;", "import java.util.Map;", "import javax.annotation.Nullable;", - "import com.google.common.base.Preconditions;", + "", "class Test {", - " static void foo(int i) { }", - " static void m1(@Nullable Integer i) {", - " // BUG: Diagnostic contains: unboxing", - " int i1 = (int) i;", - " }", - " static void m2(@Nullable Integer i) {", - " if (i != null) {", - " // this is fine", - " int i2 = (int) i;", - " }", - " }", - " static void m3(@Nullable Integer i) {", - " // BUG: Diagnostic contains: unboxing", - " int i3 = (int) i;", - " }", - " static void m4(@Nullable Integer i) {", - " Preconditions.checkNotNull(i);", - " // this is fine", - " int i4 = (int) i;", - " }", - " static private void consumeInt(int i) { }", - " static void m5(@Nullable Integer i) {", - " // BUG: Diagnostic contains: unboxing", - " consumeInt((int) i);", - " }", - " static void m6(@Nullable Integer i) {", - " Preconditions.checkNotNull(i);", - " // this is fine", - " consumeInt((int) i);", - " }", - " static void m7(@Nullable Object o) {", - " // BUG: Diagnostic contains: unboxing", - " consumeInt((int) o);", - " }", - " static void m8(@Nullable Object o) {", - " Preconditions.checkNotNull(o);", - " // this is fine", - " consumeInt((int) o);", - " }", - " static void m9(Map m) {", - " // BUG: Diagnostic contains: unboxing", - " consumeInt((int) m.get(\"foo\"));", - " }", - " static void m10(Map m) {", - " if(m.get(\"bar\") != null) {", - " // this is fine", - " consumeInt((int) m.get(\"bar\"));", - " }", + " static void foo(int i) {}", + "", + " static void m1(@Nullable Integer i) {", + " // BUG: Diagnostic contains: unboxing", + " int i1 = (int) i;", + " }", + "", + " static void m2(@Nullable Integer i) {", + " if (i != null) {", + " // this is fine", + " int i2 = (int) i;", " }", - " static void m11(Map m) {", - " Preconditions.checkNotNull(m.get(\"bar\"));", - " // this is fine", - " consumeInt((int) m.get(\"bar\"));", + " }", + "", + " static void m3(@Nullable Integer i) {", + " // BUG: Diagnostic contains: unboxing", + " int i3 = (int) i;", + " }", + "", + " static void m4(@Nullable Integer i) {", + " Preconditions.checkNotNull(i);", + " // this is fine", + " int i4 = (int) i;", + " }", + "", + " private static void consumeInt(int i) {}", + "", + " static void m5(@Nullable Integer i) {", + " // BUG: Diagnostic contains: unboxing", + " consumeInt((int) i);", + " }", + "", + " static void m6(@Nullable Integer i) {", + " Preconditions.checkNotNull(i);", + " // this is fine", + " consumeInt((int) i);", + " }", + "", + " static void m7(@Nullable Object o) {", + " // BUG: Diagnostic contains: unboxing", + " consumeInt((int) o);", + " }", + "", + " static void m8(@Nullable Object o) {", + " Preconditions.checkNotNull(o);", + " // this is fine", + " consumeInt((int) o);", + " }", + "", + " static void m9(Map m) {", + " // BUG: Diagnostic contains: unboxing", + " consumeInt((int) m.get(\"foo\"));", + " }", + "", + " static void m10(Map m) {", + " if (m.get(\"bar\") != null) {", + " // this is fine", + " consumeInt((int) m.get(\"bar\"));", " }", + " }", + "", + " static void m11(Map m) {", + " Preconditions.checkNotNull(m.get(\"bar\"));", + " // this is fine", + " consumeInt((int) m.get(\"bar\"));", + " }", "}") .doTest(); } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayCustomLibraryModelsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayCustomLibraryModelsTests.java index 6718e5d9a5..f8b8a17964 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayCustomLibraryModelsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayCustomLibraryModelsTests.java @@ -55,21 +55,25 @@ public void allowLibraryModelsOverrideAnnotations() { .addSourceLines( "Foo.java", "package com.uber;", + "", "public class Foo {", - " Object field = new Object();", - " Object bar() {", - " return new Object();", - " }", - " Object nullableReturn() {", - " // BUG: Diagnostic contains: returning @Nullable", - " return bar();", - " }", - " void run() {", - " // just to make sure, flow analysis is also impacted by library models information", - " Object temp = bar();", - " // BUG: Diagnostic contains: assigning @Nullable", - " this.field = temp;", - " }", + " Object field = new Object();", + "", + " Object bar() {", + " return new Object();", + " }", + "", + " Object nullableReturn() {", + " // BUG: Diagnostic contains: returning @Nullable", + " return bar();", + " }", + "", + " void run() {", + " // just to make sure, flow analysis is also impacted by library models information", + " Object temp = bar();", + " // BUG: Diagnostic contains: assigning @Nullable", + " this.field = temp;", + " }", "}") .doTest(); } @@ -86,23 +90,27 @@ public void libraryModelsOverrideRestrictiveAnnotations() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.uber.lib.unannotated.RestrictivelyAnnotatedFIWithModelOverride;", - "import javax.annotation.Nullable;", + "", "public class Test {", " void bar(RestrictivelyAnnotatedFIWithModelOverride f) {", - " // Param is @NullableDecl in bytecode, overridden by library model", - " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull", - " f.apply(null);", + " // Param is @NullableDecl in bytecode, overridden by library model", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull", + " f.apply(null);", " }", + "", " void foo() {", - " RestrictivelyAnnotatedFIWithModelOverride func = (x) -> {", - " // Param is @NullableDecl in bytecode, overridden by library model, thus safe", - " return x.toString();", - " };", + " RestrictivelyAnnotatedFIWithModelOverride func =", + " (x) -> {", + " // Param is @NullableDecl in bytecode, overridden by library model, thus safe", + " return x.toString();", + " };", " }", + "", " void baz() {", - " // Safe to pass, since Function can't have a null instance parameter", - " bar(Object::toString);", + " // Safe to pass, since Function can't have a null instance parameter", + " bar(Object::toString);", " }", "}") .doTest(); diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayEnsuresNonNullTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayEnsuresNonNullTests.java index 1e9e09c851..4a6c94dae1 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayEnsuresNonNullTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayEnsuresNonNullTests.java @@ -10,25 +10,28 @@ public void requiresNonNullInterpretation() { .addSourceLines( "Foo.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.nullaway.annotations.RequiresNonNull;", + "import javax.annotation.Nullable;", + "", "class Foo {", " @Nullable Item nullableItem;", + "", " @RequiresNonNull(\"nullableItem\")", " public void run() {", " nullableItem.call();", " nullableItem = null;", " // BUG: Diagnostic contains: dereferenced expression nullableItem is @Nullable", " nullableItem.call();", - " ", " }", + "", " @RequiresNonNull(\"this.nullableItem\")", " public void test() {", " nullableItem.call();", " }", "}") .addSourceLines( - "Item.java", "package com.uber;", "class Item {", " public void call() { }", "}") + "Item.java", "package com.uber;", "", "class Item {", " public void call() {}", "}") .doTest(); } @@ -42,24 +45,28 @@ public void requiresEnsuresNonNullStaticFields() { .addSourceLines( "Foo.java", "package com.uber;", - "import javax.annotation.Nullable;", - "import com.uber.nullaway.annotations.RequiresNonNull;", + "", "import com.uber.nullaway.annotations.EnsuresNonNull;", + "import com.uber.nullaway.annotations.RequiresNonNull;", + "import javax.annotation.Nullable;", + "", "class Foo {", " @Nullable static Item nullableItem;", + "", " @RequiresNonNull(\"nullableItem\")", " // BUG: Diagnostic contains: For @RequiresNonNull annotation, cannot find instance field", " public static void run() {", " // BUG: Diagnostic contains: dereferenced expression nullableItem is @Nullable", " nullableItem.call();", - " ", " }", + "", " @RequiresNonNull(\"this.nullableItem\")", " // BUG: Diagnostic contains: For @RequiresNonNull annotation, cannot find instance field", " public static void test() {", " // BUG: Diagnostic contains: dereferenced expression nullableItem is @Nullable", " nullableItem.call();", " }", + "", " @EnsuresNonNull(\"nullableItem\")", " // BUG: Diagnostic contains: For @EnsuresNonNull annotation, cannot find instance field", " public static void test2() {", @@ -67,7 +74,7 @@ public void requiresEnsuresNonNullStaticFields() { " }", "}") .addSourceLines( - "Item.java", "package com.uber;", "class Item {", " public void call() { }", "}") + "Item.java", "package com.uber;", "", "class Item {", " public void call() {}", "}") .doTest(); } @@ -77,24 +84,30 @@ public void supportRequiresNonNullOverridingTest() { .addSourceLines( "SuperClass.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.nullaway.annotations.RequiresNonNull;", + "import javax.annotation.Nullable;", + "", "class SuperClass {", " @Nullable Item a;", + "", " @RequiresNonNull(\"a\")", " public void test0() {", " a.call();", " }", - " public void test1() {", - " }", + "", + " public void test1() {}", + "", " @RequiresNonNull(\"a\")", " public void test2() {", " a.call();", " }", + "", " @RequiresNonNull(\"a\")", " public void test3() {", " a.call();", " }", + "", " @RequiresNonNull(\"a\")", " public void test4() {", " a.call();", @@ -103,25 +116,34 @@ public void supportRequiresNonNullOverridingTest() { .addSourceLines( "ChildLevelOne.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.nullaway.annotations.RequiresNonNull;", + "import javax.annotation.Nullable;", + "", "class ChildLevelOne extends SuperClass {", " @Nullable Item b;", - " public void test0() { }", + "", + " public void test0() {}", + "", " public void test1() {", " // BUG: Diagnostic contains: dereferenced expression a is @Nullable", " a.call();", " }", + "", " public void test2() {", " // BUG: Diagnostic contains: dereferenced expression a is @Nullable", " a.call();", " }", + "", " @RequiresNonNull(\"a\")", " public void test3() {", " a.call();", " }", + "", " @RequiresNonNull(\"b\")", - " // BUG: Diagnostic contains: precondition inheritance is violated, method in child class cannot have a stricter precondition than its closest overridden method, adding @requiresNonNull for fields [a] makes this method precondition stricter", + " // BUG: Diagnostic contains: precondition inheritance is violated, method in child class cannot", + " // have a stricter precondition than its closest overridden method, adding @requiresNonNull for", + " // fields [a] makes this method precondition stricter", " public void test4() {", " // BUG: Diagnostic contains: dereferenced expression a is @Nullable", " a.call();", @@ -129,7 +151,7 @@ public void supportRequiresNonNullOverridingTest() { " }", "}") .addSourceLines( - "Item.java", "package com.uber;", "class Item {", " public void call() { }", "}") + "Item.java", "package com.uber;", "", "class Item {", " public void call() {}", "}") .doTest(); } @@ -139,40 +161,52 @@ public void ensuresNonNullInterpretation() { .addSourceLines( "Foo.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.nullaway.annotations.EnsuresNonNull;", + "import javax.annotation.Nullable;", + "", "class Foo {", " @Nullable Item nullItem;", " Foo foo = new Foo();", + "", " @EnsuresNonNull(\"nullItem\")", " public void test1() {", " nullItem = new Item();", " }", + "", " @EnsuresNonNull(\"nullItem\")", - " // BUG: Diagnostic contains: test2() is annotated with @EnsuresNonNull annotation, it indicates that all fields in the annotation parameter must be guaranteed to be nonnull at exit point. However, the method's body fails to ensure this for the following fields: [nullItem]", - " public void test2() {", - " }", + " // BUG: Diagnostic contains: test2() is annotated with @EnsuresNonNull annotation, it indicates", + " // that all fields in the annotation parameter must be guaranteed to be nonnull at exit point.", + " // However, the method's body fails to ensure this for the following fields: [nullItem]", + " public void test2() {}", + "", " @EnsuresNonNull(\"this.nullItem\")", " public void test3() {", " test1();", " }", + "", " @EnsuresNonNull(\"other.nullItem\")", - " // BUG: Diagnostic contains: currently @EnsuresNonNull supports only class fields of the method receiver: other.nullItem is not supported", + " // BUG: Diagnostic contains: currently @EnsuresNonNull supports only class fields of the method", + " // receiver: other.nullItem is not supported", " public void test4() {", " nullItem = new Item();", " }", + "", " @EnsuresNonNull(\"nullItem\")", - " // BUG: Diagnostic contains: method: test5() is annotated with @EnsuresNonNull annotation, it indicates that all fields in the annotation parameter must be guaranteed to be nonnull at exit point. However, the method's body fails to ensure this for the following fields: [nullItem]", + " // BUG: Diagnostic contains: method: test5() is annotated with @EnsuresNonNull annotation, it", + " // indicates that all fields in the annotation parameter must be guaranteed to be nonnull at exit", + " // point. However, the method's body fails to ensure this for the following fields: [nullItem]", " public void test5() {", " this.foo.test1();", " }", + "", " @EnsuresNonNull(\"nullItem\")", " public void test6() {", " this.test1();", " }", "}") .addSourceLines( - "Item.java", "package com.uber;", "class Item {", " public void call() { }", "}") + "Item.java", "package com.uber;", "", "class Item {", " public void call() {}", "}") .doTest(); } @@ -182,18 +216,23 @@ public void supportEnsuresNonNullOverridingTest() { .addSourceLines( "SuperClass.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.nullaway.annotations.EnsuresNonNull;", + "import javax.annotation.Nullable;", + "", "class SuperClass {", " @Nullable Item a;", + "", " @EnsuresNonNull(\"a\")", " public void test1() {", " a = new Item();", " }", + "", " @EnsuresNonNull(\"a\")", " public void test2() {", " a = new Item();", " }", + "", " @EnsuresNonNull(\"a\")", " public void noAnnotation() {", " a = new Item();", @@ -202,26 +241,35 @@ public void supportEnsuresNonNullOverridingTest() { .addSourceLines( "ChildLevelOne.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.nullaway.annotations.EnsuresNonNull;", + "import javax.annotation.Nullable;", + "", "class ChildLevelOne extends SuperClass {", " @Nullable Item b;", + "", " @EnsuresNonNull(\"b\")", - " // BUG: Diagnostic contains: postcondition inheritance is violated, this method must guarantee that all fields written in the @EnsuresNonNull annotation of overridden method SuperClass.test1 are @NonNull at exit point as well. Fields [a] must explicitly appear as parameters at this method @EnsuresNonNull annotation", + " // BUG: Diagnostic contains: postcondition inheritance is violated, this method must guarantee", + " // that all fields written in the @EnsuresNonNull annotation of overridden method SuperClass.test1", + " // are @NonNull at exit point as well. Fields [a] must explicitly appear as parameters at this", + " // method @EnsuresNonNull annotation", " public void test1() {", " b = new Item();", " }", + "", " @EnsuresNonNull({\"b\", \"a\"})", " public void test2() {", " super.test2();", " b = new Item();", " }", - " // BUG: Diagnostic contains: postcondition inheritance is violated, this method must guarantee that all fields written in the @EnsuresNonNull annotation of overridden method SuperClass.noAnnotation are @NonNull at exit point as well. Fields [a] must explicitly appear as parameters at this method @EnsuresNonNull annotation", - " public void noAnnotation() {", - " }", + " // BUG: Diagnostic contains: postcondition inheritance is violated, this method must guarantee", + " // that all fields written in the @EnsuresNonNull annotation of overridden method", + " // SuperClass.noAnnotation are @NonNull at exit point as well. Fields [a] must explicitly appear", + " // as parameters at this method @EnsuresNonNull annotation", + " public void noAnnotation() {}", "}") .addSourceLines( - "Item.java", "package com.uber;", "class Item {", " public void call() { }", "}") + "Item.java", "package com.uber;", "", "class Item {", " public void call() {}", "}") .doTest(); } @@ -231,28 +279,35 @@ public void supportEnsuresAndRequiresNonNullContract() { .addSourceLines( "Content.java", "package com.uber;", - "import javax.annotation.Nullable;", - "import com.uber.nullaway.annotations.RequiresNonNull;", + "", "import com.uber.nullaway.annotations.EnsuresNonNull;", + "import com.uber.nullaway.annotations.RequiresNonNull;", + "import javax.annotation.Nullable;", + "", "class Content {", " @Nullable Item nullItem;", + "", " @RequiresNonNull(\"nullItem\")", " public void run() {", " nullItem.call();", " }", + "", " @EnsuresNonNull(\"nullItem\")", " public void init() {", " nullItem = new Item();", " }", + "", " public void test1() {", " init();", " run();", " }", + "", " public void test2() {", " Content content = new Content();", " content.init();", " content.run();", " }", + "", " public void test3() {", " Content content = new Content();", " init();", @@ -263,18 +318,21 @@ public void supportEnsuresAndRequiresNonNullContract() { " }", "}") .addSourceLines( - "Item.java", "package com.uber;", "class Item {", " public void call() { }", "}") + "Item.java", "package com.uber;", "", "class Item {", " public void call() {}", "}") .addSourceLines( "Box.java", "package com.uber;", + "", "class Box {", " Content content = new Content();", "}") .addSourceLines( "Office.java", "package com.uber;", + "", "class Office {", " Box box = new Box();", + "", " public void test1() {", " Office office1 = new Office();", " Office office2 = new Office();", @@ -282,11 +340,13 @@ public void supportEnsuresAndRequiresNonNullContract() { " // BUG: Diagnostic contains: Expected field nullItem to be non-null at call site", " office2.box.content.run();", " }", + "", " public void test2() {", " Office office1 = new Office();", " office1.box.content.init();", " office1.box.content.run();", " }", + "", " public void test3() {", " Box box = new Box();", " this.box.content.init();", @@ -294,6 +354,7 @@ public void supportEnsuresAndRequiresNonNullContract() { " // BUG: Diagnostic contains: Expected field nullItem to be non-null at call site", " box.content.run();", " }", + "", " public void test4(int i) {", " Office office1 = new Office();", " Office office2 = new Office();", diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayFrameworkTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayFrameworkTests.java index 438b1d8ea2..bce92b6cf2 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayFrameworkTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayFrameworkTests.java @@ -44,8 +44,10 @@ public void supportObjectsIsNull() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.Objects;", "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable String s) {", " if (!Objects.isNull(s)) {", @@ -62,18 +64,20 @@ public void testJDKPathGetParentModel() { .addSourceLines( "Test.java", "package com.uber;", - "import java.util.Optional;", + "", "import java.nio.file.Files;", "import java.nio.file.Path;", + "import java.util.Optional;", + "", "public class Test {", - " Optional findConfig(Path searchDir) {", + " Optional findConfig(Path searchDir) {", " Path configFile = searchDir.resolve(\"foo.yml\");", " if (Files.exists(configFile)) {", " return Optional.of(configFile);", " }", " // BUG: Diagnostic contains: passing @Nullable parameter 'searchDir.getParent()' where @NonNull", " return this.findConfig(searchDir.getParent());", - " }", + " }", "}") .doTest(); } @@ -84,13 +88,16 @@ public void defaultLibraryModelsObjectNonNull() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.Objects;", "import javax.annotation.Nullable;", + "", "public class Test {", " String foo(@Nullable Object o) {", " if (Objects.nonNull(o)) {", - " return o.toString();", - " };", + " return o.toString();", + " }", + " ;", " return \"\";", " }", "}") @@ -104,24 +111,36 @@ public void checkForNullSupport() { .addSourceLines( "TestNullable.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class TestNullable {", - " @Nullable", - " Object nullable = new Object();", - " public void setNullable(@Nullable Object nullable) {this.nullable = nullable;}", + " @Nullable Object nullable = new Object();", + "", + " public void setNullable(@Nullable Object nullable) {", + " this.nullable = nullable;", + " }", " // BUG: Diagnostic contains: dereferenced expression nullable is @Nullable", - " public void run() {System.out.println(nullable.toString());}", + " public void run() {", + " System.out.println(nullable.toString());", + " }", "}") .addSourceLines( "TestCheckForNull.java", "package com.uber;", + "", "import javax.annotation.CheckForNull;", + "", "class TestCheckForNull {", - " @CheckForNull", - " Object checkForNull = new Object();", - " public void setCheckForNull(@CheckForNull Object checkForNull) {this.checkForNull = checkForNull;}", + " @CheckForNull Object checkForNull = new Object();", + "", + " public void setCheckForNull(@CheckForNull Object checkForNull) {", + " this.checkForNull = checkForNull;", + " }", " // BUG: Diagnostic contains: dereferenced expression checkForNull is @Nullable", - " public void run() {System.out.println(checkForNull.toString());}", + " public void run() {", + " System.out.println(checkForNull.toString());", + " }", "}") .doTest(); } @@ -134,12 +153,15 @@ public void orElseLibraryModelSupport() { .addSourceLines( "TestOptionalOrElseNegative.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import java.util.Optional;", + "import javax.annotation.Nullable;", + "", "class TestOptionalOrElseNegative {", " public Object foo(Optional o) {", " return o.orElse(\"Something\");", " }", + "", " public @Nullable Object bar(Optional o) {", " return o.orElse(null);", " }", @@ -147,12 +169,15 @@ public void orElseLibraryModelSupport() { .addSourceLines( "TestOptionalOrElsePositive.java", "package com.uber;", + "", "import java.util.Optional;", + "", "class TestOptionalOrElsePositive {", " public Object foo(Optional o) {", " // BUG: Diagnostic contains: returning @Nullable expression", " return o.orElse(null);", " }", + "", " public void bar(Optional o) {", " // BUG: Diagnostic contains: dereferenced expression o.orElse(null) is @Nullable", " System.out.println(o.orElse(null).toString());", @@ -168,19 +193,23 @@ public void overridingNativeModelsInAnnotatedCodeDoesNotPropagateTheModel() { .addSourceLines( "NonNullGetMessage.java", "package com.uber;", + "", "import java.util.Objects;", - "import javax.annotation.Nullable;", + "", "class NonNullGetMessage extends RuntimeException {", " NonNullGetMessage(final String message) {", - " super(message);", + " super(message);", " }", + "", " @Override", " public String getMessage() {", " return Objects.requireNonNull(super.getMessage());", " }", + "", " public static void foo(NonNullGetMessage e) {", " expectsNonNull(e.getMessage());", " }", + "", " public static void expectsNonNull(String str) {", " System.out.println(str);", " }", @@ -195,12 +224,14 @@ public void overridingNativeModelsInAnnotatedCodeDoesNotGenerateSafetyHoles() { .addSourceLines( "NonNullGetMessage.java", "package com.uber;", - "import java.util.Objects;", + "", "import javax.annotation.Nullable;", + "", "class NonNullGetMessage extends RuntimeException {", " NonNullGetMessage(@Nullable String message) {", - " super(message);", + " super(message);", " }", + "", " @Override", " public String getMessage() {", " // BUG: Diagnostic contains: returning @Nullable expression", @@ -216,11 +247,14 @@ public void springAutowiredFieldTest() { .addSourceLines( "Foo.java", "package com.uber;", + "", "import javax.annotation.Nullable;", "import org.springframework.stereotype.Component;", + "", "@Component", "public class Foo {", " @Nullable String bar;", + "", " public void setBar(String s) {", " bar = s;", " }", @@ -228,12 +262,14 @@ public void springAutowiredFieldTest() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.springframework.beans.factory.annotation.Autowired;", "import org.springframework.stereotype.Service;", + "", "@Service", "public class Test {", - " @Autowired", - " Foo f;", // Initialized by spring. + " @Autowired Foo f;", + "", " public void Fun() {", " f.setBar(\"hello\");", " }", @@ -247,11 +283,14 @@ public void springAutowiredConstructorTest() { .addSourceLines( "Foo.java", "package com.uber;", + "", "import javax.annotation.Nullable;", "import org.springframework.stereotype.Component;", + "", "@Component", "public class Foo {", " @Nullable String bar;", + "", " public void setBar(String s) {", " bar = s;", " }", @@ -259,15 +298,19 @@ public void springAutowiredConstructorTest() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.springframework.beans.factory.annotation.Autowired;", "import org.springframework.stereotype.Service;", + "", "@Service", "public class Test {", - " Foo f;", // Initialized by spring. + " Foo f;", + "", " @Autowired", " public void init() {", - " f = new Foo();", + " f = new Foo();", " }", + "", " public void Fun() {", " f.setBar(\"hello\");", " }", @@ -286,29 +329,36 @@ public void testLombokBuilderWithGeneratedAsUnannotated() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.lombok.LombokDTO;", + "import javax.annotation.Nullable;", + "", "class Test {", " void testSetters(LombokDTO ldto) {", - " ldto.setNullableField(null);", - " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", - " ldto.setField(null);", + " ldto.setNullableField(null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " ldto.setField(null);", " }", + "", " String testGetterSafe(LombokDTO ldto) {", - " return ldto.getField();", + " return ldto.getField();", " }", + "", " String testGetterNullable(LombokDTO ldto) {", - " // BUG: Diagnostic contains: returning @Nullable expression from method with @NonNull return type", - " return ldto.getNullableField();", + " // BUG: Diagnostic contains: returning @Nullable expression from method with @NonNull return", + " // type", + " return ldto.getNullableField();", " }", + "", " LombokDTO testBuilderSafe(@Nullable String s1, String s2) {", - " // Safe, because s2 is non-null and nullableField can take @Nullable", - " return LombokDTO.builder().nullableField(s1).field(s2).build();", + " // Safe, because s2 is non-null and nullableField can take @Nullable", + " return LombokDTO.builder().nullableField(s1).field(s2).build();", " }", + "", " LombokDTO testBuilderUnsafe(@Nullable String s1, @Nullable String s2) {", - " // No error, because the code of LombokDTO.Builder is @Generated and we are", - " // building with TreatGeneratedAsUnannotated=true", - " return LombokDTO.builder().nullableField(s1).field(s2).build();", + " // No error, because the code of LombokDTO.Builder is @Generated and we are", + " // building with TreatGeneratedAsUnannotated=true", + " return LombokDTO.builder().nullableField(s1).field(s2).build();", " }", "}") .doTest(); @@ -324,28 +374,35 @@ public void testLombokBuilderWithoutGeneratedAsUnannotated() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.uber.lombok.LombokDTO;", + "import javax.annotation.Nullable;", + "", "class Test {", " void testSetters(LombokDTO ldto) {", - " ldto.setNullableField(null);", - " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", - " ldto.setField(null);", + " ldto.setNullableField(null);", + " // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required", + " ldto.setField(null);", " }", + "", " String testGetterSafe(LombokDTO ldto) {", - " return ldto.getField();", + " return ldto.getField();", " }", + "", " String testGetterNullable(LombokDTO ldto) {", - " // BUG: Diagnostic contains: returning @Nullable expression from method with @NonNull return type", - " return ldto.getNullableField();", + " // BUG: Diagnostic contains: returning @Nullable expression from method with @NonNull return", + " // type", + " return ldto.getNullableField();", " }", + "", " LombokDTO testBuilderSafe(@Nullable String s1, String s2) {", - " // Safe, because s2 is non-null and nullableField can take @Nullable", - " return LombokDTO.builder().nullableField(s1).field(s2).build();", + " // Safe, because s2 is non-null and nullableField can take @Nullable", + " return LombokDTO.builder().nullableField(s1).field(s2).build();", " }", + "", " LombokDTO testBuilderUnsafe(@Nullable String s1, @Nullable String s2) {", - " // BUG: Diagnostic contains: passing @Nullable parameter 's2' where @NonNull is required", - " return LombokDTO.builder().nullableField(s1).field(s2).build();", + " // BUG: Diagnostic contains: passing @Nullable parameter 's2' where @NonNull is required", + " return LombokDTO.builder().nullableField(s1).field(s2).build();", " }", "}") .doTest(); @@ -357,9 +414,10 @@ public void systemConsoleNullable() { .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", " void foo() {", - " // BUG: Diagnostic contains: dereferenced expression System.console()", + " // BUG: Diagnostic contains: dereferenced expression System.console()", " System.console().toString();", " }", "}") diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayGuavaAssertionsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayGuavaAssertionsTests.java index 83ee0c94e5..bb20ff5f1e 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayGuavaAssertionsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayGuavaAssertionsTests.java @@ -15,8 +15,10 @@ public void checkNotNullTest() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Preconditions;", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " Preconditions.checkNotNull(a);", @@ -36,10 +38,13 @@ public void checkNotNullComplexAccessPathsTest() { .addSourceLines( "TestField.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Preconditions;", + "import javax.annotation.Nullable;", + "", "class TestField {", " @Nullable private Object f = null;", + "", " private void foo(@Nullable TestField a) {", " Preconditions.checkNotNull(a);", " Preconditions.checkNotNull(a.f);", @@ -49,11 +54,13 @@ public void checkNotNullComplexAccessPathsTest() { .addSourceLines( "TestMap.java", "package com.uber;", + "", + "import com.google.common.base.Preconditions;", "import java.util.Map;", "import javax.annotation.Nullable;", - "import com.google.common.base.Preconditions;", + "", "class TestMap {", - " private void foo(@Nullable Map m) {", + " private void foo(@Nullable Map m) {", " Preconditions.checkNotNull(m);", " Preconditions.checkNotNull(m.get(\"foo\"));", " m.get(\"foo\").toString();", @@ -72,13 +79,16 @@ public void verifyNotNullTest() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Verify;", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " Verify.verifyNotNull(a);", " a.toString();", " }", + "", " private void bar(@Nullable Object a) {", " Verify.verifyNotNull(a, \"message\", new Object(), new Object());", " a.toString();", @@ -97,8 +107,10 @@ public void simpleCheckArgumentTest() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Preconditions;", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " Preconditions.checkArgument(a != null);", @@ -118,10 +130,13 @@ public void simpleCheckStateTest() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Preconditions;", + "import javax.annotation.Nullable;", + "", "class Test {", " @Nullable private Object a;", + "", " private void foo() {", " Preconditions.checkState(this.a != null);", " a.toString();", @@ -140,8 +155,10 @@ public void simpleVerifyTest() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Verify;", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " Verify.verify(a != null);", @@ -161,8 +178,10 @@ public void simpleCheckArgumentWithMessageTest() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Preconditions;", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " Preconditions.checkArgument(a != null, \"a ought to be non-null\");", @@ -182,8 +201,10 @@ public void compoundCheckArgumentTest() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Preconditions;", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " Preconditions.checkArgument(a != null && !a.equals(this));", @@ -203,8 +224,10 @@ public void compoundCheckArgumentLastTest() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Preconditions;", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " Preconditions.checkArgument(this.hashCode() != 5 && a != null);", @@ -224,10 +247,17 @@ public void compoundCheckArgumentLongTest() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Preconditions;", + "import javax.annotation.Nullable;", + "", "class Test {", - " private void foo(@Nullable Object a, @Nullable Object b, @Nullable Object c, @Nullable Object d, @Nullable Object e) {", + " private void foo(", + " @Nullable Object a,", + " @Nullable Object b,", + " @Nullable Object c,", + " @Nullable Object d,", + " @Nullable Object e) {", " Preconditions.checkArgument(a != null && b != null && c != null && d != null && e != null);", " a.toString();", " b.toString();", @@ -249,9 +279,11 @@ public void nestedCallCheckArgumentTest() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Preconditions;", "import com.google.common.base.Strings;", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable String a) {", " Preconditions.checkArgument(!Strings.isNullOrEmpty(a));", @@ -271,8 +303,10 @@ public void irrelevantCheckArgumentTest() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Preconditions;", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " Preconditions.checkArgument(this.hashCode() != 5);", @@ -293,8 +327,10 @@ public void inconclusiveCheckArgumentTest() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Preconditions;", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " Preconditions.checkArgument(this.hashCode() != 5 || a != null);", @@ -315,13 +351,16 @@ public void checkArgumentCatchException() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Preconditions;", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " try {", " Preconditions.checkArgument(a != null);", - " } catch (IllegalArgumentException e) {}", + " } catch (IllegalArgumentException e) {", + " }", " // BUG: Diagnostic contains: dereferenced expression a is @Nullable", " a.toString();", " }", @@ -339,13 +378,16 @@ public void checkStateCatchException() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Preconditions;", + "import javax.annotation.Nullable;", + "", "class Test {", " private void foo(@Nullable Object a) {", " try {", " Preconditions.checkState(a != null);", - " } catch (IllegalStateException e) {}", + " } catch (IllegalStateException e) {", + " }", " // BUG: Diagnostic contains: dereferenced expression a is @Nullable", " a.toString();", " }", diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayInitializationTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayInitializationTests.java index 1ad91e0a53..dd3aa83aa8 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayInitializationTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayInitializationTests.java @@ -35,15 +35,17 @@ public void externalInitSupport() { .addSourceLines( "ExternalInit.java", "package com.uber;", + "", "@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.CLASS)", "public @interface ExternalInit {}") .addSourceLines( "Test.java", "package com.uber;", + "", "@ExternalInit", "class Test {", " Object f;", - // no error here due to external init + "", " public Test() {}", " // BUG: Diagnostic contains: initializer method does not guarantee @NonNull field", " public Test(int x) {}", @@ -51,14 +53,15 @@ public void externalInitSupport() { .addSourceLines( "Test2.java", "package com.uber;", + "", "@ExternalInit", "class Test2 {", - // no error here due to external init " Object f;", "}") .addSourceLines( "Test3.java", "package com.uber;", + "", "@ExternalInit", "class Test3 {", " Object f;", @@ -74,35 +77,38 @@ public void externalInitSupportFields() { .addSourceLines( "ExternalFieldInit.java", "package com.uber;", + "", "@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.CLASS)", "public @interface ExternalFieldInit {}") .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", " @ExternalFieldInit Object f;", - // no error here due to external init + "", " public Test() {}", - // no error here due to external init + "", " public Test(int x) {}", "}") .addSourceLines( "Test2.java", "package com.uber;", + "", "class Test2 {", - // no error here due to external init " @ExternalFieldInit Object f;", "}") .addSourceLines( "Test3.java", "package com.uber;", + "", "class Test3 {", " @ExternalFieldInit Object f;", - // no error here due to external init - " @ExternalFieldInit", // See GitHub#184 + "", + " @ExternalFieldInit", " public Test3() {}", - // no error here due to external init - " @ExternalFieldInit", // See GitHub#184 + "", + " @ExternalFieldInit", " public Test3(int x) {}", "}") .doTest(); @@ -114,11 +120,13 @@ public void testEnumInit() { .addSourceLines( "SomeEnum.java", "package com.uber;", - "import java.util.Random;", + "", "enum SomeEnum {", - " FOO, BAR;", + " FOO,", + " BAR;", " final Object o;", " final Object p;", + "", " private SomeEnum() {", " this.o = new Object();", " this.p = new Object();", diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 3a0396792d..c3c183c508 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -12,16 +12,19 @@ public void basicTypeParamInstantiation() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.jspecify.annotations.Nullable;", + "", "class Test {", - " static class NonNullTypeParam {}", - " static class NullableTypeParam {}", + " static class NonNullTypeParam {}", + "", + " static class NullableTypeParam {}", + " // BUG: Diagnostic contains: Generic type parameter", + " static void testBadNonNull(NonNullTypeParam<@Nullable String> t1) {", " // BUG: Diagnostic contains: Generic type parameter", - " static void testBadNonNull(NonNullTypeParam<@Nullable String> t1) {", - " // BUG: Diagnostic contains: Generic type parameter", - " NonNullTypeParam<@Nullable String> t2 = null;", - " NullableTypeParam<@Nullable String> t3 = null;", - " }", + " NonNullTypeParam<@Nullable String> t2 = null;", + " NullableTypeParam<@Nullable String> t3 = null;", + " }", "}") .doTest(); } @@ -32,26 +35,33 @@ public void constructorTypeParamInstantiation() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.jspecify.annotations.Nullable;", + "", "class Test {", - " static class NonNullTypeParam {}", - " static class NullableTypeParam {}", - " static void testOkNonNull(NonNullTypeParam t) {", - " NonNullTypeParam t2 = new NonNullTypeParam();", - " }", - " static void testBadNonNull(NonNullTypeParam t) {", - " // BUG: Diagnostic contains: Generic type parameter", - " NonNullTypeParam t2 = new NonNullTypeParam<@Nullable String>();", - " // BUG: Diagnostic contains: Generic type parameter", - " testBadNonNull(new NonNullTypeParam<@Nullable String>());", - " testBadNonNull(new NonNullTypeParam<", - " // BUG: Diagnostic contains: Generic type parameter", - " @Nullable String>());", - " }", - " static void testOkNullable(NullableTypeParam t1, NullableTypeParam<@Nullable String> t2) {", - " NullableTypeParam t3 = new NullableTypeParam();", - " NullableTypeParam<@Nullable String> t4 = new NullableTypeParam<@Nullable String>();", - " }", + " static class NonNullTypeParam {}", + "", + " static class NullableTypeParam {}", + "", + " static void testOkNonNull(NonNullTypeParam t) {", + " NonNullTypeParam t2 = new NonNullTypeParam();", + " }", + "", + " static void testBadNonNull(NonNullTypeParam t) {", + " // BUG: Diagnostic contains: Generic type parameter", + " NonNullTypeParam t2 = new NonNullTypeParam<@Nullable String>();", + " // BUG: Diagnostic contains: Generic type parameter", + " testBadNonNull(new NonNullTypeParam<@Nullable String>());", + " testBadNonNull(", + " new NonNullTypeParam<", + " // BUG: Diagnostic contains: Generic type parameter", + " @Nullable String>());", + " }", + "", + " static void testOkNullable(NullableTypeParam t1, NullableTypeParam<@Nullable String> t2) {", + " NullableTypeParam t3 = new NullableTypeParam();", + " NullableTypeParam<@Nullable String> t4 = new NullableTypeParam<@Nullable String>();", + " }", "}") .doTest(); } @@ -62,16 +72,27 @@ public void multipleTypeParametersInstantiation() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.jspecify.annotations.Nullable;", + "", "class Test {", - " static class MixedTypeParam {}", - " // BUG: Diagnostic contains: Generic type parameter", - " static class PartiallyInvalidSubclass extends MixedTypeParam<@Nullable String, String, String, @Nullable String> {}", - " static class ValidSubclass1 extends MixedTypeParam {}", - " static class PartiallyInvalidSubclass2 extends MixedTypeParam {}", - " static class ValidSubclass2 extends MixedTypeParam {}", + " static class MixedTypeParam {}", + " // BUG: Diagnostic contains: Generic type parameter", + " static class PartiallyInvalidSubclass", + " extends MixedTypeParam<@Nullable String, String, String, @Nullable String> {}", + "", + " static class ValidSubclass1", + " extends MixedTypeParam {}", + "", + " static class PartiallyInvalidSubclass2", + " extends MixedTypeParam<", + " String,", + " String,", + " String,", + " // BUG: Diagnostic contains: Generic type parameter", + " @Nullable String> {}", + "", + " static class ValidSubclass2 extends MixedTypeParam {}", "}") .doTest(); } @@ -82,15 +103,19 @@ public void subClassTypeParamInstantiation() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.jspecify.annotations.Nullable;", + "", "class Test {", - " static class NonNullTypeParam {}", - " static class NullableTypeParam {}", - " static class SuperClassForValidSubclass {", - " static class ValidSubclass extends NullableTypeParam<@Nullable String> {}", - " // BUG: Diagnostic contains: Generic type parameter", - " static class InvalidSubclass extends NonNullTypeParam<@Nullable String> {}", - " }", + " static class NonNullTypeParam {}", + "", + " static class NullableTypeParam {}", + "", + " static class SuperClassForValidSubclass {", + " static class ValidSubclass extends NullableTypeParam<@Nullable String> {}", + " // BUG: Diagnostic contains: Generic type parameter", + " static class InvalidSubclass extends NonNullTypeParam<@Nullable String> {}", + " }", "}") .doTest(); } @@ -101,13 +126,18 @@ public void interfaceImplementationTypeParamInstantiation() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.jspecify.annotations.Nullable;", + "", "class Test {", - " static interface NonNullTypeParamInterface{}", - " static interface NullableTypeParamInterface{}", - " // BUG: Diagnostic contains: Generic type parameter", - " static class InvalidInterfaceImplementation implements NonNullTypeParamInterface<@Nullable String> {}", - " static class ValidInterfaceImplementation implements NullableTypeParamInterface {}", + " static interface NonNullTypeParamInterface {}", + "", + " static interface NullableTypeParamInterface {}", + " // BUG: Diagnostic contains: Generic type parameter", + " static class InvalidInterfaceImplementation", + " implements NonNullTypeParamInterface<@Nullable String> {}", + "", + " static class ValidInterfaceImplementation implements NullableTypeParamInterface {}", "}") .doTest(); } @@ -118,19 +148,22 @@ public void nestedTypeParams() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.jspecify.annotations.Nullable;", + "", "class Test {", - " static class NonNullTypeParam {}", - " static class NullableTypeParam {}", + " static class NonNullTypeParam {}", + "", + " static class NullableTypeParam {}", + " // BUG: Diagnostic contains: Generic type parameter", + " static void testBadNonNull(NullableTypeParam> t) {", + " // BUG: Diagnostic contains: Generic type parameter", + " NullableTypeParam>> t2 = null;", " // BUG: Diagnostic contains: Generic type parameter", - " static void testBadNonNull(NullableTypeParam> t) {", - " // BUG: Diagnostic contains: Generic type parameter", - " NullableTypeParam>> t2 = null;", - " // BUG: Diagnostic contains: Generic type parameter", - " t2 = new NullableTypeParam>>();", - " // this is fine", - " NullableTypeParam>> t3 = null;", - " }", + " t2 = new NullableTypeParam>>();", + " // this is fine", + " NullableTypeParam>> t3 = null;", + " }", "}") .doTest(); } @@ -141,17 +174,21 @@ public void returnTypeParamInstantiation() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.jspecify.annotations.Nullable;", + "", "class Test {", - " static class NonNullTypeParam {}", - " static class NullableTypeParam {}", - " // BUG: Diagnostic contains: Generic type parameter", - " static NonNullTypeParam<@Nullable String> testBadNonNull() {", - " return new NonNullTypeParam();", - " }", - " static NullableTypeParam<@Nullable String> testOKNull() {", - " return new NullableTypeParam<@Nullable String>();", - " }", + " static class NonNullTypeParam {}", + "", + " static class NullableTypeParam {}", + " // BUG: Diagnostic contains: Generic type parameter", + " static NonNullTypeParam<@Nullable String> testBadNonNull() {", + " return new NonNullTypeParam();", + " }", + "", + " static NullableTypeParam<@Nullable String> testOKNull() {", + " return new NullableTypeParam<@Nullable String>();", + " }", "}") .doTest(); } @@ -162,21 +199,26 @@ public void testOKNewClassInstantiationForOtherAnnotations() { .addSourceLines( "Test.java", "package com.uber;", + "", "import lombok.NonNull;", "import org.jspecify.annotations.Nullable;", + "", "class Test {", - " static class NonNullTypeParam {}", - " static class DifferentAnnotTypeParam1 {}", - " static class DifferentAnnotTypeParam2<@NonNull E> {}", - " static void testOKOtherAnnotation(NonNullTypeParam t) {", - " // should not show error for annotation other than @Nullable", - " testOKOtherAnnotation(new NonNullTypeParam<@NonNull String>());", - " DifferentAnnotTypeParam1 t1 = new DifferentAnnotTypeParam1();", - " // BUG: Diagnostic contains: Generic type parameter", - " DifferentAnnotTypeParam2 t2 = new DifferentAnnotTypeParam2<@Nullable String>();", - " // BUG: Diagnostic contains: Generic type parameter", - " DifferentAnnotTypeParam1 t3 = new DifferentAnnotTypeParam1<@Nullable String>();", - " }", + " static class NonNullTypeParam {}", + "", + " static class DifferentAnnotTypeParam1 {}", + "", + " static class DifferentAnnotTypeParam2<@NonNull E> {}", + "", + " static void testOKOtherAnnotation(NonNullTypeParam t) {", + " // should not show error for annotation other than @Nullable", + " testOKOtherAnnotation(new NonNullTypeParam<@NonNull String>());", + " DifferentAnnotTypeParam1 t1 = new DifferentAnnotTypeParam1();", + " // BUG: Diagnostic contains: Generic type parameter", + " DifferentAnnotTypeParam2 t2 = new DifferentAnnotTypeParam2<@Nullable String>();", + " // BUG: Diagnostic contains: Generic type parameter", + " DifferentAnnotTypeParam1 t3 = new DifferentAnnotTypeParam1<@Nullable String>();", + " }", "}") .doTest(); } @@ -187,9 +229,12 @@ public void downcastInstantiation() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.jspecify.annotations.Nullable;", + "", "class Test {", - " static class NonNullTypeParam { }", + " static class NonNullTypeParam {}", + "", " static void instOf(Object o) {", " // BUG: Diagnostic contains: Generic type parameter", " Object p = (NonNullTypeParam<@Nullable String>) o;", @@ -205,9 +250,12 @@ public void instantiationInUnannotatedCode() { .addSourceLines( "Test.java", "package com.other;", + "", "import org.jspecify.annotations.Nullable;", + "", "class Test {", - " static class NonNullTypeParam { }", + " static class NonNullTypeParam {}", + "", " static void instOf(Object o) {", " Object p = (NonNullTypeParam<@Nullable String>) o;", " }", diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyTests.java index ed9acb1f72..e889caf3dd 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyTests.java @@ -10,12 +10,16 @@ public void nullMarkedPackageLevel() { defaultCompilationHelper .addSourceLines( "package-info.java", - "@NullMarked package com.example.thirdparty;", + "@NullMarked", + "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;") .addSourceLines( "ThirdPartyAnnotatedUtils.java", "package com.example.thirdparty;", + "", "import org.jspecify.annotations.Nullable;", + "", "public class ThirdPartyAnnotatedUtils {", " public static String toStringOrDefault(@Nullable Object o1, String s) {", " if (o1 != null) {", @@ -27,7 +31,9 @@ public void nullMarkedPackageLevel() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.thirdparty.ThirdPartyAnnotatedUtils;", + "", "public class Test {", " public static void test(Object o) {", " // Safe: passing @NonNull on both args", @@ -47,16 +53,19 @@ public void nullMarkedPackageEnablesChecking() { defaultCompilationHelper .addSourceLines( "package-info.java", - "@NullMarked package com.example.thirdparty;", + "@NullMarked", + "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;") .addSourceLines( "Foo.java", "package com.example.thirdparty;", - "import org.jspecify.annotations.Nullable;", + "", "public class Foo {", " public static String foo(String s) {", " return s;", " }", + "", " public static void test() {", " // BUG: Diagnostic contains: passing @Nullable parameter", " foo(null);", @@ -71,7 +80,9 @@ public void nullMarkedClassLevel() { .addSourceLines( "Foo.java", "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;", + "", "@NullMarked", "public class Foo {", " public static String foo(String s) {", @@ -81,7 +92,9 @@ public void nullMarkedClassLevel() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.thirdparty.Foo;", + "", "public class Test {", " public static void test(Object o) {", " // BUG: Diagnostic contains: passing @Nullable parameter", @@ -97,12 +110,15 @@ public void nullMarkedClassLevelEnablesChecking() { .addSourceLines( "Foo.java", "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;", + "", "@NullMarked", "public class Foo {", " public static String foo(String s) {", " return s;", " }", + "", " public static void test(Object o) {", " // BUG: Diagnostic contains: passing @Nullable parameter", " foo(null);", @@ -117,8 +133,9 @@ public void nullMarkedClassLevelOuter() { .addSourceLines( "Bar.java", "package com.example.thirdparty;", - "import org.jspecify.annotations.Nullable;", + "", "import org.jspecify.annotations.NullMarked;", + "", "@NullMarked", "public class Bar {", " public static class Foo {", @@ -130,7 +147,9 @@ public void nullMarkedClassLevelOuter() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.thirdparty.Bar;", + "", "public class Test {", " public static void test(Object o) {", " // BUG: Diagnostic contains: passing @Nullable parameter", @@ -146,8 +165,9 @@ public void nullMarkedClassLevelInner() { .addSourceLines( "Bar.java", "package com.example.thirdparty;", - "import org.jspecify.annotations.Nullable;", + "", "import org.jspecify.annotations.NullMarked;", + "", "public class Bar {", " @NullMarked", " public static class Foo {", @@ -155,12 +175,15 @@ public void nullMarkedClassLevelInner() { " return s;", " }", " }", + "", " public static void unchecked(Object o) {}", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.thirdparty.Bar;", + "", "public class Test {", " public static void test(Object o) {", " // BUG: Diagnostic contains: passing @Nullable parameter", @@ -177,8 +200,9 @@ public void nullMarkedClassLevelInnerControlsChecking() { .addSourceLines( "Bar.java", "package com.example.thirdparty;", - "import org.jspecify.annotations.Nullable;", + "", "import org.jspecify.annotations.NullMarked;", + "", "public class Bar {", " @NullMarked", " public static class Foo {", @@ -191,6 +215,7 @@ public void nullMarkedClassLevelInnerControlsChecking() { " foo(null);", " }", " }", + "", " public static void unchecked() {", " Object x = null;", " // fine since this code is still unchecked", @@ -206,8 +231,10 @@ public void nullMarkedClassLevelLocalAndEnvironment() { .addSourceLines( "Test.java", "package com.example.thirdparty;", - "import org.jspecify.annotations.Nullable;", + "", "import org.jspecify.annotations.NullMarked;", + "import org.jspecify.annotations.Nullable;", + "", "public class Test {", " public static Object test() {", " Object x = null;", @@ -217,10 +244,12 @@ public void nullMarkedClassLevelLocalAndEnvironment() { " public Object returnsNonNull() {", " return y;", " }", + "", " @Nullable", " public Object returnsNullable() {", " return x;", " }", + "", " public Object returnsNonNullWithError() {", " // BUG: Diagnostic contains: returning @Nullable expression", " return x;", @@ -240,7 +269,9 @@ public void nullMarkedMethodLevel() { .addSourceLines( "Foo.java", "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;", + "", "public class Foo {", " @NullMarked", " public static String foo(String s) {", @@ -250,7 +281,9 @@ public void nullMarkedMethodLevel() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.thirdparty.Foo;", + "", "public class Test {", " public static void test(Object o) {", " // BUG: Diagnostic contains: passing @Nullable parameter", @@ -268,7 +301,9 @@ public void nullMarkedMethodLevelScan() { .addSourceLines( "Foo.java", "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;", + "", "public class Foo {", " @NullMarked", " public static String foo(String s) {", @@ -278,12 +313,15 @@ public void nullMarkedMethodLevelScan() { .addSourceLines( "Bar.java", "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;", + "", "public class Bar {", " public static void bar1() {", " // No report, unannotated caller!", " Foo.foo(null);", " }", + "", " @NullMarked", " public static void bar2() {", " // BUG: Diagnostic contains: passing @Nullable parameter", @@ -299,7 +337,9 @@ public void nullMarkedOuterMethodLevelWithAnonymousClass() { .addSourceLines( "Foo.java", "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;", + "", "public class Foo {", " @NullMarked", " public static String foo(String s) {", @@ -309,7 +349,9 @@ public void nullMarkedOuterMethodLevelWithAnonymousClass() { .addSourceLines( "Bar.java", "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;", + "", "public class Bar {", " @NullMarked", " public static Runnable runFoo() {", @@ -330,18 +372,22 @@ public void nullMarkedOuterMethodLevelUsage() { .addSourceLines( "IConsumer.java", "package com.example.thirdparty;", + "", "public interface IConsumer {", " void consume(Object s);", "}") .addSourceLines( "Foo.java", "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;", + "", "public class Foo {", " @NullMarked", " public static IConsumer getConsumer() {", " return new IConsumer() {", - " // Transitively null marked! Explicitly non-null arg, which is a safe override of unknown-nullness.", + " // Transitively null marked! Explicitly non-null arg, which is a safe override of", + " // unknown-nullness.", " public void consume(Object s) {", " System.out.println(s);", " }", @@ -351,7 +397,9 @@ public void nullMarkedOuterMethodLevelUsage() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.thirdparty.Foo;", + "", "public class Test {", " public static void test(Object o) {", " // Safe because IConsumer::consume is unmarked? And no static knowledge of Foo$1", @@ -367,7 +415,9 @@ public void nullMarkedOuterMethodLevelWithLocalClass() { .addSourceLines( "Foo.java", "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;", + "", "public class Foo {", " @NullMarked", " public static String foo(String s) {", @@ -377,7 +427,9 @@ public void nullMarkedOuterMethodLevelWithLocalClass() { .addSourceLines( "Bar.java", "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;", + "", "public class Bar {", " @NullMarked", " public static Object bar() {", @@ -401,24 +453,30 @@ public void nullMarkedOuterMethodLevelWithLocalClassInit() { .addSourceLines( "Test.java", "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;", + "", "public class Test {", " @NullMarked", " public static Object test() {", " class Foo {", " private Object o;", " // BUG: Diagnostic contains: initializer method does not guarantee @NonNull field o", - " public Foo() { }", + " public Foo() {}", + "", " public String foo(String s) {", " return s;", " }", " }", " return new Foo();", " }", + "", " public static Object test2() {", " class Foo {", - " private Object o;", // No init checking, since Test$2Foo is unmarked. - " public Foo() { }", + " private Object o;", + "", + " public Foo() {}", + "", " @NullMarked", " public String foo(String s) {", " return s;", @@ -426,10 +484,13 @@ public void nullMarkedOuterMethodLevelWithLocalClassInit() { " }", " return new Foo();", " }", + "", " public static Object test3() {", " class Foo {", - " private Object o;", // No init checking, since Test$2Foo is unmarked. - " public Foo() { }", + " private Object o;", + "", + " public Foo() {}", + "", " @NullMarked", " public String foo() {", " return o.toString();", @@ -451,12 +512,14 @@ public void configUnannotatedOverridesNullMarked() { "-XepOpt:NullAway:UnannotatedSubPackages=com.example")) .addSourceLines( "package-info.java", - "@NullMarked package com.example.thirdparty;", + "@NullMarked", + "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;") .addSourceLines( "Foo.java", "package com.example.thirdparty;", - "import org.jspecify.annotations.Nullable;", + "", "public class Foo {", " public static String foo(String s) {", " return s;", @@ -465,7 +528,9 @@ public void configUnannotatedOverridesNullMarked() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.thirdparty.Foo;", + "", "public class Test {", " public static void test(Object o) {", " // Safe: Foo is treated as unannotated", @@ -481,7 +546,9 @@ public void bytecodeNullMarkedPackageLevel() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.jspecify.annotatedpackage.Utils;", + "", "public class Test {", " public static void test(Object o) {", " // Safe: passing @NonNull on both args", @@ -502,7 +569,9 @@ public void bytecodeNullMarkedClassLevel() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.jspecify.unannotatedpackage.TopLevel;", + "", "public class Test {", " public static void test(Object o) {", " // BUG: Diagnostic contains: passing @Nullable parameter", @@ -518,7 +587,9 @@ public void bytecodeNullMarkedClassLevelInner() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.jspecify.unannotatedpackage.Outer;", + "", "public class Test {", " public static void test(Object o) {", " // BUG: Diagnostic contains: passing @Nullable parameter", @@ -535,7 +606,9 @@ public void bytecodeNullMarkedMethodLevel() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.jspecify.unannotatedpackage.Methods;", + "", "public class Test {", " public static void test(Object o) {", " // BUG: Diagnostic contains: passing @Nullable parameter", @@ -552,14 +625,21 @@ public void bytecodeNullMarkedMethodLevelOverriding() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.jspecify.unannotatedpackage.Methods;", "import org.jspecify.annotations.Nullable;", + "", "public class Test extends Methods.ExtendMe {", " @Nullable", " // BUG: Diagnostic contains: method returns @Nullable, but superclass method", - " public Object foo(@Nullable Object o) { return o; }", + " public Object foo(@Nullable Object o) {", + " return o;", + " }", + "", " @Nullable", - " public Object unchecked(@Nullable Object o) { return o; }", + " public Object unchecked(@Nullable Object o) {", + " return o;", + " }", "}") .doTest(); } @@ -569,12 +649,16 @@ public void nullUnmarkedPackageLevel() { defaultCompilationHelper .addSourceLines( "package-info.java", - "@NullUnmarked package com.uber.unmarked;", + "@NullUnmarked", + "package com.uber.unmarked;", + "", "import org.jspecify.annotations.NullUnmarked;") .addSourceLines( "MarkedBecauseAnnotatedFlag.java", "package com.uber.marked;", + "", "import org.jspecify.annotations.Nullable;", + "", "public class MarkedBecauseAnnotatedFlag {", " public static String nullSafeStringOrDefault(@Nullable Object o1, String s) {", " if (o1 != null) {", @@ -582,10 +666,12 @@ public void nullUnmarkedPackageLevel() { " }", " return s;", " }", + "", " @Nullable", " public static String nullRet() {", " return null;", " }", + "", " public static String unsafeStringOrDefault(@Nullable Object o1, String s) {", " // BUG: Diagnostic contains: dereferenced expression o1 is @Nullable", " return o1.toString();", @@ -594,17 +680,21 @@ public void nullUnmarkedPackageLevel() { .addSourceLines( "UnmarkedBecausePackageDirectAnnotation.java", "package com.uber.unmarked;", + "", "import com.uber.marked.MarkedBecauseAnnotatedFlag;", "import org.jspecify.annotations.Nullable;", + "", "public class UnmarkedBecausePackageDirectAnnotation {", " public static String directlyUnsafeStringOrDefault(@Nullable Object o1, String s) {", " // No error: unannotated", " return o1.toString();", " }", + "", " @Nullable", " public static String nullRet() {", " return null;", " }", + "", " public static String indirectlyUnsafeStringOrDefault(@Nullable Object o1, String s) {", " // No error: unannotated", " return (o1 == null ? MarkedBecauseAnnotatedFlag.nullRet() : o1.toString());", @@ -618,16 +708,20 @@ public void nullUnmarkedPackageLevel() { "import com.uber.marked.MarkedBecauseAnnotatedFlag;", "import com.uber.unmarked.UnmarkedBecausePackageDirectAnnotation;", "import org.jspecify.annotations.Nullable;", + "", "public class MarkedImplicitly {", " public static String directlyUnsafeStringOrDefault(@Nullable Object o1, String s) {", " // BUG: Diagnostic contains: dereferenced expression o1 is @Nullable", " return o1.toString();", " }", + "", " public static String indirectlyUnsafeStringOrDefault(@Nullable Object o1, String s) {", " // BUG: Diagnostic contains: returning @Nullable expression from method", " return (o1 == null ? MarkedBecauseAnnotatedFlag.nullRet() : o1.toString());", " }", - " public static String indirectlyUnsafeStringOrDefaultCallingUnmarked(@Nullable Object o1, String s) {", + "", + " public static String indirectlyUnsafeStringOrDefaultCallingUnmarked(", + " @Nullable Object o1, String s) {", " // No error: nullRet() is unannotated", " return (o1 == null ? UnmarkedBecausePackageDirectAnnotation.nullRet() : o1.toString());", " }", @@ -640,22 +734,28 @@ public void nullUnmarkedClassLevel() { defaultCompilationHelper .addSourceLines( "package-info.java", - "@NullMarked package com.example.thirdparty.marked;", + "@NullMarked", + "package com.example.thirdparty.marked;", + "", "import org.jspecify.annotations.NullMarked;") .addSourceLines( "Foo.java", "package com.uber.foo;", + "", "import org.jspecify.annotations.NullUnmarked;", "import org.jspecify.annotations.Nullable;", + "", "@NullUnmarked", "public class Foo {", " @Nullable", " public static String nullRet() {", " return null;", " }", + "", " public static String takeNonNull(Object o) {", " return o.toString();", " }", + "", " public static String takeNullable(@Nullable Object o) {", " return o.toString();", " }", @@ -663,13 +763,16 @@ public void nullUnmarkedClassLevel() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.uber.foo.Foo;", + "", "public class Test {", " public static Object test(Object o) {", " // No errors, because Foo is @NullUnmarked", " Foo.takeNonNull(null);", " return Foo.nullRet();", " }", + "", " public static String sanity() {", " // BUG: Diagnostic contains: returning @Nullable expression", " return null;", @@ -679,13 +782,16 @@ public void nullUnmarkedClassLevel() { .addSourceLines( "Test2.java", "package com.example.thirdparty.marked;", + "", "import com.uber.foo.Foo;", + "", "public class Test2 {", " public static Object test(Object o) {", " // No errors, because Foo is @NullUnmarked", " Foo.takeNonNull(null);", " return Foo.nullRet();", " }", + "", " public static String sanity() {", " // BUG: Diagnostic contains: returning @Nullable expression", " return null;", @@ -700,8 +806,10 @@ public void nullUnmarkedClassLevelOuter() { .addSourceLines( "Bar.java", "package com.uber.foo;", + "", "import org.jspecify.annotations.NullUnmarked;", "import org.jspecify.annotations.Nullable;", + "", "@NullUnmarked", "public class Bar {", " public static class Foo {", @@ -709,9 +817,11 @@ public void nullUnmarkedClassLevelOuter() { " public static String nullRet() {", " return null;", " }", + "", " public static String takeNonNull(Object o) {", " return o.toString();", " }", + "", " public static String takeNullable(@Nullable Object o) {", " // No errors, because Foo is @NullUnmarked", " return o.toString();", @@ -721,13 +831,16 @@ public void nullUnmarkedClassLevelOuter() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.uber.foo.Bar;", + "", "public class Test {", " public static Object test(Object o) {", " // No errors, because Foo is @NullUnmarked", " Bar.Foo.takeNonNull(null);", " return Bar.Foo.nullRet();", " }", + "", " public static String sanity() {", " // BUG: Diagnostic contains: returning @Nullable expression", " return null;", @@ -742,8 +855,10 @@ public void nullUnmarkedMarkedClassLevelInner() { .addSourceLines( "Bar.java", "package com.uber.foo;", + "", "import org.jspecify.annotations.NullUnmarked;", "import org.jspecify.annotations.Nullable;", + "", "public class Bar {", " @NullUnmarked", " public static class Foo {", @@ -751,9 +866,11 @@ public void nullUnmarkedMarkedClassLevelInner() { " public static String nullRet() {", " return null;", " }", + "", " public static String takeNonNull(Object o) {", " return o.toString();", " }", + "", " public static String takeNullable(@Nullable Object o) {", " // No errors, because Foo is @NullUnmarked", " return o.toString();", @@ -768,13 +885,16 @@ public void nullUnmarkedMarkedClassLevelInner() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.uber.foo.Bar;", + "", "public class Test {", " public static Object test(Object o) {", " // No errors, because Foo is @NullUnmarked", " Bar.Foo.takeNonNull(null);", " return Bar.Foo.nullRet();", " }", + "", " public static String sanity() {", " // BUG: Diagnostic contains: returning @Nullable expression", " return null;", @@ -789,9 +909,11 @@ public void nullUnmarkedClassLevelDeep() { .addSourceLines( "Bar.java", "package com.uber.foo;", + "", "import org.jspecify.annotations.NullMarked;", "import org.jspecify.annotations.NullUnmarked;", "import org.jspecify.annotations.Nullable;", + "", "public class Bar {", " @NullUnmarked", " public static class Foo {", @@ -801,9 +923,11 @@ public void nullUnmarkedClassLevelDeep() { " public static String nullRet() {", " return null;", " }", + "", " public static String takeNonNull(Object o) {", " return o.toString();", " }", + "", " public static String takeNullable(@Nullable Object o) {", " // BUG: Diagnostic contains: dereferenced expression o is @Nullable", " return o.toString();", @@ -824,12 +948,15 @@ public void nullUnmarkedClassLevelDeep() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.uber.foo.Bar;", + "", "public class Test {", " public static void test(Object o) {", " // BUG: Diagnostic contains: passing @Nullable parameter 'Bar.Foo.Deep.nullRet()'", " Bar.Foo.Deep.takeNonNull(Bar.Foo.Deep.nullRet());", " }", + "", " public static String sanity() {", " // BUG: Diagnostic contains: returning @Nullable expression", " return null;", @@ -844,8 +971,10 @@ public void nullUnmarkedMethodLevel() { .addSourceLines( "Foo.java", "package com.uber;", + "", "import org.jspecify.annotations.NullUnmarked;", "import org.jspecify.annotations.Nullable;", + "", "public class Foo {", " @NullUnmarked", " @Nullable", @@ -853,10 +982,12 @@ public void nullUnmarkedMethodLevel() { " // No error, since this code is unannotated", " return o.toString();", " }", + "", " public static String caller() {", " // No error, since callee is unannotated", " return callee(null);", " }", + "", " public static String sanity() {", " // BUG: Diagnostic contains: returning @Nullable expression", " return null;", @@ -871,13 +1002,16 @@ public void nullUnmarkedOuterMethodLevelWithLocalClass() { .addSourceLines( "Bar.java", "package com.example.thirdparty;", + "", "import org.jspecify.annotations.NullMarked;", "import org.jspecify.annotations.NullUnmarked;", + "", "@NullMarked", "public class Bar {", " public static String takeNonNull(Object o) {", " return o.toString();", " }", + "", " @NullUnmarked", " public static Object bar1() {", " class Baz {", @@ -890,6 +1024,7 @@ public void nullUnmarkedOuterMethodLevelWithLocalClass() { " b.baz();", " return b;", " }", + "", " @NullUnmarked", " public static Object bar2() {", " @NullMarked", @@ -903,6 +1038,7 @@ public void nullUnmarkedOuterMethodLevelWithLocalClass() { " b.baz();", " return b;", " }", + "", " @NullUnmarked", " public static Object bar3() {", " class Baz {", @@ -916,6 +1052,7 @@ public void nullUnmarkedOuterMethodLevelWithLocalClass() { " b.baz();", " return b;", " }", + "", " public static String sanity() {", " // BUG: Diagnostic contains: returning @Nullable expression", " return null;", @@ -930,7 +1067,9 @@ public void bytecodeNullUnmarkedMethodLevel() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.example.jspecify.unannotatedpackage.Methods;", + "", "public class Test {", " public static void test(Object o) {", " // BUG: Diagnostic contains: passing @Nullable parameter", @@ -953,30 +1092,36 @@ public void nullUnmarkedAndAcknowledgeRestrictiveAnnotations() { .addSourceLines( "Foo.java", "package com.uber;", + "", "import org.jspecify.annotations.NullMarked;", "import org.jspecify.annotations.NullUnmarked;", "import org.jspecify.annotations.Nullable;", + "", "@NullUnmarked", "public class Foo {", " // No initialization warning, Foo is unmarked", " @Nullable public Object f;", + "", " @Nullable", " public String callee(@Nullable Object o) {", " // No error, since this code is unannotated", " return o.toString() + f.toString();", " }", + "", " @NullMarked", " public String caller() {", " // Error, since callee still has restrictive annotations!", " // BUG: Diagnostic contains: returning @Nullable expression from method", " return callee(null);", " }", + "", " @NullMarked", " public Object getF() {", " // Error, since callee still has restrictive annotations!", " // BUG: Diagnostic contains: returning @Nullable expression from method", " return f;", " }", + "", " @NullMarked", " public String derefUnmarkedField() {", " // Error, since callee still has restrictive annotations!", @@ -999,23 +1144,29 @@ public void nullMarkedStaticImports() { .addSourceLines( "StaticMethods.java", "package com.uber;", + "", "import org.jspecify.annotations.NullMarked;", "import org.jspecify.annotations.Nullable;", + "", "public final class StaticMethods {", " private StaticMethods() {}", + "", " @NullMarked", " public static Object nonNullCallee(Object o) {", " return o;", " }", + "", " @NullMarked", " @Nullable", " public static Object nullableCallee(@Nullable Object o) {", " return o;", " }", + "", " public static Object unmarkedCallee(@Nullable Object o) {", " // no error, because unmarked", " return o;", " }", + "", " @Nullable", " public static Object unmarkedNullableCallee(@Nullable Object o) {", " return o;", @@ -1024,17 +1175,20 @@ public void nullMarkedStaticImports() { .addSourceLines( "Test.java", "package com.uber;", + "", "import static com.uber.StaticMethods.nonNullCallee;", "import static com.uber.StaticMethods.nullableCallee;", "import static com.uber.StaticMethods.unmarkedCallee;", "import static com.uber.StaticMethods.unmarkedNullableCallee;", + "", "import org.jspecify.annotations.NullMarked;", - "import org.jspecify.annotations.Nullable;", + "", "@NullMarked", "public class Test {", " public Object getNewObject() {", " return new Object();", " }", + "", " public void test() {", " Object o = getNewObject();", " nonNullCallee(o).toString();", @@ -1062,13 +1216,14 @@ public void dotClassSanityTest1() { .addSourceLines( "Test.java", "package com.uber;", - "import org.jspecify.annotations.NullMarked;", - "import org.jspecify.annotations.Nullable;", + "", "import java.lang.reflect.Field;", + "import org.jspecify.annotations.NullMarked;", + "", "@NullMarked", "public class Test {", - " public void takesClass(Class c) {", - " }", + " public void takesClass(Class c) {}", + "", " public Object test(boolean flag) {", " takesClass(Test.class);", " takesClass(String.class);", @@ -1079,6 +1234,7 @@ public void dotClassSanityTest1() { " // NEEDED TO TRIGGER DATAFLOW:", " return flag ? Test.class : new Object();", " }", + "", " public boolean test2(Field field) {", " if (field.getType() == int.class || field.getType() == Integer.class) {", " return true;", @@ -1104,13 +1260,14 @@ public void dotClassSanityTest2() { .addSourceLines( "Test.java", "package com.uber;", - "import org.jspecify.annotations.NullMarked;", - "import org.jspecify.annotations.Nullable;", + "", "import java.lang.reflect.Field;", + "import org.jspecify.annotations.NullMarked;", + "", "@NullMarked", "public class Test {", - " public void takesClass(Class c) {", - " }", + " public void takesClass(Class c) {}", + "", " public Object test(boolean flag) {", " takesClass(Test.class);", " takesClass(String.class);", @@ -1121,6 +1278,7 @@ public void dotClassSanityTest2() { " // NEEDED TO TRIGGER DATAFLOW:", " return flag ? Test.class : new Object();", " }", + "", " public boolean test2(Field field) {", " if (field.getType() == int.class || field.getType() == Integer.class) {", " return true;", diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayKeySetIteratorTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayKeySetIteratorTests.java index 5b39012149..3d16738570 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayKeySetIteratorTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayKeySetIteratorTests.java @@ -10,10 +10,12 @@ public void mapKeySetIteratorBasic() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.Map;", + "", "public class Test {", " public void keySetStuff(Map m) {", - " for (Object k: m.keySet()) {", + " for (Object k : m.keySet()) {", " m.get(k).toString();", " }", " }", @@ -27,11 +29,14 @@ public void mapKeySetIteratorShadowing() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.Map;", + "", "public class Test {", " private final Object k = new Object();", + "", " public void keySetStuff(Map m) {", - " for (Object k: m.keySet()) {", + " for (Object k : m.keySet()) {", " m.get(k).toString();", " }", " // BUG: Diagnostic contains: dereferenced expression m.get(k) is @Nullable", @@ -47,25 +52,34 @@ public void mapKeySetIteratorDeeperAccessPath() { .addSourceLines( "Test.java", "package com.uber;", - "import java.util.Map;", + "", "import java.util.HashMap;", + "import java.util.Map;", + "", "public class Test {", " static class MapWrapper {", - " Map mf = new HashMap<>();", - " public Map getMF() { return mf; }", - " public Map getMF2(int i) { return mf; }", + " Map mf = new HashMap<>();", + "", + " public Map getMF() {", + " return mf;", + " }", + "", + " public Map getMF2(int i) {", + " return mf;", + " }", " }", + "", " public void keySetStuff(MapWrapper mw, int j) {", - " for (Object k: mw.mf.keySet()) {", + " for (Object k : mw.mf.keySet()) {", " mw.mf.get(k).toString();", " }", - " for (Object k: mw.getMF().keySet()) {", + " for (Object k : mw.getMF().keySet()) {", " mw.getMF().get(k).toString();", " }", - " for (Object k: mw.getMF2(10).keySet()) {", + " for (Object k : mw.getMF2(10).keySet()) {", " mw.getMF2(10).get(k).toString();", " }", - " for (Object k: mw.getMF2(j).keySet()) {", + " for (Object k : mw.getMF2(j).keySet()) {", " // Report error since we cannot represent mw.getMF2(j).get(k) with an access path", " // BUG: Diagnostic contains: dereferenced expression mw.getMF2(j).get(k) is @Nullable", " mw.getMF2(j).get(k).toString();", @@ -81,8 +95,10 @@ public void unhandledCases() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.Iterator;", "import java.util.Map;", + "", "public class Test {", " public void keySetStuff(Map m) {", " // BUG: Diagnostic contains: dereferenced expression", @@ -103,11 +119,13 @@ public void nestedLoops() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.Map;", + "", "public class Test {", " public void keySetStuff(Map m, Map m2) {", - " for (Object k: m.keySet()) {", - " for (Object k2: m2.keySet()) {", + " for (Object k : m.keySet()) {", + " for (Object k2 : m2.keySet()) {", " m.get(k).toString();", " // BUG: Diagnostic contains: dereferenced expression", " m.get(k2).toString();", @@ -117,8 +135,8 @@ public void nestedLoops() { " }", " }", " // nested loop over the same map", - " for (Object k: m.keySet()) {", - " for (Object k2: m.keySet()) {", + " for (Object k : m.keySet()) {", + " for (Object k2 : m.keySet()) {", " m.get(k).toString();", " m.get(k2).toString();", " // BUG: Diagnostic contains: dereferenced expression", @@ -138,22 +156,26 @@ public void declaredTypeSubtypeOfMap() { .addSourceLines( "Test.java", "package com.uber;", + "", "import com.google.common.collect.ImmutableMap;", - "import java.util.TreeMap;", "import java.util.LinkedHashMap;", + "import java.util.TreeMap;", + "", "public class Test {", " public void keySetStuff1(ImmutableMap m) {", - " for (Object k: m.keySet()) {", + " for (Object k : m.keySet()) {", " m.get(k).toString();", " }", " }", + "", " public void keySetStuff2(TreeMap m) {", - " for (Object k: m.keySet()) {", + " for (Object k : m.keySet()) {", " m.get(k).toString();", " }", " }", + "", " public void keySetStuff3(LinkedHashMap m) {", - " for (Object k: m.keySet()) {", + " for (Object k : m.keySet()) {", " m.get(k).toString();", " }", " }", diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayOptionalEmptinessTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayOptionalEmptinessTests.java index a0cb723a17..53348ab456 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayOptionalEmptinessTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayOptionalEmptinessTests.java @@ -17,65 +17,74 @@ public void optionalEmptinessHandlerTest() { .addSourceLines( "TestNegative.java", "package com.uber;", - "import java.util.Optional;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Function;", + "import java.util.Optional;", + "", "public class TestNegative {", " void foo() {", " Optional a = Optional.empty();", - " // no error since a.isPresent() is called", - " if(a.isPresent()){", - " a.get().toString();", - " }", + " // no error since a.isPresent() is called", + " if (a.isPresent()) {", + " a.get().toString();", " }", - " public void lambdaConsumer(Function a){", - " return;", - " }", + " }", + "", + " public void lambdaConsumer(Function a) {", + " return;", + " }", + "", " void bar() {", - " Optional b = Optional.empty();", - " if(b.isPresent()){", - " lambdaConsumer(v -> b.get().toString());", - " }", + " Optional b = Optional.empty();", + " if (b.isPresent()) {", + " lambdaConsumer(v -> b.get().toString());", " }", + " }", + "", " @SuppressWarnings(\"NullAway.Optional\")", " void SupWarn() {", " Optional a = Optional.empty();", - " // no error since suppressed", - " a.get().toString();", - " }", + " // no error since suppressed", + " a.get().toString();", + " }", + "", " void SupWarn2() {", " Optional a = Optional.empty();", - " // no error since suppressed", - " @SuppressWarnings(\"NullAway.Optional\") String b = a.get().toString();", - " }", + " // no error since suppressed", + " @SuppressWarnings(\"NullAway.Optional\")", + " String b = a.get().toString();", + " }", "}") .addSourceLines( "TestPositive.java", "package com.uber;", - "import java.util.Optional;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Function;", + "import java.util.Optional;", + "", "public class TestPositive {", " void foo() {", " Optional a = Optional.empty();", " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional a", " a.get().toString();", " }", - " public void lambdaConsumer(Function a){", - " return;", - " }", + "", + " public void lambdaConsumer(Function a) {", + " return;", + " }", + "", " void bar() {", - " Optional b = Optional.empty();", + " Optional b = Optional.empty();", " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional b", - " lambdaConsumer(v -> b.get().toString());", - " }", - " // This tests if the suppression is not suppressing unrelated errors ", + " lambdaConsumer(v -> b.get().toString());", + " }", + " // This tests if the suppression is not suppressing unrelated errors", " @SuppressWarnings(\"NullAway.Optional\")", " void SupWarn() {", " Object a = null;", - " // BUG: Diagnostic contains: dereferenced expression a is @Nullable", - " a.toString();", - " }", + " // BUG: Diagnostic contains: dereferenced expression a is @Nullable", + " a.toString();", + " }", "}") .doTest(); } @@ -93,70 +102,85 @@ public void optionalEmptinessHandlerWithSingleCustomPathTest() { .addSourceLines( "TestNegative.java", "package com.uber;", - "import com.google.common.base.Optional;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Function;", + "import com.google.common.base.Optional;", + "", "public class TestNegative {", - " class ABC { Optional ob = Optional.absent();} ", + " class ABC {", + " Optional ob = Optional.absent();", + " }", + "", " void foo() {", - " ABC abc = new ABC();", - " // no error since a.isPresent() is called", - " if(abc.ob.isPresent()){", - " abc.ob.get().toString();", - " }", + " ABC abc = new ABC();", + " // no error since a.isPresent() is called", + " if (abc.ob.isPresent()) {", + " abc.ob.get().toString();", " }", - " public void lambdaConsumer(Function a){", - " return;", - " }", + " }", + "", + " public void lambdaConsumer(Function a) {", + " return;", + " }", + "", " void bar() {", - " Optional b = Optional.absent();", - " if(b.isPresent()){", - " lambdaConsumer(v -> b.get().toString());", - " }", + " Optional b = Optional.absent();", + " if (b.isPresent()) {", + " lambdaConsumer(v -> b.get().toString());", " }", + " }", "}") .addSourceLines( "TestPositive.java", "package com.uber;", - "import java.util.Optional;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Function;", + "import java.util.Optional;", + "", "public class TestPositive {", - " class ABC { Optional ob = Optional.empty();} ", + " class ABC {", + " Optional ob = Optional.empty();", + " }", + "", " void foo() {", " ABC abc = new ABC();", " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional abc.ob", " abc.ob.get().toString();", " }", - " public void lambdaConsumer(Function a){", - " return;", - " }", + "", + " public void lambdaConsumer(Function a) {", + " return;", + " }", + "", " void bar() {", - " Optional b = Optional.empty();", + " Optional b = Optional.empty();", " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional b", - " lambdaConsumer(v -> b.get().toString());", - " }", + " lambdaConsumer(v -> b.get().toString());", + " }", "}") .addSourceLines( "TestPositive2.java", "package com.uber;", - "import com.google.common.base.Optional;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Function;", + "import com.google.common.base.Optional;", + "", "public class TestPositive2 {", " void foo() {", " Optional a = Optional.absent();", " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional a", " a.get().toString();", " }", - " public void lambdaConsumer(Function a){", - " return;", - " }", + "", + " public void lambdaConsumer(Function a) {", + " return;", + " }", + "", " void bar() {", - " Optional b = Optional.absent();", + " Optional b = Optional.absent();", " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional b", - " lambdaConsumer(v -> b.get().toString());", - " }", + " lambdaConsumer(v -> b.get().toString());", + " }", "}") .doTest(); } @@ -174,68 +198,77 @@ public void optionalEmptinessHandlerWithTwoCustomPathsTest() { .addSourceLines( "TestNegative.java", "package com.uber;", - "import com.google.common.base.Optional;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Function;", + "import com.google.common.base.Optional;", + "", "public class TestNegative {", " void foo() {", " Optional a = Optional.absent();", - " // no error since a.isPresent() is called", - " if(a.isPresent()){", - " a.get().toString();", - " }", + " // no error since a.isPresent() is called", + " if (a.isPresent()) {", + " a.get().toString();", " }", - " public void lambdaConsumer(Function a){", - " return;", - " }", + " }", + "", + " public void lambdaConsumer(Function a) {", + " return;", + " }", + "", " void bar() {", - " Optional b = Optional.absent();", - " if(b.isPresent()){", - " lambdaConsumer(v -> b.get().toString());", - " }", + " Optional b = Optional.absent();", + " if (b.isPresent()) {", + " lambdaConsumer(v -> b.get().toString());", " }", + " }", "}") .addSourceLines( "TestPositive.java", "package com.uber;", - "import java.util.Optional;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Function;", + "import java.util.Optional;", + "", "public class TestPositive {", " void foo() {", " Optional a = Optional.empty();", " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional a", " a.get().toString();", " }", - " public void lambdaConsumer(Function a){", - " return;", - " }", + "", + " public void lambdaConsumer(Function a) {", + " return;", + " }", + "", " void bar() {", - " Optional b = Optional.empty();", + " Optional b = Optional.empty();", " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional b", - " lambdaConsumer(v -> b.get().toString());", - " }", + " lambdaConsumer(v -> b.get().toString());", + " }", "}") .addSourceLines( "TestPositive2.java", "package com.uber;", - "import com.google.common.base.Optional;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Function;", + "import com.google.common.base.Optional;", + "", "public class TestPositive2 {", " void foo() {", " Optional a = Optional.absent();", " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional a", " a.get().toString();", " }", - " public void lambdaConsumer(Function a){", - " return;", - " }", + "", + " public void lambdaConsumer(Function a) {", + " return;", + " }", + "", " void bar() {", - " Optional b = Optional.absent();", + " Optional b = Optional.absent();", " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional b", - " lambdaConsumer(v -> b.get().toString());", - " }", + " lambdaConsumer(v -> b.get().toString());", + " }", "}") .doTest(); } @@ -251,25 +284,28 @@ public void optionalEmptinessUncheckedTest() { .addSourceLines( "TestNegative.java", "package com.uber;", - "import java.util.Optional;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Function;", + "import java.util.Optional;", + "", "public class TestNegative {", " void foo() {", " Optional a = Optional.empty();", " // no error since the handler is not attached", " a.get().toString();", " }", - " public void lambdaConsumer(Function a){", - " return;", - " }", + "", + " public void lambdaConsumer(Function a) {", + " return;", + " }", + "", " void bar() {", - " Optional b = Optional.empty();", - " if(b.isPresent()){", - " // no error since the handler is not attached", - " lambdaConsumer(v -> b.get().toString());", - " }", + " Optional b = Optional.empty();", + " if (b.isPresent()) {", + " // no error since the handler is not attached", + " lambdaConsumer(v -> b.get().toString());", " }", + " }", "}") .doTest(); } @@ -286,21 +322,26 @@ public void optionalEmptinessRxPositiveTest() { .addSourceLines( "TestPositive.java", "package com.uber;", - "import java.util.Optional;", + "", "import io.reactivex.Observable;", + "import java.util.Optional;", + "", "public class TestPositive {", - " private static boolean perhaps() { return Math.random() > 0.5; }", + " private static boolean perhaps() {", + " return Math.random() > 0.5;", + " }", + "", " void foo(Observable> observable) {", - " observable", - " .filter(optional -> optional.isPresent() || perhaps())", - " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional optional", - " .map(optional -> optional.get().toString());", - " observable", - " .filter(optional -> optional.isPresent() || perhaps())", - " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional optional", - " .map(optional -> optional.get())", - " .map(irr -> irr.toString());", - " }", + " observable", + " .filter(optional -> optional.isPresent() || perhaps())", + " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional optional", + " .map(optional -> optional.get().toString());", + " observable", + " .filter(optional -> optional.isPresent() || perhaps())", + " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional optional", + " .map(optional -> optional.get())", + " .map(irr -> irr.toString());", + " }", "}") .doTest(); } @@ -317,21 +358,24 @@ public void optionalEmptinessRxNegativeTest() { .addSourceLines( "TestNegative.java", "package com.uber;", - "import java.util.Optional;", + "", "import io.reactivex.Observable;", + "import java.util.Optional;", + "", "public class TestNegative {", - " private static boolean perhaps() { return Math.random() > 0.5; }", + " private static boolean perhaps() {", + " return Math.random() > 0.5;", + " }", + "", " void foo(Observable> observable) {", - " observable", - " .filter(optional -> optional.isPresent())", - " .map(optional -> optional.get().toString());", - " observable", - " .filter(optional -> optional.isPresent() && perhaps())", - " .map(optional -> optional.get().toString());", - " observable", - " .filter(optional -> optional.isPresent() && perhaps())", - " .map(optional -> optional.get());", - " }", + " observable.filter(optional -> optional.isPresent()).map(optional -> optional.get().toString());", + " observable", + " .filter(optional -> optional.isPresent() && perhaps())", + " .map(optional -> optional.get().toString());", + " observable", + " .filter(optional -> optional.isPresent() && perhaps())", + " .map(optional -> optional.get());", + " }", "}") .doTest(); } @@ -349,22 +393,26 @@ public void optionalEmptinessHandleAssertionLibraryTest() { .addSourceLines( "Test.java", "package com.uber;", - "import java.util.Optional;", - "import javax.annotation.Nullable;", - "import com.google.common.base.Function;", + "", "import static com.google.common.truth.Truth.assertThat;", + "", + "import com.google.common.base.Function;", + "import java.util.Optional;", + "", "public class Test {", " void foo() {", " Optional a = Optional.empty();", - " assertThat(a.isPresent()).isTrue(); ", + " assertThat(a.isPresent()).isTrue();", " a.get().toString();", " }", - " public void lambdaConsumer(Function a){", + "", + " public void lambdaConsumer(Function a) {", " return;", " }", + "", " void bar() {", " Optional b = Optional.empty();", - " assertThat(b.isPresent()).isTrue(); ", + " assertThat(b.isPresent()).isTrue();", " lambdaConsumer(v -> b.get().toString());", " }", "}") @@ -383,24 +431,27 @@ public void optionalEmptinessAssignmentCheckNegativeTest() { .addSourceLines( "TestNegative.java", "package com.uber;", - "import java.util.Optional;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Function;", + "import java.util.Optional;", + "", "public class TestNegative {", " void foo() {", " Optional a = Optional.empty();", " Object x = a.isPresent() ? a.get() : \"something\";", " x.toString();", " }", - " public void lambdaConsumer(Function a){", - " return;", - " }", + "", + " public void lambdaConsumer(Function a) {", + " return;", + " }", + "", " void bar() {", - " Optional b = Optional.empty();", - " if(b.isPresent()){", - " lambdaConsumer(v -> b.get().toString());", - " }", + " Optional b = Optional.empty();", + " if (b.isPresent()) {", + " lambdaConsumer(v -> b.get().toString());", " }", + " }", "}") .doTest(); } @@ -417,9 +468,10 @@ public void optionalEmptinessAssignmentCheckPositiveTest() { .addSourceLines( "TestPositive.java", "package com.uber;", - "import java.util.Optional;", - "import javax.annotation.Nullable;", + "", "import com.google.common.base.Function;", + "import java.util.Optional;", + "", "public class TestPositive {", " void foo() {", " Optional a = Optional.empty();", @@ -427,14 +479,20 @@ public void optionalEmptinessAssignmentCheckPositiveTest() { " Object x = a.get();", " x.toString();", " }", - " public void lambdaConsumer(Function a){", - " return;", - " }", + "", + " public void lambdaConsumer(Function a) {", + " return;", + " }", + "", " void bar() {", - " Optional b = Optional.empty();", + " Optional b = Optional.empty();", " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional b", - " lambdaConsumer(v -> {Object x = b.get(); return \"irrelevant\";});", - " }", + " lambdaConsumer(", + " v -> {", + " Object x = b.get();", + " return \"irrelevant\";", + " });", + " }", "}") .doTest(); } @@ -451,31 +509,38 @@ public void optionalEmptinessContextualSuppressionTest() { .addSourceLines( "TestClassSuppression.java", "package com.uber;", + "", + "import com.google.common.base.Function;", "import java.util.Optional;", "import javax.annotation.Nullable;", - "import com.google.common.base.Function;", + "", "@SuppressWarnings(\"NullAway.Optional\")", "public class TestClassSuppression {", " // no error since suppressed", " Function lambdaField = opt -> opt.get().toString();", + "", " void foo() {", " Optional a = Optional.empty();", " // no error since suppressed", " a.get().toString();", " }", - " public void lambdaConsumer(Function a){", + "", + " public void lambdaConsumer(Function a) {", " return;", " }", + "", " void bar() {", " Optional b = Optional.empty();", " // no error since suppressed", " lambdaConsumer(v -> b.get().toString());", " }", + "", " void baz(@Nullable Object o) {", " // unrelated errors not suppressed", " // BUG: Diagnostic contains: dereferenced expression o is @Nullable", " o.toString();", " }", + "", " public static class Inner {", " void foo() {", " Optional a = Optional.empty();", @@ -487,8 +552,10 @@ public void optionalEmptinessContextualSuppressionTest() { .addSourceLines( "TestLambdaFieldNoSuppression.java", "package com.uber;", - "import java.util.Optional;", + "", "import com.google.common.base.Function;", + "import java.util.Optional;", + "", "public class TestLambdaFieldNoSuppression {", " // BUG: Diagnostic contains: Invoking get() on possibly empty Optional opt", " Function lambdaField = opt -> opt.get().toString();", @@ -496,8 +563,10 @@ public void optionalEmptinessContextualSuppressionTest() { .addSourceLines( "TestLambdaFieldWithSuppression.java", "package com.uber;", - "import java.util.Optional;", + "", "import com.google.common.base.Function;", + "import java.util.Optional;", + "", "public class TestLambdaFieldWithSuppression {", " // no error since suppressed", " @SuppressWarnings(\"NullAway.Optional\")", @@ -517,8 +586,10 @@ public void optionalOfMapResultTest() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.Map;", "import java.util.Optional;", + "", "class Test {", " private Optional f(Map map1, Map map2) {", " Optional opt = Optional.ofNullable(map1.get(\"key\"));", @@ -526,9 +597,9 @@ public void optionalOfMapResultTest() { " return Optional.empty();", " }", " return map2.entrySet().stream()", - " .filter(entry -> entry.getValue().equals(opt.get()))", - " .map(Map.Entry::getKey)", - " .findFirst();", + " .filter(entry -> entry.getValue().equals(opt.get()))", + " .map(Map.Entry::getKey)", + " .findFirst();", " }", "}") .doTest(); diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayThriftTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayThriftTests.java index 9753a5586b..58e5027b82 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayThriftTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayThriftTests.java @@ -7,24 +7,42 @@ public class NullAwayThriftTests extends NullAwayTestsBase { @Test public void testThriftIsSet() { defaultCompilationHelper - .addSourceLines("TBase.java", "package org.apache.thrift;", "public interface TBase {}") + .addSourceLines("TBase.java", "package org.apache.thrift;", "", "public interface TBase {}") .addSourceLines( "Generated.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public class Generated implements org.apache.thrift.TBase {", " public @Nullable Object id;", " public boolean isFixed;", - " @Nullable public Object getId() { return this.id; }", + "", + " @Nullable", + " public Object getId() {", + " return this.id;", + " }", " // this is to ensure we don't crash on unions", - " public boolean isSet() { return false; }", - " public boolean isSetId() { return this.id != null; }", - " public boolean isFixed() { return this.isFixed; }", - " public boolean isSetIsFixed() { return false; }", + " public boolean isSet() {", + " return false;", + " }", + "", + " public boolean isSetId() {", + " return this.id != null;", + " }", + "", + " public boolean isFixed() {", + " return this.isFixed;", + " }", + "", + " public boolean isSetIsFixed() {", + " return false;", + " }", "}") .addSourceLines( "Client.java", "package com.uber;", + "", "public class Client {", " public void testNeg(Generated g) {", " if (g.isSetId()) {", @@ -36,6 +54,7 @@ public void testThriftIsSet() { " }", " if (g.isSet()) {}", " }", + "", " public void testPos(Generated g) {", " if (!g.isSetId()) {", " // BUG: Diagnostic contains: dereferenced expression g.getId() is @Nullable", @@ -56,19 +75,29 @@ public void testThriftIsSet() { public void testThriftIsSetWithGenerics() { defaultCompilationHelper .addSourceLines( - "TBase.java", "package org.apache.thrift;", "public interface TBase {}") + "TBase.java", "package org.apache.thrift;", "", "public interface TBase {}") .addSourceLines( "Generated.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public class Generated implements org.apache.thrift.TBase {", " public @Nullable Object id;", - " @Nullable public Object getId() { return this.id; }", - " public boolean isSetId() { return this.id != null; }", + "", + " @Nullable", + " public Object getId() {", + " return this.id;", + " }", + "", + " public boolean isSetId() {", + " return this.id != null;", + " }", "}") .addSourceLines( "Client.java", "package com.uber;", + "", "public class Client {", " public void testNeg(Generated g) {", " if (!g.isSetId()) {", @@ -89,12 +118,14 @@ public void testThriftIsSetWithArg() { .addSourceLines( "TBase.java", "package org.apache.thrift;", + "", "public interface TBase {", " boolean isSet(String fieldName);", "}") .addSourceLines( "Client.java", "package com.uber;", + "", "public class Client {", " public void testNeg(org.apache.thrift.TBase tBase) {", " if (tBase.isSet(\"Hello\")) {", @@ -110,12 +141,14 @@ public void testThriftIsSetWithArg() { public void testThriftUnion() { defaultCompilationHelper .addSourceLines( - "TBase.java", "package org.apache.thrift;", "public interface TBase {}") + "TBase.java", "package org.apache.thrift;", "", "public interface TBase {}") .addSourceLines( "TUnion.java", "package org.apache.thrift;", + "", "public abstract class TUnion implements TBase {", " protected Object value_;", + "", " public Object getFieldValue() {", " return this.value_;", " }", @@ -123,14 +156,20 @@ public void testThriftUnion() { .addSourceLines( "Generated.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "public class Generated extends org.apache.thrift.TUnion {", - " public Object getId() { return getFieldValue(); }", - " public boolean isSetId() { return true; }", + " public Object getId() {", + " return getFieldValue();", + " }", + "", + " public boolean isSetId() {", + " return true;", + " }", "}") .addSourceLines( "Client.java", "package com.uber;", + "", "public class Client {", " public void testNeg(Generated g) {", " if (!g.isSetId()) {", @@ -157,32 +196,48 @@ public void testThriftAndCastToNonNull() { "-XepOpt:NullAway:TreatGeneratedAsUnannotated=true", "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true")) .addSourceFile("Util.java") - .addSourceLines("TBase.java", "package org.apache.thrift;", "public interface TBase {}") + .addSourceLines("TBase.java", "package org.apache.thrift;", "", "public interface TBase {}") .addSourceLines( "GeneratedClass.java", "package com.uber.lib.unannotated;", - "import javax.annotation.Nullable;", + "", "import javax.annotation.Generated;", + "import javax.annotation.Nullable;", + "", "@Generated(\"test\")", "public class GeneratedClass implements org.apache.thrift.TBase {", " public @Nullable Object id;", - " @Nullable public Object getId() { return this.id; }", + "", + " @Nullable", + " public Object getId() {", + " return this.id;", + " }", " // this is to ensure we don't crash on unions", - " public boolean isSet() { return false; }", - " public boolean isSetId() { return this.id != null; }", + " public boolean isSet() {", + " return false;", + " }", + "", + " public boolean isSetId() {", + " return this.id != null;", + " }", "}") .addSourceLines( "Client.java", "package com.uber;", + "", "import static com.uber.nullaway.testdata.Util.castToNonNull;", + "", "import com.uber.lib.unannotated.GeneratedClass;", + "", "public class Client {", " public void testPos(GeneratedClass g) {", - " // g.getId() is @NonNull because it's treated as unannotated code and RestrictiveAnnotationHandler exempts it", + " // g.getId() is @NonNull because it's treated as unannotated code and", + " // RestrictiveAnnotationHandler exempts it", " // BUG: Diagnostic contains: passing known @NonNull parameter 'g.getId()' to CastToNonNullMethod", " Object o = castToNonNull(g.getId());", " o.toString();", " }", + "", " public void testNeg(GeneratedClass g) {", " Object o = g.getId();", " o.toString();", diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationTests.java index 06e13fe46e..37840f943c 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationTests.java @@ -9,25 +9,35 @@ public void typeParameterAnnotationIsDistinctFromMethodReturnAnnotation() { .addSourceLines( "Test.java", "package com.uber;", - "import org.jspecify.annotations.Nullable;", + "", "import java.util.function.Supplier;", + "import org.jspecify.annotations.Nullable;", + "", "public class Test {", " public static @Nullable Supplier<@Nullable R> getNullableSupplierOfNullable() {", " return new Supplier() {", " @Nullable", - " public R get() { return null; }", + " public R get() {", + " return null;", + " }", " };", " }", + "", " public static Supplier<@Nullable R> getNonNullSupplierOfNullable() {", " return new Supplier() {", " @Nullable", - " public R get() { return null; }", + " public R get() {", + " return null;", + " }", " };", " }", + "", " public static String test1() {", - " // BUG: Diagnostic contains: dereferenced expression getNullableSupplierOfNullable() is @Nullable", + " // BUG: Diagnostic contains: dereferenced expression getNullableSupplierOfNullable() is", + " // @Nullable", " return getNullableSupplierOfNullable().toString();", " }", + "", " public static String test2() {", " // The supplier contains null, but isn't itself nullable. Check against a past FP", " return getNonNullSupplierOfNullable().toString();", diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationsTests.java index 7af8b253d4..25502a3928 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationsTests.java @@ -35,22 +35,28 @@ public void annotationAppliedToTypeParameter() { .addSourceLines( "Test.java", "package com.uber;", - "import java.util.List;", + "", "import java.util.ArrayList;", + "import java.util.List;", "import org.checkerframework.checker.nullness.qual.Nullable;", + "", "class TypeArgumentAnnotation {", " List<@Nullable String> fSafe = new ArrayList<>();", " @Nullable List fUnsafe = new ArrayList<>();", + "", " void useParamSafe(List<@Nullable String> list) {", " list.hashCode();", " }", + "", " void useParamUnsafe(@Nullable List list) {", " // BUG: Diagnostic contains: dereferenced", " list.hashCode();", " }", + "", " void useFieldSafe() {", " fSafe.hashCode();", " }", + "", " void useFieldUnsafe() {", " // BUG: Diagnostic contains: dereferenced", " fUnsafe.hashCode();", @@ -65,10 +71,14 @@ public void annotationAppliedToInnerTypeImplicitly() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.checkerframework.checker.nullness.qual.Nullable;", + "", "class Test {", - " @Nullable Foo f;", // i.e. Test.@Nullable Foo - " class Foo { }", + " @Nullable Foo f;", + "", + " class Foo {}", + "", " public void test() {", " // BUG: Diagnostic contains: dereferenced", " f.hashCode();", @@ -83,14 +93,17 @@ public void annotationAppliedToInnerTypeImplicitlyWithTypeArgs() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.checkerframework.checker.nullness.qual.Nullable;", + "", "class Test {", - " @Nullable Foo f1 = null;", // i.e. Test.@Nullable Foo (location [INNER]) + " @Nullable Foo f1 = null;", " // BUG: Diagnostic contains: assigning @Nullable expression to @NonNull field", - " Foo<@Nullable String> f2 = null;", // (location [INNER, TYPE_ARG(0)]) - " @Nullable Foo<@Nullable String> f3 = null;", // two annotations, each with the - // locations above - " class Foo { }", + " Foo<@Nullable String> f2 = null;", + " @Nullable Foo<@Nullable String> f3 = null;", + "", + " class Foo {}", + "", " public void test() {", " // BUG: Diagnostic contains: dereferenced", " f1.hashCode();", @@ -109,11 +122,15 @@ public void annotationAppliedToInnerTypeExplicitly() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.checkerframework.checker.nullness.qual.Nullable;", + "", "class Test {", " Test.@Nullable Foo f1;", " @Nullable Test.Foo f2;", - " class Foo { }", + "", + " class Foo {}", + "", " public void test() {", " // BUG: Diagnostic contains: dereferenced", " f1.hashCode();", @@ -128,19 +145,17 @@ public void annotationAppliedToInnerTypeExplicitly() { public void annotationAppliedToInnerTypeExplicitly2() { defaultCompilationHelper .addSourceLines( - "Bar.java", - "package com.uber;", - "import org.checkerframework.checker.nullness.qual.Nullable;", - "class Bar {", - " public class Foo { }", - "}") + "Bar.java", "package com.uber;", "", "class Bar {", " public class Foo {}", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "import org.checkerframework.checker.nullness.qual.Nullable;", + "", "class Test {", " Bar.@Nullable Foo f1;", " @Nullable Bar.Foo f2;", + "", " public void test() {", " // BUG: Diagnostic contains: dereferenced", " f1.hashCode();", @@ -157,12 +172,16 @@ public void annotationAppliedToInnerTypeOfTypeArgument() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.Set;", "import org.checkerframework.checker.nullness.qual.Nullable;", + "", "class Test {", " // BUG: Diagnostic contains: @NonNull field s not initialized", - " Set<@Nullable Foo> s;", // i.e. Set - " class Foo { }", + " Set<@Nullable Foo> s;", + "", + " class Foo {}", + "", " public void test() {", " // safe because field is @NonNull", " s.hashCode();", @@ -177,19 +196,21 @@ public void typeUseAnnotationOnArray() { .addSourceLines( "Test.java", "package com.uber;", + "", "import org.checkerframework.checker.nullness.qual.Nullable;", + "", "class Test {", " // ok only for backwards compat", " @Nullable Object[] foo1 = null;", " // ok according to spec", - " Object @Nullable[] foo2 = null;", + " Object @Nullable [] foo2 = null;", " // ok only for backwards compat", - " @Nullable Object [][] foo3 = null;", + " @Nullable Object[][] foo3 = null;", " // ok according to spec", " Object @Nullable [][] foo4 = null;", " // NOT ok; @Nullable applies to first array dimension not the elements or the array ref", " // TODO: Fix this as part of https://github.com/uber/NullAway/issues/708", - " Object [] @Nullable [] foo5 = null;", + " Object[] @Nullable [] foo5 = null;", "}") .doTest(); } @@ -200,9 +221,15 @@ public void typeUseAnnotationOnInnerMultiLevel() { .addSourceLines( "Test.java", "package com.uber;", - "import java.util.Set;", + "", "import org.checkerframework.checker.nullness.qual.Nullable;", - "class A { class B { class C {} } }", + "", + "class A {", + " class B {", + " class C {}", + " }", + "}", + "", "class Test {", " // At some point, we should not treat foo1 or foo2 as @Nullable.", " // For now we do, for ease of compatibility.", @@ -214,7 +241,7 @@ public void typeUseAnnotationOnInnerMultiLevel() { " // It neither matches were a correct type use annotation for marking foo4 as @Nullable would be,", " // nor the natural position of a declaration annotation at the start of the type!", " // BUG: Diagnostic contains: assigning @Nullable expression to @NonNull field", - " A.B.@Nullable C [][] foo4 = null;", + " A.B.@Nullable C[][] foo4 = null;", "}") .doTest(); } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayUnannotatedTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayUnannotatedTests.java index 32e844ac5c..3012a8e6d8 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayUnannotatedTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayUnannotatedTests.java @@ -26,12 +26,13 @@ public void skipClass() { .addSourceLines( "Test.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "@com.uber.lib.MyExcluded", "public class Test {", " static void bar() {", " // No error", - " Object x = null; x.toString();", + " Object x = null;", + " x.toString();", " }", "}") .doTest(); @@ -48,16 +49,20 @@ public void skipNestedClass() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public class Test {", " @com.uber.lib.MyExcluded", " static class Inner {", " @Nullable", " static Object foo() {", - " Object x = null; x.toString();", + " Object x = null;", + " x.toString();", " return x;", " }", " }", + "", " static void bar() {", " // BUG: Diagnostic contains: dereferenced expression Inner.foo()", " Inner.foo().toString();", @@ -91,8 +96,11 @@ public void generatedAsUnannotated() { .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", - " void foo() { (new Generated()).takeObj(null); }", + " void foo() {", + " (new Generated()).takeObj(null);", + " }", "}") .doTest(); } @@ -109,27 +117,36 @@ public void generatedAsUnannotatedCustomAnnotation() { .addSourceLines( "MyGeneratedMarkerAnnotation.java", "package com.uber;", - "import java.lang.annotation.Retention;", - "import java.lang.annotation.Target;", + "", "import static java.lang.annotation.ElementType.CONSTRUCTOR;", "import static java.lang.annotation.ElementType.FIELD;", - "import static java.lang.annotation.ElementType.TYPE;", "import static java.lang.annotation.ElementType.METHOD;", "import static java.lang.annotation.ElementType.PACKAGE;", + "import static java.lang.annotation.ElementType.TYPE;", "import static java.lang.annotation.RetentionPolicy.SOURCE;", + "", + "import java.lang.annotation.Retention;", + "import java.lang.annotation.Target;", + "", "@Retention(SOURCE)", "@Target({PACKAGE, TYPE, METHOD, CONSTRUCTOR, FIELD})", "public @interface MyGeneratedMarkerAnnotation {}") .addSourceLines( "Generated.java", "package com.uber;", + "", "@MyGeneratedMarkerAnnotation", - "public class Generated { public void takeObj(Object o) {} }") + "public class Generated {", + " public void takeObj(Object o) {}", + "}") .addSourceLines( "Test.java", "package com.uber;", + "", "class Test {", - " void foo() { (new Generated()).takeObj(null); }", + " void foo() {", + " (new Generated()).takeObj(null);", + " }", "}") .doTest(); } @@ -145,19 +162,29 @@ public void unannotatedClass() { .addSourceLines( "UnAnnot.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public class UnAnnot {", - " @Nullable static Object retNull() { return null; }", + " @Nullable", + " static Object retNull() {", + " return null;", + " }", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "class Test {", - " @Nullable static Object nullRetSameClass() { return null; }", + " @Nullable", + " static Object nullRetSameClass() {", + " return null;", + " }", + "", " void test() {", " UnAnnot.retNull().toString();", - // make sure other classes in the package still get analyzed " Object x = nullRetSameClass();", " // BUG: Diagnostic contains: dereferenced expression x is @Nullable", " x.hashCode();", @@ -172,21 +199,24 @@ public void overrideFailsOnExplicitlyNullableLibraryModelParam() { .addSourceLines( // Dummy android.view.GestureDetector.OnGestureListener interface "GestureDetector.java", "package android.view;", + "", "public class GestureDetector {", " public static interface OnGestureListener {", - // Ignore other methods for this test, to make code shorter on both files: " boolean onScroll(MotionEvent me1, MotionEvent me2, float f1, float f2);", " }", "}") .addSourceLines( // Dummy android.view.MotionEvent class - "MotionEvent.java", "package android.view;", "public class MotionEvent { }") + "MotionEvent.java", "package android.view;", "", "public class MotionEvent {}") .addSourceLines( "Test.java", "package com.uber;", + "", "import android.view.GestureDetector;", "import android.view.MotionEvent;", + "", "class Test implements GestureDetector.OnGestureListener {", - " Test() { }", + " Test() {}", + "", " @Override", " // BUG: Diagnostic contains: parameter me1 is @NonNull", " public boolean onScroll(MotionEvent me1, MotionEvent me2, float f1, float f2) {", @@ -196,11 +226,14 @@ public void overrideFailsOnExplicitlyNullableLibraryModelParam() { .addSourceLines( "Test2.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "import android.view.GestureDetector;", "import android.view.MotionEvent;", + "import javax.annotation.Nullable;", + "", "class Test2 implements GestureDetector.OnGestureListener {", - " Test2() { }", + " Test2() {}", + "", " @Override", " public boolean onScroll(@Nullable MotionEvent me1, MotionEvent me2, float f1, float f2) {", " return false; // NoOp", @@ -223,14 +256,16 @@ public void typeCastInUnannotatedCode() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.function.Consumer;", "import org.junit.Assert;", + "", "class Test {", " private void verifyCountZero() {", " verifyData((count) -> Assert.assertEquals(0, (long) count));", " }", - " private void verifyData(Consumer assertFunction) {", - " }", + "", + " private void verifyData(Consumer assertFunction) {}", "}") .doTest(); } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayUnsoundnessTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayUnsoundnessTests.java index a765b1346b..dfce9bdad4 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayUnsoundnessTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayUnsoundnessTests.java @@ -26,7 +26,9 @@ public void mapReassignUnsound() { .addSourceLines( "Test.java", "package com.uber;", + "", "import java.util.*;", + "", "public class Test {", " public void mapReassignUnsound(Map m, Object o) {", " if (m.containsKey(o)) {", diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayVarargsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayVarargsTests.java index 97ff399dc7..2ca662f817 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayVarargsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayVarargsTests.java @@ -11,24 +11,26 @@ public void testNonNullVarargs() { .addSourceLines( "Utilities.java", "package com.uber;", - "import javax.annotation.Nullable;", + "", "public class Utilities {", - " public static String takesNonNullVarargs(Object o, Object... others) {", - " String s = o.toString() + \" \";", - " for (Object other : others) {", - " s += other.toString() + \" \";", + " public static String takesNonNullVarargs(Object o, Object... others) {", + " String s = o.toString() + \" \";", + " for (Object other : others) {", + " s += other.toString() + \" \";", + " }", + " return s;", " }", - " return s;", - " }", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public class Test {", " public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", " Utilities.takesNonNullVarargs(o1, o2, o3);", - " Utilities.takesNonNullVarargs(o1);", // Empty var args passed + " Utilities.takesNonNullVarargs(o1);", " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", " Utilities.takesNonNullVarargs(o1, o4);", " }", @@ -42,16 +44,18 @@ public void testNullableVarargs() { .addSourceLines( "Utilities.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public class Utilities {", - " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", - " String s = o.toString() + \" \";", - " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", - " for (Object other : others) {", - " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " public static String takesNullableVarargs(Object o, @Nullable Object... others) {", + " String s = o.toString() + \" \";", + " // BUG: Diagnostic contains: enhanced-for expression others is @Nullable", + " for (Object other : others) {", + " s += (other == null) ? \"(null) \" : other.toString() + \" \";", + " }", + " return s;", " }", - " return s;", - " }", "}") .doTest(); } @@ -86,11 +90,13 @@ public void testNonNullVarargsFromHandler() { .addSourceLines( "Test.java", "package com.uber;", + "", "import javax.annotation.Nullable;", + "", "public class Test {", " public void testNonNullVarargs(Object o1, Object o2, Object o3, @Nullable Object o4) {", " Generated.takesNonNullVarargs(o1, o2, o3);", - " Generated.takesNonNullVarargs(o1);", // Empty var args passed + " Generated.takesNonNullVarargs(o1);", " // BUG: Diagnostic contains: passing @Nullable parameter 'o4' where @NonNull", " Generated.takesNonNullVarargs(o1, o4);", " }", diff --git a/nullaway/src/test/java/com/uber/nullaway/thirdpartylibs/NullAwayGrpcTest.java b/nullaway/src/test/java/com/uber/nullaway/thirdpartylibs/NullAwayGrpcTest.java index cded75fbe9..50465dbf85 100644 --- a/nullaway/src/test/java/com/uber/nullaway/thirdpartylibs/NullAwayGrpcTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/thirdpartylibs/NullAwayGrpcTest.java @@ -67,20 +67,26 @@ public void ioGrpcMetadataAsMapTest() { .addSourceLines( "Keys.java", "package com.uber;", + "", "import io.grpc.Metadata;", + "", "public final class Keys {", - " static final Metadata.Key KEY_1 = Metadata.Key.of(\"KEY1\", Metadata.ASCII_STRING_MARSHALLER);", + " static final Metadata.Key KEY_1 =", + " Metadata.Key.of(\"KEY1\", Metadata.ASCII_STRING_MARSHALLER);", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "import io.grpc.Metadata;", + "", "public class Test {", - " public void takeNonNullString(String s) { }", + " public void takeNonNullString(String s) {}", + "", " public void safeDeref(Metadata headers) {", - " if (headers.containsKey(Keys.KEY_1)) {", - " takeNonNullString(headers.get(Keys.KEY_1));", - " }", + " if (headers.containsKey(Keys.KEY_1)) {", + " takeNonNullString(headers.get(Keys.KEY_1));", + " }", " }", "}") .doTest(); @@ -92,22 +98,29 @@ public void ioGrpcMetadataAsMapPossitiveTest() { .addSourceLines( "Keys.java", "package com.uber;", + "", "import io.grpc.Metadata;", + "", "public final class Keys {", - " static final Metadata.Key KEY_1 = Metadata.Key.of(\"KEY1\", Metadata.ASCII_STRING_MARSHALLER);", - " static final Metadata.Key KEY_2 = Metadata.Key.of(\"KEY2\", Metadata.ASCII_STRING_MARSHALLER);", + " static final Metadata.Key KEY_1 =", + " Metadata.Key.of(\"KEY1\", Metadata.ASCII_STRING_MARSHALLER);", + " static final Metadata.Key KEY_2 =", + " Metadata.Key.of(\"KEY2\", Metadata.ASCII_STRING_MARSHALLER);", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "import io.grpc.Metadata;", + "", "public class Test {", - " public void takeNonNullString(String s) { }", + " public void takeNonNullString(String s) {}", + "", " public void safeDeref(Metadata headers) {", - " if (headers.containsKey(Keys.KEY_1)) {", - " // BUG: Diagnostic contains: passing @Nullable parameter 'headers.get(Keys.KEY_2)'", - " takeNonNullString(headers.get(Keys.KEY_2));", // Expect error: KEY_2 != KEY_1 - " }", + " if (headers.containsKey(Keys.KEY_1)) {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'headers.get(Keys.KEY_2)'", + " takeNonNullString(headers.get(Keys.KEY_2));", + " }", " }", "}") .doTest(); @@ -119,20 +132,26 @@ public void ioGrpcMetadataAsAccessPathTest() { .addSourceLines( "Keys.java", "package com.uber;", + "", "import io.grpc.Metadata;", + "", "public final class Keys {", - " static final Metadata.Key KEY_1 = Metadata.Key.of(\"KEY1\", Metadata.ASCII_STRING_MARSHALLER);", + " static final Metadata.Key KEY_1 =", + " Metadata.Key.of(\"KEY1\", Metadata.ASCII_STRING_MARSHALLER);", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "import io.grpc.Metadata;", + "", "public class Test {", - " public void takeNonNullString(String s) { }", + " public void takeNonNullString(String s) {}", + "", " public void safeDeref(Metadata headers) {", - " if (headers.get(Keys.KEY_1) != null) {", - " takeNonNullString(headers.get(Keys.KEY_1));", - " }", + " if (headers.get(Keys.KEY_1) != null) {", + " takeNonNullString(headers.get(Keys.KEY_1));", + " }", " }", "}") .doTest(); @@ -144,22 +163,29 @@ public void ioGrpcMetadataAsAccessPathPositiveTest() { .addSourceLines( "Keys.java", "package com.uber;", + "", "import io.grpc.Metadata;", + "", "public final class Keys {", - " static final Metadata.Key KEY_1 = Metadata.Key.of(\"KEY1\", Metadata.ASCII_STRING_MARSHALLER);", - " static final Metadata.Key KEY_2 = Metadata.Key.of(\"KEY2\", Metadata.ASCII_STRING_MARSHALLER);", + " static final Metadata.Key KEY_1 =", + " Metadata.Key.of(\"KEY1\", Metadata.ASCII_STRING_MARSHALLER);", + " static final Metadata.Key KEY_2 =", + " Metadata.Key.of(\"KEY2\", Metadata.ASCII_STRING_MARSHALLER);", "}") .addSourceLines( "Test.java", "package com.uber;", + "", "import io.grpc.Metadata;", + "", "public class Test {", - " public void takeNonNullString(String s) { }", + " public void takeNonNullString(String s) {}", + "", " public void safeDeref(Metadata headers) {", - " if (headers.get(Keys.KEY_1) != null) {", - " // BUG: Diagnostic contains: passing @Nullable parameter 'headers.get(Keys.KEY_2)'", - " takeNonNullString(headers.get(Keys.KEY_2));", // Expect error: KEY_2 != KEY_1 - " }", + " if (headers.get(Keys.KEY_1) != null) {", + " // BUG: Diagnostic contains: passing @Nullable parameter 'headers.get(Keys.KEY_2)'", + " takeNonNullString(headers.get(Keys.KEY_2));", + " }", " }", "}") .doTest();