Added a no-duplicate-switch-case rule #2937
Conversation
| } | ||
| } | ||
|
|
||
| const walk = (ctx: Lint.WalkContext<void>): void => { |
There was a problem hiding this comment.
consider converting these lambdas to regular function declarations
There was a problem hiding this comment.
Is there a reason behind the preference for regular function declarations? Elsewhere I've seen folks prefer arrow lambdas for the saner scoping & lack of hoisting weirdness.
There was a problem hiding this comment.
The rest of the codebase seems to follow the pattern to use function declarations where possible and only use arrow functions where this reference matters.
I prefer regular functions because I think it's easier to notice while scanning through the code
| const previousCases = new Set<string>(); | ||
|
|
||
| return (node: ts.CaseBlock): void => { | ||
| previousCases.clear(); |
There was a problem hiding this comment.
no need to return a closure or clear the set.
just make previousCases a local variable and loop over the cases.
There was a problem hiding this comment.
It's a minor performance optimization to keep the same set across iterations. Do you consider the lessened clarity not worth it?
There was a problem hiding this comment.
I would guess clearing the set is as expensive as creating a new one.
I don't think it's worth optimizing here
| continue; | ||
| } | ||
|
|
||
| const text = clause.expression.getText(); |
There was a problem hiding this comment.
consider passing ctx.sourceFile to .getText to avoid unnecessary work
|
@JoshuaKGoldberg The rule needs to be added to |
|
I'd like to get this merged. So I updated the configuration presets and merged master. @adidahiya do you want to take another look before merging this? |
|
Thanks @ajafff for the updates! A side note: there are a few other PRs from me waiting for action (on my part). I don't know when I'll be able to get to them :( |
PR checklist
Overview of change:
Added a
no-duplicate-switch-caserule to enforce that no switch case should contain the same textual value as a previous switch statement.CHANGELOG.md entry:
[new-rule]
no-duplicate-switch-case