From 99cdb15acfdddcf01b930398825289b7213efd0e Mon Sep 17 00:00:00 2001 From: cpovirk Date: Thu, 4 Nov 2021 14:14:10 -0700 Subject: [PATCH] Always annotate arrays now that we place type-use annotations in the right place. PiperOrigin-RevId: 407667606 --- .../nullness/ReturnMissingNullable.java | 7 - .../nullness/ReturnMissingNullableTest.java | 162 ++++++++---------- 2 files changed, 72 insertions(+), 97 deletions(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullable.java b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullable.java index 8c1a5c4e5b3..a12c0750145 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullable.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullable.java @@ -208,13 +208,6 @@ && getOnlyElement(statements) == returnTree // Buggy code, but adding @Nullable just makes it worse. return; } - if (beingConservative && state.getTypes().isArray(returnType)) { - /* - * Type-annotation syntax on arrays can be confusing, and this refactoring doesn't get it - * right yet. - */ - return; - } if (beingConservative && returnType.getKind() == TYPEVAR) { /* * Consider AbstractFuture.getDoneValue: It returns a literal `null`, but it shouldn't be diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullableTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullableTest.java index 8329c67d4ec..8ef005a77f0 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullableTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/nullness/ReturnMissingNullableTest.java @@ -433,6 +433,78 @@ public void testOnlyIfAlreadyInScopeAndItIs() { .doTest(); } + @Test + public void testArrayDeclaration() { + createRefactoringTestHelper() + .addInputLines( + "in/com/google/errorprone/bugpatterns/nullness/LiteralNullReturnTest.java", + "package com.google.errorprone.bugpatterns.nullness;", + "import javax.annotation.Nullable;", + "public class LiteralNullReturnTest {", + " public String[] getMessage(boolean b) {", + " return b ? null : new String[0];", + " }", + "}") + .addOutputLines( + "out/com/google/errorprone/bugpatterns/nullness/LiteralNullReturnTest.java", + "package com.google.errorprone.bugpatterns.nullness;", + "import javax.annotation.Nullable;", + "public class LiteralNullReturnTest {", + " @Nullable public String[] getMessage(boolean b) {", + " return b ? null : new String[0];", + " }", + "}") + .doTest(); + } + + @Test + public void testArrayTypeUse() { + createRefactoringTestHelper() + .addInputLines( + "in/com/google/errorprone/bugpatterns/nullness/LiteralNullReturnTest.java", + "package com.google.errorprone.bugpatterns.nullness;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class LiteralNullReturnTest {", + " public String[] getMessage(boolean b) {", + " return b ? null : new String[0];", + " }", + "}") + .addOutputLines( + "out/com/google/errorprone/bugpatterns/nullness/LiteralNullReturnTest.java", + "package com.google.errorprone.bugpatterns.nullness;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class LiteralNullReturnTest {", + " public String @Nullable [] getMessage(boolean b) {", + " return b ? null : new String[0];", + " }", + "}") + .doTest(); + } + + @Test + public void testArrayTypeUseTwoDimensional() { + createRefactoringTestHelper() + .addInputLines( + "in/com/google/errorprone/bugpatterns/nullness/LiteralNullReturnTest.java", + "package com.google.errorprone.bugpatterns.nullness;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class LiteralNullReturnTest {", + " public String[][] getMessage(boolean b, String[][] s) {", + " return b ? null : s;", + " }", + "}") + .addOutputLines( + "out/com/google/errorprone/bugpatterns/nullness/LiteralNullReturnTest.java", + "package com.google.errorprone.bugpatterns.nullness;", + "import org.checkerframework.checker.nullness.qual.Nullable;", + "public class LiteralNullReturnTest {", + " public String @Nullable [][] getMessage(boolean b, String[][] s) {", + " return b ? null : s;", + " }", + "}") + .doTest(); + } + @Test public void testLimitation_staticFinalFieldInitializedLater() { createCompilationTestHelper() @@ -553,20 +625,6 @@ public void testNegativeCases_onlyStatementIsNullReturn() { .doTest(); } - @Test - public void testNegativeCases_array() { - createCompilationTestHelper() - .addSourceLines( - "com/google/errorprone/bugpatterns/nullness/LiteralNullReturnTest.java", - "package com.google.errorprone.bugpatterns.nullness;", - "public class LiteralNullReturnTest {", - " public String[] getMessage(boolean b) {", - " return b ? null : new String[0];", - " }", - "}") - .doTest(); - } - @Test public void testNegativeCases_typeVariableUsage() { createCompilationTestHelper() @@ -1463,78 +1521,6 @@ public void testAggressive_onlyStatementIsNullReturn() { .doTest(); } - @Test - public void testAggressive_arrayDeclaration() { - createAggressiveRefactoringTestHelper() - .addInputLines( - "in/com/google/errorprone/bugpatterns/nullness/LiteralNullReturnTest.java", - "package com.google.errorprone.bugpatterns.nullness;", - "import javax.annotation.Nullable;", - "public class LiteralNullReturnTest {", - " public String[] getMessage(boolean b) {", - " return b ? null : new String[0];", - " }", - "}") - .addOutputLines( - "out/com/google/errorprone/bugpatterns/nullness/LiteralNullReturnTest.java", - "package com.google.errorprone.bugpatterns.nullness;", - "import javax.annotation.Nullable;", - "public class LiteralNullReturnTest {", - " @Nullable public String[] getMessage(boolean b) {", - " return b ? null : new String[0];", - " }", - "}") - .doTest(); - } - - @Test - public void testAggressive_arrayTypeUse() { - createAggressiveRefactoringTestHelper() - .addInputLines( - "in/com/google/errorprone/bugpatterns/nullness/LiteralNullReturnTest.java", - "package com.google.errorprone.bugpatterns.nullness;", - "import org.checkerframework.checker.nullness.qual.Nullable;", - "public class LiteralNullReturnTest {", - " public String[] getMessage(boolean b) {", - " return b ? null : new String[0];", - " }", - "}") - .addOutputLines( - "out/com/google/errorprone/bugpatterns/nullness/LiteralNullReturnTest.java", - "package com.google.errorprone.bugpatterns.nullness;", - "import org.checkerframework.checker.nullness.qual.Nullable;", - "public class LiteralNullReturnTest {", - " public String @Nullable [] getMessage(boolean b) {", - " return b ? null : new String[0];", - " }", - "}") - .doTest(); - } - - @Test - public void testAggressive_arrayTypeUseTwoDimensional() { - createAggressiveRefactoringTestHelper() - .addInputLines( - "in/com/google/errorprone/bugpatterns/nullness/LiteralNullReturnTest.java", - "package com.google.errorprone.bugpatterns.nullness;", - "import org.checkerframework.checker.nullness.qual.Nullable;", - "public class LiteralNullReturnTest {", - " public String[][] getMessage(boolean b, String[][] s) {", - " return b ? null : s;", - " }", - "}") - .addOutputLines( - "out/com/google/errorprone/bugpatterns/nullness/LiteralNullReturnTest.java", - "package com.google.errorprone.bugpatterns.nullness;", - "import org.checkerframework.checker.nullness.qual.Nullable;", - "public class LiteralNullReturnTest {", - " public String @Nullable [][] getMessage(boolean b, String[][] s) {", - " return b ? null : s;", - " }", - "}") - .doTest(); - } - @Test public void testAggressive_typeVariableUsage() { createAggressiveCompilationTestHelper() @@ -1579,8 +1565,4 @@ private CompilationTestHelper createAggressiveCompilationTestHelper() { private BugCheckerRefactoringTestHelper createRefactoringTestHelper() { return BugCheckerRefactoringTestHelper.newInstance(ReturnMissingNullable.class, getClass()); } - - private BugCheckerRefactoringTestHelper createAggressiveRefactoringTestHelper() { - return createRefactoringTestHelper().setArgs("-XepOpt:Nullness:Conservative=false"); - } }