Detect async arrow thenable side effects (#4120)

* Respect propertyReadSideEffects in spread elements * Track async arrow return effects
rollup · Jun 3, 2021 · 63533b7 · 63533b7
1 parent 5c05997
commit 63533b7
Show file tree

Hide file tree

Showing 10 changed files with 206 additions and 30 deletions.
diff --git a/src/ast/nodes/ArrowFunctionExpression.ts b/src/ast/nodes/ArrowFunctionExpression.ts
@@ -1,4 +1,5 @@
-import { CallOptions } from '../CallOptions';
+import { NormalizedTreeshakingOptions } from '../../rollup/types';
+import { CallOptions, NO_ARGS } from '../CallOptions';
 import { BROKEN_FLOW_NONE, HasEffectsContext, InclusionContext } from '../ExecutionContext';
 import ReturnValueScope from '../scopes/ReturnValueScope';
 import Scope from '../scopes/Scope';
@@ -13,6 +14,7 @@ import { ExpressionNode, GenericEsTreeNode, IncludeChildren, NodeBase } from './
 import { PatternNode } from './shared/Pattern';
 
 export default class ArrowFunctionExpression extends NodeBase {
+	async!: boolean;
 	body!: BlockStatement | ExpressionNode;
 	params!: PatternNode[];
 	preventChildBlockScope!: true;
@@ -35,7 +37,7 @@ export default class ArrowFunctionExpression extends NodeBase {
 	deoptimizeThisOnEventAtPath(): void {}
 
 	getReturnExpressionWhenCalledAtPath(path: ObjectPath): ExpressionEntity {
-		return path.length === 0 ? this.scope.getReturnExpression() : UNKNOWN_EXPRESSION;
+		return !this.async && path.length === 0 ? this.scope.getReturnExpression() : UNKNOWN_EXPRESSION;
 	}
 
 	hasEffects(): boolean {
@@ -56,6 +58,23 @@ export default class ArrowFunctionExpression extends NodeBase {
 		context: HasEffectsContext
 	): boolean {
 		if (path.length > 0) return true;
+		if (this.async) {
+			const { propertyReadSideEffects } = this.context.options
+				.treeshake as NormalizedTreeshakingOptions;
+			const returnExpression = this.scope.getReturnExpression();
+			if (
+				returnExpression.hasEffectsWhenCalledAtPath(
+					['then'],
+					{ args: NO_ARGS, thisParam: null, withNew: false },
+					context
+				) ||
+				(propertyReadSideEffects &&
+					(propertyReadSideEffects === 'always' ||
+						returnExpression.hasEffectsWhenAccessedAtPath(['then'], context)))
+			) {
+				return true;
+			}
+		}
 		for (const param of this.params) {
 			if (param.hasEffects(context)) return true;
 		}

diff --git a/src/ast/nodes/shared/FunctionNode.ts b/src/ast/nodes/shared/FunctionNode.ts
@@ -1,3 +1,4 @@
+import { NormalizedTreeshakingOptions } from '../../../rollup/types';
 import { CallOptions, NO_ARGS } from '../../CallOptions';
 import { BROKEN_FLOW_NONE, HasEffectsContext, InclusionContext } from '../../ExecutionContext';
 import { EVENT_CALLED, NodeEvent } from '../../NodeEvents';
@@ -81,17 +82,22 @@ export default class FunctionNode extends NodeBase {
 		context: HasEffectsContext
 	): boolean {
 		if (path.length > 0) return true;
-		if (
-			this.async &&
-			this.scope
-				.getReturnExpression()
-				.hasEffectsWhenCalledAtPath(
+		if (this.async) {
+			const { propertyReadSideEffects } = this.context.options
+				.treeshake as NormalizedTreeshakingOptions;
+			const returnExpression = this.scope.getReturnExpression();
+			if (
+				returnExpression.hasEffectsWhenCalledAtPath(
 					['then'],
 					{ args: NO_ARGS, thisParam: null, withNew: false },
 					context
-				)
-		) {
-			return true;
+				) ||
+				(propertyReadSideEffects &&
+					(propertyReadSideEffects === 'always' ||
+						returnExpression.hasEffectsWhenAccessedAtPath(['then'], context)))
+			) {
+				return true;
+			}
 		}
 		for (const param of this.params) {
 			if (param.hasEffects(context)) return true;

diff --git a/test/form/samples/async-function-effects/_config.js b/test/form/samples/async-function-effects/_config.js
@@ -1,5 +1,3 @@
-const path = require('path');
-
 module.exports = {
 	description: 'tracks effects when awaiting thenables'
 };
diff --git a/test/form/samples/async-function-effects/_expected.js b/test/form/samples/async-function-effects/_expected.js
@@ -6,10 +6,46 @@
 	};
 })();
 
+(async function () {
+	return {
+		get then() {
+			console.log(2);
+			return () => {};
+		}
+	};
+})();
+
+(async function () {
+	return {
+		get then() {
+			return () => console.log(3);
+		}
+	};
+})();
+
+(async () => ({
+	then() {
+		console.log(4);
+	}
+}))();
+
+(async () => ({
+	get then() {
+		console.log(5);
+		return () => {};
+	}
+}))();
+
+(async () => ({
+	get then() {
+		return () => console.log(6);
+	}
+}))();
+
 (async function () {
 	await {
 		then: function () {
-			console.log(2);
+			console.log(7);
 		}
 	};
 	return { then() {} };
@@ -18,7 +54,7 @@
 (async function () {
 	await {
 		get then() {
-			console.log(3);
+			console.log(8);
 			return () => {};
 		}
 	};
@@ -28,7 +64,7 @@
 (async function () {
 	await {
 		get then() {
-			return () => console.log(4);
+			return () => console.log(9);
 		}
 	};
 	return { then() {} };
@@ -39,7 +75,7 @@
 		then(resolve) {
 			resolve({
 				then() {
-					console.log(5);
+					console.log(10);
 				}
 			});
 		}
@@ -53,21 +89,21 @@ async function asyncIdentity(x) {
 
 asyncIdentity({}); // no side effects - may be dropped
 
-const promise = asyncIdentity(6);
+const promise = asyncIdentity(11);
 
 promise.then(x => console.log(x));
 
 asyncIdentity({
 	then(success, fail) {
-		success(console.log(7));
+		success(console.log(12));
 	}
 });
 
 asyncIdentity({
 	then(resolve) {
 		resolve({
 			then() {
-				console.log(8);
+				console.log(13);
 			}
 		});
 	}

diff --git a/test/form/samples/async-function-effects/main.js b/test/form/samples/async-function-effects/main.js
@@ -11,10 +11,67 @@
 	return { then() {} };
 })();
 
+(async function () {
+	return {
+		get then() {
+			console.log(2);
+			return () => {};
+		}
+	};
+})();
+
+(async function () {
+	return {
+		get then() {
+			return () => console.log(3);
+		}
+	};
+})();
+
+// removed
+(async function () {
+	return {
+		get then() {
+			return () => {};
+		}
+	};
+})();
+
+(async () => ({
+	then() {
+		console.log(4);
+	}
+}))();
+
+// removed
+(async () => ({
+	then() {}
+}))();
+
+(async () => ({
+	get then() {
+		console.log(5);
+		return () => {};
+	}
+}))();
+
+(async () => ({
+	get then() {
+		return () => console.log(6);
+	}
+}))();
+
+// removed
+(async () => ({
+	get then() {
+		return () => {};
+	}
+}))();
+
 (async function () {
 	await {
 		then: function () {
-			console.log(2);
+			console.log(7);
 		}
 	};
 	return { then() {} };
@@ -31,7 +88,7 @@
 (async function () {
 	await {
 		get then() {
-			console.log(3);
+			console.log(8);
 			return () => {};
 		}
 	};
@@ -41,7 +98,7 @@
 (async function () {
 	await {
 		get then() {
-			return () => console.log(4);
+			return () => console.log(9);
 		}
 	};
 	return { then() {} };
@@ -62,7 +119,7 @@
 		then(resolve) {
 			resolve({
 				then() {
-					console.log(5);
+					console.log(10);
 				}
 			});
 		}
@@ -76,21 +133,21 @@ async function asyncIdentity(x) {
 
 asyncIdentity({}); // no side effects - may be dropped
 
-const promise = asyncIdentity(6);
+const promise = asyncIdentity(11);
 
 promise.then(x => console.log(x));
 
 asyncIdentity({
 	then(success, fail) {
-		success(console.log(7));
+		success(console.log(12));
 	}
 });
 
 asyncIdentity({
 	then(resolve) {
 		resolve({
 			then() {
-				console.log(8);
+				console.log(13);
 			}
 		});
 	}

diff --git a/test/form/samples/ignore-property-access-side-effects/main.js b/test/form/samples/ignore-property-access-side-effects/main.js
@@ -24,6 +24,22 @@ const foo5 = {
 };
 
 const foo6 = (async function () {
+	return {
+		get then() {
+			console.log('effect');
+			return () => {};
+		}
+	};
+})();
+
+const foo7 = (async () => ({
+	get then() {
+		console.log('effect');
+		return () => {};
+	}
+}))();
+
+const foo8 = (async function () {
 	await {
 		get then() {
 			console.log('effect');

diff --git a/test/form/samples/keep-property-access-side-effects/_expected.js b/test/form/samples/keep-property-access-side-effects/_expected.js
@@ -1,6 +1,6 @@
 const getter = {
-	get foo () {
-		console.log( 'effect' );
+	get foo() {
+		console.log('effect');
 	}
 };
 getter.foo;
@@ -23,6 +23,22 @@ globalThis.unknown.unknownProperty;
 	}
 });
 
+((async function () {
+	return {
+		get then() {
+			console.log('effect');
+			return () => {};
+		}
+	};
+}))();
+
+(async () => ({
+	get then() {
+		console.log('effect');
+		return () => {};
+	}
+}))();
+
 ((async function () {
 	await {
 		get then() {

diff --git a/test/form/samples/keep-property-access-side-effects/main.js b/test/form/samples/keep-property-access-side-effects/main.js
@@ -1,6 +1,6 @@
 const getter = {
-	get foo () {
-		console.log( 'effect' );
+	get foo() {
+		console.log('effect');
 	}
 };
 const foo1 = getter.foo;
@@ -24,6 +24,22 @@ const foo5 = {
 };
 
 const foo6 = (async function () {
+	return {
+		get then() {
+			console.log('effect');
+			return () => {};
+		}
+	};
+})();
+
+const foo7 = (async () => ({
+	get then() {
+		console.log('effect');
+		return () => {};
+	}
+}))();
+
+const foo8 = (async function () {
 	await {
 		get then() {
 			console.log('effect');