From 9507493ee941ebd21641a845099160b9a9764e00 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Wed, 22 Mar 2023 14:36:50 -0700
Subject: [PATCH] Allow accessing properties of globals in switch test values

We limit the types of expressions allowed as switch case test values because we
our HIR doesn't yet preserve order-of-evaluation for switch test values (we
model them as being evaluated prior to entering the switch, as opposed to
lazily, when the case is reached). One common pattern internally is test case
values that are properties of a global, eg you have some bag of enum values and
are comparing against that:

```javascript

// at module scope, or imported from another module:

const OPTIONS = {FOO: 'foo'};

// in a component

switch (value) {

case OPTIONS.FOO: { ... }

}

```

This PR allows this specific case, ie member expressions where the innermost
object is a global identifier.
---
 compiler/forget/src/HIR/BuildHIR.ts           | 25 ++++++++++++++
 ...ch-global-propertyload-case-test.expect.md | 33 +++++++++++++++++++
 .../switch-global-propertyload-case-test.js   | 10 ++++++
 3 files changed, 68 insertions(+)
 create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/switch-global-propertyload-case-test.expect.md
 create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/switch-global-propertyload-case-test.js

diff --git a/compiler/forget/src/HIR/BuildHIR.ts b/compiler/forget/src/HIR/BuildHIR.ts
index 17a2a1c24c..620516f8e3 100644
--- a/compiler/forget/src/HIR/BuildHIR.ts
+++ b/compiler/forget/src/HIR/BuildHIR.ts
@@ -524,6 +524,31 @@ function lowerStatement(
               // ok
               break;
             }
+            case "MemberExpression": {
+              // A common pattern is switch statements where the case test values are properties of a global,
+              // eg `case ProductOptions.Option: { ... }`
+              // We therefore allow expressions where the innermost object is a global identifier, and reject
+              // all other member expressions (for now).
+              const test = testExpr as NodePath<t.MemberExpression>;
+              let innerObject: NodePath<t.Expression> = test;
+              while (innerObject.isMemberExpression()) {
+                innerObject = innerObject.get("object");
+              }
+              if (
+                innerObject.isIdentifier() &&
+                builder.resolveIdentifier(innerObject) === null // null means global
+              ) {
+                // This is a property/computed load from a global, that's safe to evaluate as a test expression
+                break;
+              }
+              builder.errors.push({
+                reason:
+                  "(BuildHIR::lowerStatement) Switch case test values must be identifiers or primitives, compound values are not yet supported",
+                severity: ErrorSeverity.Todo,
+                nodePath: testExpr,
+              });
+              break;
+            }
             default: {
               builder.errors.push({
                 reason:
diff --git a/compiler/forget/src/__tests__/fixtures/compiler/switch-global-propertyload-case-test.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/switch-global-propertyload-case-test.expect.md
new file mode 100644
index 0000000000..0134806f03
--- /dev/null
+++ b/compiler/forget/src/__tests__/fixtures/compiler/switch-global-propertyload-case-test.expect.md
@@ -0,0 +1,33 @@
+
+## Input
+
+```javascript
+function Component(props) {
+  switch (props.value) {
+    case Global.Property: {
+      return true;
+    }
+    default: {
+      return false;
+    }
+  }
+}
+
+```
+
+## Code
+
+```javascript
+function Component(props) {
+  switch (props.value) {
+    case Global.Property: {
+      return true;
+    }
+    default: {
+      return false;
+    }
+  }
+}
+
+```
+      
\ No newline at end of file
diff --git a/compiler/forget/src/__tests__/fixtures/compiler/switch-global-propertyload-case-test.js b/compiler/forget/src/__tests__/fixtures/compiler/switch-global-propertyload-case-test.js
new file mode 100644
index 0000000000..79e2707053
--- /dev/null
+++ b/compiler/forget/src/__tests__/fixtures/compiler/switch-global-propertyload-case-test.js
@@ -0,0 +1,10 @@
+function Component(props) {
+  switch (props.value) {
+    case Global.Property: {
+      return true;
+    }
+    default: {
+      return false;
+    }
+  }
+}