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