[nullsafe][annotation graph] Support provisionally nullable in fields

Summary:
This change is on par with the logic that we already have for methods
and method params.

In this logic, we explicitly distinct class under analysis & the
external class when fetching AnnotatedNullability (we needed this to support
various Nullsafe modes as well).

Here we use the same approach for fields.

NOTE: this is not intended to work with nested classes just yet.

Reviewed By: artempyanykh

Differential Revision: D24650934

fbshipit-source-id: f555bcc3f
master
Mitya Lyubarskiy 4 years ago committed by Facebook GitHub Bot
parent 671d9fa291
commit a4efdce9b0

@ -38,7 +38,18 @@ let is_enum_value tenv ~class_typ (field_info : Struct.field_info) =
let is_synthetic field_name = String.contains field_name '$' let is_synthetic field_name = String.contains field_name '$'
let get tenv field_name class_typ = (* For the special mode, return the provisionally nullable annotation, otherwise return the unchaged nullability *)
let maybe_provisionally_nullable field_name ~field_class ~class_under_analysis nullability =
if
Config.nullsafe_annotation_graph
(* Provisionally nullable mode distinct "internal" fields in the class and all the fields outside *)
&& Typ.Name.equal field_class class_under_analysis
&& AnnotatedNullability.can_be_considered_for_provisional_annotation nullability
then AnnotatedNullability.ProvisionallyNullable (ProvisionalAnnotation.Field {field_name})
else nullability
let get tenv field_name ~class_typ ~class_under_analysis =
let open IOption.Let_syntax in let open IOption.Let_syntax in
let lookup = Tenv.lookup tenv in let lookup = Tenv.lookup tenv in
(* We currently don't support field-level strict mode annotation, so fetch it from class *) (* We currently don't support field-level strict mode annotation, so fetch it from class *)
@ -63,7 +74,7 @@ let get tenv field_name class_typ =
AnnotatedNullability.of_type_and_annotation ~is_callee_in_trust_list:false ~nullsafe_mode AnnotatedNullability.of_type_and_annotation ~is_callee_in_trust_list:false ~nullsafe_mode
~is_third_party field_typ annotations ~is_third_party field_typ annotations
in in
let corrected_nullability = let special_case_nullability =
if Nullability.is_nonnullish (AnnotatedNullability.get_nullability nullability) then if Nullability.is_nonnullish (AnnotatedNullability.get_nullability nullability) then
if if
is_enum_value is_enum_value
@ -81,5 +92,13 @@ let get tenv field_name class_typ =
else nullability else nullability
else nullability else nullability
in in
let annotated_type = AnnotatedType.{nullability= corrected_nullability; typ= field_typ} in let field_class =
Option.value_exn (get_type_name class_typ)
~message:"otherwise we would not have fetched field info above"
in
let final_nullability =
maybe_provisionally_nullable field_name ~field_class ~class_under_analysis
special_case_nullability
in
let annotated_type = AnnotatedType.{nullability= final_nullability; typ= field_typ} in
{annotation_deprecated= annotations; annotated_type} {annotation_deprecated= annotations; annotated_type}

@ -11,5 +11,5 @@ open! IStd
type t = {annotation_deprecated: Annot.Item.t; annotated_type: AnnotatedType.t} type t = {annotation_deprecated: Annot.Item.t; annotated_type: AnnotatedType.t}
val get : Tenv.t -> Fieldname.t -> Typ.t -> t option val get : Tenv.t -> Fieldname.t -> class_typ:Typ.t -> class_under_analysis:Typ.name -> t option
(** Looks up for a field declaration and, in case of success, converts it to [t] *) (** Looks up for a field declaration and, in case of success, converts it to [t] *)

@ -145,3 +145,26 @@ let of_type_and_annotation ~is_callee_in_trust_list ~nullsafe_mode ~is_third_par
else UncheckedNonnull ImplicitlyNonnull else UncheckedNonnull ImplicitlyNonnull
in in
if is_callee_in_trust_list then LocallyTrustedNonnull else preliminary_nullability if is_callee_in_trust_list then LocallyTrustedNonnull else preliminary_nullability
let can_be_considered_for_provisional_annotation = function
| Nullable _ ->
(* already nullable *) false
| ProvisionallyNullable _ ->
(* already provisionally nullable *) true
| ThirdPartyNonnull ->
(* third party code is considered beyond control *) false
| UncheckedNonnull _ | LocallyTrustedNonnull | LocallyCheckedNonnull ->
(* legit non-primitive non-nullable type *) true
| StrictNonnull ExplicitNonnullThirdParty ->
(* third party code is considered beyond control *) false
| StrictNonnull ModelledNonnull ->
(* models correspond to code beyond control *) false
| StrictNonnull PrimitiveType ->
(* primitive type can not be annotated *) false
| StrictNonnull EnumValue ->
(* by design non-nullable *) false
| StrictNonnull SyntheticField ->
(* not present in source code *) false
| StrictNonnull StrictMode ->
(* legit non-nullable non-primitive type *) true

@ -81,4 +81,12 @@ val of_type_and_annotation :
processing. [is_callee_in_trust_list] defines whether the callee class is in the caller's processing. [is_callee_in_trust_list] defines whether the callee class is in the caller's
explicitly provided trust list and therefore whether its nullability should be refined. *) explicitly provided trust list and therefore whether its nullability should be refined. *)
val can_be_considered_for_provisional_annotation : t -> bool
(** A method for the special mode where imaginary (provisional) [@Nullable] annotations are added to
the code: see also [ProvisionalAnnotation.t]. This is a helper method useful for preliminary
filtration of types that:
- can be semantically annotated as [@Nullable] in the source code e.g. non-primitive types
- makes logical sense to annotate - e.g. the source code is under control. *)
val pp : Format.formatter -> t -> unit val pp : Format.formatter -> t -> unit

@ -7,14 +7,14 @@
open! IStd open! IStd
type t = type t =
| Field of {field_name: string} | Field of {field_name: Fieldname.t}
| Method of Procname.Java.t | Method of Procname.Java.t
| Param of {method_info: Procname.Java.t; num: int} | Param of {method_info: Procname.Java.t; num: int}
[@@deriving compare] [@@deriving compare]
let pp fmt = function let pp fmt = function
| Field {field_name} -> | Field {field_name} ->
Format.fprintf fmt "Field(%s)" field_name Format.fprintf fmt "Field(%a)" Fieldname.pp field_name
| Method proc_name -> | Method proc_name ->
Format.fprintf fmt "Method(%a)" Procname.pp (Procname.Java proc_name) Format.fprintf fmt "Method(%a)" Procname.pp (Procname.Java proc_name)
| Param {method_info; num} -> | Param {method_info; num} ->

@ -12,7 +12,7 @@ open! IStd
and such * element was annotated as [@Nullable]. *) and such * element was annotated as [@Nullable]. *)
type t = type t =
| Field of {field_name: string} | Field of {field_name: Fieldname.t}
| Method of Procname.Java.t | Method of Procname.Java.t
| Param of {method_info: Procname.Java.t; num: int} | Param of {method_info: Procname.Java.t; num: int}
[@@deriving compare] [@@deriving compare]

@ -233,7 +233,9 @@ let check_constructor_initialization
match Tenv.lookup tenv name with match Tenv.lookup tenv name with
| Some {fields} -> | Some {fields} ->
let do_field (field_name, field_type, _) = let do_field (field_name, field_type, _) =
let annotated_field = AnnotatedField.get tenv field_name ts in let annotated_field =
AnnotatedField.get tenv field_name ~class_typ:ts ~class_under_analysis:name
in
let is_initialized_by_framework = let is_initialized_by_framework =
match annotated_field with match annotated_field with
| None -> | None ->

@ -117,9 +117,9 @@ type find_canonical_duplicate = Procdesc.Node.t -> Procdesc.Node.t
type checks = {eradicate: bool; check_ret_type: check_return_type list} type checks = {eradicate: bool; check_ret_type: check_return_type list}
(** Typecheck an expression. *) (** Typecheck an expression. *)
let rec typecheck_expr ({IntraproceduralAnalysis.tenv; _} as analysis_data) ~nullsafe_mode let rec typecheck_expr ({IntraproceduralAnalysis.tenv; proc_desc= curr_proc_desc} as analysis_data)
find_canonical_duplicate visited checks node instr_ref typestate e tr_default loc : ~nullsafe_mode find_canonical_duplicate visited checks node instr_ref typestate e tr_default loc
TypeState.range = : TypeState.range =
L.d_with_indent ~name:"typecheck_expr" ~pp_result:TypeState.pp_range (fun () -> L.d_with_indent ~name:"typecheck_expr" ~pp_result:TypeState.pp_range (fun () ->
L.d_printfln "Expr: %a" Exp.pp e ; L.d_printfln "Expr: %a" Exp.pp e ;
match e with match e with
@ -161,8 +161,14 @@ let rec typecheck_expr ({IntraproceduralAnalysis.tenv; _} as analysis_data) ~nul
loc loc
in in
let object_origin = InferredNullability.get_simple_origin inferred_nullability in let object_origin = InferredNullability.get_simple_origin inferred_nullability in
let curr_procname =
Procdesc.get_proc_name curr_proc_desc
|> Procname.as_java_exn
~explanation:"typecheck_expr: attempt to typecheck non-Java method"
in
let class_under_analysis = Procname.Java.get_class_type_name curr_procname in
let tr_new = let tr_new =
match AnnotatedField.get tenv field_name typ with match AnnotatedField.get tenv field_name ~class_typ:typ ~class_under_analysis with
| Some AnnotatedField.{annotated_type= field_type} -> | Some AnnotatedField.{annotated_type= field_type} ->
( field_type.typ ( field_type.typ
, InferredNullability.create , InferredNullability.create
@ -252,12 +258,13 @@ let funcall_exp_to_original_pvar_exp tenv curr_pname typestate exp ~is_assignmen
exp exp
let add_field_to_typestate_if_absent tenv access_loc typestate pvar object_origin field_name typ = let add_field_to_typestate_if_absent tenv access_loc typestate pvar object_origin field_name
~field_class_typ ~class_under_analysis =
match TypeState.lookup_pvar pvar typestate with match TypeState.lookup_pvar pvar typestate with
| Some _ -> | Some _ ->
typestate typestate
| None -> ( | None -> (
match AnnotatedField.get tenv field_name typ with match AnnotatedField.get tenv field_name ~class_typ:field_class_typ ~class_under_analysis with
| Some AnnotatedField.{annotated_type= field_type} -> | Some AnnotatedField.{annotated_type= field_type} ->
let range = let range =
( field_type.typ ( field_type.typ
@ -303,7 +310,7 @@ let convert_complex_exp_to_pvar_and_register_field_in_typestate tenv idenv curr_
default ) default )
| Exp.Lvar _ -> | Exp.Lvar _ ->
default default
| Exp.Lfield (exp_, fn, typ) -> | Exp.Lfield (exp_, fn, field_class_typ) ->
let inner_origin = let inner_origin =
( match exp_ with ( match exp_ with
| Exp.Lvar pvar -> | Exp.Lvar pvar ->
@ -330,13 +337,19 @@ let convert_complex_exp_to_pvar_and_register_field_in_typestate tenv idenv curr_
let pvar_to_str pvar = let pvar_to_str pvar =
if Exp.is_this (Exp.Lvar pvar) then "" else Pvar.to_string pvar ^ "_" if Exp.is_this (Exp.Lvar pvar) then "" else Pvar.to_string pvar ^ "_"
in in
let class_under_analysis =
Procname.Java.get_class_type_name
(Procname.as_java_exn curr_pname
~explanation:"Attempt to typecheck non-Java procname")
in
let res = let res =
match exp' with match exp' with
| Exp.Lvar pv when is_parameter_field pv || is_static_field pv -> | Exp.Lvar pv when is_parameter_field pv || is_static_field pv ->
let fld_name = pvar_to_str pv ^ Fieldname.to_string fn in let fld_name = pvar_to_str pv ^ Fieldname.to_string fn in
let pvar = Pvar.mk (Mangled.from_string fld_name) curr_pname in let pvar = Pvar.mk (Mangled.from_string fld_name) curr_pname in
let typestate' = let typestate' =
add_field_to_typestate_if_absent tenv loc typestate pvar inner_origin fn typ add_field_to_typestate_if_absent tenv loc typestate pvar inner_origin fn
~field_class_typ ~class_under_analysis
in in
(Exp.Lvar pvar, typestate') (Exp.Lvar pvar, typestate')
| Exp.Lfield (_exp', fn', _) when Fieldname.is_java_outer_instance fn' -> | Exp.Lfield (_exp', fn', _) when Fieldname.is_java_outer_instance fn' ->
@ -344,7 +357,8 @@ let convert_complex_exp_to_pvar_and_register_field_in_typestate tenv idenv curr_
let fld_name = Fieldname.to_string fn' ^ "_" ^ Fieldname.to_string fn in let fld_name = Fieldname.to_string fn' ^ "_" ^ Fieldname.to_string fn in
let pvar = Pvar.mk (Mangled.from_string fld_name) curr_pname in let pvar = Pvar.mk (Mangled.from_string fld_name) curr_pname in
let typestate' = let typestate' =
add_field_to_typestate_if_absent tenv loc typestate pvar inner_origin fn typ add_field_to_typestate_if_absent tenv loc typestate pvar inner_origin fn
~field_class_typ ~class_under_analysis
in in
(Exp.Lvar pvar, typestate') (Exp.Lvar pvar, typestate')
| Exp.Lvar _ | Exp.Lfield _ -> ( | Exp.Lvar _ | Exp.Lfield _ -> (
@ -353,7 +367,8 @@ let convert_complex_exp_to_pvar_and_register_field_in_typestate tenv idenv curr_
| Some exp_str -> | Some exp_str ->
let pvar = Pvar.mk (Mangled.from_string exp_str) curr_pname in let pvar = Pvar.mk (Mangled.from_string exp_str) curr_pname in
let typestate' = let typestate' =
add_field_to_typestate_if_absent tenv loc typestate pvar inner_origin fn typ add_field_to_typestate_if_absent tenv loc typestate pvar inner_origin fn
~field_class_typ ~class_under_analysis
in in
(Exp.Lvar pvar, typestate') (Exp.Lvar pvar, typestate')
| None -> | None ->
@ -1232,12 +1247,19 @@ let typecheck_instr ({IntraproceduralAnalysis.proc_desc= curr_pdesc; tenv; _} as
let check_field_assign () = let check_field_assign () =
match e1 with match e1 with
| Exp.Lfield (_, field_name, field_class_type) -> ( | Exp.Lfield (_, field_name, field_class_type) -> (
match AnnotatedField.get tenv field_name field_class_type with let class_under_analysis =
Procname.Java.get_class_type_name
(Procname.as_java_exn curr_pname
~explanation:"Attempt to typecheck non-Java method")
in
match
AnnotatedField.get tenv field_name ~class_typ:field_class_type ~class_under_analysis
with
| Some annotated_field -> | Some annotated_field ->
if checks.eradicate then if checks.eradicate then
EradicateChecks.check_field_assignment analysis_data ~nullsafe_mode EradicateChecks.check_field_assignment analysis_data ~nullsafe_mode
find_canonical_duplicate node instr_ref typestate ~expr_rhs:e2 ~field_type:typ loc find_canonical_duplicate node instr_ref typestate ~expr_rhs:e2 ~field_type:typ
field_name annotated_field loc field_name annotated_field
(typecheck_expr analysis_data ~nullsafe_mode find_canonical_duplicate calls_this (typecheck_expr analysis_data ~nullsafe_mode find_canonical_duplicate calls_this
checks) checks)
| None -> | None ->

Loading…
Cancel
Save