Skip to content

Commit

Permalink
Add dedicated warning message for lint:rule for suppressions and CL…
Browse files Browse the repository at this point in the history
…I arguments
  • Loading branch information
MichaReiser committed Jan 24, 2025
1 parent 156a318 commit 61f473d
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 5 deletions.
10 changes: 10 additions & 0 deletions crates/red_knot_project/src/metadata/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,16 @@ impl Options {
format!("Unknown lint rule `{rule_name}`"),
Severity::Warning,
),
GetLintError::PrefixedWithCategory { suggestion, .. } => {
OptionDiagnostic::new(
DiagnosticId::UnknownRule,
format!(
"Unknown lint rule `{rule_name}`. Did you mean `{suggestion}`?"
),
Severity::Warning,
)
}

GetLintError::Removed(_) => OptionDiagnostic::new(
DiagnosticId::UnknownRule,
format!("Unknown lint rule `{rule_name}`"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,11 @@ a = 4 / 0 # error: [division-by-zero]
# error: [unknown-rule] "Unknown rule `is-equal-14`"
a = 10 + 4 # knot: ignore[is-equal-14]
```

## Code with `lint:` prefix

```py
# error:[unknown-rule] "Unknown rule `lint:division-by-zero`. Did you mean `division-by-zero`?"
# error: [division-by-zero]
a = 10 / 0 # knot: ignore[lint:division-by-zero]
```
37 changes: 33 additions & 4 deletions crates/red_knot_python_semantic/src/lint.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use itertools::Itertools;
use ruff_db::diagnostic::{LintName, Severity};
use ruff_db::diagnostic::{DiagnosticId, LintName, Severity};
use rustc_hash::FxHashMap;
use std::hash::Hasher;
use thiserror::Error;
Expand Down Expand Up @@ -345,7 +345,18 @@ impl LintRegistry {
}
}
Some(LintEntry::Removed(lint)) => Err(GetLintError::Removed(lint.name())),
None => Err(GetLintError::Unknown(code.to_string())),
None => {
if let Some(without_prefix) = DiagnosticId::strip_category(code) {
if let Some(entry) = self.by_name.get(without_prefix) {
return Err(GetLintError::PrefixedWithCategory {
prefixed: code.to_string(),
suggestion: entry.id().name.to_string(),
});
}
}

Err(GetLintError::Unknown(code.to_string()))
}
}
}

Expand Down Expand Up @@ -382,12 +393,20 @@ impl LintRegistry {
#[derive(Error, Debug, Clone, PartialEq, Eq)]
pub enum GetLintError {
/// The name maps to this removed lint.
#[error("lint {0} has been removed")]
#[error("lint `{0}` has been removed")]
Removed(LintName),

/// No lint with the given name is known.
#[error("unknown lint {0}")]
#[error("unknown lint `{0}`")]
Unknown(String),

/// The name uses the full qualified diagnostic id `lint:<rule>` instead of just `rule`.
/// The String is the name without the `lint:` category prefix.
#[error("unknown lint `{prefixed}`. Did you mean `{suggestion}`?")]
PrefixedWithCategory {
prefixed: String,
suggestion: String,
},
}

#[derive(Debug, PartialEq, Eq, Clone, Copy)]
Expand All @@ -399,6 +418,16 @@ pub enum LintEntry {
Alias(LintId),
}

impl LintEntry {
fn id(self) -> LintId {
match self {
LintEntry::Lint(id) => id,
LintEntry::Removed(id) => id,
LintEntry::Alias(id) => id,
}
}
}

impl From<&'static LintMetadata> for LintEntry {
fn from(metadata: &'static LintMetadata) -> Self {
if metadata.status.is_removed() {
Expand Down
14 changes: 13 additions & 1 deletion crates/red_knot_python_semantic/src/suppression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,17 @@ fn check_unknown_rule(context: &mut CheckSuppressionsContext) {
format_args!("Unknown rule `{rule}`"),
);
}

GetLintError::PrefixedWithCategory {
prefixed,
suggestion,
} => {
context.report_lint(
&UNKNOWN_RULE,
unknown.range,
format_args!("Unknown rule `{prefixed}`. Did you mean `{suggestion}`?"),
);
}
};
}
}
Expand Down Expand Up @@ -765,8 +776,9 @@ impl<'src> SuppressionParser<'src> {

fn eat_word(&mut self) -> bool {
if self.cursor.eat_if(char::is_alphabetic) {
// Allow `:` for better error recovery when someone uses `lint:code` instead of just `code`.
self.cursor
.eat_while(|c| c.is_alphanumeric() || matches!(c, '_' | '-'));
.eat_while(|c| c.is_alphanumeric() || matches!(c, '_' | '-' | ':'));
true
} else {
false
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_db/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ impl DiagnosticId {
matches!(self, DiagnosticId::Lint(self_name) if self_name == name)
}

pub fn strip_category(code: &str) -> Option<&str> {
code.split_once(':').map(|(_, rest)| rest)
}

/// Returns `true` if this `DiagnosticId` matches the given name.
///
/// ## Examples
Expand Down

0 comments on commit 61f473d

Please sign in to comment.