From caebbb3e682ab0b4ae95755b1f59a1b0a4e1605e Mon Sep 17 00:00:00 2001
From: Danny Su <dsc@meta.com>
Date: Wed, 8 Jan 2025 16:32:03 -0800
Subject: [PATCH] Improve match transform logic for determining if an 'in'
 check is needed for an object property's pattern

Summary:
Original Author: gkz@meta.com
Original Git: a62edf188d7dc52ac1ffd3e8543b3b25d9d07feb
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
---
 .../__tests__/MatchExpression-test.js         | 35 +++++++++++++++
 .../src/estree/TransformMatchSyntax.js        | 45 +++++++++++++++----
 2 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/tools/hermes-parser/js/hermes-parser/__tests__/MatchExpression-test.js b/tools/hermes-parser/js/hermes-parser/__tests__/MatchExpression-test.js
index d859b6aa1b4..f1788cb2b2b 100644
--- a/tools/hermes-parser/js/hermes-parser/__tests__/MatchExpression-test.js
+++ b/tools/hermes-parser/js/hermes-parser/__tests__/MatchExpression-test.js
@@ -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) {
diff --git a/tools/hermes-parser/js/hermes-parser/src/estree/TransformMatchSyntax.js b/tools/hermes-parser/js/hermes-parser/src/estree/TransformMatchSyntax.js
index 128934dece2..6c727df492c 100644
--- a/tools/hermes-parser/js/hermes-parser/src/estree/TransformMatchSyntax.js
+++ b/tools/hermes-parser/js/hermes-parser/src/estree/TransformMatchSyntax.js
@@ -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.
@@ -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);