Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add no-useless-undefined rule #718

Merged
merged 22 commits into from May 6, 2020
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
100 changes: 100 additions & 0 deletions 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 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
}
],
}
}
```
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -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',
Expand Down
6 changes: 6 additions & 0 deletions package.json
Expand Up @@ -106,6 +106,12 @@
],
"rules": {
"strict": "error",
"array-callback-return": [
"error",
{
"allowImplicit": true
}
],
"unicorn/no-null": "error",
"unicorn/string-content": "off"
}
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Expand Up @@ -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",
Expand Down Expand Up @@ -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)*
Expand Down
4 changes: 1 addition & 3 deletions rules/no-for-loop.js
Expand Up @@ -191,8 +191,6 @@ const resolveIdentifierName = (name, scope) => {

scope = scope.upper;
}

return undefined;
};

const scopeContains = (ancestor, descendant) => {
Expand Down Expand Up @@ -367,7 +365,7 @@ const create = context => {
], replacement),
...arrayReferences.map(reference => {
if (reference === elementReference) {
return undefined;
return;
}

return fixer.replaceText(reference.identifier.parent, element);
Expand Down
8 changes: 4 additions & 4 deletions rules/no-unused-properties.js
Expand Up @@ -139,7 +139,7 @@ const create = context => {
return {identifier: parent};
}

return undefined;
return;
}

if (parent.type === 'MemberExpression') {
Expand All @@ -152,7 +152,7 @@ const create = context => {
return {identifier: parent};
}

return undefined;
return;
}

if (
Expand All @@ -163,7 +163,7 @@ const create = context => {
return {identifier: parent};
}

return undefined;
return;
}

if (
Expand All @@ -174,7 +174,7 @@ const create = context => {
return {identifier: parent};
}

return undefined;
return;
}

return reference;
Expand Down
130 changes: 130 additions & 0 deletions rules/no-useless-undefined.js
@@ -0,0 +1,130 @@
'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 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];
}
}

return fixer.removeRange([start, end]);
};

context.report(problem);
}
};
};

module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
url: getDocumentationUrl(__filename)
},
messages: {
[messageId]: 'Do not use useless `undefined`.'
},
fixable: 'code'
}
};
2 changes: 1 addition & 1 deletion test/filename-case.js
Expand Up @@ -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}),
Expand Down