Skip to content

Commit

Permalink
[flow][match] Improve match logic for determining if a "property exis…
Browse files Browse the repository at this point in the history 
…ts" refinement is needed for an object property's pattern

Summary:
This diff is the Flow side of the change made to the transform in D67777384.

We need to add a "property exists" refinement in some additional cases, where a possible check of `obj.prop === undefined` is possible, since that doesn't check that the property exists.

Changelog: [internal]

Reviewed By: panagosg7

Differential Revision: D70362252

fbshipit-source-id: fd2208d8aaf85e34458eaacb20f7f0b602d0f2fe
  • Loading branch information
@gkz @facebook-github-bot
gkz authored and facebook-github-bot committed Feb 28, 2025
1 parent fd1f9b0 commit f7d7bd4
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 63 deletions.
26 changes: 22 additions & 4 deletions src/analysis/env_builder/name_resolver.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3099,6 +3099,25 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
add_output (Error_message.EMatchInvalidPatternReference { loc; binding_reason })
| _ -> ()
in
let rec needs_prop_exists_refi = function
| (loc, WildcardPattern _)
| (loc, BindingPattern _)
| (loc, IdentifierPattern _)
| (loc, MemberPattern _) ->
Some loc
| (_, NumberPattern _)
| (_, BigIntPattern _)
| (_, StringPattern _)
| (_, BooleanPattern _)
| (_, NullPattern _)
| (_, UnaryPattern _)
| (_, ObjectPattern _)
| (_, ArrayPattern _) ->
None
| (_, AsPattern { AsPattern.pattern; _ }) -> needs_prop_exists_refi pattern
| (_, OrPattern { OrPattern.patterns; _ }) ->
Base.List.find_map patterns ~f:needs_prop_exists_refi
in
let rec recurse acc pattern bindings =
( if Flow_ast_utils.match_pattern_has_binding pattern then
let (loc, _) = pattern in
Expand Down Expand Up @@ -3279,15 +3298,14 @@ module Make (Context : C) (FlowAPIUtils : F with type cx = Context.t) :
let open Ast.Expression in
(loc, Member { Member._object = acc; property; comments = None })
in
(match pattern with
| (loc, WildcardPattern _)
| (loc, BindingPattern _) ->
(match needs_prop_exists_refi pattern with
| Some loc ->
(match RefinementKey.of_expression acc with
| Some key ->
let refi = PropExistsR { propname; loc } in
this#add_single_refinement key ~refining_locs:(L.LSet.singleton loc) refi
| None -> ())
| _ -> ());
| None -> ());
recurse member pattern bindings
)
in
Expand Down
118 changes: 59 additions & 59 deletions tests/match/match.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,172 +259,172 @@ References:
^ [1]


Error ----------------------------------------------------------------------------------------------- matching.js:202:14
Error ----------------------------------------------------------------------------------------------- matching.js:220:14

`match` is not exhaustively checked: object type [1] has not been fully checked against by the match patterns below.
[match-not-exhaustive]

matching.js:202:14
202| const e2 = match (x) { // ERROR: `type: 'baz'` not checked
matching.js:220:14
220| const e2 = match (x) { // ERROR: `type: 'baz'` not checked
^^^^^

References:
matching.js:193:20
193| | {type: 'baz', val: boolean};
matching.js:211:20
211| | {type: 'baz', val: boolean};
^^^^^^^^^^^^^^^^^^^^^^^^^^^ [1]


Error ----------------------------------------------------------------------------------------------- matching.js:251:14
Error ----------------------------------------------------------------------------------------------- matching.js:269:14

`match` is not exhaustively checked: object type [1] has not been fully checked against by the match patterns below.
[match-not-exhaustive]

matching.js:251:14
251| const e3 = match (x) { // ERROR: `type: 'bar', n: 2` not checked
matching.js:269:14
269| const e3 = match (x) { // ERROR: `type: 'bar', n: 2` not checked
^^^^^

References:
matching.js:236:20
236| | {type: 'bar', n: 2, val: boolean};
matching.js:254:20
254| | {type: 'bar', n: 2, val: boolean};
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [1]


Error ----------------------------------------------------------------------------------------------- matching.js:279:14
Error ----------------------------------------------------------------------------------------------- matching.js:297:14

`match` is not exhaustively checked: object type [1] has not been fully checked against by the match patterns below.
[match-not-exhaustive]

matching.js:279:14
279| const e2 = match (x) { // ERROR: `type: 'bar'` not checked
matching.js:297:14
297| const e2 = match (x) { // ERROR: `type: 'bar'` not checked
^^^^^

References:
matching.js:271:20
271| | {type: 'bar', val: string}
matching.js:289:20
289| | {type: 'bar', val: string}
^^^^^^^^^^^^^^^^^^^^^^^^^^ [1]


Error ----------------------------------------------------------------------------------------------- matching.js:297:14
Error ----------------------------------------------------------------------------------------------- matching.js:315:14

`match` is not exhaustively checked: tuple type [1] has not been fully checked against by the match patterns below.
[match-not-exhaustive]

matching.js:297:14
297| const e2 = match (x) { // ERROR: `'baz'` element not checked
matching.js:315:14
315| const e2 = match (x) { // ERROR: `'baz'` element not checked
^^^^^

References:
matching.js:288:20
288| | ['baz', boolean];
matching.js:306:20
306| | ['baz', boolean];
^^^^^^^^^^^^^^^^ [1]


Error ----------------------------------------------------------------------------------------------- matching.js:335:22
Error ----------------------------------------------------------------------------------------------- matching.js:353:22

Cannot cast `a` to empty because boolean [1] is incompatible with empty [2]. [incompatible-cast]

matching.js:335:22
335| [const a, _, _]: a as empty, // ERROR: `boolean` is not `empty`
matching.js:353:22
353| [const a, _, _]: a as empty, // ERROR: `boolean` is not `empty`
^

References:
matching.js:330:21
330| | [boolean, boolean, boolean];
matching.js:348:21
348| | [boolean, boolean, boolean];
^^^^^^^ [1]
matching.js:335:27
335| [const a, _, _]: a as empty, // ERROR: `boolean` is not `empty`
matching.js:353:27
353| [const a, _, _]: a as empty, // ERROR: `boolean` is not `empty`
^^^^^ [2]


Error ----------------------------------------------------------------------------------------------- matching.js:355:16
Error ----------------------------------------------------------------------------------------------- matching.js:373:16

Cannot cast `a` to string because number [1] is incompatible with string [2]. [incompatible-cast]

matching.js:355:16
355| [const a]: a as string, // ERROR: `number` is not `string`
matching.js:373:16
373| [const a]: a as string, // ERROR: `number` is not `string`
^

References:
matching.js:351:21
351| declare const x: [number] | Array<string>;
matching.js:369:21
369| declare const x: [number] | Array<string>;
^^^^^^ [1]
matching.js:355:21
355| [const a]: a as string, // ERROR: `number` is not `string`
matching.js:373:21
373| [const a]: a as string, // ERROR: `number` is not `string`
^^^^^^ [2]


Error ----------------------------------------------------------------------------------------------- matching.js:375:14
Error ----------------------------------------------------------------------------------------------- matching.js:393:14

`match` is not exhaustively checked: tuple type [1] has not been fully checked against by the match patterns below.
[match-not-exhaustive]

matching.js:375:14
375| const e2 = match (x) { // ERROR: does not match all possibilities
matching.js:393:14
393| const e2 = match (x) { // ERROR: does not match all possibilities
^^^^^

References:
matching.js:368:20
368| declare const x: [a: 0, b?: 1, c?: 2];
matching.js:386:20
386| declare const x: [a: 0, b?: 1, c?: 2];
^^^^^^^^^^^^^^^^^^^^ [1]


Error ----------------------------------------------------------------------------------------------- matching.js:379:14
Error ----------------------------------------------------------------------------------------------- matching.js:397:14

`match` is not exhaustively checked: tuple type [1] has not been fully checked against by the match patterns below.
[match-not-exhaustive]

matching.js:379:14
379| const e3 = match (x) { // ERROR: does not match all possibilities
matching.js:397:14
397| const e3 = match (x) { // ERROR: does not match all possibilities
^^^^^

References:
matching.js:368:20
368| declare const x: [a: 0, b?: 1, c?: 2];
matching.js:386:20
386| declare const x: [a: 0, b?: 1, c?: 2];
^^^^^^^^^^^^^^^^^^^^ [1]


Error ----------------------------------------------------------------------------------------------- matching.js:383:14
Error ----------------------------------------------------------------------------------------------- matching.js:401:14

`match` is not exhaustively checked: tuple type [1] has not been fully checked against by the match patterns below.
[match-not-exhaustive]

matching.js:383:14
383| const e4 = match (x) { // ERROR: does not match all possibilities
matching.js:401:14
401| const e4 = match (x) { // ERROR: does not match all possibilities
^^^^^

References:
matching.js:368:20
368| declare const x: [a: 0, b?: 1, c?: 2];
matching.js:386:20
386| declare const x: [a: 0, b?: 1, c?: 2];
^^^^^^^^^^^^^^^^^^^^ [1]


Error ----------------------------------------------------------------------------------------------- matching.js:397:14
Error ----------------------------------------------------------------------------------------------- matching.js:415:14

`match` is not exhaustively checked: tuple type [1] has not been fully checked against by the match patterns below.
[match-not-exhaustive]

matching.js:397:14
397| const e2 = match (x) { // ERROR: does not match all elements
matching.js:415:14
415| const e2 = match (x) { // ERROR: does not match all elements
^^^^^

References:
matching.js:390:20
390| declare const x: [a: 0, ...];
matching.js:408:20
408| declare const x: [a: 0, ...];
^^^^^^^^^^^ [1]


Error ----------------------------------------------------------------------------------------------- matching.js:407:14
Error ----------------------------------------------------------------------------------------------- matching.js:425:14

`match` is not exhaustively checked: object type [1] has not been fully checked against by the match patterns below.
[match-not-exhaustive]

matching.js:407:14
407| const e1 = match (x) {
matching.js:425:14
425| const e1 = match (x) {
^^^^^

References:
matching.js:404:23
404| type T = {foo: 1} | {foo: 2};
matching.js:422:23
422| type T = {foo: 1} | {foo: 2};
^^^^^^^^ [1]


Expand Down
18 changes: 18 additions & 0 deletions tests/match/matching.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,24 @@
const d: d as empty, // OK: all members checked
};
}
{
declare const x:
| {foo: void, a: 0}
| {bar: void, a: 1}
| {baz: void, a: 2}
| {zap: void, a: 3};

declare const u: void;
declare const o: {u: void};

const e1 = match (x) {
{foo: u, const a}: a as 0, // OK
{bar: o.u, const a}: a as 1, // OK
{baz: undefined as v, const a}: a as 2, // OK
{zap: 2 | u, const a}: a as 3, // OK
const d: d as empty, // OK: all members checked
};
}

// Disjoint object union
{
Expand Down

0 comments on commit f7d7bd4

Please sign in to comment.