[nullsafe] Remove "Field not mutable" check

Summary:
This check was an incomplete attempt to make nullsafe check nutritious
annotations for fields that get modified.

This was never fully productionized, and this check is turned off by
default.

In near future, we don't anticipate supporting this feature, so let's
remove it to simplify the code.

Reviewed By: artempyanykh

Differential Revision: D17282015

fbshipit-source-id: d63a2f1f7
master
Mitya Lyubarskiy 5 years ago committed by Facebook Github Bot
parent fbeb58c0f2
commit b00b526928

@ -377,7 +377,6 @@ OPTIONS
ERADICATE_CONDITION_REDUNDANT (enabled by default), ERADICATE_CONDITION_REDUNDANT (enabled by default),
ERADICATE_CONDITION_REDUNDANT_NONNULL (enabled by default), ERADICATE_CONDITION_REDUNDANT_NONNULL (enabled by default),
ERADICATE_FIELD_NOT_INITIALIZED (enabled by default), ERADICATE_FIELD_NOT_INITIALIZED (enabled by default),
ERADICATE_FIELD_NOT_MUTABLE (enabled by default),
ERADICATE_FIELD_NOT_NULLABLE (enabled by default), ERADICATE_FIELD_NOT_NULLABLE (enabled by default),
ERADICATE_FIELD_OVER_ANNOTATED (enabled by default), ERADICATE_FIELD_OVER_ANNOTATED (enabled by default),
ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION (enabled ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION (enabled
@ -1298,10 +1297,6 @@ INTERNAL OPTIONS
Activates: Condition redundant warnings (Conversely: Activates: Condition redundant warnings (Conversely:
--no-eradicate-condition-redundant) --no-eradicate-condition-redundant)
--eradicate-field-not-mutable
Activates: Field not mutable warnings (Conversely:
--no-eradicate-field-not-mutable)
--eradicate-field-over-annotated --eradicate-field-over-annotated
Activates: Field over-annotated warnings (Conversely: Activates: Field over-annotated warnings (Conversely:
--no-eradicate-field-over-annotated) --no-eradicate-field-over-annotated)

@ -126,7 +126,6 @@ OPTIONS
ERADICATE_CONDITION_REDUNDANT (enabled by default), ERADICATE_CONDITION_REDUNDANT (enabled by default),
ERADICATE_CONDITION_REDUNDANT_NONNULL (enabled by default), ERADICATE_CONDITION_REDUNDANT_NONNULL (enabled by default),
ERADICATE_FIELD_NOT_INITIALIZED (enabled by default), ERADICATE_FIELD_NOT_INITIALIZED (enabled by default),
ERADICATE_FIELD_NOT_MUTABLE (enabled by default),
ERADICATE_FIELD_NOT_NULLABLE (enabled by default), ERADICATE_FIELD_NOT_NULLABLE (enabled by default),
ERADICATE_FIELD_OVER_ANNOTATED (enabled by default), ERADICATE_FIELD_OVER_ANNOTATED (enabled by default),
ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION (enabled ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION (enabled

@ -377,7 +377,6 @@ OPTIONS
ERADICATE_CONDITION_REDUNDANT (enabled by default), ERADICATE_CONDITION_REDUNDANT (enabled by default),
ERADICATE_CONDITION_REDUNDANT_NONNULL (enabled by default), ERADICATE_CONDITION_REDUNDANT_NONNULL (enabled by default),
ERADICATE_FIELD_NOT_INITIALIZED (enabled by default), ERADICATE_FIELD_NOT_INITIALIZED (enabled by default),
ERADICATE_FIELD_NOT_MUTABLE (enabled by default),
ERADICATE_FIELD_NOT_NULLABLE (enabled by default), ERADICATE_FIELD_NOT_NULLABLE (enabled by default),
ERADICATE_FIELD_OVER_ANNOTATED (enabled by default), ERADICATE_FIELD_OVER_ANNOTATED (enabled by default),
ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION (enabled ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION (enabled

@ -1399,10 +1399,6 @@ and eradicate_condition_redundant =
CLOpt.mk_bool ~long:"eradicate-condition-redundant" "Condition redundant warnings" CLOpt.mk_bool ~long:"eradicate-condition-redundant" "Condition redundant warnings"
and eradicate_field_not_mutable =
CLOpt.mk_bool ~long:"eradicate-field-not-mutable" "Field not mutable warnings"
and eradicate_field_over_annotated = and eradicate_field_over_annotated =
CLOpt.mk_bool ~long:"eradicate-field-over-annotated" "Field over-annotated warnings" CLOpt.mk_bool ~long:"eradicate-field-over-annotated" "Field over-annotated warnings"
@ -2856,8 +2852,6 @@ and eradicate = !eradicate
and eradicate_condition_redundant = !eradicate_condition_redundant and eradicate_condition_redundant = !eradicate_condition_redundant
and eradicate_field_not_mutable = !eradicate_field_not_mutable
and eradicate_field_over_annotated = !eradicate_field_over_annotated and eradicate_field_over_annotated = !eradicate_field_over_annotated
and eradicate_return_over_annotated = !eradicate_return_over_annotated and eradicate_return_over_annotated = !eradicate_return_over_annotated

@ -348,8 +348,6 @@ val eradicate : bool
val eradicate_condition_redundant : bool val eradicate_condition_redundant : bool
val eradicate_field_not_mutable : bool
val eradicate_field_over_annotated : bool val eradicate_field_over_annotated : bool
val eradicate_return_over_annotated : bool val eradicate_return_over_annotated : bool

@ -231,10 +231,6 @@ let eradicate_field_not_initialized =
register_from_string "ERADICATE_FIELD_NOT_INITIALIZED" ~hum:"Field Not Initialized" register_from_string "ERADICATE_FIELD_NOT_INITIALIZED" ~hum:"Field Not Initialized"
let eradicate_field_not_mutable =
register_from_string "ERADICATE_FIELD_NOT_MUTABLE" ~hum:"Field Not Mutable"
let eradicate_field_not_nullable = let eradicate_field_not_nullable =
register_from_string "ERADICATE_FIELD_NOT_NULLABLE" ~hum:"Field Not Nullable" register_from_string "ERADICATE_FIELD_NOT_NULLABLE" ~hum:"Field Not Nullable"

@ -148,8 +148,6 @@ val eradicate_condition_redundant_nonnull : t
val eradicate_field_not_initialized : t val eradicate_field_not_initialized : t
val eradicate_field_not_mutable : t
val eradicate_field_not_nullable : t val eradicate_field_not_nullable : t
val eradicate_field_over_annotated : t val eradicate_field_over_annotated : t

@ -51,8 +51,6 @@ let inject_prop = "InjectProp"
let inject_view = "InjectView" let inject_view = "InjectView"
let mutable_ = "Mutable"
let nonnull = "Nonnull" let nonnull = "Nonnull"
let no_allocation = "NoAllocation" let no_allocation = "NoAllocation"
@ -208,8 +206,6 @@ let ia_is_field_injector_readwrite ia =
List.exists ~f:(ia_ends_with ia) field_injector_readwrite_list List.exists ~f:(ia_ends_with ia) field_injector_readwrite_list
let ia_is_mutable ia = ia_ends_with ia mutable_
let ia_is_verify ia = ia_contains ia verify_annotation let ia_is_verify ia = ia_contains ia verify_annotation
let ia_is_expensive ia = ia_ends_with ia expensive let ia_is_expensive ia = ia_ends_with ia expensive

@ -75,8 +75,6 @@ val ia_is_field_injector_readwrite : Annot.Item.t -> bool
(** Annotations for read-write injectors. (** Annotations for read-write injectors.
The injector framework initializes the field and can write null into it. *) The injector framework initializes the field and can write null into it. *)
val ia_is_mutable : Annot.Item.t -> bool
val ia_is_nonnull : Annot.Item.t -> bool val ia_is_nonnull : Annot.Item.t -> bool
val ia_is_nullable : Annot.Item.t -> bool val ia_is_nullable : Annot.Item.t -> bool

@ -172,28 +172,10 @@ let check_field_assignment tenv find_canonical_duplicate curr_pdesc node instr_r
&& (not (field_is_injector_readwrite ())) && (not (field_is_injector_readwrite ()))
&& not (field_is_in_cleanup_context ()) && not (field_is_in_cleanup_context ())
in in
let should_report_mutable = if should_report_nullable then
let field_is_mutable () =
match t_ia_opt with Some (_, ia) -> Annotations.ia_is_mutable ia | _ -> false
in
Config.eradicate_field_not_mutable
&& (not (Typ.Procname.is_constructor curr_pname))
&& ( match curr_pname with
| Typ.Procname.Java java_pname ->
not (Typ.Procname.Java.is_class_initializer java_pname)
| _ ->
true )
&& not (field_is_mutable ())
in
( if should_report_nullable then
let origin_descr = TypeAnnotation.descr_origin ta_rhs in let origin_descr = TypeAnnotation.descr_origin ta_rhs in
report_error tenv find_canonical_duplicate report_error tenv find_canonical_duplicate
(TypeErr.Field_annotation_inconsistent (fname, origin_descr)) (TypeErr.Field_annotation_inconsistent (fname, origin_descr))
(Some instr_ref) loc curr_pdesc ) ;
if should_report_mutable then
let origin_descr = TypeAnnotation.descr_origin ta_rhs in
report_error tenv find_canonical_duplicate
(TypeErr.Field_not_mutable (fname, origin_descr))
(Some instr_ref) loc curr_pdesc (Some instr_ref) loc curr_pdesc

@ -79,7 +79,6 @@ type err_instance =
| Inconsistent_subclass_return_annotation of Typ.Procname.t * Typ.Procname.t | Inconsistent_subclass_return_annotation of Typ.Procname.t * Typ.Procname.t
| Inconsistent_subclass_parameter_annotation of string * int * Typ.Procname.t * Typ.Procname.t | Inconsistent_subclass_parameter_annotation of string * int * Typ.Procname.t * Typ.Procname.t
| Field_not_initialized of Typ.Fieldname.t * Typ.Procname.t | Field_not_initialized of Typ.Fieldname.t * Typ.Procname.t
| Field_not_mutable of Typ.Fieldname.t * origin_descr
| Field_annotation_inconsistent of Typ.Fieldname.t * origin_descr | Field_annotation_inconsistent of Typ.Fieldname.t * origin_descr
| Field_over_annotated of Typ.Fieldname.t * Typ.Procname.t | Field_over_annotated of Typ.Fieldname.t * Typ.Procname.t
| Null_field_access of string option * Typ.Fieldname.t * origin_descr * bool | Null_field_access of string option * Typ.Fieldname.t * origin_descr * bool
@ -102,27 +101,25 @@ module H = Hashtbl.Make (struct
Hashtbl.hash (1, b, string_opt_hash so, nn) Hashtbl.hash (1, b, string_opt_hash so, nn)
| Field_not_initialized (fn, pn) -> | Field_not_initialized (fn, pn) ->
Hashtbl.hash (2, string_hash (Typ.Fieldname.to_string fn ^ Typ.Procname.to_string pn)) Hashtbl.hash (2, string_hash (Typ.Fieldname.to_string fn ^ Typ.Procname.to_string pn))
| Field_not_mutable (fn, _) ->
Hashtbl.hash (3, string_hash (Typ.Fieldname.to_string fn))
| Field_annotation_inconsistent (fn, _) -> | Field_annotation_inconsistent (fn, _) ->
Hashtbl.hash (4, string_hash (Typ.Fieldname.to_string fn)) Hashtbl.hash (3, string_hash (Typ.Fieldname.to_string fn))
| Field_over_annotated (fn, pn) -> | Field_over_annotated (fn, pn) ->
Hashtbl.hash (5, string_hash (Typ.Fieldname.to_string fn ^ Typ.Procname.to_string pn)) Hashtbl.hash (4, string_hash (Typ.Fieldname.to_string fn ^ Typ.Procname.to_string pn))
| Null_field_access (so, fn, _, _) -> | Null_field_access (so, fn, _, _) ->
Hashtbl.hash (6, string_opt_hash so, string_hash (Typ.Fieldname.to_string fn)) Hashtbl.hash (5, string_opt_hash so, string_hash (Typ.Fieldname.to_string fn))
| Call_receiver_annotation_inconsistent (so, pn, _) -> | Call_receiver_annotation_inconsistent (so, pn, _) ->
Hashtbl.hash (7, string_opt_hash so, Typ.Procname.hash pn) Hashtbl.hash (6, string_opt_hash so, Typ.Procname.hash pn)
| Parameter_annotation_inconsistent (s, n, pn, _, _) -> | Parameter_annotation_inconsistent (s, n, pn, _, _) ->
Hashtbl.hash (8, string_hash s, n, Typ.Procname.hash pn) Hashtbl.hash (7, string_hash s, n, Typ.Procname.hash pn)
| Return_annotation_inconsistent (pn, _) -> | Return_annotation_inconsistent (pn, _) ->
Hashtbl.hash (9, Typ.Procname.hash pn) Hashtbl.hash (8, Typ.Procname.hash pn)
| Return_over_annotated pn -> | Return_over_annotated pn ->
Hashtbl.hash (10, Typ.Procname.hash pn) Hashtbl.hash (9, Typ.Procname.hash pn)
| Inconsistent_subclass_return_annotation (pn, opn) -> | Inconsistent_subclass_return_annotation (pn, opn) ->
Hashtbl.hash (11, Typ.Procname.hash pn, Typ.Procname.hash opn) Hashtbl.hash (10, Typ.Procname.hash pn, Typ.Procname.hash opn)
| Inconsistent_subclass_parameter_annotation (param_name, pos, pn, opn) -> | Inconsistent_subclass_parameter_annotation (param_name, pos, pn, opn) ->
let pn_hash = string_hash param_name in let pn_hash = string_hash param_name in
Hashtbl.hash (12, pn_hash, pos, Typ.Procname.hash pn, Typ.Procname.hash opn) Hashtbl.hash (11, pn_hash, pos, Typ.Procname.hash pn, Typ.Procname.hash opn)
let hash (err_inst, instr_ref_opt) = let hash (err_inst, instr_ref_opt) =
@ -151,8 +148,6 @@ let get_forall = function
true true
| Field_not_initialized _ -> | Field_not_initialized _ ->
false false
| Field_not_mutable _ ->
false
| Field_annotation_inconsistent _ -> | Field_annotation_inconsistent _ ->
false false
| Field_over_annotated _ -> | Field_over_annotated _ ->
@ -257,7 +252,6 @@ type st_report_error =
let report_error_now tenv (st_report_error : st_report_error) err_instance loc pdesc : unit = let report_error_now tenv (st_report_error : st_report_error) err_instance loc pdesc : unit =
let pname = Procdesc.get_proc_name pdesc in let pname = Procdesc.get_proc_name pdesc in
let nullable_annotation = "@Nullable" in let nullable_annotation = "@Nullable" in
let mutable_annotation = "@Mutable" in
let kind, description, field_name = let kind, description, field_name =
match err_instance with match err_instance with
| Condition_redundant (b, s_opt, nonnull) -> | Condition_redundant (b, s_opt, nonnull) ->
@ -285,12 +279,6 @@ let report_error_now tenv (st_report_error : st_report_error) err_instance loc p
(Typ.Fieldname.to_simplified_string fn) (Typ.Fieldname.to_simplified_string fn)
constructor_name MF.pp_monospaced nullable_annotation constructor_name MF.pp_monospaced nullable_annotation
, Some fn ) , Some fn )
| Field_not_mutable (fn, (origin_description, _, _)) ->
( IssueType.eradicate_field_not_mutable
, Format.asprintf "Field %a is modified but is not declared %a. %s" MF.pp_monospaced
(Typ.Fieldname.to_simplified_string fn)
MF.pp_monospaced mutable_annotation origin_description
, None )
| Field_annotation_inconsistent (fn, (origin_description, _, _)) -> | Field_annotation_inconsistent (fn, (origin_description, _, _)) ->
let kind_s, description = let kind_s, description =
( IssueType.eradicate_field_not_nullable ( IssueType.eradicate_field_not_nullable

@ -50,7 +50,6 @@ type err_instance =
| Inconsistent_subclass_return_annotation of Typ.Procname.t * Typ.Procname.t | Inconsistent_subclass_return_annotation of Typ.Procname.t * Typ.Procname.t
| Inconsistent_subclass_parameter_annotation of string * int * Typ.Procname.t * Typ.Procname.t | Inconsistent_subclass_parameter_annotation of string * int * Typ.Procname.t * Typ.Procname.t
| Field_not_initialized of Typ.Fieldname.t * Typ.Procname.t | Field_not_initialized of Typ.Fieldname.t * Typ.Procname.t
| Field_not_mutable of Typ.Fieldname.t * origin_descr
| Field_annotation_inconsistent of Typ.Fieldname.t * origin_descr | Field_annotation_inconsistent of Typ.Fieldname.t * origin_descr
| Field_over_annotated of Typ.Fieldname.t * Typ.Procname.t | Field_over_annotated of Typ.Fieldname.t * Typ.Procname.t
| Null_field_access of string option * Typ.Fieldname.t * origin_descr * bool | Null_field_access of string option * Typ.Fieldname.t * origin_descr * bool

Loading…
Cancel
Save