Skip to content

Commit

Permalink
[flow] Do not break up MaybeT, OptionalT on non-distributive conditio…
Browse files Browse the repository at this point in the history
…nal type

Summary:
MaybeT and OptionalT should not be destructed unless we explicitly decide that it should have the distributive behavior:

https://github.com/facebook/flow/blob/8e5ecac527cc76be12eae51a5219106dfbe8c69e/src/typing/flow_js.ml#L6554

https://github.com/facebook/flow/blob/8e5ecac527cc76be12eae51a5219106dfbe8c69e/src/typing/flow_js.ml#L6650-L6651

https://github.com/facebook/flow/blob/8e5ecac527cc76be12eae51a5219106dfbe8c69e/src/typing/flow_js.ml#L6659-L6660

In the main recursive flow_js, we did it correctly only for UnionT:

https://github.com/facebook/flow/blob/8e5ecac527cc76be12eae51a5219106dfbe8c69e/src/typing/flow_js.ml#L1715-L1722

but missed the case for `MaybeT` and `OptionalT`. This diff fixes that.

Changelog: [errors] Previously we incorrectly distribute-over-union for maybe or optional input types of conditional type. (e.g. `(?string) extends string ? true : false` is evaluated to `true | false` instead of `false`). This is now fixed, and code that depends on the bug might have new errors.

Reviewed By: panagosg7

Differential Revision: D67145501

fbshipit-source-id: 573933e56fef794ee29709645d48d4754aa874c5
  • Loading branch information
SamChou19815 authored and facebook-github-bot committed Dec 12, 2024
1 parent 1cd0a55 commit f543d19
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 3 deletions.
12 changes: 10 additions & 2 deletions src/typing/flow_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1152,7 +1152,11 @@ struct
rec_flow cx trace (t, u)
| (MaybeT _, ResolveUnionT { reason; resolved; unresolved; upper; id }) ->
resolve_union cx trace reason id resolved unresolved l upper
| (MaybeT (reason, t), _) ->
| (MaybeT (reason, t), _)
when match u with
| ConditionalT { distributive_tparam_name; _ } ->
Option.is_some distributive_tparam_name
| _ -> true ->
let reason = replace_desc_reason RNullOrVoid reason in
let t = push_type_alias_reason reason t in
rec_flow cx trace (NullT.make reason, u);
Expand Down Expand Up @@ -1197,7 +1201,11 @@ struct
resolve_union cx trace reason id resolved unresolved l upper
| (OptionalT { reason = _; type_ = t; use_desc = _ }, ExtractReactRefT _) ->
rec_flow cx trace (t, u)
| (OptionalT { reason = r; type_ = t; use_desc }, _) ->
| (OptionalT { reason = r; type_ = t; use_desc }, _)
when match u with
| ConditionalT { distributive_tparam_name; _ } ->
Option.is_some distributive_tparam_name
| _ -> true ->
let void = VoidT.why_with_use_desc ~use_desc r in
rec_flow cx trace (void, u);
rec_flow cx trace (t, u)
Expand Down
77 changes: 76 additions & 1 deletion tests/conditional_type/conditional_type.exp
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,81 @@ References:
^^^^^^ [2]


Error -------------------------------------------------------------------------- non_generic_maybe_nullable_tests.js:2:1

Cannot cast `true` to `MaybeStringExtendsString` because boolean [1] is incompatible with boolean literal `false` [2].
[incompatible-cast]

non_generic_maybe_nullable_tests.js:2:1
2| true as MaybeStringExtendsString; // error
^^^^ [1]

References:
non_generic_maybe_nullable_tests.js:2:9
2| true as MaybeStringExtendsString; // error
^^^^^^^^^^^^^^^^^^^^^^^^ [2]


Error -------------------------------------------------------------------------- non_generic_maybe_nullable_tests.js:7:3

Cannot cast `true` to `OptionalStringExtendsString` because boolean [1] is incompatible with boolean literal
`false` [2]. [incompatible-cast]

non_generic_maybe_nullable_tests.js:7:3
7| true as OptionalStringExtendsString; // error
^^^^ [1]

References:
non_generic_maybe_nullable_tests.js:7:11
7| true as OptionalStringExtendsString; // error
^^^^^^^^^^^^^^^^^^^^^^^^^^^ [2]


Error ------------------------------------------------------------------------- non_generic_maybe_nullable_tests.js:13:1

Cannot cast `false` to `StringExtendsMaybeString` because boolean [1] is incompatible with boolean literal `true` [2].
[incompatible-cast]

non_generic_maybe_nullable_tests.js:13:1
13| false as StringExtendsMaybeString; // error
^^^^^ [1]

References:
non_generic_maybe_nullable_tests.js:13:10
13| false as StringExtendsMaybeString; // error
^^^^^^^^^^^^^^^^^^^^^^^^ [2]


Error ------------------------------------------------------------------------- non_generic_maybe_nullable_tests.js:16:1

Cannot cast `true` to `StringOrNullOrVoidExtendsString` because boolean [1] is incompatible with boolean literal
`false` [2]. [incompatible-cast]

non_generic_maybe_nullable_tests.js:16:1
16| true as StringOrNullOrVoidExtendsString; // error
^^^^ [1]

References:
non_generic_maybe_nullable_tests.js:16:9
16| true as StringOrNullOrVoidExtendsString; // error
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [2]


Error ------------------------------------------------------------------------- non_generic_maybe_nullable_tests.js:21:1

Cannot cast `false` to `StringExtendsStringOrNullOrVoid` because boolean [1] is incompatible with boolean literal
`true` [2]. [incompatible-cast]

non_generic_maybe_nullable_tests.js:21:1
21| false as StringExtendsStringOrNullOrVoid; // error
^^^^^ [1]

References:
non_generic_maybe_nullable_tests.js:21:10
21| false as StringExtendsStringOrNullOrVoid; // error
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [2]


Error ---------------------------------------------------------------------------------------------- poly_t_input.js:5:1

Cannot cast `v1` to empty because boolean literal `false` [1] is incompatible with empty [2]. [incompatible-cast]
Expand Down Expand Up @@ -1087,7 +1162,7 @@ References:



Found 70 errors
Found 75 errors

Only showing the most relevant union/intersection branches.
To see all branches, re-run Flow with --show-all-branches
21 changes: 21 additions & 0 deletions tests/conditional_type/non_generic_maybe_nullable_tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
type MaybeStringExtendsString = (?string) extends string ? true : false;
true as MaybeStringExtendsString; // error
false as MaybeStringExtendsString; // ok

function optionalTypeTest(x?: string) {
type OptionalStringExtendsString = (typeof x) extends string ? true : false;
true as OptionalStringExtendsString; // error
false as OptionalStringExtendsString; // ok
}

type StringExtendsMaybeString = string extends (?string) ? true : false;
true as StringExtendsMaybeString; // ok
false as StringExtendsMaybeString; // error

type StringOrNullOrVoidExtendsString = (string | null | void) extends string ? true : false;
true as StringOrNullOrVoidExtendsString; // error
false as StringOrNullOrVoidExtendsString; // ok

type StringExtendsStringOrNullOrVoid = string extends (string | null | void) ? true : false;
true as StringExtendsStringOrNullOrVoid; // ok
false as StringExtendsStringOrNullOrVoid; // error

0 comments on commit f543d19

Please sign in to comment.