Skip to content

Commit

Permalink
Add no-lonely-if rule (#936)
Browse files Browse the repository at this point in the history
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
  • Loading branch information
fisker and sindresorhus committed Dec 17, 2020
1 parent bf941a1 commit 485dbf9
Show file tree
Hide file tree
Showing 8 changed files with 767 additions and 9 deletions.
64 changes: 64 additions & 0 deletions docs/rules/no-lonely-if.md
@@ -0,0 +1,64 @@
# Disallow `if` statements as the only statement in `if` blocks without `else`

This rule adds onto the built-in [`no-lonely-if`](https://eslint.org/docs/rules/no-lonely-if) rule, which only forbids `if` statements in `else`, not in `if`.

This rule is fixable.

## Fail

```js
if (foo) {
if (bar) {
//
}
}
```

```js
if (foo) {
//
} else if (bar) {
if (baz) {
//
}
}
```

## Pass

```js
if (foo && bar) {
//
}
```

```js
if (foo) {
//
} else if (bar && baz) {
//
}
```

```js
if (foo) {
//
} else if (bar) {
if (baz) {
//
}
} else {
//
}
```

```js
// Built-in rule `no-lonely-if` case https://eslint.org/docs/rules/no-lonely-if
if (foo) {
//
} else {
if (bar) {
//
}
}
```
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -37,6 +37,7 @@ module.exports = {
'unicorn/no-for-loop': 'error',
'unicorn/no-hex-escape': 'error',
'unicorn/no-keyword-prefix': 'off',
'unicorn/no-lonely-if': 'error',
'no-nested-ternary': 'off',
'unicorn/no-nested-ternary': 'error',
'unicorn/no-new-buffer': 'error',
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Expand Up @@ -53,6 +53,7 @@ Configure it in `package.json`.
"unicorn/no-for-loop": "error",
"unicorn/no-hex-escape": "error",
"unicorn/no-keyword-prefix": "off",
"unicorn/no-lonely-if": "error",
"no-nested-ternary": "off",
"unicorn/no-nested-ternary": "error",
"unicorn/no-new-buffer": "error",
Expand Down Expand Up @@ -122,6 +123,7 @@ Configure it in `package.json`.
- [no-for-loop](docs/rules/no-for-loop.md) - Do not use a `for` loop that can be replaced with a `for-of` loop. *(partly fixable)*
- [no-hex-escape](docs/rules/no-hex-escape.md) - Enforce the use of Unicode escapes instead of hexadecimal escapes. *(fixable)*
- [no-keyword-prefix](docs/rules/no-keyword-prefix.md) - Disallow identifiers starting with `new` or `class`.
- [no-lonely-if](docs/rules/no-lonely-if.md) - Disallow `if` statements as the only statement in `if` blocks without `else`. *(fixable)*
- [no-nested-ternary](docs/rules/no-nested-ternary.md) - Disallow nested ternary expressions. *(partly fixable)*
- [no-new-buffer](docs/rules/no-new-buffer.md) - Enforce the use of `Buffer.from()` and `Buffer.alloc()` instead of the deprecated `new Buffer()`. *(fixable)*
- [no-null](docs/rules/no-null.md) - Disallow the use of the `null` literal.
Expand Down
16 changes: 7 additions & 9 deletions rules/import-style.js
Expand Up @@ -183,15 +183,13 @@ const create = context => {

let effectiveAllowedImportStyles = allowedImportStyles;

if (isRequire) {
// For `require`, `'default'` style allows both `x = require('x')` (`'namespace'` style) and
// `{default: x} = require('x')` (`'default'` style) since we don't know in advance
// whether `'x'` is a compiled ES6 module (with `default` key) or a CommonJS module and `require`
// does not provide any automatic interop for this, so the user may have to use either of theese.
if (allowedImportStyles.has('default') && !allowedImportStyles.has('namespace')) {
effectiveAllowedImportStyles = new Set(allowedImportStyles);
effectiveAllowedImportStyles.add('namespace');
}
// For `require`, `'default'` style allows both `x = require('x')` (`'namespace'` style) and
// `{default: x} = require('x')` (`'default'` style) since we don't know in advance
// whether `'x'` is a compiled ES6 module (with `default` key) or a CommonJS module and `require`
// does not provide any automatic interop for this, so the user may have to use either of theese.
if (isRequire && allowedImportStyles.has('default') && !allowedImportStyles.has('namespace')) {
effectiveAllowedImportStyles = new Set(allowedImportStyles);
effectiveAllowedImportStyles.add('namespace');
}

if (actualImportStyles.every(style => effectiveAllowedImportStyles.has(style))) {
Expand Down
90 changes: 90 additions & 0 deletions rules/no-lonely-if.js
@@ -0,0 +1,90 @@
'use strict';
const getDocumentationUrl = require('./utils/get-documentation-url');
const needsSemicolon = require('./utils/needs-semicolon');

const MESSAGE_ID = 'no-lonely-if';
const messages = {
[MESSAGE_ID]: 'Unexpected `if` as the only statement in a `if` block without `else`.'
};

const ifStatementWithoutAlternate = 'IfStatement:not([alternate])';
const selector = `:matches(${
[
// `if (a) { if (b) {} }`
[
ifStatementWithoutAlternate,
'>',
'BlockStatement.consequent',
'[body.length=1]',
'>',
`${ifStatementWithoutAlternate}.body`
].join(''),
// `if (a) if (b) {}`
`${ifStatementWithoutAlternate} > ${ifStatementWithoutAlternate}.consequent`
].join(', ')
})`;

// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence#Table
// Lower precedence than `&&`
const needParenthesis = node => (
(node.type === 'LogicalExpression' && (node.operator === '||' || node.operator === '??')) ||
node.type === 'ConditionalExpression' ||
node.type === 'AssignmentExpression' ||
node.type === 'YieldExpression' ||
node.type === 'SequenceExpression'
);

const create = context => {
const sourceCode = context.getSourceCode();
const getText = node => sourceCode.getText(node);
const getTestNodeText = node => needParenthesis(node) ? `(${getText(node)})` : getText(node);

return {
[selector](inner) {
const {parent} = inner;
const outer = parent.type === 'BlockStatement' ? parent.parent : parent;

context.report({
node: inner,
messageId: MESSAGE_ID,
* fix(fixer) {
// Merge `test`
yield fixer.replaceText(outer.test, `${getTestNodeText(outer.test)} && ${getTestNodeText(inner.test)}`);

// Replace `consequent`
const {consequent} = inner;
let consequentText = getText(consequent);
// If the `if` statement has no block, and is not followed by a semicolon,
// make sure that fixing the issue would not change semantics due to ASI.
// Similar logic https://github.com/eslint/eslint/blob/2124e1b5dad30a905dc26bde9da472bf622d3f50/lib/rules/no-lonely-if.js#L61-L77
if (
consequent.type !== 'BlockStatement' &&
outer.consequent.type === 'BlockStatement' &&
!consequentText.endsWith(';')
) {
const lastToken = sourceCode.getLastToken(consequent);
const nextToken = sourceCode.getTokenAfter(outer);
if (needsSemicolon(lastToken, sourceCode, nextToken.value)) {
consequentText += ';';
}
}

yield fixer.replaceText(outer.consequent, consequentText);
}
});
}
};
};

module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
url: getDocumentationUrl(__filename)
},
fixable: 'code',
messages
}
};
126 changes: 126 additions & 0 deletions test/no-lonely-if.js
@@ -0,0 +1,126 @@
import {outdent} from 'outdent';
import {test} from './utils/test';

test({
valid: [
outdent`
if (a) {
if (b) {
}
} else {}
`,
outdent`
if (a) {
if (b) {
}
foo();
} else {}
`,
outdent`
if (a) {
} else {
if (y) {}
}
`,
outdent`
if (a) {
b ? c() : d()
}
`
],
invalid: []
});

test.visualize([
outdent`
if (a) {
if (b) {
}
}
`,
// Inner one is `BlockStatement`
outdent`
if (a) if (b) {
foo();
}
`,
// Outer one is `BlockStatement`
outdent`
if (a) {
if (b) foo();
}
`,
// No `BlockStatement`
'if (a) if (b) foo();',
// `EmptyStatement`
'if (a) if (b);',
// Nested
outdent`
if (a) {
if (b) {
// Should not report
}
} else if (c) {
if (d) {
}
}
`,
// Need parenthesis
outdent`
function * foo() {
if (a || b)
if (a ?? b)
if (a ? b : c)
if (a = b)
if (a += b)
if (a -= b)
if (a &&= b)
if (yield a)
if (a, b);
}
`,
// Should not add parenthesis
outdent`
async function foo() {
if (a)
if (await a)
if (a.b)
if (a && b);
}
`,
// Don't case parenthesis in outer test
'if (((a || b))) if (((c || d)));',
// Comments
outdent`
if /* will keep */
(
/* will keep */
a /* will keep */
.b /* will keep */
) /* keep */{
/* will remove */
if (
/* will remove */
c /* will keep */
.d /* will remove */
) {
/* will keep */
foo();
/* will keep */
}
/* will remove */
}
`,
// Semicolon
outdent`
if (a) {
if (b) foo()
}
[].forEach(bar)
`,
outdent`
if (a)
if (b) foo()
;[].forEach(bar)
`
]);

0 comments on commit 485dbf9

Please sign in to comment.