[nullsafe] Methods rendering the error accept TypeOrigin.t instead of string

Summary:
This diff also removes clumsy typeErr.origin_desc and improves interface
of TypeOrigin.

This will allow to render smarter error message, depending on the
context (see follow up diff).

Reviewed By: ngorogiannis

Differential Revision: D18527692

fbshipit-source-id: b7eb11db8

infer/src/nullsafe/AssignmentRule.ml

@@ -35,7 +35,8 @@ let check ~is_strict_mode ~lhs ~rhs =
   Result.ok_if_true is_allowed_assignment ~error:{is_strict_mode; lhs; rhs}
 
 
-let violation_description _ assignment_type ~rhs_origin_descr =
+let violation_description _ assignment_type ~rhs_origin =
+  let rhs_origin_descr = TypeOrigin.get_description rhs_origin |> Option.value ~default:"" in
   let module MF = MarkupFormatter in
   match assignment_type with
   | PassingParamToFunction {param_description; param_position; function_procname} ->

infer/src/nullsafe/AssignmentRule.mli

@@ -23,4 +23,4 @@ type assignment_type =
   | ReturningFromFunction of Typ.Procname.t
 [@@deriving compare]
 
-val violation_description : violation -> assignment_type -> rhs_origin_descr:string -> string
+val violation_description : violation -> assignment_type -> rhs_origin:TypeOrigin.t -> string

infer/src/nullsafe/DereferenceRule.ml

@@ -25,8 +25,11 @@ let check ~is_strict_mode nullability =
       Ok ()
 
 
-let violation_description _ dereference_type ~nullable_object_descr ~origin_descr =
+let violation_description _ dereference_type ~nullable_object_descr ~nullable_object_origin =
   let module MF = MarkupFormatter in
+  let origin_descr =
+    TypeOrigin.get_description nullable_object_origin |> Option.value ~default:""
+  in
   let nullable_object_descr =
     match dereference_type with
     | MethodCall _ | AccessToField _ -> (

infer/src/nullsafe/DereferenceRule.mli

@@ -25,5 +25,5 @@ val violation_description :
      violation
   -> dereference_type
   -> nullable_object_descr:string option
-  -> origin_descr:string
+  -> nullable_object_origin:TypeOrigin.t
   -> string

infer/src/nullsafe/InferredNullability.ml

@@ -21,15 +21,6 @@ let is_nonnull {nullability} =
   match nullability with Nullable -> false | DeclaredNonnull -> false | Nonnull -> true
 
 
-let descr_origin t =
-  let descr_opt = TypeOrigin.get_description t.origin in
-  match descr_opt with
-  | None ->
-      ("", None, None)
-  | Some (str, loc_opt, sig_opt) ->
-      ("(Origin: " ^ str ^ ")", loc_opt, sig_opt)
-
-
 let to_string {nullability} = Printf.sprintf "[%s]" (Nullability.to_string nullability)
 
 let join t1 t2 =

infer/src/nullsafe/InferredNullability.mli

@@ -27,11 +27,6 @@ val is_nonnull_or_declared_nonnull : t -> bool
 
 val is_nonnull : t -> bool
 
-val descr_origin : t -> TypeErr.origin_descr
-(** Human-readable description of the origin of a value.
-  (How did nullsafe infer the nullability )
- *)
-
 val get_origin : t -> TypeOrigin.t
 (** The simple explanation of how was nullability inferred.  *)
 

infer/src/nullsafe/eradicateChecks.ml

@@ -32,11 +32,11 @@ let check_object_dereference ~is_strict_mode tenv find_canonical_duplicate curr_
     (DereferenceRule.check ~is_strict_mode
        (InferredNullability.get_nullability inferred_nullability))
     ~f:(fun dereference_violation ->
-      let origin_descr = InferredNullability.descr_origin inferred_nullability in
+      let nullable_object_origin = InferredNullability.get_origin inferred_nullability in
       let nullable_object_descr = explain_expr tenv node object_exp in
       let type_error =
         TypeErr.Nullable_dereference
-          {dereference_violation; nullable_object_descr; dereference_type; origin_descr}
+          {dereference_violation; nullable_object_descr; dereference_type; nullable_object_origin}
       in
       report_error tenv find_canonical_duplicate type_error (Some instr_ref) loc curr_pname )
 
@@ -159,11 +159,11 @@ let check_field_assignment ~is_strict_mode tenv find_canonical_duplicate curr_pd
         && not (field_is_in_cleanup_context ())
       in
       if should_report then
-        let rhs_origin_descr = InferredNullability.descr_origin inferred_nullability_rhs in
+        let rhs_origin = InferredNullability.get_origin inferred_nullability_rhs in
         report_error tenv find_canonical_duplicate
           (TypeErr.Bad_assignment
              { assignment_violation
-             ; rhs_origin_descr
+             ; rhs_origin
              ; assignment_type= AssignmentRule.AssigningToField fname })
           (Some instr_ref) loc curr_pdesc )
 
@@ -326,11 +326,11 @@ let check_return_not_nullable ~is_strict_mode tenv find_canonical_duplicate loc
   let lhs = AnnotatedNullability.get_nullability ret_signature.ret_annotated_type.nullability in
   let rhs = InferredNullability.get_nullability ret_inferred_nullability in
   Result.iter_error (AssignmentRule.check ~is_strict_mode ~lhs ~rhs) ~f:(fun assignment_violation ->
-      let rhs_origin_descr = InferredNullability.descr_origin ret_inferred_nullability in
+      let rhs_origin = InferredNullability.get_origin ret_inferred_nullability in
       report_error tenv find_canonical_duplicate
         (TypeErr.Bad_assignment
            { assignment_violation
-           ; rhs_origin_descr
+           ; rhs_origin
            ; assignment_type= AssignmentRule.ReturningFromFunction curr_pname })
         None loc curr_pdesc )
 
@@ -436,11 +436,11 @@ let check_call_parameters ~is_strict_mode tenv find_canonical_duplicate curr_pde
         | None ->
             "formal parameter " ^ Mangled.to_string formal.mangled
       in
-      let rhs_origin_descr = InferredNullability.descr_origin nullability_actual in
+      let rhs_origin = InferredNullability.get_origin nullability_actual in
       report_error tenv find_canonical_duplicate
         (TypeErr.Bad_assignment
            { assignment_violation
-           ; rhs_origin_descr
+           ; rhs_origin
            ; assignment_type=
                AssignmentRule.PassingParamToFunction
                  {param_description; param_position; function_procname= callee_pname} })

infer/src/nullsafe/typeErr.ml

@@ -56,12 +56,6 @@ end
 
 (* InstrRef *)
 
-type origin_descr = string * Location.t option * AnnotatedSignature.t option
-
-(* callee signature *)
-(* ignore origin descr *)
-let compare_origin_descr _ _ = 0
-
 (** Instance of an error *)
 type err_instance =
   | Condition_redundant of (bool * string option)
@@ -78,11 +72,11 @@ type err_instance =
       { dereference_violation: DereferenceRule.violation
       ; dereference_type: DereferenceRule.dereference_type
       ; nullable_object_descr: string option
-      ; origin_descr: origin_descr }
+      ; nullable_object_origin: TypeOrigin.t }
   | Bad_assignment of
       { assignment_violation: AssignmentRule.violation
       ; assignment_type: AssignmentRule.assignment_type
-      ; rhs_origin_descr: origin_descr }
+      ; rhs_origin: TypeOrigin.t }
 [@@deriving compare]
 
 module H = Hashtbl.Make (struct
@@ -172,18 +166,18 @@ module Severity = struct
         None
 
 
-  let origin_descr_get_severity tenv origin_descr =
+  let origin_get_severity tenv origin =
-    match origin_descr with
+    match origin with
-    | _, _, Some signature ->
+    | TypeOrigin.MethodCall {annotated_signature} ->
-        this_type_get_severity tenv signature
+        this_type_get_severity tenv annotated_signature
-    | _, _, None ->
+    | _ ->
         None
 
 
   let err_instance_get_severity tenv err_instance : Exceptions.severity option =
     match err_instance with
-    | Nullable_dereference {origin_descr} ->
+    | Nullable_dereference {nullable_object_origin} ->
-        origin_descr_get_severity tenv origin_descr
+        origin_get_severity tenv nullable_object_origin
     | _ ->
         None
 end
@@ -261,16 +255,12 @@ let get_error_description err_instance =
       Format.asprintf "Field %a is not initialized in %s and is not declared %a" MF.pp_monospaced
         (Typ.Fieldname.to_simplified_string field_name)
         constructor_name MF.pp_monospaced nullable_annotation
-  | Bad_assignment {rhs_origin_descr= rhs_origin_descr, _, _; assignment_type; assignment_violation}
+  | Bad_assignment {rhs_origin; assignment_type; assignment_violation} ->
-    ->
+      AssignmentRule.violation_description assignment_violation assignment_type ~rhs_origin
-      AssignmentRule.violation_description assignment_violation assignment_type ~rhs_origin_descr
   | Nullable_dereference
-      { dereference_violation
+      {dereference_violation; nullable_object_descr; dereference_type; nullable_object_origin} ->
-      ; nullable_object_descr
-      ; dereference_type
-      ; origin_descr= origin_descr, _, _ } ->
       DereferenceRule.violation_description dereference_violation dereference_type
-        ~nullable_object_descr ~origin_descr
+        ~nullable_object_descr ~nullable_object_origin
   | Inconsistent_subclass
       {inheritance_violation; violation_type; base_proc_name; overridden_proc_name} ->
       InheritanceRule.violation_description inheritance_violation violation_type ~base_proc_name

infer/src/nullsafe/typeErr.mli

@@ -30,8 +30,6 @@ end
 (* InstrRefT *)
 module InstrRef : InstrRefT
 
-type origin_descr = string * Location.t option * AnnotatedSignature.t option
-
 (* callee signature *)
 
 (** Instance of an error *)
@@ -50,11 +48,11 @@ type err_instance =
       { dereference_violation: DereferenceRule.violation
       ; dereference_type: DereferenceRule.dereference_type
       ; nullable_object_descr: string option
-      ; origin_descr: origin_descr }
+      ; nullable_object_origin: TypeOrigin.t }
   | Bad_assignment of
       { assignment_violation: AssignmentRule.violation
       ; assignment_type: AssignmentRule.assignment_type
-      ; rhs_origin_descr: origin_descr }
+      ; rhs_origin: TypeOrigin.t }
 [@@deriving compare]
 
 val node_reset_forall : Procdesc.Node.t -> unit

infer/src/nullsafe/typeOrigin.ml

@@ -104,19 +104,16 @@ let rec to_string = function
       "Undef"
 
 
-let get_description origin =
+let get_description_impl origin =
   let atline loc = " at line " ^ string_of_int loc.Location.line in
   match origin with
   | NullConst loc ->
-      Some ("null constant" ^ atline loc, Some loc, None)
+      Some ("null constant" ^ atline loc)
   | Field {field_name; access_loc} ->
-      Some
+      Some ("field " ^ Typ.Fieldname.to_simplified_string field_name ^ atline access_loc)
-        ( "field " ^ Typ.Fieldname.to_simplified_string field_name ^ atline access_loc
-        , Some access_loc
-        , None )
   | MethodParameter {mangled} ->
-      Some ("method parameter " ^ Mangled.to_string mangled, None, None)
+      Some ("method parameter " ^ Mangled.to_string mangled)
-  | MethodCall {pname; call_loc; annotated_signature} ->
+  | MethodCall {pname; call_loc} ->
       let modelled_in =
         (* TODO(T54088319) don't calculate this info and propagate it from AnnotatedNullability instead *)
         if Models.is_modelled_for_nullability_as_internal pname then
@@ -128,7 +125,7 @@ let get_description origin =
           (Typ.Procname.to_simplified_string pname)
           modelled_in (atline call_loc)
       in
-      Some (description, Some call_loc, Some annotated_signature)
+      Some description
   (* These are origins of non-nullable expressions that are result of evaluating of some rvalue.
      Because they are non-nullable and they are rvalues, we won't get normal type violations
      With them. All we could get is things like condition redundant or overannotated.
@@ -142,6 +139,11 @@ let get_description origin =
       None
 
 
+let get_description origin =
+  get_description_impl origin
+  |> Option.map ~f:(fun description -> Format.sprintf "(Origin: %s)" description)
+
+
 let join o1 o2 =
   match (o1, o2) with
   (* left priority *)

infer/src/nullsafe/typeOrigin.mli

@@ -46,7 +46,7 @@ val equal : t -> t -> bool
 
 val get_nullability : t -> Nullability.t
 
-val get_description : t -> TypeErr.origin_descr option
+val get_description : t -> string option
 (** Get a description to be used for error messages. *)
 
 val join : t -> t -> t