From d5b574dd80989999038a8fca7215a3d8a54b6921 Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Tue, 19 Nov 2019 04:00:32 -0800 Subject: [PATCH] [nullsafe] Render strict mode violations nicely Summary: Now we point to the root cause of the problem, and also provide actionable way to solve the issue Reviewed By: artempyanykh Differential Revision: D18575650 fbshipit-source-id: ba4884fe1 --- infer/man/man1/infer-full.txt | 2 + infer/man/man1/infer-report.txt | 2 + infer/man/man1/infer.txt | 2 + infer/src/base/IssueType.ml | 10 ++ infer/src/base/IssueType.mli | 4 + infer/src/nullsafe/AssignmentRule.ml | 70 ++++++++---- infer/src/nullsafe/AssignmentRule.mli | 8 +- infer/src/nullsafe/DereferenceRule.ml | 87 ++++++++------- infer/src/nullsafe/DereferenceRule.mli | 4 +- infer/src/nullsafe/ErrorRenderingUtils.ml | 103 ++++++++++++++++++ infer/src/nullsafe/ErrorRenderingUtils.mli | 9 ++ infer/src/nullsafe/eradicateChecks.ml | 9 +- infer/src/nullsafe/typeErr.ml | 90 ++++++++------- infer/src/nullsafe/typeErr.mli | 2 + .../java/nullsafe-default/issues.exp | 10 +- 15 files changed, 302 insertions(+), 110 deletions(-) diff --git a/infer/man/man1/infer-full.txt b/infer/man/man1/infer-full.txt index 300ad95b5..ff0c09d12 100644 --- a/infer/man/man1/infer-full.txt +++ b/infer/man/man1/infer-full.txt @@ -400,6 +400,8 @@ OPTIONS ERADICATE_PARAMETER_NOT_NULLABLE (enabled by default), ERADICATE_RETURN_NOT_NULLABLE (enabled by default), ERADICATE_RETURN_OVER_ANNOTATED (enabled by default), + ERADICATE_UNCHECKED_NONSTRICT_FROM_STRICT (enabled by default), + ERADICATE_UNVETTED_THIRD_PARTY_IN_STRICT (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-report.txt b/infer/man/man1/infer-report.txt index cd353c489..94942c967 100644 --- a/infer/man/man1/infer-report.txt +++ b/infer/man/man1/infer-report.txt @@ -142,6 +142,8 @@ OPTIONS ERADICATE_PARAMETER_NOT_NULLABLE (enabled by default), ERADICATE_RETURN_NOT_NULLABLE (enabled by default), ERADICATE_RETURN_OVER_ANNOTATED (enabled by default), + ERADICATE_UNCHECKED_NONSTRICT_FROM_STRICT (enabled by default), + ERADICATE_UNVETTED_THIRD_PARTY_IN_STRICT (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 8f5e0dbd7..dab6370f9 100644 --- a/infer/man/man1/infer.txt +++ b/infer/man/man1/infer.txt @@ -400,6 +400,8 @@ OPTIONS ERADICATE_PARAMETER_NOT_NULLABLE (enabled by default), ERADICATE_RETURN_NOT_NULLABLE (enabled by default), ERADICATE_RETURN_OVER_ANNOTATED (enabled by default), + ERADICATE_UNCHECKED_NONSTRICT_FROM_STRICT (enabled by default), + ERADICATE_UNVETTED_THIRD_PARTY_IN_STRICT (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/IssueType.ml b/infer/src/base/IssueType.ml index b443d925c..3ce3ec4c7 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -272,6 +272,16 @@ let eradicate_return_over_annotated = register_from_string "ERADICATE_RETURN_OVER_ANNOTATED" ~hum:"Return Over Annotated" +let eradicate_forbidden_non_strict_in_strict = + register_from_string "ERADICATE_UNCHECKED_NONSTRICT_FROM_STRICT" + ~hum:"Strict mode: unchecked usage of a value from non-strict code" + + +let eradicate_unvetted_third_party_in_strict = + register_from_string "ERADICATE_UNVETTED_THIRD_PARTY_IN_STRICT" + ~hum:"Strict mode: unchecked usage of unvetted third-party" + + let expensive_cost_call ~kind ~is_on_cold_start ~is_on_ui_thread = register_from_cost_string ~enabled:false ~kind ~is_on_cold_start ~is_on_ui_thread "EXPENSIVE_%s" diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index 5cc5bd7e7..cce13a494 100644 --- a/infer/src/base/IssueType.mli +++ b/infer/src/base/IssueType.mli @@ -164,6 +164,10 @@ val eradicate_return_not_nullable : t val eradicate_return_over_annotated : t +val eradicate_unvetted_third_party_in_strict : t + +val eradicate_forbidden_non_strict_in_strict : t + val expensive_cost_call : kind:CostKind.t -> is_on_cold_start:bool -> is_on_ui_thread:bool -> t val exposed_insecure_intent_handling : t diff --git a/infer/src/nullsafe/AssignmentRule.ml b/infer/src/nullsafe/AssignmentRule.ml index 60983a320..1435d3b04 100644 --- a/infer/src/nullsafe/AssignmentRule.ml +++ b/infer/src/nullsafe/AssignmentRule.ml @@ -117,25 +117,51 @@ let bad_param_description nullability_evidence_as_suffix -let violation_description _ assignment_type ~rhs_origin = - let nullability_evidence = - get_origin_opt assignment_type rhs_origin - |> Option.bind ~f:(fun origin -> TypeOrigin.get_description origin) - in - let nullability_evidence_as_suffix = - Option.value_map nullability_evidence ~f:(fun evidence -> ": " ^ evidence) ~default:"" - in - let module MF = MarkupFormatter in - match assignment_type with - | PassingParamToFunction function_info -> - bad_param_description function_info nullability_evidence - | AssigningToField field_name -> - Format.asprintf "%a is declared non-nullable but is assigned a nullable%s." MF.pp_monospaced - (Typ.Fieldname.to_flat_string field_name) - nullability_evidence_as_suffix - | ReturningFromFunction function_proc_name -> - Format.asprintf - "%a: return type is declared non-nullable but the method returns a nullable value%s." - MF.pp_monospaced - (Typ.Procname.to_simplified_string ~withclass:false function_proc_name) - nullability_evidence_as_suffix +let is_declared_nonnull_to_nonnull ~lhs ~rhs = + match (lhs, rhs) with Nullability.Nonnull, Nullability.DeclaredNonnull -> true | _ -> false + + +let get_issue_type = function + | PassingParamToFunction _ -> + IssueType.eradicate_parameter_not_nullable + | AssigningToField _ -> + IssueType.eradicate_field_not_nullable + | ReturningFromFunction _ -> + IssueType.eradicate_return_not_nullable + + +let violation_description {is_strict_mode; lhs; rhs} ~assignment_location assignment_type + ~rhs_origin = + if is_declared_nonnull_to_nonnull ~lhs ~rhs then ( + if not is_strict_mode then + Logging.die InternalError "Unexpected situation: should not be a violation not in strict mode" ; + (* This type of violation is more subtle than the normal case because, so it should be rendered in a special way *) + ErrorRenderingUtils.get_strict_mode_violation_issue ~bad_usage_location:assignment_location + rhs_origin ) + else + let nullability_evidence = + get_origin_opt assignment_type rhs_origin + |> Option.bind ~f:(fun origin -> TypeOrigin.get_description origin) + in + let nullability_evidence_as_suffix = + Option.value_map nullability_evidence ~f:(fun evidence -> ": " ^ evidence) ~default:"" + in + let module MF = MarkupFormatter in + let error_message = + match assignment_type with + | PassingParamToFunction function_info -> + bad_param_description function_info nullability_evidence + | AssigningToField field_name -> + Format.asprintf "%a is declared non-nullable but is assigned a nullable%s." + MF.pp_monospaced + (Typ.Fieldname.to_flat_string field_name) + nullability_evidence_as_suffix + | ReturningFromFunction function_proc_name -> + Format.asprintf + "%a: return type is declared non-nullable but the method returns a nullable value%s." + MF.pp_monospaced + (Typ.Procname.to_simplified_string ~withclass:false function_proc_name) + nullability_evidence_as_suffix + in + let issue_type = get_issue_type assignment_type in + (error_message, issue_type, assignment_location) diff --git a/infer/src/nullsafe/AssignmentRule.mli b/infer/src/nullsafe/AssignmentRule.mli index 425b7e8ca..68b55d1cf 100644 --- a/infer/src/nullsafe/AssignmentRule.mli +++ b/infer/src/nullsafe/AssignmentRule.mli @@ -29,4 +29,10 @@ and function_info = ; param_position: int ; function_procname: Typ.Procname.t } -val violation_description : violation -> assignment_type -> rhs_origin:TypeOrigin.t -> string +val violation_description : + violation + -> assignment_location:Location.t + -> assignment_type + -> rhs_origin:TypeOrigin.t + -> string * IssueType.t * Location.t +(** Given context around violation, return error message together with the info where to put this message *) diff --git a/infer/src/nullsafe/DereferenceRule.ml b/infer/src/nullsafe/DereferenceRule.ml index 28d90f043..1fad4fe6c 100644 --- a/infer/src/nullsafe/DereferenceRule.ml +++ b/infer/src/nullsafe/DereferenceRule.ml @@ -36,42 +36,53 @@ let get_origin_opt ~nullable_object_descr origin = if should_show_origin then Some origin else None -let violation_description _ dereference_type ~nullable_object_descr ~nullable_object_origin = +let violation_description nullability ~dereference_location dereference_type ~nullable_object_descr + ~nullable_object_origin = let module MF = MarkupFormatter in - let what_is_dereferred_str = - 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 suffix = - get_origin_opt ~nullable_object_descr nullable_object_origin - |> Option.bind ~f:(fun origin -> TypeOrigin.get_description origin) - |> Option.value_map ~f:(fun origin -> ": " ^ origin) ~default:"." - in - Format.sprintf "%s is nullable and is not locally checked for null when %s%s" - what_is_dereferred_str action_descr suffix + match nullability with + | Nullability.DeclaredNonnull -> + (* This can happen only in strict mode. + This type of violation is more subtle than the normal case because, so it should be rendered in a special way *) + ErrorRenderingUtils.get_strict_mode_violation_issue ~bad_usage_location:dereference_location + nullable_object_origin + | _ -> + let what_is_dereferred_str = + 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 suffix = + get_origin_opt ~nullable_object_descr nullable_object_origin + |> Option.bind ~f:(fun origin -> TypeOrigin.get_description origin) + |> Option.value_map ~f:(fun origin -> ": " ^ origin) ~default:"." + in + let description = + Format.sprintf "%s is nullable and is not locally checked for null when %s%s" + what_is_dereferred_str action_descr suffix + in + (description, IssueType.eradicate_nullable_dereference, dereference_location) diff --git a/infer/src/nullsafe/DereferenceRule.mli b/infer/src/nullsafe/DereferenceRule.mli index f8b005602..9afaffe97 100644 --- a/infer/src/nullsafe/DereferenceRule.mli +++ b/infer/src/nullsafe/DereferenceRule.mli @@ -23,7 +23,9 @@ type dereference_type = val violation_description : violation + -> dereference_location:Location.t -> dereference_type -> nullable_object_descr:string option -> nullable_object_origin:TypeOrigin.t - -> string + -> string * IssueType.t * Location.t +(** Given context around violation, return error message together with the info where to put this message *) diff --git a/infer/src/nullsafe/ErrorRenderingUtils.ml b/infer/src/nullsafe/ErrorRenderingUtils.ml index 7d5b35cec..6e2e3886d 100644 --- a/infer/src/nullsafe/ErrorRenderingUtils.ml +++ b/infer/src/nullsafe/ErrorRenderingUtils.ml @@ -56,3 +56,106 @@ let is_object_nullability_self_explanatory ~object_expression object_origin = | TypeOrigin.OptimisticFallback | TypeOrigin.Undef -> false + + +type message_info = + { offending_object: string + ; object_loc: Location.t + ; coming_from_explanation: string + ; what_is_used: string + ; recommendation: string + ; issue_type: IssueType.t } + +let get_method_class_name procname = + match procname with + | Typ.Procname.Java java_pname -> + Some (Typ.Procname.Java.get_simple_class_name java_pname) + | _ -> + None + + +let get_field_class_name field_name = + let class_with_field = Typ.Fieldname.to_simplified_string field_name in + String.rsplit2 class_with_field ~on:'.' + |> Option.value_map ~f:(fun (classname, _) -> classname) ~default:"the field class" + + +let get_info object_origin = + match object_origin with + | TypeOrigin.MethodCall {pname; call_loc} -> + let offending_object = + Format.asprintf "%a" MarkupFormatter.pp_monospaced + (Typ.Procname.to_simplified_string ~withclass:true pname) + in + let object_loc = call_loc in + let suggested_third_party_sig_file = + ThirdPartyAnnotationInfo.lookup_related_sig_file_by_package + (ThirdPartyAnnotationGlobalRepo.get_repo ()) + pname + in + let what_is_used = "Result of this call" in + (* Two main cases: it is either FB owned code or third party. + We determine the difference based on presense of suggested_third_party_sig_file. + *) + let coming_from_explanation, recommendation, issue_type = + let what_to_strictify = + Option.value (get_method_class_name pname) ~default:offending_object + in + match suggested_third_party_sig_file with + | None -> + ( "non-strict classes" + , Format.sprintf "strictify %s" what_to_strictify + , IssueType.eradicate_forbidden_non_strict_in_strict ) + | Some sig_file_name -> + ( "not vetted third party methods" + , Format.sprintf "add the correct signature to %s" + (ThirdPartyAnnotationGlobalRepo.get_user_friendly_third_party_sig_file_name + ~filename:sig_file_name) + , IssueType.eradicate_unvetted_third_party_in_strict ) + in + { offending_object + ; object_loc + ; coming_from_explanation + ; what_is_used + ; recommendation + ; issue_type } + | TypeOrigin.Field {field_name; access_loc} -> + let offending_object = + Format.asprintf "%a" MarkupFormatter.pp_monospaced + (Typ.Fieldname.to_simplified_string field_name) + in + let object_loc = access_loc in + (* TODO: currently we do not support third-party annotations for fields. Because of this, + render error like it is a non-stict class. *) + let what_is_used = "This field" in + let coming_from_explanation = "non-strict classes" in + let recommendation = Format.sprintf "strictify %s" (get_field_class_name field_name) in + let issue_type = IssueType.eradicate_forbidden_non_strict_in_strict in + { offending_object + ; object_loc + ; coming_from_explanation + ; what_is_used + ; recommendation + ; issue_type } + | _ -> + Logging.die Logging.InternalError + "Invariant violation: unexpected origin of declared non-nullable value" + + +let get_strict_mode_violation_issue ~bad_usage_location object_origin = + let { offending_object + ; object_loc + ; coming_from_explanation + ; what_is_used + ; recommendation + ; issue_type } = + get_info object_origin + in + let description = + Format.sprintf + "%s: `@NullsafeStrict` mode prohibits using values coming from %s without a check. %s is \ + used at line %d. Either add a local check for null or assertion, or %s." + offending_object coming_from_explanation what_is_used bad_usage_location.Location.line + recommendation + in + (description, issue_type, object_loc) diff --git a/infer/src/nullsafe/ErrorRenderingUtils.mli b/infer/src/nullsafe/ErrorRenderingUtils.mli index ba4aa729a..d9ae1b8ec 100644 --- a/infer/src/nullsafe/ErrorRenderingUtils.mli +++ b/infer/src/nullsafe/ErrorRenderingUtils.mli @@ -14,3 +14,12 @@ val is_object_nullability_self_explanatory : object_expression:string -> TypeOri we render its origin. In some cases this is redundant and adds extra noise for the user. *) + +val get_strict_mode_violation_issue : + bad_usage_location:Location.t -> TypeOrigin.t -> string * IssueType.t * Location.t +(** Situation when we tried to use DeclaredNonnull as Nonnull. + This is disallowed only in strict mode. + Returns a tuple (error message, issue type, error location). + NOTE: Location of the error will be NOT in the place when the value is used (that is [bad_usage_location]), + but where the value is first obtained from. + *) diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml index dea499f44..12ef62fe3 100644 --- a/infer/src/nullsafe/eradicateChecks.ml +++ b/infer/src/nullsafe/eradicateChecks.ml @@ -36,7 +36,11 @@ let check_object_dereference ~is_strict_mode tenv find_canonical_duplicate curr_ let nullable_object_descr = explain_expr tenv node object_exp in let type_error = TypeErr.Nullable_dereference - {dereference_violation; nullable_object_descr; dereference_type; nullable_object_origin} + { dereference_violation + ; dereference_location= loc + ; nullable_object_descr + ; dereference_type + ; nullable_object_origin } in report_error tenv find_canonical_duplicate type_error (Some instr_ref) loc curr_pname ) @@ -163,6 +167,7 @@ let check_field_assignment ~is_strict_mode tenv find_canonical_duplicate curr_pd report_error tenv find_canonical_duplicate (TypeErr.Bad_assignment { assignment_violation + ; assignment_location= loc ; rhs_origin ; assignment_type= AssignmentRule.AssigningToField fname }) (Some instr_ref) loc curr_pdesc ) @@ -329,6 +334,7 @@ let check_return_not_nullable ~is_strict_mode tenv find_canonical_duplicate loc report_error tenv find_canonical_duplicate (TypeErr.Bad_assignment { assignment_violation + ; assignment_location= loc ; rhs_origin ; assignment_type= AssignmentRule.ReturningFromFunction curr_pname }) None loc curr_pdesc ) @@ -439,6 +445,7 @@ let check_call_parameters ~is_strict_mode ~callee_annotated_signature tenv find_ report_error tenv find_canonical_duplicate (TypeErr.Bad_assignment { assignment_violation + ; assignment_location= loc ; rhs_origin ; assignment_type= AssignmentRule.PassingParamToFunction diff --git a/infer/src/nullsafe/typeErr.ml b/infer/src/nullsafe/typeErr.ml index 4400417c2..575aed340 100644 --- a/infer/src/nullsafe/typeErr.ml +++ b/infer/src/nullsafe/typeErr.ml @@ -70,11 +70,13 @@ type err_instance = ; violation_type: OverAnnotatedRule.violation_type } | Nullable_dereference of { dereference_violation: DereferenceRule.violation + ; dereference_location: Location.t ; dereference_type: DereferenceRule.dereference_type ; nullable_object_descr: string option ; nullable_object_origin: TypeOrigin.t } | Bad_assignment of { assignment_violation: AssignmentRule.violation + ; assignment_location: Location.t ; assignment_type: AssignmentRule.assignment_type ; rhs_origin: TypeOrigin.t } [@@deriving compare] @@ -195,29 +197,6 @@ type st_report_error = -> string -> unit -let get_infer_issue_type = function - | Condition_redundant _ -> - IssueType.eradicate_condition_redundant - | Over_annotation {violation_type= OverAnnotatedRule.FieldOverAnnoted _} -> - IssueType.eradicate_field_over_annotated - | Over_annotation {violation_type= OverAnnotatedRule.ReturnOverAnnotated _} -> - IssueType.eradicate_return_over_annotated - | Field_not_initialized _ -> - IssueType.eradicate_field_not_initialized - | Bad_assignment {assignment_type= PassingParamToFunction _} -> - IssueType.eradicate_parameter_not_nullable - | Bad_assignment {assignment_type= AssigningToField _} -> - IssueType.eradicate_field_not_nullable - | Bad_assignment {assignment_type= ReturningFromFunction _} -> - IssueType.eradicate_return_not_nullable - | Nullable_dereference _ -> - IssueType.eradicate_nullable_dereference - | Inconsistent_subclass {violation_type= InconsistentReturn} -> - IssueType.eradicate_inconsistent_subclass_return_annotation - | Inconsistent_subclass {violation_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 @@ -234,39 +213,66 @@ let get_field_name_for_error_suppressing = function None -let get_error_description err_instance = +let get_error_info err_instance = 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 + ( P.sprintf "The condition %s is always %b according to the existing annotations." + (Option.value s_opt ~default:"") is_always_true + , IssueType.eradicate_condition_redundant + , None ) | Over_annotation {over_annotated_violation; violation_type} -> - OverAnnotatedRule.violation_description over_annotated_violation violation_type + ( OverAnnotatedRule.violation_description over_annotated_violation violation_type + , ( match violation_type with + | OverAnnotatedRule.FieldOverAnnoted _ -> + IssueType.eradicate_field_over_annotated + | OverAnnotatedRule.ReturnOverAnnotated _ -> + IssueType.eradicate_return_over_annotated ) + , None ) | Field_not_initialized field_name -> - Format.asprintf - "Field %a is declared non-nullable, so it should be initialized in the constructor or in \ - an `@Initializer` method" - MF.pp_monospaced - (Typ.Fieldname.to_flat_string field_name) - | Bad_assignment {rhs_origin; assignment_type; assignment_violation} -> - AssignmentRule.violation_description assignment_violation assignment_type ~rhs_origin + ( Format.asprintf + "Field %a is declared non-nullable, so it should be initialized in the constructor or in \ + an `@Initializer` method" + MF.pp_monospaced + (Typ.Fieldname.to_flat_string field_name) + , IssueType.eradicate_field_not_initialized + , None ) + | Bad_assignment {rhs_origin; assignment_location; assignment_type; assignment_violation} -> + let description, issue_type, error_location = + AssignmentRule.violation_description ~assignment_location assignment_violation + assignment_type ~rhs_origin + in + (description, issue_type, Some error_location) | Nullable_dereference - {dereference_violation; nullable_object_descr; dereference_type; nullable_object_origin} -> - DereferenceRule.violation_description dereference_violation dereference_type - ~nullable_object_descr ~nullable_object_origin + { dereference_violation + ; dereference_location + ; nullable_object_descr + ; dereference_type + ; nullable_object_origin } -> + let description, issue_type, error_location = + DereferenceRule.violation_description ~dereference_location dereference_violation + dereference_type ~nullable_object_descr ~nullable_object_origin + in + (description, issue_type, Some error_location) | Inconsistent_subclass {inheritance_violation; violation_type; base_proc_name; overridden_proc_name} -> - InheritanceRule.violation_description inheritance_violation violation_type ~base_proc_name - ~overridden_proc_name + ( InheritanceRule.violation_description inheritance_violation violation_type ~base_proc_name + ~overridden_proc_name + , ( match violation_type with + | InconsistentReturn -> + IssueType.eradicate_inconsistent_subclass_return_annotation + | InconsistentParam _ -> + IssueType.eradicate_inconsistent_subclass_parameter_annotation ) + , None ) (** 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 err_description, infer_issue_type, updated_location = get_error_info 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 infer_issue_type loc ~field_name + let error_location = Option.value updated_location ~default:loc in + st_report_error pname pdesc infer_issue_type error_location ~field_name ~exception_kind:(fun k d -> Exceptions.Eradicate (k, d)) ?severity err_description diff --git a/infer/src/nullsafe/typeErr.mli b/infer/src/nullsafe/typeErr.mli index 21761b89a..444ce33b9 100644 --- a/infer/src/nullsafe/typeErr.mli +++ b/infer/src/nullsafe/typeErr.mli @@ -46,11 +46,13 @@ type err_instance = ; violation_type: OverAnnotatedRule.violation_type } | Nullable_dereference of { dereference_violation: DereferenceRule.violation + ; dereference_location: Location.t ; dereference_type: DereferenceRule.dereference_type ; nullable_object_descr: string option ; nullable_object_origin: TypeOrigin.t } | Bad_assignment of { assignment_violation: AssignmentRule.violation + ; assignment_location: Location.t ; assignment_type: AssignmentRule.assignment_type ; rhs_origin: TypeOrigin.t } [@@deriving compare] diff --git a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp index 4eff5748b..ad2ff70d6 100644 --- a/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp +++ b/infer/tests/codetoanalyze/java/nullsafe-default/issues.exp @@ -216,14 +216,14 @@ codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.n codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable.return_null_in_catch():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [`return_null_in_catch()`: return type is declared non-nullable but the method returns a nullable value: null constant at line 160.] codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable.return_null_in_catch_after_throw():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [`return_null_in_catch_after_throw()`: return type is declared non-nullable but the method returns a nullable value: null constant at line 172.] codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable.tryWithResourcesReturnNullable(java.lang.String):java.lang.Object, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [`tryWithResourcesReturnNullable(...)`: return type is declared non-nullable but the method returns a nullable value: call to nullToNullableIsOK() at line 142.] -codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_convertingNonnullToNonnullIsBad():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [`nonStrictClass_convertingNonnullToNonnullIsBad()`: return type is declared non-nullable but the method returns a nullable value: call to getNonnull() at line 146.] +codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_convertingNonnullToNonnullIsBad():java.lang.String, 2, ERADICATE_UNCHECKED_NONSTRICT_FROM_STRICT, no_bucket, WARNING, [`NonStrict.getNonnull()`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. Result of this call is used at line 144. Either add a local check for null or assertion, or strictify NonStrict.] codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_convertingNonnullToNullableIsOK():java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, WARNING, [Method `nonStrictClass_convertingNonnullToNullableIsOK()` is annotated with `@Nullable` but never returns null.] codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_convertingNullableToNonnullIsBad():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [`nonStrictClass_convertingNullableToNonnullIsBad()`: return type is declared non-nullable but the method returns a nullable value: call to getNullable() at line 135.] codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullFieldAfterCheckIsOK():void, 2, ERADICATE_CONDITION_REDUNDANT, no_bucket, WARNING, [The condition NonStrict.nonnull is always true according to the existing annotations.] -codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullFieldIsBad():void, 2, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`__new(...).nonnull` is nullable and is not locally checked for null when calling `toString()`.] +codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullFieldIsBad():void, 2, ERADICATE_UNCHECKED_NONSTRICT_FROM_STRICT, no_bucket, WARNING, [`NonStrict.nonnull`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. This field is used at line 131. Either add a local check for null or assertion, or strictify NonStrict.] codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullMethodAfterCheckIsOK():void, 2, ERADICATE_CONDITION_REDUNDANT, no_bucket, WARNING, [The condition lang.String(o)V is always true according to the existing annotations.] -codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullMethodIsBad():void, 2, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`__new(...).getNonnull()` is nullable and is not locally checked for null when calling `toString()`.] -codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullStaticMethodIsBad():void, 2, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`staticNonnull()` is nullable and is not locally checked for null when calling `toString()`.] +codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullMethodIsBad():void, 2, ERADICATE_UNCHECKED_NONSTRICT_FROM_STRICT, no_bucket, WARNING, [`NonStrict.getNonnull()`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. Result of this call is used at line 113. Either add a local check for null or assertion, or strictify NonStrict.] +codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNonnullStaticMethodIsBad():void, 2, ERADICATE_UNCHECKED_NONSTRICT_FROM_STRICT, no_bucket, WARNING, [`NonStrict.staticNonnull()`: `@NullsafeStrict` mode prohibits using values coming from non-strict classes without a check. Result of this call is used at line 122. Either add a local check for null or assertion, or strictify NonStrict.] codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNullableFieldIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`__new(...).nullable` is nullable and is not locally checked for null when calling `toString()`.] codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNullableMethodIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`__new(...).getNullable()` is nullable and is not locally checked for null when calling `toString()`.] codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.nonStrictClass_dereferenceNullableStaticMethodIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`staticNullable()` is nullable and is not locally checked for null when calling `toString()`.] @@ -236,7 +236,7 @@ codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.strictClass_dereferenceNullableMethodIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`__new(...).getNullable()` is nullable and is not locally checked for null when calling `toString()`.] codetoanalyze/java/nullsafe-default/StrictMode.java, codetoanalyze.java.nullsafe_default.Strict.strictClass_dereferenceNullableStaticMethodIsBad():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`staticNullable()` is nullable and is not locally checked for null when calling `toString()`.] codetoanalyze/java/nullsafe-default/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe_default.StrictModeForThirdParty.dereferenceSpecifiedAsNullableIsBAD():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`StrictModeForThirdParty.obj.returnSpecifiedAsNullable()` is nullable and is not locally checked for null when calling `toString()`: call to ThirdPartyTestClass.returnSpecifiedAsNullable() at line 45 (declared nullable in nullsafe-default/third-party-signatures/some.test.pckg.sig at line 2)] -codetoanalyze/java/nullsafe-default/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe_default.StrictModeForThirdParty.dereferenceUnspecifiedIsBAD():void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`StrictModeForThirdParty.obj.returnUnspecified()` is nullable and is not locally checked for null when calling `toString()`.] +codetoanalyze/java/nullsafe-default/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe_default.StrictModeForThirdParty.dereferenceUnspecifiedIsBAD():void, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_STRICT, no_bucket, WARNING, [`ThirdPartyTestClass.returnUnspecified()`: `@NullsafeStrict` mode prohibits using values coming from not vetted third party methods without a check. Result of this call is used at line 41. Either add a local check for null or assertion, or add the correct signature to nullsafe-default/third-party-signatures/some.test.pckg.sig.] codetoanalyze/java/nullsafe-default/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe_default.StrictModeForThirdParty.passingNullableParamToUnspecifiedIsBAD():void, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [`ThirdPartyTestClass.paramUnspecified(...)`: parameter #1(`param`) is declared non-nullable but the argument `getNullable()` is nullable.] codetoanalyze/java/nullsafe-default/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe_default.StrictModeForThirdParty.passingNullableToParamSpecifiedAsNonnullIsBAD():void, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [Third-party `ThirdPartyTestClass.secondParamSpecifiedAsNonnull(...)` is missing a signature that would allow passing a nullable to param #2(`specifiedAsNonnull`). Actual argument `getNullable()` is nullable. Consider adding the correct signature of `ThirdPartyTestClass.secondParamSpecifiedAsNonnull(...)` to nullsafe-default/third-party-signatures/some.test.pckg.sig.] codetoanalyze/java/nullsafe-default/third-party-test-code/some/test/pckg/ThirdPartyTestClass.java, some.test.pckg.ThirdPartyTestClass.returnSpecifiedAsNullable():java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, WARNING, [Method `returnSpecifiedAsNullable()` is annotated with `@Nullable` but never returns null.]