[nullsafe] Build and output the annotation graph

Summary:
This diff glues the previous work together.
The ClassLevelAnalysis finds list of provisional violation, builds the
graph based on them, and outputs this graph as a separate issue.

Reviewed By: artempyanykh

Differential Revision: D24682802

fbshipit-source-id: 8174da91a

infer/man/man1/infer-full.txt

@@ -426,6 +426,7 @@ OPTIONS
              DIVIDE_BY_ZERO (disabled by default),
              DO_NOT_REPORT (enabled by default),
              EMPTY_VECTOR_ACCESS (enabled by default),
+             ERADICATE_ANNOTATION_GRAPH (enabled by default),
              ERADICATE_BAD_NESTED_CLASS_ANNOTATION (enabled by default),
              ERADICATE_CONDITION_REDUNDANT (enabled by default),
              ERADICATE_FIELD_NOT_INITIALIZED (enabled by default),

infer/man/man1/infer-report.txt

@@ -134,6 +134,7 @@ OPTIONS
              DIVIDE_BY_ZERO (disabled by default),
              DO_NOT_REPORT (enabled by default),
              EMPTY_VECTOR_ACCESS (enabled by default),
+             ERADICATE_ANNOTATION_GRAPH (enabled by default),
              ERADICATE_BAD_NESTED_CLASS_ANNOTATION (enabled by default),
              ERADICATE_CONDITION_REDUNDANT (enabled by default),
              ERADICATE_FIELD_NOT_INITIALIZED (enabled by default),

infer/man/man1/infer.txt

@@ -426,6 +426,7 @@ OPTIONS
              DIVIDE_BY_ZERO (disabled by default),
              DO_NOT_REPORT (enabled by default),
              EMPTY_VECTOR_ACCESS (enabled by default),
+             ERADICATE_ANNOTATION_GRAPH (enabled by default),
              ERADICATE_BAD_NESTED_CLASS_ANNOTATION (enabled by default),
              ERADICATE_CONDITION_REDUNDANT (enabled by default),
              ERADICATE_FIELD_NOT_INITIALIZED (enabled by default),

infer/src/absint/PatternMatch.ml

@@ -521,6 +521,14 @@ let lookup_attributes tenv proc_name =
   !found_attributes
 
 
+let lookup_attributes_exn tenv proc_name =
+  match lookup_attributes tenv proc_name with
+  | Some result ->
+      result
+  | None ->
+      Logging.die InternalError "Did not find attributes for %a" Procname.pp proc_name
+
+
 (** return the set of instance fields that are assigned to a null literal in [procdesc] *)
 let get_fields_nullified procdesc =
   (* walk through the instructions and look for instance fields that are assigned to null *)

infer/src/absint/PatternMatch.mli

@@ -170,6 +170,8 @@ val override_iter : (Procname.t -> unit) -> Tenv.t -> Procname.t -> unit
 
 val lookup_attributes : Tenv.t -> Procname.t -> ProcAttributes.t option
 
+val lookup_attributes_exn : Tenv.t -> Procname.t -> ProcAttributes.t
+
 val type_name_get_annotation : Tenv.t -> Typ.name -> Annot.Item.t option
 
 val type_get_annotation : Tenv.t -> Typ.t -> Annot.Item.t option

infer/src/atd/jsonbug.atd

@@ -35,12 +35,34 @@ type method_info = {
   call_line: int;
 }
 
+type annotation_point_kind = [Method | Field | Param]
+
+type access_level = [Private | Protected | Public | Default]
+
+type annotation_point_method = {
+  method_name: string;
+  params: string list;
+  access_level: access_level;
+}
+
+(* A potential place to annotate as [@Nullable] *)
+type annotation_point = {
+  id: string; (* A symbolic id of an annotation point *)
+  kind: annotation_point_kind;
+  ?method_info: annotation_point_method option; (* Present iff this is a Method or Param annotation point *)
+  ?field_name: string option; (* Present iff this is a Field annotation point *)
+  ?param_num: int option; (* Present iff this is a Param annotation point *)
+  num_violations: int; (* How many violations (NOT counting violations happening in other annotation points) would be introduced if this point was annotated *)
+  dependent_point_ids: string list; (* List of other points that need to be annotated when this one is annotated *)
+}
+
 type nullsafe_extra = {
   class_name: string;
   package: string nullable;
   ?nullable_methods: method_info list option; (* If the issue is related to unsafe use of methods being nullable, here's the list of them *)
   ?unvetted_3rd_party: string list option; (* If the issue is related to the use of a not yet registered third-party methods, here's the list of their signatures *)
-  ?meta_issue_info: nullsafe_meta_issue_info option (* Should be present if the issue is "nullsafe meta issue" *)
+  ?meta_issue_info: nullsafe_meta_issue_info option; (* Should be present if the issue is "nullsafe meta issue" *)
+  ?annotation_graph: annotation_point list option; (* Annotation graph (only for annotation mode) *)
 }
 
 type extra = {

infer/src/base/IssueType.ml

@@ -468,6 +468,13 @@ let empty_vector_access =
     ~user_documentation:[%blob "../../documentation/issues/EMPTY_VECTOR_ACCESS.md"]
 
 
+(* A technical issue needed to output the annotation graph for the class - not intended to be surfaces to the end user *)
+let eradicate_annotation_graph =
+  (* Enabled by default since this requires a special mode anyway *)
+  register ~id:"ERADICATE_ANNOTATION_GRAPH" ~hum:"Annotation Graph" Info Eradicate
+    ~user_documentation:""
+
+
 (* Condition redundant is a very non-precise issue. Depending on the origin of what is compared with
    null, this can have a lot of reasons to be actually nullable.
 

infer/src/base/IssueType.mli

@@ -154,6 +154,8 @@ val do_not_report : t
 
 val empty_vector_access : t
 
+val eradicate_annotation_graph : t
+
 val eradicate_condition_redundant : t
 
 val eradicate_field_not_initialized : t

infer/src/nullsafe/AnnotationGraph.ml

@@ -0,0 +1,210 @@
+(*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *)
+open! IStd
+
+type point_info =
+  { id: string
+  ; annotation: ProvisionalAnnotation.t
+  ; mutable num_violations: int
+  ; mutable dependent_points: ProvisionalAnnotation.t list }
+
+module AnnotationMap = Caml.Map.Make (struct
+  type t = ProvisionalAnnotation.t [@@deriving compare]
+end)
+
+let get_provisional_annotation = function
+  | AnnotatedNullability.ProvisionallyNullable provisional_annotation ->
+      Some provisional_annotation
+  | _ ->
+      None
+
+
+(* Corresponding provisional annotations for the return value and params, if any *)
+let annotations_of_signature AnnotatedSignature.{ret; params} =
+  let annotated_nullability =
+    ret.ret_annotated_type.nullability
+    :: List.map params ~f:(fun AnnotatedSignature.{param_annotated_type= {nullability}} ->
+           nullability )
+  in
+  List.filter_map annotated_nullability ~f:get_provisional_annotation
+
+
+(* Given a list of provisional annotations, construct corresponding nodes for the graph *)
+let graph_nodes_of_provisional_annotations annotations =
+  let get_id index annotation =
+    (* Symbolic prefix corresponding to annotation point type for convenience*)
+    let prefix =
+      match annotation with
+      | ProvisionalAnnotation.Field _ ->
+          "f"
+      | ProvisionalAnnotation.Method _ ->
+          "m"
+      | ProvisionalAnnotation.Param _ ->
+          "p"
+    in
+    Format.sprintf "%s%d" prefix index
+  in
+  let annotation_points =
+    List.mapi annotations ~f:(fun index annotation ->
+        ( annotation
+        , {id= get_id index annotation; annotation; num_violations= 0; dependent_points= []} ) )
+  in
+  AnnotationMap.of_seq (Stdlib.List.to_seq annotation_points)
+
+
+let build_graph_nodes tenv class_struct class_name =
+  let class_typ = Typ.mk_struct class_name in
+  let field_annotations =
+    class_struct.Struct.fields
+    |> List.filter_map ~f:(fun (field_name, _, _) ->
+           let AnnotatedField.{annotated_type= {nullability}} =
+             Option.value_exn
+               (AnnotatedField.get tenv field_name ~class_typ ~class_under_analysis:class_name)
+           in
+           get_provisional_annotation nullability )
+  in
+  let method_signatures =
+    class_struct.Struct.methods
+    |> List.map ~f:(fun proc_name ->
+           let proc_attributes = Option.value_exn (PatternMatch.lookup_attributes tenv proc_name) in
+           AnnotatedSignature.get_for_class_under_analysis tenv proc_attributes )
+  in
+  let method_and_param_annotations =
+    List.map method_signatures ~f:annotations_of_signature |> List.concat
+  in
+  graph_nodes_of_provisional_annotations (field_annotations @ method_and_param_annotations)
+
+
+let get_offending_annotations = function
+  | ProvisionalViolation.Assignment violation ->
+      AssignmentRule.ProvisionalViolation.offending_annotations violation
+  | ProvisionalViolation.Dereference violation ->
+      DereferenceRule.ProvisionalViolation.offending_annotations violation
+
+
+let get_fix_annotation = function
+  | ProvisionalViolation.Assignment violation ->
+      AssignmentRule.ProvisionalViolation.fix_annotation violation
+  | ProvisionalViolation.Dereference _ ->
+      None
+
+
+let update_by_violation nodes provisional_violation =
+  let fix_annotation = get_fix_annotation provisional_violation in
+  Option.iter fix_annotation ~f:(fun annotation ->
+      if not (AnnotationMap.mem annotation nodes) then
+        Logging.die InternalError "Did not find a node for %a in the graph" ProvisionalAnnotation.pp
+          annotation ) ;
+  let offending_annotations = get_offending_annotations provisional_violation in
+  List.iter offending_annotations ~f:(fun annotation ->
+      match AnnotationMap.find_opt annotation nodes with
+      | Some annotation_point -> (
+        match fix_annotation with
+        | Some fix_annotation ->
+            (* If that provisional annotation becomes real [@Nullable], that would raise an issue fixable by the other annotation.
+               Add the new edge in the graph.
+            *)
+            annotation_point.dependent_points <-
+              List.dedup_and_sort
+                (fix_annotation :: annotation_point.dependent_points)
+                ~compare:ProvisionalAnnotation.compare
+        | None ->
+            (* If that provisional annotation becomes real [@Nullable], that would lead to a violation
+             * (not related to other provisional annotations).
+             *)
+            annotation_point.num_violations <- annotation_point.num_violations + 1 )
+      | None ->
+          Logging.die InternalError "Did not find a node for %a in the graph"
+            ProvisionalAnnotation.pp annotation )
+
+
+(* Given a list of provisional violations, connect corresponding nodes in the annotation graph *)
+let update_nodes_and_set_edges nodes provisional_violations =
+  List.iter provisional_violations ~f:(update_by_violation nodes)
+
+
+let get_kind_json = function
+  | ProvisionalAnnotation.Field _ ->
+      `Field
+  | ProvisionalAnnotation.Method _ ->
+      `Method
+  | ProvisionalAnnotation.Param _ ->
+      `Param
+
+
+let get_field_name_json = function
+  | ProvisionalAnnotation.Field {field_name} ->
+      Some (Fieldname.get_field_name field_name)
+  | ProvisionalAnnotation.Method _ | ProvisionalAnnotation.Param _ ->
+      None
+
+
+let get_param_num_json = function
+  | ProvisionalAnnotation.Param {num} ->
+      Some num
+  | ProvisionalAnnotation.Method _ | ProvisionalAnnotation.Field _ ->
+      None
+
+
+let java_type_to_string java_type = Pp.string_of_pp (Typ.pp_java ~verbose:true) java_type
+
+let get_access_level tenv proc_name =
+  let proc_attributes = PatternMatch.lookup_attributes_exn tenv (Procname.Java proc_name) in
+  match ProcAttributes.get_access proc_attributes with
+  | Default ->
+      `Default
+  | Public ->
+      `Public
+  | Private ->
+      `Private
+  | Protected ->
+      `Protected
+
+
+let get_method_info_json tenv annotation =
+  let open IOption.Let_syntax in
+  let+ proc_name =
+    match annotation with
+    | ProvisionalAnnotation.Param {method_info} ->
+        Some method_info
+    | ProvisionalAnnotation.Method proc_name ->
+        Some proc_name
+    | ProvisionalAnnotation.Field _ ->
+        None
+  in
+  let method_name = Procname.Java.get_method proc_name in
+  let params = Procname.Java.get_parameters proc_name |> List.map ~f:java_type_to_string in
+  let access_level = get_access_level tenv proc_name in
+  Jsonbug_t.{method_name; params; access_level}
+
+
+let to_json_annotation_point tenv graph {id; annotation; num_violations; dependent_points} :
+    Jsonbug_t.annotation_point =
+  { id
+  ; kind= get_kind_json annotation
+  ; method_info= get_method_info_json tenv annotation
+  ; field_name= get_field_name_json annotation
+  ; param_num= get_param_num_json annotation
+  ; num_violations
+  ; dependent_point_ids=
+      List.map dependent_points ~f:(fun dependent_point ->
+          let node = AnnotationMap.find dependent_point graph in
+          node.id ) }
+
+
+(* Convert the graph to the JSON representation *)
+let to_json tenv graph =
+  AnnotationMap.bindings graph |> List.map ~f:snd
+  (* Sort by ids to get any stable order *)
+  |> List.sort ~compare:(fun {id= id1} {id= id2} -> String.compare id1 id2)
+  |> List.map ~f:(to_json_annotation_point tenv graph)
+
+
+let build_graph tenv class_struct class_name provisional_violations =
+  let graph = build_graph_nodes tenv class_struct class_name in
+  update_nodes_and_set_edges graph provisional_violations ;
+  to_json tenv graph

infer/src/nullsafe/AnnotationGraph.mli

@@ -0,0 +1,18 @@
+(*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *)
+open! IStd
+
+(** Annotation graph for a Java class comprises of: * Nodes (annotation points): potential places in
+    the class declaration to be annotated as [@Nullable] * For each annotation point the graph
+    stores what will happen if this annotation point becomes real [@Nullable] (new violations that
+    will arise) * Edges: if annotating a point A will require annotating another point B as
+    [@Nullable], A -> B are connected in the graph. *)
+
+val build_graph :
+  Tenv.t -> Struct.t -> Typ.name -> ProvisionalViolation.t list -> Jsonbug_t.annotation_point list
+(** Given a Java class and the list of all provisional violations found in that class, build the
+    annotation graph *)

infer/src/nullsafe/ClassLevelAnalysis.ml

@@ -188,7 +188,8 @@ let report_meta_issue_for_top_level_class tenv source_file class_name class_stru
         ; package
         ; meta_issue_info= Some meta_issue_info
         ; unvetted_3rd_party= None
-        ; nullable_methods= None }
+        ; nullable_methods= None
+        ; annotation_graph= None }
     in
     log_issue ~issue_log ~loc:class_loc ~severity ~nullsafe_extra issue_type description
 
@@ -214,7 +215,12 @@ let analyze_nullsafe_annotations tenv source_file class_name class_struct issue_
     let package = JavaClassName.package class_name in
     let class_name = JavaClassName.classname class_name in
     Jsonbug_t.
-      {class_name; package; meta_issue_info= None; unvetted_3rd_party= None; nullable_methods= None}
+      { class_name
+      ; package
+      ; meta_issue_info= None
+      ; unvetted_3rd_party= None
+      ; nullable_methods= None
+      ; annotation_graph= None }
   in
   match NullsafeMode.check_problematic_class_annotation tenv class_name with
   | Ok () ->
@@ -250,58 +256,45 @@ let analyze_nullsafe_annotations tenv source_file class_name class_struct issue_
         IssueType.eradicate_bad_nested_class_annotation description
 
 
-let process_issue_for_annotation_graph issue =
-  match issue with
-  | TypeErr.Condition_redundant _
-  | TypeErr.Field_not_initialized _
-  | TypeErr.Inconsistent_subclass _
-  | TypeErr.Over_annotation _ ->
-      ()
-  | TypeErr.Nullable_dereference {dereference_violation} ->
-      DereferenceRule.ProvisionalViolation.from dereference_violation
-      |> Option.iter ~f:(fun provisional_violation ->
-             let annotations =
-               DereferenceRule.ProvisionalViolation.offending_annotations provisional_violation
-             in
-             Logging.debug Analysis Medium
-               "Found provisional violation: dereference caused by any of %a\n"
-               (Pp.seq ProvisionalAnnotation.pp) annotations )
-  | TypeErr.Bad_assignment {assignment_violation} ->
-      AssignmentRule.ProvisionalViolation.from assignment_violation
-      |> Option.iter ~f:(fun provisional_violation ->
-             let offending_annotations =
-               AssignmentRule.ProvisionalViolation.offending_annotations provisional_violation
-             in
-             let fix_annotation =
-               AssignmentRule.ProvisionalViolation.fix_annotation provisional_violation
-             in
-             let fix_annotation_descr =
-               Option.value_map fix_annotation
-                 ~f:(fun annotation ->
-                   Format.asprintf ", fixable by %a" ProvisionalAnnotation.pp annotation )
-                 ~default:""
-             in
-             Logging.debug Analysis Medium
-               "Found provisional violation: assignment caused by any of %a%s\n"
-               (Pp.seq ProvisionalAnnotation.pp) offending_annotations fix_annotation_descr )
-
-
-let construct_annotation_graph _class_name class_info issue_log =
+let report_annotation_graph source_file class_name class_struct annotation_graph issue_log =
+  let class_loc = get_class_loc source_file class_struct in
+  let package = JavaClassName.package class_name in
+  let class_name = JavaClassName.classname class_name in
+  let nullsafe_extra =
+    Jsonbug_t.
+      { class_name
+      ; package
+      ; meta_issue_info= None
+      ; unvetted_3rd_party= None
+      ; nullable_methods= None
+      ; annotation_graph= Some annotation_graph }
+  in
+  log_issue ~issue_log ~loc:class_loc ~severity:IssueType.Info ~nullsafe_extra
+    IssueType.eradicate_annotation_graph ""
+
+
+let build_and_report_annotation_graph tenv source_file class_name class_struct class_info issue_log
+    =
   if not Config.nullsafe_annotation_graph then issue_log
-  else (
-    (* TODO: actually construct the graph (just print provisional violations for now) *)
-    AggregatedSummaries.ClassInfo.get_summaries class_info
-    |> List.map ~f:(fun NullsafeSummary.{issues} -> issues)
-    |> List.fold ~init:[] ~f:( @ )
-    |> List.iter ~f:process_issue_for_annotation_graph ;
-    issue_log )
+  else
+    let class_typ_name = Typ.JavaClass class_name in
+    let provisional_violations =
+      AggregatedSummaries.ClassInfo.get_summaries class_info
+      |> List.map ~f:(fun NullsafeSummary.{issues} -> issues)
+      |> List.concat
+      |> List.filter_map ~f:ProvisionalViolation.of_issue
+    in
+    let annotation_graph =
+      AnnotationGraph.build_graph tenv class_struct class_typ_name provisional_violations
+    in
+    report_annotation_graph source_file class_name class_struct annotation_graph issue_log
 
 
 let analyze_class_impl tenv source_file class_name class_struct class_info issue_log =
   issue_log
   |> analyze_meta_issue_for_top_level_class tenv source_file class_name class_struct class_info
   |> analyze_nullsafe_annotations tenv source_file class_name class_struct
-  |> construct_annotation_graph class_name class_info
+  |> build_and_report_annotation_graph tenv source_file class_name class_struct class_info
 
 
 let analyze_class tenv source_file class_info issue_log =

infer/src/nullsafe/NullsafeIssue.ml

@@ -93,4 +93,10 @@ let get_nullsafe_extra {third_party_dependent_methods; nullable_methods} proc_na
   let nullable_methods =
     if List.is_empty nullable_methods then None else Some (to_nullable_method_json nullable_methods)
   in
-  Jsonbug_t.{class_name; package; meta_issue_info= None; unvetted_3rd_party; nullable_methods}
+  Jsonbug_t.
+    { class_name
+    ; package
+    ; meta_issue_info= None
+    ; unvetted_3rd_party
+    ; nullable_methods
+    ; annotation_graph= None }

infer/src/nullsafe/ProvisionalViolation.ml

@@ -0,0 +1,25 @@
+(*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *)
+
+open! IStd
+
+type t =
+  | Assignment of AssignmentRule.ProvisionalViolation.t
+  | Dereference of DereferenceRule.ProvisionalViolation.t
+
+let of_issue = function
+  | TypeErr.Condition_redundant _
+  | TypeErr.Field_not_initialized _
+  | TypeErr.Inconsistent_subclass _
+  | TypeErr.Over_annotation _ ->
+      None
+  | TypeErr.Nullable_dereference {dereference_violation} ->
+      DereferenceRule.ProvisionalViolation.from dereference_violation
+      |> Option.map ~f:(fun violation -> Dereference violation)
+  | TypeErr.Bad_assignment {assignment_violation} ->
+      AssignmentRule.ProvisionalViolation.from assignment_violation
+      |> Option.map ~f:(fun violation -> Assignment violation)

infer/src/nullsafe/ProvisionalViolation.mli

@@ -0,0 +1,17 @@
+(*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *)
+
+open! IStd
+
+(** A simple wrapper over provisional violations for Rules *)
+
+type t =
+  | Assignment of AssignmentRule.ProvisionalViolation.t
+  | Dereference of DereferenceRule.ProvisionalViolation.t
+
+val of_issue : TypeErr.err_instance -> t option
+(** If the nullsafe issue is associated with a provisional violation, return it *)