From 8fc4349fd383889109c84d9e35d433bc122e5751 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Mon, 22 Dec 2025 16:08:56 +0100 Subject: [PATCH] [ty] Split `suppression.rs` into multiple smaller modules (#22141) --- crates/ty/docs/rules.md | 6 +- crates/ty_python_semantic/src/suppression.rs | 927 ++---------------- .../src/suppression/add_ignore.rs | 87 ++ .../src/suppression/parser.rs | 530 ++++++++++ .../src/suppression/unused.rs | 232 +++++ 5 files changed, 913 insertions(+), 869 deletions(-) create mode 100644 crates/ty_python_semantic/src/suppression/add_ignore.rs create mode 100644 crates/ty_python_semantic/src/suppression/parser.rs create mode 100644 crates/ty_python_semantic/src/suppression/unused.rs 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>, ParseError> { - if !self.cursor.eat_char('[') { - return Ok(None); - } - - let mut codes: SmallVec<[TextRange; 2]> = smallvec![]; - - loop { - if self.cursor.is_eof() { - return self.syntax_error(ParseErrorKind::CodesMissingClosingBracket(kind)); - } - - self.eat_whitespace(); - - // `ty: ignore[]` or `ty: ignore[a,]` - if self.cursor.eat_char(']') { - break Ok(Some(codes)); - } - - let code_start = self.offset(); - if !self.eat_word() { - return self.syntax_error(ParseErrorKind::InvalidCode(kind)); - } - - codes.push(TextRange::new(code_start, self.offset())); - - self.eat_whitespace(); - - if !self.cursor.eat_char(',') { - if self.cursor.eat_char(']') { - break Ok(Some(codes)); - } - // `ty: ignore[a b] - return self.syntax_error(ParseErrorKind::CodesMissingComma(kind)); - } - } - } - - fn eat_whitespace(&mut self) -> bool { - if self.cursor.eat_if(char::is_whitespace) { - self.cursor.eat_while(char::is_whitespace); - true - } else { - false - } - } - - 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, '_' | '-' | ':')); - true - } else { - false - } - } - - fn syntax_error(&self, kind: ParseErrorKind) -> Result { - let len = if self.cursor.is_eof() { - TextSize::default() - } else { - self.cursor.first().text_len() - }; - - Err(ParseError::new(kind, TextRange::at(self.offset(), len))) - } - - fn offset(&self) -> TextSize { - self.range.start() + self.range.len() - self.cursor.text_len() - } -} - -impl Iterator for SuppressionParser<'_> { - type Item = Result; - - fn next(&mut self) -> Option { - if self.cursor.is_eof() { - return None; - } - - match self.parse_comment() { - Ok(result) => Some(Ok(result)), - Err(error) => { - self.cursor.eat_while(|c| c != '#'); - Some(Err(error)) - } - } - } -} - -/// A single parsed suppression comment. -#[derive(Clone, Debug, Eq, PartialEq)] -struct SuppressionComment { - /// The range of the suppression comment. - /// - /// This can be a sub-range of the comment token if the comment token contains multiple `#` tokens: - /// ```py - /// # fmt: off # type: ignore - /// ^^^^^^^^^^^^^^ - /// ``` - range: TextRange, - - kind: SuppressionKind, - - /// The ranges of the codes in the optional `[...]`. - /// `None` for comments that don't specify any code. - /// - /// ```py - /// # type: ignore[unresolved-reference, invalid-exception-caught] - /// ^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^ - /// ``` - codes: Option>, -} - -#[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 Formatter<'_>) -> fmt::Result { - match self { - SuppressionKind::TypeIgnore => f.write_str("type: ignore"), - SuppressionKind::Ty => f.write_str("ty: ignore"), - } - } -} - -#[derive(Debug, Eq, PartialEq, Clone, get_size2::GetSize)] -struct ParseError { - kind: ParseErrorKind, - - /// The position/range at which the parse error occurred. - range: TextRange, -} - -impl ParseError { - fn new(kind: ParseErrorKind, range: TextRange) -> Self { - Self { kind, range } - } -} - -impl fmt::Display for ParseError { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - self.kind.fmt(f) - } -} - -impl Error for ParseError {} - -#[derive(Debug, Eq, PartialEq, Clone, Error, get_size2::GetSize)] -enum ParseErrorKind { - /// The comment isn't a suppression comment. - #[error("not a suppression comment")] - NotASuppression, - - #[error("the comment doesn't start with a `#`")] - CommentWithoutHash, - - /// A valid suppression `type: ignore` but it misses a whitespaces after the `ignore` keyword. - /// - /// ```py - /// type: ignoree - /// ``` - #[error("no whitespace after `ignore`")] - NoWhitespaceAfterIgnore(SuppressionKind), - - /// Missing comma between two codes - #[error("expected a comma separating the rule codes")] - CodesMissingComma(SuppressionKind), - - /// `ty: ignore[*.*]` - #[error("expected a alphanumeric character or `-` or `_` as code")] - InvalidCode(SuppressionKind), - - /// `ty: ignore[a, b` - #[error("expected a closing bracket")] - CodesMissingClosingBracket(SuppressionKind), -} - -fn remove_comment_fix(suppression: &Suppression, source: &str) -> Fix { - let comment_end = suppression.comment_range.end(); - let comment_start = suppression.comment_range.start(); - let after_comment = &source[comment_end.to_usize()..]; - - if !after_comment.starts_with(['\n', '\r']) { - // For example: `# ty: ignore # fmt: off` - // Don't remove the trailing whitespace up to the `ty: ignore` comment - return Fix::safe_edit(Edit::range_deletion(suppression.comment_range)); - } - - // Remove any leading whitespace before the comment - // to avoid unnecessary trailing whitespace once the comment is removed - let before_comment = &source[..comment_start.to_usize()]; - - let mut leading_len = TextSize::default(); - - for c in before_comment.chars().rev() { - match c { - '\n' | '\r' => break, - c if c.is_whitespace() => leading_len += c.text_len(), - _ => break, - } - } - - Fix::safe_edit(Edit::range_deletion(TextRange::new( - comment_start - leading_len, - comment_end, - ))) -} - -#[cfg(test)] -mod tests { - use crate::suppression::{SuppressionComment, SuppressionParser}; - use insta::assert_debug_snapshot; - use ruff_text_size::{TextLen, TextRange}; - use std::fmt; - use std::fmt::Formatter; - - #[test] - fn type_ignore_no_codes() { - assert_debug_snapshot!( - SuppressionComments::new( - "# type: ignore", - ), - @r##" - [ - SuppressionComment { - text: "# type: ignore", - kind: TypeIgnore, - codes: [], - }, - ] - "## - ); - } - - #[test] - fn type_ignore_explanation() { - assert_debug_snapshot!( - SuppressionComments::new( - "# type: ignore I tried but couldn't figure out the proper type", - ), - @r##" - [ - SuppressionComment { - text: "# type: ignore I tried but couldn't figure out the proper type", - kind: TypeIgnore, - codes: [], - }, - ] - "## - ); - } - - #[test] - fn fmt_comment_before_type_ignore() { - assert_debug_snapshot!( - SuppressionComments::new( - "# fmt: off # type: ignore", - ), - @r##" - [ - SuppressionComment { - text: "# type: ignore", - kind: TypeIgnore, - codes: [], - }, - ] - "## - ); - } - - #[test] - fn type_ignore_before_fmt_off() { - assert_debug_snapshot!( - SuppressionComments::new( - "# type: ignore # fmt: off", - ), - @r##" - [ - SuppressionComment { - text: "# type: ignore ", - kind: TypeIgnore, - codes: [], - }, - ] - "## - ); - } - - #[test] - fn multiple_type_ignore_comments() { - assert_debug_snapshot!( - SuppressionComments::new( - "# type: ignore[a] # type: ignore[b]", - ), - @r##" - [ - SuppressionComment { - text: "# type: ignore[a] ", - kind: TypeIgnore, - codes: [ - "a", - ], - }, - SuppressionComment { - text: "# type: ignore[b]", - kind: TypeIgnore, - codes: [ - "b", - ], - }, - ] - "## - ); - } - - #[test] - fn invalid_type_ignore_valid_type_ignore() { - assert_debug_snapshot!( - SuppressionComments::new( - "# type: ignore[a # type: ignore[b]", - ), - @r##" - [ - SuppressionComment { - text: "# type: ignore[b]", - kind: TypeIgnore, - codes: [ - "b", - ], - }, - ] - "## - ); - } - - #[test] - fn valid_type_ignore_invalid_type_ignore() { - assert_debug_snapshot!( - SuppressionComments::new( - "# type: ignore[a] # type: ignoreeee", - ), - @r##" - [ - SuppressionComment { - text: "# type: ignore[a] ", - kind: TypeIgnore, - codes: [ - "a", - ], - }, - ] - "## - ); - } - - #[test] - fn type_ignore_multiple_codes() { - assert_debug_snapshot!( - SuppressionComments::new( - "# type: ignore[invalid-exception-raised, invalid-exception-caught]", - ), - @r##" - [ - SuppressionComment { - text: "# type: ignore[invalid-exception-raised, invalid-exception-caught]", - kind: TypeIgnore, - codes: [ - "invalid-exception-raised", - "invalid-exception-caught", - ], - }, - ] - "## - ); - } - - #[test] - fn type_ignore_single_code() { - assert_debug_snapshot!( - SuppressionComments::new("# type: ignore[invalid-exception-raised]",), - @r##" - [ - SuppressionComment { - text: "# type: ignore[invalid-exception-raised]", - kind: TypeIgnore, - codes: [ - "invalid-exception-raised", - ], - }, - ] - "## - ); - } - - struct SuppressionComments<'a> { - source: &'a str, - } - - impl<'a> SuppressionComments<'a> { - fn new(source: &'a str) -> Self { - Self { source } - } - } - - impl fmt::Debug for SuppressionComments<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut list = f.debug_list(); - - for comment in SuppressionParser::new( - self.source, - TextRange::new(0.into(), self.source.text_len()), - ) - .flatten() - { - list.entry(&comment.debug(self.source)); - } - - list.finish() - } - } - - impl SuppressionComment { - fn debug<'a>(&'a self, source: &'a str) -> DebugSuppressionComment<'a> { - DebugSuppressionComment { - source, - comment: self, - } - } - } - - struct DebugSuppressionComment<'a> { - source: &'a str, - comment: &'a SuppressionComment, - } - - impl fmt::Debug for DebugSuppressionComment<'_> { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - struct DebugCodes<'a> { - source: &'a str, - codes: &'a [TextRange], - } - - impl fmt::Debug for DebugCodes<'_> { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - let mut f = f.debug_list(); - - for code in self.codes { - f.entry(&&self.source[*code]); - } - - f.finish() - } - } - - f.debug_struct("SuppressionComment") - .field("text", &&self.source[self.comment.range]) - .field("kind", &self.comment.kind) - .field( - "codes", - &DebugCodes { - source: self.source, - codes: self.comment.codes.as_deref().unwrap_or_default(), - }, - ) - .finish() - } - } -} diff --git a/crates/ty_python_semantic/src/suppression/add_ignore.rs b/crates/ty_python_semantic/src/suppression/add_ignore.rs new file mode 100644 index 0000000000..f5cc787e78 --- /dev/null +++ b/crates/ty_python_semantic/src/suppression/add_ignore.rs @@ -0,0 +1,87 @@ +use ruff_db::files::File; +use ruff_db::parsed::parsed_module; +use ruff_db::source::source_text; +use ruff_diagnostics::{Edit, Fix}; +use ruff_python_ast::token::TokenKind; +use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; + +use crate::Db; +use crate::lint::LintId; +use crate::suppression::{SuppressionTarget, suppressions}; + +/// 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) + }) +} diff --git a/crates/ty_python_semantic/src/suppression/parser.rs b/crates/ty_python_semantic/src/suppression/parser.rs new file mode 100644 index 0000000000..d48550394b --- /dev/null +++ b/crates/ty_python_semantic/src/suppression/parser.rs @@ -0,0 +1,530 @@ +use std::error::Error; + +use crate::suppression::SuppressionKind; +use ruff_python_trivia::Cursor; +use ruff_text_size::{TextLen, TextRange, TextSize}; +use smallvec::{SmallVec, smallvec}; +use thiserror::Error; + +pub(super) struct SuppressionParser<'src> { + cursor: Cursor<'src>, + range: TextRange, +} + +impl<'src> SuppressionParser<'src> { + pub(super) 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>, ParseError> { + if !self.cursor.eat_char('[') { + return Ok(None); + } + + let mut codes: SmallVec<[TextRange; 2]> = smallvec![]; + + loop { + if self.cursor.is_eof() { + return self.syntax_error(ParseErrorKind::CodesMissingClosingBracket(kind)); + } + + self.eat_whitespace(); + + // `ty: ignore[]` or `ty: ignore[a,]` + if self.cursor.eat_char(']') { + break Ok(Some(codes)); + } + + let code_start = self.offset(); + if !self.eat_word() { + return self.syntax_error(ParseErrorKind::InvalidCode(kind)); + } + + codes.push(TextRange::new(code_start, self.offset())); + + self.eat_whitespace(); + + if !self.cursor.eat_char(',') { + if self.cursor.eat_char(']') { + break Ok(Some(codes)); + } + // `ty: ignore[a b] + return self.syntax_error(ParseErrorKind::CodesMissingComma(kind)); + } + } + } + + fn eat_whitespace(&mut self) -> bool { + if self.cursor.eat_if(char::is_whitespace) { + self.cursor.eat_while(char::is_whitespace); + true + } else { + false + } + } + + 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, '_' | '-' | ':')); + true + } else { + false + } + } + + fn syntax_error(&self, kind: ParseErrorKind) -> Result { + let len = if self.cursor.is_eof() { + TextSize::default() + } else { + self.cursor.first().text_len() + }; + + Err(ParseError::new(kind, TextRange::at(self.offset(), len))) + } + + fn offset(&self) -> TextSize { + self.range.start() + self.range.len() - self.cursor.text_len() + } +} + +impl Iterator for SuppressionParser<'_> { + type Item = Result; + + fn next(&mut self) -> Option { + if self.cursor.is_eof() { + return None; + } + + match self.parse_comment() { + Ok(result) => Some(Ok(result)), + Err(error) => { + self.cursor.eat_while(|c| c != '#'); + Some(Err(error)) + } + } + } +} + +/// A single parsed suppression comment. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(super) struct SuppressionComment { + /// The range of the suppression comment. + /// + /// This can be a sub-range of the comment token if the comment token contains multiple `#` tokens: + /// ```py + /// # fmt: off # type: ignore + /// ^^^^^^^^^^^^^^ + /// ``` + range: TextRange, + + kind: SuppressionKind, + + /// The ranges of the codes in the optional `[...]`. + /// `None` for comments that don't specify any code. + /// + /// ```py + /// # type: ignore[unresolved-reference, invalid-exception-caught] + /// ^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^ + /// ``` + codes: Option>, +} + +impl SuppressionComment { + pub(super) fn kind(&self) -> SuppressionKind { + self.kind + } + + pub(super) fn codes(&self) -> Option<&[TextRange]> { + self.codes.as_deref() + } + + pub(super) fn range(&self) -> TextRange { + self.range + } +} + +#[derive(Debug, Eq, PartialEq, Clone, get_size2::GetSize)] +pub(super) struct ParseError { + pub(super) kind: ParseErrorKind, + + /// The position/range at which the parse error occurred. + pub(super) range: TextRange, +} + +impl ParseError { + fn new(kind: ParseErrorKind, range: TextRange) -> Self { + Self { kind, range } + } +} + +impl std::fmt::Display for ParseError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.kind.fmt(f) + } +} + +impl Error for ParseError {} + +#[derive(Debug, Eq, PartialEq, Clone, Error, get_size2::GetSize)] +pub(super) enum ParseErrorKind { + /// The comment isn't a suppression comment. + #[error("not a suppression comment")] + NotASuppression, + + #[error("the comment doesn't start with a `#`")] + CommentWithoutHash, + + /// A valid suppression `type: ignore` but it misses a whitespaces after the `ignore` keyword. + /// + /// ```py + /// type: ignoree + /// ``` + #[error("no whitespace after `ignore`")] + NoWhitespaceAfterIgnore(SuppressionKind), + + /// Missing comma between two codes + #[error("expected a comma separating the rule codes")] + CodesMissingComma(SuppressionKind), + + /// `ty: ignore[*.*]` + #[error("expected a alphanumeric character or `-` or `_` as code")] + InvalidCode(SuppressionKind), + + /// `ty: ignore[a, b` + #[error("expected a closing bracket")] + CodesMissingClosingBracket(SuppressionKind), +} + +#[cfg(test)] +mod tests { + use crate::suppression::{SuppressionComment, SuppressionParser}; + use insta::assert_debug_snapshot; + use ruff_text_size::{TextLen, TextRange}; + use std::fmt; + use std::fmt::Formatter; + + #[test] + fn type_ignore_no_codes() { + assert_debug_snapshot!( + SuppressionComments::new( + "# type: ignore", + ), + @r##" + [ + SuppressionComment { + text: "# type: ignore", + kind: TypeIgnore, + codes: [], + }, + ] + "## + ); + } + + #[test] + fn type_ignore_explanation() { + assert_debug_snapshot!( + SuppressionComments::new( + "# type: ignore I tried but couldn't figure out the proper type", + ), + @r##" + [ + SuppressionComment { + text: "# type: ignore I tried but couldn't figure out the proper type", + kind: TypeIgnore, + codes: [], + }, + ] + "## + ); + } + + #[test] + fn fmt_comment_before_type_ignore() { + assert_debug_snapshot!( + SuppressionComments::new( + "# fmt: off # type: ignore", + ), + @r##" + [ + SuppressionComment { + text: "# type: ignore", + kind: TypeIgnore, + codes: [], + }, + ] + "## + ); + } + + #[test] + fn type_ignore_before_fmt_off() { + assert_debug_snapshot!( + SuppressionComments::new( + "# type: ignore # fmt: off", + ), + @r##" + [ + SuppressionComment { + text: "# type: ignore ", + kind: TypeIgnore, + codes: [], + }, + ] + "## + ); + } + + #[test] + fn multiple_type_ignore_comments() { + assert_debug_snapshot!( + SuppressionComments::new( + "# type: ignore[a] # type: ignore[b]", + ), + @r##" + [ + SuppressionComment { + text: "# type: ignore[a] ", + kind: TypeIgnore, + codes: [ + "a", + ], + }, + SuppressionComment { + text: "# type: ignore[b]", + kind: TypeIgnore, + codes: [ + "b", + ], + }, + ] + "## + ); + } + + #[test] + fn invalid_type_ignore_valid_type_ignore() { + assert_debug_snapshot!( + SuppressionComments::new( + "# type: ignore[a # type: ignore[b]", + ), + @r##" + [ + SuppressionComment { + text: "# type: ignore[b]", + kind: TypeIgnore, + codes: [ + "b", + ], + }, + ] + "## + ); + } + + #[test] + fn valid_type_ignore_invalid_type_ignore() { + assert_debug_snapshot!( + SuppressionComments::new( + "# type: ignore[a] # type: ignoreeee", + ), + @r##" + [ + SuppressionComment { + text: "# type: ignore[a] ", + kind: TypeIgnore, + codes: [ + "a", + ], + }, + ] + "## + ); + } + + #[test] + fn type_ignore_multiple_codes() { + assert_debug_snapshot!( + SuppressionComments::new( + "# type: ignore[invalid-exception-raised, invalid-exception-caught]", + ), + @r##" + [ + SuppressionComment { + text: "# type: ignore[invalid-exception-raised, invalid-exception-caught]", + kind: TypeIgnore, + codes: [ + "invalid-exception-raised", + "invalid-exception-caught", + ], + }, + ] + "## + ); + } + + #[test] + fn type_ignore_single_code() { + assert_debug_snapshot!( + SuppressionComments::new("# type: ignore[invalid-exception-raised]",), + @r##" + [ + SuppressionComment { + text: "# type: ignore[invalid-exception-raised]", + kind: TypeIgnore, + codes: [ + "invalid-exception-raised", + ], + }, + ] + "## + ); + } + + struct SuppressionComments<'a> { + source: &'a str, + } + + impl<'a> SuppressionComments<'a> { + fn new(source: &'a str) -> Self { + Self { source } + } + } + + impl fmt::Debug for SuppressionComments<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut list = f.debug_list(); + + for comment in SuppressionParser::new( + self.source, + TextRange::new(0.into(), self.source.text_len()), + ) + .flatten() + { + list.entry(&comment.debug(self.source)); + } + + list.finish() + } + } + + impl SuppressionComment { + fn debug<'a>(&'a self, source: &'a str) -> DebugSuppressionComment<'a> { + DebugSuppressionComment { + source, + comment: self, + } + } + } + + struct DebugSuppressionComment<'a> { + source: &'a str, + comment: &'a SuppressionComment, + } + + impl fmt::Debug for DebugSuppressionComment<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + struct DebugCodes<'a> { + source: &'a str, + codes: &'a [TextRange], + } + + impl fmt::Debug for DebugCodes<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + let mut f = f.debug_list(); + + for code in self.codes { + f.entry(&&self.source[*code]); + } + + f.finish() + } + } + + f.debug_struct("SuppressionComment") + .field("text", &&self.source[self.comment.range]) + .field("kind", &self.comment.kind) + .field( + "codes", + &DebugCodes { + source: self.source, + codes: self.comment.codes.as_deref().unwrap_or_default(), + }, + ) + .finish() + } + } +} diff --git a/crates/ty_python_semantic/src/suppression/unused.rs b/crates/ty_python_semantic/src/suppression/unused.rs new file mode 100644 index 0000000000..ff42a80d84 --- /dev/null +++ b/crates/ty_python_semantic/src/suppression/unused.rs @@ -0,0 +1,232 @@ +use ruff_db::source::source_text; +use ruff_diagnostics::{Edit, Fix}; +use ruff_text_size::{TextLen, TextRange, TextSize}; +use std::fmt::Write as _; + +use crate::lint::LintId; +use crate::suppression::{ + CheckSuppressionsContext, Suppression, SuppressionTarget, UNUSED_IGNORE_COMMENT, +}; + +/// 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. +pub(super) 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"); + } +} + +fn remove_comment_fix(suppression: &Suppression, source: &str) -> Fix { + let comment_end = suppression.comment_range.end(); + let comment_start = suppression.comment_range.start(); + let after_comment = &source[comment_end.to_usize()..]; + + if !after_comment.starts_with(['\n', '\r']) { + // For example: `# ty: ignore # fmt: off` + // Don't remove the trailing whitespace up to the `ty: ignore` comment + return Fix::safe_edit(Edit::range_deletion(suppression.comment_range)); + } + + // Remove any leading whitespace before the comment + // to avoid unnecessary trailing whitespace once the comment is removed + let before_comment = &source[..comment_start.to_usize()]; + + let mut leading_len = TextSize::default(); + + for c in before_comment.chars().rev() { + match c { + '\n' | '\r' => break, + c if c.is_whitespace() => leading_len += c.text_len(), + _ => break, + } + } + + Fix::safe_edit(Edit::range_deletion(TextRange::new( + comment_start - leading_len, + comment_end, + ))) +}