-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add a ViolationMetadata::rule
method
#18234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c81c360
8c1fe5d
dc7dd7e
3cfcf64
bc8389a
1ce87b2
f70bef0
a983c37
0e9897b
2e7d2bc
2103675
daa0d26
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,7 @@ | ||
pub use diagnostic::Diagnostic; | ||
pub use edit::Edit; | ||
pub use fix::{Applicability, Fix, IsolationLevel}; | ||
pub use source_map::{SourceMap, SourceMarker}; | ||
pub use violation::{AlwaysFixableViolation, FixAvailability, Violation, ViolationMetadata}; | ||
|
||
mod diagnostic; | ||
mod edit; | ||
mod fix; | ||
mod source_map; | ||
mod violation; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,30 +3,37 @@ use log::debug; | |
|
||
use ruff_text_size::{Ranged, TextRange, TextSize}; | ||
|
||
use crate::{Fix, Violation}; | ||
use crate::registry::AsRule; | ||
use crate::violation::Violation; | ||
use crate::{Fix, codes::Rule}; | ||
|
||
#[derive(Debug, PartialEq, Eq, Clone)] | ||
pub struct Diagnostic { | ||
/// The identifier of the diagnostic, used to align the diagnostic with a rule. | ||
pub name: &'static str, | ||
/// The message body to display to the user, to explain the diagnostic. | ||
pub body: String, | ||
/// The message to display to the user, to explain the suggested fix. | ||
pub suggestion: Option<String>, | ||
pub range: TextRange, | ||
pub fix: Option<Fix>, | ||
pub parent: Option<TextSize>, | ||
|
||
pub(crate) rule: Rule, | ||
} | ||
|
||
impl Diagnostic { | ||
// TODO(brent) We temporarily allow this to avoid updating all of the call sites to add | ||
// references. I expect this method to go away or change significantly with the rest of the | ||
// diagnostic refactor, but if it still exists in this form at the end of the refactor, we | ||
// should just update the call sites. | ||
#[expect(clippy::needless_pass_by_value)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we allow There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes that's right. I was a bit confused why it wasn't already needless and thought further refactoring (like combining this with |
||
pub fn new<T: Violation>(kind: T, range: TextRange) -> Self { | ||
Self { | ||
name: T::rule_name(), | ||
body: Violation::message(&kind), | ||
suggestion: Violation::fix_title(&kind), | ||
range, | ||
fix: None, | ||
parent: None, | ||
rule: T::rule(), | ||
} | ||
} | ||
|
||
|
@@ -50,7 +57,7 @@ impl Diagnostic { | |
pub fn try_set_fix(&mut self, func: impl FnOnce() -> Result<Fix>) { | ||
match func() { | ||
Ok(fix) => self.fix = Some(fix), | ||
Err(err) => debug!("Failed to create fix for {}: {}", self.name, err), | ||
Err(err) => debug!("Failed to create fix for {}: {}", self.rule, err), | ||
} | ||
} | ||
|
||
|
@@ -61,7 +68,7 @@ impl Diagnostic { | |
match func() { | ||
Ok(None) => {} | ||
Ok(Some(fix)) => self.fix = Some(fix), | ||
Err(err) => debug!("Failed to create fix for {}: {}", self.name, err), | ||
Err(err) => debug!("Failed to create fix for {}: {}", self.rule, err), | ||
} | ||
} | ||
|
||
|
@@ -80,6 +87,12 @@ impl Diagnostic { | |
} | ||
} | ||
|
||
impl AsRule for Diagnostic { | ||
fn rule(&self) -> Rule { | ||
self.rule | ||
} | ||
} | ||
|
||
impl Ranged for Diagnostic { | ||
fn range(&self) -> TextRange { | ||
self.range | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our plan is to remove this
Diagnostic
type withruff_db::Diagnostic
. What's your plan on how to removerule
from here because we can't add it toruff_db::Diagnostic
.