diff --git a/docs/rules/no-useless-undefined.md b/docs/rules/no-useless-undefined.md new file mode 100644 index 0000000000..07d39eba3b --- /dev/null +++ b/docs/rules/no-useless-undefined.md @@ -0,0 +1,100 @@ +# 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) { +} +``` + +```js +function foo({bar}) { +} +``` + +```js +foo(); +``` + +## Conflict with ESLint `array-callback-return` rule + +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 +{ + "rules": { + "array-callback-return": [ + "error", + { + "allowImplicit": true + } + ] + } +} +``` 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/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" } 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)* diff --git a/rules/no-for-loop.js b/rules/no-for-loop.js index bf43089fa7..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 undefined; }; const scopeContains = (ancestor, descendant) => { @@ -367,7 +365,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/rules/no-useless-undefined.js b/rules/no-useless-undefined.js new file mode 100644 index 0000000000..d990bf6c57 --- /dev/null +++ b/rules/no-useless-undefined.js @@ -0,0 +1,126 @@ +'use strict'; +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[delegate=false]', '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'); + +const isUndefined = node => node && node.type === 'Identifier' && node.name === 'undefined'; + +const create = context => { + const listener = fix => node => { + context.report({ + node, + messageId, + 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(removeNodeAndLeadingSpace), + [yieldSelector]: listener(removeNodeAndLeadingSpace), + [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]]) + ), + 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 firstUndefined = undefinedArguments[0]; + const lastUndefined = undefinedArguments[undefinedArguments.length - 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]); + } + }); + } + }; +}; + +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + url: getDocumentationUrl(__filename) + }, + messages: { + [messageId]: 'Do not use useless `undefined`.' + }, + fixable: 'code' + } +}; 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}), diff --git a/test/no-useless-undefined.js b/test/no-useless-undefined.js new file mode 100644 index 0000000000..9f70ef3251 --- /dev/null +++ b/test/no-useless-undefined.js @@ -0,0 +1,219 @@ +import test from 'ava'; +import {outdent} from 'outdent'; +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 typescriptRuleTester = avaRuleTester(test, { + parser: require.resolve('@typescript-eslint/parser') +}); + +const errors = [{messageId}]; + +ruleTester.run('no-useless-undefined', 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) {}', + // I guess nobody use this, but `yield* undefined;` is valid code, and `yield*;` is not + 'function* foo() {yield* undefined;}' + ], + 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() {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;}', + 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, undefined);', + output: 'foo();', + errors + }, + { + code: 'foo(undefined,);', + output: 'foo();', + errors + }, + { + code: 'foo(undefined, undefined,);', + output: 'foo();', + errors + }, + { + code: 'foo(bar, undefined);', + output: 'foo(bar);', + errors + }, + { + code: 'foo(bar, undefined, undefined);', + output: 'foo(bar);', + errors + }, + { + code: 'foo(undefined, bar, undefined);', + output: 'foo(undefined, bar);', + errors + }, + { + code: 'foo(bar, undefined,);', + output: 'foo(bar,);', + errors + }, + { + code: 'foo(undefined, bar, undefined,);', + output: 'foo(undefined, bar,);', + errors + }, + { + code: 'foo(bar, undefined, 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 second `undefined` + line: 4, column: 2, + // The last `undefined` + endLine: 7, endColumn: 11 + } + ] + }, + { + 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 + } + ] +}); + +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: [] +});