diff --git a/README.md b/README.md index 4190890dd4..689733b3f9 100644 --- a/README.md +++ b/README.md @@ -369,12 +369,16 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com | N803 | InvalidArgumentName | Argument name `...` should be lowercase | | | N804 | InvalidFirstArgumentNameForClassMethod | First argument of a class method should be named `cls` | | | N805 | InvalidFirstArgumentNameForMethod | First argument of a method should be named `self` | | +| N806 | NonLowercaseVariableInFunction | Variable `...` in function should be lowercase | | | N807 | DunderFunctionName | Function name should not start and end with `__` | | | N811 | ConstantImportedAsNonConstant | Constant `...` imported as non-constant `...` | | | N812 | LowercaseImportedAsNonLowercase | Lowercase `...` imported as non-lowercase `...` | | | N813 | CamelcaseImportedAsLowercase | Camelcase `...` imported as lowercase `...` | | | N814 | CamelcaseImportedAsConstant | Camelcase `...` imported as constant `...` | | +| N815 | MixedCaseVariableInClassScope | Variable `mixedCase` in class scope should not be mixedCase | | +| N816 | MixedCaseVariableInGlobalScope | Variable `mixedCase` in global scope should not be mixedCase | | | N817 | CamelcaseImportedAsAcronym | Camelcase `...` imported as acronym `...` | | +| N818 | ErrorSuffixOnExceptionName | Exception name `...` should be named with an Error suffix | | ### flake8-comprehensions @@ -498,6 +502,7 @@ Ruff re-implements some of the most popular Flake8 plugins and related code qual including: - [`pydocstyle`](https://pypi.org/project/pydocstyle/) +- [`pep8-naming`](https://pypi.org/project/pep8-naming/) - [`yesqa`](https://github.com/asottile/yesqa) - [`flake8-docstrings`](https://pypi.org/project/flake8-docstrings/) - [`flake8-builtins`](https://pypi.org/project/flake8-builtins/) @@ -520,6 +525,7 @@ Beyond rule-set parity, Ruff suffers from the following limitations vis-à-vis F Today, Ruff can be used to replace Flake8 when used with any of the following plugins: +- [`pep8-naming`](https://pypi.org/project/pep8-naming/) - [`flake8-docstrings`](https://pypi.org/project/flake8-docstrings/) - [`flake8-builtins`](https://pypi.org/project/flake8-builtins/) - [`flake8-super`](https://pypi.org/project/flake8-super/) diff --git a/resources/test/fixtures/N806.py b/resources/test/fixtures/N806.py new file mode 100644 index 0000000000..891129de03 --- /dev/null +++ b/resources/test/fixtures/N806.py @@ -0,0 +1,4 @@ +def f(): + lower = 0 + Camel = 0 + CONSTANT = 0 diff --git a/resources/test/fixtures/N815.py b/resources/test/fixtures/N815.py new file mode 100644 index 0000000000..077ba0e85e --- /dev/null +++ b/resources/test/fixtures/N815.py @@ -0,0 +1,6 @@ +class C: + lower = 0 + CONSTANT = 0 + mixedCase = 0 + _mixedCase = 0 + mixed_Case = 0 diff --git a/resources/test/fixtures/N816.py b/resources/test/fixtures/N816.py new file mode 100644 index 0000000000..abed196632 --- /dev/null +++ b/resources/test/fixtures/N816.py @@ -0,0 +1,5 @@ +lower = 0 +CONSTANT = 0 +mixedCase = 0 +_mixedCase = 0 +mixed_Case = 0 diff --git a/resources/test/fixtures/N818.py b/resources/test/fixtures/N818.py new file mode 100644 index 0000000000..a6150b9316 --- /dev/null +++ b/resources/test/fixtures/N818.py @@ -0,0 +1,10 @@ +class Error(Exception): + pass + + +class AnotherError(Exception): + pass + + +class C(Exception): + pass diff --git a/src/check_ast.rs b/src/check_ast.rs index 9b278d271d..f5652b622d 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -363,6 +363,14 @@ where } } + if self.settings.enabled.contains(&CheckCode::N818) { + if let Some(check) = + pep8_naming::checks::error_suffix_on_exception_name(stmt, bases, name) + { + self.checks.push(check); + } + } + self.check_builtin_shadowing( name, self.locate_check(Range::from_located(stmt)), @@ -1799,6 +1807,30 @@ impl<'a> Checker<'a> { } } + if self.settings.enabled.contains(&CheckCode::N806) { + if let Some(check) = + pep8_naming::checks::non_lowercase_variable_in_function(current, expr, id) + { + self.checks.push(check); + } + } + + if self.settings.enabled.contains(&CheckCode::N815) { + if let Some(check) = + pep8_naming::checks::mixed_case_variable_in_class_scope(current, expr, id) + { + self.checks.push(check); + } + } + + if self.settings.enabled.contains(&CheckCode::N816) { + if let Some(check) = + pep8_naming::checks::mixed_case_variable_in_global_scope(current, expr, id) + { + self.checks.push(check); + } + } + if matches!(parent.node, StmtKind::AnnAssign { value: None, .. }) { self.add_binding( id.to_string(), diff --git a/src/checks.rs b/src/checks.rs index 741d9ebba1..d364b96744 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -168,12 +168,16 @@ pub enum CheckCode { N803, N804, N805, + N806, N807, N811, N812, N813, N814, + N815, + N816, N817, + N818, // Meta M001, } @@ -372,12 +376,16 @@ pub enum CheckKind { InvalidArgumentName(String), InvalidFirstArgumentNameForClassMethod, InvalidFirstArgumentNameForMethod, + NonLowercaseVariableInFunction(String), DunderFunctionName, ConstantImportedAsNonConstant(String, String), LowercaseImportedAsNonLowercase(String, String), CamelcaseImportedAsLowercase(String, String), CamelcaseImportedAsConstant(String, String), + MixedCaseVariableInClassScope(String), + MixedCaseVariableInGlobalScope(String), CamelcaseImportedAsAcronym(String, String), + ErrorSuffixOnExceptionName(String), // Meta UnusedNOQA(Option>), } @@ -567,6 +575,7 @@ impl CheckCode { CheckCode::N803 => CheckKind::InvalidArgumentName("...".to_string()), CheckCode::N804 => CheckKind::InvalidFirstArgumentNameForClassMethod, CheckCode::N805 => CheckKind::InvalidFirstArgumentNameForMethod, + CheckCode::N806 => CheckKind::NonLowercaseVariableInFunction("...".to_string()), CheckCode::N807 => CheckKind::DunderFunctionName, CheckCode::N811 => { CheckKind::ConstantImportedAsNonConstant("...".to_string(), "...".to_string()) @@ -580,9 +589,12 @@ impl CheckCode { CheckCode::N814 => { CheckKind::CamelcaseImportedAsConstant("...".to_string(), "...".to_string()) } + CheckCode::N815 => CheckKind::MixedCaseVariableInClassScope("mixedCase".to_string()), + CheckCode::N816 => CheckKind::MixedCaseVariableInGlobalScope("mixedCase".to_string()), CheckCode::N817 => { CheckKind::CamelcaseImportedAsAcronym("...".to_string(), "...".to_string()) } + CheckCode::N818 => CheckKind::ErrorSuffixOnExceptionName("...".to_string()), // Meta CheckCode::M001 => CheckKind::UnusedNOQA(None), } @@ -722,12 +734,16 @@ impl CheckCode { CheckCode::N803 => CheckCategory::PEP8Naming, CheckCode::N804 => CheckCategory::PEP8Naming, CheckCode::N805 => CheckCategory::PEP8Naming, + CheckCode::N806 => CheckCategory::PEP8Naming, CheckCode::N807 => CheckCategory::PEP8Naming, CheckCode::N811 => CheckCategory::PEP8Naming, CheckCode::N812 => CheckCategory::PEP8Naming, CheckCode::N813 => CheckCategory::PEP8Naming, CheckCode::N814 => CheckCategory::PEP8Naming, + CheckCode::N815 => CheckCategory::PEP8Naming, + CheckCode::N816 => CheckCategory::PEP8Naming, CheckCode::N817 => CheckCategory::PEP8Naming, + CheckCode::N818 => CheckCategory::PEP8Naming, CheckCode::M001 => CheckCategory::Meta, } } @@ -879,12 +895,16 @@ impl CheckKind { CheckKind::InvalidArgumentName(_) => &CheckCode::N803, CheckKind::InvalidFirstArgumentNameForClassMethod => &CheckCode::N804, CheckKind::InvalidFirstArgumentNameForMethod => &CheckCode::N805, + CheckKind::NonLowercaseVariableInFunction(..) => &CheckCode::N806, CheckKind::DunderFunctionName => &CheckCode::N807, CheckKind::ConstantImportedAsNonConstant(..) => &CheckCode::N811, CheckKind::LowercaseImportedAsNonLowercase(..) => &CheckCode::N812, CheckKind::CamelcaseImportedAsLowercase(..) => &CheckCode::N813, CheckKind::CamelcaseImportedAsConstant(..) => &CheckCode::N814, + CheckKind::MixedCaseVariableInClassScope(..) => &CheckCode::N815, + CheckKind::MixedCaseVariableInGlobalScope(..) => &CheckCode::N816, CheckKind::CamelcaseImportedAsAcronym(..) => &CheckCode::N817, + CheckKind::ErrorSuffixOnExceptionName(..) => &CheckCode::N818, // Meta CheckKind::UnusedNOQA(_) => &CheckCode::M001, } @@ -1305,6 +1325,9 @@ impl CheckKind { CheckKind::InvalidFirstArgumentNameForMethod => { "First argument of a method should be named `self`".to_string() } + CheckKind::NonLowercaseVariableInFunction(name) => { + format!("Variable `{name}` in function should be lowercase") + } CheckKind::DunderFunctionName => { "Function name should not start and end with `__`".to_string() } @@ -1320,9 +1343,18 @@ impl CheckKind { CheckKind::CamelcaseImportedAsConstant(name, asname) => { format!("Camelcase `{name}` imported as constant `{asname}`") } + CheckKind::MixedCaseVariableInClassScope(name) => { + format!("Variable `{name}` in class scope should not be mixedCase") + } + CheckKind::MixedCaseVariableInGlobalScope(name) => { + format!("Variable `{name}` in global scope should not be mixedCase") + } CheckKind::CamelcaseImportedAsAcronym(name, asname) => { format!("Camelcase `{name}` imported as acronym `{asname}`") } + CheckKind::ErrorSuffixOnExceptionName(name) => { + format!("Exception name `{name}` should be named with an Error suffix") + } // Meta CheckKind::UnusedNOQA(codes) => match codes { None => "Unused `noqa` directive".to_string(), diff --git a/src/checks_gen.rs b/src/checks_gen.rs index 4bdf6b6aa9..4a5fe13458 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -184,13 +184,17 @@ pub enum CheckCodePrefix { N803, N804, N805, + N806, N807, N81, N811, N812, N813, N814, + N815, + N816, N817, + N818, Q, Q0, Q00, @@ -723,12 +727,16 @@ impl CheckCodePrefix { CheckCode::N803, CheckCode::N804, CheckCode::N805, + CheckCode::N806, CheckCode::N807, CheckCode::N811, CheckCode::N812, CheckCode::N813, CheckCode::N814, + CheckCode::N815, + CheckCode::N816, CheckCode::N817, + CheckCode::N818, ], CheckCodePrefix::N8 => vec![ CheckCode::N801, @@ -736,12 +744,16 @@ impl CheckCodePrefix { CheckCode::N803, CheckCode::N804, CheckCode::N805, + CheckCode::N806, CheckCode::N807, CheckCode::N811, CheckCode::N812, CheckCode::N813, CheckCode::N814, + CheckCode::N815, + CheckCode::N816, CheckCode::N817, + CheckCode::N818, ], CheckCodePrefix::N80 => vec![ CheckCode::N801, @@ -749,6 +761,7 @@ impl CheckCodePrefix { CheckCode::N803, CheckCode::N804, CheckCode::N805, + CheckCode::N806, CheckCode::N807, ], CheckCodePrefix::N801 => vec![CheckCode::N801], @@ -756,19 +769,26 @@ impl CheckCodePrefix { CheckCodePrefix::N803 => vec![CheckCode::N803], CheckCodePrefix::N804 => vec![CheckCode::N804], CheckCodePrefix::N805 => vec![CheckCode::N805], + CheckCodePrefix::N806 => vec![CheckCode::N806], CheckCodePrefix::N807 => vec![CheckCode::N807], CheckCodePrefix::N81 => vec![ CheckCode::N811, CheckCode::N812, CheckCode::N813, - CheckCode::N814, + CheckCode::N813, + CheckCode::N815, + CheckCode::N816, CheckCode::N817, + CheckCode::N818, ], CheckCodePrefix::N811 => vec![CheckCode::N811], CheckCodePrefix::N812 => vec![CheckCode::N812], CheckCodePrefix::N813 => vec![CheckCode::N813], CheckCodePrefix::N814 => vec![CheckCode::N814], + CheckCodePrefix::N815 => vec![CheckCode::N815], + CheckCodePrefix::N816 => vec![CheckCode::N816], CheckCodePrefix::N817 => vec![CheckCode::N817], + CheckCodePrefix::N818 => vec![CheckCode::N817], CheckCodePrefix::Q => vec![ CheckCode::Q000, CheckCode::Q001, @@ -1025,13 +1045,17 @@ impl CheckCodePrefix { CheckCodePrefix::N803 => PrefixSpecificity::Explicit, CheckCodePrefix::N804 => PrefixSpecificity::Explicit, CheckCodePrefix::N805 => PrefixSpecificity::Explicit, + CheckCodePrefix::N806 => PrefixSpecificity::Explicit, CheckCodePrefix::N807 => PrefixSpecificity::Explicit, CheckCodePrefix::N81 => PrefixSpecificity::Tens, CheckCodePrefix::N811 => PrefixSpecificity::Explicit, CheckCodePrefix::N812 => PrefixSpecificity::Explicit, CheckCodePrefix::N813 => PrefixSpecificity::Explicit, CheckCodePrefix::N814 => PrefixSpecificity::Explicit, + CheckCodePrefix::N815 => PrefixSpecificity::Explicit, + CheckCodePrefix::N816 => PrefixSpecificity::Explicit, CheckCodePrefix::N817 => PrefixSpecificity::Explicit, + CheckCodePrefix::N818 => PrefixSpecificity::Explicit, CheckCodePrefix::Q => PrefixSpecificity::Category, CheckCodePrefix::Q0 => PrefixSpecificity::Hundreds, CheckCodePrefix::Q00 => PrefixSpecificity::Tens, diff --git a/src/linter.rs b/src/linter.rs index 59510966bf..f2008a407f 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -368,12 +368,16 @@ mod tests { #[test_case(CheckCode::N803, Path::new("N803.py"); "N803")] #[test_case(CheckCode::N804, Path::new("N804.py"); "N804")] #[test_case(CheckCode::N805, Path::new("N805.py"); "N805")] + #[test_case(CheckCode::N806, Path::new("N806.py"); "N806")] #[test_case(CheckCode::N807, Path::new("N807.py"); "N807")] #[test_case(CheckCode::N811, Path::new("N811.py"); "N811")] #[test_case(CheckCode::N812, Path::new("N812.py"); "N812")] #[test_case(CheckCode::N813, Path::new("N813.py"); "N813")] #[test_case(CheckCode::N814, Path::new("N814.py"); "N814")] + #[test_case(CheckCode::N815, Path::new("N815.py"); "N815")] + #[test_case(CheckCode::N816, Path::new("N816.py"); "N816")] #[test_case(CheckCode::N817, Path::new("N817.py"); "N817")] + #[test_case(CheckCode::N818, Path::new("N818.py"); "N818")] #[test_case(CheckCode::T201, Path::new("T201.py"); "T201")] #[test_case(CheckCode::T203, Path::new("T203.py"); "T203")] #[test_case(CheckCode::U001, Path::new("U001.py"); "U001")] diff --git a/src/pep8_naming/checks.rs b/src/pep8_naming/checks.rs index 87a6d00677..ffdde7e76b 100644 --- a/src/pep8_naming/checks.rs +++ b/src/pep8_naming/checks.rs @@ -1,7 +1,7 @@ use itertools::Itertools; use rustpython_ast::{Arguments, Expr, ExprKind, Stmt}; -use crate::ast::types::{Range, Scope, ScopeKind}; +use crate::ast::types::{FunctionScope, Range, Scope, ScopeKind}; use crate::checks::{Check, CheckKind}; pub fn invalid_class_name(class_def: &Stmt, name: &str) -> Option { @@ -99,6 +99,49 @@ pub fn invalid_first_argument_name_for_method( None } +pub fn non_lowercase_variable_in_function(scope: &Scope, expr: &Expr, name: &str) -> Option { + if !matches!(scope.kind, ScopeKind::Function(FunctionScope { .. })) { + return None; + } + if !is_lower(name) { + return Some(Check::new( + CheckKind::NonLowercaseVariableInFunction(name.to_string()), + Range::from_located(expr), + )); + } + None +} + +pub fn mixed_case_variable_in_class_scope(scope: &Scope, expr: &Expr, name: &str) -> Option { + if !matches!(scope.kind, ScopeKind::Class) { + return None; + } + if is_mixed_case(name) { + return Some(Check::new( + CheckKind::MixedCaseVariableInClassScope(name.to_string()), + Range::from_located(expr), + )); + } + None +} + +pub fn mixed_case_variable_in_global_scope( + scope: &Scope, + expr: &Expr, + name: &str, +) -> Option { + if !matches!(scope.kind, ScopeKind::Module) { + return None; + } + if is_mixed_case(name) { + return Some(Check::new( + CheckKind::MixedCaseVariableInGlobalScope(name.to_string()), + Range::from_located(expr), + )); + } + None +} + pub fn dunder_function_name(func_def: &Stmt, scope: &Scope, name: &str) -> Option { if matches!(scope.kind, ScopeKind::Class) { return None; @@ -169,6 +212,17 @@ pub fn lowercase_imported_as_non_lowercase( fn is_camelcase(name: &str) -> bool { !is_lower(name) && !is_upper(name) && !name.contains('_') } + +fn is_mixed_case(name: &str) -> bool { + !is_lower(name) + && name + .strip_prefix('_') + .unwrap_or(name) + .chars() + .next() + .map_or_else(|| false, |c| c.is_lowercase()) +} + fn is_acronym(name: &str, asname: &str) -> bool { name.chars().filter(|c| c.is_uppercase()).join("") == asname } @@ -215,9 +269,31 @@ pub fn camelcase_imported_as_acronym( None } +pub fn error_suffix_on_exception_name( + class_def: &Stmt, + bases: &[Expr], + name: &str, +) -> Option { + if bases.iter().any(|base| { + if let ExprKind::Name { id, .. } = &base.node { + id == "Exception" + } else { + false + } + }) { + if !name.ends_with("Error") { + return Some(Check::new( + CheckKind::ErrorSuffixOnExceptionName(name.to_string()), + Range::from_located(class_def), + )); + } + } + None +} + #[cfg(test)] mod tests { - use super::{is_acronym, is_camelcase, is_lower, is_upper}; + use super::{is_acronym, is_camelcase, is_lower, is_mixed_case, is_upper}; #[test] fn test_is_lower() -> () { @@ -251,6 +327,17 @@ mod tests { assert!(!is_camelcase("CAMEL_CASE")); } + #[test] + fn test_is_mixed_case() -> () { + assert!(is_mixed_case("mixedCase")); + assert!(is_mixed_case("mixed_Case")); + assert!(is_mixed_case("_mixed_Case")); + assert!(!is_mixed_case("mixed_case")); + assert!(!is_mixed_case("MIXED_CASE")); + assert!(!is_mixed_case("")); + assert!(!is_mixed_case("_")); + } + #[test] fn test_is_acronym() -> () { assert!(is_acronym("AB", "AB")); diff --git a/src/snapshots/ruff__linter__tests__N806_N806.py.snap b/src/snapshots/ruff__linter__tests__N806_N806.py.snap new file mode 100644 index 0000000000..d85b26ef45 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__N806_N806.py.snap @@ -0,0 +1,23 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + NonLowercaseVariableInFunction: Camel + location: + row: 3 + column: 5 + end_location: + row: 3 + column: 10 + fix: ~ +- kind: + NonLowercaseVariableInFunction: CONSTANT + location: + row: 4 + column: 5 + end_location: + row: 4 + column: 13 + fix: ~ + diff --git a/src/snapshots/ruff__linter__tests__N815_N815.py.snap b/src/snapshots/ruff__linter__tests__N815_N815.py.snap new file mode 100644 index 0000000000..5086be3551 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__N815_N815.py.snap @@ -0,0 +1,32 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + MixedCaseVariableInClassScope: mixedCase + location: + row: 4 + column: 5 + end_location: + row: 4 + column: 14 + fix: ~ +- kind: + MixedCaseVariableInClassScope: _mixedCase + location: + row: 5 + column: 5 + end_location: + row: 5 + column: 15 + fix: ~ +- kind: + MixedCaseVariableInClassScope: mixed_Case + location: + row: 6 + column: 5 + end_location: + row: 6 + column: 15 + fix: ~ + diff --git a/src/snapshots/ruff__linter__tests__N816_N816.py.snap b/src/snapshots/ruff__linter__tests__N816_N816.py.snap new file mode 100644 index 0000000000..c952b3a1ba --- /dev/null +++ b/src/snapshots/ruff__linter__tests__N816_N816.py.snap @@ -0,0 +1,32 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + MixedCaseVariableInGlobalScope: mixedCase + location: + row: 3 + column: 1 + end_location: + row: 3 + column: 10 + fix: ~ +- kind: + MixedCaseVariableInGlobalScope: _mixedCase + location: + row: 4 + column: 1 + end_location: + row: 4 + column: 11 + fix: ~ +- kind: + MixedCaseVariableInGlobalScope: mixed_Case + location: + row: 5 + column: 1 + end_location: + row: 5 + column: 11 + fix: ~ + diff --git a/src/snapshots/ruff__linter__tests__N818_N818.py.snap b/src/snapshots/ruff__linter__tests__N818_N818.py.snap new file mode 100644 index 0000000000..fb9c57a4dd --- /dev/null +++ b/src/snapshots/ruff__linter__tests__N818_N818.py.snap @@ -0,0 +1,14 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + ErrorSuffixOnExceptionName: C + location: + row: 9 + column: 1 + end_location: + row: 11 + column: 1 + fix: ~ +