From 9e4d896acdc8e63b54ad9392722604e4053f3af9 Mon Sep 17 00:00:00 2001 From: fisker Date: Mon, 4 May 2020 20:31:48 +0800 Subject: [PATCH 01/20] Add tests --- test/no-useless-undefined.js | 121 +++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 test/no-useless-undefined.js diff --git a/test/no-useless-undefined.js b/test/no-useless-undefined.js new file mode 100644 index 0000000000..13b4a38af1 --- /dev/null +++ b/test/no-useless-undefined.js @@ -0,0 +1,121 @@ +import test from 'ava'; +import avaRuleTester from 'eslint-ava-rule-tester'; +import rule from '../rules/no-useless-undefined'; + +const messageId = 'no-useless-undefined'; + +const ruleTester = avaRuleTester(test, { + parserOptions: { + ecmaVersion: 2020 + } +}); + +const errors = [{messageId}]; + +ruleTester.run('better-regex', rule, { + valid: [ + 'function foo() {return;}', + 'const foo = () => {};', + 'let foo;', + 'var foo;', + 'const foo = undefined;', + 'foo();', + 'foo(bar,);', + 'foo(undefined, bar);', + 'const {foo} = {};', + 'function foo({bar} = {}) {}', + 'function foo(bar) {}' + ], + invalid: [ + { + code: 'function foo() {return undefined;}', + output: 'function foo() {return ;}', + errors + }, + { + code: 'const foo = () => undefined;', + output: 'const foo = () => {};', + errors + }, + { + code: 'const foo = () => {return undefined;};', + output: 'const foo = () => {return ;};', + errors + }, + { + code: 'function* foo() {yield undefined;}', + output: 'function* foo() {yield ;}', + errors + }, + { + code: 'let a = undefined;', + output: 'let a;', + errors + }, + { + code: 'let a = undefined, b = 2;', + output: 'let a, b = 2;', + errors + }, + { + code: 'var a = undefined;', + output: 'var a;', + errors + }, + { + code: 'var a = undefined, b = 2;', + output: 'var a, b = 2;', + errors + }, + { + code: 'foo(undefined);', + output: 'foo();', + errors + }, + { + code: 'foo(undefined,);', + output: 'foo();', + errors + }, + { + code: 'foo(bar, undefined,);', + output: 'foo(bar, );', + errors + }, + { + code: 'const {foo = undefined} = {};', + output: 'const {foo} = {};', + errors + }, + { + code: 'const [foo = undefined] = [];', + output: 'const [foo] = [];', + errors + }, + { + code: 'function foo(bar = undefined) {}', + output: 'function foo(bar) {}', + errors + }, + { + code: 'function foo({bar = undefined}) {}', + output: 'function foo({bar}) {}', + errors + }, + { + code: 'function foo({bar = undefined} = {}) {}', + output: 'function foo({bar} = {}) {}', + errors + }, + { + code: 'function foo([bar = undefined]) {}', + output: 'function foo([bar]) {}', + errors + }, + { + code: 'function foo([bar = undefined] = []) {}', + output: 'function foo([bar] = []) {}', + errors + }, + ] +}); From 07fef5e66e1491cefd8610d30cc0bf4418ef3b40 Mon Sep 17 00:00:00 2001 From: fisker Date: Mon, 4 May 2020 21:13:24 +0800 Subject: [PATCH 02/20] Add rule logic --- rules/no-useless-undefined.js | 73 +++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 rules/no-useless-undefined.js diff --git a/rules/no-useless-undefined.js b/rules/no-useless-undefined.js new file mode 100644 index 0000000000..f97c0e3446 --- /dev/null +++ b/rules/no-useless-undefined.js @@ -0,0 +1,73 @@ +'use strict'; +const {isCommaToken} = require('eslint-utils') +const getDocumentationUrl = require('./utils/get-documentation-url'); + +const messageId = 'no-useless-undefined'; + +const create = context => { + return { + 'ReturnStatement > Identifier.argument[name="undefined"]'(node) { + context.report({ + node, + messageId, + fix: fixer => fixer.replaceText(node, '') + }) + }, + 'YieldExpression > Identifier.argument[name="undefined"]'(node) { + context.report({ + node, + messageId, + fix: fixer => fixer.replaceText(node, '') + }) + }, + 'ArrowFunctionExpression > Identifier.body[name="undefined"]'(node) { + context.report({ + node, + messageId, + fix: fixer => fixer.replaceText(node, '{}') + }) + }, + 'VariableDeclaration[kind!="const"] > VariableDeclarator > Identifier.init[name="undefined"]'(node) { + context.report({ + node, + messageId, + fix: fixer => fixer.removeRange([node.parent.id.range[1], node.range[1]]) + }) + }, + 'AssignmentPattern > Identifier.right[name="undefined"]'(node) { + context.report({ + node, + messageId, + fix: fixer => fixer.removeRange([node.parent.left.range[1], node.range[1]]) + }) + }, + 'CallExpression > Identifier.arguments:last-child[name="undefined"]'(node) { + context.report({ + node, + messageId, + fix: fixer => { + const fixes = [fixer.replaceText(node, '')]; + const tokenAfter = context.getTokenAfter(node); + if (isCommaToken(tokenAfter)) { + fixes.push(fixer.replaceText(tokenAfter, '')) + } + return fixes + } + }) + } + } +}; + +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + url: getDocumentationUrl(__filename) + }, + messages: { + [messageId]: 'Do not use `undefined`.' + }, + fixable: 'code' + } +}; From 3529c16696a23449a3561fd1837bdd351833ba60 Mon Sep 17 00:00:00 2001 From: fisker Date: Mon, 4 May 2020 21:58:00 +0800 Subject: [PATCH 03/20] Simplify logic --- rules/no-useless-undefined.js | 112 ++++++++++++++++++---------------- 1 file changed, 60 insertions(+), 52 deletions(-) diff --git a/rules/no-useless-undefined.js b/rules/no-useless-undefined.js index f97c0e3446..df18ea5d7d 100644 --- a/rules/no-useless-undefined.js +++ b/rules/no-useless-undefined.js @@ -1,61 +1,69 @@ 'use strict'; -const {isCommaToken} = require('eslint-utils') +const {isCommaToken} = require('eslint-utils'); const getDocumentationUrl = require('./utils/get-documentation-url'); const messageId = 'no-useless-undefined'; +const getSelector = (parent, property) => + `${parent} > Identifier.${property}[name="undefined"]`; + +// `return undefined` +const returnSelector = getSelector('ReturnStatement', 'argument'); + +// `yield undefined` +const yieldSelector = getSelector('YieldExpression', 'argument'); + +// `() => undefined` +const arrowFunctionSelector = getSelector('ArrowFunctionExpression', 'body'); + +// `let foo = undefined` / `var foo = undefined` +const variableInitSelector = getSelector( + [ + 'VariableDeclaration', + '[kind!="const"]', + '>', + 'VariableDeclarator' + ].join(''), + 'init' +); + +// `const {foo = undefined} = {}` +const assignmentPatternSelector = getSelector('AssignmentPattern', 'right'); + +// `foo(bar, undefined)` +const lastArgumentSelector = getSelector('CallExpression', 'arguments:last-child'); + const create = context => { + const listener = fix => node => { + context.report({ + node, + messageId, + fix: fixer => fix(node, fixer) + }); + }; + + const remove = (node, fixer) => fixer.remove(node); + return { - 'ReturnStatement > Identifier.argument[name="undefined"]'(node) { - context.report({ - node, - messageId, - fix: fixer => fixer.replaceText(node, '') - }) - }, - 'YieldExpression > Identifier.argument[name="undefined"]'(node) { - context.report({ - node, - messageId, - fix: fixer => fixer.replaceText(node, '') - }) - }, - 'ArrowFunctionExpression > Identifier.body[name="undefined"]'(node) { - context.report({ - node, - messageId, - fix: fixer => fixer.replaceText(node, '{}') - }) - }, - 'VariableDeclaration[kind!="const"] > VariableDeclarator > Identifier.init[name="undefined"]'(node) { - context.report({ - node, - messageId, - fix: fixer => fixer.removeRange([node.parent.id.range[1], node.range[1]]) - }) - }, - 'AssignmentPattern > Identifier.right[name="undefined"]'(node) { - context.report({ - node, - messageId, - fix: fixer => fixer.removeRange([node.parent.left.range[1], node.range[1]]) - }) - }, - 'CallExpression > Identifier.arguments:last-child[name="undefined"]'(node) { - context.report({ - node, - messageId, - fix: fixer => { - const fixes = [fixer.replaceText(node, '')]; - const tokenAfter = context.getTokenAfter(node); - if (isCommaToken(tokenAfter)) { - fixes.push(fixer.replaceText(tokenAfter, '')) - } - return fixes - } - }) - } - } + [returnSelector]: listener(remove), + [yieldSelector]: listener(remove), + [arrowFunctionSelector]: listener( + (node, fixer) => fixer.replaceText(node, '{}') + ), + [variableInitSelector]: listener( + (node, fixer) => fixer.removeRange([node.parent.id.range[1], node.range[1]]) + ), + [assignmentPatternSelector]: listener( + (node, fixer) => fixer.removeRange([node.parent.left.range[1], node.range[1]]) + ), + [lastArgumentSelector]: listener( + (node, fixer) => { + const tokenAfter = context.getTokenAfter(node); + return (isCommaToken(tokenAfter) ? [node, tokenAfter] : [node]) + .map(nodeOrToken => remove(nodeOrToken, fixer)) + } + ) + }; }; module.exports = { @@ -66,7 +74,7 @@ module.exports = { url: getDocumentationUrl(__filename) }, messages: { - [messageId]: 'Do not use `undefined`.' + [messageId]: '`undefined` is useless.' }, fixable: 'code' } From ab7b200be88faa68f33bb3d2b19178ab78860e18 Mon Sep 17 00:00:00 2001 From: fisker Date: Mon, 4 May 2020 22:07:36 +0800 Subject: [PATCH 04/20] Add docs --- docs/rules/no-useless-undefined.md | 83 ++++++++++++++++++++++++++++++ index.js | 1 + readme.md | 2 + 3 files changed, 86 insertions(+) create mode 100644 docs/rules/no-useless-undefined.md diff --git a/docs/rules/no-useless-undefined.md b/docs/rules/no-useless-undefined.md new file mode 100644 index 0000000000..5330339e3f --- /dev/null +++ b/docs/rules/no-useless-undefined.md @@ -0,0 +1,83 @@ +# Disallow useless `undefined` + +This rule is fixable. + +## Fail + +```js +let foo = undefined; +``` + +```js +const {foo = undefined} = bar; +``` + +```js +const noop = () => undefined; +``` + +```js +function foo() { + return undefined; +} +``` + +```js +function* foo() { + yield undefined; +} +``` + +```js +function foo(bar = undefined) { +} +``` + +```js +function foo({bar = undefined}) { +} +``` + +```js +foo(undefined); +``` + +## Pass + +```js +let foo; +``` + +```js +const {foo} = bar; +``` + +```js +const noop = () => {}; +``` + +```js +function foo() { + return ; +} +``` + +```js +function* foo() { + yield ; +} +``` + +```js +function foo(bar = undefined) { +} +``` + +```js +function foo({bar}) { +} +``` + +```js +foo(); +``` diff --git a/index.js b/index.js index 30e82b5f05..60a11d7e8f 100644 --- a/index.js +++ b/index.js @@ -43,6 +43,7 @@ module.exports = { 'unicorn/no-unreadable-array-destructuring': 'error', 'unicorn/no-unsafe-regex': 'off', 'unicorn/no-unused-properties': 'off', + 'unicorn/no-useless-undefined': 'error', 'unicorn/no-zero-fractions': 'error', 'unicorn/number-literal-case': 'error', 'unicorn/prefer-add-event-listener': 'error', diff --git a/readme.md b/readme.md index c0a06c6583..106cbaded4 100644 --- a/readme.md +++ b/readme.md @@ -59,6 +59,7 @@ Configure it in `package.json`. "unicorn/no-unreadable-array-destructuring": "error", "unicorn/no-unsafe-regex": "off", "unicorn/no-unused-properties": "off", + "unicorn/no-useless-undefined": "error", "unicorn/no-zero-fractions": "error", "unicorn/number-literal-case": "error", "unicorn/prefer-add-event-listener": "error", @@ -117,6 +118,7 @@ Configure it in `package.json`. - [no-unreadable-array-destructuring](docs/rules/no-unreadable-array-destructuring.md) - Disallow unreadable array destructuring. - [no-unsafe-regex](docs/rules/no-unsafe-regex.md) - Disallow unsafe regular expressions. - [no-unused-properties](docs/rules/no-unused-properties.md) - Disallow unused object properties. +- [no-useless-undefined](docs/rules/no-useless-undefined.md) - Disallow useless `undefined`. *(fixable)* - [no-zero-fractions](docs/rules/no-zero-fractions.md) - Disallow number literals with zero fractions or dangling dots. *(fixable)* - [number-literal-case](docs/rules/number-literal-case.md) - Enforce proper case for numeric literals. *(fixable)* - [prefer-add-event-listener](docs/rules/prefer-add-event-listener.md) - Prefer `.addEventListener()` and `.removeEventListener()` over `on`-functions. *(partly fixable)* From 55c2a4c26b82d7845ea177aaa891916a18e36b6e Mon Sep 17 00:00:00 2001 From: fisker Date: Mon, 4 May 2020 22:36:55 +0800 Subject: [PATCH 05/20] Fix code style --- rules/no-useless-undefined.js | 2 +- test/no-useless-undefined.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rules/no-useless-undefined.js b/rules/no-useless-undefined.js index df18ea5d7d..c2737169a9 100644 --- a/rules/no-useless-undefined.js +++ b/rules/no-useless-undefined.js @@ -60,7 +60,7 @@ const create = context => { (node, fixer) => { const tokenAfter = context.getTokenAfter(node); return (isCommaToken(tokenAfter) ? [node, tokenAfter] : [node]) - .map(nodeOrToken => remove(nodeOrToken, fixer)) + .map(nodeOrToken => remove(nodeOrToken, fixer)); } ) }; diff --git a/test/no-useless-undefined.js b/test/no-useless-undefined.js index 13b4a38af1..95cdf6ce8f 100644 --- a/test/no-useless-undefined.js +++ b/test/no-useless-undefined.js @@ -116,6 +116,6 @@ ruleTester.run('better-regex', rule, { code: 'function foo([bar = undefined] = []) {}', output: 'function foo([bar] = []) {}', errors - }, + } ] }); From cbe08ba12f62c0ec8e7b038573e039c3bd1c8a1b Mon Sep 17 00:00:00 2001 From: fisker Date: Mon, 4 May 2020 23:29:53 +0800 Subject: [PATCH 06/20] Update call fixer logic --- rules/no-useless-undefined.js | 17 ++++++++++++++--- test/no-useless-undefined.js | 27 ++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/rules/no-useless-undefined.js b/rules/no-useless-undefined.js index c2737169a9..a17d749479 100644 --- a/rules/no-useless-undefined.js +++ b/rules/no-useless-undefined.js @@ -58,9 +58,20 @@ const create = context => { ), [lastArgumentSelector]: listener( (node, fixer) => { - const tokenAfter = context.getTokenAfter(node); - return (isCommaToken(tokenAfter) ? [node, tokenAfter] : [node]) - .map(nodeOrToken => remove(nodeOrToken, fixer)); + const argumentNodes = node.parent.arguments; + const lastArgument = argumentNodes[argumentNodes.length - 2]; + let [start, end] = node.range; + if (lastArgument) { + start = lastArgument.range[1]; + } else { + // If it's the only argument, and there is trailing comma, we need remove it. + const tokenAfter = context.getTokenAfter(node); + if (isCommaToken(tokenAfter)) { + end = tokenAfter.range[1]; + } + } + + return fixer.removeRange([start, end]); } ) }; diff --git a/test/no-useless-undefined.js b/test/no-useless-undefined.js index 95cdf6ce8f..9696a00b68 100644 --- a/test/no-useless-undefined.js +++ b/test/no-useless-undefined.js @@ -72,14 +72,39 @@ ruleTester.run('better-regex', rule, { output: 'foo();', errors }, + { + code: 'foo(undefined, undefined);', + output: 'foo(undefined);', + errors + }, { code: 'foo(undefined,);', output: 'foo();', errors }, + { + code: 'foo(undefined, undefined,);', + output: 'foo(undefined,);', + errors + }, + { + code: 'foo(bar, undefined);', + output: 'foo(bar);', + errors + }, + { + code: 'foo(bar, undefined, undefined);', + output: 'foo(bar, undefined);', + errors + }, { code: 'foo(bar, undefined,);', - output: 'foo(bar, );', + output: 'foo(bar,);', + errors + }, + { + code: 'foo(bar, undefined, undefined,);', + output: 'foo(bar, undefined,);', errors }, { From 6f7480b4218dc0453d642a675027cc457377d7b2 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 5 May 2020 01:00:29 +0800 Subject: [PATCH 07/20] Fix crash with `typescript-eslint` --- rules/no-useless-undefined.js | 30 ++++++++++++++++++++---------- test/no-useless-undefined.js | 14 +++++++++++++- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/rules/no-useless-undefined.js b/rules/no-useless-undefined.js index a17d749479..992c376ce9 100644 --- a/rules/no-useless-undefined.js +++ b/rules/no-useless-undefined.js @@ -31,7 +31,19 @@ const variableInitSelector = getSelector( const assignmentPatternSelector = getSelector('AssignmentPattern', 'right'); // `foo(bar, undefined)` -const lastArgumentSelector = getSelector('CallExpression', 'arguments:last-child'); +// TODO: Use this selector and remove `ifLastArgument` function +// esquery throws when use `:last-child` with `typescript-eslint`, +// maybe because ESLint hasn't support OptionalCallExpression +// const lastArgumentSelector = getSelector('CallExpression', 'arguments:last-child'); +const lastArgumentSelector = getSelector('CallExpression', 'arguments'); +const ifLastArgument = listener => node => { + const argumentNodes = node.parent.arguments; + if (argumentNodes[argumentNodes.length - 1] === node) { + listener(node); + } +}; + +const removeNode = (node, fixer) => fixer.remove(node); const create = context => { const listener = fix => node => { @@ -42,11 +54,9 @@ const create = context => { }); }; - const remove = (node, fixer) => fixer.remove(node); - return { - [returnSelector]: listener(remove), - [yieldSelector]: listener(remove), + [returnSelector]: listener(removeNode), + [yieldSelector]: listener(removeNode), [arrowFunctionSelector]: listener( (node, fixer) => fixer.replaceText(node, '{}') ), @@ -56,13 +66,13 @@ const create = context => { [assignmentPatternSelector]: listener( (node, fixer) => fixer.removeRange([node.parent.left.range[1], node.range[1]]) ), - [lastArgumentSelector]: listener( + [lastArgumentSelector]: ifLastArgument(listener( (node, fixer) => { const argumentNodes = node.parent.arguments; - const lastArgument = argumentNodes[argumentNodes.length - 2]; + const previousArgument = argumentNodes[argumentNodes.length - 2]; let [start, end] = node.range; - if (lastArgument) { - start = lastArgument.range[1]; + if (previousArgument) { + start = previousArgument.range[1]; } else { // If it's the only argument, and there is trailing comma, we need remove it. const tokenAfter = context.getTokenAfter(node); @@ -73,7 +83,7 @@ const create = context => { return fixer.removeRange([start, end]); } - ) + )) }; }; diff --git a/test/no-useless-undefined.js b/test/no-useless-undefined.js index 9696a00b68..ab1e7c613b 100644 --- a/test/no-useless-undefined.js +++ b/test/no-useless-undefined.js @@ -10,9 +10,13 @@ const ruleTester = avaRuleTester(test, { } }); +const typescriptRuleTester = avaRuleTester(test, { + parser: require.resolve('@typescript-eslint/parser') +}); + const errors = [{messageId}]; -ruleTester.run('better-regex', rule, { +ruleTester.run('no-useless-undefined', rule, { valid: [ 'function foo() {return;}', 'const foo = () => {};', @@ -144,3 +148,11 @@ ruleTester.run('better-regex', rule, { } ] }); + +typescriptRuleTester.run('no-useless-undefined', rule, { + valid: [ + // https://github.com/zeit/next.js/blob/3af0fe5cf2542237f34d106872d104c3606b1858/packages/next/build/utils.ts#L620 + 'prerenderPaths?.add(entry)' + ], + invalid: [] +}); From 72af77ed154abc2ff43962ca38f66479b5a34928 Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 5 May 2020 01:04:28 +0800 Subject: [PATCH 08/20] Fix docs example --- docs/rules/no-useless-undefined.md | 2 +- rules/no-useless-undefined.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/rules/no-useless-undefined.md b/docs/rules/no-useless-undefined.md index 5330339e3f..83cbc2eceb 100644 --- a/docs/rules/no-useless-undefined.md +++ b/docs/rules/no-useless-undefined.md @@ -69,7 +69,7 @@ function* foo() { ``` ```js -function foo(bar = undefined) { +function foo(bar) { } ``` diff --git a/rules/no-useless-undefined.js b/rules/no-useless-undefined.js index 992c376ce9..46a990bc27 100644 --- a/rules/no-useless-undefined.js +++ b/rules/no-useless-undefined.js @@ -95,6 +95,7 @@ module.exports = { url: getDocumentationUrl(__filename) }, messages: { + // [TBD]: better message [messageId]: '`undefined` is useless.' }, fixable: 'code' From bd80711ca4ae6cf00112f26733a29d30370a85ac Mon Sep 17 00:00:00 2001 From: fisker Date: Tue, 5 May 2020 02:22:06 +0800 Subject: [PATCH 09/20] Remove all `undefined`, instead of one at a time --- rules/no-useless-undefined.js | 66 ++++++++++++++++++++++------------- test/no-useless-undefined.js | 52 ++++++++++++++++++++++++--- 2 files changed, 90 insertions(+), 28 deletions(-) diff --git a/rules/no-useless-undefined.js b/rules/no-useless-undefined.js index 46a990bc27..c8bf1591a2 100644 --- a/rules/no-useless-undefined.js +++ b/rules/no-useless-undefined.js @@ -30,20 +30,8 @@ const variableInitSelector = getSelector( // `const {foo = undefined} = {}` const assignmentPatternSelector = getSelector('AssignmentPattern', 'right'); -// `foo(bar, undefined)` -// TODO: Use this selector and remove `ifLastArgument` function -// esquery throws when use `:last-child` with `typescript-eslint`, -// maybe because ESLint hasn't support OptionalCallExpression -// const lastArgumentSelector = getSelector('CallExpression', 'arguments:last-child'); -const lastArgumentSelector = getSelector('CallExpression', 'arguments'); -const ifLastArgument = listener => node => { - const argumentNodes = node.parent.arguments; - if (argumentNodes[argumentNodes.length - 1] === node) { - listener(node); - } -}; - const removeNode = (node, fixer) => fixer.remove(node); +const isUndefined = node => node && node.type === 'Identifier' && node.name === 'undefined'; const create = context => { const listener = fix => node => { @@ -66,24 +54,55 @@ const create = context => { [assignmentPatternSelector]: listener( (node, fixer) => fixer.removeRange([node.parent.left.range[1], node.range[1]]) ), - [lastArgumentSelector]: ifLastArgument(listener( - (node, fixer) => { - const argumentNodes = node.parent.arguments; - const previousArgument = argumentNodes[argumentNodes.length - 2]; - let [start, end] = node.range; + CallExpression: node => { + const argumentNodes = node.arguments; + const undefinedArguments = []; + for (let index = argumentNodes.length - 1; index >= 0; index--) { + const node = argumentNodes[index]; + if (isUndefined(node)) { + undefinedArguments.unshift(node); + } else { + break; + } + } + + if (undefinedArguments.length === 0) { + return; + } + + const problem = { + messageId + }; + + const firstUndefined = undefinedArguments[0]; + const lastUndefined = undefinedArguments[undefinedArguments.length - 1]; + + problem.loc = { + start: firstUndefined.loc.start, + end: lastUndefined.loc.end + }; + + problem.fix = fixer => { + let start = firstUndefined.range[0]; + let end = lastUndefined.range[1]; + + const previousArgument = argumentNodes[argumentNodes.length - undefinedArguments.length - 1]; + if (previousArgument) { start = previousArgument.range[1]; } else { - // If it's the only argument, and there is trailing comma, we need remove it. - const tokenAfter = context.getTokenAfter(node); + // If all arguments removed, and there is trailing comma, we need remove it. + const tokenAfter = context.getTokenAfter(lastUndefined); if (isCommaToken(tokenAfter)) { end = tokenAfter.range[1]; } } return fixer.removeRange([start, end]); - } - )) + }; + + context.report(problem); + } }; }; @@ -95,8 +114,7 @@ module.exports = { url: getDocumentationUrl(__filename) }, messages: { - // [TBD]: better message - [messageId]: '`undefined` is useless.' + [messageId]: 'Do not use useless `undefined`.' }, fixable: 'code' } diff --git a/test/no-useless-undefined.js b/test/no-useless-undefined.js index ab1e7c613b..2a54bf10de 100644 --- a/test/no-useless-undefined.js +++ b/test/no-useless-undefined.js @@ -1,4 +1,5 @@ import test from 'ava'; +import {outdent} from 'outdent'; import avaRuleTester from 'eslint-ava-rule-tester'; import rule from '../rules/no-useless-undefined'; @@ -78,7 +79,7 @@ ruleTester.run('no-useless-undefined', rule, { }, { code: 'foo(undefined, undefined);', - output: 'foo(undefined);', + output: 'foo();', errors }, { @@ -88,7 +89,7 @@ ruleTester.run('no-useless-undefined', rule, { }, { code: 'foo(undefined, undefined,);', - output: 'foo(undefined,);', + output: 'foo();', errors }, { @@ -98,7 +99,12 @@ ruleTester.run('no-useless-undefined', rule, { }, { code: 'foo(bar, undefined, undefined);', - output: 'foo(bar, undefined);', + output: 'foo(bar);', + errors + }, + { + code: 'foo(undefined, bar, undefined);', + output: 'foo(undefined, bar);', errors }, { @@ -106,11 +112,49 @@ ruleTester.run('no-useless-undefined', rule, { output: 'foo(bar,);', errors }, + { + code: 'foo(undefined, bar, undefined,);', + output: 'foo(undefined, bar,);', + errors + }, { code: 'foo(bar, undefined, undefined,);', - output: 'foo(bar, undefined,);', + output: 'foo(bar,);', errors }, + { + code: 'foo(undefined, bar, undefined, undefined,);', + output: 'foo(undefined, bar,);', + errors + }, + // Test report range + { + code: outdent` + foo( + undefined, + bar, + undefined, + undefined, + undefined, + undefined, + ) + `, + output: outdent` + foo( + undefined, + bar, + ) + `, + errors: [ + { + messageId, + // The 2rd `undefined` + line: 4, column: 2, + // The last `undefined` + endLine: 7, endColumn: 11 + } + ] + }, { code: 'const {foo = undefined} = {};', output: 'const {foo} = {};', From dc15e8fdb5765a37cb5a4c956aa51603715bd756 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Wed, 6 May 2020 00:29:57 +0800 Subject: [PATCH 10/20] Update no-useless-undefined.md --- docs/rules/no-useless-undefined.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/rules/no-useless-undefined.md b/docs/rules/no-useless-undefined.md index 83cbc2eceb..21ccf282b7 100644 --- a/docs/rules/no-useless-undefined.md +++ b/docs/rules/no-useless-undefined.md @@ -58,13 +58,13 @@ const noop = () => {}; ```js function foo() { - return ; + return; } ``` ```js function* foo() { - yield ; + yield; } ``` From 38b9dccf4f84e658113153ec94f0fb31fb01fbd1 Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 10:39:36 +0800 Subject: [PATCH 11/20] exclude `yield*` --- rules/no-useless-undefined.js | 2 +- test/no-useless-undefined.js | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/rules/no-useless-undefined.js b/rules/no-useless-undefined.js index c8bf1591a2..e0bfd42652 100644 --- a/rules/no-useless-undefined.js +++ b/rules/no-useless-undefined.js @@ -11,7 +11,7 @@ const getSelector = (parent, property) => const returnSelector = getSelector('ReturnStatement', 'argument'); // `yield undefined` -const yieldSelector = getSelector('YieldExpression', 'argument'); +const yieldSelector = getSelector('YieldExpression[delegate=false]', 'argument'); // `() => undefined` const arrowFunctionSelector = getSelector('ArrowFunctionExpression', 'body'); diff --git a/test/no-useless-undefined.js b/test/no-useless-undefined.js index 2a54bf10de..dddb4b9090 100644 --- a/test/no-useless-undefined.js +++ b/test/no-useless-undefined.js @@ -29,7 +29,9 @@ ruleTester.run('no-useless-undefined', rule, { 'foo(undefined, bar);', 'const {foo} = {};', 'function foo({bar} = {}) {}', - 'function foo(bar) {}' + 'function foo(bar) {}', + // I guess nobody use this, but `yield* undefined;` is valid code, and `yield*;` is not + 'function* foo() {yield* undefined;}' ], invalid: [ { From 6f4b355a1c276d7bc7007168c9653d2329ece00d Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 10:51:37 +0800 Subject: [PATCH 12/20] Remove space --- rules/no-useless-undefined.js | 14 +++++++++++--- test/no-useless-undefined.js | 21 ++++++++++++++++++--- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/rules/no-useless-undefined.js b/rules/no-useless-undefined.js index e0bfd42652..e8eb090464 100644 --- a/rules/no-useless-undefined.js +++ b/rules/no-useless-undefined.js @@ -30,7 +30,6 @@ const variableInitSelector = getSelector( // `const {foo = undefined} = {}` const assignmentPatternSelector = getSelector('AssignmentPattern', 'right'); -const removeNode = (node, fixer) => fixer.remove(node); const isUndefined = node => node && node.type === 'Identifier' && node.name === 'undefined'; const create = context => { @@ -41,10 +40,19 @@ const create = context => { fix: fixer => fix(node, fixer) }); }; + const code = context.getSourceCode().text; + + const removeNodeAndLeadingSpace = (node, fixer) => { + const textBefore = code.slice(0, node.range[0]); + return fixer.removeRange([ + node.range[0] - (textBefore.length - textBefore.trim().length), + node.range[1] + ]) + }; return { - [returnSelector]: listener(removeNode), - [yieldSelector]: listener(removeNode), + [returnSelector]: listener(removeNodeAndLeadingSpace), + [yieldSelector]: listener(removeNodeAndLeadingSpace), [arrowFunctionSelector]: listener( (node, fixer) => fixer.replaceText(node, '{}') ), diff --git a/test/no-useless-undefined.js b/test/no-useless-undefined.js index dddb4b9090..012df19fd0 100644 --- a/test/no-useless-undefined.js +++ b/test/no-useless-undefined.js @@ -36,7 +36,7 @@ ruleTester.run('no-useless-undefined', rule, { invalid: [ { code: 'function foo() {return undefined;}', - output: 'function foo() {return ;}', + output: 'function foo() {return;}', errors }, { @@ -46,12 +46,27 @@ ruleTester.run('no-useless-undefined', rule, { }, { code: 'const foo = () => {return undefined;};', - output: 'const foo = () => {return ;};', + output: 'const foo = () => {return;};', + errors + }, + { + code: 'function foo() {return undefined;}', + output: 'function foo() {return;}', + errors + }, + { + code: 'function foo() {return /* comment */ undefined;}', + output: 'function foo() {return /* comment */;}', errors }, { code: 'function* foo() {yield undefined;}', - output: 'function* foo() {yield ;}', + output: 'function* foo() {yield;}', + errors + }, + { + code: 'function* foo() {yield undefined;}', + output: 'function* foo() {yield;}', errors }, { From fe4c222bc67e41352ffeaba9c83f7036f67ea247 Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 10:53:42 +0800 Subject: [PATCH 13/20] Code style --- rules/no-useless-undefined.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rules/no-useless-undefined.js b/rules/no-useless-undefined.js index e8eb090464..ccbac0fd9d 100644 --- a/rules/no-useless-undefined.js +++ b/rules/no-useless-undefined.js @@ -40,6 +40,7 @@ const create = context => { fix: fixer => fix(node, fixer) }); }; + const code = context.getSourceCode().text; const removeNodeAndLeadingSpace = (node, fixer) => { @@ -47,7 +48,7 @@ const create = context => { return fixer.removeRange([ node.range[0] - (textBefore.length - textBefore.trim().length), node.range[1] - ]) + ]); }; return { From e40f496af79e6254733585bb03d5904b5b8477c5 Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 10:56:46 +0800 Subject: [PATCH 14/20] Fix lint --- rules/no-for-loop.js | 4 ++-- rules/no-unused-properties.js | 8 ++++---- test/filename-case.js | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/rules/no-for-loop.js b/rules/no-for-loop.js index bf43089fa7..ecda4acf9c 100644 --- a/rules/no-for-loop.js +++ b/rules/no-for-loop.js @@ -192,7 +192,7 @@ const resolveIdentifierName = (name, scope) => { scope = scope.upper; } - return undefined; + return; }; const scopeContains = (ancestor, descendant) => { @@ -367,7 +367,7 @@ const create = context => { ], replacement), ...arrayReferences.map(reference => { if (reference === elementReference) { - return undefined; + return; } return fixer.replaceText(reference.identifier.parent, element); diff --git a/rules/no-unused-properties.js b/rules/no-unused-properties.js index ef06869364..519b4df750 100644 --- a/rules/no-unused-properties.js +++ b/rules/no-unused-properties.js @@ -139,7 +139,7 @@ const create = context => { return {identifier: parent}; } - return undefined; + return; } if (parent.type === 'MemberExpression') { @@ -152,7 +152,7 @@ const create = context => { return {identifier: parent}; } - return undefined; + return; } if ( @@ -163,7 +163,7 @@ const create = context => { return {identifier: parent}; } - return undefined; + return; } if ( @@ -174,7 +174,7 @@ const create = context => { return {identifier: parent}; } - return undefined; + return; } return reference; diff --git a/test/filename-case.js b/test/filename-case.js index 99a7a5f40d..bf921868cf 100644 --- a/test/filename-case.js +++ b/test/filename-case.js @@ -87,7 +87,7 @@ ruleTester.run('filename-case', rule, { testCase('src/foo/___foo-bar.js', 'kebabCase'), testCase('src/foo/_FooBar.js', 'pascalCase'), testCase('src/foo/___FooBar.js', 'pascalCase'), - testManyCases('src/foo/foo-bar.js', undefined), + testManyCases('src/foo/foo-bar.js'), testManyCases('src/foo/foo-bar.js', {}), testManyCases('src/foo/fooBar.js', {camelCase: true}), testManyCases('src/foo/FooBar.js', {kebabCase: true, pascalCase: true}), From 6b2c388d0e9def956d03b14922909ec0a01f4d7f Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 10:58:45 +0800 Subject: [PATCH 15/20] Run `xo` --- rules/no-for-loop.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/rules/no-for-loop.js b/rules/no-for-loop.js index ecda4acf9c..848e859617 100644 --- a/rules/no-for-loop.js +++ b/rules/no-for-loop.js @@ -191,8 +191,6 @@ const resolveIdentifierName = (name, scope) => { scope = scope.upper; } - - return; }; const scopeContains = (ancestor, descendant) => { From 5fd6e16bfa6421b566abc3ef5c924f9202801f9e Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 11:07:08 +0800 Subject: [PATCH 16/20] `allowImplicit` in `array-callback-return` --- package.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/package.json b/package.json index d56028970a..2e2c9ba855 100644 --- a/package.json +++ b/package.json @@ -106,6 +106,12 @@ ], "rules": { "strict": "error", + "array-callback-return": [ + "error", + { + "allowImplicit": true + } + ], "unicorn/no-null": "error", "unicorn/string-content": "off" } From da0c039b80b0fef64333fb595573bc7b30d69071 Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 11:11:32 +0800 Subject: [PATCH 17/20] Document conflict with `array-callback-return` --- docs/rules/no-useless-undefined.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/rules/no-useless-undefined.md b/docs/rules/no-useless-undefined.md index 21ccf282b7..58e25c255c 100644 --- a/docs/rules/no-useless-undefined.md +++ b/docs/rules/no-useless-undefined.md @@ -81,3 +81,20 @@ function foo({bar}) { ```js foo(); ``` + +## Conflict ESLint `array-callback-return` rule + +We recommend set the ESLint [`array-callback-return`](https://eslint.org/docs/rules/array-callback-return#top) rule option [`allowImplicit`](https://eslint.org/docs/rules/array-callback-return#options) to `true`: + +```json +{ + "rules": { + "array-callback-return": [ + "error", + { + "allowImplicit": true + } + ], + } +} +``` From 1376df4259bd9d16f363108484c4fbe319ac8aea Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 11:23:03 +0800 Subject: [PATCH 18/20] Simplify report --- rules/no-useless-undefined.js | 50 ++++++++++++++++------------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/rules/no-useless-undefined.js b/rules/no-useless-undefined.js index ccbac0fd9d..d990bf6c57 100644 --- a/rules/no-useless-undefined.js +++ b/rules/no-useless-undefined.js @@ -79,38 +79,34 @@ const create = context => { return; } - const problem = { - messageId - }; - const firstUndefined = undefinedArguments[0]; const lastUndefined = undefinedArguments[undefinedArguments.length - 1]; - problem.loc = { - start: firstUndefined.loc.start, - end: lastUndefined.loc.end - }; - - problem.fix = fixer => { - let start = firstUndefined.range[0]; - let end = lastUndefined.range[1]; - - const previousArgument = argumentNodes[argumentNodes.length - undefinedArguments.length - 1]; - - if (previousArgument) { - start = previousArgument.range[1]; - } else { - // If all arguments removed, and there is trailing comma, we need remove it. - const tokenAfter = context.getTokenAfter(lastUndefined); - if (isCommaToken(tokenAfter)) { - end = tokenAfter.range[1]; + context.report({ + messageId, + loc: { + start: firstUndefined.loc.start, + end: lastUndefined.loc.end + }, + fix: fixer => { + let start = firstUndefined.range[0]; + let end = lastUndefined.range[1]; + + const previousArgument = argumentNodes[argumentNodes.length - undefinedArguments.length - 1]; + + if (previousArgument) { + start = previousArgument.range[1]; + } else { + // If all arguments removed, and there is trailing comma, we need remove it. + const tokenAfter = context.getTokenAfter(lastUndefined); + if (isCommaToken(tokenAfter)) { + end = tokenAfter.range[1]; + } } - } - return fixer.removeRange([start, end]); - }; - - context.report(problem); + return fixer.removeRange([start, end]); + } + }); } }; }; From fd5edc8f21e2919343bfc379e983b2ca1f3dbb98 Mon Sep 17 00:00:00 2001 From: fisker Date: Wed, 6 May 2020 11:28:29 +0800 Subject: [PATCH 19/20] Fix typo --- test/no-useless-undefined.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/no-useless-undefined.js b/test/no-useless-undefined.js index 012df19fd0..9f70ef3251 100644 --- a/test/no-useless-undefined.js +++ b/test/no-useless-undefined.js @@ -165,7 +165,7 @@ ruleTester.run('no-useless-undefined', rule, { errors: [ { messageId, - // The 2rd `undefined` + // The second `undefined` line: 4, column: 2, // The last `undefined` endLine: 7, endColumn: 11 From 62bde3764cf79e24e25416d19331fe71d5a27f53 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Wed, 6 May 2020 12:46:20 +0800 Subject: [PATCH 20/20] Update no-useless-undefined.md --- docs/rules/no-useless-undefined.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/rules/no-useless-undefined.md b/docs/rules/no-useless-undefined.md index 58e25c255c..07d39eba3b 100644 --- a/docs/rules/no-useless-undefined.md +++ b/docs/rules/no-useless-undefined.md @@ -82,9 +82,9 @@ function foo({bar}) { foo(); ``` -## Conflict ESLint `array-callback-return` rule +## Conflict with ESLint `array-callback-return` rule -We recommend set the ESLint [`array-callback-return`](https://eslint.org/docs/rules/array-callback-return#top) rule option [`allowImplicit`](https://eslint.org/docs/rules/array-callback-return#options) to `true`: +We recommend setting the ESLint [`array-callback-return`](https://eslint.org/docs/rules/array-callback-return#top) rule option [`allowImplicit`](https://eslint.org/docs/rules/array-callback-return#options) to `true`: ```json { @@ -94,7 +94,7 @@ We recommend set the ESLint [`array-callback-return`](https://eslint.org/docs/ru { "allowImplicit": true } - ], + ] } } ```