Implement RUF028 to detect useless formatter suppression comments (#9899)

<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
Fixes #6611

## Summary

This lint rule spots comments that are _intended_ to suppress or enable
the formatter, but will be ignored by the Ruff formatter.

We borrow some functions the formatter uses for determining comment
placement / putting them in context within an AST.

The analysis function uses an AST visitor to visit each comment and
attach it to the AST. It then uses that context to check:
1. Is this comment in an expression?
2. Does this comment have bad placement? (e.g. a `# fmt: skip` above a
function instead of at the end of a line)
3. Is this comment redundant?
4. Does this comment actually suppress any code?
5. Does this comment have ambiguous placement? (e.g. a `# fmt: off`
above an `else:` block)

If any of these are true, a violation is thrown. The reported reason
depends on the order of the above check-list: in other words, a `# fmt:
skip` comment on its own line within a list expression will be reported
as being in an expression, since that reason takes priority.

The lint suggests removing the comment as an unsafe fix, regardless of
the reason.

## Test Plan

A snapshot test has been created.
This commit is contained in:
Jane Lewis 2024-02-28 11:21:06 -08:00 committed by GitHub
parent 36bc725eaa
commit 0293908b71
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
41 changed files with 1215 additions and 438 deletions

View file

@ -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<PyFormatContext<'ast>> 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

View file

@ -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<StmtAnnAssign> for FormatStmtAnnAssign {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtAssert> for FormatStmtAssert {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtAssign> for FormatStmtAssign {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtAugAssign> for FormatStmtAugAssign {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtBreak> for FormatStmtBreak {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtContinue> for FormatStmtContinue {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtDelete> for FormatStmtDelete {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtExpr> for FormatStmtExpr {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtGlobal> for FormatStmtGlobal {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtImport> for FormatStmtImport {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtImportFrom> for FormatStmtImportFrom {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtIpyEscapeCommand> for FormatStmtIpyEscapeCommand {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtNonlocal> for FormatStmtNonlocal {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtPass> for FormatStmtPass {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtRaise> for FormatStmtRaise {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtReturn> for FormatStmtReturn {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}

View file

@ -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<StmtTypeAlias> for FormatStmtTypeAlias {
trailing_comments: &[SourceComment],
context: &PyFormatContext,
) -> bool {
SuppressionKind::has_skip_comment(trailing_comments, context.source())
has_skip_comment(trailing_comments, context.source())
}
}