Skip to content

Commit ff3e286

Browse files
SamChou19815facebook-github-bot
authored andcommitted
[flow] Scoped libdef support 5/n: Error on libdef module override
Summary: Same as last diffs, but for modules. Changelog: [errors] Overriding already defined modules in library definition will now error. Reviewed By: panagosg7 Differential Revision: D70575060 fbshipit-source-id: 72edd21a089e2b30319cc3c28103b6239f9288dc
1 parent 10e2d16 commit ff3e286

File tree

5 files changed

+47
-5
lines changed

5 files changed

+47
-5
lines changed

src/parser_utils/type_sig/__tests__/type_sig_tests.ml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4657,7 +4657,12 @@ let%expect_test "builtin_cjs_ignore_later" =
46574657
info =
46584658
CJSModuleInfo {type_export_keys = [||];
46594659
type_stars = []; strict = true;
4660-
platform_availability_set = None}} |}]
4660+
platform_availability_set = None}}
4661+
Errors:
4662+
(BindingValidationError
4663+
Signature_error.ModuleOverride {
4664+
name = "foo"; override_binding_loc = [2:15-18];
4665+
existing_binding_loc = [5:15-18]}) |}]
46614666

46624667
let%expect_test "builtin_cjs_module_auto_export_type" =
46634668
(* All types in cjs modules are auto exported. *)

src/parser_utils/type_sig/type_sig_parse.ml

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ module Scope = struct
544544
let push_declare_namespace parent =
545545
DeclareNamespace { parent; values = SMap.empty; types = SMap.empty }
546546

547-
let push_declare_module loc name parent =
547+
let push_declare_module loc name parent tbls =
548548
let exports = Exports.create ~strict:true ~platform_availability_set:None in
549549
begin
550550
match parent with
@@ -554,7 +554,13 @@ module Scope = struct
554554
name
555555
(function
556556
| None -> Some (loc, exports)
557-
| Some existing -> Some existing)
557+
| Some existing ->
558+
let override_binding_loc = fst existing in
559+
tbls.additional_errors <-
560+
Signature_error.ModuleOverride
561+
{ name; override_binding_loc; existing_binding_loc = loc }
562+
:: tbls.additional_errors;
563+
Some existing)
558564
g.modules
559565
| _ -> ()
560566
end;
@@ -4824,7 +4830,7 @@ let rec statement opts scope tbls (loc, stmt) =
48244830
| S.DeclareModule.Literal (id_loc, { Ast.StringLiteral.value; _ }) ->
48254831
(push_loc tbls id_loc, value)
48264832
in
4827-
let scope = Scope.push_declare_module loc name scope in
4833+
let scope = Scope.push_declare_module loc name scope tbls in
48284834
let (_, { S.Block.body = stmts; comments = _ }) = body in
48294835
let visit_statement ((_, stmt') as stmt) =
48304836
match

tests/duplicate_libs/duplicate_libs.exp

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ surprising behaviors. [libdef-override]
77
^^^^^^^^^^ [1]
88

99

10+
Error ------------------------------------------------------------------------------------------- lib/shared/lib.js:5:16
11+
12+
This module declaration overrides an existing module `bar` [1]. Overriding in library definitions can lead to surprising
13+
behaviors. [libdef-override]
14+
15+
5| declare module 'bar' {
16+
^^^^^ [1]
17+
18+
1019
Error ------------------------------------------------------------------------------------------------------ test.js:1:2
1120

1221
Cannot cast `global_foo` to string because number [1] is incompatible with string [2]. [incompatible-cast]
@@ -24,5 +33,22 @@ References:
2433
^^^^^^ [2]
2534

2635

36+
Error ------------------------------------------------------------------------------------------------------ test.js:2:2
37+
38+
Cannot cast `require(...)` to number because string [1] is incompatible with number [2]. [incompatible-cast]
39+
40+
test.js:2:2
41+
2| (require('bar'): number); // error: string ~> number
42+
^^^^^^^^^^^^^^
43+
44+
References:
45+
lib/shared/lib.js:6:27
46+
6| declare module.exports: string;
47+
^^^^^^ [1]
48+
test.js:2:18
49+
2| (require('bar'): number); // error: string ~> number
50+
^^^^^^ [2]
51+
52+
2753

28-
Found 2 errors
54+
Found 4 errors
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
11
// The lib file is included twice, because it is included in both `lib` and
22
// `lib/shared`, which are both listed in the `.flowconfig`
33
declare var global_foo: number; // intentional-libdef-override
4+
5+
declare module 'bar' {
6+
declare module.exports: string;
7+
}

tests/duplicate_libs/test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
(global_foo: string); // error: number ~> string
2+
(require('bar'): number); // error: string ~> number

0 commit comments

Comments
 (0)