Skip to content

Commit

Permalink
[flow] Specialize TypeAppT at the declaration site
Browse files Browse the repository at this point in the history
Summary:
Similar to `EvalT`, let's force it at the declaration site. Similar to the challenges to `EvalT`, forcing it everywhere causes too many errors, which points to some gaps in the language design.

This diff alone will not fix the issue of misplaced error, but at least the error will now consistently appear at the declaration site no matter whether it's used, so users can see it in the IDE without being overridden. It's also a step towards getting rid of all misplaced errors. In a future when we can eagerly validate all these things that tend to cause misplaced errors, we can just filter out these errors in the future.

Changelog: [errors] Flow will now error on invalid type applications at the declaration site, if the type application does not use another generic type from an outer scope. [example](https://flow.org/try/#1N4Igxg9gdgZglgcxALlAIwIZoKYBsD6uEEAztvhgE6UYCe+JADpdhgCYowa5kA0I2KAFcAtiRQAXSkOz9sADwxgJ+NPTbYuQ3BMnTZA+Y2yU4IwRO4A6SFBIrGVDGM7c+IFkolXpUCWewUEAwhCQgRDH8wEH4hMnwROHlsNnw4KHwwSLAAC3wANyo4LFxscWQuHgMNZmwsiRSAWglaY1cq-hIAa2wJXNpG4Vxcdvdu3v7B0RxKUYMhKDBSqmbWwIq3eagoOrKSKgH0wtMMPznY7d2SfcoBiEZ-aG5G3Ix085AF-ZhsRoRehqUEiNMgSQHlSruBZxJrMcJwMhzAC+-EgGiCLWMAAIACpYgC8WPspigCAA3AAdPxrLEAQQJuIAPMT0ggAHxkrEAei5WJMlAglCxUAgAHcYiB8iYSHBoEEIvYTCAkUA)

Reviewed By: panagosg7

Differential Revision: D55456495

fbshipit-source-id: 03527deab4a2054d4659c1e8481a9df3f921ed64
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Mar 29, 2024
1 parent 4c4a9a9 commit 514f690
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 7 deletions.
14 changes: 11 additions & 3 deletions src/typing/type_annotation.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2597,9 +2597,17 @@ module Make (ConsGen : Type_annotation_sig.ConsGen) (Statement : Statement_sig.S
(ConsGen.mk_instance env.cx ~type_t_kind reason c, None)
| Some (loc, { Ast.Type.TypeArgs.arguments = targs; comments }) ->
let (targs, targs_ast) = convert_list env targs in
( typeapp_annot ~from_value:false ~use_desc:false annot_loc c targs,
Some (loc, { Ast.Type.TypeArgs.arguments = targs_ast; comments })
)
let t = typeapp_annot ~from_value:false ~use_desc:false annot_loc c targs in
( if Subst_name.Set.is_empty (Type_subst.free_var_finder env.cx t) then
match t with
| TypeAppT { reason; use_op; type_; targs; from_value = _; use_desc = _ } ->
Context.add_post_inference_validation_flow
env.cx
type_
(SpecializeT (use_op, reason, reason, true, Some targs, Tvar.mk env.cx reason))
| _ -> failwith "typeapp_annot should create a TypeAppT"
);
(t, Some (loc, { Ast.Type.TypeArgs.arguments = targs_ast; comments }))

and mk_type_param env ~from_infer_type (loc, type_param) =
let { cx; tparams_map; _ } = env in
Expand Down
17 changes: 16 additions & 1 deletion tests/badly_positioned_polyt/badly_positioned_polyt.exp
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
Error ----------------------------------------------------------------------------------------------- concretize.js:7:47

Cannot instantiate `SyntheticKeyboardEvent` because `C1` [1] is incompatible with `EventTarget` [2] in type argument
`T`. [incompatible-type-arg]

concretize.js:7:47
7| function _onKeyDown(e: SyntheticKeyboardEvent<C1>): void {};
^^ [1]

References:
<BUILTINS>/react-dom.js:74:42
74| declare class SyntheticKeyboardEvent<+T: EventTarget = EventTarget>
^^^^^^^^^^^ [2]


Error ----------------------------------------------------------------------------------------------- concretize.js:8:16

Cannot create `C1` element because `EventTarget` [1] is incompatible with `C1` [2] in type argument `T` [3] of the first
Expand Down Expand Up @@ -74,4 +89,4 @@ References:



Found 4 errors
Found 5 errors
18 changes: 17 additions & 1 deletion tests/contextual_typing/contextual_typing.exp
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,22 @@ References:
^^^^ [1]


Error --------------------------------------------------------------------------------- implicit_instantiation.js:211:21

Cannot instantiate `Record` because interface type [1] is not a subtype of object type [2]. Class instances are not
subtypes of object types; consider rewriting object type [2] as an interface in type argument `T`.
[class-object-subtyping]

implicit_instantiation.js:211:21
211| records: Record<interface {}>,
^^^^^^^^^^^^ [1]

References:
implicit_instantiation.js:203:18
203| type Record<T: {...}> = { ...T, ... };
^^^^^ [2]


Error -------------------------------------------------------------------------------------------- intersections.js:7:24

Cannot cast `x` to number because string [1] is incompatible with number [2]. [incompatible-cast]
Expand Down Expand Up @@ -1221,4 +1237,4 @@ References:



Found 88 errors
Found 89 errors
32 changes: 31 additions & 1 deletion tests/relay/relay.exp
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,36 @@ References:
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [2]


Error --------------------------------------------------------------------------------------------------- more.js:140:34

Cannot instantiate `FragmentReference` because object type [1] is incompatible with `FragmentTypeof` [2] in type
argument `T`. [incompatible-type-arg]

more.js:140:34
140| __fragments: FragmentReference<{thing1: true}> &
^^^^^^^^^^^^^^ [1]

References:
more.js:13:42
13| declare opaque type FragmentReference<T: FragmentTypeof>;
^^^^^^^^^^^^^^ [2]


Error --------------------------------------------------------------------------------------------------- more.js:142:23

Cannot instantiate `FragmentReference` because object type [1] is incompatible with `FragmentTypeof` [2] in type
argument `T`. [incompatible-type-arg]

more.js:142:23
142| FragmentReference<{thing2: true}>,
^^^^^^^^^^^^^^ [1]

References:
more.js:13:42
13| declare opaque type FragmentReference<T: FragmentTypeof>;
^^^^^^^^^^^^^^ [2]


Error --------------------------------------------------------------------------------------------------- more.js:153:20

Cannot create `PluralTest` element because null [1] is incompatible with read-only array type [2] in property `users`.
Expand Down Expand Up @@ -428,4 +458,4 @@ References:



Found 26 errors
Found 28 errors
16 changes: 15 additions & 1 deletion tests/types_first_reasons/types_first_reasons.exp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,20 @@ References:
^^ [2]


Error ---------------------------------------------------------------------------------------- instantiation_err.js:2:17

Cannot instantiate `T` because `T` [1] is not a polymorphic type. [incompatible-use]

instantiation_err.js:2:17
2| export type U = T<string>; // Err: cannot instantiate
^^^^^^^^^

References:
type_exports.js:1:13
1| export type T = string;
^ [1]


Error ---------------------------------------------------------------------------------------- instantiation_err.js:2:17

Cannot instantiate `T` because `T` [1] is not a polymorphic type. (FLOW BUG: This is a misplaced error. The original
Expand Down Expand Up @@ -139,4 +153,4 @@ References:



Found 9 errors
Found 10 errors

0 comments on commit 514f690

Please sign in to comment.