From c8384ca4101f2a06e318a0f2902f0f881c6bbf45 Mon Sep 17 00:00:00 2001 From: Ahmed Ilyas Date: Tue, 7 May 2024 12:05:50 +0200 Subject: [PATCH 1/3] consider with statements for too many branches lint --- .../test/fixtures/pylint/too_many_branches.py | 32 +++++++++++++++++++ .../rules/pylint/rules/too_many_branches.rs | 1 + ...__tests__PLR0912_too_many_branches.py.snap | 27 ++++++++++------ 3 files changed, 50 insertions(+), 10 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/too_many_branches.py b/crates/ruff_linter/resources/test/fixtures/pylint/too_many_branches.py index b19314dc972ec..2bc360ca13bef 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/too_many_branches.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/too_many_branches.py @@ -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. """ @@ -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 diff --git a/crates/ruff_linter/src/rules/pylint/rules/too_many_branches.rs b/crates/ruff_linter/src/rules/pylint/rules/too_many_branches.rs index ca14e12dcc5e2..08f7e500e947e 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/too_many_branches.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/too_many_branches.rs @@ -107,6 +107,7 @@ fn num_branches(stmts: &[Stmt]) -> usize { .map(|case| num_branches(&case.body)) .sum::() } + Stmt::With(ast::StmtWith { body, .. }) => num_branches(body), Stmt::For(ast::StmtFor { body, orelse, .. }) | Stmt::While(ast::StmtWhile { body, orelse, .. }) => { 1 + num_branches(body) diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0912_too_many_branches.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0912_too_many_branches.py.snap index 690939ac3ff5e..559e5d6210760 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0912_too_many_branches.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR0912_too_many_branches.py.snap @@ -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): + | From 3ffd381af307de3e394eb3a9706090f62788c081 Mon Sep 17 00:00:00 2001 From: Ahmed Ilyas Date: Tue, 7 May 2024 13:15:43 +0200 Subject: [PATCH 2/3] add a test case for with stmt --- .../src/rules/pylint/rules/too_many_branches.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/ruff_linter/src/rules/pylint/rules/too_many_branches.rs b/crates/ruff_linter/src/rules/pylint/rules/too_many_branches.rs index 08f7e500e947e..bdd6b3f69d3db 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/too_many_branches.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/too_many_branches.rs @@ -266,4 +266,20 @@ finally: test_helper(source, 5)?; Ok(()) } + + #[test] + fn with_statement() -> Result<()> { + let source: &str = r" +with suppress(Exception): + if x == 0: # 2 + return + else: + return +"; + + // The `with` statement is not considered a branch but + // the statements inside the `with` should be counted + test_helper(source, 2)?; + Ok(()) + } } From fb41eaf81020a3518505fd2cb123e30e45636ac3 Mon Sep 17 00:00:00 2001 From: Ahmed Ilyas Date: Tue, 7 May 2024 13:51:44 +0200 Subject: [PATCH 3/3] move comment to num_branches function --- .../ruff_linter/src/rules/pylint/rules/too_many_branches.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/src/rules/pylint/rules/too_many_branches.rs b/crates/ruff_linter/src/rules/pylint/rules/too_many_branches.rs index bdd6b3f69d3db..9a1ea12053672 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/too_many_branches.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/too_many_branches.rs @@ -107,7 +107,8 @@ fn num_branches(stmts: &[Stmt]) -> usize { .map(|case| num_branches(&case.body)) .sum::() } - Stmt::With(ast::StmtWith { body, .. }) => num_branches(body), + Stmt::With(ast::StmtWith { body, .. }) => num_branches(body), // The `with` statement + // is not considered a branch but the statements inside the `with` should be counted Stmt::For(ast::StmtFor { body, orelse, .. }) | Stmt::While(ast::StmtWhile { body, orelse, .. }) => { 1 + num_branches(body) @@ -277,8 +278,6 @@ with suppress(Exception): return "; - // The `with` statement is not considered a branch but - // the statements inside the `with` should be counted test_helper(source, 2)?; Ok(()) }