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
feat: Add new rule no-empty-static-block
#16325
Conversation
✅ Deploy Preview for docs-eslint canceled.
|
Is there anything I should do to make progress on this? |
@sosukesuzuki if this pull request is ready for review, please click "Ready for Review". We don't leave a lot of feedback on draft PRs. |
849be2a
to
e69ed16
Compare
|
||
## Options | ||
|
||
Nothing. |
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.
## Options | |
Nothing. |
This is not needed. If the rule doesn't have any options, this section should be omitted.
lib/rules/no-empty-static-block.js
Outdated
type: "suggestion", | ||
|
||
docs: { | ||
description: "Disallows empty static blocks", |
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.
description: "Disallows empty static blocks", | |
description: "Disallow empty static blocks", |
lib/rules/no-empty-static-block.js
Outdated
if (node.body.length === 0 && innerComments.length === 0) { | ||
context.report({ | ||
node, | ||
loc: node.loc, |
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.
loc: node.loc, |
This is redundant, node.loc
is used by default if loc
is not provided.
lib/rules/no-empty-static-block.js
Outdated
const innerComments = sourceCode.getTokens(node, { | ||
includeComments: true, | ||
filter: astUtils.isCommentToken | ||
}); |
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 think that only comments inside { }
should count.
class Foo {
static // this comment should not suppress the error
{
}
}
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.
What do you think about c697d95 ?
e69ed16
to
c697d95
Compare
lib/rules/no-empty-static-block.js
Outdated
StaticBlock(node) { | ||
const [blockOpenToken] = sourceCode.getFirstTokens(node, { | ||
filter: token => token.type === "Punctuator" && token.value === "{" | ||
}); | ||
const innerComments = | ||
sourceCode.getCommentsInside(node).filter(commentToken => blockOpenToken.range[1] < commentToken.range[0]); | ||
|
||
if (node.body.length === 0 && innerComments.length === 0) { | ||
context.report({ | ||
node, | ||
messageId: "unexpected" | ||
}); | ||
} | ||
} |
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 think we should move token calculations into if (node.body.length === 0) {}
, for performance reasons.
It could be something like this:
StaticBlock(node) { | |
const [blockOpenToken] = sourceCode.getFirstTokens(node, { | |
filter: token => token.type === "Punctuator" && token.value === "{" | |
}); | |
const innerComments = | |
sourceCode.getCommentsInside(node).filter(commentToken => blockOpenToken.range[1] < commentToken.range[0]); | |
if (node.body.length === 0 && innerComments.length === 0) { | |
context.report({ | |
node, | |
messageId: "unexpected" | |
}); | |
} | |
} | |
StaticBlock(node) { | |
if (node.body.length === 0) { | |
const closingBrace = sourceCode.getLastToken(node); | |
if (sourceCode.getCommentsBefore(closingBrace).length === 0) { | |
context.report({ | |
node, | |
messageId: "unexpected" | |
}); | |
} | |
} | |
} |
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.
LGTM, thanks!
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.
LGTM. Thanks for contributing ⭐
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.26.0` -> `8.27.0`](https://renovatebot.com/diffs/npm/eslint/8.26.0/8.27.0) | --- ### Release Notes <details> <summary>eslint/eslint</summary> ### [`v8.27.0`](https://github.com/eslint/eslint/releases/tag/v8.27.0) [Compare Source](eslint/eslint@v8.26.0...v8.27.0) #### Features - [`f14587c`](eslint/eslint@f14587c) feat: new `no-new-native-nonconstructor` rule ([#​16368](eslint/eslint#16368)) (Sosuke Suzuki) - [`978799b`](eslint/eslint@978799b) feat: add new rule `no-empty-static-block` ([#​16325](eslint/eslint#16325)) (Sosuke Suzuki) - [`69216ee`](eslint/eslint@69216ee) feat: no-empty suggest to add comment in empty BlockStatement ([#​16470](eslint/eslint#16470)) (Nitin Kumar) - [`319f0a5`](eslint/eslint@319f0a5) feat: use `context.languageOptions.ecmaVersion` in core rules ([#​16458](eslint/eslint#16458)) (Milos Djermanovic) #### Bug Fixes - [`c3ce521`](eslint/eslint@c3ce521) fix: Ensure unmatched glob patterns throw an error ([#​16462](eslint/eslint#16462)) (Nicholas C. Zakas) - [`886a038`](eslint/eslint@886a038) fix: handle files with unspecified path in `getRulesMetaForResults` ([#​16437](eslint/eslint#16437)) (Francesco Trotta) #### Documentation - [`ce93b42`](eslint/eslint@ce93b42) docs: Stylelint property-no-unknown ([#​16497](eslint/eslint#16497)) (Nick Schonning) - [`d2cecb4`](eslint/eslint@d2cecb4) docs: Stylelint declaration-block-no-shorthand-property-overrides ([#​16498](eslint/eslint#16498)) (Nick Schonning) - [`0a92805`](eslint/eslint@0a92805) docs: stylelint color-hex-case ([#​16496](eslint/eslint#16496)) (Nick Schonning) - [`74a5af4`](eslint/eslint@74a5af4) docs: fix stylelint error ([#​16491](eslint/eslint#16491)) (Milos Djermanovic) - [`324db1a`](eslint/eslint@324db1a) docs: explicit stylelint color related rules ([#​16465](eslint/eslint#16465)) (Nick Schonning) - [`94dc4f1`](eslint/eslint@94dc4f1) docs: use Stylelint for HTML files ([#​16468](eslint/eslint#16468)) (Nick Schonning) - [`cc6128d`](eslint/eslint@cc6128d) docs: enable stylelint declaration-block-no-duplicate-properties ([#​16466](eslint/eslint#16466)) (Nick Schonning) - [`d03a8bf`](eslint/eslint@d03a8bf) docs: Add heading to justification explanation ([#​16430](eslint/eslint#16430)) (Maritaria) - [`8a15968`](eslint/eslint@8a15968) docs: add Stylelint configuration and cleanup ([#​16379](eslint/eslint#16379)) (Nick Schonning) - [`9b0a469`](eslint/eslint@9b0a469) docs: note commit messages don't support scope ([#​16435](eslint/eslint#16435)) (Andy Edwards) - [`1581405`](eslint/eslint@1581405) docs: improve context.getScope() docs ([#​16417](eslint/eslint#16417)) (Ben Perlmutter) - [`b797149`](eslint/eslint@b797149) docs: update formatters template ([#​16454](eslint/eslint#16454)) (Milos Djermanovic) - [`5ac4de9`](eslint/eslint@5ac4de9) docs: fix link to formatters on the Core Concepts page ([#​16455](eslint/eslint#16455)) (Vladislav) - [`33313ef`](eslint/eslint@33313ef) docs: core-concepts: fix link to semi rule ([#​16453](eslint/eslint#16453)) (coderaiser) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xOS4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjEuMiJ9--> Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1628 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
If #16318 is accepted, I'll make this PR ready for review.
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[x] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
Fixes #16318
Adds new rule
no-empty-static-block
Is there anything you'd like reviewers to focus on?