[nullsafe][annotation graph] Account for virtual params

Summary:
Virtual "this" invisible param exists in the annotated signature, but
does not exist in some other places, which causes a lot of annoyance in
different places.

This diff does not intend to solve all this, but makes one step forward.
1/ AnnotatedNullability now explicitly distincts normal params and
VirtualThis.
2/ AnnotatedSignature accounts for this via: a) not having redundant
fake annotation points b) having corrent param indices (those were off
by 1 in non-virtul methods).

Reviewed By: artempyanykh

Differential Revision: D24726480

fbshipit-source-id: fdb8bb0fb

infer/src/nullsafe/AnnotatedNullability.ml

@@ -39,6 +39,7 @@ and strict_nonnull_origin =
   | ModelledNonnull
   | StrictMode
   | PrimitiveType
+  | ImplicitThis
   | EnumValue
   | SyntheticField
 [@@deriving compare]
@@ -85,6 +86,8 @@ let pp fmt t =
         "strict"
     | PrimitiveType ->
         "primitive"
+    | ImplicitThis ->
+        "implicit_this"
     | EnumValue ->
         "enum"
     | SyntheticField ->
@@ -162,6 +165,8 @@ let can_be_considered_for_provisional_annotation = function
       (* models correspond to code beyond control *) false
   | StrictNonnull PrimitiveType ->
       (* primitive type can not be annotated *) false
+  | StrictNonnull ImplicitThis ->
+      (* a synthetic param *) false
   | StrictNonnull EnumValue ->
       (* by design non-nullable *) false
   | StrictNonnull SyntheticField ->

infer/src/nullsafe/AnnotatedNullability.mli

@@ -60,6 +60,7 @@ and strict_nonnull_origin =
   | ModelledNonnull  (** nullsafe knows it is non-nullable via its internal models *)
   | StrictMode  (** under strict mode we consider non-null declarations to be trusted *)
   | PrimitiveType  (** Primitive types are non-nullable by language design *)
+  | ImplicitThis  (** Implicit `this` param for virtual non-static methods *)
   | EnumValue
       (** Java enum value are statically initialized with non-nulls according to language semantics *)
   | SyntheticField

infer/src/nullsafe/AnnotatedSignature.ml

@@ -68,42 +68,67 @@ let nullability_for_return ~proc_name ~is_callee_in_trust_list ~nullsafe_mode ~i
   final_nullability
 
 
-let nullability_for_param ~proc_name ~param_num ~is_callee_in_trust_list ~nullsafe_mode
-    ~is_third_party ~is_provisional_annotation_mode param_type param_annotations =
-  let nullability =
-    AnnotatedNullability.of_type_and_annotation ~is_callee_in_trust_list ~nullsafe_mode
-      ~is_third_party param_type param_annotations
-  in
-  if
-    is_provisional_annotation_mode
-    && AnnotatedNullability.can_be_considered_for_provisional_annotation nullability
-  then
-    AnnotatedNullability.ProvisionallyNullable
-      (ProvisionalAnnotation.Param {method_info= proc_name; num= param_num})
-  else nullability
-
-
 (* Given annotations for method signature, extract nullability information
    for return type and params *)
-let extract_nullability ~proc_name ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party
-    ~is_provisional_annotation_mode ret_type ret_annotations param_annotated_types =
-  let params_nullability =
-    List.mapi param_annotated_types ~f:(fun param_num (param_type, param_annotations) ->
-        nullability_for_param ~proc_name ~param_num ~is_callee_in_trust_list ~nullsafe_mode
-          ~is_third_party ~is_provisional_annotation_mode param_type param_annotations )
-  in
+let extract_for_ret ~proc_name ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party
+    ~is_provisional_annotation_mode ret_type ret_annotations param_info =
   let has_propagates_nullable_in_param =
-    List.exists params_nullability ~f:(function
-      | AnnotatedNullability.Nullable AnnotatedNullability.AnnotatedPropagatesNullable ->
-          true
-      | _ ->
-          false )
+    List.exists param_info ~f:(fun {param_annotated_type= {nullability}} ->
+        match nullability with
+        | AnnotatedNullability.Nullable AnnotatedNullability.AnnotatedPropagatesNullable ->
+            true
+        | _ ->
+            false )
   in
   let return_nullability =
     nullability_for_return ~proc_name ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party
       ret_type ~is_provisional_annotation_mode ret_annotations ~has_propagates_nullable_in_param
   in
-  (return_nullability, params_nullability)
+  { ret_annotation_deprecated= ret_annotations
+  ; ret_annotated_type= AnnotatedType.{nullability= return_nullability; typ= ret_type} }
+
+
+let get_param_nullability ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party param_name
+    param_type param_annotations =
+  if Mangled.is_this param_name then AnnotatedNullability.StrictNonnull ImplicitThis
+  else
+    AnnotatedNullability.of_type_and_annotation ~is_callee_in_trust_list ~nullsafe_mode
+      ~is_third_party param_type param_annotations
+
+
+(* When getting param indices, we might need to offset to account for synthetic virtual params *)
+let get_param_index_offset param_nullabilities =
+  match param_nullabilities with
+  | AnnotatedNullability.StrictNonnull ImplicitThis :: _ ->
+      1
+  | _ ->
+      0
+
+
+let correct_by_provisional_annotations ~proc_name param_nullabilities =
+  let index_offset = get_param_index_offset param_nullabilities in
+  List.mapi param_nullabilities ~f:(fun param_index nullability ->
+      if AnnotatedNullability.can_be_considered_for_provisional_annotation nullability then
+        AnnotatedNullability.ProvisionallyNullable
+          (ProvisionalAnnotation.Param {method_info= proc_name; num= param_index - index_offset})
+      else nullability )
+
+
+let extract_for_params ~proc_name ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party
+    ~is_provisional_annotation_mode param_info =
+  let param_nullability =
+    List.map param_info ~f:(fun (param_name, typ, annotations) ->
+        get_param_nullability ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party param_name typ
+          annotations )
+  in
+  let corrected_nullability =
+    if is_provisional_annotation_mode then
+      correct_by_provisional_annotations ~proc_name param_nullability
+    else param_nullability
+  in
+  List.map2_exn param_info corrected_nullability
+    ~f:(fun (mangled, typ, param_annotation_deprecated) nullability ->
+      {param_annotation_deprecated; mangled; param_annotated_type= AnnotatedType.{nullability; typ}} )
 
 
 let get_impl ~is_callee_in_trust_list ~nullsafe_mode ~is_provisional_annotation_mode
@@ -118,24 +143,17 @@ let get_impl ~is_callee_in_trust_list ~nullsafe_mode ~is_provisional_annotation_
       (ThirdPartyAnnotationGlobalRepo.get_repo ())
       proc_name
   in
-  let params_with_annotations = ProcAttributes.get_annotated_formals proc_attributes in
-  let param_annotated_types =
-    List.map params_with_annotations ~f:(fun ((_, typ), annotations) -> (typ, annotations))
+  let param_info =
+    ProcAttributes.get_annotated_formals proc_attributes
+    |> List.map ~f:(fun ((a, b), c) -> (a, b, c))
   in
-  let return_nullability, params_nullability =
-    extract_nullability ~proc_name ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party
-      ~is_provisional_annotation_mode ret_type ret_annotation param_annotated_types
+  let params =
+    extract_for_params ~proc_name ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party
+      ~is_provisional_annotation_mode param_info
   in
   let ret =
-    { ret_annotation_deprecated= ret_annotation
-    ; ret_annotated_type= AnnotatedType.{nullability= return_nullability; typ= ret_type} }
-  in
-  let params =
-    List.map2_exn params_with_annotations params_nullability
-      ~f:(fun ((mangled, typ), param_annotation_deprecated) nullability ->
-        { param_annotation_deprecated
-        ; mangled
-        ; param_annotated_type= AnnotatedType.{nullability; typ} } )
+    extract_for_ret ~proc_name ~is_callee_in_trust_list ~nullsafe_mode ~is_third_party
+      ~is_provisional_annotation_mode ret_type ret_annotation params
   in
   let kind = if is_third_party then ThirdParty Unregistered else FirstParty in
   {nullsafe_mode; kind; ret; params}

infer/tests/codetoanalyze/java/nullsafe-annotation-graph/issues.exp

@@ -19,20 +19,20 @@ AnnotationGraph:
     kind: Field
     field_name: fieldD
     num_violations: 1
-    dependent_point_ids: [m4]
+    dependent_point_ids: [m3]
     
   Annotation point:
-    id: m4
+    id: m3
     kind: Method
     method_info:
       method_name: methodA
       params: java.lang.String, boolean
       access_level: Private
     num_violations: 0
-    dependent_point_ids: [m7]
+    dependent_point_ids: [m5]
     
   Annotation point:
-    id: m7
+    id: m5
     kind: Method
     method_info:
       method_name: methodB
@@ -42,7 +42,7 @@ AnnotationGraph:
     dependent_point_ids: []
     
   Annotation point:
-    id: m9
+    id: m6
     kind: Method
     method_info:
       method_name: methodC
@@ -52,92 +52,15 @@ AnnotationGraph:
     dependent_point_ids: []
     
   Annotation point:
-    id: p10
-    kind: Param
-    method_info:
-      method_name: methodC
-      params: 
-      access_level: Public
-    param_num: 0
-    num_violations: 0
-    dependent_point_ids: []
-    
-  Annotation point:
-    id: p11
-    kind: Param
-    method_info:
-      method_name: methodD
-      params: 
-      access_level: Private
-    param_num: 0
-    num_violations: 0
-    dependent_point_ids: []
-    
-  Annotation point:
-    id: p12
-    kind: Param
-    method_info:
-      method_name: methodE
-      params: 
-      access_level: Private
-    param_num: 0
-    num_violations: 0
-    dependent_point_ids: []
-    
-  Annotation point:
-    id: p13
-    kind: Param
-    method_info:
-      method_name: methodF
-      params: 
-      access_level: Private
-    param_num: 0
-    num_violations: 0
-    dependent_point_ids: []
-    
-  Annotation point:
-    id: p3
-    kind: Param
-    method_info:
-      method_name: <init>
-      params: 
-      access_level: Public
-    param_num: 0
-    num_violations: 0
-    dependent_point_ids: []
-    
-  Annotation point:
-    id: p5
-    kind: Param
-    method_info:
-      method_name: methodA
-      params: java.lang.String, boolean
-      access_level: Private
-    param_num: 0
-    num_violations: 0
-    dependent_point_ids: []
-    
-  Annotation point:
-    id: p6
+    id: p4
     kind: Param
     method_info:
       method_name: methodA
       params: java.lang.String, boolean
       access_level: Private
-    param_num: 1
-    num_violations: 0
-    dependent_point_ids: [f0, m4]
-    
-  Annotation point:
-    id: p8
-    kind: Param
-    method_info:
-      method_name: methodB
-      params: 
-      access_level: Private
     param_num: 0
     num_violations: 0
-    dependent_point_ids: []
+    dependent_point_ids: [f0, m3]
     
   
 codetoanalyze/java/nullsafe-annotation-graph/AnnotationGraph.java, Linters_dummy_method, 12, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], AnnotationGraph, codetoanalyze.java.nullsafe_annotation_graph, issues: 3, curr_mode: "Default"
@@ -146,17 +69,6 @@ AnnotationGraph:
   Annotation point:
     id: p0
     kind: Param
-    method_info:
-      method_name: <init>
-      params: 
-      access_level: Default
-    param_num: 0
-    num_violations: 0
-    dependent_point_ids: []
-    
-  Annotation point:
-    id: p1
-    kind: Param
     method_info:
       method_name: doesNotAcceptNull
       params: java.lang.String