From 327aa001c3b2b4a964df795f840bab931d6b0caa Mon Sep 17 00:00:00 2001
From: Mitya Lyubarskiy <mityal@fb.com>
Date: Fri, 24 Jan 2020 07:20:02 -0800
Subject: [PATCH] [nullsafe] Propagate type origin to condition redundant type
 error

Summary:
This is/can be useful for future changes that make the reporting more
precise based on the exact origin.

See the follow up diff as an example.

Reviewed By: artempyanykh

Differential Revision: D19553567

fbshipit-source-id: c2b2c28a1
---
 infer/src/nullsafe/eradicateChecks.ml | 8 +++++++-
 infer/src/nullsafe/typeCheck.ml       | 5 ++++-
 infer/src/nullsafe/typeErr.ml         | 8 +++++---
 infer/src/nullsafe/typeErr.mli        | 3 ++-
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml
index 0a9129215..a142a68a2 100644
--- a/infer/src/nullsafe/eradicateChecks.ml
+++ b/infer/src/nullsafe/eradicateChecks.ml
@@ -106,8 +106,14 @@ let check_condition tenv case_zero find_canonical_duplicate curr_pdesc node e ty
   in
   let is_always_true = not case_zero in
   if should_report then
+    (* TODO(T61051649) this is not really condition. This is an expression.
+       This leads to inconsistent messaging like "condition `some_variable` is always true".
+       So Condition_redundant should either accept expression, or this should pass a condition.
+    *)
+    let condition_descr = explain_expr tenv node e in
+    let nonnull_origin = InferredNullability.get_origin inferred_nullability in
     report_error tenv find_canonical_duplicate
-      (TypeErr.Condition_redundant (is_always_true, explain_expr tenv node e))
+      (TypeErr.Condition_redundant {is_always_true; condition_descr; nonnull_origin})
       (Some instr_ref) loc curr_pdesc
 
 
diff --git a/infer/src/nullsafe/typeCheck.ml b/infer/src/nullsafe/typeCheck.ml
index 60641202f..6a74ca543 100644
--- a/infer/src/nullsafe/typeCheck.ml
+++ b/infer/src/nullsafe/typeCheck.ml
@@ -455,7 +455,10 @@ let do_preconditions_check_not_null instr_ref tenv find_canonical_duplicate node
         ( if checks.eradicate && should_report then
           let cond = Exp.BinOp (Binop.Ne, Exp.Lvar pvar, Exp.null) in
           EradicateChecks.report_error tenv find_canonical_duplicate
-            (TypeErr.Condition_redundant (true, EradicateChecks.explain_expr tenv node cond))
+            (TypeErr.Condition_redundant
+               { is_always_true= true
+               ; condition_descr= EradicateChecks.explain_expr tenv node cond
+               ; nonnull_origin= InferredNullability.get_origin nullability })
             (Some instr_ref) loc curr_pdesc ) ;
         let previous_origin = InferredNullability.get_origin nullability in
         let new_origin = TypeOrigin.InferredNonnull {previous_origin} in
diff --git a/infer/src/nullsafe/typeErr.ml b/infer/src/nullsafe/typeErr.ml
index 5b6d8724f..b7a030ef5 100644
--- a/infer/src/nullsafe/typeErr.ml
+++ b/infer/src/nullsafe/typeErr.ml
@@ -58,7 +58,8 @@ end
 
 (** Instance of an error *)
 type err_instance =
-  | Condition_redundant of (bool * string option)
+  | Condition_redundant of
+      {is_always_true: bool; condition_descr: string option; nonnull_origin: TypeOrigin.t}
   | Inconsistent_subclass of
       { inheritance_violation: InheritanceRule.violation
       ; violation_type: InheritanceRule.violation_type
@@ -228,9 +229,10 @@ let get_field_name_for_error_suppressing = function
 
 let get_error_info err_instance =
   match err_instance with
-  | Condition_redundant (is_always_true, s_opt) ->
+  | Condition_redundant {is_always_true; condition_descr} ->
       ( P.sprintf "The condition %s is always %b according to the existing annotations."
-          (Option.value s_opt ~default:"") is_always_true
+          (Option.value condition_descr ~default:"")
+          is_always_true
       , IssueType.eradicate_condition_redundant
       , None )
   | Over_annotation {over_annotated_violation; violation_type} ->
diff --git a/infer/src/nullsafe/typeErr.mli b/infer/src/nullsafe/typeErr.mli
index 618fe6948..29de105ad 100644
--- a/infer/src/nullsafe/typeErr.mli
+++ b/infer/src/nullsafe/typeErr.mli
@@ -34,7 +34,8 @@ module InstrRef : InstrRefT
 
 (** Instance of an error *)
 type err_instance =
-  | Condition_redundant of (bool * string option)
+  | Condition_redundant of
+      {is_always_true: bool; condition_descr: string option; nonnull_origin: TypeOrigin.t}
   | Inconsistent_subclass of
       { inheritance_violation: InheritanceRule.violation
       ; violation_type: InheritanceRule.violation_type