diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF028.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF028.py index cc9b09cfee8429..1b6cb4297908d5 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF028.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF028.py @@ -1,26 +1,3 @@ -# fmt: on -def fmt_off_used_earlier(): - if True: - a = 5 - with a: - # fmt: off - pass - elif False: - # fmt: off - pass - else: - pass - # fmt: off - if True: - # fmt: off - pass - # fmt: off - -# fmt: off - -# fmt: on - - def fmt_off_between_lists(): test_list = [ # fmt: off @@ -29,18 +6,23 @@ def fmt_off_between_lists(): 3, ] +# note: the second `fmt: skip`` should be OK +def fmt_skip_on_own_line(): + # fmt: skip + pass # fmt: skip + -@fmt_on_after_func +@fmt_skip_on_own_line # fmt: off @fmt_off_between_lists def fmt_off_between_decorators(): - # fmt: skip pass -def fmt_on_trailing(): - # fmt: off - val = 5 # fmt: on - pass +@fmt_off_between_decorators +# fmt: off +@fmt_off_between_lists +class FmtOffBetweenClassDecorators: + ... def fmt_off_in_else(): x = [1, 2, 3] @@ -51,7 +33,7 @@ def fmt_off_in_else(): print("done") while False: print("while") - # fmt: on + # fmt: off # fmt: off else: print("done") @@ -62,19 +44,7 @@ def fmt_off_in_else(): else: print("expected") -def dangling_fmt_off(): - pass - # fmt: off - -def dangling_fmt_off2(): - if True: - if True: - pass - # fmt: off - else: - pass - # fmt: off - # fmt: off - else: - pass +def fmt_on_trailing(): # fmt: off + val = 5 # fmt: on + pass # fmt: on diff --git a/crates/ruff_linter/src/checkers/ast/analyze/module.rs b/crates/ruff_linter/src/checkers/ast/analyze/module.rs index 62316f280a1de1..e80e052b55b3ca 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/module.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/module.rs @@ -9,7 +9,7 @@ pub(crate) fn module(suite: &Suite, checker: &mut Checker) { if checker.enabled(Rule::FStringDocstring) { flake8_bugbear::rules::f_string_docstring(checker, suite); } - if checker.enabled(Rule::IgnoredFormatterNOQA) { - ruff::rules::ignored_formatter_noqa(checker, suite); + if checker.enabled(Rule::InvalidFormatterSuppressionComment) { + ruff::rules::ignored_formatter_suppression_comment(checker, suite); } } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 51d9c19047a2bd..2e537621507bdf 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -944,7 +944,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "025") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryDictComprehensionForIterable), (Ruff, "026") => (RuleGroup::Preview, rules::ruff::rules::DefaultFactoryKwarg), (Ruff, "027") => (RuleGroup::Preview, rules::ruff::rules::MissingFStringSyntax), - (Ruff, "028") => (RuleGroup::Preview, rules::ruff::rules::IgnoredFormatterNOQA), + (Ruff, "028") => (RuleGroup::Preview, rules::ruff::rules::InvalidFormatterSuppressionComment), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml), #[cfg(feature = "test-rules")] diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 2780efc7fcc6e4..741ad8f0b50b6e 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -48,7 +48,7 @@ mod tests { #[test_case(Rule::DefaultFactoryKwarg, Path::new("RUF026.py"))] #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_0.py"))] #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_1.py"))] - #[test_case(Rule::IgnoredFormatterNOQA, Path::new("RUF028.py"))] + #[test_case(Rule::InvalidFormatterSuppressionComment, Path::new("RUF028.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/ruff/rules/ignored_formatter_noqa.rs b/crates/ruff_linter/src/rules/ruff/rules/invalid_formatter_suppression_comment.rs similarity index 58% rename from crates/ruff_linter/src/rules/ruff/rules/ignored_formatter_noqa.rs rename to crates/ruff_linter/src/rules/ruff/rules/invalid_formatter_suppression_comment.rs index 486994454c29b3..592ee0505906d0 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/ignored_formatter_noqa.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/invalid_formatter_suppression_comment.rs @@ -1,18 +1,20 @@ -use std::{collections::BTreeMap, fmt::Display}; +use std::fmt::Display; +use ast::{StmtClassDef, StmtFunctionDef}; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast, AnyNodeRef}; use ruff_python_trivia::{indentation_at_offset, SuppressionKind}; use ruff_source_file::Locator; -use ruff_text_size::{Ranged, TextLen}; +use ruff_text_size::{Ranged, TextLen, TextRange}; +use smallvec::SmallVec; use crate::checkers::ast::Checker; use crate::fix::edits::delete_comment; use super::suppression_comment_visitor::{ - own_line_comment_indentation, CaptureSuppressionComment, SuppressionCommentData, - SuppressionCommentVisitor, + own_line_comment_indentation, CaptureSuppressionComment, SuppressionComment, + SuppressionCommentData, SuppressionCommentVisitor, }; /// ## What it does @@ -47,15 +49,15 @@ use super::suppression_comment_visitor::{ /// # yapf: enable /// ``` #[violation] -pub struct IgnoredFormatterNOQA { +pub struct InvalidFormatterSuppressionComment { reason: IgnoredReason, } -impl AlwaysFixableViolation for IgnoredFormatterNOQA { +impl AlwaysFixableViolation for InvalidFormatterSuppressionComment { #[derive_message_formats] fn message(&self) -> String { format!( - "This suppression comment will be ignored by the formatter because {}", + "This suppression comment is invalid because {}", self.reason ) } @@ -66,35 +68,53 @@ impl AlwaysFixableViolation for IgnoredFormatterNOQA { } /// RUF028 -pub(crate) fn ignored_formatter_noqa(checker: &mut Checker, suite: &ast::Suite) { +pub(crate) fn ignored_formatter_suppression_comment(checker: &mut Checker, suite: &ast::Suite) { let indexer = checker.indexer(); let locator = checker.locator(); - let comment_ranges = indexer.comment_ranges(); + let comment_ranges: SmallVec<[SuppressionComment; 8]> = indexer + .comment_ranges() + .into_iter() + .filter_map(|range| { + Some(SuppressionComment { + range: *range, + kind: SuppressionKind::from_comment(locator.slice(range))?, + }) + }) + .collect(); + + if comment_ranges.is_empty() { + return; + } let mut comments = UselessSuppressionComments::new(locator); - let visitor = SuppressionCommentVisitor::new(comment_ranges, &mut comments, checker.locator()); + let visitor = SuppressionCommentVisitor::new( + comment_ranges.into_iter(), + &mut comments, + checker.locator(), + ); visitor.visit(suite); - for (comment, reason) in comments.ignored_comments() { + comments.sort(); + + for (range, reason) in comments.ignored_comments() { checker.diagnostics.push( - Diagnostic::new(IgnoredFormatterNOQA { reason }, comment.range).with_fix( - Fix::unsafe_edit(delete_comment(comment.range, checker.locator())), - ), + Diagnostic::new(InvalidFormatterSuppressionComment { reason }, range) + .with_fix(Fix::unsafe_edit(delete_comment(range, checker.locator()))), ); } } struct UselessSuppressionComments<'src, 'loc> { - captured: BTreeMap, IgnoredReason>, + captured: Vec<(TextRange, IgnoredReason)>, locator: &'loc Locator<'src>, } impl<'src, 'loc> UselessSuppressionComments<'src, 'loc> { fn new(locator: &'loc Locator<'src>) -> Self { Self { - captured: BTreeMap::default(), + captured: vec![], locator, } } @@ -103,7 +123,7 @@ impl<'src, 'loc> UselessSuppressionComments<'src, 'loc> { fn check_suppression_comment( &self, comment: &SuppressionCommentData, - ) -> Result, IgnoredReason> { + ) -> Result<(), IgnoredReason> { // check if the comment is inside of an expression. if comment .enclosing @@ -115,34 +135,50 @@ impl<'src, 'loc> UselessSuppressionComments<'src, 'loc> { // check if a skip comment is at the end of a line if comment.kind == SuppressionKind::Skip && !comment.line_position.is_end_of_line() { - return Err(IgnoredReason::SkipHasToBeTrailing); + if !comment.line_position.is_end_of_line() { + return Err(IgnoredReason::SkipHasToBeTrailing); + } } - if comment.kind == SuppressionKind::Off && comment.line_position.is_own_line() { - // check for a previous `fmt: off` - if comment.previous_state == Some(SuppressionKind::Off) { - return Err(IgnoredReason::FmtOffUsedEarlier); + if comment.kind == SuppressionKind::Off || comment.kind == SuppressionKind::On { + if let Some( + AnyNodeRef::StmtClassDef(StmtClassDef { + name, + decorator_list, + .. + }) + | AnyNodeRef::StmtFunctionDef(StmtFunctionDef { + name, + decorator_list, + .. + }), + ) = comment.enclosing + { + if comment.line_position.is_own_line() && comment.range.start() < name.start() { + if let Some(decorator) = decorator_list.last() { + if decorator.start() > comment.range.end() { + return Err(IgnoredReason::BetweenDecorators); + } + } + } } - let Some(following) = comment.following else { - return Err(IgnoredReason::NoCodeSuppressed); - }; - if let Some(enclosing) = comment.enclosing { - // check if this comment is dangling (in other words, in a block with nothing following it) + } - // check if this comment is before an alternative body (for example: an `else` or `elif`) - if let Some(preceding) = comment.preceding { - if is_first_statement_in_alternate_body(following, enclosing) { - // check indentation - let comment_indentation = - own_line_comment_indentation(preceding, comment.range, self.locator); + if comment.kind == SuppressionKind::Off && comment.line_position.is_own_line() { + if let (Some(enclosing), Some(preceding), Some(following)) = + (comment.enclosing, comment.preceding, comment.following) + { + if is_first_statement_in_alternate_body(following, enclosing) { + // check indentation + let comment_indentation = + own_line_comment_indentation(preceding, comment.range, self.locator); - let preceding_indentation = - indentation_at_offset(preceding.start(), self.locator) - .unwrap_or_default() - .text_len(); - if comment_indentation <= preceding_indentation { - return Err(IgnoredReason::FmtOffAboveBlock); - } + let preceding_indentation = + indentation_at_offset(preceding.start(), self.locator) + .unwrap_or_default() + .text_len(); + if comment_indentation != preceding_indentation { + return Err(IgnoredReason::FmtOffAboveBlock); } } } @@ -153,47 +189,26 @@ impl<'src, 'loc> UselessSuppressionComments<'src, 'loc> { if !comment.line_position.is_own_line() { return Err(IgnoredReason::FmtOnCannotBeTrailing); } - - // If the comment turns on formatting, we need to check if another - // comment turned formatting off within the same scope. - match comment.previous_state { - None | Some(SuppressionKind::On) => return Err(IgnoredReason::NoFmtOff), - _ => {} - } } - if comment.kind == SuppressionKind::Off || comment.kind == SuppressionKind::On { - if let Some(AnyNodeRef::StmtClassDef(class_def)) = comment.enclosing { - if comment.line_position.is_own_line() && comment.start() < class_def.name.start() { - if let Some(decorator) = class_def.decorator_list.last() { - if decorator.end() < comment.start() { - return Err(IgnoredReason::BetweenDecorators); - } - } - } - } + Ok(()) + } - // at this point, any comment being handled should be considered 'valid'. - // on/off suppression comments should be added to the scope - return Ok(Some(comment.kind)); - } - Ok(None) + fn sort(&mut self) { + self.captured.sort_by_key(|(t, _)| t.start()); } - fn ignored_comments( - &self, - ) -> impl Iterator, IgnoredReason)> { - self.captured.iter().map(|(c, r)| (c, *r)) + fn ignored_comments(&self) -> impl Iterator + '_ { + self.captured.iter().map(|(r, i)| (*r, *i)) } } impl<'src, 'loc> CaptureSuppressionComment<'src> for UselessSuppressionComments<'src, 'loc> { - fn capture(&mut self, comment: SuppressionCommentData<'src>) -> Option { + fn capture(&mut self, comment: SuppressionCommentData<'src>) { match self.check_suppression_comment(&comment) { - Ok(kind) => kind, + Ok(()) => {} Err(reason) => { - self.captured.insert(comment, reason); - None + self.captured.push((comment.range, reason)); } } } @@ -236,33 +251,27 @@ where #[derive(Clone, Copy, Debug, PartialEq, Eq)] enum IgnoredReason { InsideExpression, - FmtOffUsedEarlier, - NoFmtOff, - NoCodeSuppressed, BetweenDecorators, - FmtOnCannotBeTrailing, SkipHasToBeTrailing, + FmtOnCannotBeTrailing, FmtOffAboveBlock, } impl Display for IgnoredReason { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::InsideExpression => write!(f, "it's inside an expression"), - Self::FmtOffUsedEarlier => write!(f, "formatting is already disabled here"), - Self::NoFmtOff => write!(f, "formatting is already enabled here"), - Self::NoCodeSuppressed => write!(f, "it does not suppress formatting for any code"), + Self::InsideExpression => write!(f, "it cannot be inside an expression"), Self::BetweenDecorators => { write!(f, "it cannot be between decorators") } Self::SkipHasToBeTrailing => { - write!(f, "it has to be at the end of a line") + write!(f, "it cannot be on its own line") } Self::FmtOnCannotBeTrailing => { write!(f, "it cannot be at the end of a line") } Self::FmtOffAboveBlock => { - write!(f, "it suppresses formatting for an ambiguous region") + write!(f, "it cannot suppress formatting for an ambiguous region") } } } diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 1896e6082a4c42..d4461f1f3b6aff 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -5,8 +5,8 @@ pub(crate) use collection_literal_concatenation::*; pub(crate) use default_factory_kwarg::*; pub(crate) use explicit_f_string_type_conversion::*; pub(crate) use function_call_in_dataclass_default::*; -pub(crate) use ignored_formatter_noqa::*; pub(crate) use implicit_optional::*; +pub(crate) use invalid_formatter_suppression_comment::*; pub(crate) use invalid_index_type::*; pub(crate) use invalid_pyproject_toml::*; pub(crate) use missing_fstring_syntax::*; @@ -36,8 +36,8 @@ mod default_factory_kwarg; mod explicit_f_string_type_conversion; mod function_call_in_dataclass_default; mod helpers; -mod ignored_formatter_noqa; mod implicit_optional; +mod invalid_formatter_suppression_comment; mod invalid_index_type; mod invalid_pyproject_toml; mod missing_fstring_syntax; diff --git a/crates/ruff_linter/src/rules/ruff/rules/suppression_comment_visitor.rs b/crates/ruff_linter/src/rules/ruff/rules/suppression_comment_visitor.rs index 294c4e034ea12e..c994900347ee71 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/suppression_comment_visitor.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/suppression_comment_visitor.rs @@ -5,57 +5,47 @@ use ruff_python_ast::{ AnyNodeRef, Suite, }; use ruff_python_trivia::{ - indentation_at_offset, CommentLinePosition, CommentRanges, SimpleTokenKind, SimpleTokenizer, - SuppressionKind, + indentation_at_offset, CommentLinePosition, SimpleTokenKind, SimpleTokenizer, SuppressionKind, }; use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; #[derive(Clone, Copy, Debug)] -struct SuppressionComment { - range: TextRange, - kind: SuppressionKind, -} - -fn suppression_comments<'src>( - ranges: &'src CommentRanges, - locator: &'src Locator<'src>, -) -> Box + 'src> { - Box::new(ranges.iter().filter_map(|range| { - Some(SuppressionComment { - range: *range, - kind: SuppressionKind::from_comment(locator.slice(range))?, - }) - })) +pub(super) struct SuppressionComment { + pub(super) range: TextRange, + pub(super) kind: SuppressionKind, } /// Visitor that captures AST data for suppression comments. This uses a similar approach /// to `CommentsVisitor` in the formatter crate. -pub(super) struct SuppressionCommentVisitor<'src, 'builder> { - comments: Peekable + 'src>>, +/// The visitor has +pub(super) struct SuppressionCommentVisitor< + 'src, + 'builder, + I: Iterator + 'src, +> { + comments: Peekable, parents: Vec>, preceding_node: Option>, - // A stack of comment states in scope at the current visited node. - // The last comment state in the list is the top of the stack, - // and is essentially the 'current' state. - comments_in_scope: Vec<(AnyNodeRef<'src>, SuppressionKind)>, builder: &'builder mut (dyn CaptureSuppressionComment<'src> + 'src), locator: &'src Locator<'src>, } -impl<'src, 'builder> SuppressionCommentVisitor<'src, 'builder> { +impl<'src, 'builder, I> SuppressionCommentVisitor<'src, 'builder, I> +where + I: Iterator + 'src, +{ pub(super) fn new( - comment_ranges: &'src CommentRanges, + comment_ranges: I, builder: &'builder mut (dyn CaptureSuppressionComment<'src> + 'src), locator: &'src Locator<'src>, ) -> Self { Self { - comments: suppression_comments(comment_ranges, locator).peekable(), + comments: comment_ranges.peekable(), parents: Vec::default(), preceding_node: Option::default(), - comments_in_scope: Vec::default(), builder, locator, } @@ -72,7 +62,10 @@ impl<'src, 'builder> SuppressionCommentVisitor<'src, 'builder> { } } -impl<'ast> PreorderVisitor<'ast> for SuppressionCommentVisitor<'ast, '_> { +impl<'ast, I> PreorderVisitor<'ast> for SuppressionCommentVisitor<'ast, '_, I> +where + I: Iterator + 'ast, +{ fn enter_node(&mut self, node: AnyNodeRef<'ast>) -> TraversalSignal { let node_range = node.range(); @@ -89,21 +82,16 @@ impl<'ast> PreorderVisitor<'ast> for SuppressionCommentVisitor<'ast, '_> { let line_position = CommentLinePosition::for_range(range, self.locator.contents()); - let previous_state = self.comments_in_scope.last().map(|(_, s)| s).copied(); - let data = SuppressionCommentData { enclosing: enclosing_node, preceding: self.preceding_node, following: Some(node), - previous_state, line_position, kind, range, }; - if let Some(kind) = self.builder.capture(data) { - self.comments_in_scope.push((node, kind)); - } + self.builder.capture(data); self.comments.next(); } @@ -127,49 +115,52 @@ impl<'ast> PreorderVisitor<'ast> for SuppressionCommentVisitor<'ast, '_> { while let Some(SuppressionComment { range, kind }) = self.comments.peek().copied() { let line_position = CommentLinePosition::for_range(range, self.locator.contents()); if range.start() >= node_end { - if !line_position.is_own_line() { - break; - } - let Some(preceding) = self.preceding_node else { - break; - }; - // check indent of comment against the minimum indentation of a hypothetical body - let comment_indent = own_line_comment_indentation(preceding, range, self.locator); - let min_indentation = indentation_at_offset(node.start(), self.locator) - .unwrap_or_default() - .text_len(); - if comment_indent <= min_indentation { - break; + if line_position.is_own_line() { + if let Some(token) = + SimpleTokenizer::starts_at(node_end, self.locator.contents()) + .skip_trivia() + .next() + { + if token.end() <= range.end() { + break; + } + } + let comment_indent = indentation_at_offset(range.start(), self.locator) + .unwrap_or_default() + .len(); + let node_indent = indentation_at_offset(node.start(), self.locator) + .unwrap_or_default() + .len(); + if node_indent >= comment_indent { + break; + } + } else { + if self.locator.line_start(range.start()) + != self.locator.line_start(node.start()) + { + break; + } } + eprintln!( + "Allowing dangling comment {} for \"{}\"", + self.locator.slice(range), + self.locator.slice(node.range()) + ); } - let previous_state = self.comments_in_scope.last().map(|(_, s)| s).copied(); - let data = SuppressionCommentData { enclosing: Some(node), preceding: self.preceding_node, following: None, - previous_state, line_position, kind, range, }; - if let Some(kind) = self.builder.capture(data) { - self.comments_in_scope.push((node, kind)); - } + self.builder.capture(data); self.comments.next(); } - // remove comments that are about to become out of scope - for index in (0..self.comments_in_scope.len()).rev() { - if AnyNodeRef::ptr_eq(self.comments_in_scope[index].0, node) { - self.comments_in_scope.pop(); - } else { - break; - } - } - self.preceding_node = Some(node); } fn visit_body(&mut self, body: &'ast [ruff_python_ast::Stmt]) { @@ -201,43 +192,15 @@ pub(super) struct SuppressionCommentData<'src> { pub(super) enclosing: Option>, pub(super) preceding: Option>, pub(super) following: Option>, - pub(super) previous_state: Option, + pub(super) line_position: CommentLinePosition, pub(super) kind: SuppressionKind, pub(super) range: TextRange, } -impl<'src> PartialEq for SuppressionCommentData<'src> { - fn eq(&self, other: &Self) -> bool { - self.range.start().eq(&other.range.start()) - } -} - -impl<'src> Eq for SuppressionCommentData<'src> {} - -impl<'src> Ord for SuppressionCommentData<'src> { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.range.start().cmp(&other.range.start()) - } -} - -impl<'src> PartialOrd for SuppressionCommentData<'src> { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl<'src> Ranged for SuppressionCommentData<'src> { - fn range(&self) -> TextRange { - self.range - } -} - pub(super) trait CaptureSuppressionComment<'src> { /// This is the entrypoint for the capturer to analyze the next comment. - /// Returning a `Some` value will update the suppression state for future comments. - #[must_use] - fn capture(&mut self, comment: SuppressionCommentData<'src>) -> Option; + fn capture(&mut self, comment: SuppressionCommentData<'src>); } /// Determine the indentation level of an own-line comment, defined as the minimum indentation of diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF028_RUF028.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF028_RUF028.py.snap index aba9d73ae0acd6..81c2547dc220ba 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF028_RUF028.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF028_RUF028.py.snap @@ -1,332 +1,174 @@ --- source: crates/ruff_linter/src/rules/ruff/mod.rs --- -RUF028.py:1:1: RUF028 [*] This comment will be ignored by the formatter because formatting is already enabled here +RUF028.py:3:9: RUF028 [*] This suppression comment is invalid because it cannot be inside an expression | -1 | # fmt: on - | ^^^^^^^^^ RUF028 -2 | def fmt_off_used_earlier(): -3 | if True: +1 | def fmt_off_between_lists(): +2 | test_list = [ +3 | # fmt: off + | ^^^^^^^^^^ RUF028 +4 | 1, +5 | 2, | = help: Remove this comment ℹ Unsafe fix -1 |-# fmt: on -2 1 | def fmt_off_used_earlier(): -3 2 | if True: -4 3 | a = 5 +1 1 | def fmt_off_between_lists(): +2 2 | test_list = [ +3 |- # fmt: off +4 3 | 1, +5 4 | 2, +6 5 | 3, -RUF028.py:15:9: RUF028 [*] This comment will be ignored by the formatter because formatting is already disabled here +RUF028.py:11:5: RUF028 [*] This suppression comment is invalid because it cannot be on its own line | -13 | # fmt: off -14 | if True: -15 | # fmt: off - | ^^^^^^^^^^ RUF028 -16 | pass -17 | # fmt: off - | - = help: Remove this comment - -ℹ Unsafe fix -12 12 | pass -13 13 | # fmt: off -14 14 | if True: -15 |- # fmt: off -16 15 | pass -17 16 | # fmt: off -18 17 | - -RUF028.py:17:5: RUF028 [*] This comment will be ignored by the formatter because it does not suppress formatting for any code - | -15 | # fmt: off -16 | pass -17 | # fmt: off + 9 | # note: the second `fmt: skip`` should be OK +10 | def fmt_skip_on_own_line(): +11 | # fmt: skip | ^^^^^^^^^^^ RUF028 -18 | -19 | # fmt: off - | - = help: Remove this comment - -ℹ Unsafe fix -14 14 | if True: -15 15 | # fmt: off -16 16 | pass -17 |- # fmt: off -18 17 | -19 18 | # fmt: off -20 19 | - -RUF028.py:26:9: RUF028 [*] This comment will be ignored by the formatter because it's inside an expression - | -24 | def fmt_off_between_lists(): -25 | test_list = [ -26 | # fmt: off - | ^^^^^^^^^^ RUF028 -27 | 1, -28 | 2, - | - = help: Remove this comment - -ℹ Unsafe fix -23 23 | -24 24 | def fmt_off_between_lists(): -25 25 | test_list = [ -26 |- # fmt: off -27 26 | 1, -28 27 | 2, -29 28 | 3, - -RUF028.py:37:5: RUF028 [*] This comment will be ignored by the formatter because it has to be at the end of a line - | -35 | @fmt_off_between_lists -36 | def fmt_off_between_decorators(): -37 | # fmt: skip - | ^^^^^^^^^^^ RUF028 -38 | pass - | - = help: Remove this comment - -ℹ Unsafe fix -34 34 | # fmt: off -35 35 | @fmt_off_between_lists -36 36 | def fmt_off_between_decorators(): -37 |- # fmt: skip -38 37 | pass -39 38 | -40 39 | def fmt_on_trailing(): - -RUF028.py:41:5: RUF028 [*] This comment will be ignored by the formatter because it does not suppress formatting for any code - | -40 | def fmt_on_trailing(): -41 | # fmt: off - | ^^^^^^^^^^ RUF028 -42 | val = 5 # fmt: on -43 | pass - | - = help: Remove this comment - -ℹ Unsafe fix -38 38 | pass -39 39 | -40 40 | def fmt_on_trailing(): -41 |- # fmt: off -42 41 | val = 5 # fmt: on -43 42 | pass -44 43 | - -RUF028.py:42:13: RUF028 [*] This comment will be ignored by the formatter because it cannot be at the end of a line - | -40 | def fmt_on_trailing(): -41 | # fmt: off -42 | val = 5 # fmt: on - | ^^^^^^^^^ RUF028 -43 | pass +12 | pass # fmt: skip | = help: Remove this comment ℹ Unsafe fix -39 39 | -40 40 | def fmt_on_trailing(): -41 41 | # fmt: off -42 |- val = 5 # fmt: on - 42 |+ val = 5 -43 43 | pass -44 44 | -45 45 | def fmt_off_in_else(): +8 8 | +9 9 | # note: the second `fmt: skip`` should be OK +10 10 | def fmt_skip_on_own_line(): +11 |- # fmt: skip +12 11 | pass # fmt: skip +13 12 | +14 13 | -RUF028.py:49:5: RUF028 [*] This comment will be ignored by the formatter because it suppresses formatting for an ambiguous region +RUF028.py:16:1: RUF028 [*] This suppression comment is invalid because it cannot be between decorators | -47 | for val in x: -48 | print(x) -49 | # fmt: off - | ^^^^^^^^^^ RUF028 -50 | else: -51 | print("done") +15 | @fmt_skip_on_own_line +16 | # fmt: off + | ^^^^^^^^^^ RUF028 +17 | @fmt_off_between_lists +18 | def fmt_off_between_decorators(): | = help: Remove this comment ℹ Unsafe fix -46 46 | x = [1, 2, 3] -47 47 | for val in x: -48 48 | print(x) -49 |- # fmt: off -50 49 | else: -51 50 | print("done") -52 51 | while False: +13 13 | +14 14 | +15 15 | @fmt_skip_on_own_line +16 |-# fmt: off +17 16 | @fmt_off_between_lists +18 17 | def fmt_off_between_decorators(): +19 18 | pass -RUF028.py:54:5: RUF028 [*] This comment will be ignored by the formatter because formatting is already enabled here +RUF028.py:22:1: RUF028 [*] This suppression comment is invalid because it cannot be between decorators | -52 | while False: -53 | print("while") -54 | # fmt: on - | ^^^^^^^^^ RUF028 -55 | # fmt: off -56 | else: +21 | @fmt_off_between_decorators +22 | # fmt: off + | ^^^^^^^^^^ RUF028 +23 | @fmt_off_between_lists +24 | class FmtOffBetweenClassDecorators: | = help: Remove this comment ℹ Unsafe fix -51 51 | print("done") -52 52 | while False: -53 53 | print("while") -54 |- # fmt: on -55 54 | # fmt: off -56 55 | else: -57 56 | print("done") +19 19 | pass +20 20 | +21 21 | @fmt_off_between_decorators +22 |-# fmt: off +23 22 | @fmt_off_between_lists +24 23 | class FmtOffBetweenClassDecorators: +25 24 | ... -RUF028.py:55:5: RUF028 [*] This comment will be ignored by the formatter because it suppresses formatting for an ambiguous region +RUF028.py:31:5: RUF028 [*] This suppression comment is invalid because it cannot suppress formatting for an ambiguous region | -53 | print("while") -54 | # fmt: on -55 | # fmt: off +29 | for val in x: +30 | print(x) +31 | # fmt: off | ^^^^^^^^^^ RUF028 -56 | else: -57 | print("done") +32 | else: +33 | print("done") | = help: Remove this comment ℹ Unsafe fix -52 52 | while False: -53 53 | print("while") -54 54 | # fmt: on -55 |- # fmt: off -56 55 | else: -57 56 | print("done") -58 57 | if len(x) > 3: +28 28 | x = [1, 2, 3] +29 29 | for val in x: +30 30 | print(x) +31 |- # fmt: off +32 31 | else: +33 32 | print("done") +34 33 | while False: -RUF028.py:60:5: RUF028 [*] This comment will be ignored by the formatter because formatting is already enabled here +RUF028.py:37:5: RUF028 [*] This suppression comment is invalid because it cannot suppress formatting for an ambiguous region | -58 | if len(x) > 3: -59 | print("huh?") -60 | # fmt: on - | ^^^^^^^^^ RUF028 -61 | # fmt: off -62 | else: - | - = help: Remove this comment - -ℹ Unsafe fix -57 57 | print("done") -58 58 | if len(x) > 3: -59 59 | print("huh?") -60 |- # fmt: on -61 60 | # fmt: off -62 61 | else: -63 62 | print("expected") - -RUF028.py:61:5: RUF028 [*] This comment will be ignored by the formatter because it suppresses formatting for an ambiguous region - | -59 | print("huh?") -60 | # fmt: on -61 | # fmt: off +35 | print("while") +36 | # fmt: off +37 | # fmt: off | ^^^^^^^^^^ RUF028 -62 | else: -63 | print("expected") +38 | else: +39 | print("done") | = help: Remove this comment ℹ Unsafe fix -58 58 | if len(x) > 3: -59 59 | print("huh?") -60 60 | # fmt: on -61 |- # fmt: off -62 61 | else: -63 62 | print("expected") -64 63 | +34 34 | while False: +35 35 | print("while") +36 36 | # fmt: off +37 |- # fmt: off +38 37 | else: +39 38 | print("done") +40 39 | if len(x) > 3: -RUF028.py:67:5: RUF028 [*] This comment will be ignored by the formatter because it does not suppress formatting for any code +RUF028.py:43:5: RUF028 [*] This suppression comment is invalid because it cannot suppress formatting for an ambiguous region | -65 | def dangling_fmt_off(): -66 | pass -67 | # fmt: off +41 | print("huh?") +42 | # fmt: on +43 | # fmt: off | ^^^^^^^^^^ RUF028 -68 | -69 | def dangling_fmt_off2(): +44 | else: +45 | print("expected") | = help: Remove this comment ℹ Unsafe fix -64 64 | -65 65 | def dangling_fmt_off(): -66 66 | pass -67 |- # fmt: off -68 67 | -69 68 | def dangling_fmt_off2(): -70 69 | if True: +40 40 | if len(x) > 3: +41 41 | print("huh?") +42 42 | # fmt: on +43 |- # fmt: off +44 43 | else: +45 44 | print("expected") +46 45 | -RUF028.py:73:13: RUF028 [*] This comment will be ignored by the formatter because it does not suppress formatting for any code - | -71 | if True: -72 | pass -73 | # fmt: off - | ^^^^^^^^^^ RUF028 -74 | else: -75 | pass +RUF028.py:49:13: RUF028 [*] This suppression comment is invalid because it cannot be at the end of a line | - = help: Remove this comment - -ℹ Unsafe fix -70 70 | if True: -71 71 | if True: -72 72 | pass -73 |- # fmt: off -74 73 | else: -75 74 | pass -76 75 | # fmt: off - -RUF028.py:76:13: RUF028 [*] This comment will be ignored by the formatter because it does not suppress formatting for any code - | -74 | else: -75 | pass -76 | # fmt: off - | ^^^^^^^^^^ RUF028 -77 | # fmt: off -78 | else: - | - = help: Remove this comment - -ℹ Unsafe fix -73 73 | # fmt: off -74 74 | else: -75 75 | pass -76 |- # fmt: off -77 76 | # fmt: off -78 77 | else: -79 78 | pass - -RUF028.py:77:9: RUF028 [*] This comment will be ignored by the formatter because it does not suppress formatting for any code - | -75 | pass -76 | # fmt: off -77 | # fmt: off - | ^^^^^^^^^^ RUF028 -78 | else: -79 | pass +47 | def fmt_on_trailing(): +48 | # fmt: off +49 | val = 5 # fmt: on + | ^^^^^^^^^ RUF028 +50 | pass # fmt: on | = help: Remove this comment ℹ Unsafe fix -74 74 | else: -75 75 | pass -76 76 | # fmt: off -77 |- # fmt: off -78 77 | else: -79 78 | pass -80 79 | # fmt: off +46 46 | +47 47 | def fmt_on_trailing(): +48 48 | # fmt: off +49 |- val = 5 # fmt: on + 49 |+ val = 5 +50 50 | pass # fmt: on -RUF028.py:80:5: RUF028 [*] This comment will be ignored by the formatter because it does not suppress formatting for any code +RUF028.py:50:10: RUF028 [*] This suppression comment is invalid because it cannot be at the end of a line | -78 | else: -79 | pass -80 | # fmt: off - | ^^^^^^^^^^ RUF028 +48 | # fmt: off +49 | val = 5 # fmt: on +50 | pass # fmt: on + | ^^^^^^^^^ RUF028 | = help: Remove this comment ℹ Unsafe fix -77 77 | # fmt: off -78 78 | else: -79 79 | pass -80 |- # fmt: off +47 47 | def fmt_on_trailing(): +48 48 | # fmt: off +49 49 | val = 5 # fmt: on +50 |- pass # fmt: on + 50 |+ pass diff --git a/crates/ruff_python_formatter/src/comments/mod.rs b/crates/ruff_python_formatter/src/comments/mod.rs index 27127f6ae5a3e0..9ca2134f700d95 100644 --- a/crates/ruff_python_formatter/src/comments/mod.rs +++ b/crates/ruff_python_formatter/src/comments/mod.rs @@ -468,7 +468,7 @@ pub(crate) fn has_skip_comment(trailing_comments: &[SourceComment], source: &str trailing_comments.iter().any(|comment| { comment.line_position().is_end_of_line() && matches!( - SuppressionKind::from_comment(comment.slice().text(SourceCode::new(source))), + SuppressionKind::from_comment(comment.text(source)), Some(SuppressionKind::Skip | SuppressionKind::Off) ) }) diff --git a/crates/ruff_python_trivia/src/comments.rs b/crates/ruff_python_trivia/src/comments.rs index a0b35d330102aa..b8d1a58fd00ed3 100644 --- a/crates/ruff_python_trivia/src/comments.rs +++ b/crates/ruff_python_trivia/src/comments.rs @@ -102,8 +102,8 @@ impl CommentLinePosition { matches!(self, Self::EndOfLine) } - /// Finds the line position of a comment given a range with - /// + /// Finds the line position of a comment given a range over a valid + /// comment. pub fn for_range(comment_range: TextRange, source_code: &str) -> Self { let before = &source_code[TextRange::up_to(comment_range.start())];