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.

Sign up for GitHub

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

Jump to bottom

Change no-duplicate-case rule to comparing tokens #13485

Closed
ota-meshi opened this issue Jul 12, 2020 · 3 comments · Fixed by #13494
Closed

Change no-duplicate-case rule to comparing tokens #13485

ota-meshi opened this issue Jul 12, 2020 · 3 comments · Fixed by #13494
Assignees
@mdjermanovic
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules

Comments

@ota-meshi
Copy link
Member

What rule do you want to change?

  • no-duplicate-case

Does this change cause the rule to produce more or fewer warnings?

Produce more warnings.

How will the change be implemented? (New option, new default behavior, etc.)?

Makes the process of detecting duplicates compare by token, like no-dupe-else-if.

Please provide some example code that this change will affect:

switch (a) {
  case a.b:
  case a.b: // Already reported.
  case a.      b: // Will be reported.
  case a./**/b: // Will be reported.
}

What does the rule currently do for this code?

The comparison is text based.

What will the rule do after it's changed?

a.b, a. b and a./**/b are reported as duplicates.

Are you willing to submit a pull request to implement this change?

If there is consensus, I will submit a pull request.

refs #13466 (comment)
@ota-meshi ota-meshi added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules triage An ESLint team member will look at this issue soon labels Jul 12, 2020
@mdjermanovic mdjermanovic added 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 Jul 12, 2020
@mdjermanovic mdjermanovic self-assigned this Jul 12, 2020
@mdjermanovic
Copy link
Member

@ota-meshi thanks for the issue!

I'll champion this.

The only downside is that this increases time complexity (currently used Set methods are sublinear, plus we'll additionally have to compare arrays of tokens), but I don't think it will be an issue since it is expected to work on small lists.

Only, I'm not sure should we make this change in a minor version, or rather wait for the next major version.

@mdjermanovic 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 Jul 15, 2020
@mdjermanovic
Copy link
Member

There are 3 upvotes from team members, so marking as accepted.

If there is consensus, I will submit a pull request.

PR is welcome now!

(I think we should discuss if this should be a semver-minor or semver-major change, but that shouldn't affect the PR)

@ota-meshi
Copy link
Member Author

ota-meshi commented Jul 16, 2020 •
edited

@mdjermanovic Thank you for letting me know!

I will work on PR after today's work.

@ota-meshi ota-meshi mentioned this issue Jul 16, 2020
Merged
1 task
mdjermanovic pushed a commit that referenced this issue Jul 22, 2020
@ota-meshi
Update: Change no-duplicate-case to comparing tokens (fixes #13485) (#…
e71e298 
…13494)

Change `no-duplicate-case` rule to change from detecting duplicates comparing text to detecting duplicates comparing tokens, like `no-dupe-else-if` rule.
moeriki added a commit to moeriki/eslint that referenced this issue Aug 9, 2020
@moeriki
Merge remote-tracking branch 'upstream/master' into no-magic-number-i…
cf3a140 
…gnore-default-values

* upstream/master: (66 commits)
  Sponsors: Sync README with website
  Sponsors: Sync README with website
  Sponsors: Sync README with website
  Sponsors: Sync README with website
  Sponsors: Sync README with website
  Chore: remove leche (fixes eslint#13287) (eslint#13533)
  Sponsors: Sync README with website
  Sponsors: Sync README with website
  7.6.0
  Build: changelog update for 7.6.0
  Update: require `meta` for fixable rules in RuleTester (refs eslint#13349) (eslint#13489)
  Docs: fix broken links in developer guide (eslint#13518)
  Fix: Do not output `undefined` as line and column when it's unavailable (eslint#13519)
  Sponsors: Sync README with website
  Sponsors: Sync README with website
  Fix: Update the chatroom link to go directly to help channel (eslint#13536)
  Sponsors: Sync README with website
  Update: Change no-duplicate-case to comparing tokens (fixes eslint#13485) (eslint#13494)
  Docs: add ECMAScript 2020 to README (eslint#13510)
  7.5.0
  ...
@robatwilliams robatwilliams mentioned this issue Aug 21, 2020
Closed
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jan 19, 2021
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jan 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mdjermanovic mdjermanovic

Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update: Change no-duplicate-case to comparing tokens (fixes #13485)
2 participants
@ota-meshi @mdjermanovic