Skip to content

Commit 728b9d6

Browse files
AcelogicAcelogic
authored
[pep8-naming] Check naming conventions in match pattern bindings (N806, N815, N816) (#23899)
## Summary Fixes #16502 N806, N815, and N816 had false negatives for variables bound in `match`/`case` patterns. The rules only checked `Expr::Name` nodes with `ExprContext::Store`, but match patterns bind variables through `Pattern` nodes (`MatchAs`, `MatchStar`, `MatchMapping` rest captures), which were not checked. ## Changes - Refactored rule functions to accept `TextRange` instead of `&Expr` (they only used `expr.range()`) - Added `analyze::pattern` module that extracts bound names from match patterns and runs the same naming checks - Wired the analysis into the existing `visit_pattern` visitor method Now all three rules flag violations in: - `case Name:` (capture pattern) - `case int(Name):` (class pattern keyword) - `case [*Name]:` (star pattern) - `case pattern as Name:` (as pattern) - `case {"key": 1, **Name}:` (mapping rest) ## Test plan - [x] Added test cases to N806.py, N815.py, and N816.py fixture files - [x] Updated snapshots showing new violations detected - [x] All 52 pep8_naming tests pass - [x] Clippy clean (`cargo clippy -p ruff_linter -- -D warnings`)
1 parent 88d1eec commit 728b9d6

13 files changed

Lines changed: 251 additions & 17 deletions

‎crates/ruff_linter/resources/test/fixtures/pep8_naming/N806.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,19 @@ def model_assign() -> None:
5858

5959
Address: Type = apps.get_model("zerver", variable) # OK
6060
ValidationError = import_string(variable) # N806
61+
62+
63+
def match_case_names(x):
64+
match x:
65+
case BadName: # N806
66+
...
67+
case int(GoodName): # N806 (capture in class pattern)
68+
...
69+
case [*Rest]: # N806 (star pattern)
70+
...
71+
case {"key": Val} as Alias: # N806 (mapping value + as pattern)
72+
...
73+
case ok_name: # OK
74+
...
75+
case _: # OK
76+
...

‎crates/ruff_linter/resources/test/fixtures/pep8_naming/N815.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,14 @@ class E(D):
2828
mixedCase: bool
2929
_mixedCase: list
3030
mixed_Case: set
31+
32+
33+
class F:
34+
tP = 0
35+
match tP:
36+
case int(fN): # N815
37+
...
38+
case fN2 as fN3: # N815 (both capture and alias)
39+
...
40+
case ok_name:
41+
...

‎crates/ruff_linter/resources/test/fixtures/pep8_naming/N816.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,12 @@
1111
myObj2 = namedtuple("MyObj2", ["a", "b"])
1212
Employee = NamedTuple('Employee', [('name', str), ('id', int)])
1313
Point2D = TypedDict('Point2D', {'in': int, 'x-y': int})
14+
15+
tP = 0
16+
match tP:
17+
case fN1 as fN2: # N816 (both capture and alias)
18+
...
19+
case {"key": 1, **fN3}: # N816 (mapping rest)
20+
...
21+
case ok_name:
22+
...

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -343,15 +343,20 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
343343
if checker.is_rule_enabled(Rule::NonLowercaseVariableInFunction) {
344344
if checker.semantic.current_scope().kind.is_function() {
345345
pep8_naming::rules::non_lowercase_variable_in_function(
346-
checker, expr, id,
346+
checker,
347+
expr.range(),
348+
id,
347349
);
348350
}
349351
}
350352
if checker.is_rule_enabled(Rule::MixedCaseVariableInClassScope) {
351353
if let ScopeKind::Class(class_def) = &checker.semantic.current_scope().kind
352354
{
353355
pep8_naming::rules::mixed_case_variable_in_class_scope(
354-
checker, expr, id, class_def,
356+
checker,
357+
expr.range(),
358+
id,
359+
class_def,
355360
);
356361
}
357362
}
@@ -361,7 +366,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
361366
if checker.is_rule_enabled(Rule::MixedCaseVariableInGlobalScope) {
362367
if matches!(checker.semantic.current_scope().kind, ScopeKind::Module) {
363368
pep8_naming::rules::mixed_case_variable_in_global_scope(
364-
checker, expr, id,
369+
checker,
370+
expr.range(),
371+
id,
365372
);
366373
}
367374
}

‎crates/ruff_linter/src/checkers/ast/analyze/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ pub(super) use expression::expression;
1111
pub(super) use module::module;
1212
pub(super) use parameter::parameter;
1313
pub(super) use parameters::parameters;
14+
pub(super) use pattern::pattern;
1415
pub(super) use statement::statement;
1516
pub(super) use string_like::string_like;
1617
pub(super) use suite::suite;
@@ -29,6 +30,7 @@ mod expression;
2930
mod module;
3031
mod parameter;
3132
mod parameters;
33+
mod pattern;
3234
mod statement;
3335
mod string_like;
3436
mod suite;

‎crates/ruff_linter/src/checkers/ast/analyze/pattern.rs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
use ruff_python_ast::{self as ast, Identifier, Pattern};
2+
use ruff_python_semantic::ScopeKind;
3+
use ruff_text_size::Ranged;
4+
5+
use crate::checkers::ast::Checker;
6+
use crate::codes::Rule;
7+
use crate::rules::pep8_naming;
8+
9+
/// Run lint rules over a [`Pattern`] syntax node.
10+
pub(crate) fn pattern(pattern: &Pattern, checker: &Checker) {
11+
let (Pattern::MatchAs(ast::PatternMatchAs {
12+
name: Some(name), ..
13+
})
14+
| Pattern::MatchStar(ast::PatternMatchStar {
15+
name: Some(name), ..
16+
})
17+
| Pattern::MatchMapping(ast::PatternMatchMapping {
18+
rest: Some(name), ..
19+
})) = pattern
20+
else {
21+
return;
22+
};
23+
24+
check_pattern_name(checker, name);
25+
}
26+
27+
fn check_pattern_name(checker: &Checker, name: &Identifier) {
28+
if checker.is_rule_enabled(Rule::NonLowercaseVariableInFunction)
29+
&& checker.semantic().current_scope().kind.is_function()
30+
{
31+
pep8_naming::rules::non_lowercase_variable_in_function(checker, name.range(), &name.id);
32+
}
33+
if checker.is_rule_enabled(Rule::MixedCaseVariableInClassScope) {
34+
if let ScopeKind::Class(class_def) = &checker.semantic().current_scope().kind {
35+
pep8_naming::rules::mixed_case_variable_in_class_scope(
36+
checker,
37+
name.range(),
38+
&name.id,
39+
class_def,
40+
);
41+
}
42+
}
43+
if checker.is_rule_enabled(Rule::MixedCaseVariableInGlobalScope)
44+
&& matches!(checker.semantic().current_scope().kind, ScopeKind::Module)
45+
{
46+
pep8_naming::rules::mixed_case_variable_in_global_scope(checker, name.range(), &name.id);
47+
}
48+
}

‎crates/ruff_linter/src/checkers/ast/mod.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2332,6 +2332,9 @@ impl<'a> Visitor<'a> for Checker<'a> {
23322332

23332333
// Step 2: Traversal
23342334
walk_pattern(self, pattern);
2335+
2336+
// Step 4: Analysis
2337+
analyze::pattern(pattern, self);
23352338
}
23362339

23372340
fn visit_body(&mut self, body: &'a [Stmt]) {

‎crates/ruff_linter/src/rules/pep8_naming/rules/mixed_case_variable_in_class_scope.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use ruff_macros::{ViolationMetadata, derive_message_formats};
2-
use ruff_python_ast::{self as ast, Expr};
3-
use ruff_text_size::Ranged;
2+
use ruff_python_ast::{self as ast};
3+
use ruff_text_size::TextRange;
44

55
use crate::Violation;
66
use crate::checkers::ast::Checker;
@@ -57,7 +57,7 @@ impl Violation for MixedCaseVariableInClassScope {
5757
/// N815
5858
pub(crate) fn mixed_case_variable_in_class_scope(
5959
checker: &Checker,
60-
expr: &Expr,
60+
range: TextRange,
6161
name: &str,
6262
class_def: &ast::StmtClassDef,
6363
) {
@@ -81,6 +81,6 @@ pub(crate) fn mixed_case_variable_in_class_scope(
8181
MixedCaseVariableInClassScope {
8282
name: name.to_string(),
8383
},
84-
expr.range(),
84+
range,
8585
);
8686
}

‎crates/ruff_linter/src/rules/pep8_naming/rules/mixed_case_variable_in_global_scope.rs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
use ruff_python_ast::Expr;
2-
31
use ruff_macros::{ViolationMetadata, derive_message_formats};
4-
use ruff_text_size::Ranged;
2+
use ruff_text_size::TextRange;
53

64
use crate::Violation;
75
use crate::checkers::ast::Checker;
@@ -66,7 +64,7 @@ impl Violation for MixedCaseVariableInGlobalScope {
6664
}
6765

6866
/// N816
69-
pub(crate) fn mixed_case_variable_in_global_scope(checker: &Checker, expr: &Expr, name: &str) {
67+
pub(crate) fn mixed_case_variable_in_global_scope(checker: &Checker, range: TextRange, name: &str) {
7068
if !helpers::is_mixed_case(name) {
7169
return;
7270
}
@@ -84,6 +82,6 @@ pub(crate) fn mixed_case_variable_in_global_scope(checker: &Checker, expr: &Expr
8482
MixedCaseVariableInGlobalScope {
8583
name: name.to_string(),
8684
},
87-
expr.range(),
85+
range,
8886
);
8987
}

‎crates/ruff_linter/src/rules/pep8_naming/rules/non_lowercase_variable_in_function.rs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
1-
use ruff_python_ast::Expr;
2-
31
use ruff_macros::{ViolationMetadata, derive_message_formats};
42
use ruff_python_stdlib::str;
5-
use ruff_text_size::Ranged;
3+
use ruff_text_size::TextRange;
64

75
use crate::Violation;
86
use crate::checkers::ast::Checker;
@@ -53,7 +51,7 @@ impl Violation for NonLowercaseVariableInFunction {
5351
}
5452

5553
/// N806
56-
pub(crate) fn non_lowercase_variable_in_function(checker: &Checker, expr: &Expr, name: &str) {
54+
pub(crate) fn non_lowercase_variable_in_function(checker: &Checker, range: TextRange, name: &str) {
5755
if str::is_lowercase(name) {
5856
return;
5957
}
@@ -86,6 +84,6 @@ pub(crate) fn non_lowercase_variable_in_function(checker: &Checker, expr: &Expr,
8684
NonLowercaseVariableInFunction {
8785
name: name.to_string(),
8886
},
89-
expr.range(),
87+
range,
9088
);
9189
}

0 commit comments

Comments
 (0)