Unify `OldDiagnostic` and `Message` #18391

ntBre · 2025-05-30T19:27:53Z

Summary

This PR unifies the remaining differences between OldDiagnostic and
Message (OldDiagnostic was only missing an optional noqa_offset field) and
replaces Message with OldDiagnostic.

The biggest functional difference is that the combined OldDiagnostic kind no
longer implements AsRule for an infallible conversion to Rule. This was
pretty easy to work around with is_some_and and is_none_or in the few places
it was needed. In LintContext::report_diagnostic_if_enabled we can just use
the new Violation::rule method, which takes care of most cases.

Most of the interesting changes are in this range before I started renaming.

Test Plan

Existing tests

Future Work

I think it's time to start shifting some of these fields to the new Diagnostic
kind. I believe we want Fix for sure, but I'm less sure about the others. We
may want to keep a thin wrapper type here anyway to implement a rule method,
so we could leave some of these fields on that too.

ntBre · 2025-05-30T19:31:29Z

crates/ruff_linter/src/message/mod.rs

+    // diagnostic refactor, but if it still exists in this form at the end of the refactor, we
+    // should just update the call sites.
+    #[expect(clippy::needless_pass_by_value)]
+    pub fn new<T: Violation>(kind: T, range: TextRange, file: &SourceFile) -> Self {


It might be time to update the call sites.

With the exception of this method itself, I think the rest of this hunk was moved from diagnostic.rs with no changes.

codspeed-hq · 2025-05-30T19:33:50Z

CodSpeed Instrumentation Performance Report

Merging #18391 will not alter performance

_{Comparing brent/combine-message-diagnostic (52a54a9) with main (4e83db4)}

Summary

✅ 37 untouched benchmarks

github-actions · 2025-05-30T19:33:54Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

crates/ruff_linter/src/checkers/tokens.rs

ntBre · 2025-05-30T19:45:40Z

Well the benchmark results are disappointing, but we're still ahead of where we were before #18234:

	Benchmark	Change	#18234
❌	`linter/all-rules[numpy/globals.py]`	-6.81%	+7.9%
❌	`linter/all-rules[pydantic/types.py]`	-5.4%	+7.04%
❌	`linter/all-rules[unicode/pypinyin.py]`	-13.82%	+21.11%
❌	`linter/all-with-preview-rules[numpy/ctypeslib.py]`	-4.2%	+6.13%
❌	`linter/all-with-preview-rules[numpy/globals.py]`	-5.47%	+6.11%
❌	`linter/all-with-preview-rules[pydantic/types.py]`	-4.74%	+6.49%
❌	`linter/all-with-preview-rules[unicode/pypinyin.py]`	-12.77%	+20.21%
❌	`linter/all-rules[numpy/ctypeslib.py]`	-3%	+5.03%

I think I already went through all of the to_rule calls, but I'll look at the flamegraphs and see if I can find any improvements.

MichaReiser · 2025-05-30T21:12:57Z

I haven't reviewed this but I think we should remove the rule method if possible (not as part of this pr)

ntBre · 2025-06-02T20:09:46Z

Alright, we're finally back under the codspeed threshold, but the macro changes in the last commit feel potentially hacky to me. There are also still some ~2% perf regressions, but I think I handled all of the hot spots. The remaining new code in the flamegraphs looks pretty broadly distributed, which I think means it's just from the new Diagnostic construction.

One big thing I noticed is that all of the rules in check_tokens unconditionally push diagnostics and then a retain cleans them up at the end. Changing all of the report_diagnostic calls in that call graph to report_diagnostic_if_enabled should be a big help because both Diagnostic construction and the to_rule call in the final retain are expensive. But I think that's a better fit for a follow-up.

I'll leave this in draft because I'm a bit uncertain about the proc macro changes, especially, but I think it's ready for at least a quick review.

For a little more context on the proc macro changes, my goal was to make the conversion of NoqaCode to Rule less expensive by avoiding calling Rule::from_code:

ruff/crates/ruff_linter/src/registry.rs

Lines 18 to 26 in 87a15f7

    
           impl Rule { 
        
               pub fn from_code(code: &str) -> Result<Self, FromCodeError> { 
        
                   let (linter, code) = Linter::parse_code(code).ok_or(FromCodeError::Unknown)?; 
        
                   linter 
        
                       .all_rules() 
        
                       .find(|rule| rule.noqa_code().suffix() == code) 
        
                       .ok_or(FromCodeError::Unknown) 
        
               } 
        
           }

which both requires formatting a NoqaCode into a String to call and then reparses the NoqaCode into its parts. I moved Linter::common_prefix out of the RuleNamespace trait so that it could become const, which enabled me to pass its output to the match in generate_rule_to_code.

Rule::from_code was already faster than parsing a rule from a kebab-case rule name, which I saw in the performance report after bba7f6d. The macro changes drop the worst-case performance regression from -4.8% to -2%. That difference might be small enough not to mess with the macros anyway.

MichaReiser · 2025-06-03T06:52:04Z

I skimmed through the changes and had two thoughts:

Would it have been easier to replace the usages of Message::to_rule instead of trying to optimize its performance, given that this is probably something we want to do anyway (because we can't add Rule to Diagnostic or have it anywhere in our shared rendering logic). I skimmed through the usages and I think most places can easily be replaced by testing against the noqa code.
For the noqa code. One thing I mentioned in one of your previous PRs is that we could change the NoqaCode representation from String, String to &'static str, usizethere theusize is the split point between prefix and suffix. Using a pre-computed offset allows both prefix and suffix to return a &str instead of a String. I'm not sure if this would have been helpful here.

ntBre · 2025-06-03T12:20:50Z

Thanks for having a look!

Would it have been easier to replace the usages of Message::to_rule instead of trying to optimize its performance, given that this is probably something we want to do anyway

I tried that first, but my attempts turned out to be much larger refactors than expected or didn't help the performance like I hoped. For example, this commit was a slight regression from the commit before it and also made cmp_fix very awkward. This could still be the right approach since we'll need it later, though, like you said.

For the noqa code. One thing I mentioned in one of your previous PRs is that we could change the NoqaCode representation from String, String to &'static str, usizethere theusize is the split point between prefix and suffix. Using a pre-computed offset allows both prefix and suffix to return a &str instead of a String. I'm not sure if this would have been helpful here.

Ah I did forget about that one. I tried using NoqaCode(Linter, &'static str) instead of the current NoqaCode(&'static str, &'static str), but not (&str, usize).

I can give both of these another try, but I'll probably start in a separate PR since I think they can be handled separately.

## Summary As the title says, this PR removes the `Message::to_rule` method by replacing related uses of `Rule` with `NoqaCode` (or the rule's name in the case of the cache). Where it seemed a `Rule` was really needed, we convert back to the `Rule` by parsing either the rule name (with `str::parse`) or the `NoqaCode` (with `Rule::from_code`). I thought this was kind of like cheating and that it might not resolve this part of Micha's [comment](#18391 (comment)): > because we can't add Rule to Diagnostic or **have it anywhere in our shared rendering logic** but after looking again, the only remaining `Rule` conversion in rendering code is for the SARIF output format. The other two non-test `Rule` conversions are for caching and writing a fix summary, which I don't think fall into the shared rendering logic. That leaves the SARIF format as the only real problem, but maybe we can delay that for now. The motivation here is that we won't be able to store a `Rule` on the new `Diagnostic` type, but we should be able to store a `NoqaCode`, likely as a string. ## Test Plan Existing tests ## [Benchmarks](https://codspeed.io/astral-sh/ruff/branches/brent%2Fremove-to-rule) Almost no perf regression, only -1% on `linter/default-rules[large/dataset.py]`. --------- Co-authored-by: Micha Reiser <[email protected]>

ntBre · 2025-06-05T19:54:09Z

I spent a while today looking into refactoring NoqaCode to be a (&'static str, usize), and I think it's basically impossible. The Linter::common_prefix code is generated in the RuleNamespace derive macro, while the rest of the rule code is generated in the map_codes macro, so there's nowhere that has access to both the prefix and suffix strings to generate a single &'static str combining them.

Even with a const common_prefix method like I added here, we can't actually call that const function in the macro, so we still can't get access to it at macro-expansion time. We can get two separate const &'static strs, but I don't believe there's any way to combine them, at least without some wild transmutes through byte arrays, which is what the crates I found mentioning this did.

I think I need to do one more pass here to check the sites of to_rule calls¹ that were added as part of the OldDiagnostic and Message merger to see if it's worth keeping the macro changes that added NoqaCode::rule, but otherwise this is up to date and the perf looks even better than before (worst case is -1%, down from -2%).

To clarify, to_rule is still removed, they were added here before I rebased on the PR removing them, and I replaced them with noqa_code().and_then(|code| code.rule()). ↩

ntBre · 2025-06-17T20:05:02Z

I rebased on main and reverted the proc macro changes to see if they really help the benchmarks now or not.

Edit: then I dropped the revert again since reverting caused an 8.7% perf regression.

ntBre · 2025-06-18T12:46:51Z

With #18736 on hold for now, I think this is ready for review.

MichaReiser

Thank you.

I think this is a step in the right direction. What's unclear to me is how we can avoid the noqa_code.rule() calls entirely (or in most cases). I left a few suggestions but that might require some more larger refactors (or maybe not?).

The reason why I think we should avoid NoqaCode::rule is because: a) it's slow and b) I don't think we should store a NoqaCode on ruff_db::Diagnostic (we can, but I would prefer not to)

crates/ruff_linter/src/checkers/tokens.rs

MichaReiser · 2025-06-18T13:06:19Z

crates/ruff_linter/src/message/mod.rs

    pub diagnostic: db::Diagnostic,

    // these fields are specific to rule violations
    pub fix: Option<Fix>,
    pub parent: Option<TextSize>,
    pub(crate) noqa_offset: Option<TextSize>,
-    noqa_code: Option<NoqaCode>,
+    pub(crate) noqa_code: Option<NoqaCode>,


One challenge with the approach you've taken here is that I don't think we can store a NoqaCode on ruff_db::Diagnostic. Ideally, the type on the ruff_db::Diagnostic would just be a String or an otherwise mostly agnostic type.

I don't know how much that changes to your refactor. I guess we could call Rule::from_code in the places where we need to go back from code to Rule. I sort of would prefer this but I suspect that this would be slower because that requires actual parsing (and not just matching).

crates/ruff_linter/src/linter.rs

MichaReiser · 2025-06-18T13:37:38Z

crates/ruff_linter/src/linter.rs

+            if diagnostic
+                .noqa_code()
+                .and_then(|code| code.rule())
+                .is_none_or(|rule| !settings.rules.should_fix(rule))
+            {


This would be a larger refactor, I think. But it would be somewhat nice if diagnostic.build_fix would return an Option<FixBuilder> that's only Some if we should generate a fix. But that's probably a larger refactor too. This would have the advantage that we still have a reference to Rule inside of DiagnosticBuilder. But I fear this would be another large and very painful refactor and I'm not sure if its' worth it.

Actually. I don't think that would work because we always want to generate a fix to show it to users even when it shouldn't get applied.... Although, looking at the code here I'm not sure if that's how Ruff currently works. Either way, I think this coudl simplify things actually...

Instead of what I wrote above. DiagnosticBuilder::with_fix or set_fix could still check if the fix should be applied and then wrap the Fix in a FixWithFixability which is an enum with two variants: Fix(Fix), DisplayOnly(Fix) where DisplayOnly is a fix that we only want to show but not count in statistics or apply when applying code fixes.

Either way. I think that would be a separate refactor

I could certainly be wrong, but my reading of this section of code is that it's removing fixes that the user explicitly marked as unfixable. Should we still generate a fix for display in those cases?

If not, I think I came up with a very simple solution, just applying both the applicability from your other comment and this should_fix check in DiagnosticGuard::set_fix. I'll open a PR soon to see what you think.

MichaReiser · 2025-06-18T13:42:47Z

crates/ruff_linter/src/linter.rs

-                        .fix_safety
-                        .resolve_applicability(diagnostic.rule(), fix.applicability());
-                    diagnostic.set_fix(fix.with_applicability(fixed_applicability));
+                    if let Ok(rule) = diagnostic.name().parse() {


I guess, we could also eagerly resolve the applibabily during construction instead of doing it here.

Should this use noqa_code instead of name().parse()?

MichaReiser · 2025-06-18T13:44:28Z

crates/ruff_linter/src/test.rs

+    path: &Path,
+    source: &SourceKind,
+) -> String {
+    diagnostics.retain(|msg| !msg.is_syntax_error());


This is so confusing ... looking forward to have a single print_diagnostics function that prints all diagnostics.

MichaReiser · 2025-06-18T13:45:20Z

crates/ruff_macros/src/map_codes.rs

@@ -340,6 +350,16 @@ See also https://github.com/astral-sh/ruff/issues/2186.
                }
            }
        }
+
+        impl NoqaCode {


By how much does this blow up ruff's code size?

I guess if there's one NoqaCode per rule and ~900 rules, this adds ~900 constants (which might get inlined?) and ~900 match arms. Is that a lot? I don't really have a good intuition for the effects of these kinds of changes on code size.

Anyway, I'm still hoping we can revert this part of the PR if we can recover the perf hits in other ways, hopefully from your other suggestions.

I suggest building ruff and comparing the binary sizes. You can also use https://github.com/RazrFalcon/cargo-bloat to get an understanding of the compiled code

I got these results from du -b target/release/ruff:

41,559,344 bytes for main 41,669,840 bytes for this branch

so I guess this one is 110 kilobytes bigger. NoqaCode::rule does make an appearance in cargo-bloat too. Rule::from_str is present in both, so it would be nice if we ever get to remove that too.

cargo-bloat output

this branch:

File .text Size Crate Name 0.2% 0.3% 76.0KiB ruff_linter ruff_linter::checkers::ast::analyze::expression::expression 0.1% 0.3% 72.1KiB ruff <ruff::args::CheckCommand as clap_builder::derive::Args>::augment_args 0.1% 0.2% 51.4KiB ruff_workspace? <ruff_workspace::options::_::<impl serde::de::Deserialize for ruff_workspace::options::LintOptionsWire>::deserialize::__Visitor as serde::de::Visitor>::visit_map 0.1% 0.2% 51.4KiB ruff_workspace? <ruff_workspace::options::_::<impl serde::de::Deserialize for ruff_workspace::options::LintOptionsWire>::deserialize::__Visitor as serde::de::Visitor>::visit_map 0.1% 0.2% 51.4KiB ruff_workspace? <ruff_workspace::options::_::<impl serde::de::Deserialize for ruff_workspace::options::LintOptionsWire>::deserialize::__Visitor as serde::de::Visitor>::visit_map 0.1% 0.2% 43.8KiB ruff_linter? <ruff_linter::codes::Rule as core::str::traits::FromStr>::from_str 0.1% 0.2% 39.8KiB ruff_linter ruff_linter::checkers::ast::analyze::statement::statement 0.1% 0.2% 35.7KiB ruff_workspace? <ruff_workspace::options::_::<impl serde::de::Deserialize for ruff_workspace::options::LintOptionsWire>::deserialize::__Visitor as serde::de::Visitor>::visit_map 0.1% 0.2% 35.7KiB ruff_workspace? <ruff_workspace::options::_::<impl serde::de::Deserialize for ruff_workspace::options::LintOptionsWire>::deserialize::__Visitor as serde::de::Visitor>::visit_map 0.1% 0.2% 35.3KiB ruff_linter <alloc::vec::Vec<T,A> as core::clone::Clone>::clone 0.1% 0.2% 35.1KiB ruff_python_codegen ruff_python_codegen::generator::Generator::unparse_stmt 0.1% 0.1% 32.4KiB ruff_workspace ruff_workspace::configuration::LintConfiguration::as_rule_table 0.1% 0.1% 32.4KiB ruff_workspace ruff_workspace::configuration::Configuration::into_settings 0.1% 0.1% 31.3KiB ruff_linter ruff_linter::checkers::ast::analyze::definitions::definitions 0.1% 0.1% 31.3KiB ruff_linter ruff_linter::checkers::ast::analyze::deferred_scopes::deferred_scopes 0.1% 0.1% 31.2KiB ruff_linter ruff_linter::codes::NoqaCode::rule 0.1% 0.1% 30.8KiB ruff_workspace ruff_workspace::configuration::Configuration::from_options 0.1% 0.1% 29.8KiB libcst_native libcst_native::parser::grammar::python::__parse_compound_stmt 0.1% 0.1% 29.5KiB ruff_server <toml::value::Value as serde::de::Deserializer>::deserialize_any 0.1% 0.1% 29.5KiB ruff <toml::value::Value as serde::de::Deserializer>::deserialize_any 44.5% 95.4% 21.3MiB And 33811 smaller methods. Use -n N to show more. 46.7% 100.0% 22.4MiB .text section size, the file size is 47.9MiB

main:

File .text Size Crate Name 0.1% 0.3% 72.1KiB ruff <ruff::args::CheckCommand as clap_builder::derive::Args>::augment_args 0.1% 0.2% 51.4KiB ruff_workspace? <ruff_workspace::options::_::<impl serde::de::Deserialize for ruff_workspace::options::LintOptionsWire>::deserialize::__Visitor as serde::de::Visitor>::visit_map 0.1% 0.2% 51.4KiB ruff_workspace? <ruff_workspace::options::_::<impl serde::de::Deserialize for ruff_workspace::options::LintOptionsWire>::deserialize::__Visitor as serde::de::Visitor>::visit_map 0.1% 0.2% 51.4KiB ruff_workspace? <ruff_workspace::options::_::<impl serde::de::Deserialize for ruff_workspace::options::LintOptionsWire>::deserialize::__Visitor as serde::de::Visitor>::visit_map 0.1% 0.2% 46.2KiB ruff_linter ruff_linter::checkers::ast::analyze::statement::statement 0.1% 0.2% 43.8KiB ruff_linter? <ruff_linter::codes::Rule as core::str::traits::FromStr>::from_str 0.1% 0.2% 37.8KiB ruff_linter ruff_linter::checkers::ast::analyze::expression::expression 0.1% 0.2% 35.7KiB ruff_workspace? <ruff_workspace::options::_::<impl serde::de::Deserialize for ruff_workspace::options::LintOptionsWire>::deserialize::__Visitor as serde::de::Visitor>::visit_map 0.1% 0.2% 35.7KiB ruff_workspace? <ruff_workspace::options::_::<impl serde::de::Deserialize for ruff_workspace::options::LintOptionsWire>::deserialize::__Visitor as serde::de::Visitor>::visit_map 0.1% 0.2% 35.3KiB ruff_linter <alloc::vec::Vec<T,A> as core::clone::Clone>::clone 0.1% 0.2% 35.1KiB ruff_python_codegen ruff_python_codegen::generator::Generator::unparse_stmt 0.1% 0.1% 32.4KiB ruff_workspace ruff_workspace::configuration::LintConfiguration::as_rule_table 0.1% 0.1% 32.4KiB ruff_workspace ruff_workspace::configuration::Configuration::into_settings 0.1% 0.1% 30.8KiB ruff_workspace ruff_workspace::configuration::Configuration::from_options 0.1% 0.1% 29.8KiB libcst_native libcst_native::parser::grammar::python::__parse_compound_stmt 0.1% 0.1% 29.5KiB ruff_server <toml::value::Value as serde::de::Deserializer>::deserialize_any 0.1% 0.1% 29.5KiB ruff <toml::value::Value as serde::de::Deserializer>::deserialize_any 0.1% 0.1% 28.7KiB libcst_native? <libcst_native::nodes::expression::Expression as core::clone::Clone>::clone 0.1% 0.1% 28.7KiB ruff_linter ruff_linter::checkers::ast::analyze::deferred_scopes::deferred_scopes 0.1% 0.1% 27.9KiB ruff <ruff::args::FormatCommand as clap_builder::derive::Args>::augment_args 44.7% 95.5% 21.3MiB And 33578 smaller methods. Use -n N to show more. 46.7% 100.0% 22.3MiB .text section size, the file size is 47.8MiB

Summary -- This PR unifies the remaining differences between `OldDiagnostic` and `Message` (`OldDiagnostic` was only missing an optional `noqa_offset` field) and replaces `Message` with `OldDiagnostic`. The biggest functional difference is that the combined `OldDiagnostic` kind no longer implements `AsRule` for an infallible conversion to `Rule`. This was pretty easy to work around with `is_some_and` and `is_none_or` in the few places it was needed. In `LintContext::report_diagnostic_if_enabled` we can just use the new `Violation::rule` method, which takes care of most cases. Test Plan -- Existing tests Future Work -- I think it's time to start shifting some of these fields to the new `Diagnostic` kind. I believe we want `Fix` for sure, but I'm less sure about the others. We may want to keep a thin wrapper type here anyway to implement a `rule` method, so we could leave some of these fields on that too.

it's not exactly direct because we're still matching on a bunch of &'static strs, but it's better than having to call Rule::from_code, which has to format a string and then reparse the NoqaCode into pieces

MichaReiser · 2025-06-19T09:00:49Z

By the way. I don't think any of those other refactors should block this PR. We can merge it and then try to reduce the places where we use noqa_code().rule() (Ideally, remove them all)

* 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) ...

## Summary This PR avoids one of the three calls to `NoqaCode::rule` from #18391 by applying per-file ignores in the `LintContext`. To help with this, it also replaces all direct uses of `LinterSettings.rules.enabled` with a `LintContext::enabled` (or `Checker::enabled`, which defers to its context) method. There are still some direct accesses to `settings.rules`, but as far as I can tell these are not in a part of the code where we can really access a `LintContext`. I believe all of the code reachable from `check_path`, where the replaced per-file ignore code was, should be converted to the new methods. ## Test Plan Existing tests, with a single snapshot updated for RUF100, which I think actually shows a more accurate diagnostic message now.

from 08a227e in #18391

…ard` and remove `NoqaCode::rule` (#18834) ## Summary This PR removes the last two places we were using `NoqaCode::rule` in `linter.rs` (see #18391 (comment) and #18391 (comment)) by checking whether fixes are actually desired before adding them to a `DiagnosticGuard`. I implemented this by storing a `Violation`'s `Rule` on the `DiagnosticGuard` so that we could check if it was enabled in the embedded `LinterSettings` when trying to set a fix. All of the corresponding `set_fix` methods on `OldDiagnostic` were now unused (except in tests where I just set `.fix` directly), so I moved these to the guard instead of keeping both sets. The very last place where we were using `NoqaCode::rule` was in the cache. I just reverted this to parsing the `Rule` from the name. I had forgotten to update the comment there anyway. Hopefully this doesn't cause too much of a perf hit. In terms of binary size, we're back down almost to where `main` was two days ago (#18391 (comment)): ``` 41,559,344 bytes for main 2 days ago 41,669,840 bytes for #18391 41,653,760 bytes for main now (after #18391 merged) 41,602,224 bytes for this branch ``` Only 43 kb up, but that shouldn't all be me this time :) ## Test Plan Existing tests and benchmarks on this PR

ntBre added internal An internal refactor or improvement diagnostics Related to reporting of diagnostics. labels May 30, 2025

ntBre commented May 30, 2025

View reviewed changes

crates/ruff_linter/src/checkers/tokens.rs Outdated Show resolved Hide resolved

ntBre force-pushed the brent/combine-message-diagnostic branch from 12e4da0 to d04d40d Compare May 30, 2025 23:15

ntBre mentioned this pull request Jun 3, 2025

Remove Message::to_rule #18447

Merged

ntBre force-pushed the brent/combine-message-diagnostic branch from 87a15f7 to f8a9009 Compare June 5, 2025 13:49

ntBre force-pushed the brent/combine-message-diagnostic branch 2 times, most recently from ec38bad to 00c7690 Compare June 5, 2025 19:12

ntBre force-pushed the brent/combine-message-diagnostic branch from 00c7690 to b4930e4 Compare June 17, 2025 20:04

ntBre force-pushed the brent/combine-message-diagnostic branch from b4930e4 to 3655543 Compare June 17, 2025 20:17

ntBre mentioned this pull request Jun 17, 2025

Change NoqaCode to a single string with an offset #18736

Closed

ntBre marked this pull request as ready for review June 18, 2025 12:46

ntBre requested a review from AlexWaygood as a code owner June 18, 2025 12:46

ntBre requested review from MichaReiser and removed request for AlexWaygood June 18, 2025 12:47

MichaReiser approved these changes Jun 18, 2025

View reviewed changes

ntBre added 2 commits June 18, 2025 20:11

make the fields of OldDiagnostic the same as Message

a792d9c

ntBre added 14 commits June 18, 2025 20:11

make OldDiagnostic an alias for Message

7c1a83b

move all implementation code to Message

a6a0430

update re-export, delete type alias and diagnostic module

5d60ac8

remove useless Message::from_diagnostic method

9cff2a6

avoid duplicating Diagnostic construction from Message::diagnostic

c64a3c6

rename Message to OldDiagnostic

de7fe93

message(s) -> diagnostic(s) for fields and variables

d0cb2e2

rename Diagnostics::diagnostics to avoid lint and repetition

c77a102

OldDiagnostic::diagnostic -> OldDiagnostic::lint

61261db

make lint generic like syntax_error

ecaf5ce

inline variable to avoid constructing diagnostic for disabled rule

103f3d0

fix outdated docs

dcc4992

fix fuzz build

97f0590

convert to Rule "directly" from NoqaCode

08a227e

it's not exactly direct because we're still matching on a bunch of &'static strs, but it's better than having to call Rule::from_code, which has to format a string and then reparse the NoqaCode into pieces

ntBre force-pushed the brent/combine-message-diagnostic branch from 3655543 to 08a227e Compare June 19, 2025 00:14

ntBre closed this Jun 19, 2025

ntBre reopened this Jun 19, 2025

noqa_code().rule() instead of name().parse()

52a54a9

ntBre merged commit 10a1d9f into main Jun 19, 2025
35 checks passed

ntBre deleted the brent/combine-message-diagnostic branch June 19, 2025 13:38

ntBre mentioned this pull request Jun 19, 2025

Avoid generating diagnostics with per-file ignores #18801

Merged

ntBre added a commit that referenced this pull request Jun 20, 2025

revert RuleNamespace and NoqaCode::rule changes

b2b65e7

from 08a227e in #18391

ntBre mentioned this pull request Jun 20, 2025

Apply fix availability and applicability when adding to DiagnosticGuard and remove NoqaCode::rule #18834

Merged

Unify OldDiagnostic and Message #18391

Unify OldDiagnostic and Message #18391

Uh oh!

Conversation

ntBre commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Future Work

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codspeed-hq bot commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #18391 will not alter performance

Summary

Uh oh!

github-actions bot commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

Uh oh!

ntBre commented May 30, 2025

Uh oh!

MichaReiser commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ntBre commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaReiser commented Jun 3, 2025

Uh oh!

ntBre commented Jun 3, 2025

Uh oh!

ntBre commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Footnotes

Uh oh!

ntBre commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ntBre commented Jun 18, 2025

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser commented Jun 19, 2025

Uh oh!

Uh oh!

Uh oh!

Unify `OldDiagnostic` and `Message` #18391

Unify `OldDiagnostic` and `Message` #18391

ntBre commented May 30, 2025 •

edited

Loading

codspeed-hq bot commented May 30, 2025 •

edited

Loading

github-actions bot commented May 30, 2025 •

edited

Loading

`ruff-ecosystem` results

MichaReiser commented May 30, 2025 •

edited

Loading

ntBre commented Jun 2, 2025 •

edited

Loading

ntBre commented Jun 5, 2025 •

edited

Loading

ntBre commented Jun 17, 2025 •

edited

Loading