Skip to content

Commit 636f584

Browse files
kwonojsokra
kwonoj
and
sokra
authored
fix(ecmascript): eval assignop to the ident (vercel#4609)
### Description - closes WEB-889 When there is a variable with `+=` assignop only to the given ident ``` var css = ''; for (var i = 0; i < length; i++) { var partRule = this.rules[i]; if (typeof partRule === 'string') { css += partRule; } else if (partRule) { css += partString; } } if (css) { .... } ``` evaluated result becomes ``` var css = ''; for(var i = 0; i < length; i++){ var partRule = this.rules[i]; if (typeof partRule === 'string') { css += partRule; } else if (partRule) { css += partString; } } if ("TURBOPACK compile-time falsy", 0) { "TURBOPACK unreachable"; } ``` so `if (css)` condition never executed even though it can be executed depends on the runtime assignop condition. PR attempts to evaluate if assignop is for the ident to prevent this. --------- Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
1 parent a6773ee commit 636f584

23 files changed

+3365
-2427
lines changed

‎crates/turbopack-ecmascript/src/analyzer/graph.rs

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -320,21 +320,23 @@ impl EvalContext {
320320
JsValue::concat(values)
321321
}
322322

323+
fn eval_ident(&self, i: &Ident) -> JsValue {
324+
let id = i.to_id();
325+
if let Some(imported) = self.imports.get_import(&id) {
326+
return imported;
327+
}
328+
if is_unresolved(i, self.unresolved_mark) {
329+
JsValue::FreeVar(i.sym.clone())
330+
} else {
331+
JsValue::Variable(id)
332+
}
333+
}
334+
323335
pub fn eval(&self, e: &Expr) -> JsValue {
324336
match e {
325337
Expr::Paren(e) => self.eval(&e.expr),
326338
Expr::Lit(e) => JsValue::Constant(e.clone().into()),
327-
Expr::Ident(i) => {
328-
let id = i.to_id();
329-
if let Some(imported) = self.imports.get_import(&id) {
330-
return imported;
331-
}
332-
if is_unresolved(i, self.unresolved_mark) {
333-
JsValue::FreeVar(i.sym.clone())
334-
} else {
335-
JsValue::Variable(id)
336-
}
337-
}
339+
Expr::Ident(i) => self.eval_ident(i),
338340

339341
Expr::Unary(UnaryExpr {
340342
op: op!("!"), arg, ..
@@ -943,6 +945,29 @@ impl VisitAstPath for Analyzer<'_> {
943945
AstParentNodeRef::AssignExpr(n, AssignExprField::Left),
944946
|ast_path| match &n.left {
945947
PatOrExpr::Expr(expr) => {
948+
if let Some(key) = expr.as_ident() {
949+
let value = match n.op {
950+
AssignOp::Assign => unreachable!(
951+
"AssignOp::Assign will never have an expression in n.left"
952+
),
953+
AssignOp::AndAssign | AssignOp::OrAssign | AssignOp::NullishAssign => {
954+
let right = self.eval_context.eval(&n.right);
955+
// We can handle the right value as alternative to the existing
956+
// value
957+
Some(right)
958+
}
959+
AssignOp::AddAssign => {
960+
let left = self.eval_context.eval(expr);
961+
let right = self.eval_context.eval(&n.right);
962+
Some(JsValue::add(vec![left, right]))
963+
}
964+
_ => Some(JsValue::unknown_empty("unsupported assign operation")),
965+
};
966+
if let Some(value) = value {
967+
self.add_value(key.to_id(), value);
968+
}
969+
}
970+
946971
ast_path.with(
947972
AstParentNodeRef::PatOrExpr(&n.left, PatOrExprField::Expr),
948973
|ast_path| {
@@ -951,6 +976,7 @@ impl VisitAstPath for Analyzer<'_> {
951976
);
952977
}
953978
PatOrExpr::Pat(pat) => {
979+
debug_assert!(n.op == AssignOp::Assign);
954980
ast_path.with(
955981
AstParentNodeRef::PatOrExpr(&n.left, PatOrExprField::Pat),
956982
|ast_path| {
@@ -970,6 +996,26 @@ impl VisitAstPath for Analyzer<'_> {
970996
);
971997
}
972998

999+
fn visit_update_expr<'ast: 'r, 'r>(
1000+
&mut self,
1001+
n: &'ast UpdateExpr,
1002+
ast_path: &mut AstNodePath<AstParentNodeRef<'r>>,
1003+
) {
1004+
if let Some(key) = n.arg.as_ident() {
1005+
self.add_value(
1006+
key.to_id(),
1007+
JsValue::unknown_empty("updated with update expression"),
1008+
);
1009+
}
1010+
1011+
ast_path.with(
1012+
AstParentNodeRef::UpdateExpr(n, UpdateExprField::Arg),
1013+
|ast_path| {
1014+
self.visit_expr(&n.arg, ast_path);
1015+
},
1016+
);
1017+
}
1018+
9731019
fn visit_call_expr<'ast: 'r, 'r>(
9741020
&mut self,
9751021
n: &'ast CallExpr,

‎crates/turbopack-ecmascript/tests/analyzer/graph/assign/graph-effects.snapshot

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[]

‎crates/turbopack-ecmascript/tests/analyzer/graph/assign/graph-explained.snapshot

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
a = ("" | `${a}x`)
2+
3+
b = (0 | ???*0*)
4+
- *0* unsupported assign operation
5+
6+
c = (0 | ???*0*)
7+
- *0* updated with update expression
8+
9+
d = (0 | 1)
10+
11+
e = (1 | 2)
12+
13+
f = (1 | 2)

‎crates/turbopack-ecmascript/tests/analyzer/graph/assign/graph.snapshot

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
[
2+
(
3+
"a",
4+
Alternatives(
5+
5,
6+
[
7+
Constant(
8+
Str(
9+
Word(
10+
Atom('' type=static),
11+
),
12+
),
13+
),
14+
Concat(
15+
3,
16+
[
17+
Variable(
18+
(
19+
Atom('a' type=static),
20+
#2,
21+
),
22+
),
23+
Constant(
24+
Str(
25+
Word(
26+
Atom('x' type=static),
27+
),
28+
),
29+
),
30+
],
31+
),
32+
],
33+
),
34+
),
35+
(
36+
"b",
37+
Alternatives(
38+
3,
39+
[
40+
Constant(
41+
Num(
42+
ConstantNumber(
43+
0.0,
44+
),
45+
),
46+
),
47+
Unknown(
48+
None,
49+
"unsupported assign operation",
50+
),
51+
],
52+
),
53+
),
54+
(
55+
"c",
56+
Alternatives(
57+
3,
58+
[
59+
Constant(
60+
Num(
61+
ConstantNumber(
62+
0.0,
63+
),
64+
),
65+
),
66+
Unknown(
67+
None,
68+
"updated with update expression",
69+
),
70+
],
71+
),
72+
),
73+
(
74+
"d",
75+
Alternatives(
76+
3,
77+
[
78+
Constant(
79+
Num(
80+
ConstantNumber(
81+
0.0,
82+
),
83+
),
84+
),
85+
Constant(
86+
Num(
87+
ConstantNumber(
88+
1.0,
89+
),
90+
),
91+
),
92+
],
93+
),
94+
),
95+
(
96+
"e",
97+
Alternatives(
98+
3,
99+
[
100+
Constant(
101+
Num(
102+
ConstantNumber(
103+
1.0,
104+
),
105+
),
106+
),
107+
Constant(
108+
Num(
109+
ConstantNumber(
110+
2.0,
111+
),
112+
),
113+
),
114+
],
115+
),
116+
),
117+
(
118+
"f",
119+
Alternatives(
120+
3,
121+
[
122+
Constant(
123+
Num(
124+
ConstantNumber(
125+
1.0,
126+
),
127+
),
128+
),
129+
Constant(
130+
Num(
131+
ConstantNumber(
132+
2.0,
133+
),
134+
),
135+
),
136+
],
137+
),
138+
),
139+
]

‎crates/turbopack-ecmascript/tests/analyzer/graph/assign/input.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
let a = "";
2+
a += "x";
3+
let b = 0;
4+
b -= 1;
5+
let c = 0;
6+
c++;
7+
let d = 0;
8+
d ||= 1;
9+
let e = 1;
10+
e &&= 2;
11+
let f = 1;
12+
f ??= 2;

‎crates/turbopack-ecmascript/tests/analyzer/graph/assign/resolved-effects.snapshot

Whitespace-only changes.

‎crates/turbopack-ecmascript/tests/analyzer/graph/assign/resolved-explained.snapshot

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
a = ("" | `${("" | ???*0*)}x`)
2+
- *0* `${???*1*}x`
3+
⚠️ nested operation
4+
- *1* a
5+
⚠️ circular variable reference
6+
7+
b = (0 | ???*0*)
8+
- *0* unsupported assign operation
9+
10+
c = (0 | ???*0*)
11+
- *0* updated with update expression
12+
13+
d = (0 | 1)
14+
15+
e = (1 | 2)
16+
17+
f = (1 | 2)

‎crates/turbopack-ecmascript/tests/analyzer/graph/md5-reduced/graph-explained.snapshot

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ d#3 = (
5555

5656
d#6 = arguments[3]
5757

58-
i = (???*0* | 0)
58+
i = (???*0* | 0 | (i + 16))
5959
- *0* i
6060
⚠️ pattern without value
6161

‎crates/turbopack-ecmascript/tests/analyzer/graph/md5-reduced/graph.snapshot

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1269,7 +1269,7 @@
12691269
(
12701270
"i",
12711271
Alternatives(
1272-
3,
1272+
6,
12731273
[
12741274
Unknown(
12751275
Some(
@@ -1289,6 +1289,24 @@
12891289
),
12901290
),
12911291
),
1292+
Add(
1293+
3,
1294+
[
1295+
Variable(
1296+
(
1297+
Atom('i' type=static),
1298+
#3,
1299+
),
1300+
),
1301+
Constant(
1302+
Num(
1303+
ConstantNumber(
1304+
16.0,
1305+
),
1306+
),
1307+
),
1308+
],
1309+
),
12921310
],
12931311
),
12941312
),

‎crates/turbopack-ecmascript/tests/analyzer/graph/md5-reduced/resolved-explained.snapshot

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,15 @@ d#6 = ???*0*
3434
- *0* arguments[3]
3535
⚠️ function calls are not analysed yet
3636

37-
i = (???*0* | 0)
37+
i = (???*0* | 0 | ((???*1* | 0 | ???*2*) + 16))
3838
- *0* i
3939
⚠️ pattern without value
40+
- *1* i
41+
⚠️ pattern without value
42+
- *2* (???*3* + 16)
43+
⚠️ nested operation
44+
- *3* i
45+
⚠️ circular variable reference
4046

4147
len = ???*0*
4248
- *0* arguments[1]

0 commit comments

Comments
 (0)