From 8d85fd8059cb5a5b4916bbd6baa30bb6d81d0ef5 Mon Sep 17 00:00:00 2001 From: Sasha Manzyuk Date: Fri, 6 Dec 2019 04:52:01 -0800 Subject: [PATCH] Fix calculation of direct required ancestors Summary: The heap API for fetching class decls offers functions for fetching the set of *all* ancestors and the set of *all* required ancestored (acquired through `require extends` and `require implements`). Given these, we calculate the set of *direct* ancestors (to be put into `extends` and `implements` clauses) and the set of *direct* required ancestors (to be added by placing suitable `require extends` / `require implements` in the body of a trait or interface). We calculate these sets by subtracting from the set of all ancestors the set of all ancestors' ancestors. Except that for required ancestors we calculate the ancestors' ancestors by taking the union of all required ancestors of the required ancestors of a given trait or interface, which is not correct. Consider this example (added as a new test case below): ``` abstract class B { public function f(): void {} } interface I { require extends B; } interface J extends I {} function with_indirect_require_extends(J $x): void { $x->f(); } ``` The set of required ancestors of `J` is `{B}` and `B` doesn't have require ancestors, so we end up with `{B}` as the set of direct required ancestors of `J`, and similarly for `I`, so that the extracted code looks like ``` interface I { require extends B; } interface J extends I { require extends B; } ``` Instead, we should take the union of all required ancestors over all ancestors and required ancestors. Reviewed By: Wilfred Differential Revision: D18764680 fbshipit-source-id: fff08335e23fd990d767470e3acd04ee0423782a --- hphp/hack/src/server/serverExtractStandalone.ml | 19 +++++++++++++++---- .../data/dependencies/classes-interfaces.php | 14 ++++++++++++++ .../expected/__with_indirect_require_extends.php.exp | 16 ++++++++++++++++ hphp/hack/test/integration/test_extract_standalone.py | 1 + 4 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 hphp/hack/test/integration/data/dependencies/expected/__with_indirect_require_extends.php.exp diff --git a/hphp/hack/src/server/serverExtractStandalone.ml b/hphp/hack/src/server/serverExtractStandalone.ml index e2a0a402c82..6d1fa98c551 100644 --- a/hphp/hack/src/server/serverExtractStandalone.ml +++ b/hphp/hack/src/server/serverExtractStandalone.ml @@ -496,21 +496,32 @@ let get_direct_ancestors cls = let set_of_sequence seq = Sequence.fold seq ~f:(fun set x -> SSet.add x set) ~init:SSet.empty in - let get_direct (get_ancestors : Class.t -> string Sequence.t) = + let get_direct ~get_ancestors ~get_ancestor_ancestors = let ancestors = get_ancestors cls in let ancestors_ancestors = Sequence.concat_map ancestors ~f:(fun ancestor_name -> Option.value_map (get_class ancestor_name) ~default:Sequence.empty - ~f:get_ancestors) + ~f:get_ancestor_ancestors) |> set_of_sequence in Sequence.filter ancestors ~f:(fun x -> not (SSet.mem x ancestors_ancestors)) in - let direct_ancestors = get_direct Class.all_ancestor_names in - let direct_reqs = get_direct Class.all_ancestor_req_names in + let direct_ancestors = + get_direct + ~get_ancestors:Class.all_ancestor_names + ~get_ancestor_ancestors:Class.all_ancestor_names + in + let direct_reqs = + get_direct + ~get_ancestors:(fun cls -> + Sequence.append + (Class.all_ancestor_names cls) + (Class.all_ancestor_req_names cls)) + ~get_ancestor_ancestors:Class.all_ancestor_req_names + in let cls_kind = Class.kind cls in let filter_by_kind seq kind_condition = Sequence.to_list diff --git a/hphp/hack/test/integration/data/dependencies/classes-interfaces.php b/hphp/hack/test/integration/data/dependencies/classes-interfaces.php index 9a4c6ba0999..41190dffad7 100644 --- a/hphp/hack/test/integration/data/dependencies/classes-interfaces.php +++ b/hphp/hack/test/integration/data/dependencies/classes-interfaces.php @@ -40,3 +40,17 @@ class ImplementsBuiltin implements Stringish { } function does_not_use_class_methods(ImplementsBuiltin $arg): void {} + +abstract class Bee { + public function f(): void {} +} + +interface Eye { + require extends Bee; +} + +interface Jay extends Eye {} + +function with_indirect_require_extends(Jay $x): void { + $x->f(); +} diff --git a/hphp/hack/test/integration/data/dependencies/expected/__with_indirect_require_extends.php.exp b/hphp/hack/test/integration/data/dependencies/expected/__with_indirect_require_extends.php.exp new file mode 100644 index 00000000000..780c554fb2e --- /dev/null +++ b/hphp/hack/test/integration/data/dependencies/expected/__with_indirect_require_extends.php.exp @@ -0,0 +1,16 @@ +f(); +} +abstract class Bee { + public function f(): void { + throw new Exception(); + } +} +interface Eye { + require extends \Bee; +} +interface Jay extends \Eye {} +function extract_standalone_make_default(): nothing { + throw new Exception(); +} diff --git a/hphp/hack/test/integration/test_extract_standalone.py b/hphp/hack/test/integration/test_extract_standalone.py index bf8bea79282..4943b0de9e7 100644 --- a/hphp/hack/test/integration/test_extract_standalone.py +++ b/hphp/hack/test/integration/test_extract_standalone.py @@ -147,6 +147,7 @@ class TestExtractStandalone(TestCase[ExtractStandaloneDriver]): "\\with_enum_class_name", "\\with_type_const_from_implemented_interface", "\\with_nested_type_const", + "\\with_indirect_require_extends", ] for path in paths: -- 2.11.4.GIT