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
Conversation
db4003e
to
a99a89d
Compare
fbd2115
to
c50ebe7
Compare
c50ebe7
to
418f187
Compare
[MESSAGE_ID]: 'Do not add spaces between braces.' | ||
}; | ||
|
||
const selector = `:matches(${ |
There was a problem hiding this comment.
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])"
There was a problem hiding this comment.
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])"
'function foo(){/* */}', | ||
'foo = {/* */}', | ||
'class Foo {bar() {/* */}}', | ||
'foo = class {bar() {/* */}}' |
There was a problem hiding this comment.
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 () => {}
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rules/empty-brace-spaces.js
Outdated
}; | ||
}; | ||
|
||
const schema = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the point of adding empty schema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in template, forgot to remove.
BTW: Do you know eslint-plugin-eslint-plugin
not working on our rules? And actually this is enforced by it.
Even when there are no options for a rule, a schema should still be defined (as an empty array) so that eslint can validate that no data is passed to the rule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why it didn't catch this. It has caught issues in the past though.
I renamed
empty-brace-space
toempty-brace-spaces
, consistent withno-console-spaces
rule name.Fixes #900