diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF028.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF028.py new file mode 100644 index 0000000000000..47100c2b3ef90 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF028.py @@ -0,0 +1,63 @@ +def fmt_off_between_lists(): + test_list = [ + # fmt: off + 1, + 2, + 3, + ] + + +# note: the second `fmt: skip`` should be OK +def fmt_skip_on_own_line(): + # fmt: skip + pass # fmt: skip + + +@fmt_skip_on_own_line +# fmt: off +@fmt_off_between_lists +def fmt_off_between_decorators(): + pass + + +@fmt_off_between_decorators +# fmt: off +class FmtOffBetweenClassDecorators: + ... + + +def fmt_off_in_else(): + x = [1, 2, 3] + for val in x: + print(x) + # fmt: off + else: + print("done") + while False: + print("while") + # fmt: off + # fmt: off + else: + print("done") + if len(x) > 3: + print("huh?") + # fmt: on + # fmt: off + else: + print("expected") + + +class Test: + @classmethod + # fmt: off + def cls_method_a( + # fmt: off + cls, + ) -> None: # noqa: test # fmt: skip + 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 139d6c5294783..e80e052b55b3c 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/module.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/module.rs @@ -2,11 +2,14 @@ use ruff_python_ast::Suite; use crate::checkers::ast::Checker; use crate::codes::Rule; -use crate::rules::flake8_bugbear; +use crate::rules::{flake8_bugbear, ruff}; /// Run lint rules over a module. 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::InvalidFormatterSuppressionComment) { + ruff::rules::ignored_formatter_suppression_comment(checker, suite); + } } diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index d5d4276abb27a..8e01d671ce601 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -3,12 +3,13 @@ use std::path::Path; use itertools::Itertools; -use ruff_text_size::{Ranged, TextLen, TextRange}; +use ruff_text_size::Ranged; use ruff_diagnostics::{Diagnostic, Edit, Fix}; -use ruff_python_trivia::{CommentRanges, PythonWhitespace}; +use ruff_python_trivia::CommentRanges; use ruff_source_file::Locator; +use crate::fix::edits::delete_comment; use crate::noqa; use crate::noqa::{Directive, FileExemption, NoqaDirectives, NoqaMapping}; use crate::registry::{AsRule, Rule, RuleSet}; @@ -111,7 +112,8 @@ pub(crate) fn check_noqa( if line.matches.is_empty() { let mut diagnostic = Diagnostic::new(UnusedNOQA { codes: None }, directive.range()); - diagnostic.set_fix(Fix::safe_edit(delete_noqa(directive.range(), locator))); + diagnostic + .set_fix(Fix::safe_edit(delete_comment(directive.range(), locator))); diagnostics.push(diagnostic); } @@ -177,8 +179,10 @@ pub(crate) fn check_noqa( directive.range(), ); if valid_codes.is_empty() { - diagnostic - .set_fix(Fix::safe_edit(delete_noqa(directive.range(), locator))); + diagnostic.set_fix(Fix::safe_edit(delete_comment( + directive.range(), + locator, + ))); } else { diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( format!("# noqa: {}", valid_codes.join(", ")), @@ -195,48 +199,3 @@ pub(crate) fn check_noqa( ignored_diagnostics.sort_unstable(); ignored_diagnostics } - -/// Generate a [`Edit`] to delete a `noqa` directive. -fn delete_noqa(range: TextRange, locator: &Locator) -> Edit { - let line_range = locator.line_range(range.start()); - - // Compute the leading space. - let prefix = locator.slice(TextRange::new(line_range.start(), range.start())); - let leading_space_len = prefix.text_len() - prefix.trim_whitespace_end().text_len(); - - // Compute the trailing space. - let suffix = locator.slice(TextRange::new(range.end(), line_range.end())); - let trailing_space_len = suffix.text_len() - suffix.trim_whitespace_start().text_len(); - - // Ex) `# noqa` - if line_range - == TextRange::new( - range.start() - leading_space_len, - range.end() + trailing_space_len, - ) - { - let full_line_end = locator.full_line_end(line_range.end()); - Edit::deletion(line_range.start(), full_line_end) - } - // Ex) `x = 1 # noqa` - else if range.end() + trailing_space_len == line_range.end() { - Edit::deletion(range.start() - leading_space_len, line_range.end()) - } - // Ex) `x = 1 # noqa # type: ignore` - else if locator - .slice(TextRange::new( - range.end() + trailing_space_len, - line_range.end(), - )) - .starts_with('#') - { - Edit::deletion(range.start(), range.end() + trailing_space_len) - } - // Ex) `x = 1 # noqa here` - else { - Edit::deletion( - range.start() + "# ".text_len(), - range.end() + trailing_space_len, - ) - } -} diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 5cf0293d1292e..3ec7497eb3f89 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -945,6 +945,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::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/fix/edits.rs b/crates/ruff_linter/src/fix/edits.rs index 69d698fae6219..7adf6b2bfdbc6 100644 --- a/crates/ruff_linter/src/fix/edits.rs +++ b/crates/ruff_linter/src/fix/edits.rs @@ -62,6 +62,51 @@ pub(crate) fn delete_stmt( } } +/// Generate a [`Edit`] to delete a comment (for example: a `noqa` directive). +pub(crate) fn delete_comment(range: TextRange, locator: &Locator) -> Edit { + let line_range = locator.line_range(range.start()); + + // Compute the leading space. + let prefix = locator.slice(TextRange::new(line_range.start(), range.start())); + let leading_space_len = prefix.text_len() - prefix.trim_whitespace_end().text_len(); + + // Compute the trailing space. + let suffix = locator.slice(TextRange::new(range.end(), line_range.end())); + let trailing_space_len = suffix.text_len() - suffix.trim_whitespace_start().text_len(); + + // Ex) `# noqa` + if line_range + == TextRange::new( + range.start() - leading_space_len, + range.end() + trailing_space_len, + ) + { + let full_line_end = locator.full_line_end(line_range.end()); + Edit::deletion(line_range.start(), full_line_end) + } + // Ex) `x = 1 # noqa` + else if range.end() + trailing_space_len == line_range.end() { + Edit::deletion(range.start() - leading_space_len, line_range.end()) + } + // Ex) `x = 1 # noqa # type: ignore` + else if locator + .slice(TextRange::new( + range.end() + trailing_space_len, + line_range.end(), + )) + .starts_with('#') + { + Edit::deletion(range.start(), range.end() + trailing_space_len) + } + // Ex) `x = 1 # noqa here` + else { + Edit::deletion( + range.start() + "# ".text_len(), + range.end() + trailing_space_len, + ) + } +} + /// Generate a `Fix` to remove the specified imports from an `import` statement. pub(crate) fn remove_unused_imports<'a>( member_names: impl Iterator, diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 7239323bcbddb..1b06577295d13 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -49,6 +49,7 @@ mod tests { #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_0.py"))] #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_1.py"))] #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_2.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/invalid_formatter_suppression_comment.rs b/crates/ruff_linter/src/rules/ruff/rules/invalid_formatter_suppression_comment.rs new file mode 100644 index 0000000000000..f72e87a330d74 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/invalid_formatter_suppression_comment.rs @@ -0,0 +1,245 @@ +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, helpers::comment_indentation_after, AnyNodeRef}; +use ruff_python_trivia::{indentation_at_offset, SuppressionKind}; +use ruff_source_file::Locator; +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::{ + CaptureSuppressionComment, SuppressionComment, SuppressionCommentData, + SuppressionCommentVisitor, +}; + +/// ## What it does +/// Checks for formatter suppression comments that are ineffective or incompatible +/// with Ruff's formatter. +/// +/// ## Why is this bad? +/// Suppression comments that do not actually prevent formatting could cause unintended changes +/// when the formatter is run. +/// +/// ## Examples +/// In the following example, all suppression comments would cause +/// a rule violation. +/// +/// ```python +/// def decorator(): +/// pass +/// +/// +/// @decorator +/// # fmt: off +/// def example(): +/// if True: +/// # fmt: skip +/// expression = [ +/// # fmt: off +/// 1, +/// 2, +/// ] +/// # yapf: disable +/// # fmt: on +/// # yapf: enable +/// ``` +#[violation] +pub struct InvalidFormatterSuppressionComment { + reason: IgnoredReason, +} + +impl AlwaysFixableViolation for InvalidFormatterSuppressionComment { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "This suppression comment is invalid because {}", + self.reason + ) + } + + fn fix_title(&self) -> String { + format!("Remove this comment") + } +} + +/// RUF028 +pub(crate) fn ignored_formatter_suppression_comment(checker: &mut Checker, suite: &ast::Suite) { + let indexer = checker.indexer(); + let locator = checker.locator(); + 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.into_iter(), + &mut comments, + checker.locator(), + ); + + visitor.visit(suite); + + comments.sort(); + + for (range, reason) in comments.ignored_comments() { + checker.diagnostics.push( + Diagnostic::new(InvalidFormatterSuppressionComment { reason }, range) + .with_fix(Fix::unsafe_edit(delete_comment(range, checker.locator()))), + ); + } +} + +struct UselessSuppressionComments<'src, 'loc> { + captured: Vec<(TextRange, IgnoredReason)>, + locator: &'loc Locator<'src>, +} + +impl<'src, 'loc> UselessSuppressionComments<'src, 'loc> { + fn new(locator: &'loc Locator<'src>) -> Self { + Self { + captured: vec![], + locator, + } + } + /// This function determines whether or not `comment` is a useful suppression comment. + /// If it isn't, it will give a reason why the comment is ignored. See [`IgnoredReason`] for more. + fn check_suppression_comment( + &self, + comment: &SuppressionCommentData, + ) -> Result<(), IgnoredReason> { + // check if the comment is inside of an expression. + if comment + .enclosing + .map(|n| !AnyNodeRef::is_statement(n)) + .unwrap_or_default() + { + return Err(IgnoredReason::InNonStatement); + } + + // 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.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.first() { + if decorator.end() < comment.range.start() { + return Err(IgnoredReason::AfterDecorator); + } + } + } + } + } + + 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 following.is_first_statement_in_alternate_body(enclosing) { + // check indentation + let comment_indentation = + comment_indentation_after(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); + } + } + } + } + + if comment.kind == SuppressionKind::On { + // Ensure the comment is not a trailing comment + if !comment.line_position.is_own_line() { + return Err(IgnoredReason::FmtOnCannotBeTrailing); + } + } + + Ok(()) + } + + fn sort(&mut self) { + self.captured.sort_by_key(|(t, _)| t.start()); + } + + 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>) { + match self.check_suppression_comment(&comment) { + Ok(()) => {} + Err(reason) => { + self.captured.push((comment.range, reason)); + } + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum IgnoredReason { + InNonStatement, + AfterDecorator, + SkipHasToBeTrailing, + FmtOnCannotBeTrailing, + FmtOffAboveBlock, +} + +impl Display for IgnoredReason { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::InNonStatement => write!( + f, + "it cannot be in an expression, pattern, argument list, or other non-statement" + ), + Self::AfterDecorator => { + write!(f, "it cannot be after a decorator") + } + Self::SkipHasToBeTrailing => { + 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 cannot be directly above an alternate body") + } + } + } +} diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index b8d7fca925135..d4461f1f3b6af 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -6,6 +6,7 @@ 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 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,6 +37,7 @@ mod explicit_f_string_type_conversion; mod function_call_in_dataclass_default; mod helpers; mod implicit_optional; +mod invalid_formatter_suppression_comment; mod invalid_index_type; mod invalid_pyproject_toml; mod missing_fstring_syntax; @@ -50,6 +52,7 @@ mod sequence_sorting; mod sort_dunder_all; mod sort_dunder_slots; mod static_key_dict_comprehension; +mod suppression_comment_visitor; #[cfg(feature = "test-rules")] pub(crate) mod test_rules; mod unnecessary_dict_comprehension_for_iterable; 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 new file mode 100644 index 0000000000000..fe7f6ffa61994 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/suppression_comment_visitor.rs @@ -0,0 +1,217 @@ +use std::iter::Peekable; + +use ruff_python_ast::{ + helpers::comment_indentation_after, + visitor::preorder::{self, PreorderVisitor, TraversalSignal}, + AnyNodeRef, Suite, +}; +use ruff_python_trivia::{ + indentation_at_offset, CommentLinePosition, SimpleTokenizer, SuppressionKind, +}; +use ruff_source_file::Locator; +use ruff_text_size::{Ranged, TextRange, TextSize}; + +#[derive(Clone, Copy, Debug)] +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, + I: Iterator + 'src, +> { + comments: Peekable, + + parents: Vec>, + preceding_node: Option>, + + builder: &'builder mut (dyn CaptureSuppressionComment<'src> + 'src), + locator: &'src Locator<'src>, +} + +impl<'src, 'builder, I> SuppressionCommentVisitor<'src, 'builder, I> +where + I: Iterator + 'src, +{ + pub(super) fn new( + comment_ranges: I, + builder: &'builder mut (dyn CaptureSuppressionComment<'src> + 'src), + locator: &'src Locator<'src>, + ) -> Self { + Self { + comments: comment_ranges.peekable(), + parents: Vec::default(), + preceding_node: Option::default(), + builder, + locator, + } + } + + pub(super) fn visit(mut self, suite: &'src Suite) { + self.visit_body(suite.as_slice()); + } + + fn can_skip(&mut self, node_end: TextSize) -> bool { + self.comments + .peek() + .map_or(true, |next| next.range.start() >= node_end) + } +} + +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(); + + let enclosing_node = self.parents.last().copied(); + + // Process all remaining comments that end before this node's start position. + // If the `preceding` node is set, then it process all comments ending after the `preceding` node + // and ending before this node's start position + while let Some(SuppressionComment { range, kind }) = self.comments.peek().copied() { + // Exit if the comment is enclosed by this node or comes after it + if range.end() > node_range.start() { + break; + } + + let line_position = CommentLinePosition::for_range(range, self.locator.contents()); + + let data = SuppressionCommentData { + enclosing: enclosing_node, + preceding: self.preceding_node, + following: Some(node), + line_position, + kind, + range, + }; + + self.builder.capture(data); + self.comments.next(); + } + + // From here on, we're inside of `node`, meaning, we're passed the preceding node. + self.preceding_node = None; + self.parents.push(node); + + if self.can_skip(node_range.end()) { + TraversalSignal::Skip + } else { + TraversalSignal::Traverse + } + } + + fn leave_node(&mut self, node: AnyNodeRef<'ast>) { + self.parents.pop(); + + let node_end = node.end(); + + // Process all comments that start after the `preceding` node and end before this node's end. + 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 { + let between = TextRange::new(node_end, range.start()); + // Check if a non-trivial token exists between the end of this node and the start of the comment. + // If it doesn't, that means this comment could possibly be a trailing comment that comes after the + // end of this node. + // For example: + // ``` + // def func(x): + // pass # fmt: skip + // ``` + // We want to make sure that `# fmt: skip` is associated with the `pass` statement, + // even though it comes after the end of that node. + if SimpleTokenizer::new(self.locator.contents(), between) + .skip_trivia() + .next() + .is_some() + { + break; + } + // If the comment is on its own line, it could still be a trailing comment if it has a greater + // level of indentation compared to this node. For example: + // ``` + // def func(x): + // # fmt: off + // pass + // # fmt: on + // def func2(y): + // pass + // ``` + // We want `# fmt: on` to be considered a trailing comment of `func(x)` instead of a leading comment + // on `func2(y)`. + if line_position.is_own_line() { + let comment_indent = comment_indentation_after(node, range, self.locator); + let node_indent = TextSize::of( + indentation_at_offset(node.start(), self.locator).unwrap_or_default(), + ); + if node_indent >= comment_indent { + break; + } + } + } + + let data = SuppressionCommentData { + enclosing: Some(node), + preceding: self.preceding_node, + following: None, + line_position, + kind, + range, + }; + + self.builder.capture(data); + self.comments.next(); + } + + self.preceding_node = Some(node); + } + fn visit_body(&mut self, body: &'ast [ruff_python_ast::Stmt]) { + match body { + [] => { + // no-op + } + [only] => { + self.visit_stmt(only); + } + [first, .., last] => { + if self.can_skip(last.end()) { + // Skip traversing the body when there's no comment between the first and last statement. + // It is still necessary to visit the first statement to process all comments between + // the previous node and the first statement. + self.visit_stmt(first); + self.preceding_node = Some(last.into()); + } else { + preorder::walk_body(self, body); + } + } + } + } +} + +#[derive(Clone, Debug)] +pub(super) struct SuppressionCommentData<'src> { + /// The AST node that encloses the comment. If `enclosing` is `None`, this comment is a top-level statement. + pub(super) enclosing: Option>, + /// An AST node that comes directly before the comment. A child of `enclosing`. + pub(super) preceding: Option>, + /// An AST node that comes directly after the comment. A child of `enclosing`. + pub(super) following: Option>, + + /// The line position of the comment - it can either be on its own line, or at the end of a line. + pub(super) line_position: CommentLinePosition, + /// Whether this comment is `fmt: off`, `fmt: on`, or `fmt: skip` (or `yapf disable` / `yapf enable`) + pub(super) kind: SuppressionKind, + /// The range of text that makes up the comment. This includes the `#` prefix. + pub(super) range: TextRange, +} + +pub(super) trait CaptureSuppressionComment<'src> { + /// This is the entrypoint for the capturer to analyze the next comment. + fn capture(&mut self, comment: SuppressionCommentData<'src>); +} 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 new file mode 100644 index 0000000000000..400fe6637ef52 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF028_RUF028.py.snap @@ -0,0 +1,214 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF028.py:3:9: RUF028 [*] This suppression comment is invalid because it cannot be in an expression, pattern, argument list, or other non-statement + | +1 | def fmt_off_between_lists(): +2 | test_list = [ +3 | # fmt: off + | ^^^^^^^^^^ RUF028 +4 | 1, +5 | 2, + | + = help: Remove this comment + +ℹ Unsafe fix +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:12:5: RUF028 [*] This suppression comment is invalid because it cannot be on its own line + | +10 | # note: the second `fmt: skip`` should be OK +11 | def fmt_skip_on_own_line(): +12 | # fmt: skip + | ^^^^^^^^^^^ RUF028 +13 | pass # fmt: skip + | + = help: Remove this comment + +ℹ Unsafe fix +9 9 | +10 10 | # note: the second `fmt: skip`` should be OK +11 11 | def fmt_skip_on_own_line(): +12 |- # fmt: skip +13 12 | pass # fmt: skip +14 13 | +15 14 | + +RUF028.py:17:1: RUF028 [*] This suppression comment is invalid because it cannot be after a decorator + | +16 | @fmt_skip_on_own_line +17 | # fmt: off + | ^^^^^^^^^^ RUF028 +18 | @fmt_off_between_lists +19 | def fmt_off_between_decorators(): + | + = help: Remove this comment + +ℹ Unsafe fix +14 14 | +15 15 | +16 16 | @fmt_skip_on_own_line +17 |-# fmt: off +18 17 | @fmt_off_between_lists +19 18 | def fmt_off_between_decorators(): +20 19 | pass + +RUF028.py:24:1: RUF028 [*] This suppression comment is invalid because it cannot be after a decorator + | +23 | @fmt_off_between_decorators +24 | # fmt: off + | ^^^^^^^^^^ RUF028 +25 | class FmtOffBetweenClassDecorators: +26 | ... + | + = help: Remove this comment + +ℹ Unsafe fix +21 21 | +22 22 | +23 23 | @fmt_off_between_decorators +24 |-# fmt: off +25 24 | class FmtOffBetweenClassDecorators: +26 25 | ... +27 26 | + +RUF028.py:33:5: RUF028 [*] This suppression comment is invalid because it cannot be directly above an alternate body + | +31 | for val in x: +32 | print(x) +33 | # fmt: off + | ^^^^^^^^^^ RUF028 +34 | else: +35 | print("done") + | + = help: Remove this comment + +ℹ Unsafe fix +30 30 | x = [1, 2, 3] +31 31 | for val in x: +32 32 | print(x) +33 |- # fmt: off +34 33 | else: +35 34 | print("done") +36 35 | while False: + +RUF028.py:39:5: RUF028 [*] This suppression comment is invalid because it cannot be directly above an alternate body + | +37 | print("while") +38 | # fmt: off +39 | # fmt: off + | ^^^^^^^^^^ RUF028 +40 | else: +41 | print("done") + | + = help: Remove this comment + +ℹ Unsafe fix +36 36 | while False: +37 37 | print("while") +38 38 | # fmt: off +39 |- # fmt: off +40 39 | else: +41 40 | print("done") +42 41 | if len(x) > 3: + +RUF028.py:45:5: RUF028 [*] This suppression comment is invalid because it cannot be directly above an alternate body + | +43 | print("huh?") +44 | # fmt: on +45 | # fmt: off + | ^^^^^^^^^^ RUF028 +46 | else: +47 | print("expected") + | + = help: Remove this comment + +ℹ Unsafe fix +42 42 | if len(x) > 3: +43 43 | print("huh?") +44 44 | # fmt: on +45 |- # fmt: off +46 45 | else: +47 46 | print("expected") +48 47 | + +RUF028.py:52:5: RUF028 [*] This suppression comment is invalid because it cannot be after a decorator + | +50 | class Test: +51 | @classmethod +52 | # fmt: off + | ^^^^^^^^^^ RUF028 +53 | def cls_method_a( +54 | # fmt: off + | + = help: Remove this comment + +ℹ Unsafe fix +49 49 | +50 50 | class Test: +51 51 | @classmethod +52 |- # fmt: off +53 52 | def cls_method_a( +54 53 | # fmt: off +55 54 | cls, + +RUF028.py:54:9: RUF028 [*] This suppression comment is invalid because it cannot be in an expression, pattern, argument list, or other non-statement + | +52 | # fmt: off +53 | def cls_method_a( +54 | # fmt: off + | ^^^^^^^^^^ RUF028 +55 | cls, +56 | ) -> None: # noqa: test # fmt: skip + | + = help: Remove this comment + +ℹ Unsafe fix +51 51 | @classmethod +52 52 | # fmt: off +53 53 | def cls_method_a( +54 |- # fmt: off +55 54 | cls, +56 55 | ) -> None: # noqa: test # fmt: skip +57 56 | pass + +RUF028.py:62:13: RUF028 [*] This suppression comment is invalid because it cannot be at the end of a line + | +60 | def fmt_on_trailing(): +61 | # fmt: off +62 | val = 5 # fmt: on + | ^^^^^^^^^ RUF028 +63 | pass # fmt: on + | + = help: Remove this comment + +ℹ Unsafe fix +59 59 | +60 60 | def fmt_on_trailing(): +61 61 | # fmt: off +62 |- val = 5 # fmt: on + 62 |+ val = 5 +63 63 | pass # fmt: on + +RUF028.py:63:10: RUF028 [*] This suppression comment is invalid because it cannot be at the end of a line + | +61 | # fmt: off +62 | val = 5 # fmt: on +63 | pass # fmt: on + | ^^^^^^^^^ RUF028 + | + = help: Remove this comment + +ℹ Unsafe fix +60 60 | def fmt_on_trailing(): +61 61 | # fmt: off +62 62 | val = 5 # fmt: on +63 |- pass # fmt: on + 63 |+ pass + + diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index 154be660d37ae..d80bfef769197 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -4,9 +4,9 @@ use std::path::Path; use rustc_hash::FxHashMap; use smallvec::SmallVec; -use ruff_python_trivia::CommentRanges; +use ruff_python_trivia::{indentation_at_offset, CommentRanges, SimpleTokenKind, SimpleTokenizer}; use ruff_source_file::Locator; -use ruff_text_size::{Ranged, TextRange}; +use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; use crate::call_path::CallPath; use crate::parenthesize::parenthesized_range; @@ -1481,6 +1481,80 @@ pub fn typing_union(elts: &[Expr], binding: String) -> Expr { }) } +/// Determine the indentation level of an own-line comment, defined as the minimum indentation of +/// all comments between the preceding node and the comment, including the comment itself. In +/// other words, we don't allow successive comments to ident _further_ than any preceding comments. +/// +/// For example, given: +/// ```python +/// if True: +/// pass +/// # comment +/// ``` +/// +/// The indentation would be 4, as the comment is indented by 4 spaces. +/// +/// Given: +/// ```python +/// if True: +/// pass +/// # comment +/// else: +/// pass +/// ``` +/// +/// The indentation would be 0, as the comment is not indented at all. +/// +/// Given: +/// ```python +/// if True: +/// pass +/// # comment +/// # comment +/// ``` +/// +/// Both comments would be marked as indented at 4 spaces, as the indentation of the first comment +/// is used for the second comment. +/// +/// This logic avoids pathological cases like: +/// ```python +/// try: +/// if True: +/// if True: +/// pass +/// +/// # a +/// # b +/// # c +/// except Exception: +/// pass +/// ``` +/// +/// If we don't use the minimum indentation of any preceding comments, we would mark `# b` as +/// indented to the same depth as `pass`, which could in turn lead to us treating it as a trailing +/// comment of `pass`, despite there being a comment between them that "resets" the indentation. +pub fn comment_indentation_after( + preceding: AnyNodeRef, + comment_range: TextRange, + locator: &Locator, +) -> TextSize { + let tokenizer = SimpleTokenizer::new( + locator.contents(), + TextRange::new(locator.full_line_end(preceding.end()), comment_range.end()), + ); + + tokenizer + .filter_map(|token| { + if token.kind() == SimpleTokenKind::Comment { + indentation_at_offset(token.start(), locator).map(TextLen::text_len) + } else { + None + } + }) + .min() + .unwrap_or_default() +} + #[cfg(test)] mod tests { use std::borrow::Cow; diff --git a/crates/ruff_python_ast/src/node.rs b/crates/ruff_python_ast/src/node.rs index 54afd2e040b3e..16b19622c36d8 100644 --- a/crates/ruff_python_ast/src/node.rs +++ b/crates/ruff_python_ast/src/node.rs @@ -6196,6 +6196,120 @@ impl<'a> AnyNodeRef<'a> { body.last().map(AnyNodeRef::from) } + + /// Check if the given statement is the first statement after the colon of a branch, be it in if + /// statements, for statements, after each part of a try-except-else-finally or function/class + /// definitions. + /// + /// + /// ```python + /// if True: <- has body + /// a <- first statement + /// b + /// elif b: <- has body + /// c <- first statement + /// d + /// else: <- has body + /// e <- first statement + /// f + /// + /// class: <- has body + /// a: int <- first statement + /// b: int + /// + /// ``` + /// + /// For nodes with multiple bodies, we check all bodies that don't have their own node. For + /// try-except-else-finally, each except branch has it's own node, so for the `StmtTry`, we check + /// the `try:`, `else:` and `finally:`, bodies, while `ExceptHandlerExceptHandler` has it's own + /// check. For for-else and while-else, we check both branches for the whole statement. + /// + /// ```python + /// try: <- has body (a) + /// 6/8 <- first statement (a) + /// 1/0 + /// except: <- has body (b) + /// a <- first statement (b) + /// b + /// else: + /// c <- first statement (a) + /// d + /// finally: + /// e <- first statement (a) + /// f + /// ``` + pub fn is_first_statement_in_body(&self, body: AnyNodeRef) -> bool { + match body { + AnyNodeRef::StmtFor(ast::StmtFor { body, orelse, .. }) + | AnyNodeRef::StmtWhile(ast::StmtWhile { body, orelse, .. }) => { + are_same_optional(*self, body.first()) || are_same_optional(*self, orelse.first()) + } + + AnyNodeRef::StmtTry(ast::StmtTry { + body, + orelse, + finalbody, + .. + }) => { + are_same_optional(*self, body.first()) + || are_same_optional(*self, orelse.first()) + || are_same_optional(*self, finalbody.first()) + } + + AnyNodeRef::StmtIf(ast::StmtIf { body, .. }) + | AnyNodeRef::ElifElseClause(ast::ElifElseClause { body, .. }) + | AnyNodeRef::StmtWith(ast::StmtWith { body, .. }) + | AnyNodeRef::ExceptHandlerExceptHandler(ast::ExceptHandlerExceptHandler { + body, + .. + }) + | AnyNodeRef::MatchCase(MatchCase { body, .. }) + | AnyNodeRef::StmtFunctionDef(ast::StmtFunctionDef { body, .. }) + | AnyNodeRef::StmtClassDef(ast::StmtClassDef { body, .. }) => { + are_same_optional(*self, body.first()) + } + + AnyNodeRef::StmtMatch(ast::StmtMatch { cases, .. }) => { + are_same_optional(*self, cases.first()) + } + + _ => false, + } + } + + /// Returns `true` if `statement` is the first statement in an alternate `body` (e.g. the else of an if statement) + pub fn is_first_statement_in_alternate_body(&self, body: AnyNodeRef) -> bool { + match body { + AnyNodeRef::StmtFor(ast::StmtFor { orelse, .. }) + | AnyNodeRef::StmtWhile(ast::StmtWhile { orelse, .. }) => { + are_same_optional(*self, orelse.first()) + } + + AnyNodeRef::StmtTry(ast::StmtTry { + handlers, + orelse, + finalbody, + .. + }) => { + are_same_optional(*self, handlers.first()) + || are_same_optional(*self, orelse.first()) + || are_same_optional(*self, finalbody.first()) + } + + AnyNodeRef::StmtIf(ast::StmtIf { + elif_else_clauses, .. + }) => are_same_optional(*self, elif_else_clauses.first()), + _ => false, + } + } +} + +/// Returns `true` if `right` is `Some` and `left` and `right` are referentially equal. +fn are_same_optional<'a, T>(left: AnyNodeRef, right: Option) -> bool +where + T: Into>, +{ + right.is_some_and(|right| left.ptr_eq(right.into())) } impl<'a> From<&'a ast::ModModule> for AnyNodeRef<'a> { diff --git a/crates/ruff_python_formatter/src/comments/debug.rs b/crates/ruff_python_formatter/src/comments/debug.rs index b28d992058464..a91b61bc2e6f9 100644 --- a/crates/ruff_python_formatter/src/comments/debug.rs +++ b/crates/ruff_python_formatter/src/comments/debug.rs @@ -182,11 +182,11 @@ mod tests { use ruff_formatter::SourceCode; use ruff_python_ast::AnyNode; use ruff_python_ast::{StmtBreak, StmtContinue}; - use ruff_python_trivia::CommentRanges; + use ruff_python_trivia::{CommentLinePosition, CommentRanges}; use ruff_text_size::{TextRange, TextSize}; use crate::comments::map::MultiMap; - use crate::comments::{CommentLinePosition, Comments, CommentsMap, SourceComment}; + use crate::comments::{Comments, CommentsMap, SourceComment}; #[test] fn debug() { diff --git a/crates/ruff_python_formatter/src/comments/format.rs b/crates/ruff_python_formatter/src/comments/format.rs index 13f1cdf2880f1..b6cc694def354 100644 --- a/crates/ruff_python_formatter/src/comments/format.rs +++ b/crates/ruff_python_formatter/src/comments/format.rs @@ -3,11 +3,11 @@ use std::borrow::Cow; use ruff_formatter::{format_args, write, FormatError, FormatOptions, SourceCode}; use ruff_python_ast::{AnyNodeRef, AstNode, NodeKind, PySourceType}; use ruff_python_trivia::{ - is_pragma_comment, lines_after, lines_after_ignoring_trivia, lines_before, + is_pragma_comment, lines_after, lines_after_ignoring_trivia, lines_before, CommentLinePosition, }; use ruff_text_size::{Ranged, TextLen, TextRange}; -use crate::comments::{CommentLinePosition, SourceComment}; +use crate::comments::SourceComment; use crate::context::NodeLevel; use crate::prelude::*; use crate::preview::is_blank_line_after_nested_stub_class_enabled; diff --git a/crates/ruff_python_formatter/src/comments/mod.rs b/crates/ruff_python_formatter/src/comments/mod.rs index 92c52ff09ce83..9ca2134f700d9 100644 --- a/crates/ruff_python_formatter/src/comments/mod.rs +++ b/crates/ruff_python_formatter/src/comments/mod.rs @@ -98,7 +98,7 @@ pub(crate) use format::{ use ruff_formatter::{SourceCode, SourceCodeSlice}; use ruff_python_ast::visitor::preorder::{PreorderVisitor, TraversalSignal}; use ruff_python_ast::AnyNodeRef; -use ruff_python_trivia::{CommentRanges, PythonWhitespace}; +use ruff_python_trivia::{CommentLinePosition, CommentRanges, SuppressionKind}; use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextRange}; pub(crate) use visitor::collect_comments; @@ -168,73 +168,16 @@ impl SourceComment { DebugComment::new(self, source_code) } - pub(crate) fn suppression_kind(&self, source: &str) -> Option { - let text = self.slice.text(SourceCode::new(source)); - - // Match against `# fmt: on`, `# fmt: off`, `# yapf: disable`, and `# yapf: enable`, which - // must be on their own lines. - let trimmed = text.strip_prefix('#').unwrap_or(text).trim_whitespace(); - if let Some(command) = trimmed.strip_prefix("fmt:") { - match command.trim_whitespace_start() { - "off" => return Some(SuppressionKind::Off), - "on" => return Some(SuppressionKind::On), - "skip" => return Some(SuppressionKind::Skip), - _ => {} - } - } else if let Some(command) = trimmed.strip_prefix("yapf:") { - match command.trim_whitespace_start() { - "disable" => return Some(SuppressionKind::Off), - "enable" => return Some(SuppressionKind::On), - _ => {} - } - } - - // Search for `# fmt: skip` comments, which can be interspersed with other comments (e.g., - // `# fmt: skip # noqa: E501`). - for segment in text.split('#') { - let trimmed = segment.trim_whitespace(); - if let Some(command) = trimmed.strip_prefix("fmt:") { - if command.trim_whitespace_start() == "skip" { - return Some(SuppressionKind::Skip); - } - } - } - - None - } - - /// Returns true if this comment is a `fmt: off` or `yapf: disable` own line suppression comment. - pub(crate) fn is_suppression_off_comment(&self, source: &str) -> bool { - self.line_position.is_own_line() - && matches!(self.suppression_kind(source), Some(SuppressionKind::Off)) + pub(crate) fn is_suppression_off_comment(&self, text: &str) -> bool { + SuppressionKind::is_suppression_off(self.text(text), self.line_position) } - /// Returns true if this comment is a `fmt: on` or `yapf: enable` own line suppression comment. - pub(crate) fn is_suppression_on_comment(&self, source: &str) -> bool { - self.line_position.is_own_line() - && matches!(self.suppression_kind(source), Some(SuppressionKind::On)) + pub(crate) fn is_suppression_on_comment(&self, text: &str) -> bool { + SuppressionKind::is_suppression_on(self.text(text), self.line_position) } -} - -#[derive(Copy, Clone, Eq, PartialEq, Debug)] -pub(crate) enum SuppressionKind { - /// A `fmt: off` or `yapf: disable` comment - Off, - /// A `fmt: on` or `yapf: enable` comment - On, - /// A `fmt: skip` comment - Skip, -} -impl SuppressionKind { - pub(crate) fn has_skip_comment(trailing_comments: &[SourceComment], source: &str) -> bool { - trailing_comments.iter().any(|comment| { - comment.line_position().is_end_of_line() - && matches!( - comment.suppression_kind(source), - Some(SuppressionKind::Skip | SuppressionKind::Off) - ) - }) + fn text<'a>(&self, text: &'a str) -> &'a str { + self.slice.text(SourceCode::new(text)) } } @@ -245,48 +188,6 @@ impl Ranged for SourceComment { } } -/// The position of a comment in the source text. -#[derive(Debug, Copy, Clone, Eq, PartialEq)] -pub(crate) enum CommentLinePosition { - /// A comment that is on the same line as the preceding token and is separated by at least one line break from the following token. - /// - /// # Examples - /// - /// ## End of line - /// - /// ```python - /// a; # comment - /// b; - /// ``` - /// - /// `# comment` is an end of line comments because it is separated by at least one line break from the following token `b`. - /// Comments that not only end, but also start on a new line are [`OwnLine`](CommentLinePosition::OwnLine) comments. - EndOfLine, - - /// A Comment that is separated by at least one line break from the preceding token. - /// - /// # Examples - /// - /// ```python - /// a; - /// # comment - /// b; - /// ``` - /// - /// `# comment` line comments because they are separated by one line break from the preceding token `a`. - OwnLine, -} - -impl CommentLinePosition { - pub(crate) const fn is_own_line(self) -> bool { - matches!(self, CommentLinePosition::OwnLine) - } - - pub(crate) const fn is_end_of_line(self) -> bool { - matches!(self, CommentLinePosition::EndOfLine) - } -} - type CommentsMap<'a> = MultiMap, SourceComment>; /// The comments of a syntax tree stored by node. @@ -563,6 +464,16 @@ impl<'a> PreorderVisitor<'a> for MarkVerbatimCommentsAsFormattedVisitor<'a> { } } +pub(crate) fn has_skip_comment(trailing_comments: &[SourceComment], source: &str) -> bool { + trailing_comments.iter().any(|comment| { + comment.line_position().is_end_of_line() + && matches!( + SuppressionKind::from_comment(comment.text(source)), + Some(SuppressionKind::Skip | SuppressionKind::Off) + ) + }) +} + #[cfg(test)] mod tests { use insta::assert_debug_snapshot; diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 0033d69206e6d..c8b5af6533f1a 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1,15 +1,14 @@ use std::cmp::Ordering; +use ast::helpers::comment_indentation_after; use ruff_python_ast::whitespace::indentation; -use ruff_python_ast::{ - self as ast, AnyNodeRef, Comprehension, Expr, MatchCase, ModModule, Parameters, -}; +use ruff_python_ast::{self as ast, AnyNodeRef, Comprehension, Expr, ModModule, Parameters}; use ruff_python_trivia::{ find_only_token_in_range, indentation_at_offset, BackwardsTokenizer, CommentRanges, SimpleToken, SimpleTokenKind, SimpleTokenizer, }; use ruff_source_file::Locator; -use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; +use ruff_text_size::{Ranged, TextLen, TextRange}; use crate::comments::visitor::{CommentPlacement, DecoratedComment}; use crate::expression::expr_generator_exp::is_generator_parenthesized; @@ -344,7 +343,7 @@ fn handle_end_of_line_comment_around_body<'a>( // pass // ``` if let Some(following) = comment.following_node() { - if is_first_statement_in_body(following, comment.enclosing_node()) + if following.is_first_statement_in_body(comment.enclosing_node()) && SimpleTokenizer::new( locator.contents(), TextRange::new(comment.end(), following.start()), @@ -381,86 +380,6 @@ fn handle_end_of_line_comment_around_body<'a>( CommentPlacement::Default(comment) } -/// Check if the given statement is the first statement after the colon of a branch, be it in if -/// statements, for statements, after each part of a try-except-else-finally or function/class -/// definitions. -/// -/// -/// ```python -/// if True: <- has body -/// a <- first statement -/// b -/// elif b: <- has body -/// c <- first statement -/// d -/// else: <- has body -/// e <- first statement -/// f -/// -/// class: <- has body -/// a: int <- first statement -/// b: int -/// -/// ``` -/// -/// For nodes with multiple bodies, we check all bodies that don't have their own node. For -/// try-except-else-finally, each except branch has it's own node, so for the `StmtTry`, we check -/// the `try:`, `else:` and `finally:`, bodies, while `ExceptHandlerExceptHandler` has it's own -/// check. For for-else and while-else, we check both branches for the whole statement. -/// -/// ```python -/// try: <- has body (a) -/// 6/8 <- first statement (a) -/// 1/0 -/// except: <- has body (b) -/// a <- first statement (b) -/// b -/// else: -/// c <- first statement (a) -/// d -/// finally: -/// e <- first statement (a) -/// f -/// ``` -fn is_first_statement_in_body(statement: AnyNodeRef, has_body: AnyNodeRef) -> bool { - match has_body { - AnyNodeRef::StmtFor(ast::StmtFor { body, orelse, .. }) - | AnyNodeRef::StmtWhile(ast::StmtWhile { body, orelse, .. }) => { - are_same_optional(statement, body.first()) - || are_same_optional(statement, orelse.first()) - } - - AnyNodeRef::StmtTry(ast::StmtTry { - body, - orelse, - finalbody, - .. - }) => { - are_same_optional(statement, body.first()) - || are_same_optional(statement, orelse.first()) - || are_same_optional(statement, finalbody.first()) - } - - AnyNodeRef::StmtIf(ast::StmtIf { body, .. }) - | AnyNodeRef::ElifElseClause(ast::ElifElseClause { body, .. }) - | AnyNodeRef::StmtWith(ast::StmtWith { body, .. }) - | AnyNodeRef::ExceptHandlerExceptHandler(ast::ExceptHandlerExceptHandler { - body, .. - }) - | AnyNodeRef::MatchCase(MatchCase { body, .. }) - | AnyNodeRef::StmtFunctionDef(ast::StmtFunctionDef { body, .. }) - | AnyNodeRef::StmtClassDef(ast::StmtClassDef { body, .. }) => { - are_same_optional(statement, body.first()) - } - - AnyNodeRef::StmtMatch(ast::StmtMatch { cases, .. }) => { - are_same_optional(statement, cases.first()) - } - - _ => false, - } -} - /// Handles own-line comments around a body (at the end of the body, at the end of the header /// preceding the body, or between bodies): /// @@ -612,13 +531,13 @@ fn handle_own_line_comment_between_branches<'a>( let Some(following) = comment.following_node() else { return CommentPlacement::Default(comment); }; - if !is_first_statement_in_alternate_body(following, comment.enclosing_node()) { + if !following.is_first_statement_in_alternate_body(comment.enclosing_node()) { return CommentPlacement::Default(comment); } // It depends on the indentation level of the comment if it is a leading comment for the // following branch or if it a trailing comment of the previous body's last statement. - let comment_indentation = own_line_comment_indentation(preceding, &comment, locator); + let comment_indentation = comment_indentation_after(preceding, comment.range(), locator); let preceding_indentation = indentation(locator, &preceding) .unwrap_or_default() @@ -702,7 +621,7 @@ fn handle_own_line_comment_after_branch<'a>( // We only care about the length because indentations with mixed spaces and tabs are only valid if // the indent-level doesn't depend on the tab width (the indent level must be the same if the tab width is 1 or 8). - let comment_indentation = own_line_comment_indentation(preceding, &comment, locator); + let comment_indentation = comment_indentation_after(preceding, comment.range(), locator); // Keep the comment on the entire statement in case it's a trailing comment // ```python @@ -778,80 +697,6 @@ fn handle_own_line_comment_after_branch<'a>( } } -/// Determine the indentation level of an own-line comment, defined as the minimum indentation of -/// all comments between the preceding node and the comment, including the comment itself. In -/// other words, we don't allow successive comments to ident _further_ than any preceding comments. -/// -/// For example, given: -/// ```python -/// if True: -/// pass -/// # comment -/// ``` -/// -/// The indentation would be 4, as the comment is indented by 4 spaces. -/// -/// Given: -/// ```python -/// if True: -/// pass -/// # comment -/// else: -/// pass -/// ``` -/// -/// The indentation would be 0, as the comment is not indented at all. -/// -/// Given: -/// ```python -/// if True: -/// pass -/// # comment -/// # comment -/// ``` -/// -/// Both comments would be marked as indented at 4 spaces, as the indentation of the first comment -/// is used for the second comment. -/// -/// This logic avoids pathological cases like: -/// ```python -/// try: -/// if True: -/// if True: -/// pass -/// -/// # a -/// # b -/// # c -/// except Exception: -/// pass -/// ``` -/// -/// If we don't use the minimum indentation of any preceding comments, we would mark `# b` as -/// indented to the same depth as `pass`, which could in turn lead to us treating it as a trailing -/// comment of `pass`, despite there being a comment between them that "resets" the indentation. -fn own_line_comment_indentation( - preceding: AnyNodeRef, - comment: &DecoratedComment, - locator: &Locator, -) -> TextSize { - let tokenizer = SimpleTokenizer::new( - locator.contents(), - TextRange::new(locator.full_line_end(preceding.end()), comment.end()), - ); - - tokenizer - .filter_map(|token| { - if token.kind() == SimpleTokenKind::Comment { - indentation_at_offset(token.start(), locator).map(TextLen::text_len) - } else { - None - } - }) - .min() - .unwrap_or_default() -} - /// Attaches comments for the positional-only parameters separator `/` or the keywords-only /// parameters separator `*` as dangling comments to the enclosing [`Parameters`] node. /// @@ -2194,40 +2039,6 @@ fn handle_comprehension_comment<'a>( CommentPlacement::Default(comment) } -/// Returns `true` if `right` is `Some` and `left` and `right` are referentially equal. -fn are_same_optional<'a, T>(left: AnyNodeRef, right: Option) -> bool -where - T: Into>, -{ - right.is_some_and(|right| left.ptr_eq(right.into())) -} - -/// Returns `true` if `statement` is the first statement in an alternate `body` (e.g. the else of an if statement) -fn is_first_statement_in_alternate_body(statement: AnyNodeRef, has_body: AnyNodeRef) -> bool { - match has_body { - AnyNodeRef::StmtFor(ast::StmtFor { orelse, .. }) - | AnyNodeRef::StmtWhile(ast::StmtWhile { orelse, .. }) => { - are_same_optional(statement, orelse.first()) - } - - AnyNodeRef::StmtTry(ast::StmtTry { - handlers, - orelse, - finalbody, - .. - }) => { - are_same_optional(statement, handlers.first()) - || are_same_optional(statement, orelse.first()) - || are_same_optional(statement, finalbody.first()) - } - - AnyNodeRef::StmtIf(ast::StmtIf { - elif_else_clauses, .. - }) => are_same_optional(statement, elif_else_clauses.first()), - _ => false, - } -} - /// Returns `true` if the parameters are parenthesized (as in a function definition), or `false` if /// not (as in a lambda). fn are_parameters_parenthesized(parameters: &Parameters, contents: &str) -> bool { diff --git a/crates/ruff_python_formatter/src/comments/visitor.rs b/crates/ruff_python_formatter/src/comments/visitor.rs index f98aaabb169ef..ede7180107a3e 100644 --- a/crates/ruff_python_formatter/src/comments/visitor.rs +++ b/crates/ruff_python_formatter/src/comments/visitor.rs @@ -8,13 +8,13 @@ use ruff_python_ast::{Mod, Stmt}; // pre-order. #[allow(clippy::wildcard_imports)] use ruff_python_ast::visitor::preorder::*; -use ruff_python_trivia::{is_python_whitespace, CommentRanges}; +use ruff_python_trivia::{CommentLinePosition, CommentRanges}; use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::comments::node_key::NodeRefEqualityKey; use crate::comments::placement::place_comment; -use crate::comments::{CommentLinePosition, CommentsMap, SourceComment}; +use crate::comments::{CommentsMap, SourceComment}; /// Collect the preceding, following and enclosing node for each comment without applying /// [`place_comment`] for debugging. @@ -90,7 +90,10 @@ impl<'ast> PreorderVisitor<'ast> for CommentsVisitor<'ast, '_> { preceding: self.preceding_node, following: Some(node), parent: self.parents.iter().rev().nth(1).copied(), - line_position: text_position(*comment_range, self.source_code), + line_position: CommentLinePosition::for_range( + *comment_range, + self.source_code.as_str(), + ), slice: self.source_code.slice(*comment_range), }; @@ -130,7 +133,10 @@ impl<'ast> PreorderVisitor<'ast> for CommentsVisitor<'ast, '_> { parent: self.parents.last().copied(), preceding: self.preceding_node, following: None, - line_position: text_position(*comment_range, self.source_code), + line_position: CommentLinePosition::for_range( + *comment_range, + self.source_code.as_str(), + ), slice: self.source_code.slice(*comment_range), }; @@ -163,22 +169,6 @@ impl<'ast> PreorderVisitor<'ast> for CommentsVisitor<'ast, '_> { } } -fn text_position(comment_range: TextRange, source_code: SourceCode) -> CommentLinePosition { - let before = &source_code.as_str()[TextRange::up_to(comment_range.start())]; - - for c in before.chars().rev() { - match c { - '\n' | '\r' => { - break; - } - c if is_python_whitespace(c) => continue, - _ => return CommentLinePosition::EndOfLine, - } - } - - CommentLinePosition::OwnLine -} - /// A comment decorated with additional information about its surrounding context in the source document. /// /// Used by [`CommentStyle::place_comment`] to determine if this should become a [leading](self#leading-comments), [dangling](self#dangling-comments), or [trailing](self#trailing-comments) comment. diff --git a/crates/ruff_python_formatter/src/lib.rs b/crates/ruff_python_formatter/src/lib.rs index 011ba245a91eb..c4cf4a16c3d4c 100644 --- a/crates/ruff_python_formatter/src/lib.rs +++ b/crates/ruff_python_formatter/src/lib.rs @@ -12,7 +12,8 @@ use ruff_python_trivia::CommentRanges; use ruff_source_file::Locator; use crate::comments::{ - dangling_comments, leading_comments, trailing_comments, Comments, SourceComment, + dangling_comments, has_skip_comment, leading_comments, trailing_comments, Comments, + SourceComment, }; pub use crate::context::PyFormatContext; pub use crate::options::{ diff --git a/crates/ruff_python_formatter/src/other/decorator.rs b/crates/ruff_python_formatter/src/other/decorator.rs index 9754ac4b4aef1..38e534d9048bb 100644 --- a/crates/ruff_python_formatter/src/other/decorator.rs +++ b/crates/ruff_python_formatter/src/other/decorator.rs @@ -1,10 +1,10 @@ use ruff_formatter::write; use ruff_python_ast::Decorator; -use crate::comments::{SourceComment, SuppressionKind}; +use crate::comments::SourceComment; use crate::expression::maybe_parenthesize_expression; use crate::expression::parentheses::Parenthesize; -use crate::prelude::*; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatDecorator; @@ -30,6 +30,6 @@ impl FormatNodeRule for FormatDecorator { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/other/parameters.rs b/crates/ruff_python_formatter/src/other/parameters.rs index c3e251ebe97e5..ef5cb96bc0977 100644 --- a/crates/ruff_python_formatter/src/other/parameters.rs +++ b/crates/ruff_python_formatter/src/other/parameters.rs @@ -3,12 +3,12 @@ use std::usize; use ruff_formatter::{format_args, write, FormatRuleWithOptions}; use ruff_python_ast::Parameters; use ruff_python_ast::{AnyNodeRef, AstNode}; -use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer}; +use ruff_python_trivia::{CommentLinePosition, SimpleToken, SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::comments::{ dangling_comments, dangling_open_parenthesis_comments, leading_comments, leading_node_comments, - trailing_comments, CommentLinePosition, SourceComment, + trailing_comments, SourceComment, }; use crate::context::{NodeLevel, WithNodeLevel}; use crate::expression::parentheses::empty_parenthesized; diff --git a/crates/ruff_python_formatter/src/statement/clause.rs b/crates/ruff_python_formatter/src/statement/clause.rs index 7592c94f1c0e2..34cb2dfc5527e 100644 --- a/crates/ruff_python_formatter/src/statement/clause.rs +++ b/crates/ruff_python_formatter/src/statement/clause.rs @@ -7,13 +7,11 @@ use ruff_python_ast::{ use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{Ranged, TextRange, TextSize}; -use crate::comments::{ - leading_alternate_branch_comments, trailing_comments, SourceComment, SuppressionKind, -}; -use crate::prelude::*; +use crate::comments::{leading_alternate_branch_comments, trailing_comments, SourceComment}; use crate::preview::is_dummy_implementations_enabled; use crate::statement::suite::{contains_only_an_ellipsis, SuiteKind}; use crate::verbatim::write_suppressed_clause_header; +use crate::{has_skip_comment, prelude::*}; /// The header of a compound statement clause. /// @@ -354,7 +352,7 @@ impl<'ast> Format> for FormatClauseHeader<'_, 'ast> { leading_alternate_branch_comments(leading_comments, last_node).fmt(f)?; } - if SuppressionKind::has_skip_comment(self.trailing_colon_comment, f.context().source()) { + if has_skip_comment(self.trailing_colon_comment, f.context().source()) { write_suppressed_clause_header(self.header, f)?; } else { // Write a source map entry for the colon for range formatting to support formatting the clause header without diff --git a/crates/ruff_python_formatter/src/statement/stmt_ann_assign.rs b/crates/ruff_python_formatter/src/statement/stmt_ann_assign.rs index 5d0a2d2f31eed..aae1ff2473ae2 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_ann_assign.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_ann_assign.rs @@ -1,10 +1,9 @@ use ruff_formatter::write; use ruff_python_ast::StmtAnnAssign; -use crate::comments::{SourceComment, SuppressionKind}; +use crate::comments::SourceComment; use crate::expression::parentheses::Parentheses; use crate::expression::{has_parentheses, is_splittable_expression}; -use crate::prelude::*; use crate::preview::{ is_parenthesize_long_type_hints_enabled, is_prefer_splitting_right_hand_side_of_assignments_enabled, @@ -13,6 +12,7 @@ use crate::statement::stmt_assign::{ AnyAssignmentOperator, AnyBeforeOperator, FormatStatementsLastExpression, }; use crate::statement::trailing_semicolon; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatStmtAnnAssign; @@ -106,6 +106,6 @@ impl FormatNodeRule for FormatStmtAnnAssign { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_assert.rs b/crates/ruff_python_formatter/src/statement/stmt_assert.rs index 7b3930d252d74..91a85fe04925b 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_assert.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_assert.rs @@ -2,11 +2,11 @@ use ruff_formatter::prelude::{space, token}; use ruff_formatter::write; use ruff_python_ast::StmtAssert; -use crate::comments::{SourceComment, SuppressionKind}; +use crate::comments::SourceComment; use crate::expression::maybe_parenthesize_expression; use crate::expression::parentheses::Parenthesize; -use crate::prelude::*; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatStmtAssert; @@ -47,6 +47,6 @@ impl FormatNodeRule for FormatStmtAssert { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_assign.rs b/crates/ruff_python_formatter/src/statement/stmt_assign.rs index b709c1a08340f..a3e74aaa9b8eb 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_assign.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_assign.rs @@ -5,7 +5,7 @@ use ruff_python_ast::{ use crate::builders::parenthesize_if_expands; use crate::comments::{ - trailing_comments, Comments, LeadingDanglingTrailingComments, SourceComment, SuppressionKind, + trailing_comments, Comments, LeadingDanglingTrailingComments, SourceComment, }; use crate::context::{NodeLevel, WithNodeLevel}; use crate::expression::parentheses::{ @@ -16,12 +16,12 @@ use crate::expression::{ can_omit_optional_parentheses, has_own_parentheses, has_parentheses, maybe_parenthesize_expression, }; -use crate::prelude::*; use crate::preview::{ is_parenthesize_long_type_hints_enabled, is_prefer_splitting_right_hand_side_of_assignments_enabled, }; use crate::statement::trailing_semicolon; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatStmtAssign; @@ -110,7 +110,7 @@ impl FormatNodeRule for FormatStmtAssign { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_aug_assign.rs b/crates/ruff_python_formatter/src/statement/stmt_aug_assign.rs index 003c10a32a8e1..705b0989140a8 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_aug_assign.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_aug_assign.rs @@ -1,15 +1,15 @@ use ruff_formatter::write; use ruff_python_ast::StmtAugAssign; -use crate::comments::{SourceComment, SuppressionKind}; +use crate::comments::SourceComment; use crate::expression::parentheses::is_expression_parenthesized; -use crate::prelude::*; use crate::preview::is_prefer_splitting_right_hand_side_of_assignments_enabled; use crate::statement::stmt_assign::{ has_target_own_parentheses, AnyAssignmentOperator, AnyBeforeOperator, FormatStatementsLastExpression, }; use crate::statement::trailing_semicolon; +use crate::{has_skip_comment, prelude::*}; use crate::{AsFormat, FormatNodeRule}; #[derive(Default)] @@ -69,6 +69,6 @@ impl FormatNodeRule for FormatStmtAugAssign { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_break.rs b/crates/ruff_python_formatter/src/statement/stmt_break.rs index 67926e0a0e9ed..1e94c01f8b16b 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_break.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_break.rs @@ -1,7 +1,7 @@ use ruff_python_ast::StmtBreak; -use crate::comments::{SourceComment, SuppressionKind}; -use crate::prelude::*; +use crate::comments::SourceComment; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatStmtBreak; @@ -16,6 +16,6 @@ impl FormatNodeRule for FormatStmtBreak { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_continue.rs b/crates/ruff_python_formatter/src/statement/stmt_continue.rs index af045d241814b..39e14b511608b 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_continue.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_continue.rs @@ -1,7 +1,7 @@ use ruff_python_ast::StmtContinue; -use crate::comments::{SourceComment, SuppressionKind}; -use crate::prelude::*; +use crate::comments::SourceComment; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatStmtContinue; @@ -16,6 +16,6 @@ impl FormatNodeRule for FormatStmtContinue { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_delete.rs b/crates/ruff_python_formatter/src/statement/stmt_delete.rs index 4af373c801258..52158347c0fa0 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_delete.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_delete.rs @@ -3,10 +3,10 @@ use ruff_python_ast::StmtDelete; use ruff_text_size::Ranged; use crate::builders::{parenthesize_if_expands, PyFormatterExtensions}; -use crate::comments::{dangling_node_comments, SourceComment, SuppressionKind}; +use crate::comments::{dangling_node_comments, SourceComment}; use crate::expression::maybe_parenthesize_expression; use crate::expression::parentheses::Parenthesize; -use crate::prelude::*; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatStmtDelete; @@ -68,6 +68,6 @@ impl FormatNodeRule for FormatStmtDelete { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_expr.rs b/crates/ruff_python_formatter/src/statement/stmt_expr.rs index c0a6361b71c81..79f9c48abb1c8 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_expr.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_expr.rs @@ -1,12 +1,12 @@ use ruff_python_ast as ast; use ruff_python_ast::{Expr, Operator, StmtExpr}; -use crate::comments::{SourceComment, SuppressionKind}; +use crate::comments::SourceComment; use crate::expression::maybe_parenthesize_expression; use crate::expression::parentheses::Parenthesize; -use crate::prelude::*; use crate::statement::trailing_semicolon; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatStmtExpr; @@ -36,7 +36,7 @@ impl FormatNodeRule for FormatStmtExpr { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_global.rs b/crates/ruff_python_formatter/src/statement/stmt_global.rs index dac5c0b520c0d..ea70ee34a4230 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_global.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_global.rs @@ -2,7 +2,8 @@ use ruff_formatter::{format_args, write}; use ruff_python_ast::AstNode; use ruff_python_ast::StmtGlobal; -use crate::comments::{SourceComment, SuppressionKind}; +use crate::comments::SourceComment; +use crate::has_skip_comment; use crate::prelude::*; #[derive(Default)] @@ -53,6 +54,6 @@ impl FormatNodeRule for FormatStmtGlobal { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_import.rs b/crates/ruff_python_formatter/src/statement/stmt_import.rs index 97dc4fe830f08..8045ae7864db5 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_import.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_import.rs @@ -1,8 +1,8 @@ use ruff_formatter::{format_args, write}; use ruff_python_ast::StmtImport; -use crate::comments::{SourceComment, SuppressionKind}; -use crate::prelude::*; +use crate::comments::SourceComment; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatStmtImport; @@ -23,6 +23,6 @@ impl FormatNodeRule for FormatStmtImport { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_import_from.rs b/crates/ruff_python_formatter/src/statement/stmt_import_from.rs index e0535268a002e..7f8d1ce4e3fdd 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_import_from.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_import_from.rs @@ -4,8 +4,9 @@ use ruff_python_ast::StmtImportFrom; use ruff_text_size::Ranged; use crate::builders::{parenthesize_if_expands, PyFormatterExtensions, TrailingComma}; -use crate::comments::{SourceComment, SuppressionKind}; +use crate::comments::SourceComment; use crate::expression::parentheses::parenthesized; +use crate::has_skip_comment; use crate::other::identifier::DotDelimitedIdentifier; use crate::prelude::*; @@ -86,6 +87,6 @@ impl FormatNodeRule for FormatStmtImportFrom { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_ipy_escape_command.rs b/crates/ruff_python_formatter/src/statement/stmt_ipy_escape_command.rs index b41305f42abe8..66d52739db4bc 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_ipy_escape_command.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_ipy_escape_command.rs @@ -1,8 +1,8 @@ use ruff_python_ast::StmtIpyEscapeCommand; use ruff_text_size::Ranged; -use crate::comments::{SourceComment, SuppressionKind}; -use crate::prelude::*; +use crate::comments::SourceComment; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatStmtIpyEscapeCommand; @@ -17,6 +17,6 @@ impl FormatNodeRule for FormatStmtIpyEscapeCommand { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_nonlocal.rs b/crates/ruff_python_formatter/src/statement/stmt_nonlocal.rs index e39965e5b1c12..3280e0d93bbda 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_nonlocal.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_nonlocal.rs @@ -2,7 +2,8 @@ use ruff_formatter::{format_args, write}; use ruff_python_ast::AstNode; use ruff_python_ast::StmtNonlocal; -use crate::comments::{SourceComment, SuppressionKind}; +use crate::comments::SourceComment; +use crate::has_skip_comment; use crate::prelude::*; #[derive(Default)] @@ -53,6 +54,6 @@ impl FormatNodeRule for FormatStmtNonlocal { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_pass.rs b/crates/ruff_python_formatter/src/statement/stmt_pass.rs index d6a1e74564c63..bd542ec74bb00 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_pass.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_pass.rs @@ -1,7 +1,7 @@ use ruff_python_ast::StmtPass; -use crate::comments::{SourceComment, SuppressionKind}; -use crate::prelude::*; +use crate::comments::SourceComment; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatStmtPass; @@ -16,6 +16,6 @@ impl FormatNodeRule for FormatStmtPass { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_raise.rs b/crates/ruff_python_formatter/src/statement/stmt_raise.rs index 18c72e7672cb7..6a855e4418586 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_raise.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_raise.rs @@ -1,10 +1,10 @@ use ruff_formatter::write; use ruff_python_ast::StmtRaise; -use crate::comments::{SourceComment, SuppressionKind}; +use crate::comments::SourceComment; use crate::expression::maybe_parenthesize_expression; use crate::expression::parentheses::Parenthesize; -use crate::prelude::*; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatStmtRaise; @@ -48,6 +48,6 @@ impl FormatNodeRule for FormatStmtRaise { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_return.rs b/crates/ruff_python_formatter/src/statement/stmt_return.rs index 4eaa3d92613ab..ab782c02edbf2 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_return.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_return.rs @@ -1,10 +1,10 @@ use ruff_formatter::write; use ruff_python_ast::{Expr, StmtReturn}; -use crate::comments::{SourceComment, SuppressionKind}; +use crate::comments::SourceComment; use crate::expression::expr_tuple::TupleParentheses; -use crate::prelude::*; use crate::statement::stmt_assign::FormatStatementsLastExpression; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatStmtReturn; @@ -45,6 +45,6 @@ impl FormatNodeRule for FormatStmtReturn { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_formatter/src/statement/stmt_type_alias.rs b/crates/ruff_python_formatter/src/statement/stmt_type_alias.rs index b6ae69b4430af..6926543460761 100644 --- a/crates/ruff_python_formatter/src/statement/stmt_type_alias.rs +++ b/crates/ruff_python_formatter/src/statement/stmt_type_alias.rs @@ -1,12 +1,12 @@ use ruff_formatter::write; use ruff_python_ast::StmtTypeAlias; -use crate::comments::{SourceComment, SuppressionKind}; -use crate::prelude::*; +use crate::comments::SourceComment; use crate::preview::is_prefer_splitting_right_hand_side_of_assignments_enabled; use crate::statement::stmt_assign::{ AnyAssignmentOperator, AnyBeforeOperator, FormatStatementsLastExpression, }; +use crate::{has_skip_comment, prelude::*}; #[derive(Default)] pub struct FormatStmtTypeAlias; @@ -52,6 +52,6 @@ impl FormatNodeRule for FormatStmtTypeAlias { trailing_comments: &[SourceComment], context: &PyFormatContext, ) -> bool { - SuppressionKind::has_skip_comment(trailing_comments, context.source()) + has_skip_comment(trailing_comments, context.source()) } } diff --git a/crates/ruff_python_trivia/src/comments.rs b/crates/ruff_python_trivia/src/comments.rs new file mode 100644 index 0000000000000..6eb42c26f3074 --- /dev/null +++ b/crates/ruff_python_trivia/src/comments.rs @@ -0,0 +1,121 @@ +use ruff_text_size::TextRange; + +use crate::{is_python_whitespace, PythonWhitespace}; + +#[derive(Copy, Clone, Eq, PartialEq, Debug)] +pub enum SuppressionKind { + /// A `fmt: off` or `yapf: disable` comment + Off, + /// A `fmt: on` or `yapf: enable` comment + On, + /// A `fmt: skip` comment + Skip, +} + +impl SuppressionKind { + /// Attempts to identify the `kind` of a `comment`. + /// The comment string should be the full line with the comment on it. + pub fn from_comment(comment: &str) -> Option { + // Match against `# fmt: on`, `# fmt: off`, `# yapf: disable`, and `# yapf: enable`, which + // must be on their own lines. + let trimmed = comment + .strip_prefix('#') + .unwrap_or(comment) + .trim_whitespace(); + if let Some(command) = trimmed.strip_prefix("fmt:") { + match command.trim_whitespace_start() { + "off" => return Some(Self::Off), + "on" => return Some(Self::On), + "skip" => return Some(Self::Skip), + _ => {} + } + } else if let Some(command) = trimmed.strip_prefix("yapf:") { + match command.trim_whitespace_start() { + "disable" => return Some(Self::Off), + "enable" => return Some(Self::On), + _ => {} + } + } + + // Search for `# fmt: skip` comments, which can be interspersed with other comments (e.g., + // `# fmt: skip # noqa: E501`). + for segment in comment.split('#') { + let trimmed = segment.trim_whitespace(); + if let Some(command) = trimmed.strip_prefix("fmt:") { + if command.trim_whitespace_start() == "skip" { + return Some(SuppressionKind::Skip); + } + } + } + + None + } + + /// Returns true if this comment is a `fmt: off` or `yapf: disable` own line suppression comment. + pub fn is_suppression_on(slice: &str, position: CommentLinePosition) -> bool { + position.is_own_line() && matches!(Self::from_comment(slice), Some(Self::On)) + } + + /// Returns true if this comment is a `fmt: on` or `yapf: enable` own line suppression comment. + pub fn is_suppression_off(slice: &str, position: CommentLinePosition) -> bool { + position.is_own_line() && matches!(Self::from_comment(slice), Some(Self::Off)) + } +} +/// The position of a comment in the source text. +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +pub enum CommentLinePosition { + /// A comment that is on the same line as the preceding token and is separated by at least one line break from the following token. + /// + /// # Examples + /// + /// ## End of line + /// + /// ```python + /// a; # comment + /// b; + /// ``` + /// + /// `# comment` is an end of line comments because it is separated by at least one line break from the following token `b`. + /// Comments that not only end, but also start on a new line are [`OwnLine`](CommentLinePosition::OwnLine) comments. + EndOfLine, + + /// A Comment that is separated by at least one line break from the preceding token. + /// + /// # Examples + /// + /// ```python + /// a; + /// # comment + /// b; + /// ``` + /// + /// `# comment` line comments because they are separated by one line break from the preceding token `a`. + OwnLine, +} + +impl CommentLinePosition { + pub const fn is_own_line(self) -> bool { + matches!(self, Self::OwnLine) + } + + pub const fn is_end_of_line(self) -> bool { + matches!(self, Self::EndOfLine) + } + + /// 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())]; + + for c in before.chars().rev() { + match c { + '\n' | '\r' => { + break; + } + c if is_python_whitespace(c) => continue, + _ => return Self::EndOfLine, + } + } + Self::OwnLine + } +} diff --git a/crates/ruff_python_trivia/src/lib.rs b/crates/ruff_python_trivia/src/lib.rs index 9fed887c17ff4..782662170b6fa 100644 --- a/crates/ruff_python_trivia/src/lib.rs +++ b/crates/ruff_python_trivia/src/lib.rs @@ -1,4 +1,5 @@ mod comment_ranges; +mod comments; mod cursor; mod pragmas; pub mod textwrap; @@ -6,6 +7,7 @@ mod tokenizer; mod whitespace; pub use comment_ranges::CommentRanges; +pub use comments::*; pub use cursor::*; pub use pragmas::*; pub use tokenizer::*; diff --git a/ruff.schema.json b/ruff.schema.json index ae81665c41985..28d31208b0060 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3531,6 +3531,7 @@ "RUF025", "RUF026", "RUF027", + "RUF028", "RUF1", "RUF10", "RUF100",