Skip to content

Commit

Permalink
Fix RUF028 not allowing # fmt: skip on match cases (#10178)
Browse files Browse the repository at this point in the history
## Summary

Fixes #10174 by allowing match cases to be enclosing nodes for
suppression comments. `else/elif` clauses are now also allowed to be
enclosing nodes.

## Test Plan
I've added the offending code from the original issue to the `RUF028`
snapshot test, and I've also expanded it to test the allowed `else/elif`
clause.
  • Loading branch information
snowsignal authored Mar 1, 2024
1 parent c9931a5 commit 8ecdf53
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 3 deletions.
26 changes: 26 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF028.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,29 @@ def fmt_on_trailing():
# fmt: off
val = 5 # fmt: on
pass # fmt: on


# all of these should be fine
def match_case_and_elif():
string = "hello"
match string:
case ("C"
| "CX"
| "R"
| "RX"
| "S"
| "SP"
| "WAP"
| "XX"
| "Y"
| "YY"
| "YZ"
| "Z"
| "ZZ"
): # fmt: skip
pass
case _: # fmt: skip
if string != "Hello":
pass
elif string == "Hello": # fmt: skip
pass
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl<'src, 'loc> UselessSuppressionComments<'src, 'loc> {
// check if the comment is inside of an expression.
if comment
.enclosing
.map(|n| !AnyNodeRef::is_statement(n))
.map(|n| !is_valid_enclosing_node(n))
.unwrap_or_default()
{
return Err(IgnoredReason::InNonStatement);
Expand Down Expand Up @@ -243,3 +243,101 @@ impl Display for IgnoredReason {
}
}
}

/// Checks if an enclosing node is allowed to enclose a suppression comment.
const fn is_valid_enclosing_node(node: AnyNodeRef) -> bool {
match node {
AnyNodeRef::ModModule(_)
| AnyNodeRef::ModExpression(_)
| AnyNodeRef::StmtFunctionDef(_)
| AnyNodeRef::StmtClassDef(_)
| AnyNodeRef::StmtReturn(_)
| AnyNodeRef::StmtDelete(_)
| AnyNodeRef::StmtTypeAlias(_)
| AnyNodeRef::StmtAssign(_)
| AnyNodeRef::StmtAugAssign(_)
| AnyNodeRef::StmtAnnAssign(_)
| AnyNodeRef::StmtFor(_)
| AnyNodeRef::StmtWhile(_)
| AnyNodeRef::StmtIf(_)
| AnyNodeRef::StmtWith(_)
| AnyNodeRef::StmtMatch(_)
| AnyNodeRef::StmtRaise(_)
| AnyNodeRef::StmtTry(_)
| AnyNodeRef::StmtAssert(_)
| AnyNodeRef::StmtImport(_)
| AnyNodeRef::StmtImportFrom(_)
| AnyNodeRef::StmtGlobal(_)
| AnyNodeRef::StmtNonlocal(_)
| AnyNodeRef::StmtExpr(_)
| AnyNodeRef::StmtPass(_)
| AnyNodeRef::StmtBreak(_)
| AnyNodeRef::StmtContinue(_)
| AnyNodeRef::StmtIpyEscapeCommand(_)
| AnyNodeRef::ExceptHandlerExceptHandler(_)
| AnyNodeRef::MatchCase(_)
| AnyNodeRef::ElifElseClause(_) => true,

AnyNodeRef::ExprBoolOp(_)
| AnyNodeRef::ExprNamedExpr(_)
| AnyNodeRef::ExprBinOp(_)
| AnyNodeRef::ExprUnaryOp(_)
| AnyNodeRef::ExprLambda(_)
| AnyNodeRef::ExprIfExp(_)
| AnyNodeRef::ExprDict(_)
| AnyNodeRef::ExprSet(_)
| AnyNodeRef::ExprListComp(_)
| AnyNodeRef::ExprSetComp(_)
| AnyNodeRef::ExprDictComp(_)
| AnyNodeRef::ExprGeneratorExp(_)
| AnyNodeRef::ExprAwait(_)
| AnyNodeRef::ExprYield(_)
| AnyNodeRef::ExprYieldFrom(_)
| AnyNodeRef::ExprCompare(_)
| AnyNodeRef::ExprCall(_)
| AnyNodeRef::FStringExpressionElement(_)
| AnyNodeRef::FStringLiteralElement(_)
| AnyNodeRef::FStringFormatSpec(_)
| AnyNodeRef::ExprFString(_)
| AnyNodeRef::ExprStringLiteral(_)
| AnyNodeRef::ExprBytesLiteral(_)
| AnyNodeRef::ExprNumberLiteral(_)
| AnyNodeRef::ExprBooleanLiteral(_)
| AnyNodeRef::ExprNoneLiteral(_)
| AnyNodeRef::ExprEllipsisLiteral(_)
| AnyNodeRef::ExprAttribute(_)
| AnyNodeRef::ExprSubscript(_)
| AnyNodeRef::ExprStarred(_)
| AnyNodeRef::ExprName(_)
| AnyNodeRef::ExprList(_)
| AnyNodeRef::ExprTuple(_)
| AnyNodeRef::ExprSlice(_)
| AnyNodeRef::ExprIpyEscapeCommand(_)
| AnyNodeRef::PatternMatchValue(_)
| AnyNodeRef::PatternMatchSingleton(_)
| AnyNodeRef::PatternMatchSequence(_)
| AnyNodeRef::PatternMatchMapping(_)
| AnyNodeRef::PatternMatchClass(_)
| AnyNodeRef::PatternMatchStar(_)
| AnyNodeRef::PatternMatchAs(_)
| AnyNodeRef::PatternMatchOr(_)
| AnyNodeRef::PatternArguments(_)
| AnyNodeRef::PatternKeyword(_)
| AnyNodeRef::Comprehension(_)
| AnyNodeRef::Arguments(_)
| AnyNodeRef::Parameters(_)
| AnyNodeRef::Parameter(_)
| AnyNodeRef::ParameterWithDefault(_)
| AnyNodeRef::Keyword(_)
| AnyNodeRef::Alias(_)
| AnyNodeRef::WithItem(_)
| AnyNodeRef::Decorator(_)
| AnyNodeRef::TypeParams(_)
| AnyNodeRef::TypeParamTypeVar(_)
| AnyNodeRef::TypeParamTypeVarTuple(_)
| AnyNodeRef::TypeParamParamSpec(_)
| AnyNodeRef::FString(_)
| AnyNodeRef::StringLiteral(_)
| AnyNodeRef::BytesLiteral(_) => false,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ RUF028.py:62:13: RUF028 [*] This suppression comment is invalid because it canno
62 |- val = 5 # fmt: on
62 |+ val = 5
63 63 | pass # fmt: on
64 64 |
65 65 |

RUF028.py:63:10: RUF028 [*] This suppression comment is invalid because it cannot be at the end of a line
|
Expand All @@ -210,5 +212,6 @@ RUF028.py:63:10: RUF028 [*] This suppression comment is invalid because it canno
62 62 | val = 5 # fmt: on
63 |- pass # fmt: on
63 |+ pass


64 64 |
65 65 |
66 66 | # all of these should be fine

0 comments on commit 8ecdf53

Please sign in to comment.