mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-01 22:31:47 +00:00
Inline DiagnosticKind into other diagnostic types (#18074)
## Summary This PR deletes the `DiagnosticKind` type by inlining its three fields (`name`, `body`, and `suggestion`) into three other diagnostic types: `Diagnostic`, `DiagnosticMessage`, and `CacheMessage`. Instead of deferring to an internal `DiagnosticKind`, both `Diagnostic` and `DiagnosticMessage` now have their own macro-generated `AsRule` implementations. This should make both https://github.com/astral-sh/ruff/pull/18051 and another follow-up PR changing the type of `name` on `CacheMessage` easier since its type will be able to change separately from `Diagnostic` and `DiagnosticMessage`. ## Test Plan Existing tests
This commit is contained in:
Brent Westbrook
GitHub
parent
b35bf8ae07
commit
e2c5b83fe1
41 changed files with 604 additions and 621 deletions
|
|
@ -17,7 +17,7 @@ pub use json_lines::JsonLinesEmitter;
|
|||
pub use junit::JunitEmitter;
|
||||
pub use pylint::PylintEmitter;
|
||||
pub use rdjson::RdjsonEmitter;
|
||||
use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix};
|
||||
use ruff_diagnostics::{Diagnostic, Fix};
|
||||
use ruff_notebook::NotebookIndex;
|
||||
use ruff_python_parser::{ParseError, UnsupportedSyntaxError};
|
||||
use ruff_source_file::{LineColumn, SourceFile};
|
||||
|
|
@ -53,7 +53,9 @@ pub enum Message {
|
|||
/// A diagnostic message corresponding to a rule violation.
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct DiagnosticMessage {
|
||||
pub kind: DiagnosticKind,
|
||||
pub name: &'static str,
|
||||
pub body: String,
|
||||
pub suggestion: Option<String>,
|
||||
pub range: TextRange,
|
||||
pub fix: Option<Fix>,
|
||||
pub parent: Option<TextSize>,
|
||||
|
|
@ -96,7 +98,9 @@ impl Message {
|
|||
) -> Message {
|
||||
Message::Diagnostic(DiagnosticMessage {
|
||||
range: diagnostic.range(),
|
||||
kind: diagnostic.kind,
|
||||
name: diagnostic.name,
|
||||
body: diagnostic.body,
|
||||
suggestion: diagnostic.suggestion,
|
||||
fix: diagnostic.fix,
|
||||
parent: diagnostic.parent,
|
||||
file,
|
||||
|
|
@ -183,7 +187,7 @@ impl Message {
|
|||
/// Returns a message kind.
|
||||
pub fn kind(&self) -> MessageKind {
|
||||
match self {
|
||||
Message::Diagnostic(m) => MessageKind::Diagnostic(m.kind.rule()),
|
||||
Message::Diagnostic(m) => MessageKind::Diagnostic(m.rule()),
|
||||
Message::SyntaxError(_) => MessageKind::SyntaxError,
|
||||
}
|
||||
}
|
||||
|
|
@ -191,7 +195,7 @@ impl Message {
|
|||
/// Returns the name used to represent the diagnostic.
|
||||
pub fn name(&self) -> &str {
|
||||
match self {
|
||||
Message::Diagnostic(m) => &m.kind.name,
|
||||
Message::Diagnostic(m) => m.name,
|
||||
Message::SyntaxError(_) => "SyntaxError",
|
||||
}
|
||||
}
|
||||
|
|
@ -199,7 +203,7 @@ impl Message {
|
|||
/// Returns the message body to display to the user.
|
||||
pub fn body(&self) -> &str {
|
||||
match self {
|
||||
Message::Diagnostic(m) => &m.kind.body,
|
||||
Message::Diagnostic(m) => &m.body,
|
||||
Message::SyntaxError(m) => m
|
||||
.primary_annotation()
|
||||
.expect("Expected a primary annotation for a ruff diagnostic")
|
||||
|
|
@ -211,7 +215,7 @@ impl Message {
|
|||
/// Returns the fix suggestion for the violation.
|
||||
pub fn suggestion(&self) -> Option<&str> {
|
||||
match self {
|
||||
Message::Diagnostic(m) => m.kind.suggestion.as_deref(),
|
||||
Message::Diagnostic(m) => m.suggestion.as_deref(),
|
||||
Message::SyntaxError(_) => None,
|
||||
}
|
||||
}
|
||||
|
|
@ -240,7 +244,7 @@ impl Message {
|
|||
/// Returns the [`Rule`] corresponding to the diagnostic message.
|
||||
pub fn rule(&self) -> Option<Rule> {
|
||||
match self {
|
||||
Message::Diagnostic(m) => Some(m.kind.rule()),
|
||||
Message::Diagnostic(m) => Some(m.rule()),
|
||||
Message::SyntaxError(_) => None,
|
||||
}
|
||||
}
|
||||
|
|
@ -379,13 +383,13 @@ impl<'a> EmitterContext<'a> {
|
|||
mod tests {
|
||||
use rustc_hash::FxHashMap;
|
||||
|
||||
use ruff_diagnostics::{Diagnostic, DiagnosticKind, Edit, Fix};
|
||||
use ruff_diagnostics::{Edit, Fix};
|
||||
use ruff_notebook::NotebookIndex;
|
||||
use ruff_python_parser::{parse_unchecked, Mode, ParseOptions};
|
||||
use ruff_source_file::{OneIndexed, SourceFileBuilder};
|
||||
use ruff_text_size::{Ranged, TextRange, TextSize};
|
||||
use ruff_text_size::{TextRange, TextSize};
|
||||
|
||||
use crate::message::{Emitter, EmitterContext, Message};
|
||||
use crate::message::{DiagnosticMessage, Emitter, EmitterContext, Message};
|
||||
use crate::Locator;
|
||||
|
||||
pub(super) fn create_syntax_error_messages() -> Vec<Message> {
|
||||
|
|
@ -421,54 +425,56 @@ def fibonacci(n):
|
|||
return fibonacci(n - 1) + fibonacci(n - 2)
|
||||
"#;
|
||||
|
||||
let unused_import = Diagnostic::new(
|
||||
DiagnosticKind {
|
||||
name: "UnusedImport".to_string(),
|
||||
body: "`os` imported but unused".to_string(),
|
||||
suggestion: Some("Remove unused import: `os`".to_string()),
|
||||
},
|
||||
TextRange::new(TextSize::from(7), TextSize::from(9)),
|
||||
)
|
||||
.with_fix(Fix::unsafe_edit(Edit::range_deletion(TextRange::new(
|
||||
TextSize::from(0),
|
||||
TextSize::from(10),
|
||||
))));
|
||||
|
||||
let fib_source = SourceFileBuilder::new("fib.py", fib).finish();
|
||||
|
||||
let unused_variable = Diagnostic::new(
|
||||
DiagnosticKind {
|
||||
name: "UnusedVariable".to_string(),
|
||||
body: "Local variable `x` is assigned to but never used".to_string(),
|
||||
suggestion: Some("Remove assignment to unused variable `x`".to_string()),
|
||||
},
|
||||
TextRange::new(TextSize::from(94), TextSize::from(95)),
|
||||
)
|
||||
.with_fix(Fix::unsafe_edit(Edit::deletion(
|
||||
TextSize::from(94),
|
||||
TextSize::from(99),
|
||||
)));
|
||||
let unused_import_start = TextSize::from(7);
|
||||
let unused_import = DiagnosticMessage {
|
||||
name: "unused-import",
|
||||
body: "`os` imported but unused".to_string(),
|
||||
suggestion: Some("Remove unused import: `os`".to_string()),
|
||||
range: TextRange::new(unused_import_start, TextSize::from(9)),
|
||||
fix: Some(Fix::unsafe_edit(Edit::range_deletion(TextRange::new(
|
||||
TextSize::from(0),
|
||||
TextSize::from(10),
|
||||
)))),
|
||||
parent: None,
|
||||
noqa_offset: unused_import_start,
|
||||
file: fib_source.clone(),
|
||||
};
|
||||
|
||||
let unused_variable_start = TextSize::from(94);
|
||||
let unused_variable = DiagnosticMessage {
|
||||
name: "unused-variable",
|
||||
body: "Local variable `x` is assigned to but never used".to_string(),
|
||||
suggestion: Some("Remove assignment to unused variable `x`".to_string()),
|
||||
range: TextRange::new(unused_variable_start, TextSize::from(95)),
|
||||
fix: Some(Fix::unsafe_edit(Edit::deletion(
|
||||
TextSize::from(94),
|
||||
TextSize::from(99),
|
||||
))),
|
||||
parent: None,
|
||||
noqa_offset: unused_variable_start,
|
||||
file: fib_source,
|
||||
};
|
||||
|
||||
let file_2 = r"if a == 1: pass";
|
||||
|
||||
let undefined_name = Diagnostic::new(
|
||||
DiagnosticKind {
|
||||
name: "UndefinedName".to_string(),
|
||||
body: "Undefined name `a`".to_string(),
|
||||
suggestion: None,
|
||||
},
|
||||
TextRange::new(TextSize::from(3), TextSize::from(4)),
|
||||
);
|
||||
let undefined_name_start = TextSize::from(3);
|
||||
let undefined_name = DiagnosticMessage {
|
||||
name: "undefined-name",
|
||||
body: "Undefined name `a`".to_string(),
|
||||
suggestion: None,
|
||||
range: TextRange::new(undefined_name_start, TextSize::from(4)),
|
||||
fix: None,
|
||||
parent: None,
|
||||
noqa_offset: undefined_name_start,
|
||||
file: SourceFileBuilder::new("undef.py", file_2).finish(),
|
||||
};
|
||||
|
||||
let file_2_source = SourceFileBuilder::new("undef.py", file_2).finish();
|
||||
|
||||
let unused_import_start = unused_import.start();
|
||||
let unused_variable_start = unused_variable.start();
|
||||
let undefined_name_start = undefined_name.start();
|
||||
vec![
|
||||
Message::from_diagnostic(unused_import, fib_source.clone(), unused_import_start),
|
||||
Message::from_diagnostic(unused_variable, fib_source, unused_variable_start),
|
||||
Message::from_diagnostic(undefined_name, file_2_source, undefined_name_start),
|
||||
Message::Diagnostic(unused_import),
|
||||
Message::Diagnostic(unused_variable),
|
||||
Message::Diagnostic(undefined_name),
|
||||
]
|
||||
}
|
||||
|
||||
|
|
@ -485,47 +491,53 @@ def foo():
|
|||
x = 1
|
||||
";
|
||||
|
||||
let unused_import_os = Diagnostic::new(
|
||||
DiagnosticKind {
|
||||
name: "UnusedImport".to_string(),
|
||||
body: "`os` imported but unused".to_string(),
|
||||
suggestion: Some("Remove unused import: `os`".to_string()),
|
||||
},
|
||||
TextRange::new(TextSize::from(16), TextSize::from(18)),
|
||||
)
|
||||
.with_fix(Fix::safe_edit(Edit::range_deletion(TextRange::new(
|
||||
TextSize::from(9),
|
||||
TextSize::from(19),
|
||||
))));
|
||||
|
||||
let unused_import_math = Diagnostic::new(
|
||||
DiagnosticKind {
|
||||
name: "UnusedImport".to_string(),
|
||||
body: "`math` imported but unused".to_string(),
|
||||
suggestion: Some("Remove unused import: `math`".to_string()),
|
||||
},
|
||||
TextRange::new(TextSize::from(35), TextSize::from(39)),
|
||||
)
|
||||
.with_fix(Fix::safe_edit(Edit::range_deletion(TextRange::new(
|
||||
TextSize::from(28),
|
||||
TextSize::from(40),
|
||||
))));
|
||||
|
||||
let unused_variable = Diagnostic::new(
|
||||
DiagnosticKind {
|
||||
name: "UnusedVariable".to_string(),
|
||||
body: "Local variable `x` is assigned to but never used".to_string(),
|
||||
suggestion: Some("Remove assignment to unused variable `x`".to_string()),
|
||||
},
|
||||
TextRange::new(TextSize::from(98), TextSize::from(99)),
|
||||
)
|
||||
.with_fix(Fix::unsafe_edit(Edit::deletion(
|
||||
TextSize::from(94),
|
||||
TextSize::from(104),
|
||||
)));
|
||||
|
||||
let notebook_source = SourceFileBuilder::new("notebook.ipynb", notebook).finish();
|
||||
|
||||
let unused_import_os_start = TextSize::from(16);
|
||||
let unused_import_os = DiagnosticMessage {
|
||||
name: "unused-import",
|
||||
body: "`os` imported but unused".to_string(),
|
||||
suggestion: Some("Remove unused import: `os`".to_string()),
|
||||
range: TextRange::new(unused_import_os_start, TextSize::from(18)),
|
||||
fix: Some(Fix::safe_edit(Edit::range_deletion(TextRange::new(
|
||||
TextSize::from(9),
|
||||
TextSize::from(19),
|
||||
)))),
|
||||
parent: None,
|
||||
file: notebook_source.clone(),
|
||||
noqa_offset: unused_import_os_start,
|
||||
};
|
||||
|
||||
let unused_import_math_start = TextSize::from(35);
|
||||
let unused_import_math = DiagnosticMessage {
|
||||
name: "unused-import",
|
||||
body: "`math` imported but unused".to_string(),
|
||||
suggestion: Some("Remove unused import: `math`".to_string()),
|
||||
range: TextRange::new(unused_import_math_start, TextSize::from(39)),
|
||||
fix: Some(Fix::safe_edit(Edit::range_deletion(TextRange::new(
|
||||
TextSize::from(28),
|
||||
TextSize::from(40),
|
||||
)))),
|
||||
parent: None,
|
||||
file: notebook_source.clone(),
|
||||
noqa_offset: unused_import_math_start,
|
||||
};
|
||||
|
||||
let unused_variable_start = TextSize::from(98);
|
||||
let unused_variable = DiagnosticMessage {
|
||||
name: "unused-variable",
|
||||
body: "Local variable `x` is assigned to but never used".to_string(),
|
||||
suggestion: Some("Remove assignment to unused variable `x`".to_string()),
|
||||
range: TextRange::new(unused_variable_start, TextSize::from(99)),
|
||||
fix: Some(Fix::unsafe_edit(Edit::deletion(
|
||||
TextSize::from(94),
|
||||
TextSize::from(104),
|
||||
))),
|
||||
parent: None,
|
||||
file: notebook_source,
|
||||
noqa_offset: unused_variable_start,
|
||||
};
|
||||
|
||||
let mut notebook_indexes = FxHashMap::default();
|
||||
notebook_indexes.insert(
|
||||
"notebook.ipynb".to_string(),
|
||||
|
|
@ -557,23 +569,11 @@ def foo():
|
|||
),
|
||||
);
|
||||
|
||||
let unused_import_os_start = unused_import_os.start();
|
||||
let unused_import_math_start = unused_import_math.start();
|
||||
let unused_variable_start = unused_variable.start();
|
||||
|
||||
(
|
||||
vec![
|
||||
Message::from_diagnostic(
|
||||
unused_import_os,
|
||||
notebook_source.clone(),
|
||||
unused_import_os_start,
|
||||
),
|
||||
Message::from_diagnostic(
|
||||
unused_import_math,
|
||||
notebook_source.clone(),
|
||||
unused_import_math_start,
|
||||
),
|
||||
Message::from_diagnostic(unused_variable, notebook_source, unused_variable_start),
|
||||
Message::Diagnostic(unused_import_os),
|
||||
Message::Diagnostic(unused_import_math),
|
||||
Message::Diagnostic(unused_variable),
|
||||
],
|
||||
notebook_indexes,
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue