From 344daebb1b136a9cf059518c09ef88e7940e2d28 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 14 Mar 2023 14:40:33 -0400 Subject: [PATCH] Refine complexity rules for try-except-else-finally (#3519) --- crates/ruff/src/rules/mccabe/rules.rs | 42 +++++++++++++++---- ...ules__mccabe__tests__max_complexity_0.snap | 2 +- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/crates/ruff/src/rules/mccabe/rules.rs b/crates/ruff/src/rules/mccabe/rules.rs index 79c7500e2a..71febbc601 100644 --- a/crates/ruff/src/rules/mccabe/rules.rs +++ b/crates/ruff/src/rules/mccabe/rules.rs @@ -1,4 +1,4 @@ -use rustpython_parser::ast::{ExcepthandlerKind, ExprKind, Stmt, StmtKind}; +use rustpython_parser::ast::{ExcepthandlerKind, Stmt, StmtKind}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; @@ -74,13 +74,10 @@ fn get_complexity_number(stmts: &[Stmt]) -> usize { complexity += get_complexity_number(body); complexity += get_complexity_number(orelse); } - StmtKind::While { test, body, orelse } => { + StmtKind::While { body, orelse, .. } => { complexity += 1; complexity += get_complexity_number(body); complexity += get_complexity_number(orelse); - if let ExprKind::BoolOp { .. } = &test.node { - complexity += 1; - } } StmtKind::Match { cases, .. } => { complexity += 1; @@ -100,8 +97,10 @@ fn get_complexity_number(stmts: &[Stmt]) -> usize { orelse, finalbody, } => { - complexity += 1; complexity += get_complexity_number(body); + if !orelse.is_empty() { + complexity += 1; + } complexity += get_complexity_number(orelse); complexity += get_complexity_number(finalbody); for handler in handlers { @@ -307,7 +306,7 @@ def nested_try_finally(): print(3) "#; let stmts = parser::parse_program(source, "")?; - assert_eq!(get_complexity_number(&stmts), 3); + assert_eq!(get_complexity_number(&stmts), 1); Ok(()) } @@ -374,4 +373,33 @@ class Class: assert_eq!(get_complexity_number(&stmts), 9); Ok(()) } + + #[test] + fn finally() -> Result<()> { + let source = r#" +def process_detect_lines(): + try: + pass + finally: + pass +"#; + let stmts = parser::parse_program(source, "")?; + assert_eq!(get_complexity_number(&stmts), 1); + Ok(()) + } + + #[test] + fn if_in_finally() -> Result<()> { + let source = r#" +def process_detect_lines(): + try: + pass + finally: + if res: + errors.append(f"Non-zero exit code {res}") +"#; + let stmts = parser::parse_program(source, "")?; + assert_eq!(get_complexity_number(&stmts), 2); + Ok(()) + } } diff --git a/crates/ruff/src/rules/mccabe/snapshots/ruff__rules__mccabe__tests__max_complexity_0.snap b/crates/ruff/src/rules/mccabe/snapshots/ruff__rules__mccabe__tests__max_complexity_0.snap index 1a7c576b71..632935557d 100644 --- a/crates/ruff/src/rules/mccabe/snapshots/ruff__rules__mccabe__tests__max_complexity_0.snap +++ b/crates/ruff/src/rules/mccabe/snapshots/ruff__rules__mccabe__tests__max_complexity_0.snap @@ -160,7 +160,7 @@ expression: diagnostics parent: ~ - kind: name: ComplexStructure - body: "`nested_try_finally` is too complex (3)" + body: "`nested_try_finally` is too complex (1)" suggestion: ~ fixable: false location: