fix: uses a fresh Set of included labels when checking effects of chi…

…ldren
rollup · Dec 17, 2023 · a0c0ad0 · a0c0ad0
1 parent 4319406
commit a0c0ad0
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 17 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -140,6 +140,7 @@ module.exports = {
 			}
 		],
 		'sort-keys': ['error', 'asc', { caseSensitive: false }],
+		'unicorn/consistent-destructuring': 'off',
 		'unicorn/filename-case': 'off',
 		'unicorn/no-array-callback-reference': 'off',
 		'unicorn/no-array-reduce': 'off',

diff --git a/src/ast/nodes/IfStatement.ts b/src/ast/nodes/IfStatement.ts
@@ -40,12 +40,10 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 		if (typeof testValue === 'symbol') {
 			const { brokenFlow } = context;
 			if (this.consequent.hasEffects(context)) return true;
-			// eslint-disable-next-line unicorn/consistent-destructuring
 			const consequentBrokenFlow = context.brokenFlow;
 			context.brokenFlow = brokenFlow;
 			if (this.alternate === null) return false;
 			if (this.alternate.hasEffects(context)) return true;
-			// eslint-disable-next-line unicorn/consistent-destructuring
 			context.brokenFlow = context.brokenFlow && consequentBrokenFlow;
 			return false;
 		}
@@ -166,13 +164,11 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 		let consequentBrokenFlow = false;
 		if (this.consequent.shouldBeIncluded(context)) {
 			this.consequent.include(context, false, { asSingleStatement: true });
-			// eslint-disable-next-line unicorn/consistent-destructuring
 			consequentBrokenFlow = context.brokenFlow;
 			context.brokenFlow = brokenFlow;
 		}
 		if (this.alternate?.shouldBeIncluded(context)) {
 			this.alternate.include(context, false, { asSingleStatement: true });
-			// eslint-disable-next-line unicorn/consistent-destructuring
 			context.brokenFlow = context.brokenFlow && consequentBrokenFlow;
 		}
 	}

diff --git a/src/ast/nodes/LabeledStatement.ts b/src/ast/nodes/LabeledStatement.ts
@@ -15,30 +15,33 @@ export default class LabeledStatement extends StatementBase {
 	declare type: NodeType.tLabeledStatement;
 
 	hasEffects(context: HasEffectsContext): boolean {
-		const brokenFlow = context.brokenFlow;
+		const { brokenFlow, includedLabels } = context;
 		context.ignore.labels.add(this.label.name);
-		if (this.body.hasEffects(context)) return true;
-		context.ignore.labels.delete(this.label.name);
-		if (context.includedLabels.has(this.label.name)) {
-			context.includedLabels.delete(this.label.name);
-			context.brokenFlow = brokenFlow;
+		context.includedLabels = new Set<string>();
+		let bodyHasEffects = false;
+		if (this.body.hasEffects(context)) {
+			bodyHasEffects = true;
+		} else {
+			context.ignore.labels.delete(this.label.name);
+			if (context.includedLabels.has(this.label.name)) {
+				context.includedLabels.delete(this.label.name);
+				context.brokenFlow = brokenFlow;
+			}
 		}
-		return false;
+		context.includedLabels = new Set([...includedLabels, ...context.includedLabels]);
+		return bodyHasEffects;
 	}
 
 	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
 		this.included = true;
 		const { brokenFlow, includedLabels } = context;
 		context.includedLabels = new Set<string>();
 		this.body.include(context, includeChildrenRecursively);
-		// eslint-disable-next-line unicorn/consistent-destructuring
 		if (includeChildrenRecursively || context.includedLabels.has(this.label.name)) {
 			this.label.include();
-			// eslint-disable-next-line unicorn/consistent-destructuring
 			context.includedLabels.delete(this.label.name);
 			context.brokenFlow = brokenFlow;
 		}
-		// eslint-disable-next-line unicorn/consistent-destructuring
 		context.includedLabels = new Set([...includedLabels, ...context.includedLabels]);
 	}
 

diff --git a/src/ast/nodes/SwitchStatement.ts b/src/ast/nodes/SwitchStatement.ts
@@ -34,7 +34,6 @@ export default class SwitchStatement extends StatementBase {
 		let onlyHasBrokenFlow = true;
 		for (const switchCase of this.cases) {
 			if (switchCase.hasEffects(context)) return true;
-			// eslint-disable-next-line unicorn/consistent-destructuring
 			onlyHasBrokenFlow &&= context.brokenFlow && !context.hasBreak;
 			context.hasBreak = false;
 			context.brokenFlow = brokenFlow;
@@ -68,7 +67,6 @@ export default class SwitchStatement extends StatementBase {
 			}
 			if (isCaseIncluded) {
 				switchCase.include(context, includeChildrenRecursively);
-				// eslint-disable-next-line unicorn/consistent-destructuring
 				onlyHasBrokenFlow &&= context.brokenFlow && !context.hasBreak;
 				context.hasBreak = false;
 				context.brokenFlow = brokenFlow;

diff --git a/test/form/samples/break-control-flow/break-statement-labels/main.js b/test/form/samples/break-control-flow/break-statement-labels/main.js
@@ -20,7 +20,7 @@ outer: {
 }
 
 outer: {
-	inner:/* retained comment */ {
+	inner: /* retained comment */ {
 		console.log('retained');
 		break outer;
 		console.log('removed');
@@ -59,6 +59,22 @@ outer: {
 	}
 }
 
+// removed
+outer: {
+	if (globalThis.unknown) {
+		break outer;
+	} else {
+		(() => {
+			inner: {
+				outer: {
+					break inner;
+				}
+				console.log('removed');
+			}
+		})();
+	}
+}
+
 function withConsequentReturn() {
 	outer: {
 		inner: {