Skip to content

Commit

Permalink
Add switch-case-braces rule (#1902)
Browse files Browse the repository at this point in the history
  • Loading branch information
fisker committed Sep 20, 2022
1 parent 81efe41 commit 690ed8c
Show file tree
Hide file tree
Showing 23 changed files with 959 additions and 65 deletions.
1 change: 1 addition & 0 deletions configs/recommended.js
Expand Up @@ -118,6 +118,7 @@ module.exports = {
// See #1396
'unicorn/require-post-message-target-origin': 'off',
'unicorn/string-content': 'off',
'unicorn/switch-case-braces': 'error',
'unicorn/template-indent': 'error',
'unicorn/text-encoding-identifier-case': 'error',
'unicorn/throw-new-error': 'error',
Expand Down
87 changes: 87 additions & 0 deletions docs/rules/switch-case-braces.md
@@ -0,0 +1,87 @@
# Enforce consistent brace style for `case` clauses

<!-- Do not manually modify RULE_NOTICE part. Run: `npm run generate-rule-notices` -->
<!-- RULE_NOTICE -->
*This rule is part of the [recommended](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config) config.*

🔧 *This rule is [auto-fixable](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems).*
<!-- /RULE_NOTICE -->

1. Forbid braces for empty clauses.
1. Enforce braces for non-empty clauses.

## Fail

```js
switch (foo) {
case 1: {
}
case 2: {
doSomething();
break;
}
}
```

```js
switch (foo) {
case 1:
doSomething();
break;
}
```

## Pass

```js
switch (foo) {
case 1: {
doSomething();
break;
}
}
```

## Options

Type: `string`\
Default: `'always'`

- `'always'` (default)
- Always report when clause is not a `BlockStatement`.
- `'avoid'`
- Only allow braces when there are variable declaration or function declaration which requires a scope.

The following cases are considered valid:

```js
// eslint unicorn/switch-case-braces: ["error", "avoid"]
switch (foo) {
case 1:
doSomething();
break;
}
```

```js
// eslint unicorn/switch-case-braces: ["error", "avoid"]
switch (foo) {
case 1: {
const bar = 2;
doSomething(bar);
break;
}
}
```

The following case is considered invalid:

```js
// eslint unicorn/switch-case-braces: ["error", "avoid"]
switch (foo) {
case 1: {
doSomething();
break;
}
}
```
1 change: 1 addition & 0 deletions readme.md
Expand Up @@ -154,6 +154,7 @@ Each rule has emojis denoting:
| [require-number-to-fixed-digits-argument](docs/rules/require-number-to-fixed-digits-argument.md) | Enforce using the digits argument with `Number#toFixed()`. || 🔧 | |
| [require-post-message-target-origin](docs/rules/require-post-message-target-origin.md) | Enforce using the `targetOrigin` argument with `window.postMessage()`. | | | 💡 |
| [string-content](docs/rules/string-content.md) | Enforce better string content. | | 🔧 | 💡 |
| [switch-case-braces](docs/rules/switch-case-braces.md) | Enforce consistent brace style for `case` clauses. || 🔧 | |
| [template-indent](docs/rules/template-indent.md) | Fix whitespace-insensitive template indentation. || 🔧 | |
| [text-encoding-identifier-case](docs/rules/text-encoding-identifier-case.md) | Enforce consistent case for text encoding identifiers. || 🔧 | 💡 |
| [throw-new-error](docs/rules/throw-new-error.md) | Require `new` when throwing an error. || 🔧 | |
Expand Down
36 changes: 26 additions & 10 deletions rules/no-array-for-each.js
Expand Up @@ -59,20 +59,23 @@ function shouldSwitchReturnStatementToBlockStatement(returnStatement) {
const {parent} = returnStatement;

switch (parent.type) {
case 'IfStatement':
case 'IfStatement': {
return parent.consequent === returnStatement || parent.alternate === returnStatement;
}

// These parent's body need switch to `BlockStatement` too, but since they are "continueAble", won't fix
// case 'ForStatement':
// case 'ForInStatement':
// case 'ForOfStatement':
// case 'WhileStatement':
// case 'DoWhileStatement':
case 'WithStatement':
case 'WithStatement': {
return parent.body === returnStatement;
}

default:
default: {
return false;
}
}
}

Expand Down Expand Up @@ -315,20 +318,33 @@ function isAssignmentLeftHandSide(node) {
switch (parent.type) {
case 'AssignmentExpression':
case 'ForInStatement':
case 'ForOfStatement':
case 'ForOfStatement': {
return parent.left === node;
case 'UpdateExpression':
}

case 'UpdateExpression': {
return parent.argument === node;
case 'Property':
}

case 'Property': {
return parent.value === node && isAssignmentLeftHandSide(parent);
case 'AssignmentPattern':
}

case 'AssignmentPattern': {
return parent.left === node && isAssignmentLeftHandSide(parent);
case 'ArrayPattern':
}

case 'ArrayPattern': {
return parent.elements.includes(node) && isAssignmentLeftHandSide(parent);
case 'ObjectPattern':
}

case 'ObjectPattern': {
return parent.properties.includes(node) && isAssignmentLeftHandSide(parent);
default:
}

default: {
return false;
}
}
}

Expand Down
15 changes: 11 additions & 4 deletions rules/no-useless-spread.js
Expand Up @@ -150,18 +150,25 @@ const create = context => {
let parentDescription = '';
let messageId = ITERABLE_TO_ARRAY;
switch (parent.type) {
case 'ForOfStatement':
case 'ForOfStatement': {
messageId = ITERABLE_TO_ARRAY_IN_FOR_OF;
break;
case 'YieldExpression':
}

case 'YieldExpression': {
messageId = ITERABLE_TO_ARRAY_IN_YIELD_STAR;
break;
case 'NewExpression':
}

case 'NewExpression': {
parentDescription = `new ${parent.callee.name}(…)`;
break;
case 'CallExpression':
}

case 'CallExpression': {
parentDescription = `${parent.callee.object.name}.${parent.callee.property.name}(…)`;
break;
}
// No default
}

Expand Down
3 changes: 2 additions & 1 deletion rules/prefer-dom-node-dataset.js
Expand Up @@ -59,9 +59,10 @@ const create = context => ({
break;
}

case 'hasAttribute':
case 'hasAttribute': {
text = `Object.hasOwn(${datasetText}, ${quoteString(name, nameNode.raw.charAt(0))})`;
break;
}
// No default
}

Expand Down
22 changes: 15 additions & 7 deletions rules/prefer-export-from.js
Expand Up @@ -18,10 +18,13 @@ const NAMESPACE_SPECIFIER_NAME = Symbol('NAMESPACE_SPECIFIER_NAME');

const getSpecifierName = node => {
switch (node.type) {
case 'Identifier':
case 'Identifier': {
return Symbol.for(node.name);
case 'Literal':
}

case 'Literal': {
return node.value;
}
// No default
}
};
Expand Down Expand Up @@ -170,21 +173,23 @@ function getFixFunction({
function getExported(identifier, context, sourceCode) {
const {parent} = identifier;
switch (parent.type) {
case 'ExportDefaultDeclaration':
case 'ExportDefaultDeclaration': {
return {
node: parent,
name: DEFAULT_SPECIFIER_NAME,
text: 'default',
isTypeExport: isTypeExport(parent),
};
}

case 'ExportSpecifier':
case 'ExportSpecifier': {
return {
node: parent,
name: getSpecifierName(parent.exported),
text: sourceCode.getText(parent.exported),
isTypeExport: isTypeExport(parent),
};
}

case 'VariableDeclarator': {
if (
Expand Down Expand Up @@ -237,26 +242,29 @@ function getImported(variable, sourceCode) {
};

switch (specifier.type) {
case 'ImportDefaultSpecifier':
case 'ImportDefaultSpecifier': {
return {
name: DEFAULT_SPECIFIER_NAME,
text: 'default',
...result,
};
}

case 'ImportSpecifier':
case 'ImportSpecifier': {
return {
name: getSpecifierName(specifier.imported),
text: sourceCode.getText(specifier.imported),
...result,
};
}

case 'ImportNamespaceSpecifier':
case 'ImportNamespaceSpecifier': {
return {
name: NAMESPACE_SPECIFIER_NAME,
text: '*',
...result,
};
}

// No default
}
Expand Down
3 changes: 2 additions & 1 deletion rules/prefer-keyboard-event-key.js
Expand Up @@ -36,8 +36,9 @@ const getEventNodeAndReferences = (context, node) => {
};
}

default:
default: {
return {};
}
}
};

Expand Down
12 changes: 8 additions & 4 deletions rules/prefer-logical-operator-over-ternary.js
Expand Up @@ -21,25 +21,29 @@ function isSameNode(left, right, sourceCode) {
}

switch (left.type) {
case 'AwaitExpression':
case 'AwaitExpression': {
return isSameNode(left.argument, right.argument, sourceCode);
}

case 'LogicalExpression':
case 'LogicalExpression': {
return (
left.operator === right.operator
&& isSameNode(left.left, right.left, sourceCode)
&& isSameNode(left.right, right.right, sourceCode)
);
}

case 'UnaryExpression':
case 'UnaryExpression': {
return (
left.operator === right.operator
&& left.prefix === right.prefix
&& isSameNode(left.argument, right.argument, sourceCode)
);
}

case 'UpdateExpression':
case 'UpdateExpression': {
return false;
}

// No default
}
Expand Down
15 changes: 10 additions & 5 deletions rules/prefer-string-starts-ends-with.js
Expand Up @@ -103,7 +103,7 @@ const create = context => {

switch (fixType) {
// Goal: `(target ?? '').startsWith(pattern)`
case FIX_TYPE_NULLISH_COALESCING:
case FIX_TYPE_NULLISH_COALESCING: {
if (
!isTargetParenthesized
&& shouldAddParenthesesToLogicalExpressionChild(target, {operator: '??', property: 'left'})
Expand All @@ -120,27 +120,32 @@ const create = context => {
}

break;
}

// Goal: `String(target).startsWith(pattern)`
case FIX_TYPE_STRING_CASTING:
case FIX_TYPE_STRING_CASTING: {
// `target` was a call argument, don't need check parentheses
targetText = `String(${targetText})`;
// `CallExpression` don't need add parentheses to call `.startsWith()`
break;
}

// Goal: `target.startsWith(pattern)` or `target?.startsWith(pattern)`
case FIX_TYPE_OPTIONAL_CHAINING:
case FIX_TYPE_OPTIONAL_CHAINING: {
// Optional chaining: `target.startsWith` => `target?.startsWith`
yield fixer.replaceText(sourceCode.getTokenBefore(node.callee.property), '?.');
// Fallthrough
default:
}

// Fallthrough
default: {
if (
!isRegexParenthesized
&& !isTargetParenthesized
&& shouldAddParenthesesToMemberExpressionObject(target, sourceCode)
) {
targetText = addParentheses(targetText);
}
}
}

// The regex literal always starts with `/` or `(`, so we don't need check ASI
Expand Down

0 comments on commit 690ed8c

Please sign in to comment.