mirror of
https://github.com/astral-sh/ruff
synced 2026-01-21 13:30:49 -05:00
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
This commit is contained in:
@@ -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<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
|
||||
where
|
||||
S: serde::Serializer,
|
||||
{
|
||||
serializer.collect_str(self)
|
||||
}
|
||||
}
|
||||
|
||||
/// A diagnostic message string.
|
||||
///
|
||||
/// This is, for all intents and purposes, equivalent to a `Box<str>`.
|
||||
|
||||
@@ -52,7 +52,7 @@ impl AzureRenderer<'_> {
|
||||
f,
|
||||
"code={code};]{body}",
|
||||
code = diag.secondary_code_or_id(),
|
||||
body = diag.body(),
|
||||
body = diag.concise_message(),
|
||||
)?;
|
||||
}
|
||||
|
||||
|
||||
@@ -87,7 +87,7 @@ impl<'a> GithubRenderer<'a> {
|
||||
write!(f, "{id}:", id = diagnostic.id())?;
|
||||
}
|
||||
|
||||
writeln!(f, " {}", diagnostic.body())?;
|
||||
writeln!(f, " {}", diagnostic.concise_message())?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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<JsonFix<'a>>,
|
||||
location: Option<JsonLocation>,
|
||||
message: &'a str,
|
||||
message: ConciseMessage<'a>,
|
||||
noqa_row: Option<OneIndexed>,
|
||||
url: Option<&'a str>,
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -55,7 +55,7 @@ impl PylintRenderer<'_> {
|
||||
f,
|
||||
"{path}:{row}: [{code}] {body}",
|
||||
path = filename,
|
||||
body = diagnostic.body()
|
||||
body = diagnostic.concise_message()
|
||||
)?;
|
||||
}
|
||||
|
||||
|
||||
@@ -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<RdjsonLocation<'a>>,
|
||||
message: &'a str,
|
||||
message: ConciseMessage<'a>,
|
||||
#[serde(skip_serializing_if = "Vec::is_empty")]
|
||||
suggestions: Vec<RdjsonSuggestion<'a>>,
|
||||
}
|
||||
|
||||
@@ -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(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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=<temp_dir>/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=<temp_dir>/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=<temp_dir>/test.py,line=5,col=13,endLine=5,endColumn=26::test.py:5:13: revealed-type: Revealed type: `LiteralString`
|
||||
|
||||
----- stderr -----
|
||||
");
|
||||
|
||||
@@ -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
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user