[eradicate] promote reports as errors for generated GraphQL source of nulls

Reviewed By: mbouaziz

Differential Revision: D8519147

fbshipit-source-id: 69f6364

infer/src/IR/Localise.ml

@@ -87,13 +87,6 @@ module BucketLevel = struct
   let b5 = "B5"
 end
 
-(** takes in input a tag to extract from the given error_desc
-    and returns its value *)
-let error_desc_extract_tag_value err_desc tag_to_extract =
-  let find_value tag v = match v with t, _ when String.equal t tag -> true | _ -> false in
-  match List.find ~f:(find_value tag_to_extract) err_desc.tags with Some (_, s) -> s | None -> ""
-
-
 (** get the bucket value of an error_desc, if any *)
 let error_desc_get_bucket err_desc = Tags.get err_desc.tags Tags.bucket
 

infer/src/IR/Localise.mli

@@ -41,9 +41,6 @@ module BucketLevel : sig
   (** lowest likelihood *)
 end
 
-val error_desc_extract_tag_value : error_desc -> string -> string
-(** returns the value of a tag or the empty string *)
-
 val error_desc_get_bucket : error_desc -> string option
 (** get the bucket value of an error_desc, if any *)
 

infer/src/absint/Checkers.ml

@@ -6,15 +6,24 @@
  *)
 
 open! IStd
+module L = Logging
 
 (** Module for user-defined checkers. *)
 
 module ST = struct
   let report_error tenv proc_name proc_desc kind loc ?(field_name= None) ?(origin_loc= None)
-      ?(exception_kind= fun k d -> Exceptions.Checkers (k, d)) ?(always_report= false) description =
+      ?(exception_kind= fun k d -> Exceptions.Checkers (k, d)) ?(severity= Exceptions.Kwarning)
-    let localized_description =
+      description =
-      Localise.custom_desc description [("always_report", string_of_bool always_report)]
+    let log =
+      match severity with
+      | Exceptions.Kwarning ->
+          Reporting.log_warning_deprecated
+      | Exceptions.Kerror ->
+          Reporting.log_error_deprecated
+      | _ ->
+          L.(die InternalError) "Severity not supported"
     in
+    let localized_description = Localise.custom_desc description [] in
     let exn = exception_kind kind localized_description in
     let suppressed = Reporting.is_suppressed tenv proc_desc kind ~field_name in
     let trace =
@@ -27,5 +36,5 @@ module ST = struct
       in
       origin_elements @ [Errlog.make_trace_element 0 loc description []]
     in
-    if not suppressed then Reporting.log_warning_deprecated proc_name ~loc ~ltr:trace exn
+    if not suppressed then log proc_name ~loc ~ltr:trace exn
 end

infer/src/absint/Checkers.mli

@@ -13,7 +13,7 @@ module ST : sig
   val report_error :
     Tenv.t -> Typ.Procname.t -> Procdesc.t -> IssueType.t -> Location.t
     -> ?field_name:Typ.Fieldname.t option -> ?origin_loc:Location.t option
-    -> ?exception_kind:(IssueType.t -> Localise.error_desc -> exn) -> ?always_report:bool -> string
+    -> ?exception_kind:(IssueType.t -> Localise.error_desc -> exn) -> ?severity:Exceptions.err_kind
-    -> unit
+    -> string -> unit
   (** Report an error. *)
 end

infer/src/backend/InferPrint.ml

@@ -230,8 +230,7 @@ module IssuesJson = struct
       not (SourceFile.is_infer_model source_file) || Config.debug_mode || Config.debug_exceptions
     in
     if
-      key.in_footprint && error_filter source_file key.err_desc key.err_name
+      key.in_footprint && error_filter source_file key.err_name && should_report_source_file
-      && should_report_source_file
       && should_report key.err_kind key.err_name key.err_desc err_data.err_class
     then (
       let kind = Exceptions.err_kind_string key.err_kind in
@@ -380,7 +379,7 @@ module IssuesTxt = struct
           err_data.loc.Location.file
     in
     if
-      key.in_footprint && error_filter source_file key.err_desc key.err_name
+      key.in_footprint && error_filter source_file key.err_name
       && (not Config.filtering || String.is_empty (censored_reason key.err_name source_file))
     then
       Exceptions.pp_err ~node_key:err_data.node_id_key.node_key err_data.loc key.err_kind
@@ -472,7 +471,7 @@ module Stats = struct
     let found_errors = ref false in
     let process_row (key: Errlog.err_key) (err_data: Errlog.err_data) =
       let type_str = key.err_name.IssueType.unique_id in
-      if key.in_footprint && error_filter key.err_desc key.err_name then
+      if key.in_footprint && error_filter key.err_name then
         match key.err_kind with
         | Exceptions.Kerror ->
             found_errors := true ;
@@ -646,13 +645,10 @@ module Issue = struct
     issues'
 end
 
-let error_filter filters proc_name file error_desc error_name =
+let error_filter filters proc_name file error_name =
-  let always_report () =
-    String.equal (Localise.error_desc_extract_tag_value error_desc "always_report") "true"
-  in
   (Config.write_html || not (IssueType.(equal skip_function) error_name))
-  && (filters.Inferconfig.path_filter file || always_report ())
+  && filters.Inferconfig.path_filter file && filters.Inferconfig.error_filter error_name
-  && filters.Inferconfig.error_filter error_name && filters.Inferconfig.proc_filter proc_name
+  && filters.Inferconfig.proc_filter proc_name
 
 
 type report_kind = Issues | Procs | Stats | Summary [@@deriving compare]

infer/src/checkers/annotations.ml

@@ -85,7 +85,7 @@ let returns_ownership = "ReturnsOwnership"
 
 let synchronized_collection = "SynchronizedCollection"
 
-let strict = "com.facebook.infer.annotation.Strict"
+let generated_graphql = "GeneratedGraphQL"
 
 let suppress_lint = "SuppressLint"
 
@@ -132,8 +132,6 @@ let ia_ends_with ia ann_name = List.exists ~f:(fun (a, _) -> annot_ends_with a a
 
 let ia_contains ia ann_name = List.exists ~f:(class_name_matches ann_name) ia
 
-let ia_get ia ann_name = List.find ~f:(class_name_matches ann_name) ia |> Option.map ~f:fst
-
 let pdesc_get_return_annot pdesc =
   fst (Procdesc.get_attributes pdesc).ProcAttributes.method_annotation
 
@@ -219,8 +217,6 @@ let ia_is_field_injector_readwrite ia =
 
 let ia_is_mutable ia = ia_ends_with ia mutable_
 
-let ia_get_strict ia = ia_get ia strict
-
 let ia_is_verify ia = ia_contains ia verify_annotation
 
 let ia_is_expensive ia = ia_ends_with ia expensive

infer/src/checkers/annotations.mli

@@ -45,6 +45,8 @@ val ui_thread : string
 
 val visibleForTesting : string
 
+val generated_graphql : string
+
 val annot_ends_with : Annot.t -> string -> bool
 (** [annot_ends_with annot ann_name] returns true if the class name of [annot], without the package,
     is equal to [ann_name] *)
@@ -54,8 +56,6 @@ val ia_ends_with : Annot.Item.t -> string -> bool
 
 val ia_has_annotation_with : Annot.Item.t -> (Annot.t -> bool) -> bool
 
-val ia_get_strict : Annot.Item.t -> Annot.t option
-
 val ia_is_false_on_null : Annot.Item.t -> bool
 
 val ia_is_initializer : Annot.Item.t -> bool

infer/src/eradicate/typeErr.ml

@@ -211,63 +211,43 @@ let add_err find_canonical_duplicate err_instance instr_ref_opt loc =
 
 (* print now if it's not a forall check *)
 
-module Strict = struct
+module Severity = struct
-  let method_get_strict (signature: AnnotatedSignature.t) =
+  let get_severity ia =
-    let ia, _ = signature.ret in
+    if Annotations.ia_ends_with ia Annotations.generated_graphql then Some Exceptions.Kerror
-    Annotations.ia_get_strict ia
+    else None
 
 
-  let this_type_get_strict tenv (signature: AnnotatedSignature.t) =
+  let this_type_get_severity tenv (signature: AnnotatedSignature.t) =
     match signature.params with
-    | (p, _, this_type) :: _ when String.equal (Mangled.to_string p) "this" -> (
+    | (p, _, this_type) :: _ when String.equal (Mangled.to_string p) "this" ->
-      match PatternMatch.type_get_annotation tenv this_type with
+        Option.bind ~f:get_severity (PatternMatch.type_get_annotation tenv this_type)
-      | Some ia ->
-          Annotations.ia_get_strict ia
-      | None ->
-          None )
     | _ ->
         None
 
 
-  let signature_get_strict tenv signature =
+  let origin_descr_get_severity tenv origin_descr =
-    match method_get_strict signature with
-    | None ->
-        this_type_get_strict tenv signature
-    | Some x ->
-        Some x
-
-
-  let origin_descr_get_strict tenv origin_descr =
     match origin_descr with
     | _, _, Some signature ->
-        signature_get_strict tenv signature
+        this_type_get_severity tenv signature
     | _, _, None ->
         None
 
 
-  let report_on_method_arguments = false
+  let err_instance_get_severity tenv err_instance : Exceptions.err_kind option =
-
-  (* Return (Some parameters) if there is a method call on a @Nullable object,*)
-  (* where the origin of @Nullable in the analysis is the return value of a Strict method*)
-  (* with parameters. A method is Strict if it or its class are annotated @Strict. *)
-  let err_instance_get_strict tenv err_instance : Annot.t option =
     match err_instance with
     | Call_receiver_annotation_inconsistent (AnnotatedSignature.Nullable, _, _, origin_descr)
     | Null_field_access (_, _, origin_descr, _) ->
-        origin_descr_get_strict tenv origin_descr
+        origin_descr_get_severity tenv origin_descr
-    | Parameter_annotation_inconsistent (AnnotatedSignature.Nullable, _, _, _, _, origin_descr)
-      when report_on_method_arguments ->
-        origin_descr_get_strict tenv origin_descr
     | _ ->
         None
 end
 
-(* Strict *)
+(* Severity *)
 
 type st_report_error =
   Typ.Procname.t -> Procdesc.t -> IssueType.t -> Location.t -> ?field_name:Typ.Fieldname.t option
   -> ?origin_loc:Location.t option -> ?exception_kind:(IssueType.t -> Localise.error_desc -> exn)
-  -> ?always_report:bool -> string -> unit
+  -> ?severity:Exceptions.err_kind -> string -> unit
 
 (** Report an error right now. *)
 let report_error_now tenv (st_report_error: st_report_error) err_instance loc pdesc : unit =
@@ -447,10 +427,10 @@ let report_error_now tenv (st_report_error: st_report_error) err_instance loc pd
         , None
         , None )
   in
-  let always_report = Strict.err_instance_get_strict tenv err_instance <> None in
+  let severity = Severity.err_instance_get_severity tenv err_instance in
   st_report_error pname pdesc kind loc ~field_name ~origin_loc
     ~exception_kind:(fun k d -> Exceptions.Eradicate (k, d))
-    ~always_report description
+    ?severity description
 
 
 (** Report an error unless is has been reported already, or unless it's a forall error

infer/src/eradicate/typeErr.mli

@@ -67,7 +67,7 @@ val node_reset_forall : Procdesc.Node.t -> unit
 type st_report_error =
   Typ.Procname.t -> Procdesc.t -> IssueType.t -> Location.t -> ?field_name:Typ.Fieldname.t option
   -> ?origin_loc:Location.t option -> ?exception_kind:(IssueType.t -> Localise.error_desc -> exn)
-  -> ?always_report:bool -> string -> unit
+  -> ?severity:Exceptions.err_kind -> string -> unit
 
 val report_error :
   Tenv.t -> st_report_error -> (Procdesc.Node.t -> Procdesc.Node.t) -> err_instance

infer/tests/codetoanalyze/java/eradicate/GeneratedGraphQL.java

@@ -0,0 +1,16 @@
+/*
+ * Copyright (c) 2018-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+package codetoanalyze.java.eradicate;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Retention(RetentionPolicy.CLASS)
+@Target(ElementType.TYPE)
+public @interface GeneratedGraphQL {}

infer/tests/codetoanalyze/java/eradicate/NullMethodCall.java

@@ -305,4 +305,7 @@ public class NullMethodCall {
     manager.cancel(intent);
   }
 
+  String callingSeverSideNullableGetter(ServerSideDeserializer deserializer) {
+    return deserializer.nullableGetter().toString();
+  }
 }

infer/tests/codetoanalyze/java/eradicate/ServerSideDeserializer.java

@@ -0,0 +1,15 @@
+/*
+ * Copyright (c) 2018-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+package codetoanalyze.java.eradicate;
+
+import javax.annotation.Nullable;
+
+@GeneratedGraphQL
+public interface ServerSideDeserializer {
+
+  public @Nullable Object nullableGetter();
+}

infer/tests/codetoanalyze/java/eradicate/issues.exp

@@ -40,6 +40,7 @@ codetoanalyze/java/eradicate/NullFieldAccess.java, int NullFieldAccess.arrayLeng
 codetoanalyze/java/eradicate/NullFieldAccess.java, int NullFieldAccess.useInterface(NullFieldAccess$I), 2, ERADICATE_NULL_FIELD_ACCESS, no_bucket, WARNING, [origin,Object `c` could be null when accessing field `NullFieldAccess$C.n`. (Origin: field NullFieldAccess$I.c at line 50)]
 codetoanalyze/java/eradicate/NullFieldAccess.java, int NullFieldAccess.useX(), 2, ERADICATE_NULL_FIELD_ACCESS, no_bucket, WARNING, [origin,Object `c` could be null when accessing field `NullFieldAccess$C.n`. (Origin: field NullFieldAccess.x at line 35)]
 codetoanalyze/java/eradicate/NullFieldAccess.java, int NullFieldAccess.useZ(), 2, ERADICATE_NULL_FIELD_ACCESS, no_bucket, WARNING, [origin,Object `c` could be null when accessing field `NullFieldAccess$C.n`. (Origin: field NullFieldAccess.z at line 45)]
+codetoanalyze/java/eradicate/NullMethodCall.java, String NullMethodCall.callingSeverSideNullableGetter(ServerSideDeserializer), 1, ERADICATE_NULL_METHOD_CALL, no_bucket, ERROR, [origin,The value of `deserializer.nullableGetter()` in the call to `toString()` could be null. (Origin: call to nullableGetter() at line 309)]
 codetoanalyze/java/eradicate/NullMethodCall.java, int NullMethodCall$Inner.outerField(), 2, ERADICATE_NULL_METHOD_CALL, no_bucket, WARNING, [origin,The value of `s` in the call to `length()` could be null. (Origin: field NullMethodCall.fld at line 74)]
 codetoanalyze/java/eradicate/NullMethodCall.java, int NullMethodCall$Inner.outerPrivateField(), 2, ERADICATE_NULL_METHOD_CALL, no_bucket, WARNING, [origin,The value of `s` in the call to `length()` could be null. (Origin: field NullMethodCall.pfld at line 85)]
 codetoanalyze/java/eradicate/NullMethodCall.java, int NullMethodCall.testSystemGetenvBad(), 2, ERADICATE_NULL_METHOD_CALL, no_bucket, WARNING, [origin,The value of `envValue` in the call to `length()` could be null. (Origin: call to getenv(...) modelled in modelTables.ml at line 246)]