Streamline some remaining forEach and some loops

rollup · Apr 16, 2018 · 93f75b6 · 93f75b6
1 parent 67e4080
commit 93f75b6
Show file tree

Hide file tree

Showing 14 changed files with 104 additions and 100 deletions.
diff --git a/src/ast/nodes/ArrayPattern.ts b/src/ast/nodes/ArrayPattern.ts
@@ -18,10 +18,11 @@ export default class ArrayPattern extends NodeBase implements PatternNode {
 	}
 
 	hasEffectsWhenAssignedAtPath(path: ObjectPath, options: ExecutionPathOptions) {
-		return (
-			path.length > 0 ||
-			this.elements.some(child => child && child.hasEffectsWhenAssignedAtPath([], options))
-		);
+		if (path.length > 0) return true;
+		for (const element of this.elements) {
+			if (element !== null && element.hasEffectsWhenAssignedAtPath([], options)) return true;
+		}
+		return false;
 	}
 
 	reassignPath(path: ObjectPath, options: ExecutionPathOptions) {

diff --git a/src/ast/nodes/ArrowFunctionExpression.ts b/src/ast/nodes/ArrowFunctionExpression.ts
@@ -57,7 +57,10 @@ export default class ArrowFunctionExpression extends NodeBase {
 		if (path.length > 0) {
 			return true;
 		}
-		return this.params.some(param => param.hasEffects(options)) || this.body.hasEffects(options);
+		for (const param of this.params) {
+			if (param.hasEffects(options)) return true;
+		}
+		return this.body.hasEffects(options);
 	}
 
 	initialise() {

diff --git a/src/ast/nodes/BlockStatement.ts b/src/ast/nodes/BlockStatement.ts
@@ -27,7 +27,9 @@ export default class BlockStatement extends StatementBase {
 	}
 
 	hasEffects(options: ExecutionPathOptions) {
-		return this.body.some(child => child.hasEffects(options));
+		for (const node of this.body) {
+			if (node.hasEffects(options)) return true;
+		}
 	}
 
 	include() {

diff --git a/src/ast/nodes/CallExpression.ts b/src/ast/nodes/CallExpression.ts
@@ -59,13 +59,13 @@ export default class CallExpression extends NodeBase {
 	}
 
 	hasEffects(options: ExecutionPathOptions): boolean {
-		return (
-			this.arguments.some(child => child.hasEffects(options)) ||
-			this.callee.hasEffectsWhenCalledAtPath(
-				[],
-				this.callOptions,
-				options.getHasEffectsWhenCalledOptions()
-			)
+		for (const argument of this.arguments) {
+			if (argument.hasEffects(options)) return true;
+		}
+		return this.callee.hasEffectsWhenCalledAtPath(
+			[],
+			this.callOptions,
+			options.getHasEffectsWhenCalledOptions()
 		);
 	}
 

diff --git a/src/ast/nodes/NewExpression.ts b/src/ast/nodes/NewExpression.ts
@@ -12,13 +12,13 @@ export default class NewExpression extends NodeBase {
 	private callOptions: CallOptions;
 
 	hasEffects(options: ExecutionPathOptions): boolean {
-		return (
-			this.arguments.some(child => child.hasEffects(options)) ||
-			this.callee.hasEffectsWhenCalledAtPath(
-				[],
-				this.callOptions,
-				options.getHasEffectsWhenCalledOptions()
-			)
+		for (const argument of this.arguments) {
+			if (argument.hasEffects(options)) return true;
+		}
+		return this.callee.hasEffectsWhenCalledAtPath(
+			[],
+			this.callOptions,
+			options.getHasEffectsWhenCalledOptions()
 		);
 	}
 

diff --git a/src/ast/nodes/ObjectExpression.ts b/src/ast/nodes/ObjectExpression.ts
@@ -52,10 +52,11 @@ export default class ObjectExpression extends NodeBase {
 			path[0],
 			PROPERTY_KINDS_READ
 		);
-		return (
-			(path.length > 1 && !hasCertainHit) ||
-			properties.some(property => property.hasEffectsWhenAccessedAtPath(path.slice(1), options))
-		);
+		if (path.length > 1 && !hasCertainHit) return true;
+		for (const property of properties) {
+			if (property.hasEffectsWhenAccessedAtPath(path.slice(1), options)) return true;
+		}
+		return false;
 	}
 
 	hasEffectsWhenAssignedAtPath(path: ObjectPath, options: ExecutionPathOptions) {
@@ -65,14 +66,15 @@ export default class ObjectExpression extends NodeBase {
 			path[0],
 			path.length === 1 ? PROPERTY_KINDS_WRITE : PROPERTY_KINDS_READ
 		);
-		return (
-			(path.length > 1 && !hasCertainHit) ||
-			properties.some(
-				property =>
-					(path.length > 1 || property.kind === 'set') &&
-					property.hasEffectsWhenAssignedAtPath(path.slice(1), options)
+		if (path.length > 1 && !hasCertainHit) return true;
+		for (const property of properties) {
+			if (
+				(path.length > 1 || property.kind === 'set') &&
+				property.hasEffectsWhenAssignedAtPath(path.slice(1), options)
 			)
-		);
+				return true;
+		}
+		return false;
 	}
 
 	hasEffectsWhenCalledAtPath(
@@ -90,12 +92,11 @@ export default class ObjectExpression extends NodeBase {
 			path[0],
 			PROPERTY_KINDS_READ
 		);
-		return (
-			!hasCertainHit ||
-			properties.some(property =>
-				property.hasEffectsWhenCalledAtPath(path.slice(1), callOptions, options)
-			)
-		);
+		if (!hasCertainHit) return true;
+		for (const property of properties) {
+			if (property.hasEffectsWhenCalledAtPath(path.slice(1), callOptions, options)) return true;
+		}
+		return false;
 	}
 
 	reassignPath(path: ObjectPath, options: ExecutionPathOptions) {
@@ -142,17 +143,19 @@ export default class ObjectExpression extends NodeBase {
 			subPath,
 			PROPERTY_KINDS_READ
 		);
-		return (
-			!hasCertainHit ||
-			properties.some(property =>
+		if (!hasCertainHit) return true;
+		for (const property of properties) {
+			if (
 				property.someReturnExpressionWhenCalledAtPath(
 					path.slice(1),
 					callOptions,
 					predicateFunction,
 					options
 				)
 			)
-		);
+				return true;
+		}
+		return false;
 	}
 
 	private getPossiblePropertiesWithName(name: ObjectPathKey, kinds: ObjectPath) {

diff --git a/src/ast/nodes/ObjectPattern.ts b/src/ast/nodes/ObjectPattern.ts
@@ -17,10 +17,11 @@ export default class ObjectPattern extends NodeBase implements PatternNode {
 	}
 
 	hasEffectsWhenAssignedAtPath(path: ObjectPath, options: ExecutionPathOptions) {
-		return (
-			path.length > 0 ||
-			this.properties.some(child => child.hasEffectsWhenAssignedAtPath([], options))
-		);
+		if (path.length > 0) return true;
+		for (const property of this.properties) {
+			if (property.hasEffectsWhenAssignedAtPath([], options)) return true;
+		}
+		return false;
 	}
 
 	reassignPath(path: ObjectPath, options: ExecutionPathOptions) {

diff --git a/src/ast/nodes/SequenceExpression.ts b/src/ast/nodes/SequenceExpression.ts
@@ -36,7 +36,10 @@ export default class SequenceExpression extends NodeBase {
 	}
 
 	hasEffects(options: ExecutionPathOptions): boolean {
-		return this.expressions.some(expression => expression.hasEffects(options));
+		for (const expression of this.expressions) {
+			if (expression.hasEffects(options)) return true;
+		}
+		return false;
 	}
 
 	hasEffectsWhenAccessedAtPath(path: ObjectPath, options: ExecutionPathOptions): boolean {

diff --git a/src/ast/nodes/shared/FunctionNode.ts b/src/ast/nodes/shared/FunctionNode.ts
@@ -69,10 +69,10 @@ export default class FunctionNode extends NodeBase {
 			return true;
 		}
 		const innerOptions = this.scope.getOptionsWhenCalledWith(callOptions, options);
-		return (
-			this.params.some(param => param.hasEffects(innerOptions)) ||
-			this.body.hasEffects(innerOptions)
-		);
+		for (const param of this.params) {
+			if (param.hasEffects(innerOptions)) return true;
+		}
+		return this.body.hasEffects(innerOptions);
 	}
 
 	include() {

diff --git a/src/ast/nodes/shared/Node.ts b/src/ast/nodes/shared/Node.ts
@@ -229,9 +229,9 @@ export class NodeBase implements ExpressionNode {
 	}
 
 	parseNode(esTreeNode: GenericEsTreeNode, nodeConstructors: { [p: string]: typeof NodeBase }) {
-		for (const key in esTreeNode) {
+		for (const key of Object.keys(esTreeNode)) {
 			// That way, we can override this function to add custom initialisation and then call super.parseNode
-			if (key in this) continue;
+			if (this.hasOwnProperty(key)) continue;
 			const value = esTreeNode[key];
 			if (typeof value !== 'object' || value === null) {
 				(<GenericEsTreeNode>this)[key] = value;

diff --git a/src/ast/scopes/ModuleScope.ts b/src/ast/scopes/ModuleScope.ts
@@ -8,6 +8,14 @@ import Variable from '../variables/Variable';
 import { isNamespaceVariable } from '../variables/NamespaceVariable';
 import { isExternalVariable } from '../variables/ExternalVariable';
 
+const addDeclaredNames = (declaration: Variable, names: Set<string>) => {
+	if (isNamespaceVariable(declaration) && !isExternalVariable(declaration)) {
+		for (const name of declaration.module.getExports())
+			addDeclaredNames(declaration.module.traceExport(name), names);
+	}
+	names.add(declaration.getName());
+};
+
 export default class ModuleScope extends Scope {
 	parent: Scope;
 	module: Module;
@@ -25,26 +33,13 @@ export default class ModuleScope extends Scope {
 	deshadow(names: Set<string>, children = this.children) {
 		let localNames = new Set(names);
 
-		Object.keys(this.module.imports).forEach(importName => {
+		for (const importName of Object.keys(this.module.imports)) {
 			const specifier = this.module.imports[importName];
 
-			if (specifier.module.isExternal || specifier.module.chunk !== this.module.chunk) {
-				return;
-			}
-
-			const addDeclaration = (declaration: Variable) => {
-				if (isNamespaceVariable(declaration) && !isExternalVariable(declaration)) {
-					declaration.module
-						.getExports()
-						.forEach(name => addDeclaration(declaration.module.traceExport(name)));
-				}
-
-				localNames.add(declaration.getName());
-			};
+			if (specifier.module.isExternal || specifier.module.chunk !== this.module.chunk) continue;
 
-			(<Module>specifier.module).getAllExports().forEach(name => {
-				addDeclaration(specifier.module.traceExport(name));
-			});
+			for (const name of (<Module>specifier.module).getAllExports())
+				addDeclaredNames(specifier.module.traceExport(name), localNames);
 
 			if (specifier.name !== '*') {
 				const declaration = specifier.module.traceExport(specifier.name);
@@ -60,7 +55,7 @@ export default class ModuleScope extends Scope {
 						},
 						specifier.specifier.start
 					);
-					return;
+					continue;
 				}
 
 				const name = declaration.getName();
@@ -75,7 +70,7 @@ export default class ModuleScope extends Scope {
 					localNames.add((<ImportSpecifier>specifier.specifier).imported.name);
 				}
 			}
-		});
+		}
 
 		super.deshadow(localNames, children);
 	}

diff --git a/src/ast/scopes/Scope.ts b/src/ast/scopes/Scope.ts
@@ -81,19 +81,15 @@ export default class Scope {
 	}
 
 	deshadow(names: Set<string>, children = this.children) {
-		Object.keys(this.variables).forEach(key => {
+		for (const key of Object.keys(this.variables)) {
 			const declaration = this.variables[key];
 
 			// we can disregard exports.foo etc
-			if (declaration.exportName && declaration.isReassigned && !declaration.isId) return;
-
-			if (declaration.isDefault) return;
+			if (declaration.exportName && declaration.isReassigned && !declaration.isId) continue;
+			if (declaration.isDefault) continue;
 
 			let name = declaration.getName(true);
-
-			if (!names.has(name)) {
-				return;
-			}
+			if (!names.has(name)) continue;
 
 			name = declaration.name;
 			let deshadowed,
@@ -103,9 +99,9 @@ export default class Scope {
 			} while (names.has(deshadowed));
 
 			declaration.setSafeName(deshadowed);
-		});
+		}
 
-		children.forEach(scope => scope.deshadow(names));
+		for (const scope of children) scope.deshadow(names);
 	}
 
 	findLexicalBoundary(): Scope {

diff --git a/src/ast/values.ts b/src/ast/values.ts
@@ -349,24 +349,24 @@ export function hasMemberEffectWhenCalled(
 	callOptions: CallOptions,
 	options: ExecutionPathOptions
 ) {
-	return (
-		isUnknownKey(memberName) ||
-		!members[memberName] ||
-		(members[memberName].callsArgs &&
-			members[memberName].callsArgs.some(
-				argIndex =>
-					callOptions.args[argIndex] &&
-					callOptions.args[argIndex].hasEffectsWhenCalledAtPath(
-						[],
-						CallOptions.create({
-							withNew: false,
-							args: [],
-							callIdentifier: {} // make sure the caller is unique to avoid this check being ignored
-						}),
-						options.getHasEffectsWhenCalledOptions()
-					)
-			))
-	);
+	if (isUnknownKey(memberName) || !members[memberName]) return true;
+	if (!members[memberName].callsArgs) return false;
+	for (const argIndex of members[memberName].callsArgs) {
+		if (
+			callOptions.args[argIndex] &&
+			callOptions.args[argIndex].hasEffectsWhenCalledAtPath(
+				[],
+				CallOptions.create({
+					withNew: false,
+					args: [],
+					callIdentifier: {} // make sure the caller is unique to avoid this check being ignored
+				}),
+				options.getHasEffectsWhenCalledOptions()
+			)
+		)
+			return true;
+	}
+	return false;
 }
 
 export function someMemberReturnExpressionWhenCalled(

diff --git a/src/ast/variables/NamespaceVariable.ts b/src/ast/variables/NamespaceVariable.ts
@@ -42,7 +42,7 @@ export default class NamespaceVariable extends Variable {
 				break;
 			}
 		}
-		Object.keys(this.originals).forEach(original => this.originals[original].include());
+		for (const original of Object.keys(this.originals)) this.originals[original].include();
 		return true;
 	}