From 5e7fc9a4e1426f0cad756addb8efc63e51ddfeae Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Wed, 24 Dec 2025 10:28:24 -0500 Subject: [PATCH] Render the entire diagnostic message in all output formats (#22164) Summary -- This PR fixes https://github.com/astral-sh/ty/issues/2186 by replacing uses of `Diagnostic::body` with [`Diagnostic::concise_message`][d]. The initial report was only about ty's GitHub and GitLab output formats, but I think it makes sense to prefer `concise_message` in the other output formats too. Ruff currently only sets the primary message on its diagnostics, which is why this has no effect on Ruff, and ty currently only supports the GitHub and GitLab formats that used `body`, but removing `body` should help to avoid this problem as Ruff starts to use more diagnostic features or ty gets new output formats. Test Plan -- Updated existing GitLab and GitHub CLI tests to have `reveal_type` diagnostics [d]: https://github.com/astral-sh/ruff/blob/395bf106ab/crates/ruff_db/src/diagnostic/mod.rs#L185 [t]: https://github.com/astral-sh/ruff/blob/395bf106ab/crates/ruff/tests/cli/lint.rs#L3509 --- crates/ruff_db/src/diagnostic/mod.rs | 26 ++++++++++++++----- crates/ruff_db/src/diagnostic/render/azure.rs | 2 +- .../ruff_db/src/diagnostic/render/github.rs | 2 +- .../ruff_db/src/diagnostic/render/gitlab.rs | 2 +- crates/ruff_db/src/diagnostic/render/json.rs | 8 +++--- crates/ruff_db/src/diagnostic/render/junit.rs | 6 ++--- .../ruff_db/src/diagnostic/render/pylint.rs | 2 +- .../ruff_db/src/diagnostic/render/rdjson.rs | 6 ++--- crates/ruff_linter/src/message/grouped.rs | 6 ++--- crates/ruff_linter/src/message/sarif.rs | 2 +- crates/ruff_server/src/lint.rs | 2 +- crates/ruff_wasm/src/lib.rs | 2 +- crates/ty/tests/cli/main.rs | 24 +++++++++++++++++ crates/ty_test/src/matcher.rs | 2 +- 14 files changed, 65 insertions(+), 27 deletions(-) diff --git a/crates/ruff_db/src/diagnostic/mod.rs b/crates/ruff_db/src/diagnostic/mod.rs index 328bf70fd6..ece31af555 100644 --- a/crates/ruff_db/src/diagnostic/mod.rs +++ b/crates/ruff_db/src/diagnostic/mod.rs @@ -1,4 +1,4 @@ -use std::{fmt::Formatter, path::Path, sync::Arc}; +use std::{borrow::Cow, fmt::Formatter, path::Path, sync::Arc}; use ruff_diagnostics::{Applicability, Fix}; use ruff_source_file::{LineColumn, SourceCode, SourceFile}; @@ -411,11 +411,6 @@ impl Diagnostic { self.id().is_invalid_syntax() } - /// Returns the message body to display to the user. - pub fn body(&self) -> &str { - self.primary_message() - } - /// 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 @@ -1492,6 +1487,15 @@ pub enum ConciseMessage<'a> { Custom(&'a str), } +impl<'a> ConciseMessage<'a> { + pub fn to_str(&self) -> Cow<'a, str> { + match self { + ConciseMessage::MainDiagnostic(s) | ConciseMessage::Custom(s) => Cow::Borrowed(s), + ConciseMessage::Both { .. } => Cow::Owned(self.to_string()), + } + } +} + impl std::fmt::Display for ConciseMessage<'_> { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { match *self { @@ -1508,6 +1512,16 @@ impl std::fmt::Display for ConciseMessage<'_> { } } +#[cfg(feature = "serde")] +impl serde::Serialize for ConciseMessage<'_> { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.collect_str(self) + } +} + /// A diagnostic message string. /// /// This is, for all intents and purposes, equivalent to a `Box`. diff --git a/crates/ruff_db/src/diagnostic/render/azure.rs b/crates/ruff_db/src/diagnostic/render/azure.rs index 78d8c89dfb..99476ed7ff 100644 --- a/crates/ruff_db/src/diagnostic/render/azure.rs +++ b/crates/ruff_db/src/diagnostic/render/azure.rs @@ -52,7 +52,7 @@ impl AzureRenderer<'_> { f, "code={code};]{body}", code = diag.secondary_code_or_id(), - body = diag.body(), + body = diag.concise_message(), )?; } diff --git a/crates/ruff_db/src/diagnostic/render/github.rs b/crates/ruff_db/src/diagnostic/render/github.rs index 161f371f8a..831e62c245 100644 --- a/crates/ruff_db/src/diagnostic/render/github.rs +++ b/crates/ruff_db/src/diagnostic/render/github.rs @@ -87,7 +87,7 @@ impl<'a> GithubRenderer<'a> { write!(f, "{id}:", id = diagnostic.id())?; } - writeln!(f, " {}", diagnostic.body())?; + writeln!(f, " {}", diagnostic.concise_message())?; } Ok(()) diff --git a/crates/ruff_db/src/diagnostic/render/gitlab.rs b/crates/ruff_db/src/diagnostic/render/gitlab.rs index 0d25da4dc6..4d5a2ec82e 100644 --- a/crates/ruff_db/src/diagnostic/render/gitlab.rs +++ b/crates/ruff_db/src/diagnostic/render/gitlab.rs @@ -98,7 +98,7 @@ impl Serialize for SerializedMessages<'_> { } fingerprints.insert(message_fingerprint); - let description = diagnostic.body(); + let description = diagnostic.concise_message(); let check_name = diagnostic.secondary_code_or_id(); let severity = match diagnostic.severity() { Severity::Info => "info", diff --git a/crates/ruff_db/src/diagnostic/render/json.rs b/crates/ruff_db/src/diagnostic/render/json.rs index 2a82ec39db..944ce49b3a 100644 --- a/crates/ruff_db/src/diagnostic/render/json.rs +++ b/crates/ruff_db/src/diagnostic/render/json.rs @@ -6,7 +6,7 @@ use ruff_notebook::NotebookIndex; use ruff_source_file::{LineColumn, OneIndexed}; use ruff_text_size::Ranged; -use crate::diagnostic::{Diagnostic, DiagnosticSource, DisplayDiagnosticConfig}; +use crate::diagnostic::{ConciseMessage, Diagnostic, DiagnosticSource, DisplayDiagnosticConfig}; use super::FileResolver; @@ -101,7 +101,7 @@ pub(super) fn diagnostic_to_json<'a>( JsonDiagnostic { code: diagnostic.secondary_code_or_id(), url: diagnostic.documentation_url(), - message: diagnostic.body(), + message: diagnostic.concise_message(), fix, cell: notebook_cell_index, location: start_location.map(JsonLocation::from), @@ -113,7 +113,7 @@ pub(super) fn diagnostic_to_json<'a>( JsonDiagnostic { code: diagnostic.secondary_code_or_id(), url: diagnostic.documentation_url(), - message: diagnostic.body(), + message: diagnostic.concise_message(), fix, cell: notebook_cell_index, location: Some(start_location.unwrap_or_default().into()), @@ -226,7 +226,7 @@ pub(crate) struct JsonDiagnostic<'a> { filename: Option<&'a str>, fix: Option>, location: Option, - message: &'a str, + message: ConciseMessage<'a>, noqa_row: Option, url: Option<&'a str>, } diff --git a/crates/ruff_db/src/diagnostic/render/junit.rs b/crates/ruff_db/src/diagnostic/render/junit.rs index 2b30e3ec73..c9d6fdd499 100644 --- a/crates/ruff_db/src/diagnostic/render/junit.rs +++ b/crates/ruff_db/src/diagnostic/render/junit.rs @@ -56,17 +56,17 @@ impl<'a> JunitRenderer<'a> { start_location: location, } = diagnostic; let mut status = TestCaseStatus::non_success(NonSuccessKind::Failure); - status.set_message(diagnostic.body()); + status.set_message(diagnostic.concise_message().to_str()); if let Some(location) = location { status.set_description(format!( "line {row}, col {col}, {body}", row = location.line, col = location.column, - body = diagnostic.body() + body = diagnostic.concise_message() )); } else { - status.set_description(diagnostic.body()); + status.set_description(diagnostic.concise_message().to_str()); } let code = diagnostic diff --git a/crates/ruff_db/src/diagnostic/render/pylint.rs b/crates/ruff_db/src/diagnostic/render/pylint.rs index 7040c344af..331b23c126 100644 --- a/crates/ruff_db/src/diagnostic/render/pylint.rs +++ b/crates/ruff_db/src/diagnostic/render/pylint.rs @@ -55,7 +55,7 @@ impl PylintRenderer<'_> { f, "{path}:{row}: [{code}] {body}", path = filename, - body = diagnostic.body() + body = diagnostic.concise_message() )?; } diff --git a/crates/ruff_db/src/diagnostic/render/rdjson.rs b/crates/ruff_db/src/diagnostic/render/rdjson.rs index e2a976aa7d..9f419213c0 100644 --- a/crates/ruff_db/src/diagnostic/render/rdjson.rs +++ b/crates/ruff_db/src/diagnostic/render/rdjson.rs @@ -5,7 +5,7 @@ use ruff_diagnostics::{Edit, Fix}; use ruff_source_file::{LineColumn, SourceCode}; use ruff_text_size::Ranged; -use crate::diagnostic::Diagnostic; +use crate::diagnostic::{ConciseMessage, Diagnostic}; use super::FileResolver; @@ -76,7 +76,7 @@ fn diagnostic_to_rdjson<'a>( let edits = diagnostic.fix().map(Fix::edits).unwrap_or_default(); RdjsonDiagnostic { - message: diagnostic.body(), + message: diagnostic.concise_message(), location, code: RdjsonCode { value: diagnostic @@ -155,7 +155,7 @@ struct RdjsonDiagnostic<'a> { code: RdjsonCode<'a>, #[serde(skip_serializing_if = "Option::is_none")] location: Option>, - message: &'a str, + message: ConciseMessage<'a>, #[serde(skip_serializing_if = "Vec::is_empty")] suggestions: Vec>, } diff --git a/crates/ruff_linter/src/message/grouped.rs b/crates/ruff_linter/src/message/grouped.rs index 0426697728..86a2e71fdf 100644 --- a/crates/ruff_linter/src/message/grouped.rs +++ b/crates/ruff_linter/src/message/grouped.rs @@ -197,7 +197,7 @@ impl Display for RuleCodeAndBody<'_> { f, "{fix}{body}", fix = format_args!("[{}] ", "*".cyan()), - body = self.message.body(), + body = self.message.concise_message(), ); } } @@ -208,14 +208,14 @@ impl Display for RuleCodeAndBody<'_> { f, "{code} {body}", code = code.red().bold(), - body = self.message.body(), + body = self.message.concise_message(), ) } else { write!( f, "{code}: {body}", code = self.message.id().as_str().red().bold(), - body = self.message.body(), + body = self.message.concise_message(), ) } } diff --git a/crates/ruff_linter/src/message/sarif.rs b/crates/ruff_linter/src/message/sarif.rs index 78c316b130..764d7a737e 100644 --- a/crates/ruff_linter/src/message/sarif.rs +++ b/crates/ruff_linter/src/message/sarif.rs @@ -334,7 +334,7 @@ impl<'a> SarifResult<'a> { rule_id: RuleCode::from(diagnostic), level: "error".to_string(), message: SarifMessage { - text: diagnostic.body().to_string(), + text: diagnostic.concise_message().to_string(), }, fixes: Self::fix(diagnostic, &uri).into_iter().collect(), locations: vec![SarifLocation { diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index db3f9ce4d8..f02cd04e8c 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -243,7 +243,7 @@ fn to_lsp_diagnostic( ) -> (usize, lsp_types::Diagnostic) { let diagnostic_range = diagnostic.range().unwrap_or_default(); let name = diagnostic.name(); - let body = diagnostic.body().to_string(); + let body = diagnostic.concise_message().to_string(); let fix = diagnostic.fix(); let suggestion = diagnostic.first_help_text(); let code = diagnostic.secondary_code(); diff --git a/crates/ruff_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index 6dd49a15bf..326dc2b194 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -241,7 +241,7 @@ impl Workspace { let range = msg.range().unwrap_or_default(); ExpandedMessage { code: msg.secondary_code_or_id().to_string(), - message: msg.body().to_string(), + message: msg.concise_message().to_string(), start_location: source_code .source_location(range.start(), self.position_encoding) .into(), diff --git a/crates/ty/tests/cli/main.rs b/crates/ty/tests/cli/main.rs index 70f8073cf0..2004a91368 100644 --- a/crates/ty/tests/cli/main.rs +++ b/crates/ty/tests/cli/main.rs @@ -659,6 +659,8 @@ fn gitlab_diagnostics() -> anyhow::Result<()> { r#" print(x) # [unresolved-reference] print(4[1]) # [non-subscriptable] + from typing_extensions import reveal_type + reveal_type('str'.lower()) # [revealed-type] "#, )?; @@ -709,6 +711,25 @@ fn gitlab_diagnostics() -> anyhow::Result<()> { } } } + }, + { + "check_name": "revealed-type", + "description": "revealed-type: Revealed type: `LiteralString`", + "severity": "info", + "fingerprint": "[FINGERPRINT]", + "location": { + "path": "test.py", + "positions": { + "begin": { + "line": 5, + "column": 13 + }, + "end": { + "line": 5, + "column": 26 + } + } + } } ] ----- stderr ----- @@ -724,6 +745,8 @@ fn github_diagnostics() -> anyhow::Result<()> { r#" print(x) # [unresolved-reference] print(4[1]) # [non-subscriptable] + from typing_extensions import reveal_type + reveal_type('str'.lower()) # [revealed-type] "#, )?; @@ -733,6 +756,7 @@ fn github_diagnostics() -> anyhow::Result<()> { ----- stdout ----- ::warning title=ty (unresolved-reference),file=/test.py,line=2,col=7,endLine=2,endColumn=8::test.py:2:7: unresolved-reference: Name `x` used when not defined ::error title=ty (non-subscriptable),file=/test.py,line=3,col=7,endLine=3,endColumn=11::test.py:3:7: non-subscriptable: Cannot subscript object of type `Literal[4]` with no `__getitem__` method + ::notice title=ty (revealed-type),file=/test.py,line=5,col=13,endLine=5,endColumn=26::test.py:5:13: revealed-type: Revealed type: `LiteralString` ----- stderr ----- "); diff --git a/crates/ty_test/src/matcher.rs b/crates/ty_test/src/matcher.rs index 71b165ae37..1645aa6d3c 100644 --- a/crates/ty_test/src/matcher.rs +++ b/crates/ty_test/src/matcher.rs @@ -319,7 +319,7 @@ impl Matcher { .column .is_none_or(|col| col == self.column(diagnostic)); let message_matches = error.message_contains.is_none_or(|needle| { - normalize_paths(&diagnostic.concise_message().to_string()).contains(needle) + normalize_paths(&diagnostic.concise_message().to_str()).contains(needle) }); lint_name_matches && column_matches && message_matches });