Skip to content

Conversation

ntBre
Copy link
Contributor

@ntBre ntBre commented Jun 18, 2025

Summary

This PR avoids the Vec::retain call in check_tokens by checking if rules are enabled as their diagnostics are constructed.

context
.as_mut_vec()
.retain(|diagnostic| settings.rules.enabled(diagnostic.rule()));

Since LintContext::report_diagnostic_if_enabled required a LinterSettings, I added a settings field to the context itself instead of trying to pass it everywhere. This also turned LogicalLinesContext into a trivial wrapper around LintContext, so I just removed it in favor of using LintContext directly too.

The diff is a bit smaller with whitespace hidden since many blocks got moved into something like this:

if let Some(mut diagnostic) = context.report_diagnostic.enabled(...) {
    // old code
}

Test Plan

Existing tests

ntBre added 4 commits June 18, 2025 15:48
since we needed the LinterSettings for the report_diagnostic_if_enabled calls, I
also replaced the standalone fields that were previously extracted from the
Settings in the constructor
after the LinterSettings change, it was just a thin wrapper
@ntBre ntBre added internal An internal refactor or improvement diagnostics Related to reporting of diagnostics. labels Jun 18, 2025
@ntBre ntBre requested review from MichaReiser and AlexWaygood and removed request for AlexWaygood June 18, 2025 20:42
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Nice

@ntBre ntBre marked this pull request as ready for review June 18, 2025 20:46
@ntBre ntBre requested a review from AlexWaygood as a code owner June 18, 2025 20:46
@ntBre ntBre removed the request for review from AlexWaygood June 18, 2025 20:46
Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@ntBre ntBre merged commit e352a50 into main Jun 18, 2025
34 of 35 checks passed
@ntBre ntBre deleted the brent/check-check-tokens branch June 18, 2025 21:05
dcreager added a commit that referenced this pull request Jun 19, 2025
* main: (68 commits)
  Unify `OldDiagnostic` and `Message` (#18391)
  [`pylint`] Detect more exotic NaN literals in `PLW0177` (#18630)
  [`flake8-async`] Mark autofix for `ASYNC115` as unsafe if the call expression contains comments (#18753)
  [`flake8-bugbear`] Mark autofix for `B004` as unsafe if the `hasattr` call expr contains comments (#18755)
  Enforce `pytest` import for decorators (#18779)
  [`flake8-comprehension`] Mark autofix for `C420` as unsafe if there's comments inside the dict comprehension (#18768)
  [flake8-async] fix detection for large integer sleep durations in `ASYNC116` rule (#18767)
  Update dependency ruff to v0.12.0 (#18790)
  Update taiki-e/install-action action to v2.53.2 (#18789)
  Add lint rule for calling chmod with non-octal integers (#18541)
  Mark `RET501` fix unsafe if comments are inside (#18780)
  Use `LintContext::report_diagnostic_if_enabled` in `check_tokens` (#18769)
  [UP008]: use `super()`, not `__super__` in error messages (#18743)
  Use Depot Windows runners for `cargo test` (#18754)
  Run ty benchmarks when `ruff_benchmark` changes (#18758)
  Disallow newlines in format specifiers of single quoted f- or t-strings (#18708)
  [ty] Add more benchmarks (#18714)
  [ty] Anchor all exclude patterns (#18685)
  Include changelog reference for other major versions (#18745)
  Use updated pre-commit id (#18718)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diagnostics Related to reporting of diagnostics. internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants