diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml index a56d502cf..6fa2e434d 100644 --- a/infer/src/nullsafe/eradicateChecks.ml +++ b/infer/src/nullsafe/eradicateChecks.ml @@ -301,7 +301,6 @@ let check_return_annotation tenv find_canonical_duplicate curr_pdesc ret_range annotated_signature.params in let ret_annotated_present = Annotations.ia_is_present ret_ia in - let ret_annotated_nonnull = Annotations.ia_is_nonnull ret_ia in match ret_range with (* Disables the warnings since it is not clear how to annotate the return value of lambdas *) | Some _ @@ -316,8 +315,7 @@ let check_return_annotation tenv find_canonical_duplicate curr_pdesc ret_range let final_present = TypeAnnotation.get_value AnnotatedSignature.Present final_ta in let origin_descr = TypeAnnotation.descr_origin final_ta in let return_not_nullable = - final_nullable && (not ret_annotated_nullable) && (not ret_implicitly_nullable) - && not ret_annotated_nonnull + final_nullable && (not ret_annotated_nullable) && not ret_implicitly_nullable in let return_value_not_present = Config.eradicate_optional_present && (not final_present) && ret_annotated_present diff --git a/infer/tests/codetoanalyze/java/eradicate/ReturnNotNullable.java b/infer/tests/codetoanalyze/java/eradicate/ReturnNotNullable.java index d7ffc52fa..295195b37 100644 --- a/infer/tests/codetoanalyze/java/eradicate/ReturnNotNullable.java +++ b/infer/tests/codetoanalyze/java/eradicate/ReturnNotNullable.java @@ -27,29 +27,97 @@ public class ReturnNotNullable { return null; } - String returnNull() { + // ------------------------------------------------------------ + // Converting different things to not annotated types. + // By default, (not annotated type is treated as non nullable). + + String nullToNotAnnotatedIsBad() { return null; } - String returnNullable(@Nullable String s) { + String nullableToNotAnnotatedIsBad(@Nullable String s) { + return s; + } + + String notAnnotatedToNotAnnotatedIsOK(String s) { return s; } + String nonNullToNotAnnotatedIsOK(@Nonnull String s) { + return s; + } + + String constantToNotAnnotatedIsOK() { + return "abc"; + } + + // ------------------------------------------------------------ + // Converting different things to @Nonnull. + // (Should be the same as converting to not annotated). + @Nonnull - String returnNonnull() { + String nullToNonnullIsBad() { + return null; + } + + @Nonnull + String nullableToNonnullIsBad(@Nullable String s) { + return s; + } + + @Nonnull + String notAnnotatedToNonnullIsOK(String s) { + return s; + } + + @Nonnull + String nonNullToNonnullIsOK(@Nonnull String s) { + return s; + } + + @Nonnull + String constantToNonNullIsOK() { return "abc"; } + // ------------------------------------------------------------ + // Converting different things to @Nullable. + // This is either + // 1. OK when inferred and annotated return types are both nullable, or + // 2. Leads to ERADICATE_RETURN_OVER_ANNOTATED when inferred return type + // is not nullable, but function is still annotated with @Nullable. + // This often happens when the API author decides to return @Nullable + // (anticipating future change) even though the current implementation returns non-null. + // Because of this the warning is currently turned off by default and is recommended + // to use only in specific scenarious, like code migrations. + @Nullable - String returnNullOK() { + String nullToNullableIsOK() { return null; } @Nullable - String returnNullableOK(@Nullable String s) { + String nullableToNullableIsOK(@Nullable String s) { + return s; + } + + @Nullable + String notAnnotatedNullableIsOverannotated(String s) { + return s; + } + + @Nullable + String nonNullToNullableIsOverannotated(@Nonnull String s) { return s; } + @Nullable + String constantToNullableIsOverannotated() { + return "abc"; + } + + // ------------------------------------------------------- + String throwException(@Nullable Exception e, boolean bad) throws Exception { if (bad) { throw (e); // no ERADICATE_RETURN_NOT_NULLABLE should be reported @@ -59,14 +127,14 @@ public class ReturnNotNullable { @Nullable String redundantEq() { - String s = returnNonnull(); + String s = constantToNonNullIsOK(); int n = s == null ? 0 : s.length(); return s; } @Nullable String redundantNeq() { - String s = returnNonnull(); + String s = constantToNonNullIsOK(); int n = s != null ? 0 : s.length(); return s; } @@ -85,7 +153,7 @@ public class ReturnNotNullable { Object tryWithResourcesReturnNullable(String path) throws IOException { try (BufferedReader br = nn(new BufferedReader(new FileReader(path)))) { - return returnNullOK(); + return nullToNullableIsOK(); } } diff --git a/infer/tests/codetoanalyze/java/eradicate/issues.exp b/infer/tests/codetoanalyze/java/eradicate/issues.exp index 3c5f816c7..87d56d9cb 100644 --- a/infer/tests/codetoanalyze/java/eradicate/issues.exp +++ b/infer/tests/codetoanalyze/java/eradicate/issues.exp @@ -113,14 +113,19 @@ codetoanalyze/java/eradicate/PresentTest.java, codetoanalyze.java.eradicate.Pres codetoanalyze/java/eradicate/PresentTest.java, codetoanalyze.java.eradicate.PresentTest$TestPresentAnnotationBasic.testOptionalAbsent():void, 1, ERADICATE_PARAMETER_VALUE_ABSENT, no_bucket, WARNING, [`PresentTest$TestPresentAnnotationBasic.expectPresent(...)` needs a present value in parameter 1 but argument `absent()` can be absent. (Origin: call to absent() at line 61)] codetoanalyze/java/eradicate/PresentTest.java, codetoanalyze.java.eradicate.PresentTest.testPresent(com.google.common.base.Optional,com.google.common.base.Optional):void, 4, ERADICATE_PARAMETER_VALUE_ABSENT, no_bucket, WARNING, [`PresentTest.argPresent(...)` needs a present value in parameter 1 but argument `absent` can be absent. (Origin: method parameter absent)] codetoanalyze/java/eradicate/Redundant.java, Redundant.bad():void, 2, ERADICATE_CONDITION_REDUNDANT, no_bucket, WARNING, [The condition s is always true according to the existing annotations.] -codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable$ConditionalAssignment.test(boolean):java.lang.Object, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `test(...)` may return null but it is not annotated with `@Nullable`. (Origin: field ReturnNotNullable$ConditionalAssignment.f1 at line 143)] -codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.getResourceNullable(java.lang.Class,java.lang.String):java.net.URL, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `getResourceNullable(...)` may return null but it is not annotated with `@Nullable`. (Origin: call to getResource(...) modelled in modelTables.ml at line 123)] +codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable$ConditionalAssignment.test(boolean):java.lang.Object, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `test(...)` may return null but it is not annotated with `@Nullable`. (Origin: field ReturnNotNullable$ConditionalAssignment.f1 at line 211)] +codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.constantToNullableIsOverannotated():java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, WARNING, [Method `constantToNullableIsOverannotated()` is annotated with `@Nullable` but never returns null.] +codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.getResourceNullable(java.lang.Class,java.lang.String):java.net.URL, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `getResourceNullable(...)` may return null but it is not annotated with `@Nullable`. (Origin: call to getResource(...) modelled in modelTables.ml at line 191)] +codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.nonNullToNullableIsOverannotated(java.lang.String):java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, WARNING, [Method `nonNullToNullableIsOverannotated(...)` is annotated with `@Nullable` but never returns null.] +codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.notAnnotatedNullableIsOverannotated(java.lang.String):java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, WARNING, [Method `notAnnotatedNullableIsOverannotated(...)` is annotated with `@Nullable` but never returns null.] +codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.nullToNonnullIsBad():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `nullToNonnullIsBad()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 60)] +codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.nullToNotAnnotatedIsBad():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `nullToNotAnnotatedIsBad()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 35)] +codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.nullableToNonnullIsBad(java.lang.String):java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `nullableToNonnullIsBad(...)` may return null but it is not annotated with `@Nullable`. (Origin: method parameter s)] +codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.nullableToNotAnnotatedIsBad(java.lang.String):java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `nullableToNotAnnotatedIsBad(...)` may return null but it is not annotated with `@Nullable`. (Origin: method parameter s)] codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.redundantEq():java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, WARNING, [Method `redundantEq()` is annotated with `@Nullable` but never returns null.] codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.redundantEq():java.lang.String, 2, ERADICATE_CONDITION_REDUNDANT_NONNULL, no_bucket, WARNING, [The condition s is always false according to the existing annotations.] codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.redundantNeq():java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, WARNING, [Method `redundantNeq()` is annotated with `@Nullable` but never returns null.] codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.redundantNeq():java.lang.String, 2, ERADICATE_CONDITION_REDUNDANT_NONNULL, no_bucket, WARNING, [The condition s is always true according to the existing annotations.] -codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.returnNull():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `returnNull()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 31)] -codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.returnNullable(java.lang.String):java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `returnNullable(...)` may return null but it is not annotated with `@Nullable`. (Origin: method parameter s)] -codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.return_null_in_catch():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `return_null_in_catch()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 106)] -codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.return_null_in_catch_after_throw():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `return_null_in_catch_after_throw()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 118)] -codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.tryWithResourcesReturnNullable(java.lang.String):java.lang.Object, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `tryWithResourcesReturnNullable(...)` may return null but it is not annotated with `@Nullable`. (Origin: call to returnNullOK() at line 88)] +codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.return_null_in_catch():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `return_null_in_catch()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 174)] +codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.return_null_in_catch_after_throw():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `return_null_in_catch_after_throw()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 186)] +codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.tryWithResourcesReturnNullable(java.lang.String):java.lang.Object, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `tryWithResourcesReturnNullable(...)` may return null but it is not annotated with `@Nullable`. (Origin: call to nullToNullableIsOK() at line 156)]