Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Consider with statements for too many branches lint #11321

Merged
merged 4 commits into from
May 8, 2024
Merged

Consider with statements for too many branches lint #11321

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c8384ca
consider with statements for too many branches lint
blueraft May 7, 2024
3ffd381
add a test case for with stmt
blueraft May 7, 2024
fb41eaf
move comment to num_branches function
blueraft May 7, 2024
611f1f7
Merge branch 'main' into with-statements-too-many-branches
charliermarsh May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pylint/too_many_branches.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
Test for too many branches.
Taken from the pylint source 2023-02-03
"""
from contextlib import suppress

# pylint: disable=using-constant-test
def wrong(): # [too-many-branches]
""" Has too many branches. """
Expand Down Expand Up @@ -70,3 +72,33 @@ def nested_1():
pass
elif 7:
pass

def with_statement_wrong():
"""statements inside the with statement should get counted"""
with suppress(Exception):
if 1:
pass
elif 1:
pass
elif 1:
pass
elif 1:
pass
elif 1:
pass
elif 1:
pass
try:
pass
finally:
pass
if 2:
pass
while True:
pass
if 1:
pass
elif 2:
pass
elif 3:
pass
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/rules/too_many_branches.rs
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you include additional test cases in this file for with statements? They will be at the bottom of this file and they specifically tests the num_branches function.

Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ fn num_branches(stmts: &[Stmt]) -> usize {
.map(|case| num_branches(&case.body))
.sum::<usize>()
}
Stmt::With(ast::StmtWith { body, .. }) => num_branches(body),
dhruvmanila marked this conversation as resolved.
Show resolved Hide resolved
Stmt::For(ast::StmtFor { body, orelse, .. })
| Stmt::While(ast::StmtWhile { body, orelse, .. }) => {
1 + num_branches(body)
Expand Down
27 changes: 17 additions & 10 deletions ...les/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0912_too_many_branches.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,21 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
too_many_branches.py:6:5: PLR0912 Too many branches (13 > 12)
|
4 | """
5 | # pylint: disable=using-constant-test
6 | def wrong(): # [too-many-branches]
| ^^^^^ PLR0912
7 | """ Has too many branches. """
8 | if 1:
|

too_many_branches.py:8:5: PLR0912 Too many branches (13 > 12)
|
7 | # pylint: disable=using-constant-test
8 | def wrong(): # [too-many-branches]
| ^^^^^ PLR0912
9 | """ Has too many branches. """
10 | if 1:
|

too_many_branches.py:76:5: PLR0912 Too many branches (13 > 12)
|
74 | pass
75 |
76 | def with_statement_wrong():
| ^^^^^^^^^^^^^^^^^^^^ PLR0912
77 | """statements inside the with statement should get counted"""
78 | with suppress(Exception):
|
Loading