From 10e2d161be59f4263dfc00a8b6a307a842b8c69e Mon Sep 17 00:00:00 2001 From: Sam Zhou Date: Fri, 7 Mar 2025 09:45:42 -0800 Subject: [PATCH] [flow] Scoped libdef support 4/n: Error on libdef name override Summary: With the support for scoped libdefs, the problem of libdef override will be much more severe. Previously, it's just bad that we just forcefully replace one of the defined entries. Now with scoped libdefs where common code is checked only under common libdefs, a scoped libdef binding overriding a common libdef binding in an inconsistent way might create a false sense of security. Therefore, in this diff, we will now error on the overriding location as a first step, so that we can at least know where the override happens. We can relax this error later when we support more forms of declaration merging. To detect the name overrides, we intercept at the `bind` function. Whenever we see an illegal bind at the global scope level, we will report an error. I have updated all the updaters (those `f`s) to also return whether the binding is legal. Changelog: [errors] Overriding already defined names in library definition will now error. Reviewed By: panagosg7 Differential Revision: D70570824 fbshipit-source-id: 62e59e108143c5233367de626a1af98706bdc660 --- src/parser_utils/type_sig/type_sig_parse.ml | 151 +++++++++++++----- tests/call_caching1/.flowconfig | 2 +- tests/call_caching1/lib/core.js | 22 +-- tests/call_caching1/test.js | 4 +- tests/call_caching2/call_caching2.exp | 8 +- tests/call_caching2/lib/immutable.js | 2 - tests/computed_race/.flowconfig | 1 + tests/computed_race/lib.js | 3 - tests/duplicate_libs/duplicate_libs.exp | 13 +- tests/duplicate_libs/lib/shared/lib.js | 2 +- .../global_this_already_defined.exp | 13 +- tests/global_this_already_defined/globals.js | 2 +- .../libdef_react_types_override.exp | 18 ++- .../libdef_react_types_override/libs/react.js | 2 +- tests/libdef_scoped/bar_lib.js | 2 +- tests/libdef_scoped/libdef_scoped.exp | 34 +++- tests/react_create_element/lib.js | 2 +- .../react_create_element.exp | 17 +- tests/recheck/recheck.exp | 18 ++- tests/recheck/test.sh | 2 +- tests/type_param_variance2/libs/Promise.js | 2 +- .../type_param_variance2.exp | 18 ++- 22 files changed, 249 insertions(+), 89 deletions(-) diff --git a/src/parser_utils/type_sig/type_sig_parse.ml b/src/parser_utils/type_sig/type_sig_parse.ml index 59a597624c8..bcd8bddb50d 100644 --- a/src/parser_utils/type_sig/type_sig_parse.ml +++ b/src/parser_utils/type_sig/type_sig_parse.ml @@ -336,6 +336,33 @@ type frozen_kind = let ignore2 _ _ = () +let loc_of_binding = function + | LocalBinding node -> + (match Local_defs.value node with + | VarBinding { id_loc; _ } + | LetConstBinding { id_loc; _ } + | ConstRefBinding { id_loc; _ } + | ConstFunBinding { id_loc; _ } + | ClassBinding { id_loc; _ } + | DeclareClassBinding { id_loc; _ } + | FunBinding { id_loc; _ } + | ComponentBinding { id_loc; _ } + | EnumBinding { id_loc; _ } + | NamespaceBinding { id_loc; _ } + | TypeBinding { id_loc; _ } -> + id_loc + | DeclareFunBinding { defs_rev; _ } -> + let (id_loc, _, _) = Nel.last defs_rev in + id_loc) + | RemoteBinding node -> + (match Remote_refs.value node with + | ImportBinding { id_loc; _ } + | ImportTypeBinding { id_loc; _ } + | ImportTypeofBinding { id_loc; _ } + | ImportNsBinding { id_loc; _ } + | ImportTypeofNsBinding { id_loc; _ } -> + id_loc) + let create_tables () = { locs = Locs.create (); @@ -608,8 +635,37 @@ module Scope = struct | ID.ImportTypeof -> ImportTypeofNsBinding { id_loc; name; mref } | ID.ImportType -> failwith "unexpected import type *" - let bind ~type_only scope name f = - let bind_value values types = + let bind ~type_only scope tbls name id_loc f = + let bind_updater ~in_global_scope f existing_binding = + (* For now, we allow $JSXIntrinsics, since we intentionally kept only a minimal version in + * the builtins, and let flow-typed supply a full one. *) + if in_global_scope && name <> "$JSXIntrinsics" then ( + let (binding, legal) = f existing_binding in + (match (legal, existing_binding) with + | (false, Some existing_binding) -> + let override_binding_loc = loc_of_binding existing_binding in + (* Consider + * ``` + * declare const foo: string; + * declare const foo: number; + * ``` + * The first one is considered the overriding one, + * while the second one is considered to be the existing one. + * This ensures that the builtin libdefs (which come last) will always considered to be the + * existing ones. + * *) + tbls.additional_errors <- + Signature_error.NameOverride + { name; override_binding_loc; existing_binding_loc = id_loc } + :: tbls.additional_errors + | _ -> ()); + binding + ) else + let (binding, _legal) = f existing_binding in + binding + in + let bind_value ~in_global_scope f values types = + let f = bind_updater ~in_global_scope f in match SMap.find_opt name values with | Some _ -> SMap.update name f values | None -> @@ -617,7 +673,8 @@ module Scope = struct | None -> SMap.update name f values | Some _ -> values) in - let bind_type values types = + let bind_type ~in_global_scope f values types = + let f = bind_updater ~in_global_scope f in match SMap.find_opt name types with | Some _ -> SMap.update name f types | None -> @@ -627,19 +684,24 @@ module Scope = struct in if type_only then match scope with - | Global scope -> scope.types <- bind_type scope.values scope.types - | DeclareModule scope -> scope.types <- bind_type scope.values scope.types - | DeclareNamespace scope -> scope.types <- bind_type scope.values scope.types - | Module scope -> scope.types <- bind_type scope.values scope.types - | Lexical scope -> scope.types <- bind_type scope.values scope.types + | Global scope -> scope.types <- bind_type ~in_global_scope:true f scope.values scope.types + | DeclareModule scope -> + scope.types <- bind_type ~in_global_scope:false f scope.values scope.types + | DeclareNamespace scope -> + scope.types <- bind_type ~in_global_scope:false f scope.values scope.types + | Module scope -> scope.types <- bind_type ~in_global_scope:false f scope.values scope.types + | Lexical scope -> scope.types <- bind_type ~in_global_scope:false f scope.values scope.types | ConditionalTypeExtends _ -> () else match scope with - | Global scope -> scope.values <- bind_value scope.values scope.types - | DeclareModule scope -> scope.values <- bind_value scope.values scope.types - | DeclareNamespace scope -> scope.values <- bind_value scope.values scope.types - | Module scope -> scope.values <- bind_value scope.values scope.types - | Lexical scope -> scope.values <- bind_value scope.values scope.types + | Global scope -> scope.values <- bind_value ~in_global_scope:true f scope.values scope.types + | DeclareModule scope -> + scope.values <- bind_value ~in_global_scope:false f scope.values scope.types + | DeclareNamespace scope -> + scope.values <- bind_value ~in_global_scope:false f scope.values scope.types + | Module scope -> scope.values <- bind_value ~in_global_scope:false f scope.values scope.types + | Lexical scope -> + scope.values <- bind_value ~in_global_scope:false f scope.values scope.types | ConditionalTypeExtends _ -> () let rec lookup_value scope name = @@ -704,41 +766,41 @@ module Scope = struct | Some l when l = loc -> Some scope | _ -> None) - let bind_local scope tbls name def k = + let bind_local scope tbls name id_loc def k = let host = find_host scope def in - bind host name (function - | Some _ as existing_binding -> existing_binding + bind host tbls name id_loc (function + | Some _ as existing_binding -> (existing_binding, false) | None -> let node = push_local_def tbls def in k name node; - Some (LocalBinding node) + (Some (LocalBinding node), true) ) - let bind_remote scope tbls name ref = - bind scope name (function - | Some _ as existing_binding -> existing_binding + let bind_remote scope tbls name id_loc ref = + bind scope tbls name id_loc (function + | Some _ as existing_binding -> (existing_binding, false) | None -> let node = push_remote_ref tbls ref in - Some (RemoteBinding node) + (Some (RemoteBinding node), true) ) let bind_type scope tbls id_loc name def = - bind_local ~type_only:true scope tbls name (TypeBinding { id_loc; def }) + bind_local ~type_only:true scope tbls name id_loc (TypeBinding { id_loc; def }) let bind_class scope tbls id_loc name def = - bind_local ~type_only:false scope tbls name (ClassBinding { id_loc; name; def }) + bind_local ~type_only:false scope tbls name id_loc (ClassBinding { id_loc; name; def }) let bind_declare_class scope tbls id_loc name def = - bind_local ~type_only:false scope tbls name (DeclareClassBinding { id_loc; name; def }) + bind_local ~type_only:false scope tbls name id_loc (DeclareClassBinding { id_loc; name; def }) let bind_enum scope tbls id_loc name def = - bind_local ~type_only:false scope tbls name (EnumBinding { id_loc; name; def }) + bind_local ~type_only:false scope tbls name id_loc (EnumBinding { id_loc; name; def }) (* Function declarations preceded by declared functions are taken to have the * type of the declared functions. This is a weird special case aimed to * support overloaded signatures. *) let bind_function scope tbls id_loc fn_loc name ~async ~generator ~effect:_ def k = - bind ~type_only:false scope name (fun binding_opt -> + bind ~type_only:false scope tbls name id_loc (fun binding_opt -> match binding_opt with | None -> let statics = SMap.empty in @@ -747,13 +809,13 @@ module Scope = struct in let node = push_local_def tbls def in k name node; - Some (LocalBinding node) - | Some (RemoteBinding _) -> binding_opt + (Some (LocalBinding node), true) + | Some (RemoteBinding _) -> (binding_opt, false) | Some (LocalBinding node) -> (match Local_defs.value node with | DeclareFunBinding _ -> k name node | _ -> ()); - binding_opt + (binding_opt, false) ) (* Multiple declared functions with the same name in the same scope define an @@ -761,38 +823,46 @@ module Scope = struct * don't need to walk the scope chain since the scope argument is certainly * the host scope. *) let bind_declare_function scope tbls id_loc fn_loc name def k = - bind ~type_only:false scope name (fun binding_opt -> + bind ~type_only:false scope tbls name id_loc (fun binding_opt -> match binding_opt with | None -> let defs_rev = Nel.one (id_loc, fn_loc, def) in let def = DeclareFunBinding { name; defs_rev } in let node = push_local_def tbls def in k name node; - Some (LocalBinding node) - | Some (RemoteBinding _) -> binding_opt + (Some (LocalBinding node), true) + | Some (RemoteBinding _) -> (binding_opt, false) | Some (LocalBinding node) -> + let legal_ref = ref false in Local_defs.modify node (function | DeclareFunBinding { name; defs_rev } -> k name node; + legal_ref := true; let defs_rev = Nel.cons (id_loc, fn_loc, def) defs_rev in DeclareFunBinding { name; defs_rev } | def -> def ); - binding_opt + (binding_opt, !legal_ref) ) let bind_component scope tbls id_loc fn_loc name def = - bind_local ~type_only:false scope tbls name (ComponentBinding { id_loc; fn_loc; name; def }) + bind_local + ~type_only:false + scope + tbls + name + id_loc + (ComponentBinding { id_loc; fn_loc; name; def }) let bind_var scope tbls kind id_loc name def = - bind_local ~type_only:false scope tbls name (value_binding kind id_loc name def) + bind_local ~type_only:false scope tbls name id_loc (value_binding kind id_loc name def) let bind_const scope tbls id_loc name def = - bind_local ~type_only:false scope tbls name (LetConstBinding { id_loc; name; def }) + bind_local ~type_only:false scope tbls name id_loc (LetConstBinding { id_loc; name; def }) let bind_const_ref scope tbls id_loc name ref_loc ref_name ref_scope = let ref = Ref { ref_loc; name = ref_name; scope = ref_scope; resolved = None } in - bind_local ~type_only:false scope tbls name (ConstRefBinding { id_loc; name; ref }) + bind_local ~type_only:false scope tbls name id_loc (ConstRefBinding { id_loc; name; ref }) let bind_const_fun scope tbls id_loc name loc ~async ~generator def = let statics = SMap.empty in @@ -801,6 +871,7 @@ module Scope = struct scope tbls name + id_loc (ConstFunBinding { id_loc; name; loc; async; generator; def; statics }) let bind_import scope tbls kind id_loc ~local ~remote mref = @@ -813,7 +884,7 @@ module Scope = struct | ImportTypeof -> true in - bind_remote ~type_only scope tbls local (import_binding kind id_loc local mref ~remote) + bind_remote ~type_only scope tbls local id_loc (import_binding kind id_loc local mref ~remote) let bind_import_ns scope tbls kind id_loc name mref = let mref = push_module_ref tbls mref in @@ -825,7 +896,7 @@ module Scope = struct | ImportTypeof -> true in - bind_remote ~type_only scope tbls name (import_ns_binding kind id_loc name mref) + bind_remote ~type_only scope tbls name id_loc (import_ns_binding kind id_loc name mref) let rec assign_binding = let f prop_name prop def = @@ -1185,6 +1256,7 @@ module Scope = struct parent tbls name + id_loc (NamespaceBinding { id_loc; name; values; types }) ignore2 | _ -> failwith "The scope must be lexical" @@ -1206,6 +1278,7 @@ module Scope = struct scope tbls name + loc (NamespaceBinding { id_loc = loc; name; values; types }) ignore2 | _ -> failwith "finalize_globalThis must be called after parsing all lib files on global scope" diff --git a/tests/call_caching1/.flowconfig b/tests/call_caching1/.flowconfig index 1d1e492b909..8ab9df0a782 100644 --- a/tests/call_caching1/.flowconfig +++ b/tests/call_caching1/.flowconfig @@ -7,4 +7,4 @@ lib [options] all=true -no_flowlib=true +no_flowlib=false diff --git a/tests/call_caching1/lib/core.js b/tests/call_caching1/lib/core.js index 7b553430c24..158051746e8 100644 --- a/tests/call_caching1/lib/core.js +++ b/tests/call_caching1/lib/core.js @@ -1,24 +1,4 @@ -declare class Array { - @@iterator(): Iterator; - map(callbackfn: (value: T, index: number, array: Array) => U, thisArg?: any): Array; -} - -type IteratorResult<+Yield,+Return> = - | { done: true, +value?: Return, ...} - | { done: false, +value: Yield, ... }; - -interface $Iterator<+Yield,+Return,-Next> { - @@iterator(): $Iterator; - next(value?: Next): IteratorResult; -} -type Iterator<+T> = $Iterator; - -interface $Iterable<+Yield,+Return,-Next> { - @@iterator(): $Iterator; -} -type Iterable<+T> = $Iterable; - -declare class Map { +declare class MyMap { @@iterator(): Iterator<[K, V]>; constructor(iterable: ?Iterable<[K, V]>): void; set(key: K, value: V): Map; diff --git a/tests/call_caching1/test.js b/tests/call_caching1/test.js index ea6098b4749..e506fdfb525 100644 --- a/tests/call_caching1/test.js +++ b/tests/call_caching1/test.js @@ -1,7 +1,7 @@ const Immutable = require('immutable'); -const tasksPerStatusMap = new Map( - ([]: Array).map(taskStatus => [taskStatus, (new Map(): Map | Immutable.Map)]), +const tasksPerStatusMap = new MyMap( + ([]: Array).map(taskStatus => [taskStatus, (new MyMap(): MyMap | Immutable.Map)]), ); for (let [taskStatus, tasksMap] of tasksPerStatusMap) { tasksPerStatusMap.set(taskStatus, Immutable.Map(tasksMap)); diff --git a/tests/call_caching2/call_caching2.exp b/tests/call_caching2/call_caching2.exp index 7963e23453b..636932f5abb 100644 --- a/tests/call_caching2/call_caching2.exp +++ b/tests/call_caching2/call_caching2.exp @@ -12,14 +12,14 @@ References: test.js:1:22 1| function Foo(items: ?Iterable) { ^^^^^^^^^^^^^^^^ [1] - lib/immutable.js:7:20 - 7| static (iter: Array): Iterable; + lib/immutable.js:5:20 + 5| static (iter: Array): Iterable; ^^^^^^^^ [2] test.js:2:21 2| Iterable(items || []).size; ^^ [3] - lib/immutable.js:6:18 - 6| static >(iter: Iter): Iter; + lib/immutable.js:4:18 + 4| static >(iter: Iter): Iter; ^^^^^^^^^^^ [4] diff --git a/tests/call_caching2/lib/immutable.js b/tests/call_caching2/lib/immutable.js index d49d39aea01..ee113112081 100644 --- a/tests/call_caching2/lib/immutable.js +++ b/tests/call_caching2/lib/immutable.js @@ -1,7 +1,5 @@ // Copyright (c) Meta Platforms, Inc. and affiliates. -declare class Array { } - declare class Iterable { static >(iter: Iter): Iter; static (iter: Array): Iterable; diff --git a/tests/computed_race/.flowconfig b/tests/computed_race/.flowconfig index 5ce359f5905..e76f093c5f3 100644 --- a/tests/computed_race/.flowconfig +++ b/tests/computed_race/.flowconfig @@ -3,3 +3,4 @@ lib.js [options] all=true +no_flowlib=false diff --git a/tests/computed_race/lib.js b/tests/computed_race/lib.js index 882dbf79a05..e69de29bb2d 100644 --- a/tests/computed_race/lib.js +++ b/tests/computed_race/lib.js @@ -1,3 +0,0 @@ -declare class Object { - static freeze(o: T): T; -} diff --git a/tests/duplicate_libs/duplicate_libs.exp b/tests/duplicate_libs/duplicate_libs.exp index bf006d4aec0..154f329e9bb 100644 --- a/tests/duplicate_libs/duplicate_libs.exp +++ b/tests/duplicate_libs/duplicate_libs.exp @@ -1,3 +1,12 @@ +Error ------------------------------------------------------------------------------------------- lib/shared/lib.js:3:13 + +This name declaration overrides an existing binding `global_foo` [1]. Overriding in library definitions can lead to +surprising behaviors. [libdef-override] + + 3| declare var global_foo: number; // intentional-libdef-override + ^^^^^^^^^^ [1] + + Error ------------------------------------------------------------------------------------------------------ test.js:1:2 Cannot cast `global_foo` to string because number [1] is incompatible with string [2]. [incompatible-cast] @@ -8,7 +17,7 @@ Cannot cast `global_foo` to string because number [1] is incompatible with strin References: lib/shared/lib.js:3:25 - 3| declare var global_foo: number; + 3| declare var global_foo: number; // intentional-libdef-override ^^^^^^ [1] test.js:1:14 1| (global_foo: string); // error: number ~> string @@ -16,4 +25,4 @@ References: -Found 1 error +Found 2 errors diff --git a/tests/duplicate_libs/lib/shared/lib.js b/tests/duplicate_libs/lib/shared/lib.js index 7aed0f4810a..94c6bace474 100644 --- a/tests/duplicate_libs/lib/shared/lib.js +++ b/tests/duplicate_libs/lib/shared/lib.js @@ -1,3 +1,3 @@ // The lib file is included twice, because it is included in both `lib` and // `lib/shared`, which are both listed in the `.flowconfig` -declare var global_foo: number; +declare var global_foo: number; // intentional-libdef-override diff --git a/tests/global_this_already_defined/global_this_already_defined.exp b/tests/global_this_already_defined/global_this_already_defined.exp index 46939079f59..d8ef8f43ffa 100644 --- a/tests/global_this_already_defined/global_this_already_defined.exp +++ b/tests/global_this_already_defined/global_this_already_defined.exp @@ -1,3 +1,12 @@ +Error -------------------------------------------------------------------------------------------------- globals.js:1:15 + +This name declaration overrides an existing binding `globalThis`. Overriding in library definitions can lead to +surprising behaviors. [libdef-override] + + 1| declare const globalThis: mixed; // intentional-libdef-override + ^^^^^^^^^^ + + Error ----------------------------------------------------------------------------------------------------- test.js:1:12 Cannot call `globalThis.alert` because property `alert` is missing in mixed [1]. [incompatible-use] @@ -8,9 +17,9 @@ Cannot call `globalThis.alert` because property `alert` is missing in mixed [1]. References: globals.js:1:27 - 1| declare const globalThis: mixed; + 1| declare const globalThis: mixed; // intentional-libdef-override ^^^^^ [1] -Found 1 error +Found 2 errors diff --git a/tests/global_this_already_defined/globals.js b/tests/global_this_already_defined/globals.js index d5af0f7cb11..b87d41631e7 100644 --- a/tests/global_this_already_defined/globals.js +++ b/tests/global_this_already_defined/globals.js @@ -1,2 +1,2 @@ -declare const globalThis: mixed; +declare const globalThis: mixed; // intentional-libdef-override declare function alert(): void; diff --git a/tests/libdef_react_types_override/libdef_react_types_override.exp b/tests/libdef_react_types_override/libdef_react_types_override.exp index 2829d581f51..8633e671dcd 100644 --- a/tests/libdef_react_types_override/libdef_react_types_override.exp +++ b/tests/libdef_react_types_override/libdef_react_types_override.exp @@ -1 +1,17 @@ -Found 0 errors +Error ----------------------------------------------------------------------------------------------- libs/react.js:1:14 + +This name declaration overrides an existing binding `React.Node` [1]. Overriding in library definitions can lead to +surprising behaviors. [libdef-override] + + libs/react.js:1:14 + 1| declare type React$Node = // intentional-libdef-override + ^^^^^^^^^^ + +References: + /react.js:15:14 + 15| declare type React$Node = + ^^^^^^^^^^ [1] + + + +Found 1 error diff --git a/tests/libdef_react_types_override/libs/react.js b/tests/libdef_react_types_override/libs/react.js index 2599f56caea..6590a94576a 100644 --- a/tests/libdef_react_types_override/libs/react.js +++ b/tests/libdef_react_types_override/libs/react.js @@ -1,4 +1,4 @@ -declare type React$Node = +declare type React$Node = // intentional-libdef-override | void | null | boolean diff --git a/tests/libdef_scoped/bar_lib.js b/tests/libdef_scoped/bar_lib.js index d4a43cbbe6f..1610f92738e 100644 --- a/tests/libdef_scoped/bar_lib.js +++ b/tests/libdef_scoped/bar_lib.js @@ -1,2 +1,2 @@ -declare const commonValueWithOverride: boolean; +declare const commonValueWithOverride: boolean; // intentional-libdef-override declare const barSpecific: string; diff --git a/tests/libdef_scoped/libdef_scoped.exp b/tests/libdef_scoped/libdef_scoped.exp index 96d4b425130..4663fd01f31 100644 --- a/tests/libdef_scoped/libdef_scoped.exp +++ b/tests/libdef_scoped/libdef_scoped.exp @@ -1,3 +1,33 @@ +Error -------------------------------------------------------------------------------------------------- bar_lib.js:1:15 + +This name declaration overrides an existing binding `commonValueWithOverride` [1]. Overriding in library definitions can +lead to surprising behaviors. [libdef-override] + + bar_lib.js:1:15 + 1| declare const commonValueWithOverride: boolean; // intentional-libdef-override + ^^^^^^^^^^^^^^^^^^^^^^^ + +References: + common_lib2.js:1:15 + 1| declare const commonValueWithOverride: string; + ^^^^^^^^^^^^^^^^^^^^^^^ [1] + + +Error ------------------------------------------------------------------------------------------------- foo_lib2.js:1:15 + +This name declaration overrides an existing binding `commonValueWithOverride` [1]. Overriding in library definitions can +lead to surprising behaviors. [libdef-override] + + foo_lib2.js:1:15 + 1| declare const commonValueWithOverride: number; + ^^^^^^^^^^^^^^^^^^^^^^^ + +References: + common_lib2.js:1:15 + 1| declare const commonValueWithOverride: string; + ^^^^^^^^^^^^^^^^^^^^^^^ [1] + + Error -------------------------------------------------------------------------------------------------- bar/test.js:1:1 Cannot cast `commonValueNoOverride` to empty because string [1] is incompatible with empty [2]. [incompatible-cast] @@ -25,7 +55,7 @@ Cannot cast `commonValueWithOverride` to empty because boolean [1] is incompatib References: bar_lib.js:1:40 - 1| declare const commonValueWithOverride: boolean; + 1| declare const commonValueWithOverride: boolean; // intentional-libdef-override ^^^^^^^ [1] bar/test.js:2:28 2| commonValueWithOverride as empty; // error: boolean ~> empty @@ -167,4 +197,4 @@ Cannot resolve name `barSpecific`. [cannot-resolve-name] -Found 12 errors +Found 14 errors diff --git a/tests/react_create_element/lib.js b/tests/react_create_element/lib.js index f0cd0cef987..3c38ba8757d 100644 --- a/tests/react_create_element/lib.js +++ b/tests/react_create_element/lib.js @@ -24,7 +24,7 @@ type CheckedReactElement< > = ExactReactElement_DEPRECATED; -declare opaque type React$CreateElement: +declare opaque type React$CreateElement: // intentional-libdef-override & (< Comp: React$ElementType, Children: $ReadOnlyArray = [], diff --git a/tests/react_create_element/react_create_element.exp b/tests/react_create_element/react_create_element.exp index b19756b613d..04de5cbedd8 100644 --- a/tests/react_create_element/react_create_element.exp +++ b/tests/react_create_element/react_create_element.exp @@ -1,3 +1,18 @@ +Error ----------------------------------------------------------------------------------------------------- lib.js:27:21 + +This name declaration overrides an existing binding `React.CreateElement` [1]. Overriding in library definitions can +lead to surprising behaviors. [libdef-override] + + lib.js:27:21 + 27| declare opaque type React$CreateElement: // intentional-libdef-override + ^^^^^^^^^^^^^^^^^^^ + +References: + /react.js:172:21 + 172| declare opaque type React$CreateElement; + ^^^^^^^^^^^^^^^^^^^ [1] + + Error ------------------------------------------------------------------------------------------------------ test.js:5:7 Cannot call `React.createElement` because: [incompatible-call] @@ -1606,7 +1621,7 @@ References: -Found 86 errors +Found 87 errors Only showing the most relevant union/intersection branches. To see all branches, re-run Flow with --show-all-branches diff --git a/tests/recheck/recheck.exp b/tests/recheck/recheck.exp index 60376890c00..4458688e0f2 100644 --- a/tests/recheck/recheck.exp +++ b/tests/recheck/recheck.exp @@ -350,7 +350,23 @@ References: Found 1 error -No errors! +Error ------------------------------------------------------------------------------------------------- lib/libk2.js:1:6 + +This name declaration overrides an existing binding `LibK` [1]. Overriding in library definitions can lead to surprising +behaviors. [libdef-override] + + lib/libk2.js:1:6 + 1| type LibK = { + ^^^^ + +References: + lib/libk1.js:1:6 + 1| type LibK = {...} + ^^^^ [1] + + + +Found 1 error Error --------------------------------------------------------------------------------------------------------- k.js:4:4 Cannot get `x.p` because property `p` is missing in `LibK` [1]. [prop-missing] diff --git a/tests/recheck/test.sh b/tests/recheck/test.sh index f9157b0f70d..cedbbf2b96b 100755 --- a/tests/recheck/test.sh +++ b/tests/recheck/test.sh @@ -94,7 +94,7 @@ copy tmplibk/libk1.js lib/libk1.js copy tmpk/k.js ./k.js assert_errors "$FLOW" status . copy tmplibk/libk2.js lib/libk2.js -assert_ok "$FLOW" status . +assert_errors "$FLOW" status . remove lib/libk2.js assert_errors "$FLOW" status . remove k.js lib/*.js diff --git a/tests/type_param_variance2/libs/Promise.js b/tests/type_param_variance2/libs/Promise.js index 6bbd8c59bec..1b28bce9c70 100644 --- a/tests/type_param_variance2/libs/Promise.js +++ b/tests/type_param_variance2/libs/Promise.js @@ -7,7 +7,7 @@ // Any definitions here will override similarly-named ones in // library files declared earlier, including default flow libs. -declare class Promise<+R> { +declare class Promise<+R> { // intentional-libdef-override constructor(callback: ( resolve: (result?: Promise | R) => void, reject: (error?: any) => void diff --git a/tests/type_param_variance2/type_param_variance2.exp b/tests/type_param_variance2/type_param_variance2.exp index 2829d581f51..1e77f5a7a4a 100644 --- a/tests/type_param_variance2/type_param_variance2.exp +++ b/tests/type_param_variance2/type_param_variance2.exp @@ -1 +1,17 @@ -Found 0 errors +Error -------------------------------------------------------------------------------------------- libs/Promise.js:10:15 + +This name declaration overrides an existing binding `Promise` [1]. Overriding in library definitions can lead to +surprising behaviors. [libdef-override] + + libs/Promise.js:10:15 + 10| declare class Promise<+R> { // intentional-libdef-override + ^^^^^^^ + +References: + /core.js:2102:15 + 2102| declare class Promise<+R = mixed> { + ^^^^^^^ [1] + + + +Found 1 error