Skip to content

Commit

Permalink
[flow] Declaration merging for declare namespaces in libdefs
Browse files Browse the repository at this point in the history
Summary:
In this diff, we implement basic declaration merging for `declare namespace` in libdefs. I specifically chose to limit to libdef for now due to its relative self-contained-ness: with some addition work to propagate some additional kinds of signature errors in type sig, we can do the enforcement completely within type sig.

Within type sig, we have a specialized implementation of namespace binding. When binding the namespace, it will see whether there is already a namespace in the global libdef scope. If so, we will try to merge it (there are a lot of case analysis, including the most complicated one of promoting a type-only namespace to a value namespace due to merging, which I explained in the comments and checked in tests).

When we are merging together two namespaces, we will error on duplicate names, e.g.

```
declare namespace ns {
  declare const foo: string;
}
declare namespace ns {
  declare const foo: string; // error
}
```

This error will be added through a new signature error called validation error, which is not a result of unsupported expressions being any, but a result of simple syntactic validation. We can do the validation while we are merging, so it's very convenient.

Note that we currently don't error on overridden libdefs, even those within the same file, but we can use similar mechanism to achieve that. (We do need additional tracking, since erroring on the shadowed one will be too impractical right now, as it can cause the shadowed builtin to error, which we can't suppress)

A similar approach can be taken to support declaration merging for `declare module` as well (although with even more annoying case analysis for different kinds of modules (cjs/esm)). However, this diff won't do it, and it will be left as an exercise for an interested reader.

Changelog: [feature] Declaration merging for `declare namespace` is now supported in toplevel library definitions.

Reviewed By: panagosg7

Differential Revision: D70303553

fbshipit-source-id: b0c7b74813b0913cb273931deec3f591611d64d7
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Feb 28, 2025
1 parent 7944ce6 commit 632778a
Show file tree
Hide file tree
Showing 21 changed files with 468 additions and 12 deletions.
1 change: 1 addition & 0 deletions src/codemods/annotate_exports.ml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ module SignatureVerification = struct
(fun acc err ->
match err with
| Type_sig.CheckError -> acc
| Type_sig.BindingValidationError _ -> acc
| Type_sig.SigError err ->
let open Signature_error in
let (tot_errors, acc) = acc in
Expand Down
5 changes: 4 additions & 1 deletion src/parser_utils/file_sig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ and require_bindings =
| BindNamed of (Loc.t Ast_utils.ident * require_bindings) list
[@@deriving show]

type tolerable_error = SignatureVerificationError of Loc.t Signature_error.t [@@deriving show]
type tolerable_error =
| SignatureVerificationError of Loc.t Signature_error.t
| SignatureBindingValidationError of Loc.t Signature_error.binding_validation_t
[@@deriving show]

type tolerable_t = t * tolerable_error list

Expand Down
5 changes: 4 additions & 1 deletion src/parser_utils/file_sig.mli
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,10 @@ and require_bindings =
| BindNamed of (Loc.t Flow_ast_utils.ident * require_bindings) list
[@@deriving show]

type tolerable_error = SignatureVerificationError of Loc.t Signature_error.t [@@deriving show]
type tolerable_error =
| SignatureVerificationError of Loc.t Signature_error.t
| SignatureBindingValidationError of Loc.t Signature_error.binding_validation_t
[@@deriving show]

type tolerable_t = t * tolerable_error list

Expand Down
2 changes: 2 additions & 0 deletions src/parser_utils/signature_builder/signature_error.ml
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ type 'loc t =
| UnexpectedExpression of 'loc * Flow_ast_utils.ExpressionSort.t
[@@deriving show, iter, map]

type 'loc binding_validation_t = NameAlreadyBound of 'loc [@@deriving show, iter, map]

let compare = Stdlib.compare
102 changes: 102 additions & 0 deletions src/parser_utils/type_sig/__tests__/type_sig_tests.ml
Original file line number Diff line number Diff line change
Expand Up @@ -5245,6 +5245,108 @@ let%expect_test "builtin_declare_namespace" =
Builtin global value globalThis
Builtin global value ns |}]

let%expect_test "declare_namespace_declaration_merging" =
print_builtins [{|
declare namespace ns_v {
declare const a: string;
}
declare namespace ns_v {
declare const b: string;
}
declare namespace ns_t {
type T1 = string;
}
declare namespace ns_t {
type T2 = string;
}
declare namespace ns_v_and_then_t {
declare const a: string;
}
declare namespace ns_t_and_then_v {
type T1 = string;
}
declare namespace ns_v_and_then_t {
type T1 = string;
}
declare namespace ns_t_and_then_v {
declare const a: string;
}

declare const non_ns_value: string;
type non_ns_type = string;
// The following namespaces won't have any effect
declare namespace non_ns_value {
declare const b: string;
}
declare namespace non_ns_value {
type T1 = string;
}
declare namespace non_ns_type {
declare const b: string;
}
declare namespace non_ns_type {
type T1 = string;
}
|}];
[%expect {|
Local defs:
0. Variable {id_loc = [2:16-17]; name = "a"; def = (Annot (String [2:19-25]))}
1. NamespaceBinding {id_loc = [1:18-22];
name = "ns_v";
values =
{ "a" -> ([2:16-17], (Ref LocalRef {ref_loc = [2:16-17]; index = 0}));
"b" -> ([5:16-17], (Ref LocalRef {ref_loc = [5:16-17]; index = 2})) };
types = {}}
2. Variable {id_loc = [5:16-17]; name = "b"; def = (Annot (String [5:19-25]))}
3. TypeAlias {id_loc = [8:7-9]; name = "T1"; tparams = Mono; body = (Annot (String [8:12-18]))}
4. NamespaceBinding {id_loc = [7:18-22];
name = "ns_t"; values = {};
types =
{ "T1" -> ([8:7-9], (Ref LocalRef {ref_loc = [8:7-9]; index = 3}));
"T2" -> ([11:7-9], (Ref LocalRef {ref_loc = [11:7-9]; index = 5})) }}
5. TypeAlias {id_loc = [11:7-9];
name = "T2"; tparams = Mono;
body = (Annot (String [11:12-18]))}
6. Variable {id_loc = [14:16-17]; name = "a"; def = (Annot (String [14:19-25]))}
7. NamespaceBinding {id_loc = [13:18-33];
name = "ns_v_and_then_t";
values = { "a" -> ([14:16-17], (Ref LocalRef {ref_loc = [14:16-17]; index = 6})) };
types = { "T1" -> ([20:7-9], (Ref LocalRef {ref_loc = [20:7-9]; index = 10})) }}
8. TypeAlias {id_loc = [17:7-9];
name = "T1"; tparams = Mono;
body = (Annot (String [17:12-18]))}
9. NamespaceBinding {id_loc = [16:18-33];
name = "ns_t_and_then_v";
values = { "a" -> ([23:16-17], (Ref LocalRef {ref_loc = [23:16-17]; index = 11})) };
types = { "T1" -> ([17:7-9], (Ref LocalRef {ref_loc = [17:7-9]; index = 8})) }}
10. TypeAlias {id_loc = [20:7-9];
name = "T1"; tparams = Mono;
body = (Annot (String [20:12-18]))}
11. Variable {id_loc = [23:16-17]; name = "a"; def = (Annot (String [23:19-25]))}
12. Variable {id_loc = [25:14-26]; name = "non_ns_value"; def = (Annot (String [25:28-34]))}
13. TypeAlias {id_loc = [26:5-16];
name = "non_ns_type"; tparams = Mono;
body = (Annot (String [26:19-25]))}
14. NamespaceBinding {id_loc = [0:0];
name = "globalThis";
values =
{ "globalThis" -> ([0:0], (Ref LocalRef {ref_loc = [0:0]; index = 14}));
"non_ns_value" -> ([25:14-26], (Ref LocalRef {ref_loc = [25:14-26]; index = 12}));
"ns_t_and_then_v" -> ([16:18-33], (Ref LocalRef {ref_loc = [16:18-33]; index = 9}));
"ns_v" -> ([1:18-22], (Ref LocalRef {ref_loc = [1:18-22]; index = 1}));
"ns_v_and_then_t" -> ([13:18-33], (Ref LocalRef {ref_loc = [13:18-33]; index = 7})) };
types =
{ "non_ns_type" -> ([26:5-16], (Ref LocalRef {ref_loc = [26:5-16]; index = 13}));
"ns_t" -> ([7:18-22], (Ref LocalRef {ref_loc = [7:18-22]; index = 4})) }}

Builtin global value globalThis
Builtin global value non_ns_value
Builtin global value ns_t_and_then_v
Builtin global value ns_v
Builtin global value ns_v_and_then_t
Builtin global type non_ns_type
Builtin global type ns_t |}]

let%expect_test "builtin_pattern" =
print_builtins [{|
const o = { p: 0 };
Expand Down
1 change: 1 addition & 0 deletions src/parser_utils/type_sig/type_sig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,7 @@ type 'a op =
*)
type 'loc errno =
| CheckError
| BindingValidationError of 'loc Signature_error.binding_validation_t
| SigError of 'loc Signature_error.t
[@@deriving show { with_path = false }, map]

Expand Down
2 changes: 2 additions & 0 deletions src/parser_utils/type_sig/type_sig_mark.ml
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,8 @@ let mark_exports
SMap.iter (fun _ t -> mark_export ~locs_to_dirtify t) names;
List.iter mark_star stars

let mark_errors = List.iter (Signature_error.iter_binding_validation_t (mark_loc ~visit_loc:ignore))

let mark_builtin_module (loc, exports) =
mark_loc ~visit_loc:ignore loc;
mark_exports ~locs_to_dirtify:[] loc exports
3 changes: 3 additions & 0 deletions src/parser_utils/type_sig/type_sig_mark.mli
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,8 @@ val mark_exports :
Loc.t Type_sig_parse.exports ->
unit

val mark_errors :
Loc.t Type_sig_collections.Locs.node Signature_error.binding_validation_t list -> unit

val mark_builtin_module :
Loc.t Type_sig_collections.Locs.node * Loc.t Type_sig_parse.exports -> unit
2 changes: 1 addition & 1 deletion src/parser_utils/type_sig/type_sig_pack.ml
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ type 'loc pattern =

type 'loc cx = { mutable errs: 'loc errno list }

let create_cx () = { errs = [] }
let create_cx errs = { errs }

let pack_smap f map =
let bindings = Array.of_list (SMap.bindings map) in
Expand Down
89 changes: 87 additions & 2 deletions src/parser_utils/type_sig/type_sig_parse.ml
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ and 'loc tables = {
remote_refs: 'loc remote_binding Remote_refs.builder;
pattern_defs: 'loc parsed Pattern_defs.builder;
patterns: 'loc pattern Patterns.builder;
mutable additional_errors: 'loc loc_node Signature_error.binding_validation_t list;
}

type frozen_kind =
Expand All @@ -344,6 +345,7 @@ let create_tables () =
remote_refs = Remote_refs.create ();
pattern_defs = Pattern_defs.create ();
patterns = Patterns.create ();
additional_errors = [];
}

let push_loc tbls = Locs.push tbls.locs
Expand Down Expand Up @@ -1092,6 +1094,88 @@ module Scope = struct
(* a `declare namespace` exports every binding. *)
let finalize_declare_namespace_exn ~is_type_only scope tbls id_loc name =
match scope with
(* Special declaration merging rules for namespaces in the global scope *)
| DeclareNamespace { values; types; parent = Global global_scope; _ } ->
let (values, types) = namespace_binding_of_values_and_types scope (values, types) in
let def : _ local_binding = NamespaceBinding { id_loc; name; values; types } in
let union_values_and_types existing_values existing_types values types =
let new_binding name (l, _) =
if SMap.mem name existing_values || SMap.mem name existing_types then (
tbls.additional_errors <- Signature_error.NameAlreadyBound l :: tbls.additional_errors;
false
) else
true
in
let values = SMap.filter new_binding values in
let types = SMap.filter new_binding types in
(SMap.union existing_values values, SMap.union existing_types types)
in
(* When we decide that the namespace binding is mergeable. We call this function to
* mutate the existing node with merged names. *)
let merge_with_existing_local_binding_node (node : _ local_binding Local_defs.node) =
Local_defs.modify node (function
| NamespaceBinding { id_loc; name; values = existing_values; types = existing_types } ->
let (values, types) =
union_values_and_types existing_values existing_types values types
in
NamespaceBinding { id_loc; name; values; types }
| b -> b
)
in
(* This updater handle the simple case when we can directly update one kind of binding map
* (either value or type-only bindings). *)
let simple_updater = function
| Some (LocalBinding node) as existing_binding ->
merge_with_existing_local_binding_node node;
existing_binding
| Some (RemoteBinding _) as existing_binding -> existing_binding
| None ->
let node = push_local_def tbls def in
Some (LocalBinding node)
in
let updater values_map types_map =
match (is_type_only, SMap.find_opt name values_map, SMap.find_opt name types_map) with
| (_, Some _, Some _) ->
failwith "Invariant violation: a name cannot be in both values and types"
(* Simple case: no existing binding exist *)
| (false, None, None) -> (SMap.update name simple_updater values_map, types_map)
| (true, None, None) -> (values_map, SMap.update name simple_updater types_map)
(* Simple case: existing binding exist, but in the same value/type category *)
| (false, Some _, None) -> (SMap.update name simple_updater values_map, types_map)
| (true, None, Some _) -> (values_map, SMap.update name simple_updater types_map)
(* Originally not-type-only, now merging with a type-only namespace,
* so it's still not type-only *)
| (true, Some _, None) -> (SMap.update name simple_updater values_map, types_map)
(* Originally type-only, but now it's merging with a non-type-only namespace.
* The namespace binding needs to be promoted to a non-type-only namespace.
* (implemented by removing the node from type-only map, add to the value map, and
* mutate the binding node in place with additional names.)
*
* e.g. Consider
* ```
* declare namespace ns { type A = '' };
* declare namespace ns { declare const b: string };
* ```
*
* In order for the behavior to be equivalent to
* ```
* declare namespace ns { type A = ''; declare const b: string };
* ```
* We have to promote the original type-only namespace to a value binding.
*)
| (false, None, Some (LocalBinding node))
when match Local_defs.value node with
| NamespaceBinding _ -> true
| _ -> false ->
merge_with_existing_local_binding_node node;
(SMap.add name (LocalBinding node) values_map, SMap.remove name types_map)
(* Has an existing non-namespace binding, do nothing *)
| (false, None, Some _) -> (values_map, types_map)
in
let (values, types) = updater global_scope.values global_scope.types in
global_scope.values <- values;
global_scope.types <- types
| DeclareNamespace { values; types; parent; _ } ->
let (values, types) = namespace_binding_of_values_and_types scope (values, types) in
bind_local
Expand All @@ -1100,6 +1184,7 @@ module Scope = struct
tbls
name
(NamespaceBinding { id_loc; name; values; types })
ignore2
| _ -> failwith "The scope must be lexical"

let bind_globalThis scope tbls ~global_this_loc =
Expand Down Expand Up @@ -4172,7 +4257,7 @@ let namespace_decl
comments = _;
} =
match id with
| Ast.Statement.DeclareNamespace.Global _ -> ignore
| Ast.Statement.DeclareNamespace.Global _ -> ()
| Ast.Statement.DeclareNamespace.Local (id_loc, { Ast.Identifier.name; _ }) ->
let id_loc = push_loc tbls id_loc in
let stmts =
Expand Down Expand Up @@ -4677,7 +4762,7 @@ let rec statement opts scope tbls (loc, stmt) =
Scope.finalize_declare_module_exports_exn scope
| S.DeclareNamespace decl ->
let is_type_only = Flow_ast_utils.is_type_only_declaration_statement (loc, stmt) in
namespace_decl opts scope tbls ~is_type_only ~visit_statement:statement decl ignore2
namespace_decl opts scope tbls ~is_type_only ~visit_statement:statement decl
| S.DeclareEnum decl
| S.EnumDeclaration decl ->
enum_decl opts scope tbls decl ignore2
Expand Down
38 changes: 34 additions & 4 deletions src/parser_utils/type_sig/type_sig_utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,31 @@ let parse_libs opts ordered_asts =
Scope.bind_globalThis scope tbls ~global_this_loc:Loc.none;
(tbls, Scope.builtins_exn scope)

let create_pack_cx additional_errors =
Pack.create_cx
(List.map
(fun e ->
Type_sig.BindingValidationError (Signature_error.map_binding_validation_t Locs.index_exn e))
additional_errors
)

let pack_builtins (tbls, (global_values, global_types, global_modules)) =
let { Parse.locs; module_refs; local_defs; remote_refs; pattern_defs; patterns } = tbls in
let {
Parse.locs;
module_refs;
local_defs;
remote_refs;
pattern_defs;
patterns;
additional_errors;
} =
tbls
in
(* mark *)
SMap.iter (fun _ b -> Mark.mark_binding ~locs_to_dirtify:[] b) global_values;
SMap.iter (fun _ b -> Mark.mark_binding ~locs_to_dirtify:[] b) global_types;
SMap.iter (fun _ m -> Mark.mark_builtin_module m) global_modules;
Mark.mark_errors additional_errors;
(* compact *)
let locs = Locs.compact locs in
let module_refs = Module_refs.Interned.compact module_refs in
Expand All @@ -42,7 +61,7 @@ let pack_builtins (tbls, (global_values, global_types, global_modules)) =
let pattern_defs = Pattern_defs.compact pattern_defs in
let patterns = Patterns.compact patterns in
(* copy *)
let cx = Pack.create_cx () in
let cx = create_pack_cx additional_errors in
let (locs, _) = Locs.copy (fun x -> x) locs in
let (module_refs, _) = Module_refs.copy (fun x -> x) module_refs in
let (local_defs, _) = Local_defs.copy (Pack.pack_local_binding cx) local_defs in
Expand Down Expand Up @@ -97,9 +116,20 @@ let merge_locs loc0 loc1 =
(Loc.debug_to_string ~include_source:true loc1)

let pack ~locs_to_dirtify source (tbls, file_loc, exports) =
let { Parse.locs; module_refs; local_defs; remote_refs; pattern_defs; patterns } = tbls in
let {
Parse.locs;
module_refs;
local_defs;
remote_refs;
pattern_defs;
patterns;
additional_errors;
} =
tbls
in
(* mark *)
Mark.mark_exports ~locs_to_dirtify file_loc exports;
Mark.mark_errors additional_errors;
(* compact *)
let locs = Locs.compact ~merge:merge_locs locs in
let module_refs = Module_refs.Interned.compact module_refs in
Expand All @@ -108,7 +138,7 @@ let pack ~locs_to_dirtify source (tbls, file_loc, exports) =
let pattern_defs = Pattern_defs.compact pattern_defs in
let patterns = Patterns.compact patterns in
(* copy *)
let cx = Pack.create_cx () in
let cx = create_pack_cx additional_errors in
let (locs, _) = Locs.copy (fun x -> x) locs in
let (module_refs, _) = Module_refs.copy (fun x -> x) module_refs in
let (local_defs, dirty_local_defs) = Local_defs.copy (Pack.pack_local_binding cx) local_defs in
Expand Down
Loading

0 comments on commit 632778a

Please sign in to comment.