Implement C901 (mccabe) (#765)

src/check_ast.rs

@@ -36,8 +36,8 @@ use crate::source_code_locator::SourceCodeLocator;
 use crate::visibility::{module_visibility, transition_scope, Modifier, Visibility, VisibleScope};
 use crate::{
     docstrings, flake8_2020, flake8_annotations, flake8_bandit, flake8_bugbear, flake8_builtins,
-    flake8_comprehensions, flake8_print, flake8_tidy_imports, pep8_naming, pycodestyle, pydocstyle,
-    pyflakes, pyupgrade,
+    flake8_comprehensions, flake8_print, flake8_tidy_imports, mccabe, pep8_naming, pycodestyle,
+    pydocstyle, pyflakes, pyupgrade,
 };
 
 const GLOBAL_SCOPE_INDEX: usize = 0;
@@ -349,6 +349,16 @@ where
                 if self.settings.enabled.contains(&CheckCode::B019) {
                     flake8_bugbear::plugins::cached_instance_method(self, decorator_list);
                 }
+                if self.settings.enabled.contains(&CheckCode::C901) {
+                    if let Some(check) = mccabe::checks::function_is_too_complex(
+                        stmt,
+                        name,
+                        body,
+                        self.settings.mccabe.max_complexity,
+                    ) {
+                        self.add_check(check);
+                    }
+                }
 
                 if self.settings.enabled.contains(&CheckCode::S107) {
                     self.add_checks(

src/checks.rs

@@ -120,6 +120,8 @@ pub enum CheckCode {
     C415,
     C416,
     C417,
+    // mccabe
+    C901,
     // flake8-tidy-imports
     I252,
     // flake8-print
@@ -260,6 +262,7 @@ pub enum CheckCategory {
     Flake8Quotes,
     Flake8Annotations,
     Flake82020,
+    McCabe,
     Ruff,
     Meta,
 }
@@ -282,6 +285,7 @@ impl CheckCategory {
             CheckCategory::Pyupgrade => "pyupgrade",
             CheckCategory::Pydocstyle => "pydocstyle",
             CheckCategory::PEP8Naming => "pep8-naming",
+            CheckCategory::McCabe => "mccabe",
             CheckCategory::Ruff => "Ruff-specific rules",
             CheckCategory::Meta => "Meta rules",
         }
@@ -314,6 +318,7 @@ impl CheckCategory {
             CheckCategory::Pydocstyle => Some("https://pypi.org/project/pydocstyle/6.1.1/"),
             CheckCategory::PEP8Naming => Some("https://pypi.org/project/pep8-naming/0.13.2/"),
             CheckCategory::Flake8Bandit => Some("https://pypi.org/project/flake8-bandit/4.1.1/"),
+            CheckCategory::McCabe => Some("https://pypi.org/project/mccabe/0.7.0/"),
             CheckCategory::Ruff => None,
             CheckCategory::Meta => None,
         }
@@ -546,6 +551,8 @@ pub enum CheckKind {
     HardcodedPasswordString(String),
     HardcodedPasswordFuncArg(String),
     HardcodedPasswordDefault(String),
+    // mccabe
+    FunctionIsTooComplex(String, usize),
     // Ruff
     AmbiguousUnicodeCharacterString(char, char),
     AmbiguousUnicodeCharacterDocstring(char, char),
@@ -826,6 +833,7 @@ impl CheckCode {
             CheckCode::S105 => CheckKind::HardcodedPasswordString("...".to_string()),
             CheckCode::S106 => CheckKind::HardcodedPasswordFuncArg("...".to_string()),
             CheckCode::S107 => CheckKind::HardcodedPasswordDefault("...".to_string()),
+            CheckCode::C901 => CheckKind::FunctionIsTooComplex("...".to_string(), 10),
             // Ruff
             CheckCode::RUF001 => CheckKind::AmbiguousUnicodeCharacterString('𝐁', 'B'),
             CheckCode::RUF002 => CheckKind::AmbiguousUnicodeCharacterDocstring('𝐁', 'B'),
@@ -1030,6 +1038,7 @@ impl CheckCode {
             CheckCode::S105 => CheckCategory::Flake8Bandit,
             CheckCode::S106 => CheckCategory::Flake8Bandit,
             CheckCode::S107 => CheckCategory::Flake8Bandit,
+            CheckCode::C901 => CheckCategory::McCabe,
             CheckCode::RUF001 => CheckCategory::Ruff,
             CheckCode::RUF002 => CheckCategory::Ruff,
             CheckCode::RUF003 => CheckCategory::Ruff,
@@ -1250,6 +1259,8 @@ impl CheckKind {
             CheckKind::HardcodedPasswordString(..) => &CheckCode::S105,
             CheckKind::HardcodedPasswordFuncArg(..) => &CheckCode::S106,
             CheckKind::HardcodedPasswordDefault(..) => &CheckCode::S107,
+            // McCabe
+            CheckKind::FunctionIsTooComplex(..) => &CheckCode::C901,
             // Ruff
             CheckKind::AmbiguousUnicodeCharacterString(..) => &CheckCode::RUF001,
             CheckKind::AmbiguousUnicodeCharacterDocstring(..) => &CheckCode::RUF002,
@@ -1902,6 +1913,10 @@ impl CheckKind {
             CheckKind::HardcodedPasswordDefault(string) => {
                 format!("Possible hardcoded password: `\"{string}\"`")
             }
+            // McCabe
+            CheckKind::FunctionIsTooComplex(name, complexity) => {
+                format!("`{name}` is too complex ({complexity})")
+            }
             // Ruff
             CheckKind::AmbiguousUnicodeCharacterString(confusable, representant) => {
                 format!(

src/checks_gen.rs

@@ -83,6 +83,9 @@ pub enum CheckCodePrefix {
     C415,
     C416,
     C417,
+    C9,
+    C90,
+    C901,
     D,
     D1,
     D10,
@@ -497,6 +500,7 @@ impl CheckCodePrefix {
                 CheckCode::C415,
                 CheckCode::C416,
                 CheckCode::C417,
+                CheckCode::C901,
             ],
             CheckCodePrefix::C4 => vec![
                 CheckCode::C400,
@@ -552,6 +556,9 @@ impl CheckCodePrefix {
             CheckCodePrefix::C415 => vec![CheckCode::C415],
             CheckCodePrefix::C416 => vec![CheckCode::C416],
             CheckCodePrefix::C417 => vec![CheckCode::C417],
+            CheckCodePrefix::C9 => vec![CheckCode::C901],
+            CheckCodePrefix::C90 => vec![CheckCode::C901],
+            CheckCodePrefix::C901 => vec![CheckCode::C901],
             CheckCodePrefix::D => vec![
                 CheckCode::D100,
                 CheckCode::D101,
@@ -1246,6 +1253,9 @@ impl CheckCodePrefix {
             CheckCodePrefix::C415 => PrefixSpecificity::Explicit,
             CheckCodePrefix::C416 => PrefixSpecificity::Explicit,
             CheckCodePrefix::C417 => PrefixSpecificity::Explicit,
+            CheckCodePrefix::C9 => PrefixSpecificity::Hundreds,
+            CheckCodePrefix::C90 => PrefixSpecificity::Tens,
+            CheckCodePrefix::C901 => PrefixSpecificity::Explicit,
             CheckCodePrefix::D => PrefixSpecificity::Category,
             CheckCodePrefix::D1 => PrefixSpecificity::Hundreds,
             CheckCodePrefix::D10 => PrefixSpecificity::Tens,

src/cli.rs

@@ -91,6 +91,9 @@ pub struct Cli {
     /// formatting.
     #[arg(long)]
     pub line_length: Option<usize>,
+    /// Max McCabe complexity allowed for a function.
+    #[arg(long)]
+    pub max_complexity: Option<usize>,
     /// Round-trip auto-formatting.
     // TODO(charlie): This should be a sub-command.
     #[arg(long, hide = true)]

src/lib.rs

@@ -41,6 +41,7 @@ mod isort;
 mod lex;
 pub mod linter;
 pub mod logging;
+pub mod mccabe;
 pub mod message;
 mod noqa;
 pub mod pep8_naming;

src/main.rs

@@ -269,6 +269,9 @@ fn inner_main() -> Result<ExitCode> {
     if let Some(line_length) = cli.line_length {
         configuration.line_length = line_length;
     }
+    if let Some(max_complexity) = cli.max_complexity {
+        configuration.mccabe.max_complexity = max_complexity;
+    }
     if let Some(target_version) = cli.target_version {
         configuration.target_version = target_version;
     }

src/mccabe/checks.rs

@@ -0,0 +1,73 @@
+use rustpython_ast::{ExcepthandlerKind, ExprKind, Stmt, StmtKind};
+
+use crate::ast::types::Range;
+use crate::checks::{Check, CheckKind};
+
+fn get_complexity_number(stmts: &[Stmt]) -> usize {
+    let mut complexity = 0;
+    for stmt in stmts {
+        match &stmt.node {
+            StmtKind::If { body, orelse, .. } => {
+                complexity += 1;
+                complexity += get_complexity_number(body);
+                complexity += get_complexity_number(orelse);
+            }
+            StmtKind::For { body, orelse, .. } | StmtKind::AsyncFor { body, orelse, .. } => {
+                complexity += 1;
+                complexity += get_complexity_number(body);
+                complexity += get_complexity_number(orelse);
+            }
+            StmtKind::While { test, body, orelse } => {
+                complexity += 1;
+                complexity += get_complexity_number(body);
+                complexity += get_complexity_number(orelse);
+                if let ExprKind::BoolOp { .. } = &test.node {
+                    complexity += 1;
+                }
+            }
+            StmtKind::Try {
+                body,
+                handlers,
+                orelse,
+                finalbody,
+            } => {
+                complexity += 1;
+                complexity += get_complexity_number(body);
+                complexity += get_complexity_number(orelse);
+                complexity += get_complexity_number(finalbody);
+                for handler in handlers {
+                    complexity += 1;
+                    let ExcepthandlerKind::ExceptHandler { body, .. } = &handler.node;
+                    complexity += get_complexity_number(body);
+                }
+            }
+            StmtKind::FunctionDef { body, .. } | StmtKind::AsyncFunctionDef { body, .. } => {
+                complexity += 1;
+                complexity += get_complexity_number(body);
+            }
+            StmtKind::ClassDef { body, .. } => {
+                complexity += 1;
+                complexity += get_complexity_number(body);
+            }
+            _ => {}
+        }
+    }
+    complexity
+}
+
+pub fn function_is_too_complex(
+    stmt: &Stmt,
+    name: &str,
+    body: &[Stmt],
+    max_complexity: usize,
+) -> Option<Check> {
+    let complexity = get_complexity_number(body) + 1;
+    if complexity > max_complexity {
+        Some(Check::new(
+            CheckKind::FunctionIsTooComplex(name.to_string(), complexity),
+            Range::from_located(stmt),
+        ))
+    } else {
+        None
+    }
+}

src/mccabe/mod.rs

@@ -0,0 +1,33 @@
+pub mod checks;
+pub mod settings;
+
+#[cfg(test)]
+mod tests {
+    use std::path::Path;
+
+    use anyhow::Result;
+    use test_case::test_case;
+
+    use crate::autofix::fixer;
+    use crate::checks::CheckCode;
+    use crate::linter::test_path;
+    use crate::{mccabe, Settings};
+
+    #[test_case(0)]
+    #[test_case(3)]
+    #[test_case(10)]
+    fn max_complexity_zero(max_complexity: usize) -> Result<()> {
+        let snapshot = format!("max_complexity_{}", max_complexity);
+        let mut checks = test_path(
+            Path::new("./resources/test/fixtures/C901.py"),
+            &Settings {
+                mccabe: mccabe::settings::Settings { max_complexity },
+                ..Settings::for_rules(vec![CheckCode::C901])
+            },
+            &fixer::Mode::Generate,
+        )?;
+        checks.sort_by_key(|check| check.location);
+        insta::assert_yaml_snapshot!(snapshot, checks);
+        Ok(())
+    }
+}

src/mccabe/settings.rs

@@ -0,0 +1,28 @@
+//! Settings for the `mccabe` plugin.
+
+use serde::{Deserialize, Serialize};
+
+#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Default)]
+#[serde(deny_unknown_fields, rename_all = "kebab-case")]
+pub struct Options {
+    pub max_complexity: Option<usize>,
+}
+
+#[derive(Debug, Hash)]
+pub struct Settings {
+    pub max_complexity: usize,
+}
+
+impl Settings {
+    pub fn from_options(options: Options) -> Self {
+        Self {
+            max_complexity: options.max_complexity.unwrap_or_default(),
+        }
+    }
+}
+
+impl Default for Settings {
+    fn default() -> Self {
+        Self { max_complexity: 10 }
+    }
+}

src/mccabe/snapshots/ruff__mccabe__tests__max_complexity_0.snap

@@ -0,0 +1,170 @@
+---
+source: src/mccabe/mod.rs
+expression: checks
+---
+- kind:
+    FunctionIsTooComplex:
+      - trivial
+      - 1
+  location:
+    row: 2
+    column: 0
+  end_location:
+    row: 7
+    column: 0
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - expr_as_statement
+      - 1
+  location:
+    row: 7
+    column: 0
+  end_location:
+    row: 12
+    column: 0
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - sequential
+      - 1
+  location:
+    row: 12
+    column: 0
+  end_location:
+    row: 19
+    column: 0
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - if_elif_else_dead_path
+      - 3
+  location:
+    row: 19
+    column: 0
+  end_location:
+    row: 29
+    column: 0
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - nested_ifs
+      - 3
+  location:
+    row: 29
+    column: 0
+  end_location:
+    row: 40
+    column: 0
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - for_loop
+      - 2
+  location:
+    row: 40
+    column: 0
+  end_location:
+    row: 46
+    column: 0
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - for_else
+      - 2
+  location:
+    row: 46
+    column: 0
+  end_location:
+    row: 54
+    column: 0
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - recursive
+      - 2
+  location:
+    row: 54
+    column: 0
+  end_location:
+    row: 62
+    column: 0
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - nested_functions
+      - 3
+  location:
+    row: 62
+    column: 0
+  end_location:
+    row: 73
+    column: 0
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - a
+      - 2
+  location:
+    row: 63
+    column: 4
+  end_location:
+    row: 69
+    column: 4
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - b
+      - 1
+  location:
+    row: 64
+    column: 8
+  end_location:
+    row: 67
+    column: 8
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - try_else
+      - 4
+  location:
+    row: 73
+    column: 0
+  end_location:
+    row: 85
+    column: 0
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - nested_try_finally
+      - 3
+  location:
+    row: 85
+    column: 0
+  end_location:
+    row: 96
+    column: 0
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - foobar
+      - 3
+  location:
+    row: 96
+    column: 0
+  end_location:
+    row: 107
+    column: 0
+  fix: ~
+- kind:
+    FunctionIsTooComplex:
+      - annotated_assign
+      - 1
+  location:
+    row: 107
+    column: 0
+  end_location:
+    row: 109
+    column: 0
+  fix: ~
+

src/mccabe/snapshots/ruff__mccabe__tests__max_complexity_10.snap

@@ -0,0 +1,6 @@
+---
+source: src/mccabe/mod.rs
+expression: checks
+---
+[]
+

src/mccabe/snapshots/ruff__mccabe__tests__max_complexity_3.snap

@@ -0,0 +1,16 @@
+---
+source: src/mccabe/mod.rs
+expression: checks
+---
+- kind:
+    FunctionIsTooComplex:
+      - try_else
+      - 4
+  location:
+    row: 73
+    column: 0
+  end_location:
+    row: 85
+    column: 0
+  fix: ~
+

src/settings/configuration.rs

@@ -13,7 +13,8 @@ use crate::checks_gen::CheckCodePrefix;
 use crate::settings::pyproject::load_options;
 use crate::settings::types::{FilePattern, PerFileIgnore, PythonVersion};
 use crate::{
-    flake8_annotations, flake8_bugbear, flake8_quotes, flake8_tidy_imports, fs, isort, pep8_naming,
+    flake8_annotations, flake8_bugbear, flake8_quotes, flake8_tidy_imports, fs, isort, mccabe,
+    pep8_naming,
 };
 
 #[derive(Debug)]
@@ -36,6 +37,7 @@ pub struct Configuration {
     pub flake8_quotes: flake8_quotes::settings::Settings,
     pub flake8_tidy_imports: flake8_tidy_imports::settings::Settings,
     pub isort: isort::settings::Settings,
+    pub mccabe: mccabe::settings::Settings,
     pub pep8_naming: pep8_naming::settings::Settings,
 }
 
@@ -153,6 +155,10 @@ impl Configuration {
                 .isort
                 .map(isort::settings::Settings::from_options)
                 .unwrap_or_default(),
+            mccabe: options
+                .mccabe
+                .map(mccabe::settings::Settings::from_options)
+                .unwrap_or_default(),
             pep8_naming: options
                 .pep8_naming
                 .map(pep8_naming::settings::Settings::from_options)

src/settings/mod.rs

@@ -14,7 +14,8 @@ use crate::checks_gen::{CheckCodePrefix, PrefixSpecificity};
 use crate::settings::configuration::Configuration;
 use crate::settings::types::{FilePattern, PerFileIgnore, PythonVersion};
 use crate::{
-    flake8_annotations, flake8_bugbear, flake8_quotes, flake8_tidy_imports, isort, pep8_naming,
+    flake8_annotations, flake8_bugbear, flake8_quotes, flake8_tidy_imports, isort, mccabe,
+    pep8_naming,
 };
 
 pub mod configuration;
@@ -39,6 +40,7 @@ pub struct Settings {
     pub flake8_quotes: flake8_quotes::settings::Settings,
     pub flake8_tidy_imports: flake8_tidy_imports::settings::Settings,
     pub isort: isort::settings::Settings,
+    pub mccabe: mccabe::settings::Settings,
     pub pep8_naming: pep8_naming::settings::Settings,
 }
 
@@ -59,6 +61,7 @@ impl Settings {
             flake8_quotes: config.flake8_quotes,
             flake8_tidy_imports: config.flake8_tidy_imports,
             isort: config.isort,
+            mccabe: config.mccabe,
             line_length: config.line_length,
             pep8_naming: config.pep8_naming,
             per_file_ignores: config.per_file_ignores,
@@ -82,6 +85,7 @@ impl Settings {
             flake8_quotes: Default::default(),
             flake8_tidy_imports: Default::default(),
             isort: Default::default(),
+            mccabe: Default::default(),
             pep8_naming: Default::default(),
         }
     }
@@ -101,6 +105,7 @@ impl Settings {
             flake8_quotes: Default::default(),
             flake8_tidy_imports: Default::default(),
             isort: Default::default(),
+            mccabe: Default::default(),
             pep8_naming: Default::default(),
         }
     }
@@ -124,6 +129,7 @@ impl Hash for Settings {
         self.flake8_quotes.hash(state);
         self.flake8_tidy_imports.hash(state);
         self.isort.hash(state);
+        self.mccabe.hash(state);
         self.pep8_naming.hash(state);
     }
 }

src/settings/options.rs

@@ -6,7 +6,8 @@ use serde::{Deserialize, Serialize};
 use crate::checks_gen::CheckCodePrefix;
 use crate::settings::types::PythonVersion;
 use crate::{
-    flake8_annotations, flake8_bugbear, flake8_quotes, flake8_tidy_imports, isort, pep8_naming,
+    flake8_annotations, flake8_bugbear, flake8_quotes, flake8_tidy_imports, isort, mccabe,
+    pep8_naming,
 };
 
 #[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Default)]
@@ -29,6 +30,7 @@ pub struct Options {
     pub flake8_quotes: Option<flake8_quotes::settings::Options>,
     pub flake8_tidy_imports: Option<flake8_tidy_imports::settings::Options>,
     pub isort: Option<isort::settings::Options>,
+    pub mccabe: Option<mccabe::settings::Options>,
     pub pep8_naming: Option<pep8_naming::settings::Options>,
     // Tables are required to go last.
     pub per_file_ignores: Option<FnvHashMap<String, Vec<CheckCodePrefix>>>,

src/settings/pyproject.rs

@@ -110,7 +110,7 @@ mod tests {
         find_project_root, find_pyproject_toml, parse_pyproject_toml, Options, Pyproject, Tools,
     };
     use crate::settings::types::PatternPrefixPair;
-    use crate::{flake8_bugbear, flake8_quotes, flake8_tidy_imports, pep8_naming};
+    use crate::{flake8_bugbear, flake8_quotes, flake8_tidy_imports, mccabe, pep8_naming};
 
     #[test]
     fn deserialize() -> Result<()> {
@@ -151,6 +151,7 @@ mod tests {
                     flake8_quotes: None,
                     flake8_tidy_imports: None,
                     isort: None,
+                    mccabe: None,
                     pep8_naming: None,
                 })
             })
@@ -184,6 +185,7 @@ line-length = 79
                     flake8_quotes: None,
                     flake8_tidy_imports: None,
                     isort: None,
+                    mccabe: None,
                     pep8_naming: None,
                 })
             })
@@ -217,6 +219,7 @@ exclude = ["foo.py"]
                     flake8_quotes: None,
                     flake8_tidy_imports: None,
                     isort: None,
+                    mccabe: None,
                     pep8_naming: None,
                 })
             })
@@ -250,6 +253,7 @@ select = ["E501"]
                     flake8_quotes: None,
                     flake8_tidy_imports: None,
                     isort: None,
+                    mccabe: None,
                     pep8_naming: None,
                 })
             })
@@ -284,6 +288,7 @@ ignore = ["E501"]
                     flake8_quotes: None,
                     flake8_tidy_imports: None,
                     isort: None,
+                    mccabe: None,
                     pep8_naming: None,
                 })
             })
@@ -376,6 +381,9 @@ other-attribute = 1
                     ban_relative_imports: Some(Strictness::Parents)
                 }),
                 isort: None,
+                mccabe: Some(mccabe::settings::Options {
+                    max_complexity: Some(10),
+                }),
                 pep8_naming: Some(pep8_naming::settings::Options {
                     ignore_names: Some(vec![
                         "setUp".to_string(),