From 28c6ed2ceb5e7e51c210eee97a44363ca52e2a7d Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Wed, 13 Nov 2019 10:15:06 -0800 Subject: [PATCH] [nullsafe][TypeOrigin refactor] Rename 0None -> OptimisticFallback Summary: This diff is a part of work teaching Nullsafe to explain decisions it's making. In this diff, we merely rename the value to clearly reflect its currect usage. In follow up diffs we are going to make usages of this instance more restricted and concentrate them in several places in the code. Reviewed By: artempyanykh Differential Revision: D18451290 fbshipit-source-id: cf3773364 --- infer/src/nullsafe/eradicateChecks.ml | 6 +++--- infer/src/nullsafe/typeCheck.ml | 23 +++++++++++++---------- infer/src/nullsafe/typeOrigin.ml | 15 +++++++++++---- infer/src/nullsafe/typeOrigin.mli | 8 +++++++- 4 files changed, 34 insertions(+), 18 deletions(-) diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml index 8d405c9ce..52d69b0f6 100644 --- a/infer/src/nullsafe/eradicateChecks.ml +++ b/infer/src/nullsafe/eradicateChecks.ml @@ -123,13 +123,13 @@ let check_field_assignment ~is_strict_mode tenv find_canonical_duplicate curr_pd let t_lhs, inferred_nullability_lhs = typecheck_expr node instr_ref curr_pdesc typestate exp_lhs (* TODO(T54687014) optimistic default might be an unsoundness issue - investigate *) - (typ, InferredNullability.create_nonnull TypeOrigin.ONone) + (typ, InferredNullability.create_nonnull TypeOrigin.OptimisticFallback) loc in let _, inferred_nullability_rhs = typecheck_expr node instr_ref curr_pdesc typestate exp_rhs (* TODO(T54687014) optimistic default might be an unsoundness issue - investigate *) - (typ, InferredNullability.create_nonnull TypeOrigin.ONone) + (typ, InferredNullability.create_nonnull TypeOrigin.OptimisticFallback) loc in let field_is_injector_readwrite () = @@ -387,7 +387,7 @@ let check_call_receiver ~is_strict_mode tenv find_canonical_duplicate curr_pdesc let _, this_inferred_nullability = typecheck_expr tenv node instr_ref curr_pdesc typestate this_e (* TODO(T54687014) optimistic default might be an unsoundness issue - investigate *) - (typ, InferredNullability.create_nonnull TypeOrigin.ONone) + (typ, InferredNullability.create_nonnull TypeOrigin.OptimisticFallback) loc in check_object_dereference ~is_strict_mode tenv find_canonical_duplicate curr_pdesc node diff --git a/infer/src/nullsafe/typeCheck.ml b/infer/src/nullsafe/typeCheck.ml index d15c2d707..f33d1cae5 100644 --- a/infer/src/nullsafe/typeCheck.ml +++ b/infer/src/nullsafe/typeCheck.ml @@ -139,7 +139,7 @@ let rec typecheck_expr ~is_strict_mode find_canonical_duplicate visited checks t typecheck_expr ~is_strict_mode find_canonical_duplicate visited checks tenv node instr_ref curr_pdesc typestate exp (* TODO(T54687014) optimistic default might be an unsoundness issue - investigate *) - (typ, InferredNullability.create_nonnull TypeOrigin.ONone) + (typ, InferredNullability.create_nonnull TypeOrigin.OptimisticFallback) loc in let object_origin = InferredNullability.get_origin inferred_nullability in @@ -294,7 +294,7 @@ let convert_complex_exp_to_pvar tenv idenv curr_pname None ) |> Option.value_map ~f:(fun (_, nullability) -> InferredNullability.get_origin nullability) - ~default:TypeOrigin.ONone + ~default:TypeOrigin.OptimisticFallback in let exp' = Idenv.expand_expr_temps idenv original_node exp_ in let is_parameter_field pvar = @@ -458,7 +458,7 @@ let do_preconditions_check_not_null instr_ref tenv find_canonical_duplicate node (Some instr_ref) loc curr_pdesc ) ; TypeState.add pvar (* TODO(T54687014) optimistic default might be an unsoundness issue - investigate *) - (t, InferredNullability.create_nonnull TypeOrigin.ONone) + (t, InferredNullability.create_nonnull TypeOrigin.OptimisticFallback) typestate'' | None -> typestate' @@ -501,7 +501,7 @@ let do_preconditions_check_state instr_ref idenv tenv curr_pname curr_annotated_ | Some (t, _) -> TypeState.add pvar (* TODO(T54687014) optimistic default might be an unsoundness issue - investigate *) - (t, InferredNullability.create_nonnull TypeOrigin.ONone) + (t, InferredNullability.create_nonnull TypeOrigin.OptimisticFallback) typestate1 | None -> typestate1 @@ -699,7 +699,8 @@ let rec check_condition_for_sil_prune tenv idenv calls_this find_canonical_dupli let e1 = Exp.Lvar pvar in let typ, ta = typecheck_expr_simple ~is_strict_mode find_canonical_duplicate curr_pdesc calls_this - checks tenv original_node instr_ref typestate e1 (Typ.mk Tvoid) TypeOrigin.ONone loc + checks tenv original_node instr_ref typestate e1 (Typ.mk Tvoid) + TypeOrigin.OptimisticFallback loc in let range = (typ, ta) in let typestate1 = TypeState.add pvar range typestate in @@ -744,7 +745,8 @@ let rec check_condition_for_sil_prune tenv idenv calls_this find_canonical_dupli in let typ, inferred_nullability = typecheck_expr_simple ~is_strict_mode find_canonical_duplicate curr_pdesc calls_this checks - tenv original_node instr_ref typestate2 e' (Typ.mk Tvoid) TypeOrigin.ONone loc + tenv original_node instr_ref typestate2 e' (Typ.mk Tvoid) TypeOrigin.OptimisticFallback + loc in if checks.eradicate then EradicateChecks.check_zero tenv find_canonical_duplicate curr_pdesc node e' typ @@ -782,7 +784,8 @@ let rec check_condition_for_sil_prune tenv idenv calls_this find_canonical_dupli in let typ, inferred_nullability = typecheck_expr_simple ~is_strict_mode find_canonical_duplicate curr_pdesc calls_this checks - tenv original_node instr_ref typestate2 e' (Typ.mk Tvoid) TypeOrigin.ONone loc + tenv original_node instr_ref typestate2 e' (Typ.mk Tvoid) TypeOrigin.OptimisticFallback + loc in if checks.eradicate then EradicateChecks.check_nonzero tenv find_canonical_duplicate curr_pdesc original_node e' typ @@ -847,7 +850,7 @@ let calc_typestate_after_call find_canonical_duplicate calls_this checks tenv id typecheck_expr ~is_strict_mode find_canonical_duplicate calls_this checks tenv node instr_ref curr_pdesc typestate e2 (* TODO(T54687014) optimistic default might be an unsoundness issue - investigate *) - (t2, InferredNullability.create_nonnull TypeOrigin.ONone) + (t2, InferredNullability.create_nonnull TypeOrigin.OptimisticFallback) loc in let actual = (orig_e2, inferred_nullability_actual) in @@ -1079,7 +1082,7 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p (* cast copies the type of the first argument *) TypeState.add_id id (typecheck_expr_simple ~is_strict_mode find_canonical_duplicate curr_pdesc calls_this checks - tenv node instr_ref typestate' e' typ TypeOrigin.ONone loc) + tenv node instr_ref typestate' e' typ TypeOrigin.OptimisticFallback loc) typestate' (* myarray.length *) | Sil.Call ((id, _), Exp.Const (Const.Cfun pn), [(array_exp, t)], loc, _) @@ -1088,7 +1091,7 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p typecheck_expr ~is_strict_mode find_canonical_duplicate calls_this checks tenv node instr_ref curr_pdesc typestate array_exp (* TODO(T54687014) optimistic default might be an unsoundness issue - investigate *) - (t, InferredNullability.create_nonnull TypeOrigin.ONone) + (t, InferredNullability.create_nonnull TypeOrigin.OptimisticFallback) loc in if checks.eradicate then diff --git a/infer/src/nullsafe/typeOrigin.ml b/infer/src/nullsafe/typeOrigin.ml index 8999c66db..90f4413db 100644 --- a/infer/src/nullsafe/typeOrigin.ml +++ b/infer/src/nullsafe/typeOrigin.ml @@ -18,7 +18,14 @@ type t = | MethodCall of method_call_origin (** A result of a method call *) | New (** A new object creation *) | ArrayLengthResult (** integer value - result of accessing array.length *) - | ONone (** No origin is known *) + (* Below are two special values. *) + | OptimisticFallback + (** Something went wrong during typechecking. + We fall back to optimistic (not-nullable) type to reduce potential non-actionable false positives. + Ideally we should not see these instances. They should be either processed gracefully + (and a dedicated type constructor should be added), or fixed. + T54687014 tracks unsoundness issues caused by this type. + *) | Undef (** Undefined value before initialization *) [@@deriving compare] @@ -55,8 +62,8 @@ let rec to_string = function "New" | ArrayLengthResult -> "ArrayLength" - | ONone -> - "ONone" + | OptimisticFallback -> + "OptimisticFallback" | Undef -> "Undef" @@ -95,7 +102,7 @@ let get_description origin = | This | New | NonnullConst _ | ArrayLengthResult -> None (* Two special cases - should not really occur in normal code *) - | ONone | Undef -> + | OptimisticFallback | Undef -> None diff --git a/infer/src/nullsafe/typeOrigin.mli b/infer/src/nullsafe/typeOrigin.mli index b2134fd4e..03c7775f6 100644 --- a/infer/src/nullsafe/typeOrigin.mli +++ b/infer/src/nullsafe/typeOrigin.mli @@ -16,7 +16,13 @@ type t = | MethodCall of method_call_origin (** A result of a method call *) | New (** A new object creation *) | ArrayLengthResult (** integer value - result of accessing array.length *) - | ONone (** No origin is known *) + (* Below are two special values. *) + | OptimisticFallback + (** Something went wrong during typechecking. + We fall back to optimistic (not-nullable) type to reduce potential non-actionable false positives. + Ideally we should not see these instances. They should be either processed gracefully + (and a dedicated type constructor should be added), or fixed. + T54687014 tracks unsoundness issues caused by this type. *) | Undef (** Undefined value before initialization *) [@@deriving compare]