diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml index b8e0e6536..a7c318e62 100644 --- a/infer/src/nullsafe/eradicateChecks.ml +++ b/infer/src/nullsafe/eradicateChecks.ml @@ -156,9 +156,9 @@ let check_field_assignment tenv find_canonical_duplicate curr_pdesc node instr_r && not (field_is_in_cleanup_context ()) in if should_report_nullable then - let origin_descr = InferredNullability.descr_origin inferred_nullability_rhs in + let rhs_origin_descr = InferredNullability.descr_origin inferred_nullability_rhs in report_error tenv find_canonical_duplicate - (TypeErr.Field_annotation_inconsistent (fname, origin_descr)) + (TypeErr.Bad_assignment {rhs_origin_descr; assignment_type= TypeErr.AssigningToAField fname}) (Some instr_ref) loc curr_pdesc @@ -298,7 +298,7 @@ let check_constructor_initialization tenv find_canonical_duplicate curr_construc ~rhs_upper_bound:(field_nullability_upper_bound_over_all_typestates ()) then report_error tenv find_canonical_duplicate - (TypeErr.Field_over_annotated (field_name, curr_constructor_pname)) + (TypeErr.Over_annotation (TypeErr.FieldOverAnnotedAsNullable field_name)) None loc curr_constructor_pdesc ) in List.iter ~f:do_field fields @@ -314,9 +314,10 @@ let check_return_not_nullable tenv find_canonical_duplicate loc curr_pname curr_ 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 + let rhs_origin_descr = InferredNullability.descr_origin ret_inferred_nullability in report_error tenv find_canonical_duplicate - (TypeErr.Return_annotation_inconsistent - (curr_pname, InferredNullability.descr_origin ret_inferred_nullability)) + (TypeErr.Bad_assignment + {rhs_origin_descr; assignment_type= TypeErr.ReturningFromAFunction curr_pname}) None loc curr_pdesc @@ -330,7 +331,8 @@ let check_return_overrannotated tenv find_canonical_duplicate loc curr_pname cur *) let rhs_upper_bound = InferredNullability.get_nullability ret_inferred_nullability in if NullsafeRules.is_overannotated ~lhs ~rhs_upper_bound then - report_error tenv find_canonical_duplicate (TypeErr.Return_over_annotated curr_pname) None loc + report_error tenv find_canonical_duplicate + (TypeErr.Over_annotation (TypeErr.ReturnOverAnnotatedAsNullable curr_pname)) None loc curr_pdesc @@ -386,20 +388,22 @@ 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; actual= orig_e2, nullability_actual} = + let check {num= param_position; formal; actual= orig_e2, nullability_actual} = let report () = - let description = + let param_description = match explain_expr tenv node orig_e2 with | Some descr -> descr | None -> "formal parameter " ^ Mangled.to_string formal.mangled in - let origin_descr = InferredNullability.descr_origin nullability_actual in - let callee_loc = callee_attributes.ProcAttributes.loc in + let rhs_origin_descr = InferredNullability.descr_origin nullability_actual in report_error tenv find_canonical_duplicate - (TypeErr.Parameter_annotation_inconsistent - (description, param_num, callee_pname, callee_loc, origin_descr)) + (TypeErr.Bad_assignment + { rhs_origin_descr + ; assignment_type= + TypeErr.PassingParamToAFunction + {param_description; param_position; function_procname= callee_pname} }) (Some instr_ref) loc curr_pdesc in if PatternMatch.type_is_class formal.param_annotated_type.typ then @@ -432,7 +436,10 @@ let check_inheritance_rule_for_return find_canonical_duplicate tenv loc ~base_pr ~overridden:overridden_nullability) then report_error tenv find_canonical_duplicate - (TypeErr.Inconsistent_subclass_return_annotation (overridden_proc_name, base_proc_name)) + (TypeErr.Inconsistent_subclass + { overridden_proc_name + ; base_proc_name + ; inconsistent_subclass_type= TypeErr.InconsistentReturn }) None loc overridden_proc_desc @@ -445,11 +452,12 @@ let check_inheritance_rule_for_param find_canonical_duplicate tenv loc ~overridd ~overridden:overridden_nullability) then report_error tenv find_canonical_duplicate - (TypeErr.Inconsistent_subclass_parameter_annotation - ( Mangled.to_string overridden_param_name - , param_position - , overridden_proc_name - , base_proc_name )) + (TypeErr.Inconsistent_subclass + { base_proc_name + ; overridden_proc_name + ; inconsistent_subclass_type= + TypeErr.InconsistentParam + {param_position; param_description= Mangled.to_string overridden_param_name} }) None loc overridden_proc_desc diff --git a/infer/src/nullsafe/typeErr.ml b/infer/src/nullsafe/typeErr.ml index 0dccefd0e..841c6fb7c 100644 --- a/infer/src/nullsafe/typeErr.ml +++ b/infer/src/nullsafe/typeErr.ml @@ -62,34 +62,39 @@ type origin_descr = string * Location.t option * AnnotatedSignature.t option (* ignore origin descr *) let compare_origin_descr _ _ = 0 -type parameter_not_nullable = - string - * (* description *) - int - * (* parameter number *) - Typ.Procname.t - * Location.t - * (* callee location *) - origin_descr -[@@deriving compare] - (** Instance of an error *) type err_instance = | Condition_redundant of (bool * string option) - | 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 of + { base_proc_name: Typ.Procname.t + ; overridden_proc_name: Typ.Procname.t + ; inconsistent_subclass_type: inconsistent_subclass_type } | Field_not_initialized of Typ.Fieldname.t * Typ.Procname.t - | Field_annotation_inconsistent of Typ.Fieldname.t * origin_descr - | Field_over_annotated of Typ.Fieldname.t * Typ.Procname.t + | Over_annotation of over_annotation_type | Nullable_dereference of { nullable_object_descr: string option ; dereference_type: dereference_type ; origin_descr: origin_descr } - | Parameter_annotation_inconsistent of parameter_not_nullable - | Return_annotation_inconsistent of Typ.Procname.t * origin_descr - | Return_over_annotated of Typ.Procname.t + | Bad_assignment of {rhs_origin_descr: origin_descr; assignment_type: assignment_type} [@@deriving compare] +and inconsistent_subclass_type = + | InconsistentParam of {param_description: string; param_position: int} + | InconsistentReturn + +and over_annotation_type = + | FieldOverAnnotedAsNullable of Typ.Fieldname.t + | ReturnOverAnnotatedAsNullable of Typ.Procname.t + (** Return value of a method can be made non-nullable *) + +and assignment_type = + | PassingParamToAFunction of + { param_description: string + ; param_position: int + ; function_procname: Typ.Procname.t } + | AssigningToAField of Typ.Fieldname.t + | ReturningFromAFunction of Typ.Procname.t + and dereference_type = | MethodCall of Typ.Procname.t | AccessToField of Typ.Fieldname.t @@ -122,22 +127,14 @@ let get_forall = function true | Field_not_initialized _ -> false - | Field_annotation_inconsistent _ -> - false - | Field_over_annotated _ -> + | Over_annotation _ -> false - | Inconsistent_subclass_return_annotation _ -> + | Bad_assignment _ -> false - | Inconsistent_subclass_parameter_annotation _ -> + | Inconsistent_subclass _ -> false | Nullable_dereference _ -> false - | Parameter_annotation_inconsistent _ -> - false - | Return_annotation_inconsistent _ -> - false - | Return_over_annotated _ -> - false (** Reset the always field of the forall erros in the node, so if they are not set again @@ -221,153 +218,163 @@ type st_report_error = -> string -> unit -(** Report an error right now. *) -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 get_infer_issue_type = function + | Condition_redundant _ -> + IssueType.eradicate_condition_redundant + | Over_annotation (FieldOverAnnotedAsNullable _) -> + IssueType.eradicate_field_over_annotated + | Over_annotation (ReturnOverAnnotatedAsNullable _) -> + IssueType.eradicate_return_over_annotated + | Field_not_initialized _ -> + IssueType.eradicate_field_not_initialized + | Bad_assignment {assignment_type= PassingParamToAFunction _} -> + IssueType.eradicate_parameter_not_nullable + | Bad_assignment {assignment_type= AssigningToAField _} -> + IssueType.eradicate_field_not_nullable + | Bad_assignment {assignment_type= ReturningFromAFunction _} -> + IssueType.eradicate_return_not_nullable + | Nullable_dereference _ -> + IssueType.eradicate_nullable_dereference + | Inconsistent_subclass {inconsistent_subclass_type= InconsistentReturn} -> + IssueType.eradicate_inconsistent_subclass_return_annotation + | Inconsistent_subclass {inconsistent_subclass_type= InconsistentParam _} -> + IssueType.eradicate_inconsistent_subclass_parameter_annotation + + +(* If an error is related to a particular field, we support suppressing the + error via a supress annotation placed near the field declaration *) +let get_field_name_for_error_suppressing = function + | Over_annotation (FieldOverAnnotedAsNullable field_name) -> + Some field_name + | Field_not_initialized (field_name, _) -> + Some field_name + | Condition_redundant _ + | Over_annotation (ReturnOverAnnotatedAsNullable _) + (* In case of assignment to a field, it should be fixed case by case for each assignment *) + | Bad_assignment _ + | Nullable_dereference _ + | Inconsistent_subclass _ -> + None + + +let get_error_description err_instance = let nullable_annotation = "@Nullable" in - let kind, description, field_name = - match err_instance with - | Condition_redundant (is_always_true, s_opt) -> - ( IssueType.eradicate_condition_redundant - , P.sprintf "The condition %s is always %b according to the existing annotations." - (Option.value s_opt ~default:"") is_always_true - , None ) - | Field_not_initialized (fn, pn) -> - let constructor_name = - if Typ.Procname.is_constructor pn then "the constructor" - else - match pn with - | Typ.Procname.Java pn_java -> - MF.monospaced_to_string (Typ.Procname.Java.get_method pn_java) - | _ -> - MF.monospaced_to_string (Typ.Procname.to_simplified_string pn) - in - ( IssueType.eradicate_field_not_initialized - , Format.asprintf "Field %a is not initialized in %s and is not declared %a" - MF.pp_monospaced - (Typ.Fieldname.to_simplified_string fn) - constructor_name MF.pp_monospaced nullable_annotation - , Some fn ) - | Field_annotation_inconsistent (fn, (origin_description, _, _)) -> - let kind_s, 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) -> - let constructor_name = - if Typ.Procname.is_constructor pn then "the constructor" - else - match pn with - | Typ.Procname.Java pn_java -> - Typ.Procname.Java.get_method pn_java - | _ -> - Typ.Procname.to_simplified_string pn - in - ( IssueType.eradicate_field_over_annotated - , Format.asprintf "Field %a is always initialized in %s but is declared %a" - MF.pp_monospaced - (Typ.Fieldname.to_simplified_string fn) - constructor_name MF.pp_monospaced nullable_annotation - , Some fn ) - | Nullable_dereference {nullable_object_descr; dereference_type; origin_descr= origin_str, _, _} - -> - let nullable_object_descr = - match dereference_type with - | MethodCall _ | AccessToField _ -> ( - match nullable_object_descr with - | None -> - "Object" - (* Just describe an object itself *) - | Some descr -> - MF.monospaced_to_string descr ) - | ArrayLengthAccess | AccessByIndex _ -> ( - (* In Java, those operations can be applied only to arrays *) - match nullable_object_descr with - | None -> - "Array" - | Some descr -> - Format.sprintf "Array %s" (MF.monospaced_to_string descr) ) - in - let action_descr = - match dereference_type with - | MethodCall method_name -> - Format.sprintf "calling %s" - (MF.monospaced_to_string (Typ.Procname.to_simplified_string method_name)) - | AccessToField field_name -> - Format.sprintf "accessing field %s" - (MF.monospaced_to_string (Typ.Fieldname.to_simplified_string field_name)) - | AccessByIndex {index_desc} -> - Format.sprintf "accessing at index %s" (MF.monospaced_to_string index_desc) - | ArrayLengthAccess -> - "accessing its length" - in - let description = - Format.sprintf "%s is nullable and is not locally checked for null when %s. %s" - nullable_object_descr action_descr origin_str - in - (IssueType.eradicate_nullable_dereference, description, None) - | Parameter_annotation_inconsistent (s, n, pn, _, (origin_desc, _, _)) -> - let kind_s, description = - ( 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 (pn, (origin_description, _, _)) -> - let kind_s, 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 -> - ( IssueType.eradicate_return_over_annotated - , Format.asprintf "Method %a is annotated with %a but never returns null." MF.pp_monospaced - (Typ.Procname.to_simplified_string pn) - MF.pp_monospaced nullable_annotation - , None ) - | Inconsistent_subclass_return_annotation (pn, opn) -> - ( IssueType.eradicate_inconsistent_subclass_return_annotation - , Format.asprintf "Method %a is annotated with %a but overrides unannotated method %a." - MF.pp_monospaced - (Typ.Procname.to_simplified_string ~withclass:true pn) - MF.pp_monospaced nullable_annotation MF.pp_monospaced - (Typ.Procname.to_simplified_string ~withclass:true opn) - , None ) - | Inconsistent_subclass_parameter_annotation (param_name, pos, pn, opn) -> - let translate_position = function - | 1 -> - "First" - | 2 -> - "Second" - | 3 -> - "Third" - | n -> - string_of_int n ^ "th" - in - ( IssueType.eradicate_inconsistent_subclass_parameter_annotation - , Format.asprintf + match err_instance with + | Condition_redundant (is_always_true, s_opt) -> + P.sprintf "The condition %s is always %b according to the existing annotations." + (Option.value s_opt ~default:"") is_always_true + | Over_annotation (FieldOverAnnotedAsNullable field_name) -> + Format.asprintf "Field %a is always initialized in the constructor but is declared %a" + MF.pp_monospaced + (Typ.Fieldname.to_simplified_string field_name) + MF.pp_monospaced nullable_annotation + | Over_annotation (ReturnOverAnnotatedAsNullable proc_name) -> + Format.asprintf "Method %a is annotated with %a but never returns null." MF.pp_monospaced + (Typ.Procname.to_simplified_string proc_name) + MF.pp_monospaced nullable_annotation + | Field_not_initialized (field_name, proc_name) -> + let constructor_name = + if Typ.Procname.is_constructor proc_name then "the constructor" + else + match proc_name with + | Typ.Procname.Java pn_java -> + MF.monospaced_to_string (Typ.Procname.Java.get_method pn_java) + | _ -> + MF.monospaced_to_string (Typ.Procname.to_simplified_string proc_name) + in + Format.asprintf "Field %a is not initialized in %s and is not declared %a" MF.pp_monospaced + (Typ.Fieldname.to_simplified_string field_name) + constructor_name MF.pp_monospaced nullable_annotation + | Bad_assignment {rhs_origin_descr= origin_descr_string, _, _; assignment_type} -> ( + match assignment_type with + | PassingParamToAFunction {param_description; param_position; function_procname} -> + 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 function_procname) + param_position MF.pp_monospaced param_description origin_descr_string + | AssigningToAField field_name -> + Format.asprintf "Field %a can be null but is not declared %a. %s" MF.pp_monospaced + (Typ.Fieldname.to_simplified_string field_name) + MF.pp_monospaced nullable_annotation origin_descr_string + | ReturningFromAFunction function_proc_name -> + Format.asprintf "Method %a may return null but it is not annotated with %a. %s" + MF.pp_monospaced + (Typ.Procname.to_simplified_string function_proc_name) + MF.pp_monospaced nullable_annotation origin_descr_string ) + | Nullable_dereference {nullable_object_descr; dereference_type; origin_descr= origin_str, _, _} + -> + let nullable_object_descr = + match dereference_type with + | MethodCall _ | AccessToField _ -> ( + match nullable_object_descr with + | None -> + "Object" + (* Just describe an object itself *) + | Some descr -> + MF.monospaced_to_string descr ) + | ArrayLengthAccess | AccessByIndex _ -> ( + (* In Java, those operations can be applied only to arrays *) + match nullable_object_descr with + | None -> + "Array" + | Some descr -> + Format.sprintf "Array %s" (MF.monospaced_to_string descr) ) + in + let action_descr = + match dereference_type with + | MethodCall method_name -> + Format.sprintf "calling %s" + (MF.monospaced_to_string (Typ.Procname.to_simplified_string method_name)) + | AccessToField field_name -> + Format.sprintf "accessing field %s" + (MF.monospaced_to_string (Typ.Fieldname.to_simplified_string field_name)) + | AccessByIndex {index_desc} -> + Format.sprintf "accessing at index %s" (MF.monospaced_to_string index_desc) + | ArrayLengthAccess -> + "accessing its length" + in + Format.sprintf "%s is nullable and is not locally checked for null when %s. %s" + nullable_object_descr action_descr origin_str + | Inconsistent_subclass {base_proc_name; overridden_proc_name; inconsistent_subclass_type} -> ( + let base_method_descr = Typ.Procname.to_simplified_string ~withclass:true base_proc_name in + let overridden_method_descr = + Typ.Procname.to_simplified_string ~withclass:true overridden_proc_name + in + match inconsistent_subclass_type with + | InconsistentReturn -> + Format.asprintf "Method %a is annotated with %a but overrides unannotated method %a." + MF.pp_monospaced overridden_method_descr MF.pp_monospaced nullable_annotation + MF.pp_monospaced base_method_descr + | InconsistentParam {param_description; param_position} -> + let translate_position = function + | 1 -> + "First" + | 2 -> + "Second" + | 3 -> + "Third" + | n -> + string_of_int n ^ "th" + in + Format.asprintf "%s parameter %a of method %a is not %a but is declared %ain the parent class method \ %a." - (translate_position pos) MF.pp_monospaced param_name MF.pp_monospaced - (Typ.Procname.to_simplified_string ~withclass:true pn) + (translate_position param_position) + MF.pp_monospaced param_description MF.pp_monospaced overridden_method_descr MF.pp_monospaced nullable_annotation MF.pp_monospaced nullable_annotation - MF.pp_monospaced - (Typ.Procname.to_simplified_string ~withclass:true opn) - , None ) - in + MF.pp_monospaced base_method_descr ) + + +(** Report an error right now. *) +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 infer_issue_type = get_infer_issue_type err_instance in + let field_name = get_field_name_for_error_suppressing err_instance in + let err_description = get_error_description err_instance in let severity = Severity.err_instance_get_severity tenv err_instance in - st_report_error pname pdesc kind loc ~field_name + st_report_error pname pdesc infer_issue_type loc ~field_name ~exception_kind:(fun k d -> Exceptions.Eradicate (k, d)) - ?severity description + ?severity err_description (** Report an error unless is has been reported already, or unless it's a forall error diff --git a/infer/src/nullsafe/typeErr.mli b/infer/src/nullsafe/typeErr.mli index f1a190a1e..eb3a4a93f 100644 --- a/infer/src/nullsafe/typeErr.mli +++ b/infer/src/nullsafe/typeErr.mli @@ -34,33 +34,39 @@ type origin_descr = string * Location.t option * AnnotatedSignature.t option (* callee signature *) -type parameter_not_nullable = - string - * (* description *) - int - * (* parameter number *) - Typ.Procname.t - * Location.t - * (* callee location *) - origin_descr - (** Instance of an error *) type err_instance = | Condition_redundant of (bool * string option) - | 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 of + { base_proc_name: Typ.Procname.t + ; overridden_proc_name: Typ.Procname.t + ; inconsistent_subclass_type: inconsistent_subclass_type } | Field_not_initialized of Typ.Fieldname.t * Typ.Procname.t - | Field_annotation_inconsistent of Typ.Fieldname.t * origin_descr - | Field_over_annotated of Typ.Fieldname.t * Typ.Procname.t + | Over_annotation of over_annotation_type | Nullable_dereference of { nullable_object_descr: string option ; dereference_type: dereference_type ; origin_descr: origin_descr } - | Parameter_annotation_inconsistent of parameter_not_nullable - | Return_annotation_inconsistent of Typ.Procname.t * origin_descr - | Return_over_annotated of Typ.Procname.t + | Bad_assignment of {rhs_origin_descr: origin_descr; assignment_type: assignment_type} [@@deriving compare] +and inconsistent_subclass_type = + | InconsistentParam of {param_description: string; param_position: int} + | InconsistentReturn + +and over_annotation_type = + | FieldOverAnnotedAsNullable of Typ.Fieldname.t + | ReturnOverAnnotatedAsNullable of Typ.Procname.t + (** Return value of a method can be made non-nullable *) + +and assignment_type = + | PassingParamToAFunction of + { param_description: string + ; param_position: int + ; function_procname: Typ.Procname.t } + | AssigningToAField of Typ.Fieldname.t + | ReturningFromAFunction of Typ.Procname.t + and dereference_type = | MethodCall of Typ.Procname.t (** nullable_object.some_method() *) | AccessToField of Typ.Fieldname.t (** nullable_object.some_field *)