From 7d272615961e43851ab2ee0d57d2b849454d7f9f Mon Sep 17 00:00:00 2001 From: Shubham Ugare Date: Wed, 24 Apr 2019 21:16:51 +0530 Subject: [PATCH 1/2] Added tests for optional emptiness support with Rx --- .../java/com/uber/nullaway/NullAwayTest.java | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java index d167482205..41cb0dc545 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java @@ -1551,6 +1551,67 @@ public void OptionalEmptinessUncheckedTest() { .doTest(); } + @Test + public void OptionalEmptinessRxPositiveTest() { + compilationHelper + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber,io.reactivex", + "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.lib.unannotated", + "-XepOpt:NullAway:CheckOptionalEmptiness=true")) + .addSourceLines( + "TestPositive.java", + "package com.uber;", + "import java.util.Optional;", + "import io.reactivex.Observable;", + "public class TestPositive {", + " private static boolean perhaps() { return Math.random() > 0.5; }", + " void foo(Observable> observable) {", + " observable", + " .filter(optional -> optional.isPresent() || perhaps())", + " // BUG: Diagnostic contains: Optional optional can be empty", + " .map(optional -> optional.get().toString());", + " observable", + " .filter(optional -> optional.isPresent() || perhaps())", + " // BUG: Diagnostic contains: returning @Nullable expression from method with @NonNull", + " .map(optional -> optional.get())", + " .map(irr -> irr.toString());", + " }", + "}") + .doTest(); + } + + @Test + public void OptionalEmptinessRxNegativeTest() { + compilationHelper + .setArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.lib.unannotated", + "-XepOpt:NullAway:CheckOptionalEmptiness=true")) + .addSourceLines( + "TestNegative.java", + "package com.uber;", + "import java.util.Optional;", + "import io.reactivex.Observable;", + "public class TestNegative {", + " private static boolean perhaps() { return Math.random() > 0.5; }", + " void foo(Observable> observable) {", + " observable", + " .filter(optional -> optional.isPresent() && perhaps())", + " .map(optional -> optional.get().toString());", + " observable", + " .filter(optional -> optional.isPresent() && perhaps())", + " .map(optional -> optional.get());", + " }", + "}") + .doTest(); + } + @Test public void testCastToNonNull() { compilationHelper From 413476b2e040756faaf94b67ad097ddfc993d827 Mon Sep 17 00:00:00 2001 From: Shubham Ugare Date: Fri, 26 Apr 2019 14:36:26 +0530 Subject: [PATCH 2/2] Addressed comments --- .../src/main/java/com/uber/nullaway/NullAway.java | 12 +++++++----- .../test/java/com/uber/nullaway/NullAwayTest.java | 5 ++++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 4104da418d..3d17824579 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -639,12 +639,14 @@ private Description checkReturnExpression( return Description.NO_MATCH; } if (mayBeNullExpr(state, retExpr)) { - String message = "returning @Nullable expression from method with @NonNull return type"; + final ErrorMessage errorMessage = + new ErrorMessage( + MessageTypes.RETURN_NULLABLE, + "returning @Nullable expression from method with @NonNull return type"); + handler.onPrepareErrorMessage(retExpr, state, errorMessage); + return errorBuilder.createErrorDescriptionForNullAssignment( - new ErrorMessage(MessageTypes.RETURN_NULLABLE, message), - retExpr, - state.getPath(), - buildDescription(tree)); + errorMessage, retExpr, state.getPath(), buildDescription(tree)); } return Description.NO_MATCH; } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java index 41cb0dc545..3010628a26 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java @@ -1575,7 +1575,7 @@ public void OptionalEmptinessRxPositiveTest() { " .map(optional -> optional.get().toString());", " observable", " .filter(optional -> optional.isPresent() || perhaps())", - " // BUG: Diagnostic contains: returning @Nullable expression from method with @NonNull", + " // BUG: Diagnostic contains: Optional optional can be empty", " .map(optional -> optional.get())", " .map(irr -> irr.toString());", " }", @@ -1602,6 +1602,9 @@ public void OptionalEmptinessRxNegativeTest() { " 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",