Class method effects (#4018)

* Move logic from ClassBody into ClassNode

So that it sits in one place and is easier to extend.

* Track static class fields and improve handling of class getters/setters

This aims to improve tree-shaking of code that uses static class
properties (#3989) and to improve detection of side effects through
class getters/setters (#4016).

The first part works by keeping a map of positively known static
properties (methods and simple getters) in
`ClassNode.staticPropertyMap`, along with a flag
(`ClassNode.deoptimizedStatic`) that indicates that something happened
that removed our confidence that we know anything about the class
object.

Access and calls to these known static properties are handled by
routing the calls to `getLiteralValueAtPath`,
`getReturnExpressionWhenCalledAtPath`, and
`hasEffectsWhenCalledAtPath` to the known values in the properties. In
contrast to `ObjectExpression`, this class does not try to keep track
of multiple expressions associated with a property, since that doesn't
come up a lot on classes.

The handling of side effect detection through getters and setters is
done by, _if_ the entire class object (or its prototype in case of
access to the prototype) hasn't been deoptimized, scanning through the
directly defined getters and setters to see if one exists (calling
through to superclasses as appropriate). I believe that this is solid
because any code that would be able to change the set of getters and
setters on a class would cause the entire object to be deoptimized.

* Remove ClassNode.deoptimizeCache

* Keep a table for class property effects

* Add comment explaining property map

* Fix types

* Make getReturnExpression and getLiteralValue more similar for objects

* Use common logic for return expression and literal value

* Use common logic for return access and call effects

* Extract shared logic from ObjectExpression

* Use an object for better performance

* Simplify handling for setters and other properties

* Small simplification

* Work towards better class handling

* merge ObjectPathHandler into ObjectEntity

* Slightly refactor default values

* Separate unknown nodes from other Nodes to avoid future circular dependencies

* Introduce new prototype tracking

* Improve coverage

* Fix class deoptimization in arrow functions via this/super

* Simplify and merge property and method definition

* Improve coverage

* Replace deoptimizeProperties by deoptimizing a double unknown path

* Assume functions can add getters to parameters

* Improve class.prototype handling

* Assume created instance getters have side-effects

* Base all expressions on a base class

* Only deoptimize necessary paths when deoptimizing "this"

* Handle deoptimizing "this" in getters

* Handle deoptimizing "this" in setters

* Unify this deoptimization

* Simplify recursion tracking

* Get rid of deoptimizations during bind phase

* Get rid of unneeded double-binding checks

* Inline deoptimizations into NodeBase for simple cases

* Add more efficient way to create object entities

* Add thisParameter to CallOptions

* Move NodeEvents to separate file

* Track array elements

* Simplify namespace handling

* Use Object.values and Object.entries instead of Object.keys where useful

* Improve code and simplify literal handling

* Improve coverage

* Improve coverage

* Improve coverage in conditional and logical expressions

* Improve coverage

* 2.49.0-0

* Fix test to support pre-release versions

* Fix failed deoptimization of array props

* 2.49.0-1

Co-authored-by: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com>
Co-authored-by: Lukas Taegert-Atkinson <lukastaegert@users.noreply.github.com>

cli/run/build.ts

@@ -24,9 +24,7 @@ export default async function build(
 		} else if (inputOptions.input instanceof Array) {
 			inputFiles = inputOptions.input.join(', ');
 		} else if (typeof inputOptions.input === 'object' && inputOptions.input !== null) {
-			inputFiles = Object.keys(inputOptions.input)
-				.map(name => (inputOptions.input as Record<string, string>)[name])
-				.join(', ');
+			inputFiles = Object.values(inputOptions.input).join(', ');
 		}
 		stderr(cyan(`\n${bold(inputFiles!)} → ${bold(files.join(', '))}...`));
 	}

cli/run/watch-cli.ts

@@ -106,9 +106,7 @@ export async function watch(command: any) {
 						if (typeof input !== 'string') {
 							input = Array.isArray(input)
 								? input.join(', ')
-								: Object.keys(input as Record<string, string>)
-										.map(key => (input as Record<string, string>)[key])
-										.join(', ');
+								: Object.values(input as Record<string, string>).join(', ');
 						}
 						stderr(
 							cyan(`bundles ${bold(input)} → ${bold(event.output.map(relativeId).join(', '))}...`)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "rollup",
-  "version": "2.48.0",
+  "version": "2.49.0-1",
   "description": "Next-generation ES module bundler",
   "main": "dist/rollup.js",
   "module": "dist/es/rollup.js",

src/Bundle.ts

@@ -6,6 +6,7 @@ import {
 	GetManualChunk,
 	NormalizedInputOptions,
 	NormalizedOutputOptions,
+	OutputAsset,
 	OutputBundle,
 	OutputBundleWithPlaceholders,
 	OutputChunk,
@@ -41,11 +42,7 @@ export default class Bundle {
 	async generate(isWrite: boolean): Promise<OutputBundle> {
 		timeStart('GENERATE', 1);
 		const outputBundle: OutputBundleWithPlaceholders = Object.create(null);
-		this.pluginDriver.setOutputBundle(
-			outputBundle,
-			this.outputOptions,
-			this.facadeChunkByModule
-		);
+		this.pluginDriver.setOutputBundle(outputBundle, this.outputOptions, this.facadeChunkByModule);
 		try {
 			await this.pluginDriver.hookParallel('renderStart', [this.outputOptions, this.inputOptions]);
 
@@ -104,9 +101,9 @@ export default class Bundle {
 	): Promise<Map<Module, string>> {
 		const manualChunkAliasByEntry = new Map<Module, string>();
 		const chunkEntries = await Promise.all(
-			Object.keys(manualChunks).map(async alias => ({
+			Object.entries(manualChunks).map(async ([alias, files]) => ({
 				alias,
-				entries: await this.graph.moduleLoader.addAdditionalModules(manualChunks[alias])
+				entries: await this.graph.moduleLoader.addAdditionalModules(files)
 			}))
 		);
 		for (const { alias, entries } of chunkEntries) {
@@ -169,24 +166,23 @@ export default class Bundle {
 	}
 
 	private finaliseAssets(outputBundle: OutputBundleWithPlaceholders): void {
-		for (const key of Object.keys(outputBundle)) {
-			const file = outputBundle[key] as any;
+		for (const file of Object.values(outputBundle)) {
 			if (!file.type) {
 				warnDeprecation(
 					'A plugin is directly adding properties to the bundle object in the "generateBundle" hook. This is deprecated and will be removed in a future Rollup version, please use "this.emitFile" instead.',
 					true,
 					this.inputOptions
 				);
-				file.type = 'asset';
+				(file as OutputAsset).type = 'asset';
 			}
-			if (this.outputOptions.validate && typeof file.code == 'string') {
+			if (this.outputOptions.validate && typeof (file as OutputChunk).code == 'string') {
 				try {
-					this.graph.contextParse(file.code, {
+					this.graph.contextParse((file as OutputChunk).code, {
 						allowHashBang: true,
 						ecmaVersion: 'latest'
 					});
 				} catch (exception) {
-					this.inputOptions.onwarn(errChunkInvalid(file, exception));
+					this.inputOptions.onwarn(errChunkInvalid(file as OutputChunk, exception));
 				}
 			}
 		}

src/Chunk.ts

@@ -1326,8 +1326,8 @@ export default class Chunk {
 		if (!this.outputOptions.preserveModules) {
 			if (this.includedNamespaces.has(module)) {
 				const memberVariables = module.namespace.getMemberVariables();
-				for (const name of Object.keys(memberVariables)) {
-					moduleImports.add(memberVariables[name]);
+				for (const variable of Object.values(memberVariables)) {
+					moduleImports.add(variable);
 				}
 			}
 		}

src/Graph.ts

@@ -34,9 +34,9 @@ function normalizeEntryModules(
 			name: null
 		}));
 	}
-	return Object.keys(entryModules).map(name => ({
+	return Object.entries(entryModules).map(([name, id]) => ({
 		fileName: null,
-		id: entryModules[name],
+		id,
 		implicitlyLoadedAfter: [],
 		importer: undefined,
 		name
@@ -74,13 +74,14 @@ export default class Graph {
 			// increment access counter
 			for (const name in this.pluginCache) {
 				const cache = this.pluginCache[name];
-				for (const key of Object.keys(cache)) cache[key][0]++;
+				for (const value of Object.values(cache)) value[0]++;
 			}
 		}
 
 		if (watcher) {
 			this.watchMode = true;
-			const handleChange: WatchChangeHook = (...args) => this.pluginDriver.hookSeqSync('watchChange', args);
+			const handleChange: WatchChangeHook = (...args) =>
+				this.pluginDriver.hookSeqSync('watchChange', args);
 			const handleClose = () => this.pluginDriver.hookSeqSync('closeWatcher', []);
 			watcher.on('change', handleChange);
 			watcher.on('close', handleClose);
@@ -118,9 +119,9 @@ export default class Graph {
 
 		if (onCommentOrig && typeof onCommentOrig == 'function') {
 			options.onComment = (block, text, start, end, ...args) => {
-				comments.push({type: block ? "Block" : "Line", value: text, start, end});
+				comments.push({ type: block ? 'Block' : 'Line', value: text, start, end });
 				return onCommentOrig.call(options, block, text, start, end, ...args);
-			}
+			};
 		} else {
 			options.onComment = comments;
 		}
@@ -146,8 +147,8 @@ export default class Graph {
 		for (const name in this.pluginCache) {
 			const cache = this.pluginCache[name];
 			let allDeleted = true;
-			for (const key of Object.keys(cache)) {
-				if (cache[key][0] >= this.options.experimentalCacheExpiry) delete cache[key];
+			for (const [key, value] of Object.entries(cache)) {
+				if (value[0] >= this.options.experimentalCacheExpiry) delete cache[key];
 				else allDeleted = false;
 			}
 			if (allDeleted) delete this.pluginCache[name];
@@ -238,8 +239,7 @@ export default class Graph {
 
 	private warnForMissingExports() {
 		for (const module of this.modules) {
-			for (const importName of Object.keys(module.importDescriptions)) {
-				const importDescription = module.importDescriptions[importName];
+			for (const importDescription of Object.values(module.importDescriptions)) {
 				if (
 					importDescription.name !== '*' &&
 					!(importDescription.module as Module).getVariableForExportName(importDescription.name)

src/Module.ts

@@ -1002,8 +1002,7 @@ export default class Module {
 	private addModulesToImportDescriptions(importDescription: {
 		[name: string]: ImportDescription | ReexportDescription;
 	}) {
-		for (const name of Object.keys(importDescription)) {
-			const specifier = importDescription[name];
+		for (const specifier of Object.values(importDescription)) {
 			const id = this.resolvedIds[specifier.source].id;
 			specifier.module = this.graph.modulesById.get(id)!;
 		}

src/ast/CallOptions.ts

@@ -5,5 +5,6 @@ export const NO_ARGS = [];
 
 export interface CallOptions {
 	args: (ExpressionEntity | SpreadElement)[];
+	thisParam: ExpressionEntity | null;
 	withNew: boolean;
 }

src/ast/Entity.ts

@@ -1,9 +1,7 @@
 import { HasEffectsContext } from './ExecutionContext';
 import { ObjectPath } from './utils/PathTracker';
 
-export interface Entity {
-	toString: () => string;
-}
+export interface Entity {}
 
 export interface WritableEntity extends Entity {
 	/**
@@ -13,5 +11,5 @@ export interface WritableEntity extends Entity {
 	 * expression of this node is reassigned as well.
 	 */
 	deoptimizePath(path: ObjectPath): void;
-	hasEffectsWhenAssignedAtPath(path: ObjectPath, execution: HasEffectsContext): boolean;
+	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean;
 }

src/ast/NodeEvents.ts

@@ -0,0 +1,4 @@
+export const EVENT_ACCESSED = 0;
+export const EVENT_ASSIGNED = 1;
+export const EVENT_CALLED = 2;
+export type NodeEvent = typeof EVENT_ACCESSED | typeof EVENT_ASSIGNED | typeof EVENT_CALLED;

src/ast/keys.ts

@@ -9,7 +9,7 @@ export const keys: {
 
 export function getAndCreateKeys(esTreeNode: GenericEsTreeNode) {
 	keys[esTreeNode.type] = Object.keys(esTreeNode).filter(
-		key => key !== '_rollupAnnotations' && typeof esTreeNode[key] === 'object'
+		key => typeof esTreeNode[key] === 'object' && key !== '_rollupAnnotations'
 	);
 	return keys[esTreeNode.type];
 }

src/ast/nodes/ArrayExpression.ts

@@ -1,44 +1,94 @@
 import { CallOptions } from '../CallOptions';
+import { DeoptimizableEntity } from '../DeoptimizableEntity';
 import { HasEffectsContext } from '../ExecutionContext';
-import { ObjectPath, UNKNOWN_PATH } from '../utils/PathTracker';
-import {
-	arrayMembers,
-	getMemberReturnExpressionWhenCalled,
-	hasMemberEffectWhenCalled,
-	UNKNOWN_EXPRESSION
-} from '../values';
+import { NodeEvent } from '../NodeEvents';
+import { ObjectPath, PathTracker, UnknownInteger } from '../utils/PathTracker';
+import { UNDEFINED_EXPRESSION, UNKNOWN_LITERAL_NUMBER } from '../values';
 import * as NodeType from './NodeType';
+import { ARRAY_PROTOTYPE } from './shared/ArrayPrototype';
+import { ExpressionEntity, LiteralValueOrUnknown } from './shared/Expression';
 import { ExpressionNode, NodeBase } from './shared/Node';
+import { ObjectEntity, ObjectProperty } from './shared/ObjectEntity';
 import SpreadElement from './SpreadElement';
 
 export default class ArrayExpression extends NodeBase {
 	elements!: (ExpressionNode | SpreadElement | null)[];
 	type!: NodeType.tArrayExpression;
+	private objectEntity: ObjectEntity | null = null;
 
-	bind() {
-		super.bind();
-		for (const element of this.elements) {
-			if (element !== null) element.deoptimizePath(UNKNOWN_PATH);
-		}
+	deoptimizePath(path: ObjectPath) {
+		this.getObjectEntity().deoptimizePath(path);
+	}
+
+	deoptimizeThisOnEventAtPath(
+		event: NodeEvent,
+		path: ObjectPath,
+		thisParameter: ExpressionEntity,
+		recursionTracker: PathTracker
+	) {
+		this.getObjectEntity().deoptimizeThisOnEventAtPath(
+			event,
+			path,
+			thisParameter,
+			recursionTracker
+		);
+	}
+
+	getLiteralValueAtPath(
+		path: ObjectPath,
+		recursionTracker: PathTracker,
+		origin: DeoptimizableEntity
+	): LiteralValueOrUnknown {
+		return this.getObjectEntity().getLiteralValueAtPath(path, recursionTracker, origin);
 	}
 
-	getReturnExpressionWhenCalledAtPath(path: ObjectPath) {
-		if (path.length !== 1) return UNKNOWN_EXPRESSION;
-		return getMemberReturnExpressionWhenCalled(arrayMembers, path[0]);
+	getReturnExpressionWhenCalledAtPath(
+		path: ObjectPath,
+		callOptions: CallOptions,
+		recursionTracker: PathTracker,
+		origin: DeoptimizableEntity
+	): ExpressionEntity {
+		return this.getObjectEntity().getReturnExpressionWhenCalledAtPath(
+			path,
+			callOptions,
+			recursionTracker,
+			origin
+		);
+	}
+
+	hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext) {
+		return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
 	}
 
-	hasEffectsWhenAccessedAtPath(path: ObjectPath) {
-		return path.length > 1;
+	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext) {
+		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context);
 	}
 
 	hasEffectsWhenCalledAtPath(
 		path: ObjectPath,
 		callOptions: CallOptions,
 		context: HasEffectsContext
 	): boolean {
-		if (path.length === 1) {
-			return hasMemberEffectWhenCalled(arrayMembers, path[0], this.included, callOptions, context);
+		return this.getObjectEntity().hasEffectsWhenCalledAtPath(path, callOptions, context);
+	}
+
+	private getObjectEntity(): ObjectEntity {
+		if (this.objectEntity !== null) {
+			return this.objectEntity;
+		}
+		const properties: ObjectProperty[] = [
+			{ kind: 'init', key: 'length', property: UNKNOWN_LITERAL_NUMBER }
+		];
+		for (let index = 0; index < this.elements.length; index++) {
+			const element = this.elements[index];
+			if (element instanceof SpreadElement) {
+				properties.unshift({ kind: 'init', key: UnknownInteger, property: element });
+			} else if (!element) {
+				properties.push({ kind: 'init', key: String(index), property: UNDEFINED_EXPRESSION });
+			} else {
+				properties.push({ kind: 'init', key: String(index), property: element });
+			}
 		}
-		return true;
+		return (this.objectEntity = new ObjectEntity(properties, ARRAY_PROTOTYPE));
 	}
 }

src/ast/nodes/ArrayPattern.ts

@@ -1,8 +1,8 @@
 import { HasEffectsContext } from '../ExecutionContext';
 import { EMPTY_PATH, ObjectPath } from '../utils/PathTracker';
-import { UNKNOWN_EXPRESSION } from '../values';
 import Variable from '../variables/Variable';
 import * as NodeType from './NodeType';
+import { UNKNOWN_EXPRESSION } from './shared/Expression';
 import { NodeBase } from './shared/Node';
 import { PatternNode } from './shared/Pattern';
 

src/ast/nodes/ArrowFunctionExpression.ts

@@ -3,11 +3,11 @@ import { BROKEN_FLOW_NONE, HasEffectsContext, InclusionContext } from '../Execut
 import ReturnValueScope from '../scopes/ReturnValueScope';
 import Scope from '../scopes/Scope';
 import { ObjectPath, UnknownKey, UNKNOWN_PATH } from '../utils/PathTracker';
-import { UNKNOWN_EXPRESSION } from '../values';
 import BlockStatement from './BlockStatement';
 import Identifier from './Identifier';
 import * as NodeType from './NodeType';
 import RestElement from './RestElement';
+import { UNKNOWN_EXPRESSION } from './shared/Expression';
 import { ExpressionNode, GenericEsTreeNode, IncludeChildren, NodeBase } from './shared/Node';
 import { PatternNode } from './shared/Pattern';
 import SpreadElement from './SpreadElement';
@@ -31,6 +31,9 @@ export default class ArrowFunctionExpression extends NodeBase {
 		}
 	}
 
+	// Arrow functions do not mutate their context
+	deoptimizeThisOnEventAtPath() {}
+
 	getReturnExpressionWhenCalledAtPath(path: ObjectPath) {
 		return path.length === 0 ? this.scope.getReturnExpression() : UNKNOWN_EXPRESSION;
 	}
@@ -98,10 +101,6 @@ export default class ArrowFunctionExpression extends NodeBase {
 		}
 	}
 
-	mayModifyThisWhenCalledAtPath() {
-		return false;
-	}
-
 	parseNode(esTreeNode: GenericEsTreeNode) {
 		if (esTreeNode.body.type === NodeType.BlockStatement) {
 			this.body = new this.context.nodeConstructors.BlockStatement(

src/ast/nodes/AssignmentExpression.ts

@@ -33,7 +33,7 @@ export default class AssignmentExpression extends NodeBase {
 		| '**=';
 	right!: ExpressionNode;
 	type!: NodeType.tAssignmentExpression;
-	private deoptimized = false;
+	protected deoptimized = false;
 
 	hasEffects(context: HasEffectsContext): boolean {
 		if (!this.deoptimized) this.applyDeoptimizations();
@@ -122,7 +122,7 @@ export default class AssignmentExpression extends NodeBase {
 		}
 	}
 
-	private applyDeoptimizations() {
+	protected applyDeoptimizations() {
 		this.deoptimized = true;
 		this.left.deoptimizePath(EMPTY_PATH);
 		this.right.deoptimizePath(UNKNOWN_PATH);