[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
master
Mitya Lyubarskiy 5 years ago committed by Facebook Github Bot
parent d001db1e94
commit 64c5530f3d

@ -6,19 +6,20 @@
*) *)
open! IStd 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 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 && (* Suppress violations for anything apart from Nullable since such
issues are not very actionable and/or clear for the user. issues are not very actionable and/or clear for the user.
E.g. we technically can suggest changing [DeclaredNonnull] to [Nonnull], E.g. we technically can suggest changing [DeclaredNonnull] to [Nonnull],
but in practice that requires strictification the code, which is a but in practice that requires strictification the code, which is a
separate effort. separate effort.
*) *)
Nullability.equal lhs Nullable Nullability.equal what Nullable
then Error {lhs; rhs_upper_bound} then Error {declared_nullability= what; can_be_narrowed_to= by_rhs_upper_bound}
else Ok () else Ok ()

@ -21,8 +21,11 @@ open! IStd
type violation [@@deriving compare] type violation [@@deriving compare]
val check : lhs:Nullability.t -> rhs_upper_bound:Nullability.t -> (unit, violation) result val check : what:Nullability.t -> by_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 (**
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. is a _strict_ subtype of lhs, `lhs`'s type can be narrowed to be that upper bound.
*) *)

@ -298,12 +298,14 @@ let check_constructor_initialization tenv find_canonical_duplicate curr_construc
() ()
| Some annotated_field -> | Some annotated_field ->
if Config.eradicate_field_over_annotated then if Config.eradicate_field_over_annotated then
let lhs = let what =
AnnotatedNullability.get_nullability AnnotatedNullability.get_nullability
annotated_field.annotated_type.nullability annotated_field.annotated_type.nullability
in in
let rhs_upper_bound = field_nullability_upper_bound_over_all_typestates () in let by_rhs_upper_bound =
Result.iter_error (OverAnnotatedRule.check ~lhs ~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 -> ~f:(fun over_annotated_violation ->
report_error tenv find_canonical_duplicate report_error tenv find_canonical_duplicate
(TypeErr.Over_annotation (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 let check_return_overrannotated tenv find_canonical_duplicate loc curr_pname curr_pdesc
(ret_signature : AnnotatedSignature.ret_signature) ret_inferred_nullability = (ret_signature : AnnotatedSignature.ret_signature) ret_inferred_nullability =
(* Returning from a function is essentially an assignment the actual return value to the formal `return` *) (* 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 (* 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 (all execution flow joins are already made), hence inferreed nullability of returns gives us
correct upper bound. correct upper bound.
*) *)
let rhs_upper_bound = InferredNullability.get_nullability ret_inferred_nullability in let by_rhs_upper_bound = InferredNullability.get_nullability ret_inferred_nullability in
Result.iter_error (OverAnnotatedRule.check ~lhs ~rhs_upper_bound) Result.iter_error (OverAnnotatedRule.check ~what ~by_rhs_upper_bound)
~f:(fun over_annotated_violation -> ~f:(fun over_annotated_violation ->
report_error tenv find_canonical_duplicate report_error tenv find_canonical_duplicate
(TypeErr.Over_annotation (TypeErr.Over_annotation

Loading…
Cancel
Save