From 3442ce1999fa0c138739cfbe5d84bdf2361baa9d Mon Sep 17 00:00:00 2001 From: Jeremy Dubreil Date: Mon, 17 Sep 2018 09:38:46 -0700 Subject: [PATCH] [eradicate] no longer report Inconsistent Subclass Return Annotation when overriding external methods Summary: No longer report inconsistencies with the annotations with subtyping when the super class is in an external packages since those warnings are not necessarily accurate or actionable. Reviewed By: ezgicicek Differential Revision: D9845098 fbshipit-source-id: 1f2bcd739 --- infer/src/eradicate/eradicateChecks.ml | 11 ++++++++--- .../java/checkers/NullableSuggest.java | 4 ++-- .../codetoanalyze/java/eradicate/.inferconfig | 5 +++++ .../InconsistentSubclassAnnotation.java | 16 ++++++++++++++++ .../tests/codetoanalyze/java/eradicate/Makefile | 2 +- .../codetoanalyze/java/eradicate/issues.exp | 3 +++ .../{SomeClass.java => SomeExternalClass.java} | 9 ++++++++- 7 files changed, 43 insertions(+), 7 deletions(-) create mode 100644 infer/tests/codetoanalyze/java/eradicate/.inferconfig rename infer/tests/external/library/{SomeClass.java => SomeExternalClass.java} (56%) diff --git a/infer/src/eradicate/eradicateChecks.ml b/infer/src/eradicate/eradicateChecks.ml index d1ff02cb2..aea84c9da 100644 --- a/infer/src/eradicate/eradicateChecks.ml +++ b/infer/src/eradicate/eradicateChecks.ml @@ -478,10 +478,15 @@ let check_overridden_annotations find_canonical_duplicate tenv proc_name proc_de in let check overriden_proc_name = match Summary.proc_resolve_attributes overriden_proc_name with - | Some attributes -> + | Some attributes -> ( let overridden_signature = Models.get_modelled_annotated_signature attributes in - check_return overriden_proc_name overridden_signature ; - check_params overriden_proc_name overridden_signature + check_params overriden_proc_name overridden_signature ; + (* the analysis should not report return type inconsistencies with external code *) + match overriden_proc_name with + | Typ.Procname.Java java_pname when not (Typ.Procname.Java.is_external java_pname) -> + check_return overriden_proc_name overridden_signature + | _ -> + () ) | None -> () in diff --git a/infer/tests/codetoanalyze/java/checkers/NullableSuggest.java b/infer/tests/codetoanalyze/java/checkers/NullableSuggest.java index 5dbac54f3..938b421a1 100644 --- a/infer/tests/codetoanalyze/java/checkers/NullableSuggest.java +++ b/infer/tests/codetoanalyze/java/checkers/NullableSuggest.java @@ -7,7 +7,7 @@ package codetoanalyze.java.checkers; -import external.library.SomeClass; +import external.library.SomeExternalClass; import javax.annotation.Nullable; public class NullableSuggest { @@ -126,7 +126,7 @@ public class NullableSuggest { }; } - boolean checkExternalFieldForNullOk(SomeClass parameter) { + boolean checkExternalFieldForNullOk(SomeExternalClass parameter) { if (parameter.field == null) { // Does not report here. The field belongs to an external library so the // warning would not be actionable. diff --git a/infer/tests/codetoanalyze/java/eradicate/.inferconfig b/infer/tests/codetoanalyze/java/eradicate/.inferconfig new file mode 100644 index 000000000..984517f70 --- /dev/null +++ b/infer/tests/codetoanalyze/java/eradicate/.inferconfig @@ -0,0 +1,5 @@ +{ + "external-java-packages": [ + "external." + ] +} diff --git a/infer/tests/codetoanalyze/java/eradicate/InconsistentSubclassAnnotation.java b/infer/tests/codetoanalyze/java/eradicate/InconsistentSubclassAnnotation.java index 5546b1c31..a409a342f 100644 --- a/infer/tests/codetoanalyze/java/eradicate/InconsistentSubclassAnnotation.java +++ b/infer/tests/codetoanalyze/java/eradicate/InconsistentSubclassAnnotation.java @@ -7,6 +7,7 @@ package codetoanalyze.java.eradicate; +import external.library.SomeExternalClass; import javax.annotation.Nullable; class SubclassExample { @@ -94,6 +95,21 @@ class ConstructorsAreExcluded { } } +class ExtendsExternalLibrary extends SomeExternalClass { + + @Override + public @Nullable Object externalMethod1() { + // subtyping error on the return type not reported as we cannot + // rely on the external libraries to be correctly annotated + return null; + } + + @Override + public void externalMethod2(Object object) { + // subtyping error on the parameter type are reported + } +} + public class InconsistentSubclassAnnotation implements InconsistentSubclassAnnotationInterface { public static void callFromSuperclass(SubclassExample.A a) { diff --git a/infer/tests/codetoanalyze/java/eradicate/Makefile b/infer/tests/codetoanalyze/java/eradicate/Makefile index 3be5129dd..c0b81a5fb 100644 --- a/infer/tests/codetoanalyze/java/eradicate/Makefile +++ b/infer/tests/codetoanalyze/java/eradicate/Makefile @@ -13,6 +13,6 @@ INFER_OPTIONS = \ --eradicate-condition-redundant \ --debug-exceptions INFERPRINT_OPTIONS = --issues-tests -SOURCES = $(wildcard *.java) +SOURCES = $(wildcard *.java) $(wildcard $(TESTS_DIR)/external/library/*.java) include $(TESTS_DIR)/javac.make diff --git a/infer/tests/codetoanalyze/java/eradicate/issues.exp b/infer/tests/codetoanalyze/java/eradicate/issues.exp index d75c6410f..8c3cfe87a 100644 --- a/infer/tests/codetoanalyze/java/eradicate/issues.exp +++ b/infer/tests/codetoanalyze/java/eradicate/issues.exp @@ -27,6 +27,7 @@ codetoanalyze/java/eradicate/FieldNotNullable.java, codetoanalyze.java.eradicate codetoanalyze/java/eradicate/FieldNotNullable.java, codetoanalyze.java.eradicate.NestedFieldAccess$TestFunctionsIdempotent.NestedBAD3():void, 2, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [origin,Field `NestedFieldAccess$TestFunctionsIdempotent.dontAssignNull` can be null but is not declared `@Nullable`. (Origin: call to getS(...) at line 285)] codetoanalyze/java/eradicate/FieldNotNullable.java, codetoanalyze.java.eradicate.NestedFieldAccess$TestPut.putNull(java.util.Map,java.lang.String):void, 2, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [origin,`Map.put(...)` needs a non-null value in parameter 2 but argument `null` can be null. (Origin: null constant at line 323)] codetoanalyze/java/eradicate/FieldNotNullable.java, codetoanalyze.java.eradicate.NestedFieldAccess$TestPut.putNull(java.util.Map,java.lang.String):void, 3, ERADICATE_FIELD_NOT_NULLABLE, no_bucket, WARNING, [origin,Field `NestedFieldAccess$TestPut.dontAssignNull` can be null but is not declared `@Nullable`. (Origin: null constant at line 323)] +codetoanalyze/java/eradicate/InconsistentSubclassAnnotation.java, codetoanalyze.java.eradicate.ExtendsExternalLibrary.externalMethod2(java.lang.Object):void, 0, ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION, no_bucket, WARNING, [First parameter `object` of method `ExtendsExternalLibrary.externalMethod2(...)` is not `@Nullable` but is declared `@Nullable`in the parent class method `SomeExternalClass.externalMethod2(...)`.] codetoanalyze/java/eradicate/InconsistentSubclassAnnotation.java, codetoanalyze.java.eradicate.InconsistentSubclassAnnotation.implementInAnotherFile(java.lang.String):java.lang.String, 0, ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION, no_bucket, WARNING, [First parameter `s` of method `InconsistentSubclassAnnotation.implementInAnotherFile(...)` is not `@Nullable` but is declared `@Nullable`in the parent class method `InconsistentSubclassAnnotationInterface.implementInAnotherFile(...)`.] codetoanalyze/java/eradicate/InconsistentSubclassAnnotation.java, codetoanalyze.java.eradicate.SubclassExample$B.foo():codetoanalyze.java.eradicate.SubclassExample$T, 0, ERADICATE_INCONSISTENT_SUBCLASS_RETURN_ANNOTATION, no_bucket, WARNING, [Method `SubclassExample$B.foo()` is annotated with `@Nullable` but overrides unannotated method `SubclassExample$A.foo()`.] codetoanalyze/java/eradicate/InconsistentSubclassAnnotation.java, codetoanalyze.java.eradicate.SubclassExample$C.baz():codetoanalyze.java.eradicate.SubclassExample$T, 0, ERADICATE_INCONSISTENT_SUBCLASS_RETURN_ANNOTATION, no_bucket, WARNING, [Method `SubclassExample$C.baz()` is annotated with `@Nullable` but overrides unannotated method `SubclassExample$I.baz()`.] @@ -83,3 +84,5 @@ codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicat codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.return_null_in_catch():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [origin,Method `return_null_in_catch()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 109)] 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, [origin,Method `return_null_in_catch_after_throw()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 121)] codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.tryWithResourcesReturnNullable(java.lang.String):java.lang.Object, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [origin,Method `tryWithResourcesReturnNullable(...)` may return null but it is not annotated with `@Nullable`. (Origin: call to returnNullOK() at line 90)] +external/library/SomeExternalClass.java, external.library.SomeExternalClass.(), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `SomeExternalClass.field` is not initialized in the constructor and is not declared `@Nullable`] +external/library/SomeExternalClass.java, external.library.SomeExternalClass.externalMethod1():java.lang.Object, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [origin,Method `externalMethod1()` may return null but it is not annotated with `@Nullable`. (Origin: null constant at line 16)] diff --git a/infer/tests/external/library/SomeClass.java b/infer/tests/external/library/SomeExternalClass.java similarity index 56% rename from infer/tests/external/library/SomeClass.java rename to infer/tests/external/library/SomeExternalClass.java index d2d603e4e..4590e9b99 100644 --- a/infer/tests/external/library/SomeClass.java +++ b/infer/tests/external/library/SomeExternalClass.java @@ -7,7 +7,14 @@ package external.library; +import javax.annotation.Nullable; -public class SomeClass { +public class SomeExternalClass { public Object field; + + public Object externalMethod1() { + return null; + } + + public void externalMethod2(@Nullable Object object) {} }