[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

@@ -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),

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),

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),

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"
 

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

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 is_declared_nonnull_to_nonnull ~lhs ~rhs =
-  let nullability_evidence =
+  match (lhs, rhs) with Nullability.Nonnull, Nullability.DeclaredNonnull -> true | _ -> false
-    get_origin_opt assignment_type rhs_origin
+
-    |> Option.bind ~f:(fun origin -> TypeOrigin.get_description origin)
+
-  in
+let get_issue_type = function
-  let nullability_evidence_as_suffix =
+  | PassingParamToFunction _ ->
-    Option.value_map nullability_evidence ~f:(fun evidence -> ": " ^ evidence) ~default:""
+      IssueType.eradicate_parameter_not_nullable
-  in
+  | AssigningToField _ ->
-  let module MF = MarkupFormatter in
+      IssueType.eradicate_field_not_nullable
-  match assignment_type with
+  | ReturningFromFunction _ ->
-  | PassingParamToFunction function_info ->
+      IssueType.eradicate_return_not_nullable
-      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
+let violation_description {is_strict_mode; lhs; rhs} ~assignment_location assignment_type
-        (Typ.Fieldname.to_flat_string field_name)
+    ~rhs_origin =
-        nullability_evidence_as_suffix
+  if is_declared_nonnull_to_nonnull ~lhs ~rhs then (
-  | ReturningFromFunction function_proc_name ->
+    if not is_strict_mode then
-      Format.asprintf
+      Logging.die InternalError "Unexpected situation: should not be a violation not in strict mode" ;
-        "%a: return type is declared non-nullable but the method returns a nullable value%s."
+    (* This type of violation is more subtle than the normal case because, so it should be rendered in a special way *)
-        MF.pp_monospaced
+    ErrorRenderingUtils.get_strict_mode_violation_issue ~bad_usage_location:assignment_location
-        (Typ.Procname.to_simplified_string ~withclass:false function_proc_name)
+      rhs_origin )
-        nullability_evidence_as_suffix
+  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)

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 *)

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 nullability with
-    match dereference_type with
+  | Nullability.DeclaredNonnull ->
-    | MethodCall _ | AccessToField _ -> (
+      (* This can happen only in strict mode.
-      match nullable_object_descr with
+         This type of violation is more subtle than the normal case because, so it should be rendered in a special way *)
-      | None ->
+      ErrorRenderingUtils.get_strict_mode_violation_issue ~bad_usage_location:dereference_location
-          "Object"
+        nullable_object_origin
-      (* Just describe an object itself *)
+  | _ ->
-      | Some descr ->
+      let what_is_dereferred_str =
-          MF.monospaced_to_string descr )
+        match dereference_type with
-    | ArrayLengthAccess | AccessByIndex _ -> (
+        | MethodCall _ | AccessToField _ -> (
-      (* In Java, those operations can be applied only to arrays *)
+          match nullable_object_descr with
-      match nullable_object_descr with
+          | None ->
-      | None ->
+              "Object"
-          "Array"
+          (* Just describe an object itself *)
-      | Some descr ->
+          | Some descr ->
-          Format.sprintf "Array %s" (MF.monospaced_to_string descr) )
+              MF.monospaced_to_string descr )
-  in
+        | ArrayLengthAccess | AccessByIndex _ -> (
-  let action_descr =
+          (* In Java, those operations can be applied only to arrays *)
-    match dereference_type with
+          match nullable_object_descr with
-    | MethodCall method_name ->
+          | None ->
-        Format.sprintf "calling %s"
+              "Array"
-          (MF.monospaced_to_string (Typ.Procname.to_simplified_string method_name))
+          | Some descr ->
-    | AccessToField field_name ->
+              Format.sprintf "Array %s" (MF.monospaced_to_string descr) )
-        Format.sprintf "accessing field %s"
+      in
-          (MF.monospaced_to_string (Typ.Fieldname.to_simplified_string field_name))
+      let action_descr =
-    | AccessByIndex {index_desc} ->
+        match dereference_type with
-        Format.sprintf "accessing at index %s" (MF.monospaced_to_string index_desc)
+        | MethodCall method_name ->
-    | ArrayLengthAccess ->
+            Format.sprintf "calling %s"
-        "accessing its length"
+              (MF.monospaced_to_string (Typ.Procname.to_simplified_string method_name))
-  in
+        | AccessToField field_name ->
-  let suffix =
+            Format.sprintf "accessing field %s"
-    get_origin_opt ~nullable_object_descr nullable_object_origin
+              (MF.monospaced_to_string (Typ.Fieldname.to_simplified_string field_name))
-    |> Option.bind ~f:(fun origin -> TypeOrigin.get_description origin)
+        | AccessByIndex {index_desc} ->
-    |> Option.value_map ~f:(fun origin -> ": " ^ origin) ~default:"."
+            Format.sprintf "accessing at index %s" (MF.monospaced_to_string index_desc)
-  in
+        | ArrayLengthAccess ->
-  Format.sprintf "%s is nullable and is not locally checked for null when %s%s"
+            "accessing its length"
-    what_is_dereferred_str action_descr suffix
+      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)

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 *)

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)

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.
+ *)

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

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."
+      ( P.sprintf "The condition %s is always %b according to the existing annotations."
-        (Option.value s_opt ~default:"") is_always_true
+          (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
+      ( Format.asprintf
-        "Field %a is declared non-nullable, so it should be initialized in the constructor or in \
+          "Field %a is declared non-nullable, so it should be initialized in the constructor or in \
-         an `@Initializer` method"
+           an `@Initializer` method"
-        MF.pp_monospaced
+          MF.pp_monospaced
-        (Typ.Fieldname.to_flat_string field_name)
+          (Typ.Fieldname.to_flat_string field_name)
-  | Bad_assignment {rhs_origin; assignment_type; assignment_violation} ->
+      , IssueType.eradicate_field_not_initialized
-      AssignmentRule.violation_description assignment_violation assignment_type ~rhs_origin
+      , 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} ->
+      { dereference_violation
-      DereferenceRule.violation_description dereference_violation dereference_type
+      ; dereference_location
-        ~nullable_object_descr ~nullable_object_origin
+      ; 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
+      ( InheritanceRule.violation_description inheritance_violation violation_type ~base_proc_name
-        ~overridden_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
 

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]

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_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_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_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.]