Fix --type-at-pos-batch bug w/ mixed splice/non-splice ET positions

Summary: `--type-at-pos-batch` asks for types at multiple positions. If there is a type in a splice and type not in a splice but both in the same expression tree, we want to report the non-virtualized type for the former and the virtualized for the latter. Previously, we were incorrectly handling it assuming all or nothing.

Reviewed By: CatherineGasnier

Differential Revision: D64939267

fbshipit-source-id: b079e1626d701b632e5bd4c0141abd5fa8652af9

hphp/hack/src/client_and_server/serverInferType.ml

@@ -315,13 +315,15 @@ let base_visitor ~human_friendly ~under_dynamic line_char_pairs =
          since the contents of a splice are just plain hack and don't need
          special treatment. If it isn't, use the virtualized expression to
          get the client type focussed view of the expression tree. *)
-      let sp = self#on_block env (Aast_utils.get_splices_from_et et) in
-      if List.for_all ~f:Option.is_none sp then
+      let splice_results =
+        self#on_block env (Aast_utils.get_splices_from_et et)
+      in
+      let virtual_results =
         match Aast_utils.get_virtual_expr_from_et et with
         | Some e -> self#on_expr env e
         | _ -> self#on_expr env et.Aast_defs.et_runtime_expr
-      else
-        sp
+      in
+      List.map2_exn splice_results virtual_results ~f:Option.first_some
   end
 
 (** Return the type of the node associated with exactly the given range.

hphp/hack/test/integration/common_tests.py

@@ -825,6 +825,41 @@ def test_type_at_pos_batch_optional(self) -> None:
             options=["--type-at-pos-batch", "{root}foo_optional.php:4:4"],
         )
 
+    def test_type_at_pos_batch_splices(self) -> None:
+        """
+        Test hh_client --type-at-pos-batch for ET splices
+        """
+        self.test_driver.start_hh_server()
+
+        self.test_driver.check_cmd(
+            [
+                '{{"position":'
+                + '{{"file":"{root}et_splices.php",'
+                + '"line":7,'
+                + '"character":46}}'
+                + ',"type":{{'
+                + '"src_pos":{{"filename":"{root}et_splices.php","line":74,"char_start":39,"char_end":48}},'
+                + '"kind":"class",'
+                + '"name":"\\\\ExampleInt",'
+                + '"args":[]}}'
+                + "}}",
+                '{{"position":'
+                + '{{"file":"{root}et_splices.php",'
+                + '"line":7,'
+                + '"character":40}}'
+                + ',"type":{{'
+                + '"src_pos":{{"filename":"{root}et_splices.php","line":7,"char_start":40,"char_end":40}},'
+                + '"kind":"primitive",'
+                + '"name":"int"}}'
+                + "}}",
+            ],
+            options=[
+                "--type-at-pos-batch",
+                "{root}et_splices.php:7:40",
+                "{root}et_splices.php:7:46",
+            ],
+        )
+
     def test_ide_get_definition(self) -> None:
         """
         Test hh_client --ide-get-definition

hphp/hack/test/integration/data/simple_repo/et_splices.php

@@ -0,0 +1,104 @@
+<?hh
+
+<<file: __EnableUnstableFeatures('expression_trees')>>
+
+function foo(): void {
+  $lift_one = $_ ==> ExampleDsl`1`;
+  $one_splice = ExampleDsl`${$lift_one(1)} + 1`;
+}
+
+// smaller subset of hphp/hack/test/expr_tree.php
+
+interface Spliceable<TVisitor, +TResult, +TInfer> {
+  public function visit(TVisitor $v): TResult;
+}
+
+type ExampleDslExpression<T> = Spliceable<ExampleDsl, ExampleDsl::TAst, T>;
+
+interface ExampleInt {
+  public function __plus(ExampleInt $_): ExampleInt;
+}
+
+type ExprTreeInfo<TInfer> = shape(
+  'splices' => dict<string, mixed>,
+  'functions' => vec<mixed>,
+  'static_methods' => vec<mixed>,
+  // The virtualised expression is placed here, to cause the type checker to instantiate
+  // TInfer to the virtualised type.
+  ?'type' => (function(): TInfer),
+);
+
+final class ExprTree<TVisitor, TResult, +TInfer>
+  implements Spliceable<TVisitor, TResult, TInfer> {
+  public function __construct(
+    private ?ExprPos $pos,
+    private ExprTreeInfo<TInfer> $metadata,
+    private (function(TVisitor): TResult) $ast,
+  )[] {}
+
+  public function visit(TVisitor $v): TResult {
+    return ($this->ast)($v);
+  }
+
+  public function getExprPos(): ?ExprPos {
+    return $this->pos;
+  }
+
+  public function getSplices(): dict<string, mixed> {
+    return $this->metadata['splices'];
+  }
+}
+
+// The DSL itself: used as in ExampleDsl`...`. hackc generates a call to makeTree, which
+// should return something that implements Spliceable, here an ExprTree
+class ExampleDsl {
+  const type TAst = string;
+
+  // The desugared expression tree literal will call this method.
+  public static function makeTree<TInfer>(
+    ?ExprPos $pos,
+    shape(
+      'splices' => dict<string, mixed>,
+      'functions' => vec<mixed>,
+      'static_methods' => vec<mixed>,
+      ?'type' => (function(): TInfer),
+    ) $metadata,
+    (function(ExampleDsl): ExampleDsl::TAst) $ast,
+  )[]: ExprTree<ExampleDsl, ExampleDsl::TAst, TInfer> {
+    return new ExprTree($pos, $metadata, $ast);
+  }
+
+  // Virtual types. These do not have to be implemented, as they are only used
+  // in the virtualized version of the expression tree, to work out the virtual type
+  // of literals during type checking.
+  public static function intType()[]: ExampleInt {
+    throw new Exception();
+  }
+
+  // Desugared nodes. Calls to these are emitted by hackc, following the structure
+  // of the expression in the expression tree. Here, they compute a string
+  // representation of that expression.
+  public function visitInt(?ExprPos $_, int $i)[]: ExampleDsl::TAst {
+    return (string)$i;
+  }
+
+  // Expressions
+  public function visitBinop(
+    ?ExprPos $_,
+    ExampleDsl::TAst $lhs,
+    string $op,
+    ExampleDsl::TAst $rhs,
+  )[]: ExampleDsl::TAst {
+    return "$lhs $op $rhs";
+  }
+
+  public function splice<T>(
+    ?ExprPos $_,
+    string $_key,
+    ExampleDslExpression<T> $splice_val,
+  ): ExampleDsl::TAst {
+    return "\${".($splice_val->visit($this))."}";
+  }
+}
+
+type ExprPos = shape(...);