Skip to content

Commit bc3f4fa

Browse files
tusharsadhwanitusharsadhwani
authored
[flake8-pyi] Implement PYI059 (generic-not-last-base-class) (#11233)
1 parent 28cc71f commit bc3f4fa

File tree

10 files changed

+458
-0
lines changed

10 files changed

+458
-0
lines changed

‎crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI059.py

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
from typing import Container, Generic, Iterable, List, Sized, Tuple, TypeVar
2+
import typing as t
3+
4+
T = TypeVar('T')
5+
K = TypeVar('K')
6+
V = TypeVar('V')
7+
8+
class LinkedList(Generic[T], Sized): # PYI059
9+
def __init__(self) -> None:
10+
self._items: List[T] = []
11+
12+
def push(self, item: T) -> None:
13+
self._items.append(item)
14+
15+
class MyMapping( # PYI059
16+
t.Generic[K, V],
17+
Iterable[Tuple[K, V]],
18+
Container[Tuple[K, V]],
19+
):
20+
...
21+
22+
23+
# Inheriting from just `Generic` is a TypeError, but it's probably fine
24+
# to flag this issue in this case as well, since after fixing the error
25+
# the Generic's position issue persists.
26+
class Foo(Generic, LinkedList): # PYI059
27+
pass
28+
29+
30+
class Foo( # comment about the bracket
31+
# Part 1 of multiline comment 1
32+
# Part 2 of multiline comment 1
33+
Generic[T] # comment about Generic[T] # PYI059
34+
# another comment?
35+
, # comment about the comma?
36+
# part 1 of multiline comment 2
37+
# part 2 of multiline comment 2
38+
int, # comment about int
39+
# yet another comment?
40+
): # and another one for good measure
41+
...
42+
43+
44+
# in case of multiple Generic[] inheritance, don't fix it.
45+
class C(Generic[T], Generic[K, V]): ... # PYI059
46+
47+
48+
# Negative cases
49+
class MyList(Sized, Generic[T]): # Generic already in last place
50+
def __init__(self) -> None:
51+
self._items: List[T] = []
52+
53+
class SomeGeneric(Generic[T]): # Only one generic
54+
pass

‎crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI059.pyi

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
from typing import Container, Generic, Iterable, Sized, Tuple, TypeVar
2+
import typing as t
3+
4+
T = TypeVar('T')
5+
K = TypeVar('K')
6+
V = TypeVar('V')
7+
8+
class LinkedList(Generic[T], Sized): # PYI059
9+
def __init__(self) -> None: ...
10+
def push(self, item: T) -> None: ...
11+
12+
class MyMapping( # PYI059
13+
t.Generic[K, V],
14+
Iterable[Tuple[K, V]],
15+
Container[Tuple[K, V]],
16+
):
17+
...
18+
19+
# Inheriting from just `Generic` is a TypeError, but it's probably fine
20+
# to flag this issue in this case as well, since after fixing the error
21+
# the Generic's position issue persists.
22+
class Foo(Generic, LinkedList): ... # PYI059
23+
24+
25+
class Foo( # comment about the bracket
26+
# Part 1 of multiline comment 1
27+
# Part 2 of multiline comment 1
28+
Generic[T] # comment about Generic[T] # PYI059
29+
# another comment?
30+
, # comment about the comma?
31+
# part 1 of multiline comment 2
32+
# part 2 of multiline comment 2
33+
int, # comment about int
34+
# yet another comment?
35+
): # and another one for good measure
36+
...
37+
38+
39+
# in case of multiple Generic[] inheritance, don't fix it.
40+
class C(Generic[T], Generic[K, V]): ... # PYI059
41+
42+
43+
# Negative cases
44+
class MyList(Sized, Generic[T]): # Generic already in last place
45+
def __init__(self) -> None: ...
46+
47+
class SomeGeneric(Generic[T]): # Only one generic
48+
...

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

+3
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
478478
if checker.enabled(Rule::EllipsisInNonEmptyClassBody) {
479479
flake8_pyi::rules::ellipsis_in_non_empty_class_body(checker, body);
480480
}
481+
if checker.enabled(Rule::GenericNotLastBaseClass) {
482+
flake8_pyi::rules::generic_not_last_base_class(checker, class_def);
483+
}
481484
if checker.enabled(Rule::PytestIncorrectMarkParenthesesStyle) {
482485
flake8_pytest_style::rules::marks(checker, decorator_list);
483486
}

‎crates/ruff_linter/src/codes.rs

+1
Original file line numberDiff line numberDiff line change
@@ -808,6 +808,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
808808
(Flake8Pyi, "055") => (RuleGroup::Stable, rules::flake8_pyi::rules::UnnecessaryTypeUnion),
809809
(Flake8Pyi, "056") => (RuleGroup::Stable, rules::flake8_pyi::rules::UnsupportedMethodCallOnAll),
810810
(Flake8Pyi, "058") => (RuleGroup::Stable, rules::flake8_pyi::rules::GeneratorReturnFromIterMethod),
811+
(Flake8Pyi, "059") => (RuleGroup::Preview, rules::flake8_pyi::rules::GenericNotLastBaseClass),
811812

812813
// flake8-pytest-style
813814
(Flake8PytestStyle, "001") => (RuleGroup::Stable, rules::flake8_pytest_style::rules::PytestFixtureIncorrectParenthesesStyle),

‎crates/ruff_linter/src/rules/flake8_pyi/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ mod tests {
4141
#[test_case(Rule::FutureAnnotationsInStub, Path::new("PYI044.pyi"))]
4242
#[test_case(Rule::GeneratorReturnFromIterMethod, Path::new("PYI058.py"))]
4343
#[test_case(Rule::GeneratorReturnFromIterMethod, Path::new("PYI058.pyi"))]
44+
#[test_case(Rule::GenericNotLastBaseClass, Path::new("PYI059.py"))]
45+
#[test_case(Rule::GenericNotLastBaseClass, Path::new("PYI059.pyi"))]
4446
#[test_case(Rule::IterMethodReturnIterable, Path::new("PYI045.py"))]
4547
#[test_case(Rule::IterMethodReturnIterable, Path::new("PYI045.pyi"))]
4648
#[test_case(Rule::NoReturnArgumentAnnotationInStub, Path::new("PYI050.py"))]

‎crates/ruff_linter/src/rules/flake8_pyi/rules/generic_not_last_base_class.rs

+122
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
use ruff_diagnostics::{Diagnostic, Fix, FixAvailability, Violation};
2+
use ruff_macros::{derive_message_formats, violation};
3+
use ruff_python_ast::{self as ast, helpers::map_subscript};
4+
use ruff_text_size::Ranged;
5+
6+
use crate::checkers::ast::Checker;
7+
use crate::fix::edits::{add_argument, remove_argument, Parentheses};
8+
9+
/// ## What it does
10+
/// Checks for classes inheriting from `typing.Generic[]` where `Generic[]` is
11+
/// not the last base class in the bases tuple.
12+
///
13+
/// ## Why is this bad?
14+
/// If `Generic[]` is not the final class in the bases tuple, unexpected
15+
/// behaviour can occur at runtime (See [this CPython issue][1] for an example).
16+
/// The rule is also applied to stub files, but, unlike at runtime,
17+
/// in stubs it is purely enforced for stylistic consistency.
18+
///
19+
/// For example:
20+
/// ```python
21+
/// class LinkedList(Generic[T], Sized):
22+
/// def push(self, item: T) -> None:
23+
/// self._items.append(item)
24+
///
25+
/// class MyMapping(
26+
/// Generic[K, V],
27+
/// Iterable[Tuple[K, V]],
28+
/// Container[Tuple[K, V]],
29+
/// ):
30+
/// ...
31+
/// ```
32+
///
33+
/// Use instead:
34+
/// ```python
35+
/// class LinkedList(Sized, Generic[T]):
36+
/// def push(self, item: T) -> None:
37+
/// self._items.append(item)
38+
///
39+
/// class MyMapping(
40+
/// Iterable[Tuple[K, V]],
41+
/// Container[Tuple[K, V]],
42+
/// Generic[K, V],
43+
/// ):
44+
/// ...
45+
/// ```
46+
/// ## References
47+
/// - [`typing.Generic` documentation](https://docs.python.org/3/library/typing.html#typing.Generic)
48+
///
49+
/// [1]: https://github.com/python/cpython/issues/106102
50+
#[violation]
51+
pub struct GenericNotLastBaseClass;
52+
53+
impl Violation for GenericNotLastBaseClass {
54+
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
55+
56+
#[derive_message_formats]
57+
fn message(&self) -> String {
58+
format!("`Generic[]` should always be the last base class")
59+
}
60+
61+
fn fix_title(&self) -> Option<String> {
62+
Some("Move `Generic[]` to the end".to_string())
63+
}
64+
}
65+
66+
/// PYI059
67+
pub(crate) fn generic_not_last_base_class(checker: &mut Checker, class_def: &ast::StmtClassDef) {
68+
let Some(bases) = class_def.arguments.as_deref() else {
69+
return;
70+
};
71+
72+
let semantic = checker.semantic();
73+
if !semantic.seen_typing() {
74+
return;
75+
}
76+
77+
let Some(last_base) = bases.args.last() else {
78+
return;
79+
};
80+
81+
let mut generic_base_iter = bases
82+
.args
83+
.iter()
84+
.filter(|base| semantic.match_typing_expr(map_subscript(base), "Generic"));
85+
86+
let Some(generic_base) = generic_base_iter.next() else {
87+
return;
88+
};
89+
90+
// If `Generic[]` exists, but is the last base, don't emit a diagnostic.
91+
if generic_base.range() == last_base.range() {
92+
return;
93+
}
94+
95+
let mut diagnostic = Diagnostic::new(GenericNotLastBaseClass, bases.range());
96+
97+
// No fix if multiple `Generic[]`s are seen in the class bases.
98+
if generic_base_iter.next().is_none() {
99+
diagnostic.try_set_fix(|| generate_fix(generic_base, bases, checker));
100+
}
101+
102+
checker.diagnostics.push(diagnostic);
103+
}
104+
105+
fn generate_fix(
106+
generic_base: &ast::Expr,
107+
arguments: &ast::Arguments,
108+
checker: &Checker,
109+
) -> anyhow::Result<Fix> {
110+
let locator = checker.locator();
111+
let source = locator.contents();
112+
113+
let deletion = remove_argument(generic_base, arguments, Parentheses::Preserve, source)?;
114+
let insertion = add_argument(
115+
locator.slice(generic_base),
116+
arguments,
117+
checker.indexer().comment_ranges(),
118+
source,
119+
);
120+
121+
Ok(Fix::safe_edits(deletion, [insertion]))
122+
}

‎crates/ruff_linter/src/rules/flake8_pyi/rules/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ pub(crate) use duplicate_union_member::*;
1010
pub(crate) use ellipsis_in_non_empty_class_body::*;
1111
pub(crate) use exit_annotations::*;
1212
pub(crate) use future_annotations_in_stub::*;
13+
pub(crate) use generic_not_last_base_class::*;
1314
pub(crate) use iter_method_return_iterable::*;
1415
pub(crate) use no_return_argument_annotation::*;
1516
pub(crate) use non_empty_stub_body::*;
@@ -48,6 +49,7 @@ mod duplicate_union_member;
4849
mod ellipsis_in_non_empty_class_body;
4950
mod exit_annotations;
5051
mod future_annotations_in_stub;
52+
mod generic_not_last_base_class;
5153
mod iter_method_return_iterable;
5254
mod no_return_argument_annotation;
5355
mod non_empty_stub_body;

‎crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI059_PYI059.py.snap

+113
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
---
2+
source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs
3+
---
4+
PYI059.py:8:17: PYI059 [*] `Generic[]` should always be the last base class
5+
|
6+
6 | V = TypeVar('V')
7+
7 |
8+
8 | class LinkedList(Generic[T], Sized): # PYI059
9+
| ^^^^^^^^^^^^^^^^^^^ PYI059
10+
9 | def __init__(self) -> None:
11+
10 | self._items: List[T] = []
12+
|
13+
= help: Move `Generic[]` to the end
14+
15+
ℹ Safe fix
16+
5 5 | K = TypeVar('K')
17+
6 6 | V = TypeVar('V')
18+
7 7 |
19+
8 |-class LinkedList(Generic[T], Sized): # PYI059
20+
8 |+class LinkedList(Sized, Generic[T]): # PYI059
21+
9 9 | def __init__(self) -> None:
22+
10 10 | self._items: List[T] = []
23+
11 11 |
24+
25+
PYI059.py:15:16: PYI059 [*] `Generic[]` should always be the last base class
26+
|
27+
13 | self._items.append(item)
28+
14 |
29+
15 | class MyMapping( # PYI059
30+
| ________________^
31+
16 | | t.Generic[K, V],
32+
17 | | Iterable[Tuple[K, V]],
33+
18 | | Container[Tuple[K, V]],
34+
19 | | ):
35+
| |_^ PYI059
36+
20 | ...
37+
|
38+
= help: Move `Generic[]` to the end
39+
40+
ℹ Safe fix
41+
13 13 | self._items.append(item)
42+
14 14 |
43+
15 15 | class MyMapping( # PYI059
44+
16 |- t.Generic[K, V],
45+
17 16 | Iterable[Tuple[K, V]],
46+
18 |- Container[Tuple[K, V]],
47+
17 |+ Container[Tuple[K, V]], t.Generic[K, V],
48+
19 18 | ):
49+
20 19 | ...
50+
21 20 |
51+
52+
PYI059.py:26:10: PYI059 [*] `Generic[]` should always be the last base class
53+
|
54+
24 | # to flag this issue in this case as well, since after fixing the error
55+
25 | # the Generic's position issue persists.
56+
26 | class Foo(Generic, LinkedList): # PYI059
57+
| ^^^^^^^^^^^^^^^^^^^^^ PYI059
58+
27 | pass
59+
|
60+
= help: Move `Generic[]` to the end
61+
62+
ℹ Safe fix
63+
23 23 | # Inheriting from just `Generic` is a TypeError, but it's probably fine
64+
24 24 | # to flag this issue in this case as well, since after fixing the error
65+
25 25 | # the Generic's position issue persists.
66+
26 |-class Foo(Generic, LinkedList): # PYI059
67+
26 |+class Foo(LinkedList, Generic): # PYI059
68+
27 27 | pass
69+
28 28 |
70+
29 29 |
71+
72+
PYI059.py:30:10: PYI059 [*] `Generic[]` should always be the last base class
73+
|
74+
30 | class Foo( # comment about the bracket
75+
| __________^
76+
31 | | # Part 1 of multiline comment 1
77+
32 | | # Part 2 of multiline comment 1
78+
33 | | Generic[T] # comment about Generic[T] # PYI059
79+
34 | | # another comment?
80+
35 | | , # comment about the comma?
81+
36 | | # part 1 of multiline comment 2
82+
37 | | # part 2 of multiline comment 2
83+
38 | | int, # comment about int
84+
39 | | # yet another comment?
85+
40 | | ): # and another one for good measure
86+
| |_^ PYI059
87+
41 | ...
88+
|
89+
= help: Move `Generic[]` to the end
90+
91+
ℹ Safe fix
92+
30 30 | class Foo( # comment about the bracket
93+
31 31 | # Part 1 of multiline comment 1
94+
32 32 | # Part 2 of multiline comment 1
95+
33 |- Generic[T] # comment about Generic[T] # PYI059
96+
34 |- # another comment?
97+
35 |- , # comment about the comma?
98+
33 |+ # comment about the comma?
99+
36 34 | # part 1 of multiline comment 2
100+
37 35 | # part 2 of multiline comment 2
101+
38 |- int, # comment about int
102+
36 |+ int, Generic[T], # comment about int
103+
39 37 | # yet another comment?
104+
40 38 | ): # and another one for good measure
105+
41 39 | ...
106+
107+
PYI059.py:45:8: PYI059 `Generic[]` should always be the last base class
108+
|
109+
44 | # in case of multiple Generic[] inheritance, don't fix it.
110+
45 | class C(Generic[T], Generic[K, V]): ... # PYI059
111+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI059
112+
|
113+
= help: Move `Generic[]` to the end

0 commit comments

Comments
 (0)