From 95d3e8ee8e628777bf230b488c405db0339f0f70 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Thu, 9 Nov 2023 15:53:32 -0800 Subject: [PATCH 1/5] Adding test case and relevant logic to handle when a returned diamond operator object is calling a method --- .../java/com/uber/nullaway/GenericsChecks.java | 12 ++++++++++++ .../NullAwayJSpecifyGenericsTests.java | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index ef56490473..152f5b9b39 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -787,6 +787,18 @@ public static Nullness getGenericReturnNullnessAtInvocation( if (!(tree.getMethodSelect() instanceof MemberSelectTree) || invokedMethodSymbol.isStatic()) { return Nullness.NONNULL; } + Tree expressionTree = ((MemberSelectTree) tree.getMethodSelect()).getExpression(); + if (expressionTree instanceof NewClassTree + && ((NewClassTree) expressionTree).getIdentifier() instanceof ParameterizedTypeTree) { + ParameterizedTypeTree paramTypedTree = + (ParameterizedTypeTree) ((NewClassTree) expressionTree).getIdentifier(); + // The case of having a diamond operator + if (paramTypedTree.getTypeArguments().isEmpty()) { + // bail out + // TODO: support diamond operators + return Nullness.NONNULL; + } + } Type methodReceiverType = castToNonNull( getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state)); diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 58f09d1f8a..eb7c02fe65 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -1522,6 +1522,24 @@ public void testForStaticMethodCallAsAParam() { .doTest(); } + @Test + public void testForDiamondOperatorReturnedAsAMethodCaller() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static class B{", + " String build(){return \"x\";}", + " }", + " static String testNegative() {", + " return new B<>().build();", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From f3248b576f0c821ecd6d3cce7212880a769493db Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Sat, 11 Nov 2023 16:30:05 -0800 Subject: [PATCH 2/5] adding logic to bail out if we get null from returnTreeType --- .../java/com/uber/nullaway/GenericsChecks.java | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 152f5b9b39..dd55bcf042 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -784,21 +784,12 @@ public static Nullness getGenericReturnNullnessAtInvocation( MethodInvocationTree tree, VisitorState state, Config config) { - if (!(tree.getMethodSelect() instanceof MemberSelectTree) || invokedMethodSymbol.isStatic()) { + if (!(tree.getMethodSelect() instanceof MemberSelectTree) + || invokedMethodSymbol.isStatic() + || null + == getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state)) { return Nullness.NONNULL; } - Tree expressionTree = ((MemberSelectTree) tree.getMethodSelect()).getExpression(); - if (expressionTree instanceof NewClassTree - && ((NewClassTree) expressionTree).getIdentifier() instanceof ParameterizedTypeTree) { - ParameterizedTypeTree paramTypedTree = - (ParameterizedTypeTree) ((NewClassTree) expressionTree).getIdentifier(); - // The case of having a diamond operator - if (paramTypedTree.getTypeArguments().isEmpty()) { - // bail out - // TODO: support diamond operators - return Nullness.NONNULL; - } - } Type methodReceiverType = castToNonNull( getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state)); From 7b0d5dd50b17f00d6a693576dd4e7955c587d9f2 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Sun, 12 Nov 2023 13:26:25 -0800 Subject: [PATCH 3/5] removing redundant call to getTreeType --- .../java/com/uber/nullaway/GenericsChecks.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index dd55bcf042..8a9df3d87d 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -784,17 +784,17 @@ public static Nullness getGenericReturnNullnessAtInvocation( MethodInvocationTree tree, VisitorState state, Config config) { - if (!(tree.getMethodSelect() instanceof MemberSelectTree) - || invokedMethodSymbol.isStatic() - || null - == getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state)) { + if (!(tree.getMethodSelect() instanceof MemberSelectTree) || invokedMethodSymbol.isStatic()) { return Nullness.NONNULL; } Type methodReceiverType = - castToNonNull( - getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state)); - return getGenericMethodReturnTypeNullness( - invokedMethodSymbol, methodReceiverType, state, config); + getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state); + if (null == methodReceiverType) { + return Nullness.NONNULL; + } else { + return getGenericMethodReturnTypeNullness( + invokedMethodSymbol, castToNonNull(methodReceiverType), state, config); + } } /** From 4aac75e703a2949cf97b5cf365894fbe983c66ee Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Sun, 12 Nov 2023 13:41:08 -0800 Subject: [PATCH 4/5] removing unnecessary call to castToNonNull --- nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 8a9df3d87d..9403619006 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -793,7 +793,7 @@ public static Nullness getGenericReturnNullnessAtInvocation( return Nullness.NONNULL; } else { return getGenericMethodReturnTypeNullness( - invokedMethodSymbol, castToNonNull(methodReceiverType), state, config); + invokedMethodSymbol, methodReceiverType, state, config); } } From 35c0cc3325419455a6bb7c96f30062003cb00272 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Mon, 13 Nov 2023 10:25:01 -0800 Subject: [PATCH 5/5] Minor tweak --- nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index 9403619006..4870366771 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -789,7 +789,7 @@ public static Nullness getGenericReturnNullnessAtInvocation( } Type methodReceiverType = getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), state); - if (null == methodReceiverType) { + if (methodReceiverType == null) { return Nullness.NONNULL; } else { return getGenericMethodReturnTypeNullness(