Better fix for new-for-builtins (#1022)

Author: fisker

rules/new-for-builtins.js

@@ -2,6 +2,7 @@
 const getDocumentationUrl = require('./utils/get-documentation-url');
 const builtins = require('./utils/builtins');
 const isShadowed = require('./utils/is-shadowed');
+const isNewExpressionWithParentheses = require('./utils/is-new-expression-with-parentheses');
 
 const messages = {
 	enforce: 'Use `new {{name}}()` instead of `{{name}}()`.',
@@ -12,6 +13,8 @@ const enforceNew = new Set(builtins.enforceNew);
 const disallowNew = new Set(builtins.disallowNew);
 
 const create = context => {
+	const sourceCode = context.getSourceCode();
+
 	return {
 		CallExpression: node => {
 			const {callee, parent} = node;
@@ -38,7 +41,7 @@ const create = context => {
 		},
 		NewExpression: node => {
 			const {callee, range} = node;
-			const {name, range: calleeRange} = callee;
+			const {name} = callee;
 
 			if (disallowNew.has(name) && !isShadowed(context.getScope(), callee)) {
 				const problem = {
@@ -48,10 +51,19 @@ const create = context => {
 				};
 
 				if (name !== 'String' && name !== 'Boolean' && name !== 'Number') {
-					problem.fix = fixer => fixer.removeRange([
-						range[0],
-						calleeRange[0]
-					]);
+					problem.fix = function * (fixer) {
+						const [start] = range;
+						let end = start + 3; // `3` = length of `new`
+						const textAfter = sourceCode.text.slice(end);
+						const [leadingSpaces] = textAfter.match(/^\s*/);
+						end += leadingSpaces.length;
+
+						yield fixer.removeRange([start, end]);
+
+						if (!isNewExpressionWithParentheses(node, sourceCode)) {
+							yield fixer.insertTextAfter(node, '()');
+						}
+					};
 				}
 
 				context.report(problem);

rules/utils/is-new-expression-with-parentheses.js

@@ -0,0 +1,26 @@
+'use strict';
+
+const {isOpeningParenToken, isClosingParenToken} = require('eslint-utils');
+
+/**
+Determine if a constructor function is newed-up with parens.
+
+@param {Node} node - The `NewExpression` node to be checked.
+@param {SourceCode} sourceCode - The source code object.
+@returns {boolean} True if the constructor is called with parens.
+
+Copied from https://github.com/eslint/eslint/blob/cc4871369645c3409dc56ded7a555af8a9f63d51/lib/rules/no-extra-parens.js#L252
+*/
+function isNewExpressionWithParentheses(node, sourceCode) {
+	if (node.arguments.length > 0) {
+		return true;
+	}
+
+	const [penultimateToken, lastToken] = sourceCode.getLastTokens(node, 2);
+	// The expression should end with its own parens, for example, `new new Foo()` is not a new expression with parens.
+	return isOpeningParenToken(penultimateToken) &&
+		isClosingParenToken(lastToken) &&
+		node.callee.range[1] < node.range[1];
+}
+
+module.exports = isNewExpressionWithParentheses;

rules/utils/should-add-parentheses-to-member-expression-object.js

@@ -1,6 +1,6 @@
 'use strict';
 
-const {isOpeningParenToken, isClosingParenToken} = require('eslint-utils');
+const isNewExpressionWithParentheses = require('./is-new-expression-with-parentheses');
 
 // Determine whether this node is a decimal integer literal.
 // Copied from https://github.com/eslint/eslint/blob/cc4871369645c3409dc56ded7a555af8a9f63d51/lib/rules/utils/ast-utils.js#L1237
@@ -10,27 +10,6 @@ const isDecimalInteger = node =>
 	typeof node.value === 'number' &&
 	DECIMAL_INTEGER_PATTERN.test(node.raw);
 
-/**
-Determine if a constructor function is newed-up with parens.
-
-@param {Node} node - The `NewExpression` node to be checked.
-@param {SourceCode} sourceCode - The source code object.
-@returns {boolean} True if the constructor is called with parens.
-
-Copied from https://github.com/eslint/eslint/blob/cc4871369645c3409dc56ded7a555af8a9f63d51/lib/rules/no-extra-parens.js#L252
-*/
-function isNewExpressionWithParentheses(node, sourceCode) {
-	if (node.arguments.length > 0) {
-		return true;
-	}
-
-	const [penultimateToken, lastToken] = sourceCode.getLastTokens(node, 2);
-	// The expression should end with its own parens, for example, `new new Foo()` is not a new expression with parens.
-	return isOpeningParenToken(penultimateToken) &&
-		isClosingParenToken(lastToken) &&
-		node.callee.range[1] < node.range[1];
-}
-
 /**
 Check if parentheses should to be added to a `node` when it's used as an `object` of `MemberExpression`.
 

test/new-for-builtins.js

@@ -321,3 +321,13 @@ test({
 		}
 	]
 });
+
+test.visualize({
+	valid: [],
+	invalid: [
+		'const object = (Object)();',
+		'const symbol = new (Symbol)("");',
+		'const symbol = new /* comment */ Symbol("");',
+		'const symbol = new Symbol;'
+	]
+});

test/snapshots/new-for-builtins.js.md

@@ -0,0 +1,69 @@
+# Snapshot report for `test/new-for-builtins.js`
+
+The actual snapshot is saved in `new-for-builtins.js.snap`.
+
+Generated by [AVA](https://avajs.dev).
+
+## Invalid #1
+      1 | const object = (Object)();
+
+> Output
+
+    `␊
+      1 | const object = new (Object)();␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | const object = (Object)();␊
+        |                ^^^^^^^^^^ Use `new Object()` instead of `Object()`.␊
+    `
+
+## Invalid #2
+      1 | const symbol = new (Symbol)("");
+
+> Output
+
+    `␊
+      1 | const symbol = (Symbol)("");␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | const symbol = new (Symbol)("");␊
+        |                ^^^^^^^^^^^^^^^^ Use `Symbol()` instead of `new Symbol()`.␊
+    `
+
+## Invalid #3
+      1 | const symbol = new /* comment */ Symbol("");
+
+> Output
+
+    `␊
+      1 | const symbol = /* comment */ Symbol("");␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | const symbol = new /* comment */ Symbol("");␊
+        |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Symbol()` instead of `new Symbol()`.␊
+    `
+
+## Invalid #4
+      1 | const symbol = new Symbol;
+
+> Output
+
+    `␊
+      1 | const symbol = Symbol();␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | const symbol = new Symbol;␊
+        |                ^^^^^^^^^^ Use `Symbol()` instead of `new Symbol()`.␊
+    `