Skip to content

Commit

Permalink
fix(es/fixer): Wrap in expr in for-in head (#9209)
Browse files Browse the repository at this point in the history
**Related issue:**

- Closes #9200
  • Loading branch information
magic-akari authored Jul 13, 2024
1 parent 78b2964 commit 5cd837f
Show file tree
Hide file tree
Showing 10 changed files with 225 additions and 32 deletions.
20 changes: 20 additions & 0 deletions crates/swc/tests/fixture/issues-9xxx/9200/input/.swcrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"jsc": {
"parser": {
"syntax": "typescript",
"tsx": true,
"decorators": true
},
"loose": false,
"minify": {
"compress": false,
"mangle": false
},
"target": "es2022"
},
"module": {
"type": "es6"
},
"minify": false,
"isModule": false
}
3 changes: 3 additions & 0 deletions crates/swc/tests/fixture/issues-9xxx/9200/input/1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
let count = 0;
for (var a = 1 || (2 in {}) in { x: 1 }) count++;
console.log(count);
47 changes: 47 additions & 0 deletions crates/swc/tests/fixture/issues-9xxx/9200/input/2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
for (var a = (b in c) in {});
for (var a = 1 || (b in c) in {});
for (var a = 1 + (2 || (b in c)) in {});
for (var a = (() => b in c) in {});
for (var a = 1 || (() => b in c) in {});
for (var a = (() => { b in c; }) in {});
for (var a = [b in c] in {});
for (var a = {b: b in c} in {});
for (var a = (x = b in c) => {} in {});
for (var a = class extends (b in c) {} in {});
for (var a = function (x = b in c) {} in {});

for (var a = (b in c);;);
for (var a = 1 || (b in c);;);
for (var a = 1 + (2 || (b in c));;);
for (var a = (() => b in c);;);
for (var a = 1 || (() => b in c);;);
for (var a = (() => { b in c; });;);
for (var a = [b in c];;);
for (var a = {b: b in c};;);
for (var a = (x = b in c) => {};;);
for (var a = class extends (b in c) {};;);
for (var a = function (x = b in c) {};;);

for (var a in (b in c));
for (var a in 1 || (b in c));
for (var a in 1 + (2 || (b in c)));
for (var a in (() => b in c));
for (var a in 1 || (() => b in c));
for (var a in (() => { b in c; }));
for (var a in [b in c]);
for (var a in {b: b in c});
for (var a in (x = b in c) => {});
for (var a in class extends (b in c) {});
for (var a in function (x = b in c) {});

for (;a = (b in c););
for (;a = 1 || (b in c););
for (;a = 1 + (2 || (b in c)););
for (;a = (() => b in c););
for (;a = 1 || (() => b in c););
for (;a = (() => { b in c; }););
for (;a = [b in c];);
for (;a = {b: b in c};);
for (;a = (x = b in c) => {};);
for (;a = class extends (b in c) {};);
for (;a = function (x = b in c) {};);
5 changes: 5 additions & 0 deletions crates/swc/tests/fixture/issues-9xxx/9200/output/1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
let count = 0;
for(var a = 1 || (2 in {}) in {
x: 1
})count++;
console.log(count);
72 changes: 72 additions & 0 deletions crates/swc/tests/fixture/issues-9xxx/9200/output/2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
for(var a = (b in c) in {});
for(var a = 1 || (b in c) in {});
for(var a = 1 + (2 || (b in c)) in {});
for(var a = ()=>(b in c) in {});
for(var a = 1 || (()=>(b in c)) in {});
for(var a = ()=>{
b in c;
} in {});
for(var a = [
b in c
] in {});
for(var a = {
b: b in c
} in {});
for(var a = (x = b in c)=>{} in {});
for(var a = class extends (b in c) {
} in {});
for(var a = function(x = b in c) {} in {});
for(var a = (b in c);;);
for(var a = 1 || (b in c);;);
for(var a = 1 + (2 || (b in c));;);
for(var a = ()=>(b in c);;);
for(var a = 1 || (()=>(b in c));;);
for(var a = ()=>{
b in c;
};;);
for(var a = [
b in c
];;);
for(var a = {
b: b in c
};;);
for(var a = (x = b in c)=>{};;);
for(var a = class extends (b in c) {
};;);
for(var a = function(x = b in c) {};;);
for(var a in b in c);
for(var a in 1 || b in c);
for(var a in 1 + (2 || b in c));
for(var a in ()=>b in c);
for(var a in 1 || (()=>b in c));
for(var a in ()=>{
b in c;
});
for(var a in [
b in c
]);
for(var a in {
b: b in c
});
for(var a in (x = b in c)=>{});
for(var a in class extends (b in c) {
});
for(var a in function(x = b in c) {});
for(; a = b in c;);
for(; a = 1 || b in c;);
for(; a = 1 + (2 || b in c););
for(; a = ()=>b in c;);
for(; a = 1 || (()=>b in c););
for(; a = ()=>{
b in c;
};);
for(; a = [
b in c
];);
for(; a = {
b: b in c
};);
for(; a = (x = b in c)=>{};);
for(; a = class extends (b in c) {
};);
for(; a = function(x = b in c) {};);
2 changes: 1 addition & 1 deletion crates/swc_ecma_minifier/tests/benches-full/echarts.js
Original file line number Diff line number Diff line change
Expand Up @@ -6154,7 +6154,7 @@
needDrawBg && this._renderBackground(style, style, boxX, boxY, outerWidth_1, outerHeight);
}
textY += lineHeight / 2, textPadding && (textX = getTextXForPadding(baseX, textAlign, textPadding), 'top' === verticalAlign ? textY += textPadding[0] : 'bottom' === verticalAlign && (textY -= textPadding[2]));
for(var defaultLineWidth = 0, useDefaultFill = !1, textFill = null == (fill = ('fill' in style) ? style.fill : (useDefaultFill = !0, defaultStyle.fill)) || 'none' === fill ? null : fill.image || fill.colorStops ? '#000' : fill, textStroke = getStroke(('stroke' in style) ? style.stroke : bgColorDrawn || defaultStyle.autoStroke && !useDefaultFill ? null : (defaultLineWidth = 2, defaultStyle.stroke)), hasShadow = style.textShadowBlur > 0, fixedBoundingRect = null != style.width && ('truncate' === style.overflow || 'break' === style.overflow || 'breakAll' === style.overflow), calculatedLineHeight = contentBlock.calculatedLineHeight, i = 0; i < textLines.length; i++){
for(var defaultLineWidth = 0, useDefaultFill = !1, textFill = null == (fill = ('fill' in style) ? style.fill : (useDefaultFill = !0, defaultStyle.fill)) || 'none' === fill ? null : fill.image || fill.colorStops ? '#000' : fill, textStroke = getStroke('stroke' in style ? style.stroke : bgColorDrawn || defaultStyle.autoStroke && !useDefaultFill ? null : (defaultLineWidth = 2, defaultStyle.stroke)), hasShadow = style.textShadowBlur > 0, fixedBoundingRect = null != style.width && ('truncate' === style.overflow || 'break' === style.overflow || 'breakAll' === style.overflow), calculatedLineHeight = contentBlock.calculatedLineHeight, i = 0; i < textLines.length; i++){
var el = this._getOrCreateChild(TSpan), subElStyle = el.createStyle();
el.useStyle(subElStyle), subElStyle.text = textLines[i], subElStyle.x = textX, subElStyle.y = textY, textAlign && (subElStyle.textAlign = textAlign), subElStyle.textBaseline = 'middle', subElStyle.opacity = style.opacity, subElStyle.strokeFirst = !0, hasShadow && (subElStyle.shadowBlur = style.textShadowBlur || 0, subElStyle.shadowColor = style.textShadowColor || 'transparent', subElStyle.shadowOffsetX = style.textShadowOffsetX || 0, subElStyle.shadowOffsetY = style.textShadowOffsetY || 0), textStroke && (subElStyle.stroke = textStroke, subElStyle.lineWidth = style.lineWidth || defaultLineWidth, subElStyle.lineDash = style.lineDash, subElStyle.lineDashOffset = style.lineDashOffset || 0), textFill && (subElStyle.fill = textFill), subElStyle.font = textFont, textY += lineHeight, fixedBoundingRect && el.setBoundingRect(new BoundingRect(adjustTextX(subElStyle.x, style.width, subElStyle.textAlign), adjustTextY(subElStyle.y, calculatedLineHeight, subElStyle.textBaseline), style.width, calculatedLineHeight));
}
Expand Down
4 changes: 2 additions & 2 deletions crates/swc_ecma_minifier/tests/benches-full/jquery.js
Original file line number Diff line number Diff line change
Expand Up @@ -3125,9 +3125,9 @@
for(!function(props, specialEasing) {
var index, name, easing, value, hooks;
// camelCase, specialEasing and expand cssHook pass
for(index in props)if (easing = specialEasing[name = camelCase(index)], Array.isArray(value = props[index]) && (easing = value[1], value = props[index] = value[0]), index !== name && (props[name] = value, delete props[index]), (hooks = jQuery.cssHooks[name]) && ("expand" in hooks)) // Not quite $.extend, this won't overwrite existing keys.
for(index in props)if (easing = specialEasing[name = camelCase(index)], Array.isArray(value = props[index]) && (easing = value[1], value = props[index] = value[0]), index !== name && (props[name] = value, delete props[index]), (hooks = jQuery.cssHooks[name]) && "expand" in hooks) // Not quite $.extend, this won't overwrite existing keys.
// Reusing 'index' because we have the correct "name"
for(index in value = hooks.expand(value), delete props[name], value)(index in props) || (props[index] = value[index], specialEasing[index] = easing);
for(index in value = hooks.expand(value), delete props[name], value)index in props || (props[index] = value[index], specialEasing[index] = easing);
else specialEasing[name] = easing;
}(props, animation.opts.specialEasing); index < length; index++)if (result = Animation.prefilters[index].call(animation, elem, props, animation.opts)) return isFunction(result.stop) && (jQuery._queueHooks(animation.elem, animation.opts.queue).stop = result.stop.bind(result)), result;
return jQuery.map(props, createTween, animation), isFunction(animation.opts.start) && animation.opts.start.call(elem, animation), // Attach callbacks from options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8201,14 +8201,14 @@
for(var RegExpWrapper = function(pattern, flags) {
var rawFlags, dotAll, sticky, handled, result, state, thisIsRegExp = this instanceof RegExpWrapper, patternIsRegExp = isRegExp(pattern), flagsAreUndefined = void 0 === flags, groups = [], rawPattern = pattern;
if (!thisIsRegExp && patternIsRegExp && flagsAreUndefined && pattern.constructor === RegExpWrapper) return pattern;
if ((patternIsRegExp || pattern instanceof RegExpWrapper) && (pattern = pattern.source, flagsAreUndefined && (flags = ("flags" in rawPattern) ? rawPattern.flags : getFlags.call(rawPattern))), pattern = void 0 === pattern ? "" : toString1(pattern), flags = void 0 === flags ? "" : toString1(flags), rawPattern = pattern, UNSUPPORTED_DOT_ALL && ("dotAll" in re1) && (dotAll = !!flags && flags.indexOf("s") > -1) && (flags = flags.replace(/s/g, "")), rawFlags = flags, UNSUPPORTED_Y && ("sticky" in re1) && (sticky = !!flags && flags.indexOf("y") > -1) && (flags = flags.replace(/y/g, "")), UNSUPPORTED_NCG && (pattern = (handled = handleNCG(pattern))[0], groups = handled[1]), result = inheritIfRequired(NativeRegExp(pattern, flags), thisIsRegExp ? this : RegExpPrototype, RegExpWrapper), (dotAll || sticky || groups.length) && (state = enforceInternalState(result), dotAll && (state.dotAll = !0, state.raw = RegExpWrapper(handleDotAll(pattern), rawFlags)), sticky && (state.sticky = !0), groups.length && (state.groups = groups)), pattern !== rawPattern) try {
if ((patternIsRegExp || pattern instanceof RegExpWrapper) && (pattern = pattern.source, flagsAreUndefined && (flags = "flags" in rawPattern ? rawPattern.flags : getFlags.call(rawPattern))), pattern = void 0 === pattern ? "" : toString1(pattern), flags = void 0 === flags ? "" : toString1(flags), rawPattern = pattern, UNSUPPORTED_DOT_ALL && "dotAll" in re1 && (dotAll = !!flags && flags.indexOf("s") > -1) && (flags = flags.replace(/s/g, "")), rawFlags = flags, UNSUPPORTED_Y && "sticky" in re1 && (sticky = !!flags && flags.indexOf("y") > -1) && (flags = flags.replace(/y/g, "")), UNSUPPORTED_NCG && (pattern = (handled = handleNCG(pattern))[0], groups = handled[1]), result = inheritIfRequired(NativeRegExp(pattern, flags), thisIsRegExp ? this : RegExpPrototype, RegExpWrapper), (dotAll || sticky || groups.length) && (state = enforceInternalState(result), dotAll && (state.dotAll = !0, state.raw = RegExpWrapper(handleDotAll(pattern), rawFlags)), sticky && (state.sticky = !0), groups.length && (state.groups = groups)), pattern !== rawPattern) try {
// fails in old engines, but we have no alternatives for unsupported regex syntax
createNonEnumerableProperty(result, "source", "" === rawPattern ? "(?:)" : rawPattern);
} catch (error) {
/* empty */ }
return result;
}, proxy = function(key) {
(key in RegExpWrapper) || defineProperty(RegExpWrapper, key, {
key in RegExpWrapper || defineProperty(RegExpWrapper, key, {
configurable: !0,
get: function() {
return NativeRegExp[key];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3855,9 +3855,9 @@
for(function(props, specialEasing) {
var value, name1, index, easing, hooks;
// camelCase, specialEasing and expand cssHook pass
for(index in props)if (easing = specialEasing[name1 = jQuery.camelCase(index)], value = props[index], jQuery.isArray(value) && (easing = value[1], value = props[index] = value[0]), index !== name1 && (props[name1] = value, delete props[index]), (hooks = jQuery.cssHooks[name1]) && ("expand" in hooks)) // not quite $.extend, this wont overwrite keys already present.
for(index in props)if (easing = specialEasing[name1 = jQuery.camelCase(index)], value = props[index], jQuery.isArray(value) && (easing = value[1], value = props[index] = value[0]), index !== name1 && (props[name1] = value, delete props[index]), (hooks = jQuery.cssHooks[name1]) && "expand" in hooks) // not quite $.extend, this wont overwrite keys already present.
// also - reusing 'index' from above because we have the correct "name"
for(index in value = hooks.expand(value), delete props[name1], value)(index in props) || (props[index] = value[index], specialEasing[index] = easing);
for(index in value = hooks.expand(value), delete props[name1], value)index in props || (props[index] = value[index], specialEasing[index] = easing);
else specialEasing[name1] = easing;
}(props, animation.opts.specialEasing); index < length; index++)if (result = animationPrefilters[index].call(animation, elem, props, animation.opts)) return result;
// attach callbacks from options
Expand Down
96 changes: 71 additions & 25 deletions crates/swc_ecma_transforms_base/src/fixer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,6 @@ enum Context {
FreeExpr,
}

macro_rules! array {
($name:ident, $T:tt) => {
fn $name(&mut self, e: &mut $T) {
let old = self.ctx;
self.ctx = Context::ForcedExpr;
e.elems.visit_mut_with(self);
self.ctx = old;
}
};
}

impl Fixer<'_> {
fn wrap_callee(&mut self, e: &mut Expr) {
match e {
Expand All @@ -102,7 +91,13 @@ impl Fixer<'_> {
impl VisitMut for Fixer<'_> {
noop_visit_mut_type!();

array!(visit_mut_array_lit, ArrayLit);
fn visit_mut_array_lit(&mut self, e: &mut ArrayLit) {
let ctx = mem::replace(&mut self.ctx, Context::ForcedExpr);
let in_for_stmt_head = mem::replace(&mut self.in_for_stmt_head, false);
e.elems.visit_mut_with(self);
self.in_for_stmt_head = in_for_stmt_head;
self.ctx = ctx;
}

fn visit_mut_arrow_expr(&mut self, node: &mut ArrowExpr) {
let old = self.ctx;
Expand Down Expand Up @@ -173,7 +168,9 @@ impl VisitMut for Fixer<'_> {
}

fn visit_mut_assign_pat(&mut self, node: &mut AssignPat) {
let in_for_stmt_head = mem::replace(&mut self.in_for_stmt_head, false);
node.visit_mut_children_with(self);
self.in_for_stmt_head = in_for_stmt_head;

if let Expr::Seq(..) = &*node.right {
self.wrap(&mut node.right);
Expand All @@ -185,7 +182,9 @@ impl VisitMut for Fixer<'_> {

let old = self.ctx;
self.ctx = Context::ForcedExpr;
let in_for_stmt_head = mem::replace(&mut self.in_for_stmt_head, false);
node.value.visit_mut_with(self);
self.in_for_stmt_head = in_for_stmt_head;
self.ctx = old;
}

Expand Down Expand Up @@ -345,6 +344,12 @@ impl VisitMut for Fixer<'_> {
}
}

fn visit_mut_block_stmt(&mut self, n: &mut BlockStmt) {
let in_for_stmt_head = mem::replace(&mut self.in_for_stmt_head, false);
n.visit_mut_children_with(self);
self.in_for_stmt_head = in_for_stmt_head;
}

fn visit_mut_block_stmt_or_expr(&mut self, body: &mut BlockStmtOrExpr) {
body.visit_mut_children_with(self);

Expand Down Expand Up @@ -376,9 +381,14 @@ impl VisitMut for Fixer<'_> {
}

fn visit_mut_class(&mut self, node: &mut Class) {
let old = self.ctx;
self.ctx = Context::Default;
node.visit_mut_children_with(self);
let ctx = mem::replace(&mut self.ctx, Context::Default);

node.super_class.visit_mut_with(self);

let in_for_stmt_head = mem::replace(&mut self.in_for_stmt_head, false);
node.body.visit_mut_with(self);
self.in_for_stmt_head = in_for_stmt_head;

match &mut node.super_class {
Some(e)
if e.is_seq()
Expand All @@ -393,7 +403,7 @@ impl VisitMut for Fixer<'_> {
}
_ => {}
};
self.ctx = old;
self.ctx = ctx;

node.body.retain(|m| !matches!(m, ClassMember::Empty(..)));
}
Expand Down Expand Up @@ -472,6 +482,12 @@ impl VisitMut for Fixer<'_> {
self.handle_expr_stmt(&mut s.expr);
}

fn visit_mut_for_head(&mut self, n: &mut ForHead) {
let in_for_stmt_head = mem::replace(&mut self.in_for_stmt_head, true);
n.visit_mut_children_with(self);
self.in_for_stmt_head = in_for_stmt_head;
}

fn visit_mut_for_of_stmt(&mut self, s: &mut ForOfStmt) {
s.visit_mut_children_with(self);

Expand Down Expand Up @@ -507,15 +523,13 @@ impl VisitMut for Fixer<'_> {
}

fn visit_mut_for_stmt(&mut self, n: &mut ForStmt) {
let old = self.in_for_stmt_head;
self.in_for_stmt_head = true;
let in_for_stmt_head = mem::replace(&mut self.in_for_stmt_head, true);
n.init.visit_mut_with(self);
self.in_for_stmt_head = in_for_stmt_head;

n.test.visit_mut_with(self);
n.update.visit_mut_with(self);

self.in_for_stmt_head = false;
n.body.visit_mut_with(self);
self.in_for_stmt_head = old;
}

fn visit_mut_if_stmt(&mut self, node: &mut IfStmt) {
Expand Down Expand Up @@ -594,10 +608,9 @@ impl VisitMut for Fixer<'_> {
}

fn visit_mut_new_expr(&mut self, node: &mut NewExpr) {
let old = self.ctx;
self.ctx = Context::ForcedExpr;
let ctx = mem::replace(&mut self.ctx, Context::ForcedExpr);

node.args.visit_mut_with(self);
self.ctx = old;

self.ctx = Context::Callee { is_new: true };
node.callee.visit_mut_with(self);
Expand All @@ -612,7 +625,7 @@ impl VisitMut for Fixer<'_> {
| Expr::Lit(..) => self.wrap(&mut node.callee),
_ => {}
}
self.ctx = old;
self.ctx = ctx;
}

fn visit_mut_opt_call(&mut self, node: &mut OptCall) {
Expand Down Expand Up @@ -767,13 +780,46 @@ impl VisitMut for Fixer<'_> {
expr.arg.visit_mut_with(self);
self.ctx = old;
}

fn visit_mut_object_lit(&mut self, n: &mut ObjectLit) {
let in_for_stmt_head = mem::replace(&mut self.in_for_stmt_head, false);
n.visit_mut_children_with(self);
self.in_for_stmt_head = in_for_stmt_head;
}

fn visit_mut_params(&mut self, n: &mut Vec<Param>) {
let in_for_stmt_head = mem::replace(&mut self.in_for_stmt_head, false);
n.visit_mut_children_with(self);
self.in_for_stmt_head = in_for_stmt_head;
}

// only used in ArrowExpr
fn visit_mut_pats(&mut self, n: &mut Vec<Pat>) {
let in_for_stmt_head = mem::replace(&mut self.in_for_stmt_head, false);
n.visit_mut_children_with(self);
self.in_for_stmt_head = in_for_stmt_head;
}

fn visit_mut_expr_or_spreads(&mut self, n: &mut Vec<ExprOrSpread>) {
let in_for_stmt_head = mem::replace(&mut self.in_for_stmt_head, false);
n.visit_mut_children_with(self);
self.in_for_stmt_head = in_for_stmt_head;
}
}

impl Fixer<'_> {
fn wrap_with_paren_if_required(&mut self, e: &mut Expr) {
let mut has_padding_value = false;
match e {
Expr::Bin(BinExpr { op: op!("in"), .. }) if self.in_for_stmt_head => {
// TODO:
// if the in expression is in a parentheses, we should not wrap it with a
// parentheses again. But the parentheses is added later,
// so we don't have enough information to detect it at this moment.
// Example:
// for(var a = 1 + (2 || b in c) in {});
// |~~~~~~~~~~~|
// this parentheses is removed by unwrap_expr and added again later
self.wrap(e);
}

Expand Down

0 comments on commit 5cd837f

Please sign in to comment.