[flow] disallow current identifier in default value of parameter

Summary:
Error on definitions of the form:
```
function f(x: number = x) {}
```
where the default expression contains a reference to the parameter being defined.

The key idea of the fix is to keep track of the current pattern bindings introduced by a parameter when visiting the default expression (if one exists). If, when visiting the expression, we encounter one of the current identifiers we raise a new error.

Changelog: [fix] Flow will now error when a function parameter appears in the default expression for the same binding. (e.g. [try-Flow](https://flow.org/try/#1N4Igxg9gdgZglgcxALlAIwIZoKYBsD6uEEAztvhgE6UYCe+JADpdhgCYowa5kA0I2KAFcAtiRQAXSkOz9sADwxgJ+NPTbYuQ3BMnTZA+Y2yU4IwRO4A6SFBIrGVDGM7c+h46fNRLuKxJIGWh8MeT0ZfhYlCStpHzNsFBAMIQkIEQwJODAQfiEyfBE4eWw2fDgofDBMsAALfAA3KjgsXGxxZC4eAw0G-GhcWn9aY3wWZldu-g1mbGqJUoBaCRHEzrcDEgBrbAk62kXhXFxJ923d-cPRHEpTgyEoMDaqZdW7vKgoOfaSKgOKpqmDA+d4gB5fMA-P6LCCMLLQbiLOoYCqgh6-GDYRYIXYLSgkRZkCR4jpddwPfJLZjpOBkUEKTwJEJ+DAkMiUFSwkyZCC3dbdAC+-EgGiSMAeyjg0AABDAABTyZDS4QiG7SgC80vkAEppcABdKAPSG6UAdzgx2lJkovKV8mlLExLEe7WlElq2GljhoImlOAqCGlGieVFKAB1YBL4VBpQg5cAMAKlcDaBrpRhdfqjSbzZbrbb0w7NCZBJCSG6PV6nL7-VBA8HcKG2BHpbkQA0TCQpVAkg0AAxWABMAE4ACxWPsgAVAA))

Reviewed By: SamChou19815

Differential Revision: D89254628

fbshipit-source-id: efbeff0830b995956b233484a46df27b20c9c209

Author: panagosg7

src/analysis/env_builder/name_resolver.ml

@@ -711,6 +711,9 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
     (* Track parameter binding def_locs currently being processed, so that we can
        error when these appear in the corresponding annotation. *)
     current_bindings: string L.LMap.t;
+    (* Track when we're visiting a parameter default expression, so we can
+       produce the appropriate error message for self-references. *)
+    in_param_default: bool;
   }
 
   type pattern_write_kind =
@@ -1110,6 +1113,7 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
           jsx_base_name;
           pred_func_map = L.LMap.empty;
           current_bindings = L.LMap.empty;
+          in_param_default = false;
         }
 
       method jsx_base_name = env_state.jsx_base_name
@@ -2259,6 +2263,31 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
         | _ -> ());
         env_state <- { env_state with write_entries }
 
+      (* Override pattern_object_property to detect SELF-references in pattern-inline defaults.
+         For {a = a}, check if a's default references a itself (self-reference) - ERROR.
+         For {a, b = a}, b's default references a, not b - OK (not a self-reference).
+         For {a = b, b}, a's default references b - handled by reference-before-declaration.
+         We only check for self-references here; forward references are caught elsewhere. *)
+      method! pattern_object_property ?kind prop =
+        let open Ast.Pattern.Object.Property in
+        let (loc, { key; pattern; default; shorthand }) = prop in
+        ignore @@ this#pattern_object_property_key ?kind key;
+        ignore @@ this#pattern_object_property_pattern ?kind pattern;
+        Base.Option.iter default ~f:(fun default_expr ->
+            this#visit_default_with_pattern_bindings pattern default_expr
+        );
+        (loc, { key; pattern; default; shorthand })
+
+      method! function_param param =
+        let open Ast.Function.Param in
+        let (loc, { argument; default }) = param in
+        this#visit_function_or_component_param_pattern ~is_rest:false argument;
+        ignore @@ super#function_param_pattern argument;
+        Base.Option.iter default ~f:(fun default_expr ->
+            this#visit_default_with_pattern_bindings argument default_expr
+        );
+        (loc, { argument; default })
+
       (* 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
        * this read
@@ -2308,6 +2337,18 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
             env_state <- { env_state with current_bindings = old_val }
         )
 
+      method private with_in_param_default ~f =
+        let old_in_param_default = env_state.in_param_default in
+        env_state <- { env_state with in_param_default = true };
+        Exception.protect ~f ~finally:(fun () ->
+            env_state <- { env_state with in_param_default = old_in_param_default }
+        )
+
+      method private visit_default_with_pattern_bindings pattern default_expr =
+        this#with_current_pattern_bindings pattern ~f:(fun () ->
+            this#with_in_param_default ~f:(fun () -> ignore @@ this#expression default_expr)
+        )
+
       (* Override the object type constuctor to disable the EReferenceInAnnotation check
        * since this is a common and safe way to encode recursive object types. *)
       method! object_type loc ot =
@@ -2334,6 +2375,20 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
               add_output (Error_message.EReferenceInAnnotation (def_loc, name, loc))
         )
 
+      method private error_on_reference_to_currently_declared_id_in_default id =
+        let (loc, { Ast.Identifier.name; _ }) = id in
+        let { val_ref = _; def_loc; _ } = this#env_read name in
+        Base.Option.iter def_loc ~f:(fun def_loc ->
+            match L.LMap.find_opt def_loc env_state.current_bindings with
+            | None -> ()
+            | Some binding_name ->
+              (* Only add the error - do NOT modify binding state (val_ref, write_entries).
+                 For forward references (e.g., `b` in `{a = b, b}`), modifying the binding
+                 state would corrupt it and cause spurious "name-already-bound" errors
+                 when the actual binding is later processed. *)
+              add_output (Error_message.EReferenceInDefault (def_loc, binding_name, loc))
+        )
+
       method! type_identifier_reference id =
         this#error_on_reference_to_currently_declared_id id;
         super#type_identifier_reference id
@@ -2352,6 +2407,8 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
 
       method! identifier (ident : (ALoc.t, ALoc.t) Ast.Identifier.t) =
         let (loc, { Ast.Identifier.name = x; comments = _ }) = ident in
+        if env_state.in_param_default then
+          this#error_on_reference_to_currently_declared_id_in_default ident;
         this#any_identifier loc x;
         super#identifier ident
 
@@ -6513,17 +6570,27 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
         | _ -> ()
 
       method! component_param param =
-        let (_, { Ast.Statement.ComponentDeclaration.Param.name; _ }) = param in
+        let (loc, { Ast.Statement.ComponentDeclaration.Param.name; local; default; shorthand }) =
+          param
+        in
         begin
           match name with
           | Ast.Statement.ComponentDeclaration.Param.Identifier
-              (loc, { Ast.Identifier.name = "ref"; _ })
+              (ref_loc, { Ast.Identifier.name = "ref"; _ })
           | Ast.Statement.ComponentDeclaration.Param.StringLiteral
-              (loc, { Ast.StringLiteral.value = "ref"; _ }) ->
-            this#any_identifier loc "React"
+              (ref_loc, { Ast.StringLiteral.value = "ref"; _ }) ->
+            this#any_identifier ref_loc "React"
           | _ -> ()
         end;
-        super#component_param param
+        ignore @@ this#component_param_name name;
+        this#visit_function_or_component_param_pattern ~is_rest:false local;
+        this#with_current_pattern_bindings local ~f:(fun () ->
+            ignore @@ super#component_param_pattern local;
+            Base.Option.iter default ~f:(fun default_expr ->
+                this#with_in_param_default ~f:(fun () -> ignore @@ this#expression default_expr)
+            )
+        );
+        (loc, { Ast.Statement.ComponentDeclaration.Param.name; local; default; shorthand })
 
       method! jsx_element loc expr =
         let open Ast.JSX in

src/typing/debug_js.ml

@@ -1953,6 +1953,7 @@ let dump_error_message =
     | EDuplicateClassMember { loc; name; _ } ->
       spf "EDuplicateClassMember (%s) (%s)" (string_of_aloc loc) name
     | EReferenceInAnnotation _ -> "EReferenceInAnnotation"
+    | EReferenceInDefault _ -> "EReferenceInDefault"
     | EEmptyArrayNoProvider { loc } -> spf "EEmptyArrayNoProvider (%s)" (string_of_aloc loc)
     | EUnusedPromise { loc; _ } -> spf "EUnusedPromise (%s)" (string_of_aloc loc)
     | EReactIntrinsicOverlap _ -> "EReactIntrinsicOverlap (_, _, _)"

src/typing/errors/error_message.ml

@@ -621,6 +621,7 @@ and 'loc t' =
       annot_locs: 'loc Env_api.annot_loc list;
     }
   | EReferenceInAnnotation of ('loc * string * 'loc)
+  | EReferenceInDefault of ('loc * string * 'loc)
   | EDuplicateClassMember of {
       loc: 'loc;
       name: string;
@@ -1761,6 +1762,7 @@ let rec map_loc_of_error_message (f : 'a -> 'b) : 'a t' -> 'b t' =
         recursion = Base.List.map ~f recursion;
       }
   | EReferenceInAnnotation (bind_loc, name, loc) -> EReferenceInAnnotation (f bind_loc, name, f loc)
+  | EReferenceInDefault (bind_loc, name, loc) -> EReferenceInDefault (f bind_loc, name, f loc)
   | EDuplicateClassMember { loc; name; static; class_kind } ->
     EDuplicateClassMember { loc = f loc; name; static; class_kind }
   | EEmptyArrayNoProvider { loc } -> EEmptyArrayNoProvider { loc = f loc }
@@ -2243,6 +2245,7 @@ let util_use_op_of_msg nope util = function
   | EDefinitionCycle _
   | ERecursiveDefinition _
   | EReferenceInAnnotation _
+  | EReferenceInDefault _
   | EAnnotationInference _
   | ETrivialRecursiveDefinition _
   | EDuplicateClassMember _
@@ -2473,6 +2476,7 @@ let loc_of_msg : 'loc t' -> 'loc option = function
   | EInvalidMappedType { loc; _ }
   | ETSSyntax { loc; _ }
   | EReferenceInAnnotation (loc, _, _)
+  | EReferenceInDefault (_, _, loc)
   | EDuplicateComponentProp { spread = loc; _ }
   | ERefComponentProp { spread = loc; _ }
   | EKeySpreadProp { spread = loc; _ }
@@ -3661,6 +3665,8 @@ let friendly_message_of_msg = function
   | EDefinitionCycle dependencies -> Normal (MessageDefinitionCycle dependencies)
   | EReferenceInAnnotation (_, name, loc) ->
     Normal (MessageInvalidSelfReferencingTypeAnnotation { name; loc })
+  | EReferenceInDefault (def_loc, name, ref_loc) ->
+    Normal (MessageInvalidSelfReferencingDefault { name; def_loc; ref_loc })
   | EUnusedPromise { async = true; _ } -> Normal MessageUnusedPromiseInAsyncScope
   | EUnusedPromise { async = false; _ } -> Normal MessageUnusedPromiseInSyncScope
   | EReactIntrinsicOverlap { use; def; type_; mixed } ->
@@ -4111,6 +4117,7 @@ let error_code_of_message err : error_code option =
   | EDefinitionCycle _ -> Some DefinitionCycle
   | ERecursiveDefinition _ -> Some RecursiveDefinition
   | EReferenceInAnnotation _ -> Some RecursiveDefinition
+  | EReferenceInDefault _ -> Some RecursiveDefinition
   | EDuplicateClassMember _ -> Some DuplicateClassMember
   | EEmptyArrayNoProvider _ -> Some EmptyArrayNoAnnot
   | EBigIntRShift3 _ -> Some BigIntRShift3

src/typing/errors/flow_intermediate_error.ml

@@ -4076,6 +4076,14 @@ let to_printable_error :
         hardcoded_string_desc_ref "reference" loc;
         text " to the binding being declared.";
       ]
+    | MessageInvalidSelfReferencingDefault { name; def_loc; ref_loc } ->
+      [
+        text "Invalid default expression for parameter ";
+        hardcoded_string_desc_ref name def_loc;
+        text ". It contains a ";
+        hardcoded_string_desc_ref "reference" ref_loc;
+        text " to the binding being declared.";
+      ]
     | MessageInvalidTrivialRecursiveDefinition description ->
       [text "Invalid trivially recursive definition of "; desc description; text ". "]
     | MessageInvalidTupleRequiredAfterOptional { reason_tuple; reason_required; reason_optional } ->

src/typing/errors/flow_intermediate_error_types.ml

@@ -935,6 +935,11 @@ type 'loc message =
       name: string;
       loc: 'loc;
     }
+  | MessageInvalidSelfReferencingDefault of {
+      name: string;
+      def_loc: 'loc;
+      ref_loc: 'loc;
+    }
   | MessageInvalidTrivialRecursiveDefinition of 'loc virtual_reason_desc
   | MessageInvalidTupleRequiredAfterOptional of {
       reason_tuple: 'loc virtual_reason;

tests/binding/binding.exp

@@ -1416,6 +1416,171 @@ References:
                   ^^^ [1]
 
 
+Error ----------------------------------------------------------------------------------------------- rec_decls.js:31:26
+
+Invalid default expression for parameter x [1]. It contains a reference [2] to the binding being declared.
+[recursive-definition]
+
+   rec_decls.js:31:26
+   31|   function f(x: number = x) {} // error: x references the param being declared
+                                ^ [2]
+
+References:
+   rec_decls.js:31:14
+   31|   function f(x: number = x) {} // error: x references the param being declared
+                    ^ [1]
+
+
+Error ----------------------------------------------------------------------------------------------- rec_decls.js:32:33
+
+Cannot assign `a` to object type because number [1] is incompatible with object type [2]. [incompatible-type]
+
+   rec_decls.js:32:33
+   32|   function g({a}: {a: number} = a) {} // error: a references the param being declared
+                                       ^
+
+References:
+   rec_decls.js:32:23
+   32|   function g({a}: {a: number} = a) {} // error: a references the param being declared
+                             ^^^^^^ [1]
+   rec_decls.js:32:19
+   32|   function g({a}: {a: number} = a) {} // error: a references the param being declared
+                         ^^^^^^^^^^^ [2]
+
+
+Error ----------------------------------------------------------------------------------------------- rec_decls.js:32:33
+
+Invalid default expression for parameter a [1]. It contains a reference [2] to the binding being declared.
+[recursive-definition]
+
+   rec_decls.js:32:33
+   32|   function g({a}: {a: number} = a) {} // error: a references the param being declared
+                                       ^ [2]
+
+References:
+   rec_decls.js:32:15
+   32|   function g({a}: {a: number} = a) {} // error: a references the param being declared
+                     ^ [1]
+
+
+Error ----------------------------------------------------------------------------------------------- rec_decls.js:33:26
+
+Invalid default expression for parameter y [1]. It contains a reference [2] to the binding being declared.
+[recursive-definition]
+
+   rec_decls.js:33:26
+   33|   function h(y: number = y + 1) {} // error: y references the param being declared
+                                ^ [2]
+
+References:
+   rec_decls.js:33:14
+   33|   function h(y: number = y + 1) {} // error: y references the param being declared
+                    ^ [1]
+
+
+Error ----------------------------------------------------------------------------------------------- rec_decls.js:34:26
+
+Invalid default expression for parameter x [1]. It contains a reference [2] to the binding being declared.
+[recursive-definition]
+
+   rec_decls.js:34:26
+   34|   function i(x: number = x + x) {} // error: x references the param being declared (twice)
+                                ^ [2]
+
+References:
+   rec_decls.js:34:14
+   34|   function i(x: number = x + x) {} // error: x references the param being declared (twice)
+                    ^ [1]
+
+
+Error ----------------------------------------------------------------------------------------------- rec_decls.js:34:30
+
+Invalid default expression for parameter x [1]. It contains a reference [2] to the binding being declared.
+[recursive-definition]
+
+   rec_decls.js:34:30
+   34|   function i(x: number = x + x) {} // error: x references the param being declared (twice)
+                                    ^ [2]
+
+References:
+   rec_decls.js:34:14
+   34|   function i(x: number = x + x) {} // error: x references the param being declared (twice)
+                    ^ [1]
+
+
+Error ----------------------------------------------------------------------------------------------- rec_decls.js:40:40
+
+Cannot use variable `b` [1] because the declaration either comes later or was skipped. [reference-before-declaration]
+
+   rec_decls.js:40:40
+   40|   function default_expr_scope_err({a = b, b}: {a?: string, b: string}) {} // error: a's default references b, which comes after
+                                              ^
+
+References:
+   rec_decls.js:40:43
+   40|   function default_expr_scope_err({a = b, b}: {a?: string, b: string}) {} // error: a's default references b, which comes after
+                                                 ^ [1]
+
+
+Error ----------------------------------------------------------------------------------------------- rec_decls.js:44:20
+
+Invalid default expression for parameter c [1]. It contains a reference [2] to the binding being declared.
+[recursive-definition]
+
+   rec_decls.js:44:20
+   44|     {a, b = a, c = c}: {a: string, b?: string, c?: string}, // b = a OK (earlier), c = c ERROR (self)
+                          ^ [2]
+
+References:
+   rec_decls.js:44:16
+   44|     {a, b = a, c = c}: {a: string, b?: string, c?: string}, // b = a OK (earlier), c = c ERROR (self)
+                      ^ [1]
+
+
+Error ----------------------------------------------------------------------------------------------- rec_decls.js:45:24
+
+Cannot use variable `g` [1] because the declaration either comes later or was skipped. [reference-before-declaration]
+
+   rec_decls.js:45:24
+   45|     {d = a, e = d, f = g, g}: {d?: string, e?: string, f?: string, g: string}, // d = a OK (earlier param), e = d OK (earlier in same pattern), f = g ERROR (forward)
+                              ^
+
+References:
+   rec_decls.js:45:27
+   45|     {d = a, e = d, f = g, g}: {d?: string, e?: string, f?: string, g: string}, // d = a OK (earlier param), e = d OK (earlier in same pattern), f = g ERROR (forward)
+                                 ^ [1]
+
+
+Error ----------------------------------------------------------------------------------------------- rec_decls.js:58:19
+
+Invalid default expression for parameter z [1]. It contains a reference [2] to the binding being declared.
+[recursive-definition]
+
+   rec_decls.js:58:19
+   58| function Foo({z = z ?? null}: O) { // error: z references the param being declared
+                         ^ [2]
+
+References:
+   rec_decls.js:58:15
+   58| function Foo({z = z ?? null}: O) { // error: z references the param being declared
+                     ^ [1]
+
+
+Error ----------------------------------------------------------------------------------------------- rec_decls.js:62:28
+
+Invalid default expression for parameter scope [1]. It contains a reference [2] to the binding being declared.
+[recursive-definition]
+
+   rec_decls.js:62:28
+   62| component Comp(...{scope = scope ?? null}: {scope?: ?string}) { // error: scope references the param being declared
+                                  ^^^^^ [2]
+
+References:
+   rec_decls.js:62:20
+   62| component Comp(...{scope = scope ?? null}: {scope?: ?string}) { // error: scope references the param being declared
+                          ^^^^^ [1]
+
+
 Error ----------------------------------------------------------------------------------------------- rec_params.js:7:26
 
 Cannot resolve name `x`. [cannot-resolve-name]
@@ -2161,4 +2326,4 @@ Cannot resolve name `unboundFunction2`. [cannot-resolve-name]
 
 
 
-Found 164 errors
+Found 175 errors