diff --git a/infer/man/man1/infer-full.txt b/infer/man/man1/infer-full.txt index 6f538e551..dad63e257 100644 --- a/infer/man/man1/infer-full.txt +++ b/infer/man/man1/infer-full.txt @@ -379,18 +379,14 @@ OPTIONS ERADICATE_FIELD_NOT_MUTABLE (enabled by default), ERADICATE_FIELD_NOT_NULLABLE (enabled by default), ERADICATE_FIELD_OVER_ANNOTATED (enabled by default), - ERADICATE_FIELD_VALUE_ABSENT (enabled by default), ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION (enabled by default), ERADICATE_INCONSISTENT_SUBCLASS_RETURN_ANNOTATION (enabled by default), ERADICATE_NULLABLE_DEREFERENCE (enabled by default), ERADICATE_PARAMETER_NOT_NULLABLE (enabled by default), - ERADICATE_PARAMETER_VALUE_ABSENT (enabled by default), ERADICATE_RETURN_NOT_NULLABLE (enabled by default), ERADICATE_RETURN_OVER_ANNOTATED (enabled by default), - ERADICATE_RETURN_VALUE_NOT_PRESENT (enabled by default), - ERADICATE_VALUE_NOT_PRESENT (enabled by default), EXECUTION_TIME_COMPLEXITY_INCREASE (enabled by default), EXECUTION_TIME_COMPLEXITY_INCREASE_COLD_START (enabled by default), @@ -1309,10 +1305,6 @@ INTERNAL OPTIONS Activates: Field over-annotated warnings (Conversely: --no-eradicate-field-over-annotated) - --eradicate-optional-present - Activates: Check for @Present annotations (Conversely: - --no-eradicate-optional-present) - --eradicate-return-over-annotated Activates: Return over-annotated warning (Conversely: --no-eradicate-return-over-annotated) diff --git a/infer/man/man1/infer-report.txt b/infer/man/man1/infer-report.txt index db2b9dc38..048eb30a7 100644 --- a/infer/man/man1/infer-report.txt +++ b/infer/man/man1/infer-report.txt @@ -128,18 +128,14 @@ OPTIONS ERADICATE_FIELD_NOT_MUTABLE (enabled by default), ERADICATE_FIELD_NOT_NULLABLE (enabled by default), ERADICATE_FIELD_OVER_ANNOTATED (enabled by default), - ERADICATE_FIELD_VALUE_ABSENT (enabled by default), ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION (enabled by default), ERADICATE_INCONSISTENT_SUBCLASS_RETURN_ANNOTATION (enabled by default), ERADICATE_NULLABLE_DEREFERENCE (enabled by default), ERADICATE_PARAMETER_NOT_NULLABLE (enabled by default), - ERADICATE_PARAMETER_VALUE_ABSENT (enabled by default), ERADICATE_RETURN_NOT_NULLABLE (enabled by default), ERADICATE_RETURN_OVER_ANNOTATED (enabled by default), - ERADICATE_RETURN_VALUE_NOT_PRESENT (enabled by default), - ERADICATE_VALUE_NOT_PRESENT (enabled by default), EXECUTION_TIME_COMPLEXITY_INCREASE (enabled by default), EXECUTION_TIME_COMPLEXITY_INCREASE_COLD_START (enabled by default), diff --git a/infer/man/man1/infer.txt b/infer/man/man1/infer.txt index d46fea83f..ba1154b0c 100644 --- a/infer/man/man1/infer.txt +++ b/infer/man/man1/infer.txt @@ -379,18 +379,14 @@ OPTIONS ERADICATE_FIELD_NOT_MUTABLE (enabled by default), ERADICATE_FIELD_NOT_NULLABLE (enabled by default), ERADICATE_FIELD_OVER_ANNOTATED (enabled by default), - ERADICATE_FIELD_VALUE_ABSENT (enabled by default), ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION (enabled by default), ERADICATE_INCONSISTENT_SUBCLASS_RETURN_ANNOTATION (enabled by default), ERADICATE_NULLABLE_DEREFERENCE (enabled by default), ERADICATE_PARAMETER_NOT_NULLABLE (enabled by default), - ERADICATE_PARAMETER_VALUE_ABSENT (enabled by default), ERADICATE_RETURN_NOT_NULLABLE (enabled by default), ERADICATE_RETURN_OVER_ANNOTATED (enabled by default), - ERADICATE_RETURN_VALUE_NOT_PRESENT (enabled by default), - ERADICATE_VALUE_NOT_PRESENT (enabled by default), EXECUTION_TIME_COMPLEXITY_INCREASE (enabled by default), EXECUTION_TIME_COMPLEXITY_INCREASE_COLD_START (enabled by default), diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml index bf289adf9..6c3581340 100644 --- a/infer/src/base/Config.ml +++ b/infer/src/base/Config.ml @@ -1407,10 +1407,6 @@ and eradicate_field_over_annotated = CLOpt.mk_bool ~long:"eradicate-field-over-annotated" "Field over-annotated warnings" -and eradicate_optional_present = - CLOpt.mk_bool ~long:"eradicate-optional-present" "Check for @Present annotations" - - and eradicate_return_over_annotated = CLOpt.mk_bool ~long:"eradicate-return-over-annotated" "Return over-annotated warning" @@ -2864,8 +2860,6 @@ and eradicate_field_not_mutable = !eradicate_field_not_mutable and eradicate_field_over_annotated = !eradicate_field_over_annotated -and eradicate_optional_present = !eradicate_optional_present - and eradicate_return_over_annotated = !eradicate_return_over_annotated and eradicate_verbose = !eradicate_verbose diff --git a/infer/src/base/Config.mli b/infer/src/base/Config.mli index 63f44123e..4c9b9dee9 100644 --- a/infer/src/base/Config.mli +++ b/infer/src/base/Config.mli @@ -352,8 +352,6 @@ val eradicate_field_not_mutable : bool val eradicate_field_over_annotated : bool -val eradicate_optional_present : bool - val eradicate_return_over_annotated : bool val eradicate_verbose : bool diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index c88c1210e..cce8f2113 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -243,10 +243,6 @@ let eradicate_field_over_annotated = register_from_string "ERADICATE_FIELD_OVER_ANNOTATED" ~hum:"Field Over Annotated" -let eradicate_field_value_absent = - register_from_string "ERADICATE_FIELD_VALUE_ABSENT" ~hum:"Field Value Absent" - - let eradicate_inconsistent_subclass_parameter_annotation = register_from_string "ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION" ~hum:"Inconsistent Subclass Parameter Annotation" @@ -265,10 +261,6 @@ let eradicate_parameter_not_nullable = register_from_string "ERADICATE_PARAMETER_NOT_NULLABLE" ~hum:"Parameter Not Nullable" -let eradicate_parameter_value_absent = - register_from_string "ERADICATE_PARAMETER_VALUE_ABSENT" ~hum:"Parameter Value Absent" - - let eradicate_return_not_nullable = register_from_string "ERADICATE_RETURN_NOT_NULLABLE" ~hum:"Return Not Nullable" @@ -277,14 +269,6 @@ let eradicate_return_over_annotated = register_from_string "ERADICATE_RETURN_OVER_ANNOTATED" ~hum:"Return Over Annotated" -let eradicate_return_value_not_present = - register_from_string "ERADICATE_RETURN_VALUE_NOT_PRESENT" ~hum:"Return Value Not Present" - - -let eradicate_value_not_present = - register_from_string "ERADICATE_VALUE_NOT_PRESENT" ~hum:"Value Not Present" - - let expensive_cost_call ~kind ~is_on_cold_start = register_from_cost_string ~enabled:false ~kind ~is_on_cold_start "EXPENSIVE_%s" diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index 9b5617618..047ad8566 100644 --- a/infer/src/base/IssueType.mli +++ b/infer/src/base/IssueType.mli @@ -32,7 +32,7 @@ val register_from_string : `enabled`. This trick allows to deal with disabling/enabling dynamic AL issues from the config, when we don't know all params yet. Thus, the human-readable description can be updated when we encounter the - definition of the issue type, eg in AL. + definition of the issue type, eg in AL. *) val set_enabled : t -> bool -> unit @@ -152,8 +152,6 @@ val eradicate_field_not_nullable : t val eradicate_field_over_annotated : t -val eradicate_field_value_absent : t - val eradicate_inconsistent_subclass_parameter_annotation : t val eradicate_inconsistent_subclass_return_annotation : t @@ -162,16 +160,10 @@ val eradicate_nullable_dereference : t val eradicate_parameter_not_nullable : t -val eradicate_parameter_value_absent : t - val eradicate_return_not_nullable : t val eradicate_return_over_annotated : t -val eradicate_return_value_not_present : t - -val eradicate_value_not_present : t - val expensive_cost_call : kind:CostKind.t -> is_on_cold_start:bool -> t val exposed_insecure_intent_handling : t diff --git a/infer/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml index 23b77f26b..6aa9d5760 100644 --- a/infer/src/checkers/annotations.ml +++ b/infer/src/checkers/annotations.ml @@ -69,8 +69,6 @@ let not_thread_safe = "NotThreadSafe" let performance_critical = "PerformanceCritical" -let present = "Present" - let prop = "Prop" let propagates_nullable = "PropagatesNullable" @@ -163,8 +161,6 @@ let ia_is_propagates_nullable ia = ia_ends_with ia propagates_nullable let ia_is_nullable ia = ia_ends_with ia nullable || ia_is_propagates_nullable ia -let ia_is_present ia = ia_ends_with ia present - let ia_is_nonnull ia = List.exists ~f:(ia_ends_with ia) [nonnull; notnull; camel_nonnull] let ia_is_false_on_null ia = ia_ends_with ia false_on_null diff --git a/infer/src/checkers/annotations.mli b/infer/src/checkers/annotations.mli index 1b23bf558..677dfc1ef 100644 --- a/infer/src/checkers/annotations.mli +++ b/infer/src/checkers/annotations.mli @@ -25,8 +25,6 @@ val nonnull : string val performance_critical : string -val present : string - val prop : string val for_non_ui_thread : string @@ -83,8 +81,6 @@ val ia_is_nonnull : Annot.Item.t -> bool val ia_is_nullable : Annot.Item.t -> bool -val ia_is_present : Annot.Item.t -> bool - val ia_is_true_on_null : Annot.Item.t -> bool val ia_is_verify : Annot.Item.t -> bool diff --git a/infer/src/nullsafe/AnnotatedSignature.ml b/infer/src/nullsafe/AnnotatedSignature.ml index f83e4e642..34f07bad8 100644 --- a/infer/src/nullsafe/AnnotatedSignature.ml +++ b/infer/src/nullsafe/AnnotatedSignature.ml @@ -11,15 +11,9 @@ module L = Logging type t = {ret: Annot.Item.t * Typ.t; params: (Mangled.t * Annot.Item.t * Typ.t) list} [@@deriving compare] -type annotation = Nullable | Present [@@deriving compare] - -let ia_is ann ia = - match ann with - | Nullable -> - Annotations.ia_is_nullable ia - | Present -> - Annotations.ia_is_present ia +type annotation = Nullable [@@deriving compare] +let ia_is ann ia = match ann with Nullable -> Annotations.ia_is_nullable ia let get proc_attributes : t = let method_annotation = proc_attributes.ProcAttributes.method_annotation in @@ -64,12 +58,7 @@ let pp proc_name fmt annotated_signature = let mk_ann_str s = {Annot.class_name= s; parameters= []} -let mk_ann = function - | Nullable -> - mk_ann_str Annotations.nullable - | Present -> - mk_ann_str Annotations.present - +let mk_ann = function Nullable -> mk_ann_str Annotations.nullable let mk_ia ann ia = if ia_is ann ia then ia else (mk_ann ann, true) :: ia diff --git a/infer/src/nullsafe/AnnotatedSignature.mli b/infer/src/nullsafe/AnnotatedSignature.mli index e438bcf12..7c71aa6dc 100644 --- a/infer/src/nullsafe/AnnotatedSignature.mli +++ b/infer/src/nullsafe/AnnotatedSignature.mli @@ -14,7 +14,7 @@ type t = ; params: (Mangled.t * Annot.Item.t * Typ.t) list (** Annotated parameters. *) } [@@deriving compare] -type annotation = Nullable | Present [@@deriving compare] +type annotation = Nullable [@@deriving compare] val param_has_annot : (Annot.Item.t -> bool) -> Pvar.t -> t -> bool (** Check if the given parameter has an annotation in the given signature *) diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml index 6fa2e434d..419757500 100644 --- a/infer/src/nullsafe/eradicateChecks.ml +++ b/infer/src/nullsafe/eradicateChecks.ml @@ -39,7 +39,7 @@ let is_virtual = function (p, _, _) :: _ when Mangled.is_this p -> true | _ -> f (** Check an access (read or write) to a field. *) let check_field_access tenv find_canonical_duplicate curr_pname node instr_ref exp fname ta loc : unit = - if TypeAnnotation.get_value AnnotatedSignature.Nullable ta then + if TypeAnnotation.is_nullable ta then let origin_descr = TypeAnnotation.descr_origin ta in report_error tenv find_canonical_duplicate (TypeErr.Null_field_access (explain_expr tenv node exp, fname, origin_descr, false)) @@ -49,7 +49,7 @@ let check_field_access tenv find_canonical_duplicate curr_pname node instr_ref e (** Check an access to an array *) let check_array_access tenv find_canonical_duplicate curr_pname node instr_ref array_exp fname ta loc indexed = - if TypeAnnotation.get_value AnnotatedSignature.Nullable ta then + if TypeAnnotation.is_nullable ta then let origin_descr = TypeAnnotation.descr_origin ta in report_error tenv find_canonical_duplicate (TypeErr.Null_field_access (explain_expr tenv node array_exp, fname, origin_descr, indexed)) @@ -62,7 +62,6 @@ type from_call = | From_instanceof (** x instanceof C *) | From_is_false_on_null (** returns false on null *) | From_is_true_on_null (** returns true on null *) - | From_optional_isPresent (** x.isPresent *) | From_containsKey (** x.containsKey *) [@@deriving compare] @@ -117,7 +116,7 @@ let check_condition tenv case_zero find_canonical_duplicate curr_pdesc node e ty let is_temp = Idenv.exp_is_temp idenv e in let nonnull = is_fun_nonnull ta in let should_report = - (not (TypeAnnotation.get_value AnnotatedSignature.Nullable ta)) + (not (TypeAnnotation.is_nullable ta)) && (Config.eradicate_condition_redundant || nonnull) && true_branch && ((not is_temp) || nonnull) && PatternMatch.type_is_class typ && (not (from_try_with_resources ())) @@ -147,12 +146,12 @@ let check_field_assignment tenv find_canonical_duplicate curr_pdesc node instr_r let curr_pattrs = Procdesc.get_attributes curr_pdesc in let t_lhs, ta_lhs, _ = typecheck_expr node instr_ref curr_pdesc typestate exp_lhs - (typ, TypeAnnotation.const AnnotatedSignature.Nullable false TypeOrigin.ONone, [loc]) + (typ, TypeAnnotation.const_nullable false TypeOrigin.ONone, [loc]) loc in let _, ta_rhs, _ = typecheck_expr node instr_ref curr_pdesc typestate exp_rhs - (typ, TypeAnnotation.const AnnotatedSignature.Nullable false TypeOrigin.ONone, [loc]) + (typ, TypeAnnotation.const_nullable false TypeOrigin.ONone, [loc]) loc in let field_is_injector_readwrite () = @@ -168,19 +167,12 @@ let check_field_assignment tenv find_canonical_duplicate curr_pdesc node instr_r in let should_report_nullable = (not (AndroidFramework.is_destroy_method curr_pname)) - && (not (TypeAnnotation.get_value AnnotatedSignature.Nullable ta_lhs)) - && TypeAnnotation.get_value AnnotatedSignature.Nullable ta_rhs - && PatternMatch.type_is_class t_lhs + && (not (TypeAnnotation.is_nullable ta_lhs)) + && TypeAnnotation.is_nullable ta_rhs && PatternMatch.type_is_class t_lhs && (not (Typ.Fieldname.Java.is_outer_instance fname)) && (not (field_is_injector_readwrite ())) && not (field_is_in_cleanup_context ()) in - let should_report_absent = - Config.eradicate_optional_present - && TypeAnnotation.get_value AnnotatedSignature.Present ta_lhs - && (not (TypeAnnotation.get_value AnnotatedSignature.Present ta_rhs)) - && not (Typ.Fieldname.Java.is_outer_instance fname) - in let should_report_mutable = let field_is_mutable () = match t_ia_opt with Some (_, ia) -> Annotations.ia_is_mutable ia | _ -> false @@ -194,13 +186,10 @@ let check_field_assignment tenv find_canonical_duplicate curr_pdesc node instr_r true ) && not (field_is_mutable ()) in - ( if should_report_nullable || should_report_absent then - let ann = - if should_report_nullable then AnnotatedSignature.Nullable else AnnotatedSignature.Present - in + ( if should_report_nullable then let origin_descr = TypeAnnotation.descr_origin ta_rhs in report_error tenv find_canonical_duplicate - (TypeErr.Field_annotation_inconsistent (ann, 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 @@ -253,7 +242,7 @@ let check_constructor_initialization tenv find_canonical_duplicate curr_pname cu in let may_be_nullable_in_final_typestate () = final_type_annotation_with true (Lazy.force final_constructor_typestates) (fun ta -> - TypeAnnotation.get_value AnnotatedSignature.Nullable ta ) + TypeAnnotation.is_nullable ta ) in let should_check_field_initialization = let in_current_class = @@ -300,7 +289,6 @@ let check_return_annotation tenv find_canonical_duplicate curr_pdesc ret_range ~f:(fun (_, ia, _) -> Annotations.ia_is_propagates_nullable ia) annotated_signature.params in - let ret_annotated_present = Annotations.ia_is_present ret_ia in match ret_range with (* Disables the warnings since it is not clear how to annotate the return value of lambdas *) | Some _ @@ -311,25 +299,18 @@ let check_return_annotation tenv find_canonical_duplicate curr_pdesc ret_range false -> () | Some (_, final_ta, _) -> - let final_nullable = TypeAnnotation.get_value AnnotatedSignature.Nullable final_ta in - let final_present = TypeAnnotation.get_value AnnotatedSignature.Present final_ta in + let is_final_nullable = TypeAnnotation.is_nullable final_ta in let origin_descr = TypeAnnotation.descr_origin final_ta in let return_not_nullable = - final_nullable && (not ret_annotated_nullable) && not ret_implicitly_nullable - in - let return_value_not_present = - Config.eradicate_optional_present && (not final_present) && ret_annotated_present + is_final_nullable && (not ret_annotated_nullable) && not ret_implicitly_nullable in let return_over_annotated = - (not final_nullable) && ret_annotated_nullable && Config.eradicate_return_over_annotated + (not is_final_nullable) && ret_annotated_nullable && Config.eradicate_return_over_annotated in - ( if return_not_nullable || return_value_not_present then - let ann = - if return_not_nullable then AnnotatedSignature.Nullable else AnnotatedSignature.Present - in + if return_not_nullable then report_error tenv find_canonical_duplicate - (TypeErr.Return_annotation_inconsistent (ann, curr_pname, origin_descr)) - None loc curr_pdesc ) ; + (TypeErr.Return_annotation_inconsistent (curr_pname, origin_descr)) + None loc curr_pdesc ; if return_over_annotated then report_error tenv find_canonical_duplicate (TypeErr.Return_over_annotated curr_pname) None loc curr_pdesc @@ -344,23 +325,15 @@ let check_call_receiver tenv find_canonical_duplicate curr_pdesc node typestate | ((original_this_e, this_e), typ) :: _ -> let _, this_ta, _ = typecheck_expr tenv node instr_ref curr_pdesc typestate this_e - (typ, TypeAnnotation.const AnnotatedSignature.Nullable false TypeOrigin.ONone, []) + (typ, TypeAnnotation.const_nullable false TypeOrigin.ONone, []) loc in - let null_method_call = TypeAnnotation.get_value AnnotatedSignature.Nullable this_ta in - let optional_get_on_absent = - Config.eradicate_optional_present - && Models.is_optional_get callee_pname - && not (TypeAnnotation.get_value AnnotatedSignature.Present this_ta) - in - if null_method_call || optional_get_on_absent then - let ann = - if null_method_call then AnnotatedSignature.Nullable else AnnotatedSignature.Present - in + let null_method_call = TypeAnnotation.is_nullable this_ta in + if null_method_call then let descr = explain_expr tenv node original_this_e in let origin_descr = TypeAnnotation.descr_origin this_ta in report_error tenv find_canonical_duplicate - (TypeErr.Call_receiver_annotation_inconsistent (ann, descr, callee_pname, origin_descr)) + (TypeErr.Call_receiver_annotation_inconsistent (descr, callee_pname, origin_descr)) (Some instr_ref) loc curr_pdesc | [] -> () @@ -376,8 +349,9 @@ type resolved_param = let check_call_parameters tenv find_canonical_duplicate curr_pdesc node callee_attributes resolved_params loc instr_ref : unit = let callee_pname = callee_attributes.ProcAttributes.proc_name in - let check {num= param_num; formal= s1, ta1, t1; actual= orig_e2, ta2} = - let report ann = + let check {num= param_num; formal= s1, annotation_formal, t1; actual= orig_e2, annotation_actual} + = + let report () = let description = match explain_expr tenv node orig_e2 with | Some descr -> @@ -385,27 +359,17 @@ let check_call_parameters tenv find_canonical_duplicate curr_pdesc node callee_a | None -> "formal parameter " ^ Mangled.to_string s1 in - let origin_descr = TypeAnnotation.descr_origin ta2 in + let origin_descr = TypeAnnotation.descr_origin annotation_actual in let callee_loc = callee_attributes.ProcAttributes.loc in report_error tenv find_canonical_duplicate (TypeErr.Parameter_annotation_inconsistent - (ann, description, param_num, callee_pname, callee_loc, origin_descr)) + (description, param_num, callee_pname, callee_loc, origin_descr)) (Some instr_ref) loc curr_pdesc in - let check_ann ann = - let b1 = TypeAnnotation.get_value ann ta1 in - let b2 = TypeAnnotation.get_value ann ta2 in - match (ann, b1, b2) with - | AnnotatedSignature.Nullable, false, true -> - report ann - | AnnotatedSignature.Present, true, false -> - report ann - | _ -> - () - in - if PatternMatch.type_is_class t1 then ( - check_ann AnnotatedSignature.Nullable ; - if Config.eradicate_optional_present then check_ann AnnotatedSignature.Present ) + if PatternMatch.type_is_class t1 then + let is_nullable_formal = TypeAnnotation.is_nullable annotation_formal in + let is_nullable_actual = TypeAnnotation.is_nullable annotation_actual in + if (not is_nullable_formal) && is_nullable_actual then report () in let should_check_parameters = match callee_pname with diff --git a/infer/src/nullsafe/modelTables.ml b/infer/src/nullsafe/modelTables.ml index becaebbb3..df7091079 100644 --- a/infer/src/nullsafe/modelTables.ml +++ b/infer/src/nullsafe/modelTables.ml @@ -182,16 +182,6 @@ let check_argument_list = ) ] -let optional_get_list : ((_ * bool list) * _) list = - [ ((o, []), "Optional.get():java.lang.Object") - ; ((o, []), "com.google.common.base.Optional.get():java.lang.Object") ] - - -let optional_isPresent_list : ((_ * bool list) * _) list = - [ ((o, []), "Optional.isPresent():boolean") - ; ((o, []), "com.google.common.base.Optional.isPresent():boolean") ] - - (** Models for boolean functions that return true on null. *) let true_on_null_list : ((_ * bool list) * _) list = [ (n1, "android.text.TextUtils.isEmpty(java.lang.CharSequence):boolean") @@ -581,13 +571,6 @@ let annotated_list_nullable = ; (ng, "java.util.concurrent.atomic.AtomicReference.get():java.lang.Object") ] -(** Models for @Present annotations *) -let annotated_list_present = - [ ((n, [o]), "Optional.of(java.lang.Object):Optional") - ; ( (n, [o]) - , "com.google.common.base.Optional.of(java.lang.Object):com.google.common.base.Optional" ) ] - - (** Models for methods that do not return *) let noreturn_list = [((o, [o]), "java.lang.System.exit(int):void")] @@ -603,8 +586,6 @@ let this_file = Filename.basename __FILE__ let annotated_table_nullable = mk_table annotated_list_nullable -let annotated_table_present = mk_table annotated_list_present - let check_not_null_table, check_not_null_parameter_table = (mk_table check_not_null_list, mk_table check_not_null_parameter_list) @@ -617,10 +598,6 @@ let containsKey_table = mk_table containsKey_list let mapPut_table = mk_table mapPut_list -let optional_get_table = mk_table optional_get_list - -let optional_isPresent_table = mk_table optional_isPresent_list - let noreturn_table = mk_table noreturn_list let true_on_null_table = mk_table true_on_null_list diff --git a/infer/src/nullsafe/modelTables.mli b/infer/src/nullsafe/modelTables.mli index 7b7ca65ea..268a3eb93 100644 --- a/infer/src/nullsafe/modelTables.mli +++ b/infer/src/nullsafe/modelTables.mli @@ -14,8 +14,6 @@ val this_file : string val annotated_table_nullable : model_table_t -val annotated_table_present : model_table_t - val check_not_null_table : model_table_t val check_not_null_parameter_table : (string, int) Caml.Hashtbl.t @@ -28,10 +26,6 @@ val containsKey_table : model_table_t val mapPut_table : model_table_t -val optional_get_table : model_table_t - -val optional_isPresent_table : model_table_t - val noreturn_table : model_table_t val true_on_null_table : model_table_t diff --git a/infer/src/nullsafe/models.ml b/infer/src/nullsafe/models.ml index 4c07862c2..13535fa87 100644 --- a/infer/src/nullsafe/models.ml +++ b/infer/src/nullsafe/models.ml @@ -38,13 +38,7 @@ let get_modelled_annotated_signature proc_attributes = AnnotatedSignature.mark proc_name AnnotatedSignature.Nullable ann_sig mark with Caml.Not_found -> ann_sig in - let lookup_models_present ann_sig = - try - let mark = Hashtbl.find annotated_table_present proc_id in - AnnotatedSignature.mark proc_name AnnotatedSignature.Present ann_sig mark - with Caml.Not_found -> ann_sig - in - annotated_signature |> lookup_models_nullable |> lookup_models_present + annotated_signature |> lookup_models_nullable (** Return true when the procedure has been modelled for nullable. *) @@ -80,12 +74,6 @@ let is_check_argument proc_name = (** Check if the procedure does not return. *) let is_noreturn proc_name = table_has_procedure noreturn_table proc_name -(** Check if the procedure is Optional.get(). *) -let is_optional_get proc_name = table_has_procedure optional_get_table proc_name - -(** Check if the procedure is Optional.isPresent(). *) -let is_optional_isPresent proc_name = table_has_procedure optional_isPresent_table proc_name - (** Check if the procedure returns true on null. *) let is_true_on_null proc_name = table_has_procedure true_on_null_table proc_name diff --git a/infer/src/nullsafe/typeAnnotation.ml b/infer/src/nullsafe/typeAnnotation.ml index e68a7d9b1..d9c04e075 100644 --- a/infer/src/nullsafe/typeAnnotation.ml +++ b/infer/src/nullsafe/typeAnnotation.ml @@ -17,20 +17,12 @@ type t = {map: bool AnnotationsMap.t; origin: TypeOrigin.t} [@@deriving compare] let equal = [%compare.equal: t] -let get_value ann ta = try AnnotationsMap.find ann ta.map with Caml.Not_found -> false +let is_nullable ta = try AnnotationsMap.find Nullable ta.map with Caml.Not_found -> false -let set_value ann b ta = - if Bool.equal (get_value ann ta) b then ta else {ta with map= AnnotationsMap.add ann b ta.map} +let set_nullable b ta = + if Bool.equal (is_nullable ta) b then ta else {ta with map= AnnotationsMap.add Nullable b ta.map} -let get_nullable = get_value AnnotatedSignature.Nullable - -let get_present = get_value Present - -let set_nullable b = set_value Nullable b - -let set_present b = set_value Present b - let descr_origin ta = let descr_opt = TypeOrigin.get_description ta.origin in match descr_opt with @@ -40,22 +32,17 @@ let descr_origin ta = ("(Origin: " ^ str ^ ")", loc_opt, sig_opt) -let to_string ta = - let nullable_s = if get_nullable ta then " @Nullable" else "" in - let present_s = if get_present ta then " @Present" else "" in - nullable_s ^ present_s - +let to_string ta = if is_nullable ta then " @Nullable" else "" let join ta1 ta2 = - let nul1, nul2 = (get_nullable ta1, get_nullable ta2) in + 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 present = get_present ta1 && get_present ta2 in let origin = if Bool.equal nul1 nul2 then TypeOrigin.join ta_chosen.origin ta_other.origin else ta_chosen.origin in - let ta' = set_present present {ta_chosen with origin} in + let ta' = {ta_chosen with origin} in if equal ta' ta1 then None else Some ta' @@ -69,20 +56,11 @@ let origin_is_fun_library ta = false -let const annotation b origin = - let nullable, present = - match annotation with - | AnnotatedSignature.Nullable -> - (b, false) - | AnnotatedSignature.Present -> - (false, b) - in +let const_nullable b origin = let ta = {origin; map= AnnotationsMap.empty} in - set_present present (set_nullable nullable ta) + set_nullable b ta let with_origin ta o = {ta with origin= o} -let from_item_annotation ia origin = - let ta = const Nullable (Annotations.ia_is_nullable ia) origin in - set_value Present (Annotations.ia_is_present ia) ta +let from_item_annotation ia origin = const_nullable (Annotations.ia_is_nullable ia) origin diff --git a/infer/src/nullsafe/typeAnnotation.mli b/infer/src/nullsafe/typeAnnotation.mli index b26659489..8aba0b72f 100644 --- a/infer/src/nullsafe/typeAnnotation.mli +++ b/infer/src/nullsafe/typeAnnotation.mli @@ -11,7 +11,7 @@ open! IStd type t [@@deriving compare] -val const : AnnotatedSignature.annotation -> bool -> TypeOrigin.t -> t +val const_nullable : bool -> TypeOrigin.t -> t val descr_origin : t -> TypeErr.origin_descr (** Human-readable description of the origin of a nullable value. *) @@ -20,13 +20,13 @@ val from_item_annotation : Annot.Item.t -> TypeOrigin.t -> t val get_origin : t -> TypeOrigin.t -val get_value : AnnotatedSignature.annotation -> t -> bool +val is_nullable : t -> bool val join : t -> t -> t option val origin_is_fun_library : t -> bool -val set_value : AnnotatedSignature.annotation -> bool -> t -> t +val set_nullable : bool -> t -> t val to_string : t -> string diff --git a/infer/src/nullsafe/typeCheck.ml b/infer/src/nullsafe/typeCheck.ml index fde696115..80c0ac684 100644 --- a/infer/src/nullsafe/typeCheck.ml +++ b/infer/src/nullsafe/typeCheck.ml @@ -13,8 +13,6 @@ module DExp = DecompiledExp (** Module to treat selected complex expressions as constants. *) module ComplexExpressions = struct - let procname_optional_isPresent = Models.is_optional_isPresent - let procname_instanceof = Typ.Procname.equal BuiltinDecl.__instanceof let procname_is_false_on_null tenv pn = @@ -44,8 +42,7 @@ module ComplexExpressions = struct (** Recognize *all* the procedures treated specially in conditionals *) let procname_used_in_condition pn = - procname_optional_isPresent pn || procname_instanceof pn || procname_containsKey pn - || BuiltinDecl.is_declared pn + procname_instanceof pn || procname_containsKey pn || BuiltinDecl.is_declared pn exception Not_handled @@ -118,7 +115,7 @@ let rec typecheck_expr find_canonical_duplicate visited checks tenv node instr_r | _ when Exp.is_null_literal e -> let typ, ta, locs = tr_default in if PatternMatch.type_is_class typ then - (typ, TypeAnnotation.const AnnotatedSignature.Nullable true (TypeOrigin.Const loc), locs) + (typ, TypeAnnotation.const_nullable true (TypeOrigin.Const loc), locs) else (typ, TypeAnnotation.with_origin ta (TypeOrigin.Const loc), locs) | Exp.Lvar pvar -> ( match TypeState.lookup_pvar pvar typestate with @@ -137,13 +134,13 @@ let rec typecheck_expr find_canonical_duplicate visited checks tenv node instr_r typestate e1 tr_default loc | Exp.Const _ -> let typ, _, locs = tr_default in - (typ, TypeAnnotation.const AnnotatedSignature.Nullable false (TypeOrigin.Const loc), locs) + (typ, TypeAnnotation.const_nullable false (TypeOrigin.Const loc), locs) | Exp.Lfield (exp, fn, typ) -> let _, _, locs = tr_default in let _, ta, locs' = typecheck_expr find_canonical_duplicate visited checks tenv node instr_ref curr_pdesc typestate exp - (typ, TypeAnnotation.const AnnotatedSignature.Nullable false TypeOrigin.ONone, locs) + (typ, TypeAnnotation.const_nullable false TypeOrigin.ONone, locs) loc in let exp_origin = TypeAnnotation.get_origin ta in @@ -407,7 +404,7 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p let typecheck_expr_simple typestate1 exp1 typ1 origin1 loc1 = typecheck_expr find_canonical_duplicate calls_this checks tenv node instr_ref curr_pdesc typestate1 exp1 - (typ1, TypeAnnotation.const AnnotatedSignature.Nullable false origin1, [loc1]) + (typ1, TypeAnnotation.const_nullable false origin1, [loc1]) loc1 in (* check if there are errors in exp1 *) @@ -461,7 +458,7 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p when Typ.Procname.equal pn BuiltinDecl.__new || Typ.Procname.equal pn BuiltinDecl.__new_array -> TypeState.add_id id - (typ, TypeAnnotation.const AnnotatedSignature.Nullable false TypeOrigin.New, [loc]) + (typ, TypeAnnotation.const_nullable false TypeOrigin.New, [loc]) typestate (* new never returns null *) | Sil.Call ((id, _), Exp.Const (Const.Cfun pn), (e, typ) :: _, loc, _) @@ -475,7 +472,7 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p let _, ta, _ = typecheck_expr find_canonical_duplicate calls_this checks tenv node instr_ref curr_pdesc typestate array_exp - (t, TypeAnnotation.const AnnotatedSignature.Nullable false TypeOrigin.ONone, [loc]) + (t, TypeAnnotation.const_nullable false TypeOrigin.ONone, [loc]) loc in if checks.eradicate then @@ -484,9 +481,7 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p (Typ.Fieldname.Java.from_string "length") ta loc false ; TypeState.add_id id - ( Typ.mk (Tint Typ.IInt) - , TypeAnnotation.const AnnotatedSignature.Nullable false TypeOrigin.New - , [loc] ) + (Typ.mk (Tint Typ.IInt), TypeAnnotation.const_nullable false TypeOrigin.New, [loc]) typestate | Sil.Call (_, Exp.Const (Const.Cfun pn), _, _, _) when BuiltinDecl.is_declared pn -> typestate (* skip othe builtins *) @@ -550,7 +545,7 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p | Some (t, ta, _) -> let should_report = Config.eradicate_condition_redundant - && (not (TypeAnnotation.get_value AnnotatedSignature.Nullable ta)) + && (not (TypeAnnotation.is_nullable ta)) && not (TypeAnnotation.origin_is_fun_library ta) in ( if checks.eradicate && should_report then @@ -560,7 +555,7 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p (true, EradicateChecks.explain_expr tenv node cond, false)) (Some instr_ref) loc curr_pdesc ) ; TypeState.add pvar - (t, TypeAnnotation.const AnnotatedSignature.Nullable false TypeOrigin.ONone, [loc]) + (t, TypeAnnotation.const_nullable false TypeOrigin.ONone, [loc]) typestate'' | None -> typestate' @@ -592,18 +587,20 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p in (* Handle Preconditions.checkState for &&-separated conditions x!=null. *) let do_preconditions_check_state typestate' = - let handle_pvar ann b typestate1 pvar = + let handle_pvar b typestate1 pvar = (* handle the annotation flag for pvar *) match TypeState.lookup_pvar pvar typestate1 with | Some (t, _, _) -> - TypeState.add pvar (t, TypeAnnotation.const ann b TypeOrigin.ONone, [loc]) typestate1 + TypeState.add pvar + (t, TypeAnnotation.const_nullable b TypeOrigin.ONone, [loc]) + typestate1 | None -> typestate1 in let res_typestate = ref typestate' in - let set_flag pvar ann b = + let set_nullable_flag pvar b = (* set the annotation flag for pvar *) - res_typestate := pvar_apply loc (handle_pvar ann b) !res_typestate pvar + res_typestate := pvar_apply loc (handle_pvar b) !res_typestate pvar in let handle_negated_condition cond_node = let do_instr instr = @@ -611,7 +608,7 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p let cond_e = Idenv.expand_expr_temps idenv cond_node expression in match convert_complex_exp_to_pvar cond_node false cond_e typestate' loc with | Exp.Lvar pvar', _ -> - set_flag pvar' AnnotatedSignature.Nullable false + set_nullable_flag pvar' false | _ -> () in @@ -627,13 +624,6 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p in Instrs.iter ~f:do_instr (Procdesc.Node.get_instrs cond_node) in - let handle_optional_isPresent node' e = - match convert_complex_exp_to_pvar node' false e typestate' loc with - | Exp.Lvar pvar', _ -> - set_flag pvar' AnnotatedSignature.Present true - | _ -> - () - in match call_params with | ((_, Exp.Lvar pvar), _) :: _ -> ( (* temporary variable for the value of the boolean condition *) @@ -647,19 +637,6 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p (Procdesc.Node.get_preds boolean_assignment_node) ; !res_typestate | None -> - ( match Errdesc.find_program_variable_assignment curr_node pvar with - | None -> - () - | Some (node', id) -> - let () = - match Errdesc.find_normal_variable_funcall node' id with - | Some (Exp.Const (Const.Cfun pn), [e], _, _) - when ComplexExpressions.procname_optional_isPresent pn -> - handle_optional_isPresent node' e - | _ -> - () - in - () ) ; !res_typestate ) | _ -> typestate' @@ -712,14 +689,14 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p let _, ta2, _ = typecheck_expr find_canonical_duplicate calls_this checks tenv node instr_ref curr_pdesc typestate e2 - (t2, TypeAnnotation.const AnnotatedSignature.Nullable false TypeOrigin.ONone, []) + (t2, TypeAnnotation.const_nullable false TypeOrigin.ONone, []) loc in let formal = (s1, ta1, t1) in let actual = (orig_e2, ta2) in let num = i + 1 in let formal_is_propagates_nullable = Annotations.ia_is_propagates_nullable ia1 in - let actual_is_nullable = TypeAnnotation.get_value AnnotatedSignature.Nullable ta2 in + let actual_is_nullable = TypeAnnotation.is_nullable ta2 in let propagates_nullable = formal_is_propagates_nullable && actual_is_nullable in EradicateChecks.{num; formal; actual; propagates_nullable} in @@ -733,14 +710,10 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p let resolved_ret' = let ret_ta, ret_typ = resolved_ret in let ret_ta' = - let actual_nullable = - TypeAnnotation.get_value AnnotatedSignature.Nullable actual_ta - in - let old_nullable = - TypeAnnotation.get_value AnnotatedSignature.Nullable ret_ta - in - let new_nullable = old_nullable || actual_nullable in - TypeAnnotation.set_value AnnotatedSignature.Nullable new_nullable ret_ta + let is_actual_nullable = TypeAnnotation.is_nullable actual_ta in + let is_old_nullable = TypeAnnotation.is_nullable ret_ta in + let is_new_nullable = is_old_nullable || is_actual_nullable in + TypeAnnotation.set_nullable is_new_nullable ret_ta in (ret_ta', ret_typ) in @@ -834,10 +807,6 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p let from_instanceof e : Exp.t option = from_call ComplexExpressions.procname_instanceof e in - (* check if the expression is coming from Optional.isPresent *) - let from_optional_isPresent e : Exp.t option = - from_call ComplexExpressions.procname_optional_isPresent e - in (* check if the expression is coming from a procedure returning false on null *) let from_is_false_on_null e : Exp.t option = from_call (ComplexExpressions.procname_is_false_on_null tenv) e @@ -882,13 +851,13 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p | None -> (typestate, e, EradicateChecks.From_condition) in - let set_flag e' ann b typestate2 = + let set_nullable_flag e' b typestate2 = (* add constraint on e' for annotation ann *) let handle_pvar typestate' pvar = match TypeState.lookup_pvar pvar typestate' with | Some (t, ta1, locs) -> - if TypeAnnotation.get_value ann ta1 <> b then - let ta2 = TypeAnnotation.set_value ann b ta1 in + if TypeAnnotation.is_nullable ta1 <> b then + let ta2 = TypeAnnotation.set_nullable b ta1 in TypeState.add pvar (t, ta2, locs) typestate' else typestate' | None -> @@ -922,8 +891,7 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p match from_call with | EradicateChecks.From_is_true_on_null -> (* if f returns true on null, then false branch implies != null *) - if TypeAnnotation.get_value AnnotatedSignature.Nullable ta then - set_flag e' AnnotatedSignature.Nullable false typestate2 + if TypeAnnotation.is_nullable ta then set_nullable_flag e' false typestate2 else typestate2 | _ -> typestate2 ) @@ -937,16 +905,12 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p (* (e1 instanceof C) implies (e1 != null) *) (typestate, e1, EradicateChecks.From_instanceof) | None -> ( - match from_optional_isPresent e with + match from_is_false_on_null e with | Some e1 -> - (typestate, e1, EradicateChecks.From_optional_isPresent) - | None -> ( - match from_is_false_on_null e with - | Some e1 -> - (typestate, e1, EradicateChecks.From_is_false_on_null) - | None -> - if Option.is_some (from_containsKey e) then handle_containsKey e - else (typestate, e, EradicateChecks.From_condition) ) ) + (typestate, e1, EradicateChecks.From_is_false_on_null) + | None -> + if Option.is_some (from_containsKey e) then handle_containsKey e + else (typestate, e, EradicateChecks.From_condition) ) in let e', typestate2 = convert_complex_exp_to_pvar node' false e1 typestate1 loc in let typ, ta, _ = @@ -956,18 +920,13 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p EradicateChecks.check_nonzero tenv find_canonical_duplicate curr_pdesc node e' typ ta true_branch from_call idenv linereader loc instr_ref ; match from_call with - | EradicateChecks.From_optional_isPresent -> - if not (TypeAnnotation.get_value AnnotatedSignature.Present ta) then - set_flag e' AnnotatedSignature.Present true typestate2 - else typestate2 | EradicateChecks.From_is_true_on_null -> typestate2 | EradicateChecks.From_condition | EradicateChecks.From_containsKey | EradicateChecks.From_instanceof | EradicateChecks.From_is_false_on_null -> - if TypeAnnotation.get_value AnnotatedSignature.Nullable ta then - set_flag e' AnnotatedSignature.Nullable false typestate2 + if TypeAnnotation.is_nullable ta then set_nullable_flag e' false typestate2 else typestate2 ) | Exp.UnOp (Unop.LNot, Exp.BinOp (Binop.Eq, e1, e2), _) -> check_condition node' (Exp.BinOp (Binop.Ne, e1, e2)) diff --git a/infer/src/nullsafe/typeErr.ml b/infer/src/nullsafe/typeErr.ml index b0aa10869..1a2446e45 100644 --- a/infer/src/nullsafe/typeErr.ml +++ b/infer/src/nullsafe/typeErr.ml @@ -63,8 +63,7 @@ type origin_descr = string * Location.t option * AnnotatedSignature.t option let compare_origin_descr _ _ = 0 type parameter_not_nullable = - AnnotatedSignature.annotation - * string + string * (* description *) int * (* parameter number *) @@ -81,13 +80,12 @@ type err_instance = | 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_mutable of Typ.Fieldname.t * origin_descr - | Field_annotation_inconsistent of AnnotatedSignature.annotation * Typ.Fieldname.t * origin_descr + | Field_annotation_inconsistent of Typ.Fieldname.t * origin_descr | Field_over_annotated of Typ.Fieldname.t * Typ.Procname.t | Null_field_access of string option * Typ.Fieldname.t * origin_descr * bool - | Call_receiver_annotation_inconsistent of - AnnotatedSignature.annotation * string option * Typ.Procname.t * origin_descr + | Call_receiver_annotation_inconsistent of string option * Typ.Procname.t * origin_descr | Parameter_annotation_inconsistent of parameter_not_nullable - | Return_annotation_inconsistent of AnnotatedSignature.annotation * Typ.Procname.t * origin_descr + | Return_annotation_inconsistent of Typ.Procname.t * origin_descr | Return_over_annotated of Typ.Procname.t [@@deriving compare] @@ -106,18 +104,18 @@ module H = Hashtbl.Make (struct 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 (ann, fn, _) -> - Hashtbl.hash (4, ann, string_hash (Typ.Fieldname.to_string fn)) + | Field_annotation_inconsistent (fn, _) -> + Hashtbl.hash (4, string_hash (Typ.Fieldname.to_string fn)) | Field_over_annotated (fn, pn) -> Hashtbl.hash (5, string_hash (Typ.Fieldname.to_string fn ^ Typ.Procname.to_string pn)) | Null_field_access (so, fn, _, _) -> Hashtbl.hash (6, string_opt_hash so, string_hash (Typ.Fieldname.to_string fn)) - | Call_receiver_annotation_inconsistent (ann, so, pn, _) -> - Hashtbl.hash (7, ann, string_opt_hash so, Typ.Procname.hash pn) - | Parameter_annotation_inconsistent (ann, s, n, pn, _, _) -> - Hashtbl.hash (8, ann, string_hash s, n, Typ.Procname.hash pn) - | Return_annotation_inconsistent (ann, pn, _) -> - Hashtbl.hash (9, ann, Typ.Procname.hash pn) + | Call_receiver_annotation_inconsistent (so, pn, _) -> + Hashtbl.hash (7, string_opt_hash so, Typ.Procname.hash pn) + | Parameter_annotation_inconsistent (s, n, pn, _, _) -> + Hashtbl.hash (8, string_hash s, n, Typ.Procname.hash pn) + | Return_annotation_inconsistent (pn, _) -> + Hashtbl.hash (9, Typ.Procname.hash pn) | Return_over_annotated pn -> Hashtbl.hash (10, Typ.Procname.hash pn) | Inconsistent_subclass_return_annotation (pn, opn) -> @@ -235,7 +233,7 @@ module Severity = struct let err_instance_get_severity tenv err_instance : Exceptions.severity option = match err_instance with - | Call_receiver_annotation_inconsistent (AnnotatedSignature.Nullable, _, _, origin_descr) + | Call_receiver_annotation_inconsistent (_, _, origin_descr) | Null_field_access (_, _, origin_descr, _) -> origin_descr_get_severity tenv origin_descr | _ -> @@ -260,7 +258,6 @@ let report_error_now tenv (st_report_error : st_report_error) err_instance loc p let pname = Procdesc.get_proc_name pdesc in let nullable_annotation = "@Nullable" in let mutable_annotation = "@Mutable" in - let present_annotation = "@Present" in let kind, description, field_name = match err_instance with | Condition_redundant (b, s_opt, nonnull) -> @@ -294,21 +291,12 @@ let report_error_now tenv (st_report_error : st_report_error) err_instance loc p (Typ.Fieldname.to_simplified_string fn) MF.pp_monospaced mutable_annotation origin_description , None ) - | Field_annotation_inconsistent (ann, fn, (origin_description, _, _)) -> + | Field_annotation_inconsistent (fn, (origin_description, _, _)) -> let kind_s, description = - match ann with - | AnnotatedSignature.Nullable -> - ( IssueType.eradicate_field_not_nullable - , Format.asprintf "Field %a can be null but is not declared %a. %s" MF.pp_monospaced - (Typ.Fieldname.to_simplified_string fn) - MF.pp_monospaced nullable_annotation origin_description ) - | AnnotatedSignature.Present -> - ( IssueType.eradicate_field_value_absent - , Format.asprintf - "Field %a is assigned a possibly absent value but is declared %a. %s" - MF.pp_monospaced - (Typ.Fieldname.to_simplified_string fn) - MF.pp_monospaced present_annotation origin_description ) + ( IssueType.eradicate_field_not_nullable + , Format.asprintf "Field %a can be null but is not declared %a. %s" MF.pp_monospaced + (Typ.Fieldname.to_simplified_string fn) + MF.pp_monospaced nullable_annotation origin_description ) in (kind_s, description, None) | Field_over_annotated (fn, pn) -> @@ -336,60 +324,34 @@ let report_error_now tenv (st_report_error : st_report_error) err_instance loc p (Typ.Fieldname.to_simplified_string fn) origin_description , None ) - | Call_receiver_annotation_inconsistent (ann, s_opt, pn, (origin_description, _, _)) -> + | Call_receiver_annotation_inconsistent (s_opt, pn, (origin_description, _, _)) -> let kind_s, description = - match ann with - | AnnotatedSignature.Nullable -> - ( IssueType.eradicate_nullable_dereference - , Format.asprintf - "The value of %a in the call to %a is nullable and is not locally checked for \ - null. %s" - MF.pp_monospaced (Option.value s_opt ~default:"") MF.pp_monospaced - (Typ.Procname.to_simplified_string pn) - origin_description ) - | AnnotatedSignature.Present -> - ( IssueType.eradicate_value_not_present - , Format.asprintf "The value of %a in the call to %a is not %a. %s" MF.pp_monospaced - (Option.value s_opt ~default:"") MF.pp_monospaced - (Typ.Procname.to_simplified_string pn) - MF.pp_monospaced present_annotation origin_description ) + ( IssueType.eradicate_nullable_dereference + , Format.asprintf + "The value of %a in the call to %a is nullable and is not locally checked for null. \ + %s" + MF.pp_monospaced (Option.value s_opt ~default:"") MF.pp_monospaced + (Typ.Procname.to_simplified_string pn) + origin_description ) in (kind_s, description, None) - | Parameter_annotation_inconsistent (ann, s, n, pn, _, (origin_desc, _, _)) -> + | Parameter_annotation_inconsistent (s, n, pn, _, (origin_desc, _, _)) -> let kind_s, description = - match ann with - | AnnotatedSignature.Nullable -> - ( IssueType.eradicate_parameter_not_nullable - , Format.asprintf - "%a needs a non-null value in parameter %d but argument %a can be null. %s" - MF.pp_monospaced - (Typ.Procname.to_simplified_string ~withclass:true pn) - n MF.pp_monospaced s origin_desc ) - | AnnotatedSignature.Present -> - ( IssueType.eradicate_parameter_value_absent - , Format.asprintf - "%a needs a present value in parameter %d but argument %a can be absent. %s" - MF.pp_monospaced - (Typ.Procname.to_simplified_string ~withclass:true pn) - n MF.pp_monospaced s origin_desc ) + ( IssueType.eradicate_parameter_not_nullable + , Format.asprintf + "%a needs a non-null value in parameter %d but argument %a can be null. %s" + MF.pp_monospaced + (Typ.Procname.to_simplified_string ~withclass:true pn) + n MF.pp_monospaced s origin_desc ) in (kind_s, description, None) - | Return_annotation_inconsistent (ann, pn, (origin_description, _, _)) -> + | Return_annotation_inconsistent (pn, (origin_description, _, _)) -> let kind_s, description = - match ann with - | AnnotatedSignature.Nullable -> - ( IssueType.eradicate_return_not_nullable - , Format.asprintf "Method %a may return null but it is not annotated with %a. %s" - MF.pp_monospaced - (Typ.Procname.to_simplified_string pn) - MF.pp_monospaced nullable_annotation origin_description ) - | AnnotatedSignature.Present -> - ( IssueType.eradicate_return_value_not_present - , Format.asprintf - "Method %a may return an absent value but it is annotated with %a. %s" - MF.pp_monospaced - (Typ.Procname.to_simplified_string pn) - MF.pp_monospaced present_annotation origin_description ) + ( IssueType.eradicate_return_not_nullable + , Format.asprintf "Method %a may return null but it is not annotated with %a. %s" + MF.pp_monospaced + (Typ.Procname.to_simplified_string pn) + MF.pp_monospaced nullable_annotation origin_description ) in (kind_s, description, None) | Return_over_annotated pn -> diff --git a/infer/src/nullsafe/typeErr.mli b/infer/src/nullsafe/typeErr.mli index 2aae36a28..782c68da1 100644 --- a/infer/src/nullsafe/typeErr.mli +++ b/infer/src/nullsafe/typeErr.mli @@ -35,8 +35,7 @@ type origin_descr = string * Location.t option * AnnotatedSignature.t option (* callee signature *) type parameter_not_nullable = - AnnotatedSignature.annotation - * string + string * (* description *) int * (* parameter number *) @@ -52,13 +51,12 @@ type err_instance = | 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_mutable of Typ.Fieldname.t * origin_descr - | Field_annotation_inconsistent of AnnotatedSignature.annotation * Typ.Fieldname.t * origin_descr + | Field_annotation_inconsistent of Typ.Fieldname.t * origin_descr | Field_over_annotated of Typ.Fieldname.t * Typ.Procname.t | Null_field_access of string option * Typ.Fieldname.t * origin_descr * bool - | Call_receiver_annotation_inconsistent of - AnnotatedSignature.annotation * string option * Typ.Procname.t * origin_descr + | Call_receiver_annotation_inconsistent of string option * Typ.Procname.t * origin_descr | Parameter_annotation_inconsistent of parameter_not_nullable - | Return_annotation_inconsistent of AnnotatedSignature.annotation * Typ.Procname.t * origin_descr + | Return_annotation_inconsistent of Typ.Procname.t * origin_descr | Return_over_annotated of Typ.Procname.t val node_reset_forall : Procdesc.Node.t -> unit diff --git a/infer/tests/codetoanalyze/java/eradicate/Makefile b/infer/tests/codetoanalyze/java/eradicate/Makefile index dc96fe528..aaba3f768 100644 --- a/infer/tests/codetoanalyze/java/eradicate/Makefile +++ b/infer/tests/codetoanalyze/java/eradicate/Makefile @@ -8,7 +8,6 @@ TESTS_DIR = ../../.. INFER_OPTIONS = \ --eradicate-only \ --eradicate-return-over-annotated \ - --eradicate-optional-present \ --eradicate-condition-redundant \ --debug-exceptions INFERPRINT_OPTIONS = --issues-tests diff --git a/infer/tests/codetoanalyze/java/eradicate/PresentTest.java b/infer/tests/codetoanalyze/java/eradicate/PresentTest.java deleted file mode 100644 index ab365cc8f..000000000 --- a/infer/tests/codetoanalyze/java/eradicate/PresentTest.java +++ /dev/null @@ -1,92 +0,0 @@ -/* - * 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. - */ - -package codetoanalyze.java.eradicate; - -import com.facebook.infer.annotation.Assertions; -import com.facebook.infer.annotation.Present; -import com.google.common.base.Optional; - -public class PresentTest { - - void argPresent(@Present Optional present, Optional absent) {} - - void testPresent(@Present Optional present, Optional absent) { - argPresent(present, absent); // OK - argPresent(present, present); // OK - argPresent(present, absent); // OK - argPresent(absent, absent); // Bad - } - - class TestPresentAnnotationBasic { - void testBasicConditional(Optional o) { - if (o.isPresent()) { - o.get(); - } - } - - Optional absent = Optional.absent(); - @Present Optional present = Optional.of("abc"); - - @Present - Optional returnPresent() { - if (absent.isPresent()) { - return absent; - } else return Optional.of("abc"); - } - - @Present - Optional returnPresentBad() { - absent.get(); // Bad: get is unsafe - return absent; // Bad: should return present - } - - void expectPresent(@Present Optional x) {} - - void bar() { - expectPresent(present); - String s; - s = returnPresent().get(); - s = present.get(); - - Assertions.assertCondition(absent.isPresent()); - expectPresent(absent); - } - - void testOptionalAbsent() { - expectPresent(Optional.absent()); // Bad - } - } - - class TestPresentFieldOfInnerClass { - class D { - @SuppressFieldNotInitialized Optional s; - } - - class D1 { - // Different bytecode generated when the field is private - @SuppressFieldNotInitialized private Optional s; - } - - void testD(D d) { - if (d.s.isPresent()) { - d.s.get(); - } - } - - void testD1(D1 d1) { - if (d1.s.isPresent()) { - d1.s.get(); - } - } - - void testD1Condition(D1 d1) { - Assertions.assertCondition(d1.s.isPresent()); - d1.s.get(); - } - } -} diff --git a/infer/tests/codetoanalyze/java/eradicate/issues.exp b/infer/tests/codetoanalyze/java/eradicate/issues.exp index 240894051..133e76f61 100644 --- a/infer/tests/codetoanalyze/java/eradicate/issues.exp +++ b/infer/tests/codetoanalyze/java/eradicate/issues.exp @@ -108,10 +108,6 @@ codetoanalyze/java/eradicate/ParameterNotNullable.java, codetoanalyze.java.eradi codetoanalyze/java/eradicate/ParameterNotNullable.java, codetoanalyze.java.eradicate.ParameterNotNullable.testThreeParameters():void, 2, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [`ParameterNotNullable.threeParameters(...)` needs a non-null value in parameter 1 but argument `null` can be null. (Origin: null constant at line 88)] codetoanalyze/java/eradicate/ParameterNotNullable.java, codetoanalyze.java.eradicate.ParameterNotNullable.testThreeParameters():void, 3, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [`ParameterNotNullable.threeParameters(...)` needs a non-null value in parameter 2 but argument `null` can be null. (Origin: null constant at line 89)] codetoanalyze/java/eradicate/ParameterNotNullable.java, codetoanalyze.java.eradicate.ParameterNotNullable.testThreeParameters():void, 4, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [`ParameterNotNullable.threeParameters(...)` needs a non-null value in parameter 3 but argument `null` can be null. (Origin: null constant at line 90)] -codetoanalyze/java/eradicate/PresentTest.java, codetoanalyze.java.eradicate.PresentTest$TestPresentAnnotationBasic.returnPresentBad():com.google.common.base.Optional, 0, ERADICATE_RETURN_VALUE_NOT_PRESENT, no_bucket, WARNING, [Method `returnPresentBad()` may return an absent value but it is annotated with `@Present`. (Origin: field PresentTest$TestPresentAnnotationBasic.absent at line 44)] -codetoanalyze/java/eradicate/PresentTest.java, codetoanalyze.java.eradicate.PresentTest$TestPresentAnnotationBasic.returnPresentBad():com.google.common.base.Optional, 1, ERADICATE_VALUE_NOT_PRESENT, no_bucket, WARNING, [The value of `PresentTest$TestPresentAnnotationBasic.absent` in the call to `get()` is not `@Present`. (Origin: field PresentTest$TestPresentAnnotationBasic.absent at line 44)] -codetoanalyze/java/eradicate/PresentTest.java, codetoanalyze.java.eradicate.PresentTest$TestPresentAnnotationBasic.testOptionalAbsent():void, 1, ERADICATE_PARAMETER_VALUE_ABSENT, no_bucket, WARNING, [`PresentTest$TestPresentAnnotationBasic.expectPresent(...)` needs a present value in parameter 1 but argument `absent()` can be absent. (Origin: call to absent() at line 61)] -codetoanalyze/java/eradicate/PresentTest.java, codetoanalyze.java.eradicate.PresentTest.testPresent(com.google.common.base.Optional,com.google.common.base.Optional):void, 4, ERADICATE_PARAMETER_VALUE_ABSENT, no_bucket, WARNING, [`PresentTest.argPresent(...)` needs a present value in parameter 1 but argument `absent` can be absent. (Origin: method parameter absent)] codetoanalyze/java/eradicate/Redundant.java, Redundant.bad():void, 2, ERADICATE_CONDITION_REDUNDANT, no_bucket, WARNING, [The condition s is always true according to the existing annotations.] codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable$ConditionalAssignment.test(boolean):java.lang.Object, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [Method `test(...)` may return null but it is not annotated with `@Nullable`. (Origin: field ReturnNotNullable$ConditionalAssignment.f1 at line 213)] codetoanalyze/java/eradicate/ReturnNotNullable.java, codetoanalyze.java.eradicate.ReturnNotNullable.constantToNullableIsOverannotated():java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, WARNING, [Method `constantToNullableIsOverannotated()` is annotated with `@Nullable` but never returns null.]