feat: Add new rule `no-empty-static-block` #16325

sosukesuzuki · 2022-09-17T16:00:37Z

If #16318 is accepted, I'll make this PR ready for review.

Prerequisites checklist

I have read the contributing guidelines.

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?

netlify · 2022-09-17T16:00:50Z

✅ Deploy Preview for docs-eslint canceled.

Name	Link
🔨 Latest commit	`8f26be5`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/63655e48de41760009465ad1

tests/lib/rules/no-empty-static-block.js

sosukesuzuki · 2022-10-07T01:16:33Z

Is there anything I should do to make progress on this?

nzakas · 2022-10-25T19:15:07Z

@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.

mdjermanovic · 2022-10-30T12:11:12Z

docs/src/rules/no-empty-static-block.md

+
+## Options
+
+Nothing.


Suggested change

## Options

Nothing.

This is not needed. If the rule doesn't have any options, this section should be omitted.

mdjermanovic · 2022-10-30T12:15:52Z

lib/rules/no-empty-static-block.js

+        type: "suggestion",
+
+        docs: {
+            description: "Disallows empty static blocks",


Suggested change

description: "Disallows empty static blocks",

description: "Disallow empty static blocks",

mdjermanovic · 2022-10-30T12:20:17Z

lib/rules/no-empty-static-block.js

+                if (node.body.length === 0 && innerComments.length === 0) {
+                    context.report({
+                        node,
+                        loc: node.loc,


Suggested change

loc: node.loc,

This is redundant, node.loc is used by default if loc is not provided.

mdjermanovic · 2022-10-30T12:30:16Z

lib/rules/no-empty-static-block.js

+                const innerComments = sourceCode.getTokens(node, {
+                    includeComments: true,
+                    filter: astUtils.isCommentToken
+                });


I think that only comments inside { } should count.

class Foo { static // this comment should not suppress the error { } }

What do you think about c697d95 ?

mdjermanovic · 2022-10-31T22:41:12Z

lib/rules/no-empty-static-block.js

+            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"
+                    });
+                }
+            }


I think we should move token calculations into if (node.body.length === 0) {}, for performance reasons.

It could be something like this:

Suggested change

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"

});

}

}

}

mdjermanovic

LGTM, thanks!

snitin315

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. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  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>

eslint-github-bot bot added feature This change adds a new feature to ESLint triage An ESLint team member will look at this issue soon labels Sep 17, 2022

mdjermanovic added rule Relates to ESLint's core rules evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Sep 19, 2022

aladdin-add reviewed Sep 20, 2022

View reviewed changes

tests/lib/rules/no-empty-static-block.js Outdated Show resolved Hide resolved

sosukesuzuki force-pushed the no-empty-static-block branch from 849be2a to e69ed16 Compare October 26, 2022 02:48

sosukesuzuki marked this pull request as ready for review October 26, 2022 02:50

mdjermanovic requested changes Oct 30, 2022

View reviewed changes

sosukesuzuki added 10 commits October 31, 2022 09:07

feat: add no-empty-static-block

3f3ba34

docs: add docs for no-empty-static-block

b043a11

chore: add no-empty-static-block

7e6e412

test: add tests for multiple static blocks

14f6e6a

Add further_reading

690f40d

test: specify ecmaVersion at RuleTester

27d1256

docs: remove options section

d3fc888

fix: disallows -> disallow

ff14329

fix: remove loc parameters

5943523

feat: ignore comment that isn't inside block

c697d95

sosukesuzuki force-pushed the no-empty-static-block branch from e69ed16 to c697d95 Compare October 31, 2022 00:57

mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Oct 31, 2022

mdjermanovic reviewed Oct 31, 2022

View reviewed changes

fix: address review

8f26be5

mdjermanovic approved these changes Nov 4, 2022

View reviewed changes

mdjermanovic added the contributor pool label Nov 4, 2022

snitin315 approved these changes Nov 6, 2022

View reviewed changes

snitin315 merged commit 978799b into eslint:main Nov 6, 2022

freaktechnik mentioned this pull request Nov 7, 2022

make sure new eslint rules are used freaktechnik/eslint-configs#530

Closed

mdjermanovic mentioned this pull request Nov 9, 2022

chore: stricter validation of meta.docs.description in core rules #16529

Merged

1 task

eslint-github-bot bot locked and limited conversation to collaborators May 6, 2023

eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label May 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add new rule `no-empty-static-block` #16325

feat: Add new rule `no-empty-static-block` #16325

sosukesuzuki commented Sep 17, 2022

netlify bot commented Sep 17, 2022 •

edited

sosukesuzuki commented Oct 7, 2022

nzakas commented Oct 25, 2022

mdjermanovic Oct 30, 2022

mdjermanovic Oct 30, 2022

mdjermanovic Oct 30, 2022

mdjermanovic Oct 30, 2022

sosukesuzuki Oct 31, 2022

mdjermanovic Oct 31, 2022 •

edited

mdjermanovic left a comment

snitin315 left a comment

	description: "Disallows empty static blocks",
	description: "Disallow empty static blocks",

feat: Add new rule no-empty-static-block #16325

feat: Add new rule no-empty-static-block #16325

Conversation

sosukesuzuki commented Sep 17, 2022

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

netlify bot commented Sep 17, 2022 • edited

✅ Deploy Preview for docs-eslint canceled.

sosukesuzuki commented Oct 7, 2022

nzakas commented Oct 25, 2022

mdjermanovic Oct 30, 2022

Choose a reason for hiding this comment

mdjermanovic Oct 30, 2022

Choose a reason for hiding this comment

mdjermanovic Oct 30, 2022

Choose a reason for hiding this comment

mdjermanovic Oct 30, 2022

Choose a reason for hiding this comment

sosukesuzuki Oct 31, 2022

Choose a reason for hiding this comment

mdjermanovic Oct 31, 2022 • edited

Choose a reason for hiding this comment

mdjermanovic left a comment

Choose a reason for hiding this comment

snitin315 left a comment

Choose a reason for hiding this comment

feat: Add new rule `no-empty-static-block` #16325

feat: Add new rule `no-empty-static-block` #16325

netlify bot commented Sep 17, 2022 •

edited

mdjermanovic Oct 31, 2022 •

edited