Fix more variable deconflicting issues (#5728)

* Ensure we also include declared variables if they are included for a key or default side effect

* Always include full array patterns

Author: lukastaegert

src/ast/nodes/ArrayPattern.ts

@@ -87,8 +87,21 @@ export default class ArrayPattern extends NodeBase implements DeclarationPattern
 		let included = false;
 		const includedPatternPath = getIncludedPatternPath(destructuredInitPath);
 		for (const element of this.elements) {
-			included =
-				element?.includeDestructuredIfNecessary(context, includedPatternPath, init) || included;
+			if (element) {
+				element.included ||= included;
+				included =
+					element.includeDestructuredIfNecessary(context, includedPatternPath, init) || included;
+			}
+		}
+		if (included) {
+			// This is necessary so that if any pattern element is included, all are
+			// included for proper deconflicting
+			for (const element of this.elements) {
+				if (element && !element.included) {
+					element.included = true;
+					element.includeDestructuredIfNecessary(context, includedPatternPath, init);
+				}
+			}
 		}
 		return (this.included ||= included);
 	}

src/ast/nodes/AssignmentPattern.ts

@@ -70,6 +70,12 @@ export default class AssignmentPattern extends NodeBase implements DeclarationPa
 			this.included;
 		if ((included ||= this.right.shouldBeIncluded(context))) {
 			this.right.includePath(UNKNOWN_PATH, context, false);
+			if (!this.left.included) {
+				this.left.included = true;
+				// Unfortunately, we need to include the left side again now, so that
+				// any declared variables are properly included.
+				this.left.includeDestructuredIfNecessary(context, destructuredInitPath, init);
+			}
 		}
 		return (this.included = included);
 	}

src/ast/nodes/Property.ts

@@ -77,15 +77,18 @@ export default class Property extends MethodBase implements DeclarationPatternNo
 		destructuredInitPath: ObjectPath,
 		init: ExpressionEntity
 	): boolean {
+		const path = this.getPathInProperty(destructuredInitPath);
 		let included =
-			(this.value as PatternNode).includeDestructuredIfNecessary(
-				context,
-				this.getPathInProperty(destructuredInitPath),
-				init
-			) || this.included;
-		included ||= this.key.hasEffects(createHasEffectsContext());
-		if (included) {
+			(this.value as PatternNode).includeDestructuredIfNecessary(context, path, init) ||
+			this.included;
+		if ((included ||= this.key.hasEffects(createHasEffectsContext()))) {
 			this.key.includePath(EMPTY_PATH, context, false);
+			if (!this.value.included) {
+				this.value.included = true;
+				// Unfortunately, we need to include the value again now, so that any
+				// declared variables are properly included.
+				(this.value as PatternNode).includeDestructuredIfNecessary(context, path, init);
+			}
 		}
 		return (this.included = included);
 	}

test/function/samples/object-expression-treeshaking/deconflict-destructured-array-pattern/_config.js

@@ -0,0 +1,3 @@
+module.exports = defineTest({
+	description: 'makes sure to deconflict all variables in an array pattern if included'
+});

test/function/samples/object-expression-treeshaking/deconflict-destructured-array-pattern/dep.js

@@ -0,0 +1,2 @@
+const Foo = { ok: true };
+export { Foo as default };

test/function/samples/object-expression-treeshaking/deconflict-destructured-array-pattern/main.js

@@ -0,0 +1,6 @@
+import bar from './dep.js';
+
+const [Foo, Bar] = [1, 2];
+
+assert.deepStrictEqual(bar, { ok: true });
+assert.deepStrictEqual(Bar, 2);

test/function/samples/object-expression-treeshaking/deconflict-destructured-for-default-side-effects-array/_config.js

@@ -0,0 +1,4 @@
+module.exports = defineTest({
+	description:
+		'makes sure to deconflict variables that are destructured for side effects in their array pattern default values only'
+});

test/function/samples/object-expression-treeshaking/deconflict-destructured-for-default-side-effects-array/dep.js

@@ -0,0 +1,2 @@
+const Foo = { ok: true };
+export { Foo as default };

test/function/samples/object-expression-treeshaking/deconflict-destructured-for-default-side-effects-array/main.js

@@ -0,0 +1,9 @@
+import bar from './dep.js';
+let mutated = false;
+const [
+	Foo = (() => {
+		mutated = true;
+	})()
+] = [];
+assert.ok(mutated);
+assert.deepStrictEqual(bar, { ok: true });

test/function/samples/object-expression-treeshaking/deconflict-destructured-for-default-side-effects/_config.js

@@ -0,0 +1,4 @@
+module.exports = defineTest({
+	description:
+		'makes sure to deconflict variables that are destructured for side effects in their default values only'
+});

test/function/samples/object-expression-treeshaking/deconflict-destructured-for-default-side-effects/dep.js

@@ -0,0 +1,2 @@
+const Foo = { ok: true };
+export { Foo as default };

test/function/samples/object-expression-treeshaking/deconflict-destructured-for-default-side-effects/main.js

@@ -0,0 +1,9 @@
+import bar from './dep.js';
+let mutated = false;
+const {
+	Foo = (() => {
+		mutated = true;
+	})()
+} = {};
+assert.ok(mutated);
+assert.deepStrictEqual(bar, { ok: true });

test/function/samples/object-expression-treeshaking/deconflict-destructured-for-key-side-effects/_config.js

@@ -0,0 +1,4 @@
+module.exports = defineTest({
+	description:
+		'makes sure to deconflict variables that are destructured for side effects in their key only'
+});

test/function/samples/object-expression-treeshaking/deconflict-destructured-for-key-side-effects/dep.js

@@ -0,0 +1,2 @@
+const Foo = { ok: true };
+export { Foo as default };

test/function/samples/object-expression-treeshaking/deconflict-destructured-for-key-side-effects/main.js

@@ -0,0 +1,10 @@
+import bar from './dep.js';
+let mutated = false;
+const {
+	[(() => {
+		mutated = true;
+		return 'Foo';
+	})()]: Foo
+} = { Foo: true };
+assert.ok(mutated);
+assert.deepStrictEqual(bar, { ok: true });