Consider match-case stmts for C901, PLR0912, and PLR0915#11521
Consider match-case stmts for C901, PLR0912, and PLR0915#11521dhruvmanila merged 2 commits intoastral-sh:mainfrom
C901, PLR0912, and PLR0915#11521Conversation
| match a: | ||
| case 30: | ||
| print('foo') | ||
| case _: |
There was a problem hiding this comment.
I'm not so sure about this, the equivalent else statement has the following check:
if clause.test.is_some() {
complexity += 1;
}There was a problem hiding this comment.
Can you clarify what you're not sure about? Is it that the catch-all case (_) acts as an else block of an if statement?
There was a problem hiding this comment.
Is it that the catch-all case (_) acts as an else block of an if statement?
Yep, complexity wise to me it's equivalent to the following if else statement.
if x == 30:
print("foo")
else:
print("bar")There was a problem hiding this comment.
Aren't they equivalent? We'd consider the above if else statement with complexity 2 and similarly for the following match statement:
match subject:
case foo: ...
case _: ...There was a problem hiding this comment.
The if else statement has a complexity of 1, the &clause.test.is_some() evaluates to false for the else.
[crates/ruff_linter/src/rules/mccabe/rules/function_is_too_complex.rs:413:9] &source = "\nif x == 3:\n print(\"foo\")\nelse:\n print(\"bar\")\n "
[crates/ruff_linter/src/rules/mccabe/rules/function_is_too_complex.rs:79:21] &clause.test.is_some() = false
[crates/ruff_linter/src/rules/mccabe/rules/function_is_too_complex.rs:415:9] &get_complexity_number(&stmts) = 1There was a problem hiding this comment.
Oh I see. Hmm, that's interesting. Yeah, I think we could do that i.e., check if the last case pattern is either a _ or any variable as both act as a catch-all pattern.
There was a problem hiding this comment.
I think only _ acts as a wild card pattern if my understanding of the documentation is correct. In any case, I can make a follow up PR to handle this. :)
There was a problem hiding this comment.
I think only
_acts as a wild card pattern if my understanding of the documentation is correct.
Yes, although a name pattern would also act in a similar way except that it also binds the value to the variable. So, in the following example foo will be same as subject. It's like doing foo = subject.
match subject:
case 2: ...
case foo: ...You can even see that in CPython where it raises a syntax error similar to _ if the name pattern is not the last pattern:
match 1:
case x:
print(x)
case 2:
...Running python test.py:
$ python3.13 parser/_.py
File "/Users/dhruv/playground/ruff/parser/_.py", line 2
case x:
^
SyntaxError: name capture 'x' makes remaining patterns unreachableIn any case, I can make a follow up PR to handle this. :)
Sounds good.
|
dhruvmanila
left a comment
There was a problem hiding this comment.
Thank you for working on this!
crates/ruff_linter/src/rules/mccabe/rules/function_is_too_complex.rs
Outdated
Show resolved
Hide resolved
C901, PLR0912, and PLR0915
## Summary Follow up to #11521 Removes the extra added complexity for catch all match cases. This matches the implementation of plain `else` statements. ## Test Plan Added new test cases. --------- Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
Resolves #11421
Summary
Instead of counting match/case as one statement, consider each
caseas a conditional.Test Plan
cargo test