From c55fd76743562c96b0534e75555fc576b15cb175 Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Mon, 17 Oct 2022 00:58:39 +0900 Subject: [PATCH] Implement N801 ~ N805 (#439) --- README.md | 10 ++ resources/test/fixtures/N801.py | 34 +++++++ resources/test/fixtures/N802.py | 26 +++++ resources/test/fixtures/N803.py | 7 ++ resources/test/fixtures/N804.py | 19 ++++ resources/test/fixtures/N805.py | 26 +++++ src/ast/checkers.rs | 96 +++++++++++++++++++ src/check_ast.rs | 40 ++++++++ src/checks.rs | 47 +++++++++ src/linter.rs | 5 + .../ruff__linter__tests__N801_N801.py.snap | 50 ++++++++++ .../ruff__linter__tests__N802_N802.py.snap | 41 ++++++++ .../ruff__linter__tests__N803_N803.py.snap | 23 +++++ .../ruff__linter__tests__N804_N804.py.snap | 13 +++ .../ruff__linter__tests__N805_N805.py.snap | 21 ++++ 15 files changed, 458 insertions(+) create mode 100644 resources/test/fixtures/N801.py create mode 100644 resources/test/fixtures/N802.py create mode 100644 resources/test/fixtures/N803.py create mode 100644 resources/test/fixtures/N804.py create mode 100644 resources/test/fixtures/N805.py create mode 100644 src/snapshots/ruff__linter__tests__N801_N801.py.snap create mode 100644 src/snapshots/ruff__linter__tests__N802_N802.py.snap create mode 100644 src/snapshots/ruff__linter__tests__N803_N803.py.snap create mode 100644 src/snapshots/ruff__linter__tests__N804_N804.py.snap create mode 100644 src/snapshots/ruff__linter__tests__N805_N805.py.snap diff --git a/README.md b/README.md index a3296a063f..d25981cd44 100644 --- a/README.md +++ b/README.md @@ -337,6 +337,16 @@ Flake8. | U007 | UsePEP604Annotation | Use `X \| Y` for type annotations | | U008 | SuperCallWithParameters | Use `super()` instead of `super(__class__, self)` | +### pep8-naming + +| Code | Name | Message | +| ---- | ---- | ------- | +| N801 | InvalidClassName | class name '...' should use CapWords convention | +| N802 | InvalidFunctionName | function name '...' should be lowercase | +| N803 | InvalidArgumentName | argument name '...' should be lowercase | +| N804 | InvalidFirstArgumentNameForClassMethod | first argument of a classmethod should be named 'cls' | +| N805 | InvalidFirstArgumentNameForMethod | first argument of a method should be named 'self' | + ### flake8-comprehensions | Code | Name | Message | diff --git a/resources/test/fixtures/N801.py b/resources/test/fixtures/N801.py new file mode 100644 index 0000000000..58175cd3fe --- /dev/null +++ b/resources/test/fixtures/N801.py @@ -0,0 +1,34 @@ +class bad: + pass + + +class _bad: + pass + + +class bad_class: + pass + + +class Bad_Class: + pass + + +class BAD_CLASS: + pass + + +class Good: + pass + + +class _Good: + pass + + +class GoodClass: + pass + + +class GOOD: + pass diff --git a/resources/test/fixtures/N802.py b/resources/test/fixtures/N802.py new file mode 100644 index 0000000000..7cc1c90191 --- /dev/null +++ b/resources/test/fixtures/N802.py @@ -0,0 +1,26 @@ +def Bad(): + pass + + +def _Bad(): + pass + + +def BAD(): + pass + + +def BAD_FUNC(): + pass + + +def good(): + pass + + +def _good(): + pass + + +def good_func(): + pass diff --git a/resources/test/fixtures/N803.py b/resources/test/fixtures/N803.py new file mode 100644 index 0000000000..6f030e09d1 --- /dev/null +++ b/resources/test/fixtures/N803.py @@ -0,0 +1,7 @@ +def func(a, A): + return a, A + + +class Class: + def method(self, a, A): + return a, A diff --git a/resources/test/fixtures/N804.py b/resources/test/fixtures/N804.py new file mode 100644 index 0000000000..5edec023a1 --- /dev/null +++ b/resources/test/fixtures/N804.py @@ -0,0 +1,19 @@ +class Class: + @classmethod + def bad_class_method(this): + pass + + @classmethod + def good_class_method(cls): + pass + + def method(self): + pass + + @staticmethod + def static_method(x): + return x + + +def func(x): + return x diff --git a/resources/test/fixtures/N805.py b/resources/test/fixtures/N805.py new file mode 100644 index 0000000000..9d5f01bd2f --- /dev/null +++ b/resources/test/fixtures/N805.py @@ -0,0 +1,26 @@ +import random + + +class Class: + def bad_method(this): + pass + + if random.random(0, 2) == 0: + + def extra_bad_method(this): + pass + + def good_method(self): + pass + + @classmethod + def class_method(cls): + pass + + @staticmethod + def static_method(x): + return x + + +def func(x): + return x diff --git a/src/ast/checkers.rs b/src/ast/checkers.rs index ec3d72314a..02e231e38b 100644 --- a/src/ast/checkers.rs +++ b/src/ast/checkers.rs @@ -1331,3 +1331,99 @@ pub fn print_call( None } + +// pep8-naming +pub fn invalid_class_name(class_def: &Stmt, name: &str) -> Option { + let stripped = name.strip_prefix('_').unwrap_or(name); + if !stripped + .chars() + .next() + .map(|c| c.is_uppercase()) + .unwrap_or(false) + || stripped.contains('_') + { + return Some(Check::new( + CheckKind::InvalidClassName(name.to_string()), + Range::from_located(class_def), + )); + } + None +} + +pub fn invalid_function_name(func_def: &Stmt, name: &str) -> Option { + if name.chars().any(|c| c.is_uppercase()) { + return Some(Check::new( + CheckKind::InvalidFunctionName(name.to_string()), + Range::from_located(func_def), + )); + } + None +} + +pub fn invalid_argument_name(location: Range, name: &str) -> Option { + if name.chars().any(|c| c.is_uppercase()) { + return Some(Check::new( + CheckKind::InvalidArgumentName(name.to_string()), + location, + )); + } + None +} + +pub fn invalid_first_argument_name_for_class_method( + scope: &Scope, + decorator_list: &[Expr], + args: &Arguments, +) -> Option { + if !matches!(scope.kind, ScopeKind::Class) { + return None; + } + + if decorator_list.iter().any(|decorator| { + if let ExprKind::Name { id, .. } = &decorator.node { + id == "classmethod" + } else { + false + } + }) { + if let Some(arg) = args.args.first() { + if arg.node.arg != "cls" { + return Some(Check::new( + CheckKind::InvalidFirstArgumentNameForClassMethod, + Range::from_located(arg), + )); + } + } + } + None +} + +pub fn invalid_first_argument_name_for_method( + scope: &Scope, + decorator_list: &[Expr], + args: &Arguments, +) -> Option { + if !matches!(scope.kind, ScopeKind::Class) { + return None; + } + + if decorator_list.iter().any(|decorator| { + if let ExprKind::Name { id, .. } = &decorator.node { + id == "classmethod" || id == "staticmethod" + } else { + false + } + }) { + return None; + } + + if let Some(arg) = args.args.first() { + if arg.node.arg != "self" { + return Some(Check::new( + CheckKind::InvalidFirstArgumentNameForMethod, + Range::from_located(arg), + )); + } + } + None +} diff --git a/src/check_ast.rs b/src/check_ast.rs index ae6b762781..ebf19f398c 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -216,6 +216,32 @@ where } } + if self.settings.enabled.contains(&CheckCode::N802) { + if let Some(check) = checkers::invalid_function_name(stmt, name) { + self.checks.push(check); + } + } + + if self.settings.enabled.contains(&CheckCode::N804) { + if let Some(check) = checkers::invalid_first_argument_name_for_class_method( + self.current_scope(), + decorator_list, + args, + ) { + self.checks.push(check); + } + } + + if self.settings.enabled.contains(&CheckCode::N805) { + if let Some(check) = checkers::invalid_first_argument_name_for_method( + self.current_scope(), + decorator_list, + args, + ) { + self.checks.push(check); + } + } + self.check_builtin_shadowing(name, Range::from_located(stmt), true); // Visit the decorators and arguments, but avoid the body, which will be deferred. @@ -304,6 +330,12 @@ where } } + if self.settings.enabled.contains(&CheckCode::N801) { + if let Some(check) = checkers::invalid_class_name(stmt, name) { + self.checks.push(check); + } + } + self.check_builtin_shadowing( name, self.locate_check(Range::from_located(stmt)), @@ -1336,6 +1368,14 @@ where } } + if self.settings.enabled.contains(&CheckCode::N803) { + if let Some(check) = + checkers::invalid_argument_name(Range::from_located(arg), &arg.node.arg) + { + self.checks.push(check); + } + } + self.check_builtin_arg_shadowing(&arg.node.arg, Range::from_located(arg)); } } diff --git a/src/checks.rs b/src/checks.rs index 8660186d6b..e38bbdf842 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -200,6 +200,12 @@ pub enum CheckCode { D417, D418, D419, + // pep8-naming + N801, + N802, + N803, + N804, + N805, // Meta M001, } @@ -210,6 +216,7 @@ pub enum CheckCategory { Pycodestyle, Pydocstyle, Pyupgrade, + Pep8Naming, Flake8Comprehensions, Flake8Bugbear, Flake8Builtins, @@ -228,6 +235,7 @@ impl CheckCategory { CheckCategory::Flake8Print => "flake8-print", CheckCategory::Pyupgrade => "pyupgrade", CheckCategory::Pydocstyle => "pydocstyle", + CheckCategory::Pep8Naming => "pep8-naming", CheckCategory::Meta => "Meta rules", } } @@ -375,6 +383,12 @@ pub enum CheckKind { SectionUnderlineNotOverIndented(String), SkipDocstring, UsesTripleQuotes, + // pep8-naming + InvalidClassName(String), + InvalidFunctionName(String), + InvalidArgumentName(String), + InvalidFirstArgumentNameForClassMethod, + InvalidFirstArgumentNameForMethod, // Meta UnusedNOQA(Option>), } @@ -544,6 +558,12 @@ impl CheckCode { } CheckCode::D418 => CheckKind::SkipDocstring, CheckCode::D419 => CheckKind::NonEmpty, + // pep8-naming + CheckCode::N801 => CheckKind::InvalidClassName("...".to_string()), + CheckCode::N802 => CheckKind::InvalidFunctionName("...".to_string()), + CheckCode::N803 => CheckKind::InvalidArgumentName("...".to_string()), + CheckCode::N804 => CheckKind::InvalidFirstArgumentNameForClassMethod, + CheckCode::N805 => CheckKind::InvalidFirstArgumentNameForMethod, // Meta CheckCode::M001 => CheckKind::UnusedNOQA(None), } @@ -670,6 +690,11 @@ impl CheckCode { CheckCode::D417 => CheckCategory::Pydocstyle, CheckCode::D418 => CheckCategory::Pydocstyle, CheckCode::D419 => CheckCategory::Pydocstyle, + CheckCode::N801 => CheckCategory::Pep8Naming, + CheckCode::N802 => CheckCategory::Pep8Naming, + CheckCode::N803 => CheckCategory::Pep8Naming, + CheckCode::N804 => CheckCategory::Pep8Naming, + CheckCode::N805 => CheckCategory::Pep8Naming, CheckCode::M001 => CheckCategory::Meta, } } @@ -806,6 +831,12 @@ impl CheckKind { CheckKind::SectionUnderlineNotOverIndented(_) => &CheckCode::D215, CheckKind::SkipDocstring => &CheckCode::D418, CheckKind::UsesTripleQuotes => &CheckCode::D300, + // pep8-naming + CheckKind::InvalidClassName(_) => &CheckCode::N801, + CheckKind::InvalidFunctionName(_) => &CheckCode::N802, + CheckKind::InvalidArgumentName(_) => &CheckCode::N803, + CheckKind::InvalidFirstArgumentNameForClassMethod => &CheckCode::N804, + CheckKind::InvalidFirstArgumentNameForMethod => &CheckCode::N805, // Meta CheckKind::UnusedNOQA(_) => &CheckCode::M001, } @@ -1180,6 +1211,22 @@ impl CheckKind { } CheckKind::NoUnderIndentation => "Docstring is under-indented".to_string(), CheckKind::NoOverIndentation => "Docstring is over-indented".to_string(), + // pep8-naming + CheckKind::InvalidClassName(name) => { + format!("class name '{name}' should use CapWords convention ") + } + CheckKind::InvalidFunctionName(name) => { + format!("function name '{name}' should be lowercase") + } + CheckKind::InvalidArgumentName(name) => { + format!("argument name '{name}' should be lowercase") + } + CheckKind::InvalidFirstArgumentNameForClassMethod => { + "first argument of a classmethod should be named 'cls'".to_string() + } + CheckKind::InvalidFirstArgumentNameForMethod => { + "first argument of a method should be named 'self'".to_string() + } // Meta CheckKind::UnusedNOQA(codes) => match codes { None => "Unused `noqa` directive".to_string(), diff --git a/src/linter.rs b/src/linter.rs index e5d9bbc335..0a5d2c3601 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -353,6 +353,11 @@ mod tests { #[test_case(CheckCode::F831, Path::new("F831.py"); "F831")] #[test_case(CheckCode::F841, Path::new("F841.py"); "F841")] #[test_case(CheckCode::F901, Path::new("F901.py"); "F901")] + #[test_case(CheckCode::N801, Path::new("N801.py"); "N801")] + #[test_case(CheckCode::N802, Path::new("N802.py"); "N802")] + #[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::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/snapshots/ruff__linter__tests__N801_N801.py.snap b/src/snapshots/ruff__linter__tests__N801_N801.py.snap new file mode 100644 index 0000000000..67cafd0fd0 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__N801_N801.py.snap @@ -0,0 +1,50 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + InvalidClassName: bad + location: + row: 1 + column: 1 + end_location: + row: 5 + column: 1 + fix: ~ +- kind: + InvalidClassName: _bad + location: + row: 5 + column: 1 + end_location: + row: 9 + column: 1 + fix: ~ +- kind: + InvalidClassName: bad_class + location: + row: 9 + column: 1 + end_location: + row: 13 + column: 1 + fix: ~ +- kind: + InvalidClassName: Bad_Class + location: + row: 13 + column: 1 + end_location: + row: 17 + column: 1 + fix: ~ +- kind: + InvalidClassName: BAD_CLASS + location: + row: 17 + column: 1 + end_location: + row: 21 + column: 1 + fix: ~ + diff --git a/src/snapshots/ruff__linter__tests__N802_N802.py.snap b/src/snapshots/ruff__linter__tests__N802_N802.py.snap new file mode 100644 index 0000000000..63da8ffbd5 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__N802_N802.py.snap @@ -0,0 +1,41 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + InvalidFunctionName: Bad + location: + row: 1 + column: 1 + end_location: + row: 5 + column: 1 + fix: ~ +- kind: + InvalidFunctionName: _Bad + location: + row: 5 + column: 1 + end_location: + row: 9 + column: 1 + fix: ~ +- kind: + InvalidFunctionName: BAD + location: + row: 9 + column: 1 + end_location: + row: 13 + column: 1 + fix: ~ +- kind: + InvalidFunctionName: BAD_FUNC + location: + row: 13 + column: 1 + end_location: + row: 17 + column: 1 + fix: ~ + diff --git a/src/snapshots/ruff__linter__tests__N803_N803.py.snap b/src/snapshots/ruff__linter__tests__N803_N803.py.snap new file mode 100644 index 0000000000..a43b1a5f0a --- /dev/null +++ b/src/snapshots/ruff__linter__tests__N803_N803.py.snap @@ -0,0 +1,23 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + InvalidArgumentName: A + location: + row: 1 + column: 13 + end_location: + row: 1 + column: 14 + fix: ~ +- kind: + InvalidArgumentName: A + location: + row: 6 + column: 25 + end_location: + row: 6 + column: 26 + fix: ~ + diff --git a/src/snapshots/ruff__linter__tests__N804_N804.py.snap b/src/snapshots/ruff__linter__tests__N804_N804.py.snap new file mode 100644 index 0000000000..8d97d4d819 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__N804_N804.py.snap @@ -0,0 +1,13 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: InvalidFirstArgumentNameForClassMethod + location: + row: 3 + column: 26 + end_location: + row: 3 + column: 30 + fix: ~ + diff --git a/src/snapshots/ruff__linter__tests__N805_N805.py.snap b/src/snapshots/ruff__linter__tests__N805_N805.py.snap new file mode 100644 index 0000000000..9f9246e136 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__N805_N805.py.snap @@ -0,0 +1,21 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: InvalidFirstArgumentNameForMethod + location: + row: 5 + column: 20 + end_location: + row: 5 + column: 24 + fix: ~ +- kind: InvalidFirstArgumentNameForMethod + location: + row: 10 + column: 30 + end_location: + row: 10 + column: 34 + fix: ~ +