[nullsafe] ThirdPartyAnnotationInfoStorage stores files names and line numbers for signatures

Summary:
In the follow up diff we will use it to render more informative
error message when type error is relevant to the fact that the code is
third-party

Reviewed By: artempyanykh

Differential Revision: D18569783

fbshipit-source-id: 4f6dcc404

infer/src/nullsafe/ThirdPartyAnnotationInfo.ml

@@ -7,9 +7,13 @@
 open! IStd
 module Hashtbl = Caml.Hashtbl
 
-type storage = (ThirdPartyMethod.unique_repr, ThirdPartyMethod.nullability) Hashtbl.t
+type signature_info = {filename: string; line_number: int; nullability: ThirdPartyMethod.nullability}
 
-let create_storage () = Hashtbl.create 1
+type storage = {signature_map: signature_map; filenames: string list}
+
+and signature_map = (ThirdPartyMethod.unique_repr, signature_info) Hashtbl.t
+
+let create_storage () = {signature_map= Hashtbl.create 1; filenames= []}
 
 type file_parsing_error =
   {line_number: int; unparsable_method: string; parsing_error: ThirdPartyMethod.parsing_error}
@@ -28,21 +32,25 @@ let bind_list_with_index ~init list ~f =
       Result.bind acc ~f:(fun acc -> f acc index elem) )
 
 
-let parse_line_and_add_to_storage storage _line_index line =
+let parse_line_and_add_to_storage signature_map ~filename ~line_index line =
   let open Result in
   ThirdPartyMethod.parse line
   >>= fun (signature, nullability) ->
   Ok
-    ( Hashtbl.add storage signature nullability ;
-      storage )
+    ( Hashtbl.add signature_map signature {filename; line_number= line_index + 1; nullability} ;
+      signature_map )
 
 
-let add_from_signature_file storage ~lines =
+let add_from_signature_file storage ~filename ~lines =
   (* each line in a file should represent a method signature *)
-  bind_list_with_index lines ~init:storage ~f:(fun storage index method_as_str ->
-      parse_line_and_add_to_storage storage index method_as_str
+  let open Result in
+  let new_filenames = storage.filenames @ [filename] in
+  bind_list_with_index lines ~init:storage.signature_map
+    ~f:(fun signature_map line_index method_as_str ->
+      parse_line_and_add_to_storage signature_map ~filename ~line_index method_as_str
       |> Result.map_error ~f:(fun parsing_error ->
-             {line_number= index + 1; unparsable_method= method_as_str; parsing_error} ) )
+             {line_number= line_index + 1; unparsable_method= method_as_str; parsing_error} ) )
+  >>= fun new_map -> Ok {signature_map= new_map; filenames= new_filenames}
 
 
-let find_nullability_info storage unique_repr = Hashtbl.find_opt storage unique_repr
+let find_nullability_info {signature_map} unique_repr = Hashtbl.find_opt signature_map unique_repr

infer/src/nullsafe/ThirdPartyAnnotationInfo.mli

@@ -8,6 +8,11 @@ open! IStd
 
 (** In-memory storage the information about nullability annotation of third-party methods. *)
 
+type signature_info =
+  { filename: string  (** File where the particular signature is stored *)
+  ; line_number: int  (** Line number with this signature *)
+  ; nullability: ThirdPartyMethod.nullability }
+
 type storage
 
 val create_storage : unit -> storage
@@ -17,11 +22,11 @@ type file_parsing_error =
 
 val pp_parsing_error : Format.formatter -> file_parsing_error -> unit
 
-val add_from_signature_file : storage -> lines:string list -> (storage, file_parsing_error) result
+val add_from_signature_file :
+  storage -> filename:string -> lines:string list -> (storage, file_parsing_error) result
 (** Parse the information from the signature file, and add it to the storage *)
 
-val find_nullability_info :
-  storage -> ThirdPartyMethod.unique_repr -> ThirdPartyMethod.nullability option
+val find_nullability_info : storage -> ThirdPartyMethod.unique_repr -> signature_info option
 (** The main method. Do we have an information about the third-party method?
     If we do not, or it is not a third-party method, returns None.
     Otherwise returns the nullability information.

infer/src/nullsafe/ThirdPartyAnnotationInfoLoader.ml

@@ -15,9 +15,8 @@ let pp_load_error fmt {filename; parsing_error} =
 
 let add_from_file storage ~path_to_repo_dir ~sig_file =
   let lines = In_channel.read_lines (path_to_repo_dir ^ "/" ^ sig_file) in
-  ThirdPartyAnnotationInfo.add_from_signature_file storage ~lines
+  ThirdPartyAnnotationInfo.add_from_signature_file storage ~filename:sig_file ~lines
   |> Result.map_error ~f:(fun parsing_error -> {filename= sig_file; parsing_error})
-  |> Result.bind ~f:(fun _ -> Ok storage)
 
 
 let load ~path_to_repo_dir =

infer/src/nullsafe/models.ml

@@ -85,7 +85,8 @@ let get_modelled_annotated_signature tenv proc_attributes =
          ~f:
            (ThirdPartyAnnotationInfo.find_nullability_info
               (ThirdPartyAnnotationGlobalRepo.get_repo ()))
-    |> Option.map ~f:to_modelled_nullability
+    |> Option.map ~f:(fun ThirdPartyAnnotationInfo.{nullability} ->
+           to_modelled_nullability nullability )
     |> Option.value_map
        (* If we found information in third-party repo, overwrite annotated signature *)
          ~f:(AnnotatedSignature.set_modelled_nullability proc_name ann_sig)

infer/src/unit/nullsafe/ThirdPartyAnnotationInfoTests.ml

@@ -9,32 +9,37 @@ open! IStd
 open OUnit2
 module F = Format
 
-let assert_has_nullability_info storage unique_repr ~expected_nullability =
+let assert_has_nullability_info ?expected_file ?expected_line storage unique_repr
+    ~expected_nullability =
   match ThirdPartyAnnotationInfo.find_nullability_info storage unique_repr with
   | None ->
       assert_failure
         (F.asprintf "Expected to find info for %a, but it was not found"
            ThirdPartyMethod.pp_unique_repr unique_repr)
-  | Some nullability ->
+  | Some {filename; line_number; nullability} ->
       assert_equal expected_nullability nullability
         ~msg:
           (F.asprintf "Nullability info for %a does not match" ThirdPartyMethod.pp_unique_repr
              unique_repr)
-        ~printer:(Pp.string_of_pp ThirdPartyMethod.pp_nullability)
+        ~printer:(Pp.string_of_pp ThirdPartyMethod.pp_nullability) ;
+      Option.iter expected_file ~f:(fun expected_file ->
+          assert_equal expected_file filename ~msg:"Filename does not match" ) ;
+      Option.iter expected_line ~f:(fun expected_line ->
+          assert_equal expected_line line_number ~msg:"Line number does not match" )
 
 
 let assert_no_info storage unique_repr =
   match ThirdPartyAnnotationInfo.find_nullability_info storage unique_repr with
   | None ->
       ()
-  | Some nullability ->
+  | Some {nullability} ->
       assert_failure
         (F.asprintf "Did not expect to find nullability info for method %a, but found %a"
            ThirdPartyMethod.pp_unique_repr unique_repr ThirdPartyMethod.pp_nullability nullability)
 
 
-let add_from_annot_file_and_check_success storage ~lines =
-  match ThirdPartyAnnotationInfo.add_from_signature_file storage ~lines with
+let add_from_annot_file_and_check_success storage ~filename ~lines =
+  match ThirdPartyAnnotationInfo.add_from_signature_file storage ~filename ~lines with
   | Ok storage ->
       storage
   | Error parsing_error ->
@@ -43,8 +48,8 @@ let add_from_annot_file_and_check_success storage ~lines =
            ThirdPartyAnnotationInfo.pp_parsing_error parsing_error)
 
 
-let add_from_annot_file_and_check_failure storage ~lines ~expected_error_line_number =
-  match ThirdPartyAnnotationInfo.add_from_signature_file storage ~lines with
+let add_from_annot_file_and_check_failure storage ~filename ~lines ~expected_error_line_number =
+  match ThirdPartyAnnotationInfo.add_from_signature_file storage ~filename ~lines with
   | Ok _ ->
       assert_failure
         "Expected to not be able to parse the file, but it was successfully parsed instead"
@@ -60,15 +65,19 @@ let basic_find =
   let lines = ["a.A#foo(b.B)"; "b.B#bar(c.C, @Nullable d.D) @Nullable"] in
   (* Load some functions from the file *)
   let storage =
-    add_from_annot_file_and_check_success (ThirdPartyAnnotationInfo.create_storage ()) ~lines
+    add_from_annot_file_and_check_success ~filename:"test.sig"
+      (ThirdPartyAnnotationInfo.create_storage ())
+      ~lines
   in
   (* Make sure we can find what we just stored *)
   assert_has_nullability_info storage
     {class_name= "a.A"; method_name= Method "foo"; param_types= ["b.B"]}
-    ~expected_nullability:{ret_nullability= Nonnull; param_nullability= [Nonnull]} ;
+    ~expected_nullability:{ret_nullability= Nonnull; param_nullability= [Nonnull]}
+    ~expected_file:"test.sig" ~expected_line:1 ;
   assert_has_nullability_info storage
     {class_name= "b.B"; method_name= Method "bar"; param_types= ["c.C"; "d.D"]}
-    ~expected_nullability:{ret_nullability= Nullable; param_nullability= [Nonnull; Nullable]} ;
+    ~expected_nullability:{ret_nullability= Nullable; param_nullability= [Nonnull; Nullable]}
+    ~expected_file:"test.sig" ~expected_line:2 ;
   (* Make sure we can not find stuff we did not store *)
   (* Wrong class name *)
   assert_no_info storage {class_name= "a.AB"; method_name= Method "foo"; param_types= ["b.B"]} ;
@@ -97,7 +106,9 @@ let overload_resolution =
   in
   (* Load some functions from the file *)
   let storage =
-    add_from_annot_file_and_check_success (ThirdPartyAnnotationInfo.create_storage ()) ~lines
+    add_from_annot_file_and_check_success ~filename:"test.sig"
+      (ThirdPartyAnnotationInfo.create_storage ())
+      ~lines
   in
   (* Make sure we can find what we just stored *)
   (* a.b.SomeClass.foo with 1 param *)
@@ -153,21 +164,26 @@ let can_add_several_files =
   (* 1. Add file and check if we added info *)
   let file1 = ["a.A#foo(b.B)"; "b.B#bar(c.C, @Nullable d.D) @Nullable"] in
   let storage =
-    add_from_annot_file_and_check_success (ThirdPartyAnnotationInfo.create_storage ()) ~lines:file1
+    add_from_annot_file_and_check_success
+      (ThirdPartyAnnotationInfo.create_storage ())
+      ~filename:"file1.sig" ~lines:file1
   in
   assert_has_nullability_info storage
     {class_name= "a.A"; method_name= Method "foo"; param_types= ["b.B"]}
-    ~expected_nullability:{ret_nullability= Nonnull; param_nullability= [Nonnull]} ;
+    ~expected_nullability:{ret_nullability= Nonnull; param_nullability= [Nonnull]}
+    ~expected_file:"file1.sig" ~expected_line:1 ;
   (* 2. Add another file and check if we added info *)
   let file2 = ["e.E#baz(f.F)"; "g.G#<init>(h.H, @Nullable i.I) @Nullable"] in
-  let storage = add_from_annot_file_and_check_success storage ~lines:file2 in
+  let storage = add_from_annot_file_and_check_success storage ~filename:"file2.sig" ~lines:file2 in
   assert_has_nullability_info storage
     {class_name= "e.E"; method_name= Method "baz"; param_types= ["f.F"]}
-    ~expected_nullability:{ret_nullability= Nonnull; param_nullability= [Nonnull]} ;
+    ~expected_nullability:{ret_nullability= Nonnull; param_nullability= [Nonnull]}
+    ~expected_file:"file2.sig" ~expected_line:1 ;
   (* 3. Ensure we did not forget the content from the first file *)
   assert_has_nullability_info storage
     {class_name= "a.A"; method_name= Method "foo"; param_types= ["b.B"]}
     ~expected_nullability:{ret_nullability= Nonnull; param_nullability= [Nonnull]}
+    ~expected_file:"file1.sig" ~expected_line:1
 
 
 let should_not_forgive_unparsable_strings =
@@ -183,7 +199,7 @@ let should_not_forgive_unparsable_strings =
   (* Ensure we can add the good file, but can not add the bad one *)
   add_from_annot_file_and_check_success (ThirdPartyAnnotationInfo.create_storage ()) ~lines:file_ok
   |> ignore ;
-  add_from_annot_file_and_check_failure
+  add_from_annot_file_and_check_failure ~filename:"test.sig"
     (ThirdPartyAnnotationInfo.create_storage ())
     ~lines:file_bad ~expected_error_line_number:2