diff --git a/infer/src/nullsafe/AnnotatedNullability.ml b/infer/src/nullsafe/AnnotatedNullability.ml index afb17d4e3..36238b09c 100644 --- a/infer/src/nullsafe/AnnotatedNullability.ml +++ b/infer/src/nullsafe/AnnotatedNullability.ml @@ -36,6 +36,13 @@ and nonnull_origin = | ModelledNonnull (** nullsafe knows it is non-nullable via its internal models *) [@@deriving compare] +let get_nullability = function + | Nullable _ -> + Nullability.Nullable + | Nonnull _ -> + Nullability.Nonnull + + let pp fmt t = let string_of_nullable_origin nullable_origin = match nullable_origin with diff --git a/infer/src/nullsafe/AnnotatedNullability.mli b/infer/src/nullsafe/AnnotatedNullability.mli index 2ab8a35a4..22a9fb6c3 100644 --- a/infer/src/nullsafe/AnnotatedNullability.mli +++ b/infer/src/nullsafe/AnnotatedNullability.mli @@ -38,6 +38,8 @@ and nonnull_origin = | ModelledNonnull (** nullsafe knows it is non-nullable via its internal models *) [@@deriving compare] +val get_nullability : t -> Nullability.t + val of_annot_item : Annot.Item.t -> t (** Converts the information from the annotation to nullability. NOTE: it does not take into account models etc., so this is intended to be used diff --git a/infer/src/nullsafe/InferredNullability.ml b/infer/src/nullsafe/InferredNullability.ml index e79ed4dc7..32665b367 100644 --- a/infer/src/nullsafe/InferredNullability.ml +++ b/infer/src/nullsafe/InferredNullability.ml @@ -7,15 +7,17 @@ open! IStd -type t = {is_nullable: bool; origin: TypeOrigin.t} [@@deriving compare] +type t = {nullability: Nullability.t; origin: TypeOrigin.t} [@@deriving compare] let equal = [%compare.equal: t] -let is_nullable t = t.is_nullable +let get_nullability {nullability} = nullability -let is_nonnull t = not t.is_nullable +let is_nullable {nullability} = match nullability with Nullable -> true | Nonnull -> false -let set_nonnull t = {t with is_nullable= false} +let is_nonnull {nullability} = match nullability with Nullable -> false | Nonnull -> true + +let set_nonnull t = {t with nullability= Nullability.Nonnull} let descr_origin t = let descr_opt = TypeOrigin.get_description t.origin in @@ -26,18 +28,33 @@ let descr_origin t = ("(Origin: " ^ str ^ ")", loc_opt, sig_opt) -let to_string t = if is_nullable t then " @Nullable" else "" - -let join ta1 ta2 = - let nul1, nul2 = (is_nullable ta1, is_nullable ta2) in - let choose_left = match (nul1, nul2) with false, true -> false | _ -> true in - let ta_chosen, ta_other = if choose_left then (ta1, ta2) else (ta2, ta1) in - let origin = - if Bool.equal nul1 nul2 then TypeOrigin.join ta_chosen.origin ta_other.origin - else ta_chosen.origin +let to_string {nullability} = Printf.sprintf "[%s]" (Nullability.to_string nullability) + +let join t1 t2 = + let joined_nullability = Nullability.join t1.nullability t2.nullability in + let is_equal_to_t1 = Nullability.equal t1.nullability joined_nullability in + let is_equal_to_t2 = Nullability.equal t2.nullability joined_nullability in + (* Origin complements nullability information. It is the best effort to explain how was the nullability inferred. + If nullability is fully determined by one of the arguments, origin should be get from this argument. + Otherwise we apply heuristics to choose origin either from t1 or t2. + *) + let joined_origin = + match (is_equal_to_t1, is_equal_to_t2) with + | true, false -> + (* Nullability was fully determined by t1. *) + t1.origin + | false, true -> + (* Nullability was fully determined by t2 *) + t2.origin + | false, false | true, true -> + (* Nullability is not fully determined by neither t1 nor t2 + Let TypeOrigin logic to decide what to prefer in this case. + *) + TypeOrigin.join t1.origin t2.origin in - let ta' = {ta_chosen with origin} in - if equal ta' ta1 then None else Some ta' + let result = {nullability= joined_nullability; origin= joined_origin} in + (* TODO: make the result return non optional value *) + if equal result t1 then None else Some result let get_origin t = t.origin @@ -50,15 +67,15 @@ let origin_is_fun_library t = false -let create_nullable origin = {origin; is_nullable= true} +let create_nullable origin = {origin; nullability= Nullability.Nullable} -let create_nonnull origin = {origin; is_nullable= false} +let create_nonnull origin = {origin; nullability= Nullability.Nonnull} let with_origin t o = {t with origin= o} let of_annotated_nullability annotated_nullability origin = match annotated_nullability with | AnnotatedNullability.Nullable _ -> - {origin; is_nullable= true} + {origin; nullability= Nullability.Nullable} | AnnotatedNullability.Nonnull _ -> - {origin; is_nullable= false} + {origin; nullability= Nullability.Nonnull} diff --git a/infer/src/nullsafe/InferredNullability.mli b/infer/src/nullsafe/InferredNullability.mli index 969be26f3..000b4a75c 100644 --- a/infer/src/nullsafe/InferredNullability.mli +++ b/infer/src/nullsafe/InferredNullability.mli @@ -19,6 +19,8 @@ open! IStd type t [@@deriving compare] +val get_nullability : t -> Nullability.t + val create_nullable : TypeOrigin.t -> t val create_nonnull : TypeOrigin.t -> t diff --git a/infer/src/nullsafe/Nullability.ml b/infer/src/nullsafe/Nullability.ml new file mode 100644 index 000000000..59f2784f8 --- /dev/null +++ b/infer/src/nullsafe/Nullability.ml @@ -0,0 +1,24 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +open! IStd + +type t = + | Nullable (** No guarantees on the nullability *) + | Nonnull + (** We believe that this value can not be null. If it is not the case, this is + an unsoundness issue for Nullsafe, and we aim to minimize number of such issues + occuring in real-world programs. *) +[@@deriving compare, equal] + +let join x y = + match (x, y) with Nullable, _ | _, Nullable -> Nullable | Nonnull, Nonnull -> Nonnull + + +let is_subtype ~subtype ~supertype = equal (join subtype supertype) supertype + +let to_string = function Nullable -> "Nullable" | Nonnull -> "Nonnull" diff --git a/infer/src/nullsafe/Nullability.mli b/infer/src/nullsafe/Nullability.mli new file mode 100644 index 000000000..37888db2e --- /dev/null +++ b/infer/src/nullsafe/Nullability.mli @@ -0,0 +1,35 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +open! IStd + +(** Nullability is a central concept for Nullsafe type-checker. + Informally, nullability is a "type" - set of values together with some additional context. + All nullsafe is interested about if whether a value can be null, and if it can, + what are potential causes of leaking the null inside it. + Formally, nullability values form a lattice with partial order and upper bound operations. + Practically, most of nullsafe core should remain agnostic over exact values of nullability + (e.g. pattern-mathching over them should be a rare case. Core of typechecker should deal with subtyping and joins instead.) + *) + +type t = + | Nullable (** No guarantees on the nullability *) + | Nonnull + (** We believe that this value can not be null. If it is not the case, this is + an unsoundness issue for Nullsafe, and we aim to minimize number of such issues + occuring in real-world programs. *) +[@@deriving compare, equal] + +val is_subtype : subtype:t -> supertype:t -> bool +(** A is a subtype of B, if all values of A can be represented in B. + Subtype relation is reflexive: everything is a subtype of itself. *) + +val join : t -> t -> t +(** Unique upper bound over two types: the most precise type that is a supertype of both. + Practically, joins occur e.g. when two branches of execution flow are getting merged. *) + +val to_string : t -> string diff --git a/infer/src/nullsafe/NullsafeRules.ml b/infer/src/nullsafe/NullsafeRules.ml index 0b9d60e9c..e9d02efe0 100644 --- a/infer/src/nullsafe/NullsafeRules.ml +++ b/infer/src/nullsafe/NullsafeRules.ml @@ -7,63 +7,18 @@ open! IStd -type nullability = Nullable | Nonnull - -let is_nullable nullability = match nullability with Nullable -> true | Nonnull -> false - -let is_nonnull nullability = match nullability with Nullable -> false | Nonnull -> true - -(* - Subtype in the standard definition of subtyping `<:` operator: - S <: T means that any term of type S can be safely used in a context - where a term of type T is expected. - The key fact is that Nonnull <: Nullable, but not the reverse. - *) -let is_subtype ~subtype ~supertype = - (* - When it is NOT a subtype? When subtype is nullable and supertype is non-nullable. - Everything else is allowed. - *) - not (is_nullable subtype && is_nonnull supertype) - - -let nullability_of_annotated_nullability annotated_nullability = - match annotated_nullability with - | AnnotatedNullability.Nullable _ -> - Nullable - | AnnotatedNullability.Nonnull _ -> - Nonnull - - -let nullability_of_inferred_nullability inferred_nullability = - (* This needs to be modified in order to support unknown nullability *) - if InferredNullability.is_nullable inferred_nullability then Nullable else Nonnull - - -let passes_assignment_rule_for_annotated_nullability ~lhs ~rhs = - let lhs_nullability = nullability_of_annotated_nullability lhs in - let rhs_nullability = nullability_of_inferred_nullability rhs in - is_subtype ~subtype:rhs_nullability ~supertype:lhs_nullability - - -let passes_assignment_rule_for_inferred_nullability ~lhs ~rhs = - let lhs_nullability = nullability_of_inferred_nullability lhs in - let rhs_nullability = nullability_of_inferred_nullability rhs in - is_subtype ~subtype:rhs_nullability ~supertype:lhs_nullability - +let passes_assignment_rule ~lhs ~rhs = Nullability.is_subtype ~subtype:rhs ~supertype:lhs type type_role = Param | Ret let passes_inheritance_rule type_role ~base ~overridden = - let base_nullability = nullability_of_annotated_nullability base in - let overridden_nullability = nullability_of_annotated_nullability overridden in let subtype, supertype = match type_role with | Ret -> (* covariance for ret *) - (overridden_nullability, base_nullability) + (overridden, base) | Param -> (* contravariance for param *) - (base_nullability, overridden_nullability) + (base, overridden) in - is_subtype ~subtype ~supertype + Nullability.is_subtype ~subtype ~supertype diff --git a/infer/src/nullsafe/NullsafeRules.mli b/infer/src/nullsafe/NullsafeRules.mli index 0ff74feae..154c2343f 100644 --- a/infer/src/nullsafe/NullsafeRules.mli +++ b/infer/src/nullsafe/NullsafeRules.mli @@ -18,29 +18,14 @@ open! IStd this module instead of doing things on their own. *) -(******************************************************************************************* - *** Assignment rule ********************************************************************** - -Assignment rule: No expression of nullable type is ever assigned to a location -of non-nullable type. +val passes_assignment_rule : lhs:Nullability.t -> rhs:Nullability.t -> bool +(** Assignment rule: No expression of nullable type is ever assigned to a location + of non-nullable type. *) -val passes_assignment_rule_for_annotated_nullability : - lhs:AnnotatedNullability.t -> rhs:InferredNullability.t -> bool -(** Variant of assignment rule where lhs nullability is fully determined by its formal Nullsafe type *) - -val passes_assignment_rule_for_inferred_nullability : - lhs:InferredNullability.t -> rhs:InferredNullability.t -> bool -(** Variant of assignment rule where lhs nullability is inferred (e.g. might differ from formal nullability of a corresponding type) *) - -(******************************************************************************************* - *** Inheritance rule ********************************************************************** - *) - type type_role = Param | Ret -val passes_inheritance_rule : - type_role -> base:AnnotatedNullability.t -> overridden:AnnotatedNullability.t -> bool +val passes_inheritance_rule : type_role -> base:Nullability.t -> overridden:Nullability.t -> bool (** Inheritance rule: a) Return type for an overridden method is covariant: overridden method is allowed to narrow down the return value to a subtype of the one from the diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml index 29c675a9d..2926a7619 100644 --- a/infer/src/nullsafe/eradicateChecks.ml +++ b/infer/src/nullsafe/eradicateChecks.ml @@ -151,8 +151,9 @@ let check_field_assignment tenv find_canonical_duplicate curr_pdesc node instr_r in let should_report_nullable = (not - (NullsafeRules.passes_assignment_rule_for_inferred_nullability ~lhs:inferred_nullability_lhs - ~rhs:inferred_nullability_rhs)) + (NullsafeRules.passes_assignment_rule + ~lhs:(InferredNullability.get_nullability inferred_nullability_lhs) + ~rhs:(InferredNullability.get_nullability inferred_nullability_rhs))) && (not (AndroidFramework.is_destroy_method curr_pname)) && PatternMatch.type_is_class t_lhs && (not (Typ.Fieldname.Java.is_outer_instance fname)) @@ -305,11 +306,10 @@ let check_constructor_initialization tenv find_canonical_duplicate curr_construc let check_return_not_nullable tenv find_canonical_duplicate loc curr_pname curr_pdesc (ret_signature : AnnotatedSignature.ret_signature) ret_inferred_nullability = - if - not - (NullsafeRules.passes_assignment_rule_for_annotated_nullability - ~lhs:ret_signature.ret_annotated_type.nullability ~rhs:ret_inferred_nullability) - then + (* 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 rhs = InferredNullability.get_nullability ret_inferred_nullability in + if not (NullsafeRules.passes_assignment_rule ~lhs ~rhs) then report_error tenv find_canonical_duplicate (TypeErr.Return_annotation_inconsistent (curr_pname, InferredNullability.descr_origin ret_inferred_nullability)) @@ -406,11 +406,11 @@ let check_call_parameters tenv find_canonical_duplicate curr_pdesc node callee_a (Some instr_ref) loc curr_pdesc in if PatternMatch.type_is_class formal.param_annotated_type.typ then - if - not - (NullsafeRules.passes_assignment_rule_for_annotated_nullability - ~lhs:formal.param_annotated_type.nullability ~rhs:nullability_actual) - then report () + (* Passing a param to a function is essentially an assignment the actual param value + to the formal param *) + let lhs = AnnotatedNullability.get_nullability formal.param_annotated_type.nullability in + let rhs = InferredNullability.get_nullability nullability_actual in + if not (NullsafeRules.passes_assignment_rule ~lhs ~rhs) then report () in let should_check_parameters = match callee_pname with @@ -467,15 +467,17 @@ let check_inheritance_rule_for_params find_canonical_duplicate tenv loc ~base_pr (* Check the rule for each pair of base and overridden param *) List.iteri base_and_overridden_params ~f:(fun index - ( AnnotatedSignature.{param_annotated_type= {nullability= base_nullability}} + ( AnnotatedSignature.{param_annotated_type= {nullability= annotated_nullability_base}} , AnnotatedSignature. { mangled= overridden_param_name - ; param_annotated_type= {nullability= overridden_nullability} } ) + ; param_annotated_type= {nullability= annotated_nullability_overridden} } ) -> check_inheritance_rule_for_param find_canonical_duplicate tenv loc ~overridden_param_name ~base_proc_name ~overridden_proc_name ~overridden_proc_desc ~param_position:(if should_index_from_zero then index else index + 1) - ~base_nullability ~overridden_nullability ) + ~base_nullability:(AnnotatedNullability.get_nullability annotated_nullability_base) + ~overridden_nullability: + (AnnotatedNullability.get_nullability annotated_nullability_overridden) ) | Unequal_lengths -> (* Skip checking. TODO (T5280249): investigate why argument lists can be of different length. *) @@ -494,10 +496,12 @@ let check_inheritance_rule_for_signature find_canonical_duplicate tenv loc ~base | Typ.Procname.Java java_pname when not (Typ.Procname.Java.is_external java_pname) -> (* Check if return value is consistent with the base *) let base_nullability = - base_signature.AnnotatedSignature.ret.ret_annotated_type.nullability + AnnotatedNullability.get_nullability + base_signature.AnnotatedSignature.ret.ret_annotated_type.nullability in let overridden_nullability = - overridden_signature.AnnotatedSignature.ret.ret_annotated_type.nullability + AnnotatedNullability.get_nullability + overridden_signature.AnnotatedSignature.ret.ret_annotated_type.nullability in check_inheritance_rule_for_return find_canonical_duplicate tenv loc ~base_proc_name ~overridden_proc_name ~overridden_proc_desc ~base_nullability ~overridden_nullability