[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

@@ -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

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.

infer/tests/codetoanalyze/java/eradicate/.inferconfig

@@ -0,0 +1,5 @@
+{
+  "external-java-packages": [
+    "external."
+  ]
+}

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) {

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

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.<init>(), 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)]

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) {}
 }