From 8ecdf5369a4ed7c77b825617cd77bbaf9ded071b Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Fri, 1 Mar 2024 00:36:23 -0800 Subject: [PATCH] Fix RUF028 not allowing `# fmt: skip` on match cases (#10178) ## 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. --- .../resources/test/fixtures/ruff/RUF028.py | 26 +++++ .../invalid_formatter_suppression_comment.rs | 100 +++++++++++++++++- ..._rules__ruff__tests__RUF028_RUF028.py.snap | 7 +- 3 files changed, 130 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF028.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF028.py index 47100c2b3ef90..267bb4b5d87a3 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF028.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF028.py @@ -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 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 index f72e87a330d74..f5a5aaab0cfaf 100644 --- 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 @@ -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); @@ -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, + } +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF028_RUF028.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF028_RUF028.py.snap index 400fe6637ef52..a2846ab41fa89 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF028_RUF028.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF028_RUF028.py.snap @@ -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 | @@ -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