Skip to content

Commit

Permalink
[flow] Scope analysis for declare global {...}
Browse files Browse the repository at this point in the history
Summary:
This diff adds scope analysis for local checking of `declare global {...}`.

Note that we don't have to do it for normal modules, since by the time we are checking normal modules, it will already have the extended globals injected into the context from type sig, so it doesn't have to read the globals from nested `declare global` block.

For `declare global` within `declare module`, we special case it during scope analysis. We first hoist all the globals within declare global within the declare module, create a scope with it, and then create a scope for everything else. This will implement the scoping rule as described in D68246824.

With this setup, we have the feature mostly working, and the only two issue remainings are globalThis interaction, and missing error for having value-decls within the declare global block.

Changelog: [internal]

Reviewed By: panagosg7

Differential Revision: D68851619

fbshipit-source-id: 1122ee2ec84fb0b6df6e7a3f47529ce3c2be6e50
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Feb 4, 2025
1 parent 70c5bf1 commit 2f97af0
Show file tree
Hide file tree
Showing 12 changed files with 335 additions and 20 deletions.
54 changes: 45 additions & 9 deletions src/analysis/env_builder/name_resolver.ml
Original file line number Diff line number Diff line change
Expand Up @@ -6311,18 +6311,54 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
let v = val_simplify def_loc (Val.SourceLevelBinding kind) (Some name) v in
v

method! declare_module _loc ({ Ast.Statement.DeclareModule.id = _; body; _ } as m) =
method! declare_module loc ({ Ast.Statement.DeclareModule.id = _; body; _ } as m) =
let (block_loc, { Ast.Statement.Block.body = statements; comments = _ }) = body in
let bindings =
let hoist =
new Hoister.hoister ~flowmin_compatibility:false ~enable_enums ~with_types:true
let within_possible_declare_globals ~f =
let declare_global_bodies =
Base.List.filter_map statements ~f:(function
| ( _,
Ast.Statement.DeclareNamespace
{
Ast.Statement.DeclareNamespace.id = Ast.Statement.DeclareNamespace.Global _;
body;
comments = _;
}
) ->
Some body
| _ -> None
)
in
hoist#eval hoist#statement_list statements
if Base.List.is_empty declare_global_bodies then
f ()
else
let hoist = new hoister ~flowmin_compatibility:false ~enable_enums ~with_types:true in
Base.List.iter declare_global_bodies ~f:(fun (loc, body) ->
Flow_ast_visitor.run (hoist#block loc) body
);
let global_bindings = hoist#acc in
this#with_bindings
loc
global_bindings
(fun () ->
Base.List.iter declare_global_bodies ~f:(fun (loc, body) ->
Flow_ast_visitor.run (this#block loc) body
);
f ())
();
()
in
let saved_exclude_syms = env_state.exclude_syms in
env_state <- { env_state with exclude_syms = SSet.empty };
ignore @@ this#statements_with_bindings block_loc bindings statements;
env_state <- { env_state with exclude_syms = saved_exclude_syms };
within_possible_declare_globals ~f:(fun () ->
let bindings =
let hoist =
new Hoister.hoister ~flowmin_compatibility:false ~enable_enums ~with_types:true
in
hoist#eval hoist#statement_list statements
in
let saved_exclude_syms = env_state.exclude_syms in
env_state <- { env_state with exclude_syms = SSet.empty };
ignore @@ this#statements_with_bindings block_loc bindings statements;
env_state <- { env_state with exclude_syms = saved_exclude_syms }
);
m

method! declare_namespace loc ({ Ast.Statement.DeclareNamespace.id; body; _ } as m) =
Expand Down
44 changes: 39 additions & 5 deletions src/analysis/scope_builder.ml
Original file line number Diff line number Diff line change
Expand Up @@ -601,12 +601,46 @@ module Make (L : Loc_sig.S) (Api : Scope_api_sig.S with module L = L) :
let open Ast.Statement.DeclareModule in
let { id = _; body; comments = _ } = m in
let (loc, body) = body in
let bindings =
let hoist = new hoister ~flowmin_compatibility ~enable_enums ~with_types in
run (hoist#block loc) body;
hoist#acc
let within_possible_declare_globals ~f =
let declare_global_bodies =
Base.List.filter_map body.Ast.Statement.Block.body ~f:(function
| ( _,
Ast.Statement.DeclareNamespace
{
Ast.Statement.DeclareNamespace.id = Ast.Statement.DeclareNamespace.Global _;
body;
comments = _;
}
) ->
Some body
| _ -> None
)
in
if Base.List.is_empty declare_global_bodies then
f ()
else
let hoist = new hoister ~flowmin_compatibility ~enable_enums ~with_types in
Base.List.iter declare_global_bodies ~f:(fun (loc, body) -> run (hoist#block loc) body);
let global_bindings = hoist#acc in
this#with_bindings
loc
global_bindings
(fun () ->
Base.List.iter declare_global_bodies ~f:(fun (loc, body) ->
run (this#block loc) body
);
f ())
();
()
in
this#with_bindings loc bindings (fun () -> run (this#block loc) body) ();
within_possible_declare_globals ~f:(fun () ->
let bindings =
let hoist = new hoister ~flowmin_compatibility ~enable_enums ~with_types in
run (hoist#block loc) body;
hoist#acc
in
this#with_bindings loc bindings (fun () -> run (this#block loc) body) ()
);
m

method! declare_namespace _loc n =
Expand Down
14 changes: 10 additions & 4 deletions src/typing/statement.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2106,10 +2106,16 @@ module Make
reason
body_statements
in
Flow.add_output
cx
(Error_message.EUnsupportedSyntax (name_loc, Flow_intermediate_error_types.DeclareGlobal)
);
let () =
match (Context.enable_declare_global cx, prev_scope_kind) with
| (true, (Name_def.DeclareModule | Name_def.Module)) -> ()
| (_, _) ->
Flow.add_output
cx
(Error_message.EUnsupportedSyntax
(name_loc, Flow_intermediate_error_types.DeclareGlobal)
)
in
Flow.add_output cx (Error_message.EUndocumentedFeature { loc = name_loc });
(t, Ast.Statement.DeclareNamespace.Global id)
| Ast.Statement.DeclareNamespace.Local id ->
Expand Down
4 changes: 2 additions & 2 deletions src/typing/type_inference_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -469,9 +469,9 @@ class lib_def_loc_mapper_and_validator cx =
| DeclareNamespace.Local _ -> None
| DeclareNamespace.Global _ ->
if in_toplevel_scope then
None
Some (error "declare global")
else
Some (error "declare global"))
None)
| Block _ -> Some (error "block")
| Break _ -> Some (error "break")
| ClassDeclaration _ -> Some (error "class declaration")
Expand Down
7 changes: 7 additions & 0 deletions tests/declare_global_in_react_declare_module/.flowconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[libs]
react.js

[options]
all=true
experimental.declare_global=true
no_flowlib=false
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
Error --------------------------------------------------------------------------------------------------- react.js:10:10

Cannot redeclare global `React.Node` [1] because the global is already declared in another file. [name-already-bound]

react.js:10:10
10| type React$Node = 1; // bad-shadow
^^^^^^^^^^

References:
<BUILTINS>/react.js:15:14
15| declare type React$Node =
^^^^^^^^^^ [1]


Error ------------------------------------------------------------------------------------------------------ test.js:2:1

Cannot cast `2` to `React.Node` because number [1] is incompatible with number literal `1` [2]. [incompatible-cast]

test.js:2:1
2| 2 as React$Node; // error: 2 ~> 1: The bad shadow wins over the real global.
^ [1]

References:
test.js:2:6
2| 2 as React$Node; // error: 2 ~> 1: The bad shadow wins over the real global.
^^^^^^^^^^ [2]


Error ------------------------------------------------------------------------------------------------------ test.js:3:1

Cannot cast empty string to `MyReact` because string [1] is incompatible with string literal `react` [2].
[incompatible-cast]

test.js:3:1
3| '' as ReactTypes.MyReact; // error: '' ~> 'react'
^^ [1]

References:
test.js:3:7
3| '' as ReactTypes.MyReact; // error: '' ~> 'react'
^^^^^^^^^^^^^^^^^^ [2]


Error ------------------------------------------------------------------------------------------------------ test.js:4:1

Cannot resolve name `ReactValue`. [cannot-resolve-name]

4| ReactValue; // error: value-namespaces in declare global are completely ignored
^^^^^^^^^^


Error ------------------------------------------------------------------------------------------------------ test.js:5:1

Cannot resolve name `willBeIgnored`. [cannot-resolve-name]

5| willBeIgnored; // error: values in declare global are completely ignored
^^^^^^^^^^^^^


Error ------------------------------------------------------------------------------------------------------ test.js:8:1

Cannot cast empty string to `T1` because string [1] is incompatible with string literal `react` [2]. [incompatible-cast]

test.js:8:1
8| '' as T1; // error: '' ~> 'react'
^^ [1]

References:
test.js:8:7
8| '' as T1; // error: '' ~> 'react'
^^ [2]


Error ------------------------------------------------------------------------------------------------------ test.js:9:1

Cannot cast empty string to `T2` because string [1] is incompatible with number literal `3` [2]. [incompatible-cast]

test.js:9:1
9| '' as T2 // error: '' ~> 3
^^ [1]

References:
test.js:9:7
9| '' as T2 // error: '' ~> 3
^^ [2]



Found 7 errors
17 changes: 17 additions & 0 deletions tests/declare_global_in_react_declare_module/react.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
declare module 'react' {
// $FlowExpectedError[undocumented-feature]
declare global {
declare namespace ReactTypes {
type MyReact = 'react';
}
declare namespace ReactValue { // TODO error: error on ignored value defs in declare global
declare const willBeIgnored: string;
}
type React$Node = 1; // bad-shadow
type ReactGlobalType = 3;
declare const willBeIgnored: string;
}

export type T1 = ReactTypes.MyReact;
export type T2 = ReactGlobalType;
}
9 changes: 9 additions & 0 deletions tests/declare_global_in_react_declare_module/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// $FlowExpectedError[internal-type]
2 as React$Node; // error: 2 ~> 1: The bad shadow wins over the real global.
'' as ReactTypes.MyReact; // error: '' ~> 'react'
ReactValue; // error: value-namespaces in declare global are completely ignored
willBeIgnored; // error: values in declare global are completely ignored

import type {T1, T2} from 'react'; // test that the normal exported types can reference names within declare global
'' as T1; // error: '' ~> 'react'
'' as T2 // error: '' ~> 3
4 changes: 4 additions & 0 deletions tests/declare_global_in_react_module/.flowconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[options]
all=true
experimental.declare_global=true
no_flowlib=false
15 changes: 15 additions & 0 deletions tests/declare_global_in_react_module/@flowtyped/react.js.flow
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// $FlowExpectedError[undocumented-feature]
declare global {
declare namespace ReactTypes {
type MyReact = 'react';
}
declare namespace ReactValue { // TODO error: error on ignored value defs in declare global
declare const willBeIgnored: string;
}
type React$Node = 1; // bad-shadow
type ReactGlobalType = 3;
declare const willBeIgnored: string;
}

export type T1 = ReactTypes.MyReact;
export type T2 = ReactGlobalType;
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
Error ------------------------------------------------------------------------------------- @flowtyped/react.js.flow:9:8

Cannot redeclare global `React.Node` [1] because the global is already declared in another file. [name-already-bound]

@flowtyped/react.js.flow:9:8
9| type React$Node = 1; // bad-shadow
^^^^^^^^^^

References:
<BUILTINS>/react.js:15:14
15| declare type React$Node =
^^^^^^^^^^ [1]


Error ------------------------------------------------------------------------------------------------------ test.js:2:1

Cannot cast `2` to `React.Node` because number [1] is incompatible with number literal `1` [2]. [incompatible-cast]

test.js:2:1
2| 2 as React$Node; // error: 2 ~> 1: The bad shadow wins over the real global.
^ [1]

References:
test.js:2:6
2| 2 as React$Node; // error: 2 ~> 1: The bad shadow wins over the real global.
^^^^^^^^^^ [2]


Error ------------------------------------------------------------------------------------------------------ test.js:3:1

Cannot cast empty string to `MyReact` because string [1] is incompatible with string literal `react` [2].
[incompatible-cast]

test.js:3:1
3| '' as ReactTypes.MyReact; // error: '' ~> 'react'
^^ [1]

References:
test.js:3:7
3| '' as ReactTypes.MyReact; // error: '' ~> 'react'
^^^^^^^^^^^^^^^^^^ [2]


Error ------------------------------------------------------------------------------------------------------ test.js:4:1

Cannot resolve name `ReactValue`. [cannot-resolve-name]

4| ReactValue; // error: value-namespaces in declare global are completely ignored
^^^^^^^^^^


Error ------------------------------------------------------------------------------------------------------ test.js:5:1

Cannot resolve name `willBeIgnored`. [cannot-resolve-name]

5| willBeIgnored; // error: values in declare global are completely ignored
^^^^^^^^^^^^^


Error ------------------------------------------------------------------------------------------------------ test.js:8:1

Cannot cast empty string to `T1` because string [1] is incompatible with string literal `react` [2]. [incompatible-cast]

test.js:8:1
8| '' as T1; // error: '' ~> 'react'
^^ [1]

References:
test.js:8:7
8| '' as T1; // error: '' ~> 'react'
^^ [2]


Error ------------------------------------------------------------------------------------------------------ test.js:9:1

Cannot cast empty string to `T2` because string [1] is incompatible with number literal `3` [2]. [incompatible-cast]

test.js:9:1
9| '' as T2 // error: '' ~> 3
^^ [1]

References:
test.js:9:7
9| '' as T2 // error: '' ~> 3
^^ [2]



Found 7 errors
9 changes: 9 additions & 0 deletions tests/declare_global_in_react_module/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// $FlowExpectedError[internal-type]
2 as React$Node; // error: 2 ~> 1: The bad shadow wins over the real global.
'' as ReactTypes.MyReact; // error: '' ~> 'react'
ReactValue; // error: value-namespaces in declare global are completely ignored
willBeIgnored; // error: values in declare global are completely ignored

import type {T1, T2} from 'react'; // test that the normal exported types can reference names within declare global
'' as T1; // error: '' ~> 'react'
'' as T2 // error: '' ~> 3

0 comments on commit 2f97af0

Please sign in to comment.