From 64c5530f3df51776bcaed9af4bdc4eb2d574151d Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Wed, 23 Oct 2019 07:15:33 -0700 Subject: [PATCH] [nullsafe] nit: small change in the signature for over-annotated rule Summary: As suggested by artempyanykh, this will make it bit more clear Reviewed By: artempyanykh Differential Revision: D18037204 fbshipit-source-id: 65cb96815 --- infer/src/nullsafe/OverAnnotatedRule.ml | 11 ++++++----- infer/src/nullsafe/OverAnnotatedRule.mli | 7 +++++-- infer/src/nullsafe/eradicateChecks.ml | 14 ++++++++------ 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/infer/src/nullsafe/OverAnnotatedRule.ml b/infer/src/nullsafe/OverAnnotatedRule.ml index a76e6b563..9ef7915da 100644 --- a/infer/src/nullsafe/OverAnnotatedRule.ml +++ b/infer/src/nullsafe/OverAnnotatedRule.ml @@ -6,19 +6,20 @@ *) open! IStd -type violation = {lhs: Nullability.t; rhs_upper_bound: Nullability.t} [@@deriving compare] +type violation = {declared_nullability: Nullability.t; can_be_narrowed_to: Nullability.t} +[@@deriving compare] -let check ~lhs ~rhs_upper_bound = +let check ~what ~by_rhs_upper_bound = if - Nullability.is_strict_subtype ~subtype:rhs_upper_bound ~supertype:lhs + Nullability.is_strict_subtype ~subtype:by_rhs_upper_bound ~supertype:what && (* Suppress violations for anything apart from Nullable since such issues are not very actionable and/or clear for the user. E.g. we technically can suggest changing [DeclaredNonnull] to [Nonnull], but in practice that requires strictification the code, which is a separate effort. *) - Nullability.equal lhs Nullable - then Error {lhs; rhs_upper_bound} + Nullability.equal what Nullable + then Error {declared_nullability= what; can_be_narrowed_to= by_rhs_upper_bound} else Ok () diff --git a/infer/src/nullsafe/OverAnnotatedRule.mli b/infer/src/nullsafe/OverAnnotatedRule.mli index a11f8f558..d462b0de8 100644 --- a/infer/src/nullsafe/OverAnnotatedRule.mli +++ b/infer/src/nullsafe/OverAnnotatedRule.mli @@ -21,8 +21,11 @@ open! IStd type violation [@@deriving compare] -val check : lhs:Nullability.t -> rhs_upper_bound:Nullability.t -> (unit, violation) result -(** If an upper bound of `rhs_i` over ALL assignents `lhs = rhs_i` that exist in the program +val check : what:Nullability.t -> by_rhs_upper_bound:Nullability.t -> (unit, violation) result +(** + Checks if the declared type for `what` can be narrowed, based on the information about + all assignments `what = rhs_i`. + If an upper bound of `rhs_i` over ALL assignents `what = rhs_i` that exist in the program is a _strict_ subtype of lhs, `lhs`'s type can be narrowed to be that upper bound. *) diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml index 986eb4acf..8662fa283 100644 --- a/infer/src/nullsafe/eradicateChecks.ml +++ b/infer/src/nullsafe/eradicateChecks.ml @@ -298,12 +298,14 @@ let check_constructor_initialization tenv find_canonical_duplicate curr_construc () | Some annotated_field -> if Config.eradicate_field_over_annotated then - let lhs = + let what = AnnotatedNullability.get_nullability annotated_field.annotated_type.nullability in - let rhs_upper_bound = field_nullability_upper_bound_over_all_typestates () in - Result.iter_error (OverAnnotatedRule.check ~lhs ~rhs_upper_bound) + let by_rhs_upper_bound = + field_nullability_upper_bound_over_all_typestates () + in + Result.iter_error (OverAnnotatedRule.check ~what ~by_rhs_upper_bound) ~f:(fun over_annotated_violation -> report_error tenv find_canonical_duplicate (TypeErr.Over_annotation @@ -337,13 +339,13 @@ let check_return_not_nullable ~is_strict_mode tenv find_canonical_duplicate loc let check_return_overrannotated tenv find_canonical_duplicate loc curr_pname curr_pdesc (ret_signature : AnnotatedSignature.ret_signature) ret_inferred_nullability = (* Returning from a function is essentially an assignment the actual return value to the formal `return` *) - let lhs = AnnotatedNullability.get_nullability ret_signature.ret_annotated_type.nullability in + let what = AnnotatedNullability.get_nullability ret_signature.ret_annotated_type.nullability in (* In our CFG implementation, there is only one place where we return from a function (all execution flow joins are already made), hence inferreed nullability of returns gives us correct upper bound. *) - let rhs_upper_bound = InferredNullability.get_nullability ret_inferred_nullability in - Result.iter_error (OverAnnotatedRule.check ~lhs ~rhs_upper_bound) + let by_rhs_upper_bound = InferredNullability.get_nullability ret_inferred_nullability in + Result.iter_error (OverAnnotatedRule.check ~what ~by_rhs_upper_bound) ~f:(fun over_annotated_violation -> report_error tenv find_canonical_duplicate (TypeErr.Over_annotation