[flow][tslib] Fix LocEnvEntryNotFound crash for destructuring in function type params

Summary:
D97435945 added parser support for destructuring patterns in arrow function
type params (e.g. `type A = ({ a, b }: T) => R`). The env builder's
`function_param_type_pattern` override was skipping the entire pattern,
which correctly avoided creating runtime bindings for the destructured
identifiers but also skipped visiting the type annotation. This caused
type references like `T` to never be registered in `env_values`, leading
to `LocEnvEntryNotFound` crashes during type checking.

Fix: visit the type annotation hint on destructuring patterns in function
type params while still skipping the binding identifiers.

Changelog: [internal]

Reviewed By: panagosg7

Differential Revision: D98849947

fbshipit-source-id: 83541ef14ae803128659c60725d1c744b8e4a97d

Author: gkz

rust_port/crates/flow_env_builder_resolver/src/name_def_ordering.rs

@@ -1101,10 +1101,19 @@ where
 
     // Skip destructuring patterns in function type params (e.g. in
     // `declare function f({a}: T): R`), as they are not runtime bindings.
+    // But still visit the type annotation so type references get registered.
     fn function_param_type_pattern(
         &mut self,
-        _patt: &ast::pattern::Pattern<ALoc, ALoc>,
+        patt: &ast::pattern::Pattern<ALoc, ALoc>,
     ) -> Result<(), !> {
+        use ast::pattern::Pattern;
+        let annot = match patt {
+            Pattern::Object { inner, .. } => &inner.annot,
+            Pattern::Array { inner, .. } => &inner.annot,
+            Pattern::Identifier { inner, .. } => &inner.annot,
+            Pattern::Expression { .. } => return Ok(()),
+        };
+        let Ok(()) = self.type_annotation_hint(annot);
         Ok(())
     }
 

rust_port/crates/flow_env_builder_resolver/src/name_resolver.rs

@@ -10342,10 +10342,19 @@ impl<'ast, 'a, Cx: Context, Fl: Flow<Cx = Cx>>
 
     // Skip destructuring patterns in function type params (e.g. in
     // `declare function f({a}: T): R`), as they are not runtime bindings.
+    // But still visit the type annotation so type references get registered.
     fn function_param_type_pattern(
         &mut self,
-        _patt: &'ast flow_parser::ast::pattern::Pattern<ALoc, ALoc>,
+        patt: &'ast flow_parser::ast::pattern::Pattern<ALoc, ALoc>,
     ) -> Result<(), AbruptCompletion> {
+        use flow_parser::ast::pattern::Pattern;
+        let annot = match patt {
+            Pattern::Object { inner, .. } => &inner.annot,
+            Pattern::Array { inner, .. } => &inner.annot,
+            Pattern::Identifier { inner, .. } => &inner.annot,
+            Pattern::Expression { .. } => return Ok(()),
+        };
+        self.type_annotation_hint(annot)?;
         Ok(())
     }
 }
@@ -11014,10 +11023,19 @@ impl<'ast, 'a, Cx: Context> AstVisitor<'ast, ALoc, ALoc, &'ast ALoc, !> for Dead
 
     // Skip destructuring patterns in function type params (e.g. in
     // `declare function f({a}: T): R`), as they are not runtime bindings.
+    // But still visit the type annotation so type references get registered.
     fn function_param_type_pattern(
         &mut self,
-        _patt: &'ast flow_parser::ast::pattern::Pattern<ALoc, ALoc>,
+        patt: &'ast flow_parser::ast::pattern::Pattern<ALoc, ALoc>,
     ) -> Result<(), !> {
+        use flow_parser::ast::pattern::Pattern;
+        let annot = match patt {
+            Pattern::Object { inner, .. } => &inner.annot,
+            Pattern::Array { inner, .. } => &inner.annot,
+            Pattern::Identifier { inner, .. } => &inner.annot,
+            Pattern::Expression { .. } => return Ok(()),
+        };
+        let Ok(()) = self.type_annotation_hint(annot);
         Ok(())
     }
 }

src/analysis/env_builder/name_def_ordering.ml

@@ -399,8 +399,18 @@ struct
         method! function_param_type_identifier (id : ('loc, 'loc) Ast.Identifier.t) = id
 
         (* Skip destructuring patterns in function type params (e.g. in
-           `declare function f({a}: T): R`), as they are not runtime bindings. *)
-        method! function_param_type_pattern (patt : ('loc, 'loc) Ast.Pattern.t) = patt
+           `declare function f({a}: T): R`), as they are not runtime bindings.
+           But still visit the type annotation so type references get registered. *)
+        method! function_param_type_pattern (patt : ('loc, 'loc) Ast.Pattern.t) =
+          let open Ast.Pattern in
+          let (_annot, p) = patt in
+          (match p with
+          | Object { Object.annot; _ }
+          | Array { Array.annot; _ }
+          | Identifier { Identifier.annot; _ } ->
+            ignore (this#type_annotation_hint annot)
+          | Expression _ -> ());
+          patt
 
         method! member_property_identifier (id : (ALoc.t, ALoc.t) Ast.Identifier.t) = id
 

src/analysis/env_builder/name_resolver.ml

@@ -2211,8 +2211,18 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
           (loc, ParamProperty prop)
 
       (* Skip destructuring patterns in function type params (e.g. in
-         `declare function f({a}: T): R`), as they are not runtime bindings. *)
-      method! function_param_type_pattern (patt : (ALoc.t, ALoc.t) Ast.Pattern.t) = patt
+         `declare function f({a}: T): R`), as they are not runtime bindings.
+         But still visit the type annotation so type references get registered. *)
+      method! function_param_type_pattern (patt : (ALoc.t, ALoc.t) Ast.Pattern.t) =
+        let open Ast.Pattern in
+        let (_annot, p) = patt in
+        (match p with
+        | Object { Object.annot; _ }
+        | Array { Array.annot; _ }
+        | Identifier { Identifier.annot; _ } ->
+          ignore (this#type_annotation_hint annot)
+        | Expression _ -> ());
+        patt
 
       (* This method is called during every read of an identifier. We need to ensure that
        * if the identifier is refined that we record the refiner as the write that reaches
@@ -6956,8 +6966,18 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
         | _ -> super#function_param param
 
       (* Skip destructuring patterns in function type params (e.g. in
-         `declare function f({a}: T): R`), as they are not runtime bindings. *)
-      method! function_param_type_pattern (patt : (ALoc.t, ALoc.t) Ast.Pattern.t) = patt
+         `declare function f({a}: T): R`), as they are not runtime bindings.
+         But still visit the type annotation so type references get registered. *)
+      method! function_param_type_pattern (patt : (ALoc.t, ALoc.t) Ast.Pattern.t) =
+        let open Ast.Pattern in
+        let (_annot, p) = patt in
+        (match p with
+        | Object { Object.annot; _ }
+        | Array { Array.annot; _ }
+        | Identifier { Identifier.annot; _ } ->
+          ignore (this#type_annotation_hint annot)
+        | Expression _ -> ());
+        patt
 
       method! import_named_specifier ~import_kind specifier =
         import_named_specifier

tests/tslib_syntax/destructuring_function_type.d.ts

@@ -29,3 +29,9 @@ export declare function restArr(...[a, b]: [number, string]): number;
 export type ArrowDestructObj = ({a, b}: {a: number, b: string}) => number;
 export type ArrowDestructArr = ([a, b]: [number, string]) => number;
 export type ArrowDestructRename = ({x: x1}: {x: number}) => number;
+// Arrow function types with destructuring using named type references
+export type ObjParams = {a: number, b: string};
+export type TupleParams = [number, string];
+export type ArrowDestructNamedObj = ({a, b}: ObjParams) => number;
+export type ArrowDestructNamedArr = ([a, b]: TupleParams) => number;
+export declare const arrowDestructNamedConst: ({a, b}: ObjParams) => number;

tests/tslib_syntax/destructuring_function_type_usage.js

@@ -1,5 +1,5 @@
-import {f, g, h, nested, rest, optObj, optArr, C, restArr} from "./destructuring_function_type";
-import type {I, ArrowDestructObj, ArrowDestructArr, ArrowDestructRename} from "./destructuring_function_type";
+import {f, g, h, nested, rest, optObj, optArr, C, restArr, arrowDestructNamedConst} from "./destructuring_function_type";
+import type {I, ArrowDestructObj, ArrowDestructArr, ArrowDestructRename, ArrowDestructNamedObj, ArrowDestructNamedArr} from "./destructuring_function_type";
 
 // Object destructuring with shorthand properties
 f({a: 1, b: "hello"}) as number; // OK
@@ -64,3 +64,15 @@ arrowArr([1, "hello"]) as string; // ERROR
 declare const arrowRename: ArrowDestructRename;
 arrowRename({x: 1}) as number; // OK
 arrowRename({x: 1}) as string; // ERROR
+
+// Arrow function types with destructuring using named type references
+declare const arrowNamedObj: ArrowDestructNamedObj;
+arrowNamedObj({a: 1, b: "hello"}) as number; // OK
+arrowNamedObj({a: 1, b: "hello"}) as string; // ERROR
+
+declare const arrowNamedArr: ArrowDestructNamedArr;
+arrowNamedArr([1, "hello"]) as number; // OK
+arrowNamedArr([1, "hello"]) as string; // ERROR
+
+arrowDestructNamedConst({a: 1, b: "hello"}) as number; // OK
+arrowDestructNamedConst({a: 1, b: "hello"}) as string; // ERROR

tests/tslib_syntax/tslib_syntax.exp

@@ -701,6 +701,58 @@ References:
                               ^^^^^^ [2]
 
 
+Error ------------------------------------------------------------------------ destructuring_function_type_usage.js:71:1
+
+Cannot cast `arrowNamedObj(...)` to string because number [1] is incompatible with string [2]. [incompatible-type]
+
+   destructuring_function_type_usage.js:71:1
+   71| arrowNamedObj({a: 1, b: "hello"}) as string; // ERROR
+       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+References:
+   destructuring_function_type.d.ts:35:60
+   35| export type ArrowDestructNamedObj = ({a, b}: ObjParams) => number;
+                                                                  ^^^^^^ [1]
+   destructuring_function_type_usage.js:71:38
+   71| arrowNamedObj({a: 1, b: "hello"}) as string; // ERROR
+                                            ^^^^^^ [2]
+
+
+Error ------------------------------------------------------------------------ destructuring_function_type_usage.js:75:1
+
+Cannot cast `arrowNamedArr(...)` to string because number [1] is incompatible with string [2]. [incompatible-type]
+
+   destructuring_function_type_usage.js:75:1
+   75| arrowNamedArr([1, "hello"]) as string; // ERROR
+       ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+References:
+   destructuring_function_type.d.ts:36:62
+   36| export type ArrowDestructNamedArr = ([a, b]: TupleParams) => number;
+                                                                    ^^^^^^ [1]
+   destructuring_function_type_usage.js:75:32
+   75| arrowNamedArr([1, "hello"]) as string; // ERROR
+                                      ^^^^^^ [2]
+
+
+Error ------------------------------------------------------------------------ destructuring_function_type_usage.js:78:1
+
+Cannot cast `arrowDestructNamedConst(...)` to string because number [1] is incompatible with string [2].
+[incompatible-type]
+
+   destructuring_function_type_usage.js:78:1
+   78| arrowDestructNamedConst({a: 1, b: "hello"}) as string; // ERROR
+       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+References:
+   destructuring_function_type.d.ts:37:70
+   37| export declare const arrowDestructNamedConst: ({a, b}: ObjParams) => number;
+                                                                            ^^^^^^ [1]
+   destructuring_function_type_usage.js:78:48
+   78| arrowDestructNamedConst({a: 1, b: "hello"}) as string; // ERROR
+                                                      ^^^^^^ [2]
+
+
 Error --------------------------------------------------------------------------------------- export_as_namespace.js:1:1
 
 `export as namespace` is not enabled. [unsupported-syntax]
@@ -1378,4 +1430,4 @@ References:
 
 
 
-Found 91 errors
+Found 94 errors