A better fix for no-instanceof-array (#1020)

Author: fisker

rules/no-instanceof-array.js

@@ -1,5 +1,9 @@
 'use strict';
+const {isParenthesized, isOpeningParenToken, isClosingParenToken} = require('eslint-utils');
 const getDocumentationUrl = require('./utils/get-documentation-url');
+const replaceNodeOrTokenAndSpacesBefore = require('./utils/replace-node-or-token-and-spaces-before');
+
+const isInstanceofToken = token => token.value === 'instanceof' && token.type === 'Keyword';
 
 const MESSAGE_ID = 'no-instanceof-array';
 const messages = {
@@ -19,10 +23,23 @@ const create = context => {
 		[selector]: node => context.report({
 			node,
 			messageId: MESSAGE_ID,
-			fix: fixer => fixer.replaceText(
-				node,
-				`Array.isArray(${sourceCode.getText(node.left)})`
-			)
+			* fix(fixer) {
+				const {left, right} = node;
+
+				let leftStartNodeOrToken = left;
+				let leftEndNodeOrToken = left;
+				if (isParenthesized(left, sourceCode)) {
+					leftStartNodeOrToken = sourceCode.getTokenBefore(left, isOpeningParenToken);
+					leftEndNodeOrToken = sourceCode.getTokenAfter(left, isClosingParenToken);
+				}
+
+				yield fixer.insertTextBefore(leftStartNodeOrToken, 'Array.isArray(');
+				yield fixer.insertTextAfter(leftEndNodeOrToken, ')');
+
+				const instanceofToken = sourceCode.getTokenAfter(left, isInstanceofToken);
+				yield * replaceNodeOrTokenAndSpacesBefore(instanceofToken, '', fixer, sourceCode);
+				yield * replaceNodeOrTokenAndSpacesBefore(right, '', fixer, sourceCode);
+			}
 		})
 	};
 };

rules/utils/get-parenthesized-times.js

@@ -0,0 +1,13 @@
+'use strict';
+const {isParenthesized} = require('eslint-utils');
+
+const getParenthesizedTimes = (node, sourceCode) => {
+	let times = 0;
+	while (isParenthesized(times + 1, node, sourceCode)) {
+		times++;
+	}
+
+	return times;
+};
+
+module.exports = getParenthesizedTimes;

rules/utils/replace-node-or-token-and-spaces-before.js

@@ -0,0 +1,31 @@
+'use strict';
+const {isOpeningParenToken, isClosingParenToken} = require('eslint-utils');
+const getParenthesizedTimes = require('./get-parenthesized-times');
+
+function * replaceNodeOrTokenAndSpacesBefore(nodeOrToken, replacement, fixer, sourceCode) {
+	const parenthesizedTimes = getParenthesizedTimes(nodeOrToken, sourceCode);
+
+	if (parenthesizedTimes > 0) {
+		let lastBefore = nodeOrToken;
+		let lastAfter = nodeOrToken;
+		for (let index = 0; index < parenthesizedTimes; index++) {
+			const openingParenthesisToken = sourceCode.getTokenBefore(lastBefore, isOpeningParenToken);
+			const closingParenthesisToken = sourceCode.getTokenAfter(lastAfter, isClosingParenToken);
+			yield * replaceNodeOrTokenAndSpacesBefore(openingParenthesisToken, '', fixer, sourceCode);
+			yield * replaceNodeOrTokenAndSpacesBefore(closingParenthesisToken, '', fixer, sourceCode);
+			lastBefore = openingParenthesisToken;
+			lastAfter = closingParenthesisToken;
+		}
+	}
+
+	let [start, end] = nodeOrToken.range;
+
+	const textBefore = sourceCode.text.slice(0, start);
+	const [trailingSpaces] = textBefore.match(/\s*$/);
+	const [lineBreak] = trailingSpaces.match(/(?:\r?\n|\r){0,1}/);
+	start -= trailingSpaces.length;
+
+	yield fixer.replaceTextRange([start, end], `${lineBreak}${replacement}`);
+}
+
+module.exports = replaceNodeOrTokenAndSpacesBefore;

test/no-instanceof-array.js

@@ -1,12 +1,7 @@
+import {outdent} from 'outdent';
 import {test} from './utils/test.js';
 
-const errors = [
-	{
-		messageId: 'no-instanceof-array'
-	}
-];
-
-test({
+test.visualize({
 	valid: [
 		'Array.isArray(arr)',
 		'arr instanceof Object',
@@ -18,39 +13,47 @@ test({
 		'"arr instanceof Array"'
 	],
 	invalid: [
-		{
-			code: 'arr instanceof Array',
-			output: 'Array.isArray(arr)',
-			errors
-		},
-		{
-			code: '[] instanceof Array',
-			output: 'Array.isArray([])',
-			errors
-		},
-		{
-			code: '[1,2,3] instanceof Array === true',
-			output: 'Array.isArray([1,2,3]) === true',
-			errors
-		},
-		{
-			code: 'fun.call(1, 2, 3) instanceof Array',
-			output: 'Array.isArray(fun.call(1, 2, 3))',
-			errors
-		},
-		{
-			code: 'obj.arr instanceof Array',
-			output: 'Array.isArray(obj.arr)',
-			errors
-		},
-		{
-			code: 'foo.bar[2] instanceof Array',
-			output: 'Array.isArray(foo.bar[2])',
-			errors
-		}
+		'arr instanceof Array',
+		'[] instanceof Array',
+		'[1,2,3] instanceof Array === true',
+		'fun.call(1, 2, 3) instanceof Array',
+		'obj.arr instanceof Array',
+		'foo.bar[2] instanceof Array',
+		'(0, array) instanceof Array',
+		outdent`
+			(
+				// comment
+				((
+					// comment
+					(
+						// comment
+						foo
+						// comment
+					)
+					// comment
+				))
+				// comment
+			)
+			// comment before instanceof\r      instanceof
+
+			// comment after instanceof
+
+			(
+				// comment
+
+				(
+
+					// comment
+
+					Array
+
+					// comment
+				)
+
+					// comment
+			)
+
+			// comment
+		`
 	]
 });
-
-test.visualize([
-	'if (arr instanceof Array) {}'
-]);