From 4fa0bae62a2ac3c4b070ee385d4b26b42a31a500 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 21 Jul 2025 09:53:05 -0400 Subject: [PATCH 1/9] Move fix suggestion to subdiagnostic Summary -- This PR tweaks Ruff's internal usage of the new diagnostic model to more closely match the intended use, as I understand it. Specifically, it moves the fix/help suggestion from the primary annotation's message to a subdiagnostic. In turn, it adds the secondary/noqa code as the new primary annotation message. As shown in the new `ruff_db` tests, this more closely mirrors Ruff's current diagnostic output. I also added `Severity::Help` to render the fix suggestion with a `help:` prefix instead of `info:`. These changes don't have any external impact now but should help a bit with Test Plan -- New full output format tests in `ruff_db` --- crates/ruff_db/src/diagnostic/mod.rs | 4 +- crates/ruff_db/src/diagnostic/render.rs | 42 ++++++------- crates/ruff_db/src/diagnostic/render/full.rs | 66 ++++++++++++++++++++ crates/ruff_linter/src/message/mod.rs | 11 ++-- 4 files changed, 96 insertions(+), 27 deletions(-) create mode 100644 crates/ruff_db/src/diagnostic/render/full.rs diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 947075becbf2e..25d957ce7dafe 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -379,7 +379,9 @@ impl Diagnostic { /// Returns the fix suggestion for the violation. pub fn suggestion(&self) -> Option<&str> { - self.primary_annotation()?.get_message() + self.sub_diagnostics() + .first() + .map(|sub| sub.inner.message.as_str()) } /// Returns the URL for the rule documentation, if it exists. diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index d567e5d5b507e..5ed5328144266 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -26,6 +26,7 @@ use azure::AzureRenderer; use pylint::PylintRenderer; mod azure; +mod full; #[cfg(feature = "serde")] mod json; #[cfg(feature = "serde")] @@ -864,7 +865,9 @@ mod tests { use ruff_diagnostics::{Edit, Fix}; - use crate::diagnostic::{Annotation, DiagnosticId, SecondaryCode, Severity, Span}; + use crate::diagnostic::{ + Annotation, DiagnosticId, IntoDiagnosticMessage, SecondaryCode, Severity, Span, + }; use crate::files::system_path_to_file; use crate::system::{DbWithWritableSystem, SystemPath}; use crate::tests::TestDb; @@ -2494,6 +2497,11 @@ watermelon self.diag.set_noqa_offset(noqa_offset); self } + + fn info(mut self, message: impl IntoDiagnosticMessage) -> DiagnosticBuilder<'e> { + self.diag.info(message); + self + } } /// A helper builder for tersely populating a `SubDiagnostic`. @@ -2600,7 +2608,8 @@ def fibonacci(n): let diagnostics = vec![ env.builder("unused-import", Severity::Error, "`os` imported but unused") - .primary("fib.py", "1:7", "1:9", "Remove unused import: `os`") + .primary("fib.py", "1:7", "1:9", "F401") + .info("Remove unused import: `os`") .secondary_code("F401") .fix(Fix::unsafe_edit(Edit::range_deletion(TextRange::new( TextSize::from(0), @@ -2613,12 +2622,8 @@ def fibonacci(n): Severity::Error, "Local variable `x` is assigned to but never used", ) - .primary( - "fib.py", - "6:4", - "6:5", - "Remove assignment to unused variable `x`", - ) + .primary("fib.py", "6:4", "6:5", "F841") + .info("Remove assignment to unused variable `x`") .secondary_code("F841") .fix(Fix::unsafe_edit(Edit::deletion( TextSize::from(94), @@ -2627,7 +2632,7 @@ def fibonacci(n): .noqa_offset(TextSize::from(94)) .build(), env.builder("undefined-name", Severity::Error, "Undefined name `a`") - .primary("undef.py", "1:3", "1:4", "") + .primary("undef.py", "1:3", "1:4", "F821") .secondary_code("F821") .noqa_offset(TextSize::from(3)) .build(), @@ -2720,7 +2725,8 @@ if call(foo let diagnostics = vec![ env.builder("unused-import", Severity::Error, "`os` imported but unused") - .primary("notebook.ipynb", "2:7", "2:9", "Remove unused import: `os`") + .primary("notebook.ipynb", "2:7", "2:9", "F401") + .info("Remove unused import: `os`") .secondary_code("F401") .fix(Fix::safe_edit(Edit::range_deletion(TextRange::new( TextSize::from(9), @@ -2733,12 +2739,8 @@ if call(foo Severity::Error, "`math` imported but unused", ) - .primary( - "notebook.ipynb", - "4:7", - "4:11", - "Remove unused import: `math`", - ) + .primary("notebook.ipynb", "4:7", "4:11", "F401") + .info("Remove unused import: `math`") .secondary_code("F401") .fix(Fix::safe_edit(Edit::range_deletion(TextRange::new( TextSize::from(28), @@ -2751,12 +2753,8 @@ if call(foo Severity::Error, "Local variable `x` is assigned to but never used", ) - .primary( - "notebook.ipynb", - "10:4", - "10:5", - "Remove assignment to unused variable `x`", - ) + .primary("notebook.ipynb", "10:4", "10:5", "F841") + .info("Remove assignment to unused variable `x`") .secondary_code("F841") .fix(Fix::unsafe_edit(Edit::range_deletion(TextRange::new( TextSize::from(94), diff --git a/crates/ruff_db/src/diagnostic/render/full.rs b/crates/ruff_db/src/diagnostic/render/full.rs new file mode 100644 index 0000000000000..b1380edea6229 --- /dev/null +++ b/crates/ruff_db/src/diagnostic/render/full.rs @@ -0,0 +1,66 @@ +#[cfg(test)] +mod tests { + use crate::diagnostic::{ + DiagnosticFormat, + render::tests::{create_diagnostics, create_syntax_error_diagnostics}, + }; + + #[test] + fn output() { + let (env, diagnostics) = create_diagnostics(DiagnosticFormat::Full); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r#" + error[unused-import]: `os` imported but unused + --> fib.py:1:8 + | + 1 | import os + | ^^ F401 + | + info: Remove unused import: `os` + + error[unused-variable]: Local variable `x` is assigned to but never used + --> fib.py:6:5 + | + 4 | def fibonacci(n): + 5 | """Compute the nth number in the Fibonacci sequence.""" + 6 | x = 1 + | ^ F841 + 7 | if n == 0: + 8 | return 0 + | + info: Remove assignment to unused variable `x` + + error[undefined-name]: Undefined name `a` + --> undef.py:1:4 + | + 1 | if a == 1: pass + | ^ F821 + | + "#); + } + + #[test] + fn syntax_errors() { + let (env, diagnostics) = create_syntax_error_diagnostics(DiagnosticFormat::Full); + insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r" + error[invalid-syntax]: SyntaxError: Expected one or more symbol names after import + --> syntax_errors.py:1:15 + | + 1 | from os import + | ^ + 2 | + 3 | if call(foo + | + + error[invalid-syntax]: SyntaxError: Expected ')', found newline + --> syntax_errors.py:3:12 + | + 1 | from os import + 2 | + 3 | if call(foo + | ^ + 4 | def bar(): + 5 | pass + | + "); + } +} diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 417ae1fbb5692..1646882e0562d 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -74,12 +74,15 @@ where body, ); + let noqa_code = rule.noqa_code(); + let span = Span::from(file).with_range(range); - let mut annotation = Annotation::primary(span); + let annotation = Annotation::primary(span).message(noqa_code); + diagnostic.annotate(annotation); + if let Some(suggestion) = suggestion { - annotation = annotation.message(suggestion); + diagnostic.info(suggestion); } - diagnostic.annotate(annotation); if let Some(fix) = fix { diagnostic.set_fix(fix); @@ -93,7 +96,7 @@ where diagnostic.set_noqa_offset(noqa_offset); } - diagnostic.set_secondary_code(SecondaryCode::new(rule.noqa_code().to_string())); + diagnostic.set_secondary_code(SecondaryCode::new(noqa_code.to_string())); diagnostic } From 04c820d7f55afa70be6aa80876b9691ecfca7c58 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 21 Jul 2025 10:07:31 -0400 Subject: [PATCH 2/9] add and use Severity::Help --- crates/ruff_db/src/diagnostic/mod.rs | 9 +++++++++ crates/ruff_db/src/diagnostic/render.rs | 16 +++++++++------- crates/ruff_db/src/diagnostic/render/azure.rs | 2 +- crates/ruff_db/src/diagnostic/render/full.rs | 4 ++-- crates/ruff_linter/src/message/mod.rs | 2 +- crates/ty/src/lib.rs | 2 +- crates/ty_server/src/server/api/diagnostics.rs | 1 + crates/ty_wasm/src/lib.rs | 2 ++ 8 files changed, 26 insertions(+), 12 deletions(-) diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 25d957ce7dafe..cccaf12d784ef 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -125,6 +125,13 @@ impl Diagnostic { self.sub(SubDiagnostic::new(Severity::Info, message)); } + /// Adds a "help" sub-diagnostic with the given message. + /// + /// See the closely related [`Diagnostic::info`] method for more details. + pub fn help<'a>(&mut self, message: impl IntoDiagnosticMessage + 'a) { + self.sub(SubDiagnostic::new(Severity::Help, message)); + } + /// Adds a "sub" diagnostic to this diagnostic. /// /// This is useful when a sub diagnostic has its own annotations attached @@ -1144,6 +1151,7 @@ impl From for Span { #[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd, get_size2::GetSize)] pub enum Severity { + Help, Info, Warning, Error, @@ -1153,6 +1161,7 @@ pub enum Severity { impl Severity { fn to_annotate(self) -> AnnotateLevel { match self { + Self::Help => AnnotateLevel::Help, Severity::Info => AnnotateLevel::Info, Severity::Warning => AnnotateLevel::Warning, Severity::Error => AnnotateLevel::Error, diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 5ed5328144266..c9f1e29f3c190 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -113,6 +113,7 @@ impl std::fmt::Display for DisplayDiagnostics<'_> { for diag in self.diagnostics { let (severity, severity_style) = match diag.severity() { + Severity::Help => ("help", stylesheet.info), Severity::Info => ("info", stylesheet.info), Severity::Warning => ("warning", stylesheet.warning), Severity::Error => ("error", stylesheet.error), @@ -2498,8 +2499,9 @@ watermelon self } - fn info(mut self, message: impl IntoDiagnosticMessage) -> DiagnosticBuilder<'e> { - self.diag.info(message); + /// Adds a "help" sub-diagnostic with the given message. + fn help(mut self, message: impl IntoDiagnosticMessage) -> DiagnosticBuilder<'e> { + self.diag.help(message); self } } @@ -2609,7 +2611,7 @@ def fibonacci(n): let diagnostics = vec![ env.builder("unused-import", Severity::Error, "`os` imported but unused") .primary("fib.py", "1:7", "1:9", "F401") - .info("Remove unused import: `os`") + .help("Remove unused import: `os`") .secondary_code("F401") .fix(Fix::unsafe_edit(Edit::range_deletion(TextRange::new( TextSize::from(0), @@ -2623,7 +2625,7 @@ def fibonacci(n): "Local variable `x` is assigned to but never used", ) .primary("fib.py", "6:4", "6:5", "F841") - .info("Remove assignment to unused variable `x`") + .help("Remove assignment to unused variable `x`") .secondary_code("F841") .fix(Fix::unsafe_edit(Edit::deletion( TextSize::from(94), @@ -2726,7 +2728,7 @@ if call(foo let diagnostics = vec![ env.builder("unused-import", Severity::Error, "`os` imported but unused") .primary("notebook.ipynb", "2:7", "2:9", "F401") - .info("Remove unused import: `os`") + .help("Remove unused import: `os`") .secondary_code("F401") .fix(Fix::safe_edit(Edit::range_deletion(TextRange::new( TextSize::from(9), @@ -2740,7 +2742,7 @@ if call(foo "`math` imported but unused", ) .primary("notebook.ipynb", "4:7", "4:11", "F401") - .info("Remove unused import: `math`") + .help("Remove unused import: `math`") .secondary_code("F401") .fix(Fix::safe_edit(Edit::range_deletion(TextRange::new( TextSize::from(28), @@ -2754,7 +2756,7 @@ if call(foo "Local variable `x` is assigned to but never used", ) .primary("notebook.ipynb", "10:4", "10:5", "F841") - .info("Remove assignment to unused variable `x`") + .help("Remove assignment to unused variable `x`") .secondary_code("F841") .fix(Fix::unsafe_edit(Edit::range_deletion(TextRange::new( TextSize::from(94), diff --git a/crates/ruff_db/src/diagnostic/render/azure.rs b/crates/ruff_db/src/diagnostic/render/azure.rs index d607e08d861a7..318fdab4e85e9 100644 --- a/crates/ruff_db/src/diagnostic/render/azure.rs +++ b/crates/ruff_db/src/diagnostic/render/azure.rs @@ -22,7 +22,7 @@ impl AzureRenderer<'_> { ) -> std::fmt::Result { for diag in diagnostics { let severity = match diag.severity() { - Severity::Info | Severity::Warning => "warning", + Severity::Help | Severity::Info | Severity::Warning => "warning", Severity::Error | Severity::Fatal => "error", }; write!(f, "##vso[task.logissue type={severity};")?; diff --git a/crates/ruff_db/src/diagnostic/render/full.rs b/crates/ruff_db/src/diagnostic/render/full.rs index b1380edea6229..fd193e33d85e8 100644 --- a/crates/ruff_db/src/diagnostic/render/full.rs +++ b/crates/ruff_db/src/diagnostic/render/full.rs @@ -15,7 +15,7 @@ mod tests { 1 | import os | ^^ F401 | - info: Remove unused import: `os` + help: Remove unused import: `os` error[unused-variable]: Local variable `x` is assigned to but never used --> fib.py:6:5 @@ -27,7 +27,7 @@ mod tests { 7 | if n == 0: 8 | return 0 | - info: Remove assignment to unused variable `x` + help: Remove assignment to unused variable `x` error[undefined-name]: Undefined name `a` --> undef.py:1:4 diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 1646882e0562d..c974366cbd292 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -81,7 +81,7 @@ where diagnostic.annotate(annotation); if let Some(suggestion) = suggestion { - diagnostic.info(suggestion); + diagnostic.help(suggestion); } if let Some(fix) = fix { diff --git a/crates/ty/src/lib.rs b/crates/ty/src/lib.rs index 8a0c5b9fcd590..9f4c6190c1380 100644 --- a/crates/ty/src/lib.rs +++ b/crates/ty/src/lib.rs @@ -349,7 +349,7 @@ impl MainLoop { if self.watcher.is_none() { return Ok(match max_severity { - Severity::Info => ExitStatus::Success, + Severity::Help | Severity::Info => ExitStatus::Success, Severity::Warning => { if terminal_settings.error_on_warning { ExitStatus::Failure diff --git a/crates/ty_server/src/server/api/diagnostics.rs b/crates/ty_server/src/server/api/diagnostics.rs index 11e39796adb9e..8d0cb33ed4a2d 100644 --- a/crates/ty_server/src/server/api/diagnostics.rs +++ b/crates/ty_server/src/server/api/diagnostics.rs @@ -263,6 +263,7 @@ pub(super) fn to_lsp_diagnostic( }; let severity = match diagnostic.severity() { + Severity::Help => DiagnosticSeverity::HINT, Severity::Info => DiagnosticSeverity::INFORMATION, Severity::Warning => DiagnosticSeverity::WARNING, Severity::Error | Severity::Fatal => DiagnosticSeverity::ERROR, diff --git a/crates/ty_wasm/src/lib.rs b/crates/ty_wasm/src/lib.rs index de101ae3c097d..632a7482cfb94 100644 --- a/crates/ty_wasm/src/lib.rs +++ b/crates/ty_wasm/src/lib.rs @@ -726,6 +726,7 @@ impl Position { #[wasm_bindgen] #[derive(Copy, Clone, Hash, PartialEq, Eq)] pub enum Severity { + Help, Info, Warning, Error, @@ -735,6 +736,7 @@ pub enum Severity { impl From for Severity { fn from(value: diagnostic::Severity) -> Self { match value { + diagnostic::Severity::Help => Self::Help, diagnostic::Severity::Info => Self::Info, diagnostic::Severity::Warning => Self::Warning, diagnostic::Severity::Error => Self::Error, From 745b4ce51944094e04df685903e371f7a2962111 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 21 Jul 2025 10:38:18 -0400 Subject: [PATCH 3/9] try using SmallVec for subdiagnostics --- Cargo.lock | 1 + crates/ruff_db/Cargo.toml | 1 + crates/ruff_db/src/diagnostic/mod.rs | 5 +++-- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b0d761cfeee94..ea4c2c5399c91 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2876,6 +2876,7 @@ dependencies = [ "schemars", "serde", "serde_json", + "smallvec", "tempfile", "thiserror 2.0.12", "tracing", diff --git a/crates/ruff_db/Cargo.toml b/crates/ruff_db/Cargo.toml index ccc9edf4804b3..ece00ce38f8ab 100644 --- a/crates/ruff_db/Cargo.toml +++ b/crates/ruff_db/Cargo.toml @@ -39,6 +39,7 @@ salsa = { workspace = true } schemars = { workspace = true, optional = true } serde = { workspace = true, optional = true } serde_json = { workspace = true, optional = true } +smallvec = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } tracing-subscriber = { workspace = true, optional = true } diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index cccaf12d784ef..7effb77d3fbda 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -5,6 +5,7 @@ use ruff_source_file::{LineColumn, SourceCode, SourceFile}; use ruff_annotate_snippets::Level as AnnotateLevel; use ruff_text_size::{Ranged, TextRange, TextSize}; +use smallvec::SmallVec; pub use self::render::{DisplayDiagnostic, DisplayDiagnostics, FileResolver, Input}; use crate::{Db, files::File}; @@ -61,7 +62,7 @@ impl Diagnostic { severity, message: message.into_diagnostic_message(), annotations: vec![], - subs: vec![], + subs: SmallVec::new(), fix: None, parent: None, noqa_offset: None, @@ -479,7 +480,7 @@ struct DiagnosticInner { severity: Severity, message: DiagnosticMessage, annotations: Vec, - subs: Vec, + subs: SmallVec<[SubDiagnostic; 1]>, fix: Option, parent: Option, noqa_offset: Option, From 76185257bc55b8438526956e537c4131c1a7fb8f Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 21 Jul 2025 10:50:43 -0400 Subject: [PATCH 4/9] Revert "try using SmallVec for subdiagnostics" This reverts commit 7c0e0ff1c9d6e0e1b2808da35bb203290825f238. --- Cargo.lock | 1 - crates/ruff_db/Cargo.toml | 1 - crates/ruff_db/src/diagnostic/mod.rs | 5 ++--- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ea4c2c5399c91..b0d761cfeee94 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2876,7 +2876,6 @@ dependencies = [ "schemars", "serde", "serde_json", - "smallvec", "tempfile", "thiserror 2.0.12", "tracing", diff --git a/crates/ruff_db/Cargo.toml b/crates/ruff_db/Cargo.toml index ece00ce38f8ab..ccc9edf4804b3 100644 --- a/crates/ruff_db/Cargo.toml +++ b/crates/ruff_db/Cargo.toml @@ -39,7 +39,6 @@ salsa = { workspace = true } schemars = { workspace = true, optional = true } serde = { workspace = true, optional = true } serde_json = { workspace = true, optional = true } -smallvec = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } tracing-subscriber = { workspace = true, optional = true } diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 7effb77d3fbda..cccaf12d784ef 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -5,7 +5,6 @@ use ruff_source_file::{LineColumn, SourceCode, SourceFile}; use ruff_annotate_snippets::Level as AnnotateLevel; use ruff_text_size::{Ranged, TextRange, TextSize}; -use smallvec::SmallVec; pub use self::render::{DisplayDiagnostic, DisplayDiagnostics, FileResolver, Input}; use crate::{Db, files::File}; @@ -62,7 +61,7 @@ impl Diagnostic { severity, message: message.into_diagnostic_message(), annotations: vec![], - subs: SmallVec::new(), + subs: vec![], fix: None, parent: None, noqa_offset: None, @@ -480,7 +479,7 @@ struct DiagnosticInner { severity: Severity, message: DiagnosticMessage, annotations: Vec, - subs: SmallVec<[SubDiagnostic; 1]>, + subs: Vec, fix: Option, parent: Option, noqa_offset: Option, From a4521acbf068e61c597b75f9bb7b702587b1127d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 21 Jul 2025 10:55:43 -0400 Subject: [PATCH 5/9] try converting to string earlier the idea here is that NoqaCode::display is the expensive step and this basically reduces it to cloning a string instead of reformatting the noqa code --- crates/ruff_linter/src/message/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index c974366cbd292..56c8af2a792ec 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -74,10 +74,10 @@ where body, ); - let noqa_code = rule.noqa_code(); + let noqa_code = rule.noqa_code().to_string(); let span = Span::from(file).with_range(range); - let annotation = Annotation::primary(span).message(noqa_code); + let annotation = Annotation::primary(span).message(&noqa_code); diagnostic.annotate(annotation); if let Some(suggestion) = suggestion { @@ -96,7 +96,7 @@ where diagnostic.set_noqa_offset(noqa_offset); } - diagnostic.set_secondary_code(SecondaryCode::new(noqa_code.to_string())); + diagnostic.set_secondary_code(SecondaryCode::new(noqa_code)); diagnostic } From a16780350d2abb1d75cb1f4ceedd12d417efab78 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 21 Jul 2025 12:22:33 -0400 Subject: [PATCH 6/9] avoid duplicate noqa code in primary message --- crates/ruff_db/src/diagnostic/render.rs | 12 ++++++------ crates/ruff_db/src/diagnostic/render/full.rs | 6 +++--- crates/ruff_linter/src/message/mod.rs | 6 ++---- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index c9f1e29f3c190..8fb8bb9347312 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -2610,7 +2610,7 @@ def fibonacci(n): let diagnostics = vec![ env.builder("unused-import", Severity::Error, "`os` imported but unused") - .primary("fib.py", "1:7", "1:9", "F401") + .primary("fib.py", "1:7", "1:9", "") .help("Remove unused import: `os`") .secondary_code("F401") .fix(Fix::unsafe_edit(Edit::range_deletion(TextRange::new( @@ -2624,7 +2624,7 @@ def fibonacci(n): Severity::Error, "Local variable `x` is assigned to but never used", ) - .primary("fib.py", "6:4", "6:5", "F841") + .primary("fib.py", "6:4", "6:5", "") .help("Remove assignment to unused variable `x`") .secondary_code("F841") .fix(Fix::unsafe_edit(Edit::deletion( @@ -2634,7 +2634,7 @@ def fibonacci(n): .noqa_offset(TextSize::from(94)) .build(), env.builder("undefined-name", Severity::Error, "Undefined name `a`") - .primary("undef.py", "1:3", "1:4", "F821") + .primary("undef.py", "1:3", "1:4", "") .secondary_code("F821") .noqa_offset(TextSize::from(3)) .build(), @@ -2727,7 +2727,7 @@ if call(foo let diagnostics = vec![ env.builder("unused-import", Severity::Error, "`os` imported but unused") - .primary("notebook.ipynb", "2:7", "2:9", "F401") + .primary("notebook.ipynb", "2:7", "2:9", "") .help("Remove unused import: `os`") .secondary_code("F401") .fix(Fix::safe_edit(Edit::range_deletion(TextRange::new( @@ -2741,7 +2741,7 @@ if call(foo Severity::Error, "`math` imported but unused", ) - .primary("notebook.ipynb", "4:7", "4:11", "F401") + .primary("notebook.ipynb", "4:7", "4:11", "") .help("Remove unused import: `math`") .secondary_code("F401") .fix(Fix::safe_edit(Edit::range_deletion(TextRange::new( @@ -2755,7 +2755,7 @@ if call(foo Severity::Error, "Local variable `x` is assigned to but never used", ) - .primary("notebook.ipynb", "10:4", "10:5", "F841") + .primary("notebook.ipynb", "10:4", "10:5", "") .help("Remove assignment to unused variable `x`") .secondary_code("F841") .fix(Fix::unsafe_edit(Edit::range_deletion(TextRange::new( diff --git a/crates/ruff_db/src/diagnostic/render/full.rs b/crates/ruff_db/src/diagnostic/render/full.rs index fd193e33d85e8..35f5e3e5a1829 100644 --- a/crates/ruff_db/src/diagnostic/render/full.rs +++ b/crates/ruff_db/src/diagnostic/render/full.rs @@ -13,7 +13,7 @@ mod tests { --> fib.py:1:8 | 1 | import os - | ^^ F401 + | ^^ | help: Remove unused import: `os` @@ -23,7 +23,7 @@ mod tests { 4 | def fibonacci(n): 5 | """Compute the nth number in the Fibonacci sequence.""" 6 | x = 1 - | ^ F841 + | ^ 7 | if n == 0: 8 | return 0 | @@ -33,7 +33,7 @@ mod tests { --> undef.py:1:4 | 1 | if a == 1: pass - | ^ F821 + | ^ | "#); } diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 56c8af2a792ec..d49cbe9659c52 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -74,10 +74,8 @@ where body, ); - let noqa_code = rule.noqa_code().to_string(); - let span = Span::from(file).with_range(range); - let annotation = Annotation::primary(span).message(&noqa_code); + let annotation = Annotation::primary(span); diagnostic.annotate(annotation); if let Some(suggestion) = suggestion { @@ -96,7 +94,7 @@ where diagnostic.set_noqa_offset(noqa_offset); } - diagnostic.set_secondary_code(SecondaryCode::new(noqa_code)); + diagnostic.set_secondary_code(SecondaryCode::new(rule.noqa_code().to_string())); diagnostic } From 12226e37a3d3cb3526df555f38c616787b538caf Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Mon, 21 Jul 2025 13:14:17 -0400 Subject: [PATCH 7/9] Diagnostic::suggestion -> first_help_text, update docs --- crates/ruff/src/cache.rs | 2 +- crates/ruff_db/src/diagnostic/mod.rs | 10 +++++++--- crates/ruff_db/src/diagnostic/render/json.rs | 2 +- crates/ruff_linter/src/message/text.rs | 2 +- crates/ruff_linter/src/test.rs | 2 +- crates/ruff_server/src/lint.rs | 2 +- crates/ruff_wasm/src/lib.rs | 2 +- 7 files changed, 13 insertions(+), 9 deletions(-) diff --git a/crates/ruff/src/cache.rs b/crates/ruff/src/cache.rs index 6eaef9382da09..d782d8c9ecdc0 100644 --- a/crates/ruff/src/cache.rs +++ b/crates/ruff/src/cache.rs @@ -454,7 +454,7 @@ impl LintCacheData { CacheMessage { rule, body: msg.body().to_string(), - suggestion: msg.suggestion().map(ToString::to_string), + suggestion: msg.first_help_text().map(ToString::to_string), range: msg.expect_range(), parent: msg.parent(), fix: msg.fix().cloned(), diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index cccaf12d784ef..7efd08bad309d 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -384,10 +384,14 @@ impl Diagnostic { self.primary_message() } - /// Returns the fix suggestion for the violation. - pub fn suggestion(&self) -> Option<&str> { + /// Returns the message of the first sub-diagnostic with a `Help` severity. + /// + /// Note that this is used as the fix title/suggestion for some of Ruff's output formats, but in + /// general this is not the guaranteed meaning of such a message. + pub fn first_help_text(&self) -> Option<&str> { self.sub_diagnostics() - .first() + .iter() + .find(|sub| matches!(sub.inner.severity, Severity::Help)) .map(|sub| sub.inner.message.as_str()) } diff --git a/crates/ruff_db/src/diagnostic/render/json.rs b/crates/ruff_db/src/diagnostic/render/json.rs index 98c35b1ddda0a..0e7a35b436769 100644 --- a/crates/ruff_db/src/diagnostic/render/json.rs +++ b/crates/ruff_db/src/diagnostic/render/json.rs @@ -87,7 +87,7 @@ pub(super) fn diagnostic_to_json<'a>( let fix = diagnostic.fix().map(|fix| JsonFix { applicability: fix.applicability(), - message: diagnostic.suggestion(), + message: diagnostic.first_help_text(), edits: ExpandedEdits { edits: fix.edits(), notebook_index, diff --git a/crates/ruff_linter/src/message/text.rs b/crates/ruff_linter/src/message/text.rs index 0578fd7731577..1cd6dd370dce3 100644 --- a/crates/ruff_linter/src/message/text.rs +++ b/crates/ruff_linter/src/message/text.rs @@ -186,7 +186,7 @@ pub(super) struct MessageCodeFrame<'a> { impl Display for MessageCodeFrame<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - let suggestion = self.message.suggestion(); + let suggestion = self.message.first_help_text(); let footers = if let Some(suggestion) = suggestion { vec![Level::Help.title(suggestion)] } else { diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index bab73eb86ec3f..cf63762b8a6f8 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -272,7 +272,7 @@ Either ensure you always emit a fix or change `Violation::FIX_AVAILABILITY` to e } assert!( - !(fixable && diagnostic.suggestion().is_none()), + !(fixable && diagnostic.first_help_text().is_none()), "Diagnostic emitted by {rule:?} is fixable but \ `Violation::fix_title` returns `None`" ); diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index d1dfb20267b6a..5c764bfa58fc7 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -238,7 +238,7 @@ fn to_lsp_diagnostic( let name = diagnostic.name(); let body = diagnostic.body().to_string(); let fix = diagnostic.fix(); - let suggestion = diagnostic.suggestion(); + let suggestion = diagnostic.first_help_text(); let code = diagnostic.secondary_code(); let fix = fix.and_then(|fix| fix.applies(Applicability::Unsafe).then_some(fix)); diff --git a/crates/ruff_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index bf378e0eb47a8..8fcb8d791b500 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -234,7 +234,7 @@ impl Workspace { start_location: source_code.line_column(msg.expect_range().start()).into(), end_location: source_code.line_column(msg.expect_range().end()).into(), fix: msg.fix().map(|fix| ExpandedFix { - message: msg.suggestion().map(ToString::to_string), + message: msg.first_help_text().map(ToString::to_string), edits: fix .edits() .iter() From 90497deddff10331a441fda74cb51fec7aadc04c Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 22 Jul 2025 08:41:28 -0400 Subject: [PATCH 8/9] add SubDiagnosticSeverity --- crates/ruff_db/src/diagnostic/mod.rs | 39 +++++++++++++++---- crates/ruff_db/src/diagnostic/render.rs | 31 ++++++++------- crates/ruff_db/src/diagnostic/render/azure.rs | 2 +- crates/ty/src/lib.rs | 2 +- crates/ty_ide/src/goto_declaration.rs | 3 +- crates/ty_ide/src/goto_definition.rs | 3 +- crates/ty_ide/src/goto_type_definition.rs | 3 +- crates/ty_project/src/lib.rs | 26 ++++++++----- crates/ty_project/src/metadata/options.rs | 36 ++++++++--------- crates/ty_python_semantic/src/types.rs | 8 ++-- .../ty_python_semantic/src/types/call/bind.rs | 28 ++++++++----- .../ty_python_semantic/src/types/context.rs | 4 +- .../src/types/diagnostic.rs | 14 +++---- .../src/util/diagnostics.rs | 6 +-- .../ty_server/src/server/api/diagnostics.rs | 1 - crates/ty_wasm/src/lib.rs | 1 - 16 files changed, 126 insertions(+), 81 deletions(-) diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 7efd08bad309d..63707302eb25e 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -122,14 +122,14 @@ impl Diagnostic { /// directly. If callers want or need to avoid cloning the diagnostic /// message, then they can also pass a `DiagnosticMessage` directly. pub fn info<'a>(&mut self, message: impl IntoDiagnosticMessage + 'a) { - self.sub(SubDiagnostic::new(Severity::Info, message)); + self.sub(SubDiagnostic::new(SubDiagnosticSeverity::Info, message)); } /// Adds a "help" sub-diagnostic with the given message. /// /// See the closely related [`Diagnostic::info`] method for more details. pub fn help<'a>(&mut self, message: impl IntoDiagnosticMessage + 'a) { - self.sub(SubDiagnostic::new(Severity::Help, message)); + self.sub(SubDiagnostic::new(SubDiagnosticSeverity::Help, message)); } /// Adds a "sub" diagnostic to this diagnostic. @@ -391,7 +391,7 @@ impl Diagnostic { pub fn first_help_text(&self) -> Option<&str> { self.sub_diagnostics() .iter() - .find(|sub| matches!(sub.inner.severity, Severity::Help)) + .find(|sub| matches!(sub.inner.severity, SubDiagnosticSeverity::Help)) .map(|sub| sub.inner.message.as_str()) } @@ -578,7 +578,10 @@ impl SubDiagnostic { /// Callers can pass anything that implements `std::fmt::Display` /// directly. If callers want or need to avoid cloning the diagnostic /// message, then they can also pass a `DiagnosticMessage` directly. - pub fn new<'a>(severity: Severity, message: impl IntoDiagnosticMessage + 'a) -> SubDiagnostic { + pub fn new<'a>( + severity: SubDiagnosticSeverity, + message: impl IntoDiagnosticMessage + 'a, + ) -> SubDiagnostic { let inner = Box::new(SubDiagnosticInner { severity, message: message.into_diagnostic_message(), @@ -656,7 +659,7 @@ impl SubDiagnostic { #[derive(Debug, Clone, Eq, PartialEq, get_size2::GetSize)] struct SubDiagnosticInner { - severity: Severity, + severity: SubDiagnosticSeverity, message: DiagnosticMessage, annotations: Vec, } @@ -1155,7 +1158,6 @@ impl From for Span { #[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd, get_size2::GetSize)] pub enum Severity { - Help, Info, Warning, Error, @@ -1165,7 +1167,6 @@ pub enum Severity { impl Severity { fn to_annotate(self) -> AnnotateLevel { match self { - Self::Help => AnnotateLevel::Help, Severity::Info => AnnotateLevel::Info, Severity::Warning => AnnotateLevel::Warning, Severity::Error => AnnotateLevel::Error, @@ -1185,6 +1186,30 @@ impl Severity { } } +/// Like [`Severity`] but exclusively for sub-diagnostics. +/// +/// This supports an additional `Help` severity that may not be needed in main diagnostics. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd, get_size2::GetSize)] +pub enum SubDiagnosticSeverity { + Help, + Info, + Warning, + Error, + Fatal, +} + +impl SubDiagnosticSeverity { + fn to_annotate(self) -> AnnotateLevel { + match self { + SubDiagnosticSeverity::Help => AnnotateLevel::Help, + SubDiagnosticSeverity::Info => AnnotateLevel::Info, + SubDiagnosticSeverity::Warning => AnnotateLevel::Warning, + SubDiagnosticSeverity::Error => AnnotateLevel::Error, + SubDiagnosticSeverity::Fatal => AnnotateLevel::Error, + } + } +} + /// Configuration for rendering diagnostics. #[derive(Clone, Debug)] pub struct DisplayDiagnosticConfig { diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 8fb8bb9347312..70bb05f385b8a 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -113,7 +113,6 @@ impl std::fmt::Display for DisplayDiagnostics<'_> { for diag in self.diagnostics { let (severity, severity_style) = match diag.severity() { - Severity::Help => ("help", stylesheet.info), Severity::Info => ("info", stylesheet.info), Severity::Warning => ("warning", stylesheet.warning), Severity::Error => ("error", stylesheet.error), @@ -258,7 +257,7 @@ impl<'a> Resolved<'a> { /// both.) #[derive(Debug)] struct ResolvedDiagnostic<'a> { - severity: Severity, + level: AnnotateLevel, id: Option, message: String, annotations: Vec>, @@ -283,7 +282,7 @@ impl<'a> ResolvedDiagnostic<'a> { let id = Some(diag.inner.id.to_string()); let message = diag.inner.message.as_str().to_string(); ResolvedDiagnostic { - severity: diag.inner.severity, + level: diag.inner.severity.to_annotate(), id, message, annotations, @@ -306,7 +305,7 @@ impl<'a> ResolvedDiagnostic<'a> { }) .collect(); ResolvedDiagnostic { - severity: diag.inner.severity, + level: diag.inner.severity.to_annotate(), id: None, message: diag.inner.message.as_str().to_string(), annotations, @@ -373,7 +372,7 @@ impl<'a> ResolvedDiagnostic<'a> { snippets_by_input .sort_by(|snips1, snips2| snips1.has_primary.cmp(&snips2.has_primary).reverse()); RenderableDiagnostic { - severity: self.severity, + level: self.level, id: self.id.as_deref(), message: &self.message, snippets_by_input, @@ -461,7 +460,7 @@ struct Renderable<'r> { #[derive(Debug)] struct RenderableDiagnostic<'r> { /// The severity of the diagnostic. - severity: Severity, + level: AnnotateLevel, /// The ID of the diagnostic. The ID can usually be used on the CLI or in a /// config file to change the severity of a lint. /// @@ -480,7 +479,6 @@ struct RenderableDiagnostic<'r> { impl RenderableDiagnostic<'_> { /// Convert this to an "annotate" snippet. fn to_annotate(&self) -> AnnotateMessage<'_> { - let level = self.severity.to_annotate(); let snippets = self.snippets_by_input.iter().flat_map(|snippets| { let path = snippets.path; snippets @@ -488,7 +486,7 @@ impl RenderableDiagnostic<'_> { .iter() .map(|snippet| snippet.to_annotate(path)) }); - let mut message = level.title(self.message); + let mut message = self.level.title(self.message); if let Some(id) = self.id { message = message.id(id); } @@ -868,6 +866,7 @@ mod tests { use crate::diagnostic::{ Annotation, DiagnosticId, IntoDiagnosticMessage, SecondaryCode, Severity, Span, + SubDiagnosticSeverity, }; use crate::files::system_path_to_file; use crate::system::{DbWithWritableSystem, SystemPath}; @@ -1552,7 +1551,7 @@ watermelon let mut diag = env.err().primary("animals", "3", "3", "").build(); diag.sub( - env.sub_builder(Severity::Info, "this is a helpful note") + env.sub_builder(SubDiagnosticSeverity::Info, "this is a helpful note") .build(), ); insta::assert_snapshot!( @@ -1581,15 +1580,15 @@ watermelon let mut diag = env.err().primary("animals", "3", "3", "").build(); diag.sub( - env.sub_builder(Severity::Info, "this is a helpful note") + env.sub_builder(SubDiagnosticSeverity::Info, "this is a helpful note") .build(), ); diag.sub( - env.sub_builder(Severity::Info, "another helpful note") + env.sub_builder(SubDiagnosticSeverity::Info, "another helpful note") .build(), ); diag.sub( - env.sub_builder(Severity::Info, "and another helpful note") + env.sub_builder(SubDiagnosticSeverity::Info, "and another helpful note") .build(), ); insta::assert_snapshot!( @@ -2374,7 +2373,7 @@ watermelon /// sub-diagnostic with "error" severity and canned values for /// its identifier and message. fn sub_warn(&mut self) -> SubDiagnosticBuilder<'_> { - self.sub_builder(Severity::Warning, "sub-diagnostic message") + self.sub_builder(SubDiagnosticSeverity::Warning, "sub-diagnostic message") } /// Returns a builder for tersely constructing diagnostics. @@ -2395,7 +2394,11 @@ watermelon } /// Returns a builder for tersely constructing sub-diagnostics. - fn sub_builder(&mut self, severity: Severity, message: &str) -> SubDiagnosticBuilder<'_> { + fn sub_builder( + &mut self, + severity: SubDiagnosticSeverity, + message: &str, + ) -> SubDiagnosticBuilder<'_> { let subdiag = SubDiagnostic::new(severity, message); SubDiagnosticBuilder { env: self, subdiag } } diff --git a/crates/ruff_db/src/diagnostic/render/azure.rs b/crates/ruff_db/src/diagnostic/render/azure.rs index 318fdab4e85e9..d607e08d861a7 100644 --- a/crates/ruff_db/src/diagnostic/render/azure.rs +++ b/crates/ruff_db/src/diagnostic/render/azure.rs @@ -22,7 +22,7 @@ impl AzureRenderer<'_> { ) -> std::fmt::Result { for diag in diagnostics { let severity = match diag.severity() { - Severity::Help | Severity::Info | Severity::Warning => "warning", + Severity::Info | Severity::Warning => "warning", Severity::Error | Severity::Fatal => "error", }; write!(f, "##vso[task.logissue type={severity};")?; diff --git a/crates/ty/src/lib.rs b/crates/ty/src/lib.rs index 9f4c6190c1380..8a0c5b9fcd590 100644 --- a/crates/ty/src/lib.rs +++ b/crates/ty/src/lib.rs @@ -349,7 +349,7 @@ impl MainLoop { if self.watcher.is_none() { return Ok(match max_severity { - Severity::Help | Severity::Info => ExitStatus::Success, + Severity::Info => ExitStatus::Success, Severity::Warning => { if terminal_settings.error_on_warning { ExitStatus::Failure diff --git a/crates/ty_ide/src/goto_declaration.rs b/crates/ty_ide/src/goto_declaration.rs index 5f5f358153a19..9f916ccbc420a 100644 --- a/crates/ty_ide/src/goto_declaration.rs +++ b/crates/ty_ide/src/goto_declaration.rs @@ -32,6 +32,7 @@ mod tests { use insta::assert_snapshot; use ruff_db::diagnostic::{ Annotation, Diagnostic, DiagnosticId, LintName, Severity, Span, SubDiagnostic, + SubDiagnosticSeverity, }; use ruff_db::files::FileRange; use ruff_text_size::Ranged; @@ -1349,7 +1350,7 @@ class MyClass: impl IntoDiagnostic for GotoDeclarationDiagnostic { fn into_diagnostic(self) -> Diagnostic { - let mut source = SubDiagnostic::new(Severity::Info, "Source"); + let mut source = SubDiagnostic::new(SubDiagnosticSeverity::Info, "Source"); source.annotate(Annotation::primary( Span::from(self.source.file()).with_range(self.source.range()), )); diff --git a/crates/ty_ide/src/goto_definition.rs b/crates/ty_ide/src/goto_definition.rs index 4c4d090fb3248..49b462e13a2a0 100644 --- a/crates/ty_ide/src/goto_definition.rs +++ b/crates/ty_ide/src/goto_definition.rs @@ -37,6 +37,7 @@ mod test { use insta::assert_snapshot; use ruff_db::diagnostic::{ Annotation, Diagnostic, DiagnosticId, LintName, Severity, Span, SubDiagnostic, + SubDiagnosticSeverity, }; use ruff_db::files::FileRange; use ruff_text_size::Ranged; @@ -575,7 +576,7 @@ class MyClass: ... impl IntoDiagnostic for GotoDefinitionDiagnostic { fn into_diagnostic(self) -> Diagnostic { - let mut source = SubDiagnostic::new(Severity::Info, "Source"); + let mut source = SubDiagnostic::new(SubDiagnosticSeverity::Info, "Source"); source.annotate(Annotation::primary( Span::from(self.source.file()).with_range(self.source.range()), )); diff --git a/crates/ty_ide/src/goto_type_definition.rs b/crates/ty_ide/src/goto_type_definition.rs index f8348a73ee08c..b89bc6c21294a 100644 --- a/crates/ty_ide/src/goto_type_definition.rs +++ b/crates/ty_ide/src/goto_type_definition.rs @@ -33,6 +33,7 @@ mod tests { use insta::assert_snapshot; use ruff_db::diagnostic::{ Annotation, Diagnostic, DiagnosticId, LintName, Severity, Span, SubDiagnostic, + SubDiagnosticSeverity, }; use ruff_db::files::FileRange; use ruff_text_size::Ranged; @@ -640,7 +641,7 @@ f(**kwargs) impl IntoDiagnostic for GotoTypeDefinitionDiagnostic { fn into_diagnostic(self) -> Diagnostic { - let mut source = SubDiagnostic::new(Severity::Info, "Source"); + let mut source = SubDiagnostic::new(SubDiagnosticSeverity::Info, "Source"); source.annotate(Annotation::primary( Span::from(self.source.file()).with_range(self.source.range()), )); diff --git a/crates/ty_project/src/lib.rs b/crates/ty_project/src/lib.rs index be45869ab2188..84a9e40228ded 100644 --- a/crates/ty_project/src/lib.rs +++ b/crates/ty_project/src/lib.rs @@ -5,7 +5,9 @@ pub use db::{ChangeResult, CheckMode, Db, ProjectDatabase, SalsaMemoryDump}; use files::{Index, Indexed, IndexedFiles}; use metadata::settings::Settings; pub use metadata::{ProjectMetadata, ProjectMetadataError}; -use ruff_db::diagnostic::{Annotation, Diagnostic, DiagnosticId, Severity, Span, SubDiagnostic}; +use ruff_db::diagnostic::{ + Annotation, Diagnostic, DiagnosticId, Severity, Span, SubDiagnostic, SubDiagnosticSeverity, +}; use ruff_db::files::{File, FileRootKind}; use ruff_db::parsed::parsed_module; use ruff_db::source::{SourceTextError, source_text}; @@ -674,14 +676,17 @@ where let mut diagnostic = Diagnostic::new(DiagnosticId::Panic, Severity::Fatal, message); diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "This indicates a bug in ty.", )); let report_message = "If you could open an issue at https://github.com/astral-sh/ty/issues/new?title=%5Bpanic%5D, we'd be very appreciative!"; - diagnostic.sub(SubDiagnostic::new(Severity::Info, report_message)); diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, + report_message, + )); + diagnostic.sub(SubDiagnostic::new( + SubDiagnosticSeverity::Info, format!( "Platform: {os} {arch}", os = std::env::consts::OS, @@ -690,13 +695,13 @@ where )); if let Some(version) = ruff_db::program_version() { diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format!("Version: {version}"), )); } diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format!( "Args: {args:?}", args = std::env::args().collect::>() @@ -707,13 +712,13 @@ where match backtrace.status() { BacktraceStatus::Disabled => { diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "run with `RUST_BACKTRACE=1` environment variable to show the full backtrace information", )); } BacktraceStatus::Captured => { diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format!("Backtrace:\n{backtrace}"), )); } @@ -723,7 +728,10 @@ where if let Some(backtrace) = error.salsa_backtrace { salsa::attach(db, || { - diagnostic.sub(SubDiagnostic::new(Severity::Info, backtrace.to_string())); + diagnostic.sub(SubDiagnostic::new( + SubDiagnosticSeverity::Info, + backtrace.to_string(), + )); }); } diff --git a/crates/ty_project/src/metadata/options.rs b/crates/ty_project/src/metadata/options.rs index e627e01718be1..3d01e6e2a9c79 100644 --- a/crates/ty_project/src/metadata/options.rs +++ b/crates/ty_project/src/metadata/options.rs @@ -12,7 +12,7 @@ use ordermap::OrderMap; use ruff_db::RustDoc; use ruff_db::diagnostic::{ Annotation, Diagnostic, DiagnosticFormat, DiagnosticId, DisplayDiagnosticConfig, Severity, - Span, SubDiagnostic, + Span, SubDiagnostic, SubDiagnosticSeverity, }; use ruff_db::files::system_path_to_file; use ruff_db::system::{System, SystemPath, SystemPathBuf}; @@ -318,7 +318,7 @@ impl Options { if self.environment.or_default().root.is_some() { diagnostic = diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "The `src.root` setting was ignored in favor of the `environment.root` setting", )); } @@ -811,7 +811,7 @@ fn build_include_filter( Severity::Warning, ) .sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "Remove the `include` option to match all files or add a pattern to match specific files", )); @@ -853,13 +853,13 @@ fn build_include_filter( )) } else { diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format!("The pattern is defined in the `{}` option in your configuration file", context.include_name()), )) } } ValueSource::Cli => diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "The pattern was specified on the CLI", )), ValueSource::PythonVSCodeExtension => unreachable!("Can't configure includes from the Python VSCode extension"), @@ -883,7 +883,7 @@ fn build_include_filter( Severity::Error, ); Box::new(diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "Please open an issue on the ty repository and share the patterns that caused the error.", ))) }) @@ -936,13 +936,13 @@ fn build_exclude_filter( )) } else { diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format!("The pattern is defined in the `{}` option in your configuration file", context.exclude_name()), )) } } ValueSource::Cli => diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "The pattern was specified on the CLI", )), ValueSource::PythonVSCodeExtension => unreachable!( @@ -960,7 +960,7 @@ fn build_exclude_filter( Severity::Error, ); Box::new(diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "Please open an issue on the ty repository and share the patterns that caused the error.", ))) }) @@ -1197,26 +1197,26 @@ impl RangedValue { diagnostic = if self.rules.is_none() { diagnostic = diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "It has no `rules` table", )); diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "Add a `[overrides.rules]` table...", )) } else { diagnostic = diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "The rules table is empty", )); diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "Add a rule to `[overrides.rules]` to override specific rules...", )) }; diagnostic = diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "or remove the `[[overrides]]` section if there's nothing to override", )); @@ -1251,23 +1251,23 @@ impl RangedValue { diagnostic = if self.exclude.is_none() { diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "It has no `include` or `exclude` option restricting the files", )) } else { diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "It has no `include` option and `exclude` is empty", )) }; diagnostic = diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "Restrict the files by adding a pattern to `include` or `exclude`...", )); diagnostic = diagnostic.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "or remove the `[[overrides]]` section and merge the configuration into the root `[rules]` table if the configuration should apply to all files", )); diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index b5b7933a77ff0..8a01a5440b284 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -11,7 +11,7 @@ use diagnostic::{ INVALID_CONTEXT_MANAGER, INVALID_SUPER_ARGUMENT, NOT_ITERABLE, POSSIBLY_UNBOUND_IMPLICIT_CALL, UNAVAILABLE_IMPLICIT_SUPER_ARGUMENTS, }; -use ruff_db::diagnostic::{Annotation, Diagnostic, Severity, Span, SubDiagnostic}; +use ruff_db::diagnostic::{Annotation, Diagnostic, Span, SubDiagnostic, SubDiagnosticSeverity}; use ruff_db::files::File; use ruff_python_ast::name::Name; use ruff_python_ast::{self as ast, AnyNodeRef}; @@ -7077,7 +7077,7 @@ impl<'db> BoolError<'db> { not_boolable_type.display(context.db()) )); let mut sub = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "`__bool__` methods must only have a `self` parameter", ); if let Some((func_span, parameter_span)) = not_boolable_type @@ -7102,7 +7102,7 @@ impl<'db> BoolError<'db> { not_boolable = not_boolable_type.display(context.db()), )); let mut sub = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format_args!( "`{return_type}` is not assignable to `bool`", return_type = return_type.display(context.db()), @@ -7128,7 +7128,7 @@ impl<'db> BoolError<'db> { not_boolable_type.display(context.db()) )); let sub = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format_args!( "`__bool__` on `{}` must be callable", not_boolable_type.display(context.db()) diff --git a/crates/ty_python_semantic/src/types/call/bind.rs b/crates/ty_python_semantic/src/types/call/bind.rs index 2f7149b7ebdd1..1cb121e3ed5ed 100644 --- a/crates/ty_python_semantic/src/types/call/bind.rs +++ b/crates/ty_python_semantic/src/types/call/bind.rs @@ -30,7 +30,7 @@ use crate::types::{ MethodWrapperKind, PropertyInstanceType, SpecialFormType, TypeMapping, UnionType, WrapperDescriptorKind, enums, ide_support, todo_type, }; -use ruff_db::diagnostic::{Annotation, Diagnostic, Severity, SubDiagnostic}; +use ruff_db::diagnostic::{Annotation, Diagnostic, SubDiagnostic, SubDiagnosticSeverity}; use ruff_python_ast as ast; /// Binding information for a possible union of callables. At a call site, the arguments must be @@ -1668,8 +1668,10 @@ impl<'db> CallableBinding<'db> { .first() .and_then(|overload| overload.spans(context.db())) { - let mut sub = - SubDiagnostic::new(Severity::Info, "First overload defined here"); + let mut sub = SubDiagnostic::new( + SubDiagnosticSeverity::Info, + "First overload defined here", + ); sub.annotate(Annotation::primary(spans.signature)); diag.sub(sub); } @@ -1696,7 +1698,7 @@ impl<'db> CallableBinding<'db> { implementation.and_then(|function| function.spans(context.db())) { let mut sub = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "Overload implementation defined here", ); sub.annotate(Annotation::primary(spans.signature)); @@ -2570,8 +2572,10 @@ impl<'db> BindingError<'db> { overload.parameter_span(context.db(), Some(parameter.index)) }) { - let mut sub = - SubDiagnostic::new(Severity::Info, "Matching overload defined here"); + let mut sub = SubDiagnostic::new( + SubDiagnosticSeverity::Info, + "Matching overload defined here", + ); sub.annotate(Annotation::primary(name_span)); sub.annotate( Annotation::secondary(parameter_span) @@ -2607,7 +2611,8 @@ impl<'db> BindingError<'db> { } else if let Some((name_span, parameter_span)) = callable_ty.parameter_span(context.db(), Some(parameter.index)) { - let mut sub = SubDiagnostic::new(Severity::Info, "Function defined here"); + let mut sub = + SubDiagnostic::new(SubDiagnosticSeverity::Info, "Function defined here"); sub.annotate(Annotation::primary(name_span)); sub.annotate( Annotation::secondary(parameter_span).message("Parameter declared here"), @@ -2733,7 +2738,10 @@ impl<'db> BindingError<'db> { let module = parsed_module(context.db(), typevar_definition.file(context.db())) .load(context.db()); let typevar_range = typevar_definition.full_range(context.db(), &module); - let mut sub = SubDiagnostic::new(Severity::Info, "Type variable defined here"); + let mut sub = SubDiagnostic::new( + SubDiagnosticSeverity::Info, + "Type variable defined here", + ); sub.annotate(Annotation::primary(typevar_range.into())); diag.sub(sub); } @@ -2801,7 +2809,7 @@ impl UnionDiagnostic<'_, '_> { /// diagnostic. fn add_union_context(&self, db: &'_ dyn Db, diag: &mut Diagnostic) { let sub = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format_args!( "Union variant `{callable_ty}` is incompatible with this call site", callable_ty = self.binding.callable_type.display(db), @@ -2810,7 +2818,7 @@ impl UnionDiagnostic<'_, '_> { diag.sub(sub); let sub = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format_args!( "Attempted to call union type `{}`", self.callable_type.display(db) diff --git a/crates/ty_python_semantic/src/types/context.rs b/crates/ty_python_semantic/src/types/context.rs index b7205b69d77cb..bbf6b59b66aaa 100644 --- a/crates/ty_python_semantic/src/types/context.rs +++ b/crates/ty_python_semantic/src/types/context.rs @@ -1,7 +1,7 @@ use std::fmt; use drop_bomb::DebugDropBomb; -use ruff_db::diagnostic::{DiagnosticTag, SubDiagnostic}; +use ruff_db::diagnostic::{DiagnosticTag, SubDiagnostic, SubDiagnosticSeverity}; use ruff_db::parsed::ParsedModuleRef; use ruff_db::{ diagnostic::{Annotation, Diagnostic, DiagnosticId, IntoDiagnosticMessage, Severity, Span}, @@ -330,7 +330,7 @@ impl Drop for LintDiagnosticGuard<'_, '_> { let mut diag = self.diag.take().unwrap(); diag.sub(SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, match self.source { LintSource::Default => format!("rule `{}` is enabled by default", diag.id()), LintSource::Cli => format!("rule `{}` was selected on the command line", diag.id()), diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index d1d499ca8fcd0..ecb4bc4aca19d 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -20,7 +20,7 @@ use crate::types::{SpecialFormType, Type, protocol_class::ProtocolClassLiteral}; use crate::util::diagnostics::format_enumeration; use crate::{Db, FxIndexMap, Module, ModuleName, Program, declare_lint}; use itertools::Itertools; -use ruff_db::diagnostic::{Annotation, Diagnostic, Severity, SubDiagnostic}; +use ruff_db::diagnostic::{Annotation, Diagnostic, SubDiagnostic, SubDiagnosticSeverity}; use ruff_python_ast::{self as ast, AnyNodeRef}; use ruff_text_size::{Ranged, TextRange}; use rustc_hash::FxHashSet; @@ -1929,7 +1929,7 @@ pub(super) fn report_implicit_return_type( )); let mut sub_diagnostic = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "Only classes that directly inherit from `typing.Protocol` \ or `typing_extensions.Protocol` are considered protocol classes", ); @@ -2037,7 +2037,7 @@ pub(crate) fn report_instance_layout_conflict( )); let mut subdiagnostic = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, "Two classes cannot coexist in a class's MRO if their instances \ have incompatible memory layouts", ); @@ -2230,7 +2230,7 @@ pub(crate) fn report_bad_argument_to_get_protocol_members( diagnostic.info("Only protocol classes can be passed to `get_protocol_members`"); let mut class_def_diagnostic = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format_args!( "`{}` is declared here, but it is not a protocol class:", class.name(db) @@ -2292,7 +2292,7 @@ pub(crate) fn report_runtime_check_against_non_runtime_checkable_protocol( diagnostic.set_primary_message("This call will raise `TypeError` at runtime"); let mut class_def_diagnostic = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format_args!( "`{class_name}` is declared as a protocol class, \ but it is not declared as runtime-checkable" @@ -2326,7 +2326,7 @@ pub(crate) fn report_attempted_protocol_instantiation( diagnostic.set_primary_message("This call will raise `TypeError` at runtime"); let mut class_def_diagnostic = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format_args!("Protocol classes cannot be instantiated"), ); class_def_diagnostic.annotate( @@ -2360,7 +2360,7 @@ pub(crate) fn report_duplicate_bases( builder.into_diagnostic(format_args!("Duplicate base class `{duplicate_name}`",)); let mut sub_diagnostic = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format_args!( "The definition of class `{}` will raise `TypeError` at runtime", class.name(db) diff --git a/crates/ty_python_semantic/src/util/diagnostics.rs b/crates/ty_python_semantic/src/util/diagnostics.rs index 5639f4d511b71..82ce6b1c3a8f0 100644 --- a/crates/ty_python_semantic/src/util/diagnostics.rs +++ b/crates/ty_python_semantic/src/util/diagnostics.rs @@ -1,5 +1,5 @@ use crate::{Db, Program, PythonVersionWithSource}; -use ruff_db::diagnostic::{Annotation, Diagnostic, Severity, SubDiagnostic}; +use ruff_db::diagnostic::{Annotation, Diagnostic, SubDiagnostic, SubDiagnosticSeverity}; use std::fmt::Write; /// Add a subdiagnostic to `diagnostic` that explains why a certain Python version was inferred. @@ -23,7 +23,7 @@ pub fn add_inferred_python_version_hint_to_diagnostic( crate::PythonVersionSource::ConfigFile(source) => { if let Some(span) = source.span(db) { let mut sub_diagnostic = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format_args!("Python {version} was assumed when {action}"), ); sub_diagnostic.annotate(Annotation::primary(span).message(format_args!( @@ -39,7 +39,7 @@ pub fn add_inferred_python_version_hint_to_diagnostic( crate::PythonVersionSource::PyvenvCfgFile(source) => { if let Some(span) = source.span(db) { let mut sub_diagnostic = SubDiagnostic::new( - Severity::Info, + SubDiagnosticSeverity::Info, format_args!( "Python {version} was assumed when {action} because of your virtual environment" ), diff --git a/crates/ty_server/src/server/api/diagnostics.rs b/crates/ty_server/src/server/api/diagnostics.rs index 8d0cb33ed4a2d..11e39796adb9e 100644 --- a/crates/ty_server/src/server/api/diagnostics.rs +++ b/crates/ty_server/src/server/api/diagnostics.rs @@ -263,7 +263,6 @@ pub(super) fn to_lsp_diagnostic( }; let severity = match diagnostic.severity() { - Severity::Help => DiagnosticSeverity::HINT, Severity::Info => DiagnosticSeverity::INFORMATION, Severity::Warning => DiagnosticSeverity::WARNING, Severity::Error | Severity::Fatal => DiagnosticSeverity::ERROR, diff --git a/crates/ty_wasm/src/lib.rs b/crates/ty_wasm/src/lib.rs index 632a7482cfb94..faed8bb7f3402 100644 --- a/crates/ty_wasm/src/lib.rs +++ b/crates/ty_wasm/src/lib.rs @@ -736,7 +736,6 @@ pub enum Severity { impl From for Severity { fn from(value: diagnostic::Severity) -> Self { match value { - diagnostic::Severity::Help => Self::Help, diagnostic::Severity::Info => Self::Info, diagnostic::Severity::Warning => Self::Warning, diagnostic::Severity::Error => Self::Error, From 10676cbedbcb587fe44b963dced0457814d44370 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 22 Jul 2025 09:20:33 -0400 Subject: [PATCH 9/9] delete leftover Help variant in ty_wasm --- crates/ty_wasm/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/ty_wasm/src/lib.rs b/crates/ty_wasm/src/lib.rs index faed8bb7f3402..de101ae3c097d 100644 --- a/crates/ty_wasm/src/lib.rs +++ b/crates/ty_wasm/src/lib.rs @@ -726,7 +726,6 @@ impl Position { #[wasm_bindgen] #[derive(Copy, Clone, Hash, PartialEq, Eq)] pub enum Severity { - Help, Info, Warning, Error,