Skip to content

Conversation

@Nohgh
Copy link
Contributor

@Nohgh Nohgh commented Aug 11, 2025

Prerequisites checklist

What is the purpose of this pull request?

Enhance the no-empty-definitions rule to include identifier data in its verification process,
ensuring more informative and accurate error messages.

What changes did you make? (Give an overview)

  • Updated no-empty-definitions rule logic to verify and expose identifier data.
  • Added corresponding test cases in no-empty-definitions.test.js to cover new behavior.
  • Ensured all existing tests pass.

Related Issues

refs #499

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

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 11, 2025

CLA Signed


The committers listed above are authorized under a signed CLA.

@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Aug 12, 2025
@lumirlumir lumirlumir linked an issue Aug 12, 2025 that may be closed by this pull request
1 task
Copy link
Member

@lumirlumir lumirlumir left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Would like another review before merging, since it's a small change but adds a new feature.

@lumirlumir lumirlumir moved this from Needs Triage to Second Review Needed in Triage Aug 12, 2025
messages: {
emptyDefinition: "Unexpected empty definition found.",
emptyDefinition:
"Unexpected empty definition `{{ identifier }}` found.",
Copy link
Member

Choose a reason for hiding this comment

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

To the team: I think using backticks here would be fine, since the similar rule no-unused-definition uses backticks as well.

"Unexpected unused footnote definition `{{ identifier }}` found.",

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@nzakas nzakas merged commit 738f366 into eslint:main Aug 19, 2025
23 of 24 checks passed
@github-project-automation github-project-automation bot moved this from Second Review Needed to Complete in Triage Aug 19, 2025
@injae-kim
Copy link

Nice work @Nohgh 🎉🎉🎉

@nzakas
Copy link
Member

nzakas commented Sep 4, 2025

@Nohgh we'd like to pay you for this contribution. Please contact us contact (at) eslint (dot) org so we can send you the details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

Rule Change: Expose identifiers in the message for the no-*-definitions rule

4 participants