Skip to content

Commit

Permalink
Add prefer-switch rule (#1181)
Browse files Browse the repository at this point in the history
Co-authored-by: Sindre Sorhus <[email protected]>
  • Loading branch information
fisker and sindresorhus authored Apr 19, 2021
1 parent 3521a0a commit 10e7a0c
Show file tree
Hide file tree
Showing 10 changed files with 2,180 additions and 52 deletions.
171 changes: 171 additions & 0 deletions docs/rules/prefer-switch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
# Prefer `switch` over multiple `else-if`

A switch statement is easier to read than multiple if statements with simple equality comparisons.

This rule is partly fixable.

## Fail

```js
if (foo === 1) {
// 1
} else if (foo === 2) {
// 2
} else if (foo === 3) {
// 3
} else {
// default
}
```

## Pass

```js
if (foo === 1) {
// 1
} else if (foo === 2) {
// 2
}
```

```js
switch (foo) {
case 1: {
// 1
break;
}
case 2: {
// 2
break;
}
case 3: {
// 3
break;
}
default: {
// default
}
}
```

### `options`

Type: `object`

#### `minimumCases`

Type: `integer`\
Minimum: `2`\
Default: `3`

The minimum number of cases before reporting.

The `default` branch doesn't count. Multiple comparisons on the same `if` block is considered one case.

Examples:

```js
// eslint unicorn/prefer-switch: ["error", {"minimumCases": 4}]
if (foo === 1) {}
else (foo === 2) {}
else (foo === 3) {}

// Passes, only 3 cases.
```

```js
// eslint unicorn/prefer-switch: ["error", {"minimumCases": 4}]
if (foo === 1) {}
else (foo === 2) {}
else (foo === 3) {}
else {}

// Passes, only 3 cases.
```

```js
// eslint unicorn/prefer-switch: ["error", {"minimumCases": 4}]
if (foo === 1) {}
else if (foo === 2 || foo === 3) {}
else if (foo === 4) {}

// Passes, only 3 cases.
```

```js
// eslint unicorn/prefer-switch: ["error", {"minimumCases": 2}]
if (foo === 1) {}
else if (foo === 2) {}

// Fails
```

#### `emptyDefaultCase`

Type: `string`\
Default: `'no-default-comment'`

To avoid conflict with the [`default-case`](https://eslint.org/docs/rules/default-case) rule, choose the fix style you prefer:

- `'no-default-comment'` (default)
Insert `// No default` comment after last case.
- `'do-nothing-comment'`
Insert `default` case and add `// Do nothing` comment.
- `'no-default-case'`
Don't insert default case or comment.

```js
if (foo === 1) {}
else (foo === 2) {}
else (foo === 3) {}
```

Fixed

```js
/* eslint unicorn/prefer-switch: ["error", { "emptyDefaultCase": "no-default-comment" }] */
switch (foo) {
case 1: {
break;
}
case 2: {
break;
}
case 3: {
break;
}
// No default
}
```

```js
/* eslint unicorn/prefer-switch: ["error", { "emptyDefaultCase": "do-nothing-comment" }] */
switch (foo) {
case 1: {
break;
}
case 2: {
break;
}
case 3: {
break;
}
default:
// Do nothing
}
```

```js
/* eslint unicorn/prefer-switch: ["error", { "emptyDefaultCase": "no-default-case" }] */
switch (foo) {
case 1: {
break;
}
case 2: {
break;
}
case 3: {
break;
}
}
```
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ module.exports = {
'unicorn/prefer-string-slice': 'error',
'unicorn/prefer-string-starts-ends-with': 'error',
'unicorn/prefer-string-trim-start-end': 'error',
'unicorn/prefer-switch': 'error',
'unicorn/prefer-ternary': 'error',
'unicorn/prefer-type-error': 'error',
'unicorn/prevent-abbreviations': 'error',
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ Configure it in `package.json`.
"unicorn/prefer-string-slice": "error",
"unicorn/prefer-string-starts-ends-with": "error",
"unicorn/prefer-string-trim-start-end": "error",
"unicorn/prefer-switch": "error",
"unicorn/prefer-ternary": "off",
"unicorn/prefer-type-error": "error",
"unicorn/prevent-abbreviations": "error",
Expand Down Expand Up @@ -192,6 +193,7 @@ Each rule has emojis denoting:
| [prefer-string-slice](docs/rules/prefer-string-slice.md) | Prefer `String#slice()` over `String#substr()` and `String#substring()`. || 🔧 |
| [prefer-string-starts-ends-with](docs/rules/prefer-string-starts-ends-with.md) | Prefer `String#startsWith()` & `String#endsWith()` over `RegExp#test()`. || 🔧 |
| [prefer-string-trim-start-end](docs/rules/prefer-string-trim-start-end.md) | Prefer `String#trimStart()` / `String#trimEnd()` over `String#trimLeft()` / `String#trimRight()`. || 🔧 |
| [prefer-switch](docs/rules/prefer-switch.md) | Prefer `switch` over multiple `else-if`. || 🔧 |
| [prefer-ternary](docs/rules/prefer-ternary.md) | Prefer ternary expressions over simple `if-else` statements. || 🔧 |
| [prefer-type-error](docs/rules/prefer-type-error.md) | Enforce throwing `TypeError` in type checking conditions. || 🔧 |
| [prevent-abbreviations](docs/rules/prevent-abbreviations.md) | Prevent abbreviations. || 🔧 |
Expand Down
111 changes: 67 additions & 44 deletions rules/prefer-string-slice.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,34 +72,45 @@ const create = context => {

let sliceArguments;

if (argumentNodes.length === 0) {
sliceArguments = [];
} else if (argumentNodes.length === 1) {
sliceArguments = [firstArgument];
} else if (argumentNodes.length === 2) {
if (firstArgument === '0') {
switch (argumentNodes.length) {
case 0: {
sliceArguments = [];
break;
}

case 1: {
sliceArguments = [firstArgument];
if (isLiteralNumber(secondArgument) || isLengthProperty(argumentNodes[1])) {
sliceArguments.push(secondArgument);
} else if (typeof getNumericValue(argumentNodes[1]) === 'number') {
sliceArguments.push(Math.max(0, getNumericValue(argumentNodes[1])));
} else {
sliceArguments.push(`Math.max(0, ${secondArgument})`);
}
} else if (
isLiteralNumber(argumentNodes[0]) &&
isLiteralNumber(argumentNodes[1])
) {
sliceArguments = [
firstArgument,
argumentNodes[0].value + argumentNodes[1].value
];
} else if (
isLikelyNumeric(argumentNodes[0]) &&
break;
}

case 2: {
if (firstArgument === '0') {
sliceArguments = [firstArgument];
if (isLiteralNumber(secondArgument) || isLengthProperty(argumentNodes[1])) {
sliceArguments.push(secondArgument);
} else if (typeof getNumericValue(argumentNodes[1]) === 'number') {
sliceArguments.push(Math.max(0, getNumericValue(argumentNodes[1])));
} else {
sliceArguments.push(`Math.max(0, ${secondArgument})`);
}
} else if (
isLiteralNumber(argumentNodes[0]) &&
isLiteralNumber(argumentNodes[1])
) {
sliceArguments = [
firstArgument,
argumentNodes[0].value + argumentNodes[1].value
];
} else if (
isLikelyNumeric(argumentNodes[0]) &&
isLikelyNumeric(argumentNodes[1])
) {
sliceArguments = [firstArgument, firstArgument + ' + ' + secondArgument];
) {
sliceArguments = [firstArgument, firstArgument + ' + ' + secondArgument];
}

break;
}
// No default
}

if (sliceArguments) {
Expand Down Expand Up @@ -129,33 +140,45 @@ const create = context => {

let sliceArguments;

if (argumentNodes.length === 0) {
sliceArguments = [];
} else if (argumentNodes.length === 1) {
if (firstNumber !== undefined) {
sliceArguments = [Math.max(0, firstNumber)];
} else if (isLengthProperty(argumentNodes[0])) {
sliceArguments = [firstArgument];
} else {
sliceArguments = [`Math.max(0, ${firstArgument})`];
switch (argumentNodes.length) {
case 0: {
sliceArguments = [];
break;
}
} else if (argumentNodes.length === 2) {
const secondNumber = getNumericValue(argumentNodes[1]);

if (firstNumber !== undefined && secondNumber !== undefined) {
sliceArguments = firstNumber > secondNumber ?
[Math.max(0, secondNumber), Math.max(0, firstNumber)] :
[Math.max(0, firstNumber), Math.max(0, secondNumber)];
} else if (firstNumber === 0 || secondNumber === 0) {
sliceArguments = [0, `Math.max(0, ${firstNumber === 0 ? secondArgument : firstArgument})`];
} else {

case 1: {
if (firstNumber !== undefined) {
sliceArguments = [Math.max(0, firstNumber)];
} else if (isLengthProperty(argumentNodes[0])) {
sliceArguments = [firstArgument];
} else {
sliceArguments = [`Math.max(0, ${firstArgument})`];
}

break;
}

case 2: {
const secondNumber = getNumericValue(argumentNodes[1]);

if (firstNumber !== undefined && secondNumber !== undefined) {
sliceArguments = firstNumber > secondNumber ?
[Math.max(0, secondNumber), Math.max(0, firstNumber)] :
[Math.max(0, firstNumber), Math.max(0, secondNumber)];
} else if (firstNumber === 0 || secondNumber === 0) {
sliceArguments = [0, `Math.max(0, ${firstNumber === 0 ? secondArgument : firstArgument})`];
} else {
// As values aren't Literal, we can not know whether secondArgument will become smaller than the first or not, causing an issue:
// .substring(0, 2) and .substring(2, 0) returns the same result
// .slice(0, 2) and .slice(2, 0) doesn't return the same result
// There's also an issue with us now knowing whether the value will be negative or not, due to:
// .substring() treats a negative number the same as it treats a zero.
// The latter issue could be solved by wrapping all dynamic numbers in Math.max(0, <value>), but the resulting code would not be nice
}

break;
}
// No default
}

if (sliceArguments) {
Expand Down
Loading

0 comments on commit 10e7a0c

Please sign in to comment.