Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Invalid RUF028 for match case header #10174

Closed
qartik opened this issue Feb 29, 2024 · 1 comment · Fixed by #10178
Closed

Invalid RUF028 for match case header #10174

qartik opened this issue Feb 29, 2024 · 1 comment · Fixed by #10178
Assignees
@snowsignal
Labels
bug Something isn't working rule Implementing or modifying a lint rule

Comments

@qartik
Copy link

qartik commented Feb 29, 2024

The doc suggests:

# fmt: skip comments suppress formatting for a preceding statement, case header, decorator, function definition, or class definition

but for the following use case, I get a RUF028 warning. I believe this should be acceptable.

string = "hello"
match string:
    case ("C"
        | "CX"
        | "R"
        | "RX"
        | "S"
        | "SP"
        | "WAP"
        | "XX"
        | "Y"
        | "YY"
        | "YZ"
        | "Z"
        | "ZZ"
    ):  # fmt: skip
        pass
    case _:
        pass
❯ ruff --version
ruff 0.3.0

relevant ruff.toml setting:

[format]
preview = true
@AlexWaygood AlexWaygood added bug Something isn't working rule Implementing or modifying a lint rule labels Feb 29, 2024
@MichaReiser
Copy link
Member

MichaReiser commented Feb 29, 2024 •
edited
Loading

I agree. This looks correct.
@snowsignal would you mind taking a look at this?

@snowsignal snowsignal mentioned this issue Mar 1, 2024
Merged
snowsignal added a commit that referenced this issue Mar 1, 2024
@snowsignal
Fix RUF028 not allowing # fmt: skip on match cases (#10178)
8ecdf53 
## 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.
nkxxll pushed a commit to nkxxll/ruff that referenced this issue Mar 10, 2024
@snowsignal @nkxxll
Fix RUF028 not allowing # fmt: skip on match cases (astral-sh#10178)
bc467c8 
## Summary

Fixes astral-sh#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@snowsignal snowsignal

Labels
bug Something isn't working rule Implementing or modifying a lint rule
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix RUF028 not allowing # fmt: skip on match cases astral-sh/ruff
4 participants
@MichaReiser @snowsignal @AlexWaygood @qartik