Skip to content

Commit caebbb3

Browse files
dannysufacebook-github-bot
authored andcommitted
Improve match transform logic for determining if an 'in' check is needed for an object property's pattern
Summary: Original Author: [email protected] Original Git: a62edf1 Original Reviewed By: alexmckenley Original Revision: D67777384 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: fbmal7 Differential Revision: D67946144 fbshipit-source-id: aedc7ab25c13231dd83b87ed729245f4f9959c48
1 parent d6c8fe6 commit caebbb3

File tree

2 files changed

+71
-9
lines changed

2 files changed

+71
-9
lines changed

tools/hermes-parser/js/hermes-parser/__tests__/MatchExpression-test.js

+35
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,41 @@ describe('MatchExpression', () => {
705705
expect(runMatchExp(output, {foo: false, bar: 2})).toBe(1);
706706
});
707707

708+
test('object property with `undefined` value', async () => {
709+
const code = `
710+
const e = match (x) {
711+
{foo: undefined}: true,
712+
{bar: undefined as const a}: true,
713+
{baz: 0 | undefined}: true,
714+
_: false,
715+
};
716+
`;
717+
const output = await transform(code);
718+
expect(output).toMatchInlineSnapshot(`
719+
"const e = (() => {
720+
if (typeof x === "object" && x !== null && "foo" in x && x.foo === undefined) {
721+
return true;
722+
}
723+
724+
if (typeof x === "object" && x !== null && "bar" in x && x.bar === undefined) {
725+
const a = x.bar;
726+
return true;
727+
}
728+
729+
if (typeof x === "object" && x !== null && "baz" in x && (x.baz === 0 || x.baz === undefined)) {
730+
return true;
731+
}
732+
733+
return false;
734+
})();"
735+
`);
736+
737+
expect(runMatchExp(output, {})).toBe(false);
738+
expect(runMatchExp(output, {foo: undefined})).toBe(true);
739+
expect(runMatchExp(output, {bar: undefined})).toBe(true);
740+
expect(runMatchExp(output, {baz: undefined})).toBe(true);
741+
});
742+
708743
test('nested', async () => {
709744
const code = `
710745
const e = match (x) {

tools/hermes-parser/js/hermes-parser/src/estree/TransformMatchSyntax.js

+36-9
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,36 @@ function checkBindingKind(node: MatchPattern, kind: BindingKind): void {
175175
}
176176
}
177177

178+
/**
179+
* Does an object property's pattern require a `prop-exists` condition added?
180+
* If the pattern is a literal like `0`, then it's not required, since the `eq`
181+
* condition implies the prop exists. However, if we could be doing an equality
182+
* check against `undefined`, then it is required, since that will be true even
183+
* if the property doesn't exist.
184+
*/
185+
function needsPropExistsCond(pattern: MatchPattern): boolean {
186+
switch (pattern.type) {
187+
case 'MatchWildcardPattern':
188+
case 'MatchBindingPattern':
189+
case 'MatchIdentifierPattern':
190+
case 'MatchMemberPattern':
191+
return true;
192+
case 'MatchLiteralPattern':
193+
case 'MatchUnaryPattern':
194+
case 'MatchObjectPattern':
195+
case 'MatchArrayPattern':
196+
return false;
197+
case 'MatchAsPattern': {
198+
const {pattern: asPattern} = pattern;
199+
return needsPropExistsCond(asPattern);
200+
}
201+
case 'MatchOrPattern': {
202+
const {patterns} = pattern;
203+
return patterns.some(needsPropExistsCond);
204+
}
205+
}
206+
}
207+
178208
/**
179209
* Analyzes a match pattern, and produced both the conditions and bindings
180210
* produced by that pattern.
@@ -301,15 +331,12 @@ function analyzePattern(
301331
}
302332
seenNames.add(name);
303333
const propKey: Key = key.concat(objKey);
304-
switch (propPattern.type) {
305-
case 'MatchWildcardPattern':
306-
case 'MatchBindingPattern':
307-
conditions.push({
308-
type: 'prop-exists',
309-
key,
310-
propName: name,
311-
});
312-
break;
334+
if (needsPropExistsCond(propPattern)) {
335+
conditions.push({
336+
type: 'prop-exists',
337+
key,
338+
propName: name,
339+
});
313340
}
314341
const {conditions: childConditions, bindings: childBindings} =
315342
analyzePattern(propPattern, propKey, seenBindingNames);

0 commit comments

Comments
 (0)