Call includeNode for self or children nodes in includeDestructuredIfNecessary (#6090)

* Add test

* Call includeNode for self or children nodes in includeDestructuredIfNecessary

* Replace includeDestructuredIfNecessary with a stub in MemberExpression

Author: TrickyPi

src/ast/nodes/ArrayPattern.ts

@@ -86,24 +86,19 @@ export default class ArrayPattern extends NodeBase implements DeclarationPattern
 	): boolean {
 		let included = false;
 		const includedPatternPath = getIncludedPatternPath(destructuredInitPath);
-		for (const element of this.elements) {
+		for (const element of [...this.elements].reverse()) {
 			if (element) {
-				element.included ||= included;
+				if (included && !element.included) {
+					element.includeNode(context);
+				}
 				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);
-				}
-			}
+		if (!this.included && included) {
+			this.includeNode(context);
 		}
-		return (this.included ||= included);
+		return this.included;
 	}
 
 	markDeclarationReached(): void {

src/ast/nodes/AssignmentPattern.ts

@@ -71,13 +71,16 @@ export default class AssignmentPattern extends NodeBase implements DeclarationPa
 		if ((included ||= this.right.shouldBeIncluded(context))) {
 			this.right.include(context, false);
 			if (!this.left.included) {
-				this.left.included = true;
+				this.left.includeNode(context);
 				// 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);
+		if (!this.included && included) {
+			this.includeNode(context);
+		}
+		return this.included;
 	}
 
 	includeNode(context: InclusionContext) {

src/ast/nodes/Identifier.ts

@@ -115,8 +115,9 @@ export default class Identifier extends IdentifierBase implements DeclarationPat
 		}
 		const { propertyReadSideEffects } = this.scope.context.options
 			.treeshake as NormalizedTreeshakingOptions;
+		let included = this.included;
 		if (
-			(this.included ||=
+			(included ||=
 				destructuredInitPath.length > 0 &&
 				!context.brokenFlow &&
 				propertyReadSideEffects &&
@@ -131,9 +132,11 @@ export default class Identifier extends IdentifierBase implements DeclarationPat
 				this.scope.context.includeVariableInModule(this.variable, EMPTY_PATH, context);
 			}
 			init.includePath(destructuredInitPath, context);
-			return true;
 		}
-		return false;
+		if (!this.included && included) {
+			this.includeNode(context);
+		}
+		return this.included;
 	}
 
 	markDeclarationReached(): void {

src/ast/nodes/MemberExpression.ts

@@ -7,7 +7,7 @@ import { logIllegalImportReassignment, logMissingExport } from '../../utils/logs
 import type { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers';
 import type { DeoptimizableEntity } from '../DeoptimizableEntity';
 import type { HasEffectsContext, InclusionContext } from '../ExecutionContext';
-import { createHasEffectsContext, createInclusionContext } from '../ExecutionContext';
+import { createInclusionContext } from '../ExecutionContext';
 import type {
 	NodeInteraction,
 	NodeInteractionAccessed,
@@ -440,25 +440,14 @@ export default class MemberExpression
 		}
 	}
 
-	includeDestructuredIfNecessary(
-		context: InclusionContext,
-		destructuredInitPath: ObjectPath,
-		init: ExpressionEntity
-	): boolean {
-		if (
-			(this.included ||=
-				destructuredInitPath.length > 0 &&
-				!context.brokenFlow &&
-				init.hasEffectsOnInteractionAtPath(
-					destructuredInitPath,
-					NODE_INTERACTION_UNKNOWN_ACCESS,
-					createHasEffectsContext()
-				))
-		) {
-			init.include(context, false);
-			return true;
-		}
-		return false;
+	includeDestructuredIfNecessary(): boolean {
+		/* istanbul ignore next */
+		this.scope.context.error(
+			{
+				message: 'includeDestructuredIfNecessary is currently not supported for MemberExpressions'
+			},
+			this.start
+		);
 	}
 
 	initialise(): void {

src/ast/nodes/ObjectPattern.ts

@@ -87,25 +87,23 @@ export default class ObjectPattern extends NodeBase implements DeclarationPatter
 		destructuredInitPath: ObjectPath,
 		init: ExpressionEntity
 	): boolean {
-		if (!this.properties.length) return false;
+		if (!this.properties.length) return this.included;
 
 		const lastProperty = this.properties.at(-1)!;
-		const lastPropertyIncluded = lastProperty.includeDestructuredIfNecessary(
-			context,
-			destructuredInitPath,
-			init
-		);
+		let included = lastProperty.includeDestructuredIfNecessary(context, destructuredInitPath, init);
 		const lastPropertyIsRestElement = lastProperty.type === NodeType.RestElement;
 
-		let included = lastPropertyIsRestElement ? lastPropertyIncluded : false;
 		for (const property of this.properties.slice(0, -1)) {
-			if (lastPropertyIsRestElement && lastPropertyIncluded) {
+			if (lastPropertyIsRestElement && included && !property.included) {
 				property.includeNode(context);
 			}
 			included =
 				property.includeDestructuredIfNecessary(context, destructuredInitPath, init) || included;
 		}
-		return (this.included ||= included);
+		if (!this.included && included) {
+			this.includeNode(context);
+		}
+		return this.included;
 	}
 
 	markDeclarationReached(): void {

src/ast/nodes/Property.ts

@@ -84,13 +84,16 @@ export default class Property extends MethodBase implements DeclarationPatternNo
 		if ((included ||= this.key.hasEffects(createHasEffectsContext()))) {
 			this.key.include(context, false);
 			if (!this.value.included) {
-				this.value.included = true;
+				this.value.includeNode(context);
 				// 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);
+		if (!this.included && included) {
+			this.includeNode(context);
+		}
+		return this.included;
 	}
 
 	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren) {

src/ast/nodes/RestElement.ts

@@ -73,12 +73,15 @@ export default class RestElement extends NodeBase implements DeclarationPatternN
 		destructuredInitPath: ObjectPath,
 		init: ExpressionEntity
 	): boolean {
-		return (this.included =
-			this.argument.includeDestructuredIfNecessary(
-				context,
-				getIncludedPatternPath(destructuredInitPath),
-				init
-			) || this.included);
+		const included = this.argument.includeDestructuredIfNecessary(
+			context,
+			getIncludedPatternPath(destructuredInitPath),
+			init
+		);
+		if (!this.included && included) {
+			this.includeNode(context);
+		}
+		return this.included;
 	}
 
 	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren) {

test/function/samples/keep-the-non-first-assignment-in-destructuring-unknown-array-variables/_config.js

@@ -0,0 +1,6 @@
+module.exports = defineTest({
+	description: 'Keep the non-first assignment in destructuring unknown array variables',
+	context: {
+		unknownVariable: []
+	}
+});

test/function/samples/keep-the-non-first-assignment-in-destructuring-unknown-array-variables/main.js

@@ -0,0 +1,2 @@
+const [a, last2 = { foo: true }] = unknownVariable;
+assert.ok(last2.foo);