capture overriding when no override annotations

Summary: This solves T181646571 `hack.MethodOverride` only captured method annotated with Override. ``` predicate MethodOverrides : { derived : MethodDeclaration, base : MethodDeclaration, annotation : maybe bool, // to distinguish override <<__Override>> annotation } ``` ## How it works Define a method to compute origin of an overridden method ``` Decl_provider.get_overridden_method ctx ~class_name ~method_name ~is_static |> Decl_entry.to_option >>= fun method_ -> Some method_.Typing_defs.ce_origin ``` and we move the logic from `index_xrefs.ml` (which indexes xrefs, and identified the override from the attribute xref), to `index_decl.ml`which indexes all methods). Reviewed By: pepeiborra Differential Revision: D57452291 fbshipit-source-id: 812d061f52501de21a5f6952c7cc60ae9cb6f612
facebook · May 18, 2024 · 417d359 · 417d359
1 parent 13c9e6f
commit 417d359
Show file tree

Hide file tree

Showing 9 changed files with 80 additions and 49 deletions.
diff --git a/hphp/hack/src/typing/write_symbol_info/add_fact.ml b/hphp/hack/src/typing/write_symbol_info/add_fact.ml
@@ -345,10 +345,17 @@ let method_defn source_text meth decl_id fa =
   Fact_acc.add_fact Predicate.(Hack MethodDefinition) json fa
 
 let method_overrides
-    meth_name base_cont_name base_cont_type der_cont_name der_cont_type fa =
+    ~annotation
+    meth_name
+    base_cont_name
+    base_cont_type
+    der_cont_name
+    der_cont_type
+    fa =
   let json =
     MethodOverrides.(
       {
+        annotation = Some annotation;
         derived = Build_fact.method_decl meth_name der_cont_name der_cont_type;
         base = Build_fact.method_decl meth_name base_cont_name base_cont_type;
       }

diff --git a/hphp/hack/src/typing/write_symbol_info/add_fact.mli b/hphp/hack/src/typing/write_symbol_info/add_fact.mli
@@ -74,6 +74,7 @@ val method_defn :
   Fact_id.t * Fact_acc.t
 
 val method_overrides :
+  annotation:bool ->
   string ->
   string ->
   Predicate.parent_container_type ->

diff --git a/hphp/hack/src/typing/write_symbol_info/glean/hack.ml b/hphp/hack/src/typing/write_symbol_info/glean/hack.ml
@@ -688,6 +688,7 @@ and MethodOverridden: sig
   and key= {
     base: MethodDeclaration.t;
     derived: MethodDeclaration.t;
+    annotation: bool option;
   }
   [@@deriving ord]
 
@@ -704,18 +705,23 @@ end = struct
   and key= {
     base: MethodDeclaration.t;
     derived: MethodDeclaration.t;
+    annotation: bool option;
   }
   [@@deriving ord]
 
   let rec to_json = function
     | Id f -> Util.id f
     | Key t -> Util.key (to_json_key t)
 
-  and to_json_key {base; derived} = 
+  and to_json_key {base; derived; annotation} = 
     let fields = [
       ("base", MethodDeclaration.to_json base);
       ("derived", MethodDeclaration.to_json derived);
     ] in
+    let fields =
+      match annotation with
+      | None -> fields
+      | Some annotation -> ("annotation", JSON_Bool annotation) :: fields in
     JSON_Object fields
 
 end
@@ -1887,6 +1893,7 @@ and MethodOverrides: sig
   and key= {
     derived: MethodDeclaration.t;
     base: MethodDeclaration.t;
+    annotation: bool option;
   }
   [@@deriving ord]
 
@@ -1903,18 +1910,23 @@ end = struct
   and key= {
     derived: MethodDeclaration.t;
     base: MethodDeclaration.t;
+    annotation: bool option;
   }
   [@@deriving ord]
 
   let rec to_json = function
     | Id f -> Util.id f
     | Key t -> Util.key (to_json_key t)
 
-  and to_json_key {derived; base} = 
+  and to_json_key {derived; base; annotation} = 
     let fields = [
       ("derived", MethodDeclaration.to_json derived);
       ("base", MethodDeclaration.to_json base);
     ] in
+    let fields =
+      match annotation with
+      | None -> fields
+      | Some annotation -> ("annotation", JSON_Bool annotation) :: fields in
     JSON_Object fields
 
 end

diff --git a/hphp/hack/src/typing/write_symbol_info/index_batch.ml b/hphp/hack/src/typing/write_symbol_info/index_batch.ml
@@ -47,7 +47,7 @@ let build_json ctx files_info ~ownership =
     let fa = process_source_text ctx fa file_info in
     let (fa, xrefs) = Index_xrefs.process_xrefs_and_calls ctx fa file_info in
     Fact_acc.set_pos_map fa xrefs.Xrefs.pos_map;
-    let (mod_xrefs, fa) = Index_decls.process_decls fa file_info in
+    let (mod_xrefs, fa) = Index_decls.process_decls ctx fa file_info in
     let fact_map_xrefs = xrefs.Xrefs.fact_map in
     let fact_map_module_xrefs = mod_xrefs.Xrefs.fact_map in
     let merge _ x _ = Some x in

diff --git a/hphp/hack/src/typing/write_symbol_info/index_decls.ml b/hphp/hack/src/typing/write_symbol_info/index_decls.ml
@@ -126,7 +126,7 @@ let process_member_cluster mc fa =
        mc.File_info.class_constants
 
 let process_container_decl
-    thrift_ctx path source_text con member_clusters (xrefs, all_decls, fa) =
+    ctx thrift_ctx path source_text con member_clusters (xrefs, all_decls, fa) =
   let (con_pos, con_name) = con.c_name in
   let parent_kind = Predicate.get_parent_kind con.c_kind in
   let decl_pred = Predicate.parent_decl_predicate parent_kind in
@@ -191,6 +191,33 @@ let process_container_decl
   in
   let (method_decls, fa) =
     List.fold_right con.c_methods ~init:([], fa) ~f:(fun meth (decls, fa) ->
+        let (_, method_name) = meth.m_name in
+        let class_name = con_name in
+        let fa =
+          match
+            Sym_def.get_overridden_method_origin
+              ctx
+              ~class_name
+              ~method_name
+              ~is_static:meth.m_static
+          with
+          | None -> fa
+          | Some (origin, override_kind) ->
+            let annotation =
+              List.exists meth.m_user_attributes ~f:(function
+                  | { ua_name = (_, "__Override"); _ } -> true
+                  | _ -> false)
+            in
+            Add_fact.method_overrides
+              ~annotation
+              method_name
+              origin
+              override_kind
+              class_name
+              parent_kind
+              fa
+            |> snd
+        in
         let (pos, id) = meth.m_name in
         let (decl_id, fa) =
           process_decl_loc
@@ -397,13 +424,14 @@ let process_mod_xref fa xrefs (pos, id) =
   in
   (Xrefs.add xrefs target_id pos Xrefs.{ target; receiver_type = None }, fa)
 
-let process_tast_decls ~path tast source_text (decls, fa) =
+let process_tast_decls ctx ~path tast source_text (decls, fa) =
   let thrift_ctx = Thrift.empty () in
   List.fold tast ~init:(Xrefs.empty, decls, fa) ~f:(fun acc (def, im) ->
       match def with
       | Class en when Util.is_enum_or_enum_class en.c_kind ->
         process_enum_decl path source_text en acc
-      | Class cd -> process_container_decl thrift_ctx path source_text cd im acc
+      | Class cd ->
+        process_container_decl ctx thrift_ctx path source_text cd im acc
       | Constant gd -> process_gconst_decl path source_text gd acc
       | Fun fd -> process_func_decl path source_text fd acc
       | Typedef td -> process_typedef_decl path source_text td acc
@@ -414,11 +442,11 @@ let process_tast_decls ~path tast source_text (decls, fa) =
         (xrefs, decls, fa)
       | _ -> acc)
 
-let process_decls fa File_info.{ path; tast; source_text; cst; _ } =
+let process_decls ctx fa File_info.{ path; tast; source_text; cst; _ } =
   Fact_acc.set_ownership_unit fa (Some path);
   let (_, fa) = Add_fact.file_lines ~path source_text fa in
   let (mod_xrefs, decls, fa) =
-    process_tast_decls ~path tast source_text ([], fa)
+    process_tast_decls ctx ~path tast source_text ([], fa)
   in
   let (decls, fa) = process_cst_decls source_text path cst (decls, fa) in
   (mod_xrefs, Add_fact.file_decls ~path decls fa |> snd)
diff --git a/hphp/hack/src/typing/write_symbol_info/index_decls.mli b/hphp/hack/src/typing/write_symbol_info/index_decls.mli
@@ -9,4 +9,7 @@
 (** Generate facts for all declarations in AST, and
     a XRefs map for the module references *)
 val process_decls :
-  Predicate.Fact_acc.t -> File_info.t -> Xrefs.t * Predicate.Fact_acc.t
+  Provider_context.t ->
+  Predicate.Fact_acc.t ->
+  File_info.t ->
+  Xrefs.t * Predicate.Fact_acc.t
diff --git a/hphp/hack/src/typing/write_symbol_info/index_xrefs.ml b/hphp/hack/src/typing/write_symbol_info/index_xrefs.ml
@@ -166,7 +166,7 @@ let process_container_xref (con_type, decl_pred) symbol_name pos (xrefs, fa) =
     pos
     (xrefs, fa)
 
-let process_attribute_xref ctx File_info.{ occ; def } opt_info (xrefs, fa) =
+let process_attribute_xref ctx File_info.{ occ; _ } (xrefs, fa) =
   let get_con_preds_from_name con_name =
     let con_name_with_ns = Utils.add_ns con_name in
     match Sym_def.get_class_by_name ctx con_name_with_ns with
@@ -189,43 +189,7 @@ let process_attribute_xref ctx File_info.{ occ; def } opt_info (xrefs, fa) =
   (* Process <<__Override>>, for which we write a MethodOverrides fact
      instead of a cross-reference *)
   let SymbolOccurrence.{ name; pos; _ } = occ in
-  if String.equal name "__Override" then
-    match opt_info with
-    | None ->
-      Hh_logger.log "WARNING: no override info for <<__Override>> instance";
-      (xrefs, fa)
-    | Some SymbolOccurrence.{ class_name; method_name; _ } ->
-      (match get_con_preds_from_name class_name with
-      | None -> (xrefs, fa)
-      | Some override_con_pred_types ->
-        (match def with
-        | None -> (xrefs, fa)
-        | Some Sym_def.{ full_name; _ } ->
-          (match Str.split (Str.regexp "::") full_name with
-          | [] -> (xrefs, fa)
-          | base_con_name :: _mem_name ->
-            (match get_con_preds_from_name base_con_name with
-            | None ->
-              Hh_logger.log
-                "WARNING: could not compute parent container type for override %s::%s"
-                class_name
-                method_name;
-              (xrefs, fa)
-            | Some base_con_pred_types ->
-              let (_fid, fa) =
-                Add_fact.method_overrides
-                  method_name
-                  base_con_name
-                  (fst base_con_pred_types)
-                  class_name
-                  (fst override_con_pred_types)
-                  fa
-              in
-              (* Cross-references for overrides could be added to xefs by calling
-                 'process_member_xref' here with 'sym_def' and 'occ.pos' *)
-              (xrefs, fa)))))
-  (* Ignore other built-in attributes *)
-  else if String.is_prefix name ~prefix:"__" then
+  if String.is_prefix name ~prefix:"__" then
     (xrefs, fa)
   (* Process user-defined attributes *)
   else
@@ -267,7 +231,7 @@ let process_xrefs ctx symbols fa : Xrefs.t * Fact_acc.t =
       else
         let pos = occ.pos in
         match occ.type_ with
-        | Attribute info -> process_attribute_xref ctx sym info (xrefs, fa)
+        | Attribute _info -> process_attribute_xref ctx sym (xrefs, fa)
         | _ ->
           (match def with
           | None ->

diff --git a/hphp/hack/src/typing/write_symbol_info/sym_def.ml b/hphp/hack/src/typing/write_symbol_info/sym_def.ml
@@ -38,3 +38,12 @@ let get_kind ctx class_ =
   match ServerSymbolDefinition.get_class_by_name ctx class_ with
   | None -> None
   | Some cls -> Some cls.Aast.c_kind
+
+let get_overridden_method_origin ctx ~class_name ~method_name ~is_static =
+  let open Option in
+  Decl_provider.get_overridden_method ctx ~class_name ~method_name ~is_static
+  |> Decl_entry.to_option
+  >>= fun method_ ->
+  Some method_.Typing_defs.ce_origin >>= fun origin ->
+  get_kind ctx origin >>= fun kind ->
+  Some (origin, Predicate.get_parent_kind kind)
diff --git a/hphp/hack/src/typing/write_symbol_info/sym_def.mli b/hphp/hack/src/typing/write_symbol_info/sym_def.mli
@@ -30,3 +30,10 @@ val get_class_by_name :
   Provider_context.t -> string -> [ `None | `Enum | `Class of Nast.class_ ]
 
 val get_kind : Provider_context.t -> string -> Ast_defs.classish_kind option
+
+val get_overridden_method_origin :
+  Provider_context.t ->
+  class_name:string ->
+  method_name:string ->
+  is_static:bool ->
+  (string * Predicate.parent_container_type) option