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 empty-brace-spaces rule #933

Merged
merged 4 commits into from Dec 12, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 28 additions & 0 deletions docs/rules/empty-brace-spaces.md
@@ -0,0 +1,28 @@
# Enforce no spaces between braces

This rule is fixable.

## Fail

```js
class Unicorn {
}
```

```js
try {
foo();
} catch { }
```

## Pass

```js
class Unicorn {}
```

```js
try {
foo();
} catch {}
```
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -21,6 +21,7 @@ module.exports = {
'unicorn/catch-error-name': 'error',
'unicorn/consistent-function-scoping': 'error',
'unicorn/custom-error-definition': 'off',
'unicorn/empty-brace-spaces': 'error',
'unicorn/error-message': 'error',
'unicorn/escape-case': 'error',
'unicorn/expiring-todo-comments': 'error',
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Expand Up @@ -37,6 +37,7 @@ Configure it in `package.json`.
"unicorn/catch-error-name": "error",
"unicorn/consistent-function-scoping": "error",
"unicorn/custom-error-definition": "off",
"unicorn/empty-brace-spaces": "error",
"unicorn/error-message": "error",
"unicorn/escape-case": "error",
"unicorn/expiring-todo-comments": "error",
Expand Down Expand Up @@ -104,6 +105,7 @@ Configure it in `package.json`.
- [catch-error-name](docs/rules/catch-error-name.md) - Enforce a specific parameter name in catch clauses. *(fixable)*
- [consistent-function-scoping](docs/rules/consistent-function-scoping.md) - Move function definitions to the highest possible scope.
- [custom-error-definition](docs/rules/custom-error-definition.md) - Enforce correct `Error` subclassing. *(fixable)*
- [empty-brace-spaces](docs/rules/empty-brace-spaces.md) - Enforce no spaces between braces. *(fixable)*
- [error-message](docs/rules/error-message.md) - Enforce passing a `message` value when creating a built-in error.
- [escape-case](docs/rules/escape-case.md) - Require escape sequences to use uppercase values. *(fixable)*
- [expiring-todo-comments](docs/rules/expiring-todo-comments.md) - Add expiration conditions to TODO comments.
Expand Down
51 changes: 51 additions & 0 deletions rules/empty-brace-spaces.js
@@ -0,0 +1,51 @@
'use strict';
const getDocumentationUrl = require('./utils/get-documentation-url');

const MESSAGE_ID = 'empty-brace-spaces';
const messages = {
[MESSAGE_ID]: 'Do not add spaces between braces.'
};

const selector = `:matches(${
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it easier to just do this?

const selector = ":matches(BlockStatement[body.length=0], ClassBody[body.length=0], ObjectExpression[properties.length=0])"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, same. But I prefer current one, it's more readable add diff friendly, if we want add more.

const selector = `:matches(${
	[
		'BlockStatement[body.length=0]',
		'ClassBody[body.length=0]',
+		'ImportNamespaceSpecifier[assertions.length=0]',
		'ObjectExpression[properties.length=0]'
	].join(', ')
})`;

VS

- const selector = ":matches(BlockStatement[body.length=0], ClassBody[body.length=0], ObjectExpression[properties.length=0])"
+ const selector = ":matches(BlockStatement[body.length=0], ClassBody[body.length=0], ImportNamespaceSpecifier[assertions.length=0], ObjectExpression[properties.length=0])"

[
'BlockStatement[body.length=0]',
'ClassBody[body.length=0]',
'ObjectExpression[properties.length=0]'
].join(', ')
})`;

const create = context => {
const sourceCode = context.getSourceCode();
return {
[selector](node) {
let [start, end] = node.range;
sindresorhus marked this conversation as resolved.
Show resolved Hide resolved
start += 1;
end -= 1;

if (!/^\s+$/.test(sourceCode.text.slice(start, end))) {
return;
}

context.report({
loc: {
start: sourceCode.getLocFromIndex(start),
end: sourceCode.getLocFromIndex(end)
},
messageId: MESSAGE_ID,
fix: fixer => fixer.replaceTextRange([start, end], '')
});
}
};
};

module.exports = {
create,
meta: {
type: 'layout',
docs: {
url: getDocumentationUrl(__filename)
},
fixable: 'whitespace',
messages
}
};
89 changes: 89 additions & 0 deletions test/empty-brace-spaces.js
@@ -0,0 +1,89 @@
import {outdent} from 'outdent';
import {flatten} from 'lodash';
import {test} from './utils/test';

const SPACES_PLACEHOLDER = '/* */';
const cases = [
'{/* */}',
'function foo(){/* */}',
'if(foo) {/* */}',
'if(foo) {} else if (bar) {/* */}',
'if(foo) {} else {/* */}',
'for(;;){/* */}',
'for(foo in bar){/* */}',
'for(foo of bar){/* */}',
'switch (foo) {case bar: {/* */}}',
'switch (foo) {default: {/* */}}',
'try {/* */} catch(foo){}',
'try {} catch(foo){/* */}',
'try {} catch(foo){} finally {/* */}',
'do {/* */} while (foo)',
'while (foo){/* */}',
'foo = () => {/* */}',
'foo = function (){/* */}',
'function foo(){/* */}',
'foo = {/* */}',
'class Foo {bar() {/* */}}',
'foo = class {bar() {/* */}}'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to test for () => {}.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

];
const classBodyCases = [
'class Foo {/* */}',
'foo = class {/* */}'
];
const allCases = [...cases, ...classBodyCases];

const ignoredCases = [
'switch (foo) {/* */}',
'const {/* */} = foo',
'import {/* */} from "foo"'
];

test({
valid: [
...flatten([
'',
'/* comment */',
'\n\t// comment \n'
].map(body => allCases.map(code => code.replace(SPACES_PLACEHOLDER, body)))),
// Not empty
...cases.map(code => code.replace(SPACES_PLACEHOLDER, 'unicorn')),
...classBodyCases.map(code => code.replace(SPACES_PLACEHOLDER, 'bar() {}')),
// `with`
{
code: 'with (foo) {}',
parserOptions: {ecmaVersion: 5, sourceType: 'script'}
},
// We don't check these cases
...ignoredCases.map(code => code.replace(SPACES_PLACEHOLDER, ' '))
],
invalid: [
...flatten([
' ',
'\t',
' \t \t ',
'\n\n',
'\r\n'
].map(spaces => allCases.map(code => ({
code: code.replace(SPACES_PLACEHOLDER, spaces),
output: code.replace(SPACES_PLACEHOLDER, ''),
errors: 1
})))),
// `with`
{
code: 'with (foo) { }',
output: 'with (foo) {}',
errors: 1,
parserOptions: {ecmaVersion: 5, sourceType: 'script'}
}
]
});

test.visualize([
outdent`
try {
foo();
} catch (error) {
\u0020\u0020\u0020\u0020\u0020\u0020\u0020
}
`
]);
3 changes: 1 addition & 2 deletions test/prevent-abbreviations.js
Expand Up @@ -21,8 +21,7 @@ const browserES5RuleTester = avaRuleTester(test, {
const noFixingTestCase = test => ({...test, output: test.code});

const createErrors = message => {
const error = {
};
const error = {};

if (message) {
error.message = message;
Expand Down
33 changes: 33 additions & 0 deletions test/snapshots/empty-brace-spaces.js.md
@@ -0,0 +1,33 @@
# Snapshot report for `test/empty-brace-spaces.js`

The actual snapshot is saved in `empty-brace-spaces.js.snap`.

Generated by [AVA](https://avajs.dev).

## empty-brace-spaces - #1

> Snapshot 1

`␊
Input:␊
1 | try {␊
2 | foo();␊
3 | } catch (error) {␊
4 | ␊
5 | }␊
Output:␊
1 | try {␊
2 | foo();␊
3 | } catch (error) {}␊
Error 1/1:␊
1 | try {␊
2 | foo();␊
> 3 | } catch (error) {␊
| ^␊
> 4 | ␊
| ^^^^^^^^␊
> 5 | }␊
| ^ Do not add spaces between braces.␊
`
Binary file added test/snapshots/empty-brace-spaces.js.snap
Binary file not shown.