refactor(linter): improve eslint/no-constant-condition
#8845
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
github-actions
bot
added
A-linter
CodSpeed Performance ReportMerging #8845 will not alter performanceComparing Summary
|
| fn run_once(&self, ctx: &LintContext) { | ||
| let mut current_scope = Span::default(); | ||
| let mut scopes = Vec::<(Span, FxHashSet<Span>)>::new(); | ||
| for node in ctx.semantic().nodes() { |
There was a problem hiding this comment.
For correctness rules, we must not loop over the ast nodes again due to performance reasons. You can use a ref cell in the rule config if you really need to store state.
There was a problem hiding this comment.
We seem to have tried this before, but because the rule is shared across multiple threads, it seems impossible to store it in the rule config.
I've re-ported all the test cases for
eslint/no-constant-conditionand ensured they pass successfully. Additionally, I have aligned its options configuration witheslint.