new-for-builtins: Refactor to use ReferenceTracker (#1829)

sindresorhus · May 26, 2022 · aee56fd · aee56fd
1 parent c9a572d
commit aee56fd
Show file tree

Hide file tree

Showing 12 changed files with 340 additions and 75 deletions.
diff --git a/rules/new-for-builtins.js b/rules/new-for-builtins.js
@@ -1,7 +1,6 @@
 'use strict';
+const {ReferenceTracker} = require('eslint-utils');
 const builtins = require('./utils/builtins.js');
-const isShadowed = require('./utils/is-shadowed.js');
-const {callExpressionSelector, newExpressionSelector} = require('./selectors/index.js');
 const {
 	switchCallExpressionToNewExpression,
 	switchNewExpressionToCallExpression,
@@ -12,60 +11,64 @@ const messages = {
 	disallow: 'Use `{{name}}()` instead of `new {{name}}()`.',
 };
 
-/** @param {import('eslint').Rule.RuleContext} context */
-const create = context => {
-	const sourceCode = context.getSourceCode();
-
-	return {
-		[callExpressionSelector(builtins.enforceNew)](node) {
-			const {callee, parent} = node;
-			if (isShadowed(context.getScope(), callee)) {
-				return;
-			}
-
-			const {name} = callee;
+function * enforceNewExpression({sourceCode, tracker}) {
+	const traceMap = Object.fromEntries(
+		builtins.enforceNew.map(name => [name, {[ReferenceTracker.CALL]: true}]),
+	);
 
+	for (const {node, path: [name]} of tracker.iterateGlobalReferences(traceMap)) {
+		if (name === 'Object') {
+			const {parent} = node;
 			if (
-				name === 'Object'
-				&& parent
-				&& parent.type === 'BinaryExpression'
+				parent.type === 'BinaryExpression'
 				&& (parent.operator === '===' || parent.operator === '!==')
 				&& (parent.left === node || parent.right === node)
 			) {
-				return;
+				continue;
 			}
+		}
 
-			return {
-				node,
-				messageId: 'enforce',
-				data: {name},
-				fix: fixer => switchCallExpressionToNewExpression(node, sourceCode, fixer),
-			};
-		},
-		[newExpressionSelector(builtins.disallowNew)](node) {
-			const {callee} = node;
+		yield {
+			node,
+			messageId: 'enforce',
+			data: {name},
+			fix: fixer => switchCallExpressionToNewExpression(node, sourceCode, fixer),
+		};
+	}
+}
 
-			if (isShadowed(context.getScope(), callee)) {
-				return;
-			}
+function * enforceCallExpression({sourceCode, tracker}) {
+	const traceMap = Object.fromEntries(
+		builtins.disallowNew.map(name => [name, {[ReferenceTracker.CONSTRUCT]: true}]),
+	);
 
-			const {name} = callee;
-			const problem = {
-				node,
-				messageId: 'disallow',
-				data: {name},
+	for (const {node, path: [name]} of tracker.iterateGlobalReferences(traceMap)) {
+		const problem = {
+			node,
+			messageId: 'disallow',
+			data: {name},
+		};
+
+		if (name !== 'String' && name !== 'Boolean' && name !== 'Number') {
+			problem.fix = function * (fixer) {
+				yield * switchNewExpressionToCallExpression(node, sourceCode, fixer);
 			};
+		}
 
-			if (name !== 'String' && name !== 'Boolean' && name !== 'Number') {
-				problem.fix = function * (fixer) {
-					yield * switchNewExpressionToCallExpression(node, sourceCode, fixer);
-				};
-			}
+		yield problem;
+	}
+}
 
-			return problem;
-		},
-	};
-};
+/** @param {import('eslint').Rule.RuleContext} context */
+const create = context => ({
+	* 'Program:exit'() {
+		const sourceCode = context.getSourceCode();
+		const tracker = new ReferenceTracker(context.getScope());
+
+		yield * enforceNewExpression({sourceCode, tracker});
+		yield * enforceCallExpression({sourceCode, tracker});
+	},
+});
 
 /** @type {import('eslint').Rule.RuleModule} */
 module.exports = {

diff --git a/test/new-for-builtins.mjs b/test/new-for-builtins.mjs
@@ -14,17 +14,6 @@ const disallowNewError = builtin => ({
 });
 
 test({
-	testerOptions: {
-		// Make sure globals don't effect shadowed check result
-		globals: {
-			Map: 'writable',
-			Set: 'readonly',
-			WeakMap: 'off',
-			BigInt: 'writable',
-			Boolean: 'readonly',
-			Number: 'off',
-		},
-	},
 	valid: [
 		'const foo = new Object()',
 		'const foo = new Array()',
@@ -115,6 +104,7 @@ test({
 		'Foo();new Bar();',
 		// Ignored
 		'const isObject = v => Object(v) === v;',
+		'const isObject = v => globalThis.Object(v) === v;',
 		'(x) !== Object(x)',
 	],
 	invalid: [
@@ -323,7 +313,12 @@ test({
 });
 
 test.snapshot({
-	valid: [],
+	valid: [
+		{
+			code: 'new Symbol("")',
+			globals: {Symbol: 'off'},
+		},
+	],
 	invalid: [
 		'const object = (Object)();',
 		'const symbol = new (Symbol)("");',
@@ -383,5 +378,41 @@ test.snapshot({
 				return new /**/ Symbol;
 			}
 		`,
+		// Trace
+		'new globalThis.String()',
+		'new global.String()',
+		'new self.String()',
+		'new window.String()',
+		outdent`
+			const {String} = globalThis;
+			new String();
+		`,
+		outdent`
+			const {String: RenamedString} = globalThis;
+			new RenamedString();
+		`,
+		outdent`
+			const RenamedString = globalThis.String;
+			new RenamedString();
+		`,
+		'globalThis.Array()',
+		'global.Array()',
+		'self.Array()',
+		'window.Array()',
+		outdent`
+			const {Array: RenamedArray} = globalThis;
+			RenamedArray();
+		`,
+		{
+			code: 'globalThis.Array()',
+			globals: {Array: 'off'},
+		},
+		{
+			code: outdent`
+				const {Array} = globalThis;
+				Array();
+			`,
+			globals: {Array: 'off'},
+		},
 	],
 });
diff --git a/test/prevent-abbreviations.mjs b/test/prevent-abbreviations.mjs
@@ -242,7 +242,7 @@ const tests = {
 	invalid: [
 		{
 			code: 'let e',
-			errors: createErrors('Please rename the variable `e`. Suggested names are: `error`, `event`. A more descriptive name will do too.'),
+			errors: createErrors('Please rename the variable `e`. Suggested names are: `error`, `event_`. A more descriptive name will do too.'),
 		},
 		{
 			code: 'let eCbOpts',
@@ -369,8 +369,8 @@ const tests = {
 
 		{
 			code: 'let evt',
-			output: 'let event',
-			errors: createErrors('The variable `evt` should be named `event`. A more descriptive name will do too.'),
+			output: 'let event_',
+			errors: createErrors('The variable `evt` should be named `event_`. A more descriptive name will do too.'),
 		},
 		{
 			code: '({evt: 1})',
@@ -767,7 +767,7 @@ const tests = {
 		},
 		{
 			code: 'class Res {}',
-			errors: createErrors('Please rename the variable `Res`. Suggested names are: `Response`, `Result`. A more descriptive name will do too.'),
+			errors: createErrors('Please rename the variable `Res`. Suggested names are: `Response_`, `Result`. A more descriptive name will do too.'),
 		},
 		{
 			code: 'const Err = 1;',
@@ -792,7 +792,7 @@ const tests = {
 
 		{
 			code: 'let doc',
-			output: 'let document',
+			output: 'let document_',
 			errors: createErrors(),
 		},
 
@@ -1817,8 +1817,8 @@ test.typescript({
 				}
 			`,
 			output: outdent`
-				function onKeyDown(event: KeyboardEvent) {
-					if (event.keyCode) {}
+				function onKeyDown(event_: KeyboardEvent) {
+					if (event_.keyCode) {}
 				}
 			`,
 			options: [