Track calls through possible TDZ violations

rollup · May 14, 2022 · 1888f1e · 1888f1e
1 parent 9b71326
commit 1888f1e
Show file tree

Hide file tree

Showing 20 changed files with 47 additions and 40 deletions.
diff --git a/src/ast/nodes/ArrayExpression.ts b/src/ast/nodes/ArrayExpression.ts
@@ -68,7 +68,7 @@ export default class ArrayExpression extends NodeBase {
 		return super.hasEffects(context);
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
 	}
 

diff --git a/src/ast/nodes/AssignmentExpression.ts b/src/ast/nodes/AssignmentExpression.ts
@@ -54,7 +54,7 @@ export default class AssignmentExpression extends NodeBase {
 		);
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		return path.length > 0 && this.right.hasEffectsWhenAccessedAtPath(path, context);
 	}
 

diff --git a/src/ast/nodes/ConditionalExpression.ts b/src/ast/nodes/ConditionalExpression.ts
@@ -117,7 +117,7 @@ export default class ConditionalExpression extends NodeBase implements Deoptimiz
 		return usedBranch.hasEffects(context);
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		const usedBranch = this.getUsedBranch();
 		if (usedBranch === null) {
 			return (

diff --git a/src/ast/nodes/Identifier.ts b/src/ast/nodes/Identifier.ts
@@ -43,13 +43,13 @@ export default class Identifier extends NodeBase implements PatternNode {
 		variables: Variable[],
 		exportNamesByVariable: ReadonlyMap<Variable, readonly string[]>
 	): void {
-		if (this.variable !== null && exportNamesByVariable.has(this.variable)) {
-			variables.push(this.variable);
+		if (exportNamesByVariable.has(this.variable!)) {
+			variables.push(this.variable!);
 		}
 	}
 
 	bind(): void {
-		if (this.variable === null && isReference(this, this.parent as NodeWithFieldDefinition)) {
+		if (!this.variable && isReference(this, this.parent as NodeWithFieldDefinition)) {
 			this.variable = this.scope.findVariable(this.name);
 			this.variable.addReference(this);
 		}
@@ -108,7 +108,7 @@ export default class Identifier extends NodeBase implements PatternNode {
 		recursionTracker: PathTracker,
 		origin: DeoptimizableEntity
 	): LiteralValueOrUnknown {
-		return this.getVariableRespectingTDZ().getLiteralValueAtPath(path, recursionTracker, origin);
+		return this.getVariableRespectingTDZ()!.getLiteralValueAtPath(path, recursionTracker, origin);
 	}
 
 	getReturnExpressionWhenCalledAtPath(
@@ -117,7 +117,7 @@ export default class Identifier extends NodeBase implements PatternNode {
 		recursionTracker: PathTracker,
 		origin: DeoptimizableEntity
 	): ExpressionEntity {
-		return this.getVariableRespectingTDZ().getReturnExpressionWhenCalledAtPath(
+		return this.getVariableRespectingTDZ()!.getReturnExpressionWhenCalledAtPath(
 			path,
 			callOptions,
 			recursionTracker,
@@ -137,32 +137,22 @@ export default class Identifier extends NodeBase implements PatternNode {
 		);
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
-		return (
-			this.variable !== null &&
-			this.getVariableRespectingTDZ().hasEffectsWhenAccessedAtPath(path, context)
-		);
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
+		return this.getVariableRespectingTDZ()?.hasEffectsWhenAccessedAtPath(path, context);
 	}
 
 	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
 		return (
-			!this.variable ||
-			(path.length > 0
-				? this.getVariableRespectingTDZ()
-				: this.variable
-			).hasEffectsWhenAssignedAtPath(path, context)
-		);
+			path.length > 0 ? this.getVariableRespectingTDZ() : this.variable
+		)!.hasEffectsWhenAssignedAtPath(path, context);
 	}
 
 	hasEffectsWhenCalledAtPath(
 		path: ObjectPath,
 		callOptions: CallOptions,
 		context: HasEffectsContext
 	): boolean {
-		return (
-			!this.variable ||
-			this.getVariableRespectingTDZ().hasEffectsWhenCalledAtPath(path, callOptions, context)
-		);
+		return this.getVariableRespectingTDZ()!.hasEffectsWhenCalledAtPath(path, callOptions, context);
 	}
 
 	include(): void {
@@ -180,7 +170,7 @@ export default class Identifier extends NodeBase implements PatternNode {
 		context: InclusionContext,
 		args: readonly (ExpressionEntity | SpreadElement)[]
 	): void {
-		this.getVariableRespectingTDZ().includeArgumentsWhenCalledAtPath(path, context, args);
+		this.variable!.includeArgumentsWhenCalledAtPath(path, context, args);
 	}
 
 	isPossibleTDZ(): boolean {
@@ -267,11 +257,11 @@ export default class Identifier extends NodeBase implements PatternNode {
 		);
 	}
 
-	private getVariableRespectingTDZ(): ExpressionEntity {
+	private getVariableRespectingTDZ(): ExpressionEntity | null {
 		if (this.isPossibleTDZ()) {
 			return UNKNOWN_EXPRESSION;
 		}
-		return this.variable!;
+		return this.variable;
 	}
 }
 

diff --git a/src/ast/nodes/LogicalExpression.ts b/src/ast/nodes/LogicalExpression.ts
@@ -114,7 +114,7 @@ export default class LogicalExpression extends NodeBase implements Deoptimizable
 		return false;
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		const usedBranch = this.getUsedBranch();
 		if (usedBranch === null) {
 			return (

diff --git a/src/ast/nodes/MemberExpression.ts b/src/ast/nodes/MemberExpression.ts
@@ -211,7 +211,7 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
 		return UNKNOWN_EXPRESSION;
 	}
 
-	hasEffects(context: HasEffectsContext): boolean {
+	hasEffects(context: HasEffectsContext): boolean | undefined {
 		if (!this.deoptimized) this.applyDeoptimizations();
 		const { propertyReadSideEffects } = this.context.options
 			.treeshake as NormalizedTreeshakingOptions;
@@ -230,7 +230,7 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
 		);
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		if (this.variable !== null) {
 			return this.variable.hasEffectsWhenAccessedAtPath(path, context);
 		}

diff --git a/src/ast/nodes/ObjectExpression.ts b/src/ast/nodes/ObjectExpression.ts
@@ -75,7 +75,7 @@ export default class ObjectExpression extends NodeBase implements DeoptimizableE
 		);
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
 	}
 

diff --git a/src/ast/nodes/PropertyDefinition.ts b/src/ast/nodes/PropertyDefinition.ts
@@ -59,7 +59,7 @@ export default class PropertyDefinition extends NodeBase {
 		return this.key.hasEffects(context) || (this.static && this.value?.hasEffects(context));
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		return !this.value || this.value.hasEffectsWhenAccessedAtPath(path, context);
 	}
 

diff --git a/src/ast/nodes/SequenceExpression.ts b/src/ast/nodes/SequenceExpression.ts
@@ -58,7 +58,7 @@ export default class SequenceExpression extends NodeBase {
 		return false;
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		return (
 			path.length > 0 &&
 			this.expressions[this.expressions.length - 1].hasEffectsWhenAccessedAtPath(path, context)

diff --git a/src/ast/nodes/SpreadElement.ts b/src/ast/nodes/SpreadElement.ts
@@ -27,7 +27,7 @@ export default class SpreadElement extends NodeBase {
 		}
 	}
 
-	hasEffects(context: HasEffectsContext): boolean {
+	hasEffects(context: HasEffectsContext): boolean | undefined {
 		if (!this.deoptimized) this.applyDeoptimizations();
 		const { propertyReadSideEffects } = this.context.options
 			.treeshake as NormalizedTreeshakingOptions;

diff --git a/src/ast/nodes/shared/CallExpressionBase.ts b/src/ast/nodes/shared/CallExpressionBase.ts
@@ -116,7 +116,7 @@ export default abstract class CallExpressionBase extends NodeBase implements Deo
 		);
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		return (
 			!context.accessed.trackEntityAtPathAndGetIfTracked(path, this) &&
 			this.getReturnExpression().hasEffectsWhenAccessedAtPath(path, context)

diff --git a/src/ast/nodes/shared/ClassNode.ts b/src/ast/nodes/shared/ClassNode.ts
@@ -49,6 +49,7 @@ export default class ClassNode extends NodeBase implements DeoptimizableEntity {
 		}
 	}
 
+	// TODO Lukas also track arguments -> track and deoptimize call parameters?
 	deoptimizeThisOnEventAtPath(
 		event: NodeEvent,
 		path: ObjectPath,
@@ -92,7 +93,7 @@ export default class ClassNode extends NodeBase implements DeoptimizableEntity {
 		return initEffect || super.hasEffects(context);
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
 	}
 

diff --git a/src/ast/nodes/shared/Expression.ts b/src/ast/nodes/shared/Expression.ts
@@ -56,7 +56,10 @@ export class ExpressionEntity implements WritableEntity {
 		return UNKNOWN_EXPRESSION;
 	}
 
-	hasEffectsWhenAccessedAtPath(_path: ObjectPath, _context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(
+		_path: ObjectPath,
+		_context: HasEffectsContext
+	): boolean | undefined {
 		return true;
 	}
 

diff --git a/src/ast/nodes/shared/FunctionBase.ts b/src/ast/nodes/shared/FunctionBase.ts
@@ -118,7 +118,7 @@ export default abstract class FunctionBase extends NodeBase implements Deoptimiz
 		return this.scope.getReturnExpression();
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
 	}
 

diff --git a/src/ast/nodes/shared/MethodBase.ts b/src/ast/nodes/shared/MethodBase.ts
@@ -95,7 +95,7 @@ export default class MethodBase extends NodeBase implements DeoptimizableEntity
 		return this.key.hasEffects(context);
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		if (this.kind === 'get' && path.length === 0) {
 			return this.value.hasEffectsWhenCalledAtPath(EMPTY_PATH, this.accessorCallOptions, context);
 		}

diff --git a/src/ast/nodes/shared/ObjectEntity.ts b/src/ast/nodes/shared/ObjectEntity.ts
@@ -272,7 +272,7 @@ export class ObjectEntity extends ExpressionEntity {
 		return UNKNOWN_EXPRESSION;
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		const [key, ...subPath] = path;
 		if (path.length > 1) {
 			if (typeof key !== 'string') {

diff --git a/src/ast/nodes/shared/ObjectMember.ts b/src/ast/nodes/shared/ObjectMember.ts
@@ -50,7 +50,7 @@ export class ObjectMember extends ExpressionEntity {
 		);
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean | undefined {
 		if (path.length === 0) return false;
 		return this.object.hasEffectsWhenAccessedAtPath([this.key, ...path], context);
 	}

diff --git a/test/function/samples/parameter-defaults-module-side-effects/_config.js b/test/function/samples/parameter-defaults-module-side-effects/_config.js
@@ -0,0 +1,9 @@
+const path = require('path');
+
+module.exports = {
+	description:
+		'does not tree-shake necessary parameter defaults when modulesSideEffects are disabled',
+	options: {
+		treeshake: { moduleSideEffects: false }
+	}
+};
diff --git a/test/function/samples/parameter-defaults-module-side-effects/main.js b/test/function/samples/parameter-defaults-module-side-effects/main.js
@@ -0,0 +1,3 @@
+import { foo } from './other';
+
+assert.strictEqual(foo(), 'fallback');
diff --git a/test/function/samples/parameter-defaults-module-side-effects/other.js b/test/function/samples/parameter-defaults-module-side-effects/other.js
@@ -0,0 +1 @@
+export const foo = (a = 'fallback') => a;