Skip to content

Commit

Permalink
Improve match transform logic for determining if an 'in' check is nee…
Browse files Browse the repository at this point in the history
…ded for an object property's pattern

Summary:
The most basic example is:

```
const e = match (x) {
  {foo: undefined}: 0,
}
```

Previously we only outputted a check that `x.foo === undefined`, but if `foo` doesn't exist, this would still pass (e.g. input `{}`). To fix this, add an `in` check in that scenario. Also properly handle As and Or Patterns. Note that since an Identifier or Member Pattern may resolve to `undefined`, we add the `in` check for all those cases.

Reviewed By: alexmckenley

Differential Revision: D67777384

fbshipit-source-id: 4d348f6ea306c6aee741f1d5d939fb9e17227a63
  • Loading branch information
gkz authored and facebook-github-bot committed Jan 8, 2025
1 parent e9b0b8e commit a62edf1
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,41 @@ describe('MatchExpression', () => {
expect(runMatchExp(output, {foo: false, bar: 2})).toBe(1);
});

test('object property with `undefined` value', async () => {
const code = `
const e = match (x) {
{foo: undefined}: true,
{bar: undefined as const a}: true,
{baz: 0 | undefined}: true,
_: false,
};
`;
const output = await transform(code);
expect(output).toMatchInlineSnapshot(`
"const e = (() => {
if (typeof x === "object" && x !== null && "foo" in x && x.foo === undefined) {
return true;
}
if (typeof x === "object" && x !== null && "bar" in x && x.bar === undefined) {
const a = x.bar;
return true;
}
if (typeof x === "object" && x !== null && "baz" in x && (x.baz === 0 || x.baz === undefined)) {
return true;
}
return false;
})();"
`);

expect(runMatchExp(output, {})).toBe(false);
expect(runMatchExp(output, {foo: undefined})).toBe(true);
expect(runMatchExp(output, {bar: undefined})).toBe(true);
expect(runMatchExp(output, {baz: undefined})).toBe(true);
});

test('nested', async () => {
const code = `
const e = match (x) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,36 @@ function checkBindingKind(node: MatchPattern, kind: BindingKind): void {
}
}

/**
* Does an object property's pattern require a `prop-exists` condition added?
* If the pattern is a literal like `0`, then it's not required, since the `eq`
* condition implies the prop exists. However, if we could be doing an equality
* check against `undefined`, then it is required, since that will be true even
* if the property doesn't exist.
*/
function needsPropExistsCond(pattern: MatchPattern): boolean {
switch (pattern.type) {
case 'MatchWildcardPattern':
case 'MatchBindingPattern':
case 'MatchIdentifierPattern':
case 'MatchMemberPattern':
return true;
case 'MatchLiteralPattern':
case 'MatchUnaryPattern':
case 'MatchObjectPattern':
case 'MatchArrayPattern':
return false;
case 'MatchAsPattern': {
const {pattern: asPattern} = pattern;
return needsPropExistsCond(asPattern);
}
case 'MatchOrPattern': {
const {patterns} = pattern;
return patterns.some(needsPropExistsCond);
}
}
}

/**
* Analyzes a match pattern, and produced both the conditions and bindings
* produced by that pattern.
Expand Down Expand Up @@ -301,15 +331,12 @@ function analyzePattern(
}
seenNames.add(name);
const propKey: Key = key.concat(objKey);
switch (propPattern.type) {
case 'MatchWildcardPattern':
case 'MatchBindingPattern':
conditions.push({
type: 'prop-exists',
key,
propName: name,
});
break;
if (needsPropExistsCond(propPattern)) {
conditions.push({
type: 'prop-exists',
key,
propName: name,
});
}
const {conditions: childConditions, bindings: childBindings} =
analyzePattern(propPattern, propKey, seenBindingNames);
Expand Down

0 comments on commit a62edf1

Please sign in to comment.