[hir] Disallow unconditional load from optional memberexpr
Mofei Zhang committed
Mar 30, 2023 at 19:23 UTC
c77c1b16445a5cb4207d8a44950c4aeed2bbee70
7 files changed
+94
-23
compiler/forget/src/HIR/BuildHIR.ts
+41
-13
@@ -1796,39 +1796,67 @@ function lowerMemberExpression(
1796
): { object: Place; property: Place | string; value: InstructionValue } {
1797
const exprNode = expr.node;
1798
const exprLoc = exprNode.loc ?? GeneratedSource;
1799
- const object = lowerExpressionToTemporary(builder, expr.get("object"));
1800
- const property = expr.get("property");
1799
+ const objectNode = expr.get("object");
1800
+ const propertyNode = expr.get("property");
1801
+ const object = lowerExpressionToTemporary(builder, objectNode);
1802
+
1803
+ if (
1804
+ objectNode.isOptionalMemberExpression() &&
1805
+ !expr.isOptionalMemberExpression()
1806
+ ) {
1807
+ // Babel's `isOptionalMemberExpression` indicates whether this property load itself
1808
+ // is conditional (i.e. within an "optional chain"). This is different from the
1809
+ // `optional` property, which is only true for property loads at the start of an
1810
+ // optional chain. e.g. `a.b?.c.d` decomposes into
1811
+ // [0] MemberExpr a.b; // not in an optional chain
1812
+ // [1] OptionalMemberExpr [0]?.c
1813
+ // [2] OptionalMemberExpr [1].c
1814
+ // [3] OptionalMemberExpr [2].d
1815
+ // We currently do not handle non-conditional loads from an optional memberexpr
1816
+ // e.g. `(a?.b).c`
1817
+ // See error.nonoptional-load-from-optional-memberexpr test fixture for details
1818
+ builder.errors.push({
1819
+ reason: `(BuildHIR::lowerMemberExpression) Handle optional chaining for non-optional member expr.`,
1820
+ severity: ErrorSeverity.Todo,
1821
+ nodePath: propertyNode,
1822
+ });
1823
+ return {
1824
+ object,
1825
+ property: propertyNode.toString(),
1826
+ value: { kind: "UnsupportedNode", node: exprNode, loc: exprLoc },
1827
+ };
1828
+ }
1829
if (!expr.node.computed) {
1802
- if (!property.isIdentifier()) {
1830
+ if (!propertyNode.isIdentifier()) {
1831
builder.errors.push({
1804
- reason: `(BuildHIR::lowerMemberExpression) Handle ${property.type} property`,
1832
+ reason: `(BuildHIR::lowerMemberExpression) Handle ${propertyNode.type} property`,
1833
severity: ErrorSeverity.Todo,
1806
- nodePath: property,
1834
+ nodePath: propertyNode,
1835
});
1836
return {
1837
object,
1810
- property: property.toString(),
1838
+ property: propertyNode.toString(),
1839
value: { kind: "UnsupportedNode", node: exprNode, loc: exprLoc },
1840
};
1841
}
1842
const value: InstructionValue = {
1843
kind: "PropertyLoad",
1844
object: { ...object },
1817
- property: property.node.name,
1845
+ property: propertyNode.node.name,
1846
loc: exprLoc,
1847
optional: expr.node.optional ?? false,
1848
};
1821
- return { object, property: property.node.name, value };
1849
+ return { object, property: propertyNode.node.name, value };
1850
} else {
1823
- if (!property.isExpression()) {
1851
+ if (!propertyNode.isExpression()) {
1852
builder.errors.push({
1825
- reason: `(BuildHIR::lowerMemberExpression) Expected Expression, got ${property.type} property`,
1853
+ reason: `(BuildHIR::lowerMemberExpression) Expected Expression, got ${propertyNode.type} property`,
1854
severity: ErrorSeverity.InvalidInput,
1827
- nodePath: property,
1855
+ nodePath: propertyNode,
1856
});
1857
return {
1858
object,
1831
- property: property.toString(),
1859
+ property: propertyNode.toString(),
1860
value: {
1861
kind: "UnsupportedNode",
1862
node: exprNode,
@@ -1843,7 +1871,7 @@ function lowerMemberExpression(
1871
nodePath: expr,
1872
});
1873
}
1846
- const propertyPlace = lowerExpressionToTemporary(builder, property);
1874
+ const propertyPlace = lowerExpressionToTemporary(builder, propertyNode);
1875
const value: InstructionValue = {
1876
kind: "ComputedLoad",
1877
object: { ...object },
compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+10
-5
@@ -752,17 +752,22 @@ function codegenInstructionValue(
752
break;
753
}
754
case "PropertyLoad": {
755
- if (instrValue.optional) {
755
+ const object = codegenPlace(cx, instrValue.object);
756
+ // We currently only lower single chains of optional memberexpr.
757
+ // (See BuildHIR.ts for more detail.)
758
+ if (t.isOptionalMemberExpression(object) || instrValue.optional) {
759
value = t.optionalMemberExpression(
757
- codegenPlace(cx, instrValue.object),
760
+ object,
761
t.identifier(instrValue.property),
762
undefined,
760
- true
763
+ instrValue.optional
764
);
765
} else {
766
value = t.memberExpression(
764
- codegenPlace(cx, instrValue.object),
765
- t.identifier(instrValue.property)
767
+ object,
768
+ t.identifier(instrValue.property),
769
+ undefined,
770
+ instrValue.optional
771
);
772
}
773
break;
compiler/forget/src/__tests__/fixtures/compiler/error.nonoptional-load-from-optional-memberexpr.expect.md
new
+30
@@ -0,0 +1,30 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// Note that `a?.b.c` is semantically different from `(a?.b).c`
6
+// Here, 'props?.a` is an optional chain, and `.b` is an unconditional load
7
+// (nullthrows if a is nullish)
8
+
9
+function Component(props) {
10
+ let x = (props?.a).b;
11
+ return x;
12
+}
13
+
14
+```
15
+
16
+
17
+## Error
18
+
19
+```
20
+[ReactForget] TodoError: (BuildHIR::lowerMemberExpression) Handle optional chaining for non-optional member expr.
21
+ 4 |
22
+ 5 | function Component(props) {
23
+> 6 | let x = (props?.a).b;
24
+ | ^
25
+ 7 | return x;
26
+ 8 | }
27
+ 9 |
28
+```
29
+
30
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/error.nonoptional-load-from-optional-memberexpr.js
new
+8
@@ -0,0 +1,8 @@
1
+// Note that `a?.b.c` is semantically different from `(a?.b).c`
2
+// Here, 'props?.a` is an optional chain, and `.b` is an unconditional load
3
+// (nullthrows if a is nullish)
4
+
5
+function Component(props) {
6
+ let x = (props?.a).b;
7
+ return x;
8
+}
compiler/forget/src/__tests__/fixtures/compiler/nested-optional-member-expr.expect.md
+1
-1
@@ -21,7 +21,7 @@ function Component(props) {
21
const c_0 = $[0] !== props.a;
22
let t0;
23
if (c_0) {
24
- t0 = foo((props.a?.b).c.d);
24
+ t0 = foo(props.a?.b.c.d);
25
$[0] = props.a;
26
$[1] = t0;
27
} else {
compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-chain.expect.md
renamed
+3
-3
@@ -6,7 +6,7 @@
6
// We should codegen the correct member expressions
7
function Component(props) {
8
let x = props?.b.c;
9
- let y = (props?.x).y;
9
+ let y = props?.b.c.d?.e.f.g?.h;
10
return { x, y };
11
}
12
@@ -19,8 +19,8 @@ function Component(props) {
19
// We should codegen the correct member expressions
20
function Component(props) {
21
const $ = React.unstable_useMemoCache(3);
22
- const x = (props?.b).c;
23
- const y = (props?.x).y;
22
+ const x = props?.b.c;
23
+ const y = props?.b.c.d?.e.f.g?.h;
24
const c_0 = $[0] !== x;
25
const c_1 = $[1] !== y;
26
let t0;
compiler/forget/src/__tests__/fixtures/compiler/optional-member-expression-chain.js
renamed
+1
-1
@@ -2,6 +2,6 @@
2
// We should codegen the correct member expressions
3
function Component(props) {
4
let x = props?.b.c;
5
- let y = (props?.x).y;
5
+ let y = props?.b.c.d?.e.f.g?.h;
6
return { x, y };
7
}