[nullsafe] @Initializer annotation is inherited from parent signatures

Summary:
We already take into account inheritance if the method inherits a known
modelled initializer method (e.g. Activity.onCreate()).

But if the method is explicitly marked as Initializer, we require its
overrides to be also marked.

This diffs fixes the behavior and makes it consistent, now this is
enough to annotate only parent class.

Reviewed By: jvillard

Differential Revision: D23135177

fbshipit-source-id: a21ff4a0e

infer/src/nullsafe/Initializers.ml

@@ -103,24 +103,31 @@ let get_class pn =
   match pn with Procname.Java pn_java -> Some (Procname.Java.get_class_name pn_java) | _ -> None
 
 
+let is_annotated_initializer tenv proc_name =
+  PatternMatch.lookup_attributes tenv proc_name
+  |> Option.value_map
+       ~f:(fun ProcAttributes.{method_annotation= {return}} -> Annotations.ia_is_initializer return)
+       ~default:false
+
+
+let is_annotated_initializer_in_chain tenv proc_name =
+  PatternMatch.override_exists (is_annotated_initializer tenv) tenv proc_name
+
+
+(* Should the (non-constructor) function be considered an initializer method *)
+let is_initializer tenv proc_attributes =
+  (* Either modelled as initializer or the descendent of such a method *)
+  PatternMatch.Java.method_is_initializer tenv proc_attributes
+  || (* Or explicitly marked @Initializer or the descendent of such a method *)
+  is_annotated_initializer_in_chain tenv proc_attributes.ProcAttributes.proc_name
+
+
 (** Typestates after the current procedure and all initializer procedures. *)
 let final_initializer_typestates_lazy tenv curr_pname curr_pdesc typecheck_proc =
   lazy
-    (let is_initializer proc_attributes =
+    (let initializers_current_class =
-       PatternMatch.Java.method_is_initializer tenv proc_attributes
-       ||
-       let ia =
-         (* TODO(T62825735): support trusted callees for fields *)
-         (Models.get_modelled_annotated_signature ~is_callee_in_trust_list:false tenv
-            proc_attributes)
-           .AnnotatedSignature.ret
-           .ret_annotation_deprecated
-       in
-       Annotations.ia_is_initializer ia
-     in
-     let initializers_current_class =
        pname_and_pdescs_with tenv curr_pname (function pname, proc_attributes ->
-           is_initializer proc_attributes
+           is_initializer tenv proc_attributes
            && equal_class_opt (get_class pname) (get_class curr_pname) )
      in
      final_typestates ((curr_pname, curr_pdesc) :: initializers_current_class) tenv typecheck_proc

infer/tests/codetoanalyze/java/nullsafe/FieldNotInitialized.java

@@ -328,4 +328,65 @@ class TestInitializerAnnotation {
     // return some meaninful object
     return "";
   }
+
+  abstract class TestFieldNotInitializedBase {
+    @Initializer
+    protected abstract void markedInitializerInBase();
+
+    protected abstract void notMarkedInitializerInBase1();
+
+    protected abstract void notMarkedInitializerInBase2();
+  }
+
+  // Test to ensure we respect parent @Initializer annotations
+  class TestFieldNotInitializedDerived extends TestFieldNotInitializedBase {
+    private String field1_OK;
+    private String field2_BAD;
+    private String field3_OK;
+
+    @Override
+    public void markedInitializerInBase() {
+      // OK: implicitly @Initializer (inherited from the base)
+      field1_OK = "";
+    }
+
+    @Override
+    public void notMarkedInitializerInBase1() {
+      // BAD: field not initialized
+      field2_BAD = "";
+    }
+
+    @Override
+    @Initializer
+    public void notMarkedInitializerInBase2() {
+      // OK: explicitly marked as an initializer
+      field3_OK = "";
+    }
+  }
+
+  // Ensure that chains with non-trivial length work as well
+  class TestFieldNotInitializedDerivedDerived extends TestFieldNotInitializedDerived {
+    private String field1_OK;
+    private String field2_BAD;
+    private String field3_OK;
+
+    @Override
+    public void markedInitializerInBase() {
+      // OK: implicitly @Initializer (inherited from the base through the chain)
+      field1_OK = "";
+    }
+
+    @Override
+    public void notMarkedInitializerInBase1() {
+      // BAD: field not initialized, and the method is not marked as @Initializer in any of the
+      // bases
+      field2_BAD = "";
+    }
+
+    @Override
+    public void notMarkedInitializerInBase2() {
+      // OK: explicitly marked as an initializer in the direct base
+      field3_OK = "";
+    }
+  }
 }

infer/tests/codetoanalyze/java/nullsafe/issues.exp

@@ -37,7 +37,7 @@ codetoanalyze/java/nullsafe/ConditionRedundant.java, codetoanalyze.java.nullsafe
 codetoanalyze/java/nullsafe/ConditionRedundant.java, codetoanalyze.java.nullsafe.ConditionRedundant.testFlowSensitivity(java.lang.String,java.lang.String):void, 4, ERADICATE_CONDITION_REDUNDANT, no_bucket, ADVICE, [The condition nullable1 might be always true according to the existing annotations.]
 codetoanalyze/java/nullsafe/FieldNotInitialized.java, Linters_dummy_method, 25, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], FieldNotInitialized, codetoanalyze.java.nullsafe, issues: 31, curr_mode: "Default"
 codetoanalyze/java/nullsafe/FieldNotInitialized.java, Linters_dummy_method, 233, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], TestKnownInitializers, codetoanalyze.java.nullsafe, issues: 2, curr_mode: "Default"
-codetoanalyze/java/nullsafe/FieldNotInitialized.java, Linters_dummy_method, 280, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], TestInitializerAnnotation, codetoanalyze.java.nullsafe, issues: 3, curr_mode: "Default"
+codetoanalyze/java/nullsafe/FieldNotInitialized.java, Linters_dummy_method, 280, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], TestInitializerAnnotation, codetoanalyze.java.nullsafe, issues: 5, curr_mode: "Default"
 codetoanalyze/java/nullsafe/FieldNotInitialized.java, codetoanalyze.java.nullsafe.FieldNotInitialized$InitCircular.<init>(codetoanalyze.java.nullsafe.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `stillBad` is declared non-nullable, so it should be initialized in the constructor or in an `@Initializer` method]
 codetoanalyze/java/nullsafe/FieldNotInitialized.java, codetoanalyze.java.nullsafe.FieldNotInitialized$InitCircular.<init>(codetoanalyze.java.nullsafe.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `bad` is declared non-nullable, so it should be initialized in the constructor or in an `@Initializer` method]
 codetoanalyze/java/nullsafe/FieldNotInitialized.java, codetoanalyze.java.nullsafe.FieldNotInitialized$InitIfNull.<init>(codetoanalyze.java.nullsafe.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `shouldBeGood_FIXME` is declared non-nullable, so it should be initialized in the constructor or in an `@Initializer` method]
@@ -63,6 +63,8 @@ codetoanalyze/java/nullsafe/FieldNotInitialized.java, codetoanalyze.java.nullsaf
 codetoanalyze/java/nullsafe/FieldNotInitialized.java, codetoanalyze.java.nullsafe.FieldNotInitialized$Suppressions.<init>(codetoanalyze.java.nullsafe.FieldNotInitialized,int), 2, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [`FieldNotInitialized$Suppressions.f(...)`: parameter #1(`a`) is declared non-nullable but the argument is `null`.]
 codetoanalyze/java/nullsafe/FieldNotInitialized.java, codetoanalyze.java.nullsafe.FieldNotInitialized$Suppressions.<init>(codetoanalyze.java.nullsafe.FieldNotInitialized,int,int,int), 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [`FieldNotInitialized$Suppressions.f(...)`: parameter #1(`a`) is declared non-nullable but the argument is `null`.]
 codetoanalyze/java/nullsafe/FieldNotInitialized.java, codetoanalyze.java.nullsafe.FieldNotInitialized$WriteItselfIsBAD.<init>(codetoanalyze.java.nullsafe.FieldNotInitialized), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `bad` is declared non-nullable, so it should be initialized in the constructor or in an `@Initializer` method]
+codetoanalyze/java/nullsafe/FieldNotInitialized.java, codetoanalyze.java.nullsafe.TestInitializerAnnotation$TestFieldNotInitializedDerived.<init>(codetoanalyze.java.nullsafe.TestInitializerAnnotation), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `field2_BAD` is declared non-nullable, so it should be initialized in the constructor or in an `@Initializer` method]
+codetoanalyze/java/nullsafe/FieldNotInitialized.java, codetoanalyze.java.nullsafe.TestInitializerAnnotation$TestFieldNotInitializedDerivedDerived.<init>(codetoanalyze.java.nullsafe.TestInitializerAnnotation), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `field2_BAD` is declared non-nullable, so it should be initialized in the constructor or in an `@Initializer` method]
 codetoanalyze/java/nullsafe/FieldNotInitialized.java, codetoanalyze.java.nullsafe.TestInitializerAnnotation.<init>(), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `dontInitAtAllIsBAD` is declared non-nullable, so it should be initialized in the constructor or in an `@Initializer` method]
 codetoanalyze/java/nullsafe/FieldNotInitialized.java, codetoanalyze.java.nullsafe.TestInitializerAnnotation.<init>(), 0, ERADICATE_FIELD_NOT_INITIALIZED, no_bucket, WARNING, [Field `initInAnyOtherMethodIsBAD` is declared non-nullable, so it should be initialized in the constructor or in an `@Initializer` method]
 codetoanalyze/java/nullsafe/FieldNotInitialized.java, codetoanalyze.java.nullsafe.TestInitializerAnnotation.build():java.lang.Object, 5, ERADICATE_CONDITION_REDUNDANT, no_bucket, ADVICE, [The condition TestInitializerAnnotation.initInInitilizerMethod1IsOK might be always true according to the existing annotations.]