Track call side-effects both by entity and CallExpression to avoid un…

…tracked side-effects in nested calls (#3539)

src/ast/ExecutionContext.ts

@@ -1,5 +1,5 @@
 import { ExpressionEntity } from './nodes/shared/Expression';
-import { PathTracker } from './utils/PathTracker';
+import { DiscriminatedPathTracker, PathTracker } from './utils/PathTracker';
 import ThisVariable from './variables/ThisVariable';
 
 interface ExecutionContextIgnore {
@@ -22,9 +22,9 @@ export interface HasEffectsContext extends InclusionContext {
 	accessed: PathTracker;
 	assigned: PathTracker;
 	brokenFlow: number;
-	called: PathTracker;
+	called: DiscriminatedPathTracker;
 	ignore: ExecutionContextIgnore;
-	instantiated: PathTracker;
+	instantiated: DiscriminatedPathTracker;
 	replacedVariableInits: Map<ThisVariable, ExpressionEntity>;
 }
 
@@ -40,15 +40,15 @@ export function createHasEffectsContext(): HasEffectsContext {
 		accessed: new PathTracker(),
 		assigned: new PathTracker(),
 		brokenFlow: BROKEN_FLOW_NONE,
-		called: new PathTracker(),
+		called: new DiscriminatedPathTracker(),
 		ignore: {
 			breaks: false,
 			continues: false,
 			labels: new Set(),
 			returnAwaitYield: false
 		},
 		includedLabels: new Set(),
-		instantiated: new PathTracker(),
+		instantiated: new DiscriminatedPathTracker(),
 		replacedVariableInits: new Map()
 	};
 }

src/ast/nodes/CallExpression.ts

@@ -184,7 +184,7 @@ export default class CallExpression extends NodeBase implements DeoptimizableEnt
 		const trackedExpressions = (callOptions.withNew
 			? context.instantiated
 			: context.called
-		).getEntities(path);
+		).getEntities(path, callOptions);
 		if (trackedExpressions.has(this)) return false;
 		trackedExpressions.add(this);
 		return this.returnExpression!.hasEffectsWhenCalledAtPath(path, callOptions, context);

src/ast/nodes/Property.ts

@@ -124,7 +124,7 @@ export default class Property extends NodeBase implements DeoptimizableEntity {
 			const trackedExpressions = (callOptions.withNew
 				? context.instantiated
 				: context.called
-			).getEntities(path);
+			).getEntities(path, callOptions);
 			if (trackedExpressions.has(this)) return false;
 			trackedExpressions.add(this);
 			return this.returnExpression!.hasEffectsWhenCalledAtPath(path, callOptions, context);

src/ast/utils/PathTracker.ts

@@ -17,7 +17,7 @@ interface EntityPaths {
 export class PathTracker {
 	entityPaths: EntityPaths = Object.create(null, { [EntitiesKey]: { value: new Set<Entity>() } });
 
-	getEntities(path: ObjectPath) {
+	getEntities(path: ObjectPath): Set<Entity> {
 		let currentPaths = this.entityPaths;
 		for (const pathSegment of path) {
 			currentPaths = currentPaths[pathSegment] =
@@ -29,3 +29,28 @@ export class PathTracker {
 }
 
 export const SHARED_RECURSION_TRACKER = new PathTracker();
+
+interface DiscriminatedEntityPaths {
+	[EntitiesKey]: Map<object, Set<Entity>>;
+	[UnknownKey]?: DiscriminatedEntityPaths;
+	[pathSegment: string]: DiscriminatedEntityPaths;
+}
+
+export class DiscriminatedPathTracker {
+	entityPaths: DiscriminatedEntityPaths = Object.create(null, {
+		[EntitiesKey]: { value: new Map<object, Set<Entity>>() }
+	});
+
+	getEntities(path: ObjectPath, discriminator: object): Set<Entity> {
+		let currentPaths = this.entityPaths;
+		for (const pathSegment of path) {
+			currentPaths = currentPaths[pathSegment] =
+				currentPaths[pathSegment] ||
+				Object.create(null, { [EntitiesKey]: { value: new Map<object, Set<Entity>>() } });
+		}
+		const entities = currentPaths[EntitiesKey];
+		const result = entities.get(discriminator) || new Set();
+		entities.set(discriminator, result);
+		return result;
+	}
+}

src/ast/variables/LocalVariable.ts

@@ -150,7 +150,7 @@ export default class LocalVariable extends Variable {
 		const trackedExpressions = (callOptions.withNew
 			? context.instantiated
 			: context.called
-		).getEntities(path);
+		).getEntities(path, callOptions);
 		if (trackedExpressions.has(this)) return false;
 		trackedExpressions.add(this);
 		return (this.init && this.init.hasEffectsWhenCalledAtPath(path, callOptions, context))!;

test/cli/samples/watch/watch-config-error/_config.js

@@ -20,7 +20,8 @@ module.exports = {
 		);
 	},
 	after() {
-		fs.unlinkSync(configFile);
+		// synchronous sometimes does not seem to work, probably because the watch is not yet removed properly
+		setTimeout(() => fs.unlinkSync(configFile), 100);
 	},
 	abortOnStderr(data) {
 		if (data.includes(`created _actual${path.sep}main1.js`)) {

test/function/samples/nested-foreach/_config.js

@@ -0,0 +1,6 @@
+const assert = require('assert');
+
+module.exports = {
+	description: 'detects side-effects in nested .forEach calls (#3533)',
+	exports: exports => assert.strictEqual(exports.result, 9)
+};

test/function/samples/nested-foreach/main.js

@@ -0,0 +1,4 @@
+const list = [1, 2];
+export let result = 0;
+
+list.forEach(p => list.forEach(pp => (result += p * pp)));