Skip to content

Commit 82cb867

Browse files
[pep8-naming] Ignore @override methods (N803) (#15954)
## Summary Resolves #15925. `N803` now checks for functions instead of parameters. In preview mode, if a method is decorated with `@override` and the current scope is that of a class, it will be ignored. ## Test Plan `cargo nextest run` and `cargo insta test`.
1 parent 5852217 commit 82cb867

File tree

8 files changed

+161
-27
lines changed

8 files changed

+161
-27
lines changed

crates/ruff_linter/resources/test/fixtures/pep8_naming/N803.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,21 @@ def method(self, _, a, A):
99

1010
def func(_, setUp):
1111
return _, setUp
12+
13+
14+
from typing import override
15+
16+
class Extended(Class):
17+
@override
18+
def method(self, _, a, A): ...
19+
20+
21+
@override # Incorrect usage
22+
def func(_, a, A): ...
23+
24+
25+
func = lambda _, a, A: ...
26+
27+
28+
class Extended(Class):
29+
method = override(lambda self, _, a, A: ...) # Incorrect usage

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1746,9 +1746,12 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
17461746
ruff::rules::parenthesize_chained_logical_operators(checker, bool_op);
17471747
}
17481748
}
1749-
Expr::Lambda(lambda_expr) => {
1749+
Expr::Lambda(lambda) => {
17501750
if checker.enabled(Rule::ReimplementedOperator) {
1751-
refurb::rules::reimplemented_operator(checker, &lambda_expr.into());
1751+
refurb::rules::reimplemented_operator(checker, &lambda.into());
1752+
}
1753+
if checker.enabled(Rule::InvalidArgumentName) {
1754+
pep8_naming::rules::invalid_argument_name_lambda(checker, lambda);
17521755
}
17531756
}
17541757
_ => {}

crates/ruff_linter/src/checkers/ast/analyze/parameter.rs

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use ruff_text_size::Ranged;
33

44
use crate::checkers::ast::Checker;
55
use crate::codes::Rule;
6-
use crate::rules::{flake8_builtins, pep8_naming, pycodestyle};
6+
use crate::rules::{flake8_builtins, pycodestyle};
77

88
/// Run lint rules over a [`Parameter`] syntax node.
99
pub(crate) fn parameter(parameter: &Parameter, checker: &mut Checker) {
@@ -14,15 +14,6 @@ pub(crate) fn parameter(parameter: &Parameter, checker: &mut Checker) {
1414
parameter.name.range(),
1515
);
1616
}
17-
if checker.enabled(Rule::InvalidArgumentName) {
18-
if let Some(diagnostic) = pep8_naming::rules::invalid_argument_name(
19-
&parameter.name,
20-
parameter,
21-
&checker.settings.pep8_naming.ignore_names,
22-
) {
23-
checker.diagnostics.push(diagnostic);
24-
}
25-
}
2617
if checker.enabled(Rule::BuiltinArgumentShadowing) {
2718
flake8_builtins::rules::builtin_argument_shadowing(checker, parameter);
2819
}

crates/ruff_linter/src/checkers/ast/analyze/statement.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
379379
if checker.enabled(Rule::NonPEP695GenericFunction) {
380380
pyupgrade::rules::non_pep695_generic_function(checker, function_def);
381381
}
382+
if checker.enabled(Rule::InvalidArgumentName) {
383+
pep8_naming::rules::invalid_argument_name_function(checker, function_def);
384+
}
382385
}
383386
Stmt::Return(_) => {
384387
if checker.enabled(Rule::ReturnOutsideFunction) {

crates/ruff_linter/src/rules/pep8_naming/mod.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ mod tests {
1414
use crate::registry::Rule;
1515
use crate::rules::pep8_naming::settings::IgnoreNames;
1616
use crate::rules::{flake8_import_conventions, pep8_naming};
17+
use crate::settings::types::PreviewMode;
1718
use crate::test::test_path;
1819
use crate::{assert_messages, settings};
1920

@@ -88,6 +89,24 @@ mod tests {
8889
Ok(())
8990
}
9091

92+
#[test_case(Rule::InvalidArgumentName, Path::new("N803.py"))]
93+
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
94+
let snapshot = format!(
95+
"preview__{}_{}",
96+
rule_code.noqa_code(),
97+
path.to_string_lossy()
98+
);
99+
let diagnostics = test_path(
100+
Path::new("pep8_naming").join(path).as_path(),
101+
&settings::LinterSettings {
102+
preview: PreviewMode::Enabled,
103+
..settings::LinterSettings::for_rule(rule_code)
104+
},
105+
)?;
106+
assert_messages!(snapshot, diagnostics);
107+
Ok(())
108+
}
109+
91110
#[test]
92111
fn camelcase_imported_as_incorrect_convention() -> Result<()> {
93112
let diagnostics = test_path(

crates/ruff_linter/src/rules/pep8_naming/rules/invalid_argument_name.rs

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
use ruff_python_ast::Parameter;
2-
31
use ruff_diagnostics::{Diagnostic, Violation};
42
use ruff_macros::{derive_message_formats, ViolationMetadata};
3+
use ruff_python_ast::{ExprLambda, Parameters, StmtFunctionDef};
4+
use ruff_python_semantic::analyze::visibility::is_override;
5+
use ruff_python_semantic::ScopeKind;
56
use ruff_python_stdlib::str;
67
use ruff_text_size::Ranged;
78

8-
use crate::rules::pep8_naming::settings::IgnoreNames;
9+
use crate::checkers::ast::Checker;
910

1011
/// ## What it does
1112
/// Checks for argument names that do not follow the `snake_case` convention.
@@ -22,6 +23,8 @@ use crate::rules::pep8_naming::settings::IgnoreNames;
2223
/// > mixedCase is allowed only in contexts where that’s already the
2324
/// > prevailing style (e.g. threading.py), to retain backwards compatibility.
2425
///
26+
/// In [preview], overridden methods are ignored.
27+
///
2528
/// ## Example
2629
/// ```python
2730
/// def my_function(A, myArg):
@@ -35,6 +38,7 @@ use crate::rules::pep8_naming::settings::IgnoreNames;
3538
/// ```
3639
///
3740
/// [PEP 8]: https://peps.python.org/pep-0008/#function-and-method-arguments
41+
/// [preview]: https://docs.astral.sh/ruff/preview/
3842
#[derive(ViolationMetadata)]
3943
pub(crate) struct InvalidArgumentName {
4044
name: String,
@@ -49,22 +53,54 @@ impl Violation for InvalidArgumentName {
4953
}
5054

5155
/// N803
52-
pub(crate) fn invalid_argument_name(
53-
name: &str,
54-
parameter: &Parameter,
55-
ignore_names: &IgnoreNames,
56-
) -> Option<Diagnostic> {
57-
if !str::is_lowercase(name) {
58-
// Ignore any explicitly-allowed names.
56+
pub(crate) fn invalid_argument_name_function(
57+
checker: &mut Checker,
58+
function_def: &StmtFunctionDef,
59+
) {
60+
let semantic = checker.semantic();
61+
let scope = semantic.current_scope();
62+
63+
if checker.settings.preview.is_enabled()
64+
&& matches!(scope.kind, ScopeKind::Class(_))
65+
&& is_override(&function_def.decorator_list, semantic)
66+
{
67+
return;
68+
}
69+
70+
invalid_argument_name(checker, &function_def.parameters);
71+
}
72+
73+
/// N803
74+
pub(crate) fn invalid_argument_name_lambda(checker: &mut Checker, lambda: &ExprLambda) {
75+
let Some(parameters) = &lambda.parameters else {
76+
return;
77+
};
78+
79+
invalid_argument_name(checker, parameters);
80+
}
81+
82+
/// N803
83+
fn invalid_argument_name(checker: &mut Checker, parameters: &Parameters) {
84+
let ignore_names = &checker.settings.pep8_naming.ignore_names;
85+
86+
for parameter in parameters {
87+
let name = parameter.name().as_str();
88+
89+
if str::is_lowercase(name) {
90+
continue;
91+
}
92+
5993
if ignore_names.matches(name) {
60-
return None;
94+
continue;
6195
}
62-
return Some(Diagnostic::new(
96+
97+
let diagnostic = Diagnostic::new(
6398
InvalidArgumentName {
6499
name: name.to_string(),
65100
},
66101
parameter.range(),
67-
));
102+
);
103+
104+
checker.diagnostics.push(diagnostic);
68105
}
69-
None
70106
}

crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N803_N803.py.snap

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
---
22
source: crates/ruff_linter/src/rules/pep8_naming/mod.rs
3-
snapshot_kind: text
43
---
54
N803.py:1:16: N803 Argument name `A` should be lowercase
65
|
@@ -16,3 +15,31 @@ N803.py:6:28: N803 Argument name `A` should be lowercase
1615
| ^ N803
1716
7 | return _, a, A
1817
|
18+
19+
N803.py:18:28: N803 Argument name `A` should be lowercase
20+
|
21+
16 | class Extended(Class):
22+
17 | @override
23+
18 | def method(self, _, a, A): ...
24+
| ^ N803
25+
|
26+
27+
N803.py:22:16: N803 Argument name `A` should be lowercase
28+
|
29+
21 | @override # Incorrect usage
30+
22 | def func(_, a, A): ...
31+
| ^ N803
32+
|
33+
34+
N803.py:25:21: N803 Argument name `A` should be lowercase
35+
|
36+
25 | func = lambda _, a, A: ...
37+
| ^ N803
38+
|
39+
40+
N803.py:29:42: N803 Argument name `A` should be lowercase
41+
|
42+
28 | class Extended(Class):
43+
29 | method = override(lambda self, _, a, A: ...) # Incorrect usage
44+
| ^ N803
45+
|
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
source: crates/ruff_linter/src/rules/pep8_naming/mod.rs
3+
---
4+
N803.py:1:16: N803 Argument name `A` should be lowercase
5+
|
6+
1 | def func(_, a, A):
7+
| ^ N803
8+
2 | return _, a, A
9+
|
10+
11+
N803.py:6:28: N803 Argument name `A` should be lowercase
12+
|
13+
5 | class Class:
14+
6 | def method(self, _, a, A):
15+
| ^ N803
16+
7 | return _, a, A
17+
|
18+
19+
N803.py:22:16: N803 Argument name `A` should be lowercase
20+
|
21+
21 | @override # Incorrect usage
22+
22 | def func(_, a, A): ...
23+
| ^ N803
24+
|
25+
26+
N803.py:25:21: N803 Argument name `A` should be lowercase
27+
|
28+
25 | func = lambda _, a, A: ...
29+
| ^ N803
30+
|
31+
32+
N803.py:29:42: N803 Argument name `A` should be lowercase
33+
|
34+
28 | class Extended(Class):
35+
29 | method = override(lambda self, _, a, A: ...) # Incorrect usage
36+
| ^ N803
37+
|

0 commit comments

Comments
 (0)