diff --git a/nullaway/build.gradle b/nullaway/build.gradle index 026a89094a..019cb8b5ff 100644 --- a/nullaway/build.gradle +++ b/nullaway/build.gradle @@ -123,6 +123,8 @@ def jdk8Test = tasks.register("testJdk8", Test) { testClassesDirs = testTask.testClassesDirs jvmArgs "-Xbootclasspath/p:${configurations.errorproneJavac.asPath}" filter { + // JDK 8 does not support diamonds on anonymous classes + excludeTestsMatching "com.uber.nullaway.NullAwayJSpecifyGenericsTests.overrideDiamondAnonymousClass" // tests cannot run on JDK 8 since Mockito version no longer supports it excludeTestsMatching "com.uber.nullaway.NullAwaySerializationTest.initializationError" excludeTestsMatching "com.uber.nullaway.handlers.contract.ContractUtilsTest.getEmptyAntecedent" diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index a9f3a6d717..0d814a7bc1 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -1,5 +1,6 @@ package com.uber.nullaway; +import static com.google.common.base.Verify.verify; import static com.uber.nullaway.NullabilityUtil.castToNonNull; import static java.util.stream.Collectors.joining; @@ -22,6 +23,7 @@ import com.sun.source.tree.Tree; import com.sun.source.tree.VariableTree; import com.sun.source.util.SimpleTreeVisitor; +import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Attribute; import com.sun.tools.javac.code.BoundKind; import com.sun.tools.javac.code.Symbol; @@ -686,14 +688,53 @@ public static void checkTypeParameterNullnessForMethodOverriding( * String}. * * @param method the generic method - * @param enclosingType the enclosing type in which we want to know {@code method}'s return type - * nullability + * @param enclosingSymbol the enclosing class in which we want to know {@code method}'s return + * type nullability * @param state Visitor state * @param config The analysis config * @return nullability of the return type of {@code method} in the context of {@code * enclosingType} */ public static Nullness getGenericMethodReturnTypeNullness( + Symbol.MethodSymbol method, Symbol enclosingSymbol, VisitorState state, Config config) { + Type enclosingType = getTypeForSymbol(enclosingSymbol, state); + if (enclosingType == null) { + // we have no additional information from generics, so return NONNULL (presence of a @Nullable + // annotation should have been handled by the caller) + return Nullness.NONNULL; + } + return getGenericMethodReturnTypeNullness(method, enclosingType, state, config); + } + + /** + * Get the type for the symbol, accounting for anonymous classes + * + * @param symbol the symbol + * @param state the visitor state + * @return the type for {@code symbol} + */ + @Nullable + private static Type getTypeForSymbol(Symbol symbol, VisitorState state) { + if (symbol.isAnonymous()) { + // For anonymous classes, symbol.type does not contain annotations on generic type parameters. + // So, we get a correct type from the enclosing NewClassTree. + TreePath path = state.getPath(); + NewClassTree newClassTree = ASTHelpers.findEnclosingNode(path, NewClassTree.class); + if (newClassTree == null) { + throw new RuntimeException( + "method should be inside a NewClassTree " + state.getSourceForNode(path.getLeaf())); + } + Type typeFromTree = getTreeType(newClassTree, state); + if (typeFromTree != null) { + verify(state.getTypes().isAssignable(symbol.type, typeFromTree)); + } + return typeFromTree; + } else { + return symbol.type; + } + } + + private static Nullness getGenericMethodReturnTypeNullness( Symbol.MethodSymbol method, Type enclosingType, VisitorState state, Config config) { Type overriddenMethodType = state.getTypes().memberType(enclosingType, method); if (!(overriddenMethodType instanceof Type.MethodType)) { @@ -743,7 +784,7 @@ public static Nullness getGenericReturnNullnessAtInvocation( } Type methodReceiverType = castToNonNull( - ASTHelpers.getType(((MemberSelectTree) tree.getMethodSelect()).getExpression())); + getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state)); return getGenericMethodReturnTypeNullness( invokedMethodSymbol, methodReceiverType, state, config); } @@ -791,11 +832,11 @@ public static Nullness getGenericParameterNullnessAtInvocation( if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { return Nullness.NONNULL; } - Type methodReceiverType = + Type enclosingType = castToNonNull( - ASTHelpers.getType(((MemberSelectTree) tree.getMethodSelect()).getExpression())); + getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state)); return getGenericMethodParameterNullness( - paramIndex, invokedMethodSymbol, methodReceiverType, state, config); + paramIndex, invokedMethodSymbol, enclosingType, state, config); } /** @@ -822,6 +863,35 @@ public static Nullness getGenericParameterNullnessAtInvocation( * * @param parameterIndex index of the parameter * @param method the generic method + * @param enclosingSymbol the enclosing symbol in which we want to know {@code method}'s parameter + * type nullability + * @param state the visitor state + * @param config the config + * @return nullability of the relevant parameter type of {@code method} in the context of {@code + * enclosingSymbol} + */ + public static Nullness getGenericMethodParameterNullness( + int parameterIndex, + Symbol.MethodSymbol method, + Symbol enclosingSymbol, + VisitorState state, + Config config) { + Type enclosingType = getTypeForSymbol(enclosingSymbol, state); + if (enclosingType == null) { + // we have no additional information from generics, so return NONNULL (presence of a @Nullable + // annotation should have been handled by the caller) + return Nullness.NONNULL; + } + return getGenericMethodParameterNullness(parameterIndex, method, enclosingType, state, config); + } + + /** + * Just like {@link #getGenericMethodParameterNullness(int, Symbol.MethodSymbol, Symbol, + * VisitorState, Config)}, but takes the enclosing {@code Type} rather than the enclosing {@code + * Symbol}. + * + * @param parameterIndex index of the parameter + * @param method the generic method * @param enclosingType the enclosing type in which we want to know {@code method}'s parameter * type nullability * @param state the visitor state diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index bfed0af259..79c7c8ca34 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -738,7 +738,7 @@ private Description checkParamOverriding( ? GenericsChecks.getGenericMethodParameterNullness( i, overriddenMethod, - overridingParamSymbols.get(i).owner.owner.type, + overridingParamSymbols.get(i).owner.owner, state, config) : Nullness.NONNULL); @@ -944,7 +944,7 @@ private Description checkOverriding( // if the super method returns nonnull, overriding method better not return nullable // Note that, for the overriding method, the permissive default is non-null, // but it's nullable for the overridden one. - if (overriddenMethodReturnsNonNull(overriddenMethod, overridingMethod.owner.type, state) + if (overriddenMethodReturnsNonNull(overriddenMethod, overridingMethod.owner, state) && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL) .equals(Nullness.NULLABLE) && (memberReferenceTree == null @@ -983,7 +983,7 @@ && getMethodReturnNullness(overridingMethod, state, Nullness.NONNULL) } private boolean overriddenMethodReturnsNonNull( - Symbol.MethodSymbol overriddenMethod, Type overridingMethodType, VisitorState state) { + Symbol.MethodSymbol overriddenMethod, Symbol enclosingSymbol, VisitorState state) { Nullness methodReturnNullness = getMethodReturnNullness(overriddenMethod, state, Nullness.NULLABLE); if (!methodReturnNullness.equals(Nullness.NONNULL)) { @@ -993,7 +993,7 @@ private boolean overriddenMethodReturnsNonNull( // using the type parameters from the type enclosing the overriding method if (config.isJSpecifyMode()) { return GenericsChecks.getGenericMethodReturnTypeNullness( - overriddenMethod, overridingMethodType, state, config) + overriddenMethod, enclosingSymbol, state, config) .equals(Nullness.NONNULL); } return true; diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 14eeb16cbc..7f236cacfd 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -2,6 +2,7 @@ import com.google.errorprone.CompilationTestHelper; import java.util.Arrays; +import org.junit.Ignore; import org.junit.Test; public class NullAwayJSpecifyGenericsTests extends NullAwayTestsBase { @@ -974,6 +975,163 @@ public void overrideParameterType() { .doTest(); } + @Test + public void overrideExplicitlyTypedAnonymousClass() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface Fn

{", + " R apply(P p);", + " }", + " static abstract class FnClass

{", + " abstract R apply(P p);", + " }", + " static void anonymousClasses() {", + " Fn<@Nullable String, String> fn1 = new Fn<@Nullable String, String>() {", + " // BUG: Diagnostic contains: parameter s is @NonNull, but parameter in superclass method", + " public String apply(String s) { return s; }", + " };", + " FnClass fn2 = new FnClass() {", + " // BUG: Diagnostic contains: method returns @Nullable, but superclass method", + " public @Nullable String apply(String s) { return null; }", + " };", + " Fn fn3 = new Fn() {", + " public @Nullable String apply(String s) { return null; }", + " };", + " FnClass<@Nullable String, String> fn4 = new FnClass<@Nullable String, String>() {", + " public String apply(@Nullable String s) { return \"hello\"; }", + " };", + " }", + " static void anonymousClassesFullName() {", + " Test.Fn<@Nullable String, String> fn1 = new Test.Fn<@Nullable String, String>() {", + " // BUG: Diagnostic contains: parameter s is @NonNull, but parameter in superclass method", + " public String apply(String s) { return s; }", + " };", + " Test.FnClass fn2 = new Test.FnClass() {", + " // BUG: Diagnostic contains: method returns @Nullable, but superclass method", + " public @Nullable String apply(String s) { return null; }", + " };", + " Test.Fn fn3 = new Test.Fn() {", + " public @Nullable String apply(String s) { return null; }", + " };", + " Test.FnClass<@Nullable String, String> fn4 = new Test.FnClass<@Nullable String, String>() {", + " public String apply(@Nullable String s) { return \"hello\"; }", + " };", + " }", + "}") + .doTest(); + } + + @Ignore("https://github.com/uber/NullAway/issues/836") + @Test + public void overrideAnonymousNestedClass() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " class Wrapper

{", + " abstract class Fn {", + " abstract R apply(P p);", + " }", + " }", + " void anonymousNestedClasses() {", + " Wrapper<@Nullable String>.Fn fn1 = (this.new Wrapper<@Nullable String>()).new Fn() {", + " // BUG: Diagnostic contains: parameter s is @NonNull, but parameter in superclass method", + " public String apply(String s) { return s; }", + " };", + " }", + "}") + .doTest(); + } + + @Test + public void explicitlyTypedAnonymousClassAsReceiver() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface Fn

{", + " R apply(P p);", + " }", + " static abstract class FnClass

{", + " abstract R apply(P p);", + " }", + " static void anonymousClasses() {", + " String s1 = (new Fn() {", + " public @Nullable String apply(String s) { return null; }", + " }).apply(\"hi\");", + " // BUG: Diagnostic contains: dereferenced expression s1", + " s1.hashCode();", + " String s2 = (new FnClass() {", + " public @Nullable String apply(String s) { return null; }", + " }).apply(\"hi\");", + " // BUG: Diagnostic contains: dereferenced expression s2", + " s2.hashCode();", + " (new Fn() {", + " public String apply(String s) { return \"hi\"; }", + " // BUG: Diagnostic contains: passing @Nullable parameter", + " }).apply(null);", + " (new FnClass() {", + " public String apply(String s) { return \"hi\"; }", + " // BUG: Diagnostic contains: passing @Nullable parameter", + " }).apply(null);", + " (new Fn<@Nullable String, String>() {", + " public String apply(@Nullable String s) { return \"hi\"; }", + " }).apply(null);", + " (new FnClass<@Nullable String, String>() {", + " public String apply(@Nullable String s) { return \"hi\"; }", + " }).apply(null);", + " }", + "}") + .doTest(); + } + + /** Diamond anonymous classes are not supported yet; tests are for future reference */ + @Test + public void overrideDiamondAnonymousClass() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " interface Fn

{", + " R apply(P p);", + " }", + " static abstract class FnClass

{", + " abstract R apply(P p);", + " }", + " static void anonymousClasses() {", + " Fn<@Nullable String, String> fn1 = new Fn<>() {", + " // TODO: should report a bug here", + " public String apply(String s) { return s; }", + " };", + " FnClass<@Nullable String, String> fn2 = new FnClass<>() {", + " // TODO: should report a bug here", + " public String apply(String s) { return s; }", + " };", + " Fn fn3 = new Fn<>() {", + " // TODO: this is a false positive", + " // BUG: Diagnostic contains: method returns @Nullable, but superclass method", + " public @Nullable String apply(String s) { return null; }", + " };", + " FnClass fn4 = new FnClass<>() {", + " // TODO: this is a false positive", + " // BUG: Diagnostic contains: method returns @Nullable, but superclass method", + " public @Nullable String apply(String s) { return null; }", + " };", + " }", + "}") + .doTest(); + } + @Test public void nullableGenericTypeVariableReturnType() { makeHelper()