Make accessing unknown globals a side-effect (#3068)

* Make accessing unknown globals a side-effect; also, introduce global object concept

* Test or remove some edge-cases

* Add option to control unknown global side effects

* Add additional known globals test

* Refactor global declarations to unite access and pureness

docs/999-big-list-of-options.md

@@ -802,7 +802,7 @@ Default: `false`
 If this option is provided, bundling will not fail if bindings are imported from a file that does not define these bindings. Instead, new variables will be created for these bindings with the value `undefined`.
 
 #### treeshake
-Type: `boolean | { annotations?: boolean, moduleSideEffects?: ModuleSideEffectsOption, propertyReadSideEffects?: boolean }`<br>
+Type: `boolean | { annotations?: boolean, moduleSideEffects?: ModuleSideEffectsOption, propertyReadSideEffects?: boolean, tryCatchDeoptimization?: boolean, unknownGlobalSideEffects?: boolean }`<br>
 CLI: `--treeshake`/`--no-treeshake`<br>
 Default: `true`
 
@@ -938,6 +938,29 @@ test(otherFn);
 
 ```
 
+**treeshake.unknownGlobalSideEffects**
+Type: `boolean`<br>
+CLI: `--treeshake.unknownGlobalSideEffects`/`--no-treeshake.unknownGlobalSideEffects`<br>
+Default: `true`
+
+Since accessing a non-existing global variable will throw an error, Rollup does by default retain any accesses to non-builtin global variables. Set this option to `false` to avoid this check. This is probably safe for most code-bases.
+
+```js
+// input
+const jQuery = $;
+const requestTimeout = setTimeout;
+const element = angular.element;
+
+// output with unknownGlobalSideEffects == true
+const jQuery = $;
+const element = angular.element;
+
+// output with unknownGlobalSideEffects == false
+const element = angular.element;
+```
+
+In the example, the last line is always retained as accessing the `element` property could also throw an error if `angular` is e.g. `null`. To avoid this check, set `treeshake.propertyReadSideEffects` to `false` as well.
+
 ### Experimental options
 
 These options reflect new features that have not yet been fully finalized. Availability, behaviour and usage may therefore be subject to change between minor versions.

src/Graph.ts

@@ -116,13 +116,16 @@ export default class Graph {
 							(options.treeshake as TreeshakingOptions).propertyReadSideEffects !== false,
 						pureExternalModules: (options.treeshake as TreeshakingOptions).pureExternalModules,
 						tryCatchDeoptimization:
-							(options.treeshake as TreeshakingOptions).tryCatchDeoptimization !== false
+							(options.treeshake as TreeshakingOptions).tryCatchDeoptimization !== false,
+						unknownGlobalSideEffects:
+							(options.treeshake as TreeshakingOptions).unknownGlobalSideEffects !== false
 				  }
 				: {
 						annotations: true,
 						moduleSideEffects: true,
 						propertyReadSideEffects: true,
-						tryCatchDeoptimization: true
+						tryCatchDeoptimization: true,
+						unknownGlobalSideEffects: true
 				  };
 			if (typeof this.treeshakingOptions.pureExternalModules !== 'undefined') {
 				this.warnDeprecation(

src/Module.ts

@@ -110,6 +110,7 @@ export interface AstContext {
 	traceVariable: (name: string) => Variable | null;
 	treeshake: boolean;
 	tryCatchDeoptimization: boolean;
+	unknownGlobalSideEffects: boolean;
 	usesTopLevelAwait: boolean;
 	warn: (warning: RollupWarning, pos: number) => void;
 	warnDeprecation: (deprecation: string | RollupWarning, activeDeprecation: boolean) => void;
@@ -606,6 +607,8 @@ export default class Module {
 			treeshake: !!this.graph.treeshakingOptions,
 			tryCatchDeoptimization: (!this.graph.treeshakingOptions ||
 				this.graph.treeshakingOptions.tryCatchDeoptimization) as boolean,
+			unknownGlobalSideEffects: (!this.graph.treeshakingOptions ||
+				this.graph.treeshakingOptions.unknownGlobalSideEffects) as boolean,
 			usesTopLevelAwait: false,
 			warn: this.warn.bind(this),
 			warnDeprecation: this.graph.warnDeprecation.bind(this.graph)

src/ast/nodes/Identifier.ts

@@ -7,7 +7,8 @@ import { DeoptimizableEntity } from '../DeoptimizableEntity';
 import { ExecutionPathOptions } from '../ExecutionPathOptions';
 import FunctionScope from '../scopes/FunctionScope';
 import { ImmutableEntityPathTracker } from '../utils/ImmutableEntityPathTracker';
-import { LiteralValueOrUnknown, ObjectPath, UNKNOWN_EXPRESSION, UNKNOWN_VALUE } from '../values';
+import { EMPTY_PATH, LiteralValueOrUnknown, ObjectPath } from '../values';
+import GlobalVariable from '../variables/GlobalVariable';
 import LocalVariable from '../variables/LocalVariable';
 import Variable from '../variables/Variable';
 import * as NodeType from './NodeType';
@@ -62,24 +63,20 @@ export default class Identifier extends NodeBase implements PatternNode {
 			case 'parameter':
 				variable = (this.scope as FunctionScope).addParameterDeclaration(this);
 				break;
+			/* istanbul ignore next */
 			default:
-				throw new Error(`Unexpected identifier kind ${kind}.`);
+				/* istanbul ignore next */
+				throw new Error(`Internal Error: Unexpected identifier kind ${kind}.`);
 		}
 		return [(this.variable = variable)];
 	}
 
 	deoptimizePath(path: ObjectPath) {
 		if (!this.bound) this.bind();
-		if (this.variable !== null) {
-			if (
-				path.length === 0 &&
-				this.name in this.context.importDescriptions &&
-				!this.scope.contains(this.name)
-			) {
-				this.disallowImportReassignment();
-			}
-			this.variable.deoptimizePath(path);
+		if (path.length === 0 && !this.scope.contains(this.name)) {
+			this.disallowImportReassignment();
 		}
+		(this.variable as Variable).deoptimizePath(path);
 	}
 
 	getLiteralValueAtPath(
@@ -88,10 +85,7 @@ export default class Identifier extends NodeBase implements PatternNode {
 		origin: DeoptimizableEntity
 	): LiteralValueOrUnknown {
 		if (!this.bound) this.bind();
-		if (this.variable !== null) {
-			return this.variable.getLiteralValueAtPath(path, recursionTracker, origin);
-		}
-		return UNKNOWN_VALUE;
+		return (this.variable as Variable).getLiteralValueAtPath(path, recursionTracker, origin);
 	}
 
 	getReturnExpressionWhenCalledAtPath(
@@ -100,10 +94,19 @@ export default class Identifier extends NodeBase implements PatternNode {
 		origin: DeoptimizableEntity
 	) {
 		if (!this.bound) this.bind();
-		if (this.variable !== null) {
-			return this.variable.getReturnExpressionWhenCalledAtPath(path, recursionTracker, origin);
-		}
-		return UNKNOWN_EXPRESSION;
+		return (this.variable as Variable).getReturnExpressionWhenCalledAtPath(
+			path,
+			recursionTracker,
+			origin
+		);
+	}
+
+	hasEffects(): boolean {
+		return (
+			this.context.unknownGlobalSideEffects &&
+			this.variable instanceof GlobalVariable &&
+			this.variable.hasEffectsWhenAccessedAtPath(EMPTY_PATH)
+		);
 	}
 
 	hasEffectsWhenAccessedAtPath(path: ObjectPath, options: ExecutionPathOptions): boolean {
@@ -132,9 +135,7 @@ export default class Identifier extends NodeBase implements PatternNode {
 	}
 
 	includeCallArguments(args: (ExpressionNode | SpreadElement)[]): void {
-		if (this.variable) {
-			this.variable.includeCallArguments(args);
-		}
+		(this.variable as Variable).includeCallArguments(args);
 	}
 
 	render(

src/ast/nodes/shared/FunctionNode.ts

@@ -41,8 +41,8 @@ export default class FunctionNode extends NodeBase {
 		return path.length === 0 ? this.scope.getReturnExpression() : UNKNOWN_EXPRESSION;
 	}
 
-	hasEffects(options: ExecutionPathOptions) {
-		return this.id !== null && this.id.hasEffects(options);
+	hasEffects() {
+		return this.id !== null && this.id.hasEffects();
 	}
 
 	hasEffectsWhenAccessedAtPath(path: ObjectPath) {