diff --git a/crates/ty/docs/rules.md b/crates/ty/docs/rules.md
index 7f9931cdf5..e2b08952a7 100644
--- a/crates/ty/docs/rules.md
+++ b/crates/ty/docs/rules.md
@@ -436,7 +436,7 @@ def test(): -> "int":
Default level: warn ·
Added in 0.0.1-alpha.1 ·
Related issues ·
-View source
+View source
@@ -1016,7 +1016,7 @@ class D(Generic[U, T]): ...
Default level: warn ·
Added in 0.0.1-alpha.1 ·
Related issues ·
-View source
+View source
@@ -2790,7 +2790,7 @@ A() + A() # TypeError: unsupported operand type(s) for +: 'A' and 'A'
Default level: ignore ·
Added in 0.0.1-alpha.1 ·
Related issues ·
-View source
+View source
diff --git a/crates/ty_python_semantic/src/suppression.rs b/crates/ty_python_semantic/src/suppression.rs
index 94590c076d..fa1fb7fa77 100644
--- a/crates/ty_python_semantic/src/suppression.rs
+++ b/crates/ty_python_semantic/src/suppression.rs
@@ -1,24 +1,27 @@
-use smallvec::{SmallVec, smallvec};
-use std::error::Error;
-use std::fmt;
-use std::fmt::Formatter;
-use std::fmt::Write as _;
-use thiserror::Error;
+mod add_ignore;
+mod parser;
+mod unused;
-use crate::diagnostic::DiagnosticGuard;
-use crate::lint::{GetLintError, Level, LintMetadata, LintRegistry, LintStatus};
-use crate::types::TypeCheckDiagnostics;
-use crate::{Db, declare_lint, lint::LintId};
+use smallvec::SmallVec;
+use std::fmt;
use ruff_db::diagnostic::{
Annotation, Diagnostic, DiagnosticId, IntoDiagnosticMessage, Severity, Span,
};
use ruff_db::{files::File, parsed::parsed_module, source::source_text};
-use ruff_diagnostics::{Edit, Fix};
use ruff_python_ast::token::TokenKind;
-use ruff_python_trivia::Cursor;
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
+use crate::diagnostic::DiagnosticGuard;
+use crate::lint::{GetLintError, Level, LintMetadata, LintRegistry, LintStatus};
+pub use crate::suppression::add_ignore::create_suppression_fix;
+use crate::suppression::parser::{
+ ParseError, ParseErrorKind, SuppressionComment, SuppressionParser,
+};
+use crate::suppression::unused::check_unused_suppressions;
+use crate::types::TypeCheckDiagnostics;
+use crate::{Db, declare_lint, lint::LintId};
+
declare_lint! {
/// ## What it does
/// Checks for `type: ignore` or `ty: ignore` directives that are no longer applicable.
@@ -183,275 +186,6 @@ fn check_invalid_suppression(context: &mut CheckSuppressionsContext) {
}
}
-/// Checks for unused suppression comments in `file` and
-/// adds diagnostic for each of them to `diagnostics`.
-///
-/// Does nothing if the [`UNUSED_IGNORE_COMMENT`] rule is disabled.
-fn check_unused_suppressions(context: &mut CheckSuppressionsContext) {
- if context.is_lint_disabled(&UNUSED_IGNORE_COMMENT) {
- return;
- }
-
- let diagnostics = context.diagnostics.get_mut();
-
- let all = context.suppressions;
- let mut unused = Vec::with_capacity(
- all.file
- .len()
- .saturating_add(all.line.len())
- .saturating_sub(diagnostics.used_len()),
- );
-
- // Collect all suppressions that are unused after type-checking.
- for suppression in all {
- if diagnostics.is_used(suppression.id()) {
- continue;
- }
-
- // `unused-ignore-comment` diagnostics can only be suppressed by specifying a
- // code. This is necessary because every `type: ignore` would implicitly also
- // suppress its own unused-ignore-comment diagnostic.
- if let Some(unused_suppression) = all
- .lint_suppressions(suppression.range, LintId::of(&UNUSED_IGNORE_COMMENT))
- .find(|unused_ignore_suppression| unused_ignore_suppression.target.is_lint())
- {
- // A `unused-ignore-comment` suppression can't ignore itself.
- // It can only ignore other suppressions.
- if unused_suppression.id() != suppression.id() {
- diagnostics.mark_used(unused_suppression.id());
- continue;
- }
- }
-
- unused.push(suppression);
- }
-
- let mut unused_iter = unused
- .iter()
- .filter(|suppression| {
- // This looks silly but it's necessary to check again if a `unused-ignore-comment` is indeed unused
- // in case the "unused" directive comes after it:
- // ```py
- // a = 10 / 2 # ty: ignore[unused-ignore-comment, division-by-zero]
- // ```
- !context.is_suppression_used(suppression.id())
- })
- .peekable();
-
- let source = source_text(context.db, context.file);
-
- while let Some(suppression) = unused_iter.next() {
- let mut diag = match suppression.target {
- SuppressionTarget::All => {
- let Some(diag) =
- context.report_unchecked(&UNUSED_IGNORE_COMMENT, suppression.range)
- else {
- continue;
- };
-
- diag.into_diagnostic(format_args!(
- "Unused blanket `{}` directive",
- suppression.kind
- ))
- }
- SuppressionTarget::Lint(lint) => {
- // A single code in a `ty: ignore[, , ...]` directive
-
- // Is this the first code directly after the `[`?
- let includes_first_code = source[..suppression.range.start().to_usize()]
- .trim_end()
- .ends_with('[');
-
- let mut current = suppression;
- let mut unused_codes = Vec::new();
-
- // Group successive codes together into a single diagnostic,
- // or report the entire directive if all codes are unused.
- while let Some(next) = unused_iter.peek() {
- if let SuppressionTarget::Lint(next_lint) = next.target
- && next.comment_range == current.comment_range
- && source[TextRange::new(current.range.end(), next.range.start())]
- .chars()
- .all(|c| c.is_whitespace() || c == ',')
- {
- unused_codes.push(next_lint);
- current = *next;
- unused_iter.next();
- } else {
- break;
- }
- }
-
- // Is the last suppression code the last code before the closing `]`.
- let includes_last_code = source[current.range.end().to_usize()..]
- .trim_start()
- .starts_with(']');
-
- // If only some codes are unused
- if !includes_first_code || !includes_last_code {
- let mut codes = format!("'{}'", lint.name());
- for code in &unused_codes {
- let _ = write!(&mut codes, ", '{code}'", code = code.name());
- }
-
- if let Some(diag) = context.report_unchecked(
- &UNUSED_IGNORE_COMMENT,
- TextRange::new(suppression.range.start(), current.range.end()),
- ) {
- let mut diag = diag.into_diagnostic(format_args!(
- "Unused `{kind}` directive: {codes}",
- kind = suppression.kind
- ));
-
- diag.primary_annotation_mut()
- .unwrap()
- .push_tag(ruff_db::diagnostic::DiagnosticTag::Unnecessary);
-
- // Delete everything up to the start of the next code.
- let trailing_len: TextSize = source[current.range.end().to_usize()..]
- .chars()
- .take_while(|c: &char| c.is_whitespace() || *c == ',')
- .map(TextLen::text_len)
- .sum();
-
- // If we delete the last codes before `]`, ensure we delete any trailing comma
- let leading_len: TextSize = if includes_last_code {
- source[..suppression.range.start().to_usize()]
- .chars()
- .rev()
- .take_while(|c: &char| c.is_whitespace() || *c == ',')
- .map(TextLen::text_len)
- .sum()
- } else {
- TextSize::default()
- };
-
- let fix_range = TextRange::new(
- suppression.range.start() - leading_len,
- current.range.end() + trailing_len,
- );
- diag.set_fix(Fix::safe_edit(Edit::range_deletion(fix_range)));
-
- if unused_codes.is_empty() {
- diag.help("Remove the unused suppression code");
- } else {
- diag.help("Remove the unused suppression codes");
- }
- }
-
- continue;
- }
-
- // All codes are unused
- let Some(diag) =
- context.report_unchecked(&UNUSED_IGNORE_COMMENT, suppression.comment_range)
- else {
- continue;
- };
-
- diag.into_diagnostic(format_args!(
- "Unused `{kind}` directive",
- kind = suppression.kind
- ))
- }
- SuppressionTarget::Empty => {
- let Some(diag) =
- context.report_unchecked(&UNUSED_IGNORE_COMMENT, suppression.range)
- else {
- continue;
- };
- diag.into_diagnostic(format_args!(
- "Unused `{kind}` without a code",
- kind = suppression.kind
- ))
- }
- };
-
- diag.primary_annotation_mut()
- .unwrap()
- .push_tag(ruff_db::diagnostic::DiagnosticTag::Unnecessary);
- diag.set_fix(remove_comment_fix(suppression, &source));
- diag.help("Remove the unused suppression comment");
- }
-}
-
-/// Creates a fix for adding a suppression comment to suppress `lint` for `range`.
-///
-/// The fix prefers adding the code to an existing `ty: ignore[]` comment over
-/// adding a new suppression comment.
-pub fn create_suppression_fix(db: &dyn Db, file: File, id: LintId, range: TextRange) -> Fix {
- let suppressions = suppressions(db, file);
- let source = source_text(db, file);
-
- let mut existing_suppressions = suppressions.line_suppressions(range).filter(|suppression| {
- matches!(
- suppression.target,
- SuppressionTarget::Lint(_) | SuppressionTarget::Empty,
- )
- });
-
- // If there's an existing `ty: ignore[]` comment, append the code to it instead of creating a new suppression comment.
- if let Some(existing) = existing_suppressions.next() {
- let comment_text = &source[existing.comment_range];
- // Only add to the existing ignore comment if it has no reason.
- if let Some(before_closing_paren) = comment_text.trim_end().strip_suffix(']') {
- let up_to_last_code = before_closing_paren.trim_end();
-
- let insertion = if up_to_last_code.ends_with(',') {
- format!(" {id}", id = id.name())
- } else {
- format!(", {id}", id = id.name())
- };
-
- let relative_offset_from_end = comment_text.text_len() - up_to_last_code.text_len();
-
- return Fix::safe_edit(Edit::insertion(
- insertion,
- existing.comment_range.end() - relative_offset_from_end,
- ));
- }
- }
-
- // Always insert a new suppression at the end of the range to avoid having to deal with multiline strings
- // etc. Also make sure to not pass a sub-token range to `Tokens::after`.
- let parsed = parsed_module(db, file).load(db);
- let tokens = parsed.tokens().at_offset(range.end());
- let token_range = match tokens {
- ruff_python_ast::token::TokenAt::None => range,
- ruff_python_ast::token::TokenAt::Single(token) => token.range(),
- ruff_python_ast::token::TokenAt::Between(..) => range,
- };
- let tokens_after = parsed.tokens().after(token_range.end());
-
- // Same as for `line_end` when building up the `suppressions`: Ignore newlines
- // in multiline-strings, inside f-strings, or after a line continuation because we can't
- // place a comment on those lines.
- let line_end = tokens_after
- .iter()
- .find(|token| {
- matches!(
- token.kind(),
- TokenKind::Newline | TokenKind::NonLogicalNewline
- )
- })
- .map(Ranged::start)
- .unwrap_or(source.text_len());
-
- let up_to_line_end = &source[..line_end.to_usize()];
- let up_to_first_content = up_to_line_end.trim_end();
- let trailing_whitespace_len = up_to_line_end.text_len() - up_to_first_content.text_len();
-
- let insertion = format!(" # ty:ignore[{id}]", id = id.name());
-
- Fix::safe_edit(if trailing_whitespace_len == TextSize::ZERO {
- Edit::insertion(insertion, line_end)
- } else {
- // `expr # fmt: off`
- // Trim the trailing whitespace
- Edit::replacement(insertion, line_end - trailing_whitespace_len, line_end)
- })
-}
-
struct CheckSuppressionsContext<'a> {
db: &'a dyn Db,
file: File,
@@ -691,6 +425,34 @@ impl Suppression {
}
}
+#[derive(Copy, Clone, Debug, Eq, PartialEq, get_size2::GetSize)]
+enum SuppressionKind {
+ TypeIgnore,
+ Ty,
+}
+
+impl SuppressionKind {
+ const fn is_type_ignore(self) -> bool {
+ matches!(self, SuppressionKind::TypeIgnore)
+ }
+
+ fn len_utf8(self) -> usize {
+ match self {
+ SuppressionKind::TypeIgnore => "type".len(),
+ SuppressionKind::Ty => "ty".len(),
+ }
+ }
+}
+
+impl fmt::Display for SuppressionKind {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match self {
+ SuppressionKind::TypeIgnore => f.write_str("type: ignore"),
+ SuppressionKind::Ty => f.write_str("ty: ignore"),
+ }
+ }
+}
+
/// Unique ID for a suppression in a file.
///
/// ## Implementation
@@ -763,6 +525,7 @@ impl<'a> SuppressionsBuilder<'a> {
}
}
+ #[expect(clippy::needless_pass_by_value)]
fn add_comment(&mut self, comment: SuppressionComment, line_range: TextRange) {
// `type: ignore` comments at the start of the file apply to the entire range.
// > A # type: ignore comment on a line by itself at the top of a file, before any docstrings,
@@ -770,7 +533,7 @@ impl<'a> SuppressionsBuilder<'a> {
// > Blank lines and other comments, such as shebang lines and coding cookies,
// > may precede the # type: ignore comment.
// > https://typing.python.org/en/latest/spec/directives.html#type-ignore-comments
- let is_file_suppression = comment.kind.is_type_ignore() && !self.seen_non_trivia_token;
+ let is_file_suppression = comment.kind().is_type_ignore() && !self.seen_non_trivia_token;
let suppressed_range = if is_file_suppression {
TextRange::new(0.into(), self.source.text_len())
@@ -786,14 +549,14 @@ impl<'a> SuppressionsBuilder<'a> {
}
};
- match comment.codes {
+ match comment.codes() {
// `type: ignore`
None => {
push_type_ignore_suppression(Suppression {
target: SuppressionTarget::All,
- kind: comment.kind,
- comment_range: comment.range,
- range: comment.range,
+ kind: comment.kind(),
+ comment_range: comment.range(),
+ range: comment.range(),
suppressed_range,
});
}
@@ -801,45 +564,45 @@ impl<'a> SuppressionsBuilder<'a> {
// `type: ignore[..]`
// The suppression applies to all lints if it is a `type: ignore`
// comment. `type: ignore` apply to all lints for better mypy compatibility.
- Some(_) if comment.kind.is_type_ignore() => {
+ Some(_) if comment.kind().is_type_ignore() => {
push_type_ignore_suppression(Suppression {
target: SuppressionTarget::All,
- kind: comment.kind,
- comment_range: comment.range,
- range: comment.range,
+ kind: comment.kind(),
+ comment_range: comment.range(),
+ range: comment.range(),
suppressed_range,
});
}
// `ty: ignore[]`
- Some(codes) if codes.is_empty() => {
+ Some([]) => {
self.line.push(Suppression {
target: SuppressionTarget::Empty,
- kind: comment.kind,
- range: comment.range,
- comment_range: comment.range,
+ kind: comment.kind(),
+ range: comment.range(),
+ comment_range: comment.range(),
suppressed_range,
});
}
// `ty: ignore[a, b]`
Some(codes) => {
- for code_range in codes {
+ for &code_range in codes {
let code = &self.source[code_range];
match self.lint_registry.get(code) {
Ok(lint) => {
self.line.push(Suppression {
target: SuppressionTarget::Lint(lint),
- kind: comment.kind,
+ kind: comment.kind(),
range: code_range,
- comment_range: comment.range,
+ comment_range: comment.range(),
suppressed_range,
});
}
Err(error) => self.unknown.push(UnknownSuppression {
range: code_range,
- comment_range: comment.range,
+ comment_range: comment.range(),
reason: error,
}),
}
@@ -870,571 +633,3 @@ struct InvalidSuppression {
kind: SuppressionKind,
error: ParseError,
}
-
-struct SuppressionParser<'src> {
- cursor: Cursor<'src>,
- range: TextRange,
-}
-
-impl<'src> SuppressionParser<'src> {
- fn new(source: &'src str, range: TextRange) -> Self {
- let cursor = Cursor::new(&source[range]);
-
- Self { cursor, range }
- }
-
- fn parse_comment(&mut self) -> Result {
- let comment_start = self.offset();
- self.cursor.start_token();
-
- if !self.cursor.eat_char('#') {
- return self.syntax_error(ParseErrorKind::CommentWithoutHash);
- }
-
- self.eat_whitespace();
-
- // type: ignore[code]
- // ^^^^^^^^^^^^
- let Some(kind) = self.eat_kind() else {
- return Err(ParseError::new(
- ParseErrorKind::NotASuppression,
- TextRange::new(comment_start, self.offset()),
- ));
- };
-
- let has_trailing_whitespace = self.eat_whitespace();
-
- // type: ignore[code1, code2]
- // ^^^^^^
- let codes = self.eat_codes(kind)?;
-
- if self.cursor.is_eof() || codes.is_some() || has_trailing_whitespace {
- // Consume the comment until its end or until the next "sub-comment" starts.
- self.cursor.eat_while(|c| c != '#');
- Ok(SuppressionComment {
- kind,
- codes,
- range: TextRange::at(comment_start, self.cursor.token_len()),
- })
- } else {
- self.syntax_error(ParseErrorKind::NoWhitespaceAfterIgnore(kind))
- }
- }
-
- fn eat_kind(&mut self) -> Option {
- let kind = if self.cursor.as_str().starts_with("type") {
- SuppressionKind::TypeIgnore
- } else if self.cursor.as_str().starts_with("ty") {
- SuppressionKind::Ty
- } else {
- return None;
- };
-
- self.cursor.skip_bytes(kind.len_utf8());
-
- self.eat_whitespace();
-
- if !self.cursor.eat_char(':') {
- return None;
- }
-
- self.eat_whitespace();
-
- if !self.cursor.as_str().starts_with("ignore") {
- return None;
- }
-
- self.cursor.skip_bytes("ignore".len());
-
- Some(kind)
- }
-
- fn eat_codes(
- &mut self,
- kind: SuppressionKind,
- ) -> Result