mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-04 10:48:32 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[ty] Improve UX for [duplicate-base] diagnostics (#17914)

Browse source
This commit is contained in:
Alex Waygood 2025-05-07 16:27:37 +01:00 • committed by GitHub
parent ad658f4d68
commit 2ec0d7e072
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 594 additions and 44 deletions
Download patch file Download diff file Expand all files Collapse all files

85
crates/ty_python_semantic/resources/mdtest/mro.md
View file

@ -288,26 +288,105 @@ reveal_type(Z.__mro__) # revealed: tuple[<class 'Z'>, Unknown, <class 'object'>
## `__bases__` lists with duplicate bases
<!-- snapshot-diagnostics -->
```py
class Foo(str, str): ... # error: 16 [duplicate-base] "Duplicate base class `str`"
from typing_extensions import reveal_type
class Foo(str, str): ... # error: [duplicate-base] "Duplicate base class `str`"
reveal_type(Foo.__mro__) # revealed: tuple[<class 'Foo'>, Unknown, <class 'object'>]
class Spam: ...
class Eggs: ...
class Bar: ...
class Baz: ...
# fmt: off
# error: [duplicate-base] "Duplicate base class `Spam`"
# error: [duplicate-base] "Duplicate base class `Eggs`"
class Ham(
Spam,
Eggs,
Spam, # error: [duplicate-base] "Duplicate base class `Spam`"
Eggs, # error: [duplicate-base] "Duplicate base class `Eggs`"
Bar,
Baz,
Spam,
Eggs,
): ...
# fmt: on
reveal_type(Ham.__mro__) # revealed: tuple[<class 'Ham'>, Unknown, <class 'object'>]
class Mushrooms: ...
class Omelette(Spam, Eggs, Mushrooms, Mushrooms): ... # error: [duplicate-base]
reveal_type(Omelette.__mro__) # revealed: tuple[<class 'Omelette'>, Unknown, <class 'object'>]
# fmt: off
# error: [duplicate-base] "Duplicate base class `Eggs`"
class VeryEggyOmelette(
Eggs,
Ham,
Spam,
Eggs,
Mushrooms,
Bar,
Eggs,
Baz,
Eggs,
): ...
# fmt: off
```
A `type: ignore` comment can suppress `duplicate-bases` errors if it is on the first or last line of
the class "header":
```py
# fmt: off
class A: ...
class B( # type: ignore[duplicate-base]
A,
A,
): ...
class C(
A,
A
): # type: ignore[duplicate-base]
x: int
# fmt: on
```
But it will not suppress the error if it occurs in the class body, or on the duplicate base itself.
The justification for this is that it is the class definition as a whole that will raise an
exception at runtime, not a sub-expression in the class's bases list.
```py
# fmt: off
# error: [duplicate-base]
class D(
A,
# error: [unused-ignore-comment]
A, # type: ignore[duplicate-base]
): ...
# error: [duplicate-base]
class E(
A,
A
):
# error: [unused-ignore-comment]
x: int # type: ignore[duplicate-base]
# fmt: on
```
## `__bases__` lists with duplicate `Unknown` bases

402
crates/ty_python_semantic/resources/mdtest/snapshots/mro.md_-_Method_Resolution_Order_tests_-_`__bases__`_lists_with_duplicate_bases.snap Normal file
View file

@ -0,0 +1,402 @@
---
source: crates/ty_test/src/lib.rs
expression: snapshot
---
---
mdtest name: mro.md - Method Resolution Order tests - `__bases__` lists with duplicate bases
mdtest path: crates/ty_python_semantic/resources/mdtest/mro.md
---
# Python source files
## mdtest_snippet.py
```
1 | from typing_extensions import reveal_type
2 |
3 | class Foo(str, str): ... # error: [duplicate-base] "Duplicate base class `str`"
4 |
5 | reveal_type(Foo.__mro__) # revealed: tuple[<class 'Foo'>, Unknown, <class 'object'>]
6 |
7 | class Spam: ...
8 | class Eggs: ...
9 | class Bar: ...
10 | class Baz: ...
11 |
12 | # fmt: off
13 |
14 | # error: [duplicate-base] "Duplicate base class `Spam`"
15 | # error: [duplicate-base] "Duplicate base class `Eggs`"
16 | class Ham(
17 | Spam,
18 | Eggs,
19 | Bar,
20 | Baz,
21 | Spam,
22 | Eggs,
23 | ): ...
24 |
25 | # fmt: on
26 |
27 | reveal_type(Ham.__mro__) # revealed: tuple[<class 'Ham'>, Unknown, <class 'object'>]
28 |
29 | class Mushrooms: ...
30 | class Omelette(Spam, Eggs, Mushrooms, Mushrooms): ... # error: [duplicate-base]
31 |
32 | reveal_type(Omelette.__mro__) # revealed: tuple[<class 'Omelette'>, Unknown, <class 'object'>]
33 |
34 | # fmt: off
35 |
36 | # error: [duplicate-base] "Duplicate base class `Eggs`"
37 | class VeryEggyOmelette(
38 | Eggs,
39 | Ham,
40 | Spam,
41 | Eggs,
42 | Mushrooms,
43 | Bar,
44 | Eggs,
45 | Baz,
46 | Eggs,
47 | ): ...
48 |
49 | # fmt: off
50 | # fmt: off
51 |
52 | class A: ...
53 |
54 | class B( # type: ignore[duplicate-base]
55 | A,
56 | A,
57 | ): ...
58 |
59 | class C(
60 | A,
61 | A
62 | ): # type: ignore[duplicate-base]
63 | x: int
64 |
65 | # fmt: on
66 | # fmt: off
67 |
68 | # error: [duplicate-base]
69 | class D(
70 | A,
71 | # error: [unused-ignore-comment]
72 | A, # type: ignore[duplicate-base]
73 | ): ...
74 |
75 | # error: [duplicate-base]
76 | class E(
77 | A,
78 | A
79 | ):
80 | # error: [unused-ignore-comment]
81 | x: int # type: ignore[duplicate-base]
82 |
83 | # fmt: on
```
# Diagnostics
```
info: revealed-type: Revealed type
--> src/mdtest_snippet.py:5:1
|
3 | class Foo(str, str): ... # error: [duplicate-base] "Duplicate base class `str`"
4 |
5 | reveal_type(Foo.__mro__) # revealed: tuple[<class 'Foo'>, Unknown, <class 'object'>]
| ^^^^^^^^^^^^^^^^^^^^^^^^ `tuple[<class 'Foo'>, Unknown, <class 'object'>]`
6 |
7 | class Spam: ...
|
```
```
info: revealed-type: Revealed type
--> src/mdtest_snippet.py:27:1
|
25 | # fmt: on
26 |
27 | reveal_type(Ham.__mro__) # revealed: tuple[<class 'Ham'>, Unknown, <class 'object'>]
| ^^^^^^^^^^^^^^^^^^^^^^^^ `tuple[<class 'Ham'>, Unknown, <class 'object'>]`
28 |
29 | class Mushrooms: ...
|
```
```
info: revealed-type: Revealed type
--> src/mdtest_snippet.py:32:1
|
30 | class Omelette(Spam, Eggs, Mushrooms, Mushrooms): ... # error: [duplicate-base]
31 |
32 | reveal_type(Omelette.__mro__) # revealed: tuple[<class 'Omelette'>, Unknown, <class 'object'>]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `tuple[<class 'Omelette'>, Unknown, <class 'object'>]`
33 |
34 | # fmt: off
|
```
```
error: lint:duplicate-base: Duplicate base class `Spam`
--> src/mdtest_snippet.py:16:7
|
14 | # error: [duplicate-base] "Duplicate base class `Spam`"
15 | # error: [duplicate-base] "Duplicate base class `Eggs`"
16 | class Ham(
| _______^
17 | | Spam,
18 | | Eggs,
19 | | Bar,
20 | | Baz,
21 | | Spam,
22 | | Eggs,
23 | | ): ...
| |_^
24 |
25 | # fmt: on
|
info: The definition of class `Ham` will raise `TypeError` at runtime
--> src/mdtest_snippet.py:17:5
|
15 | # error: [duplicate-base] "Duplicate base class `Eggs`"
16 | class Ham(
17 | Spam,
| ---- Class `Spam` first included in bases list here
18 | Eggs,
19 | Bar,
20 | Baz,
21 | Spam,
| ^^^^ Class `Spam` later repeated here
22 | Eggs,
23 | ): ...
|
info: `lint:duplicate-base` is enabled by default
```
```
error: lint:duplicate-base: Duplicate base class `Eggs`
--> src/mdtest_snippet.py:16:7
|
14 | # error: [duplicate-base] "Duplicate base class `Spam`"
15 | # error: [duplicate-base] "Duplicate base class `Eggs`"
16 | class Ham(
| _______^
17 | | Spam,
18 | | Eggs,
19 | | Bar,
20 | | Baz,
21 | | Spam,
22 | | Eggs,
23 | | ): ...
| |_^
24 |
25 | # fmt: on
|
info: The definition of class `Ham` will raise `TypeError` at runtime
--> src/mdtest_snippet.py:18:5
|
16 | class Ham(
17 | Spam,
18 | Eggs,
| ---- Class `Eggs` first included in bases list here
19 | Bar,
20 | Baz,
21 | Spam,
22 | Eggs,
| ^^^^ Class `Eggs` later repeated here
23 | ): ...
|
info: `lint:duplicate-base` is enabled by default
```
```
error: lint:duplicate-base: Duplicate base class `A`
--> src/mdtest_snippet.py:76:7
|
75 | # error: [duplicate-base]
76 | class E(
| _______^
77 | | A,
78 | | A
79 | | ):
| |_^
80 | # error: [unused-ignore-comment]
81 | x: int # type: ignore[duplicate-base]
|
info: The definition of class `E` will raise `TypeError` at runtime
--> src/mdtest_snippet.py:77:5
|
75 | # error: [duplicate-base]
76 | class E(
77 | A,
| - Class `A` first included in bases list here
78 | A
| ^ Class `A` later repeated here
79 | ):
80 | # error: [unused-ignore-comment]
|
info: `lint:duplicate-base` is enabled by default
```
```
error: lint:duplicate-base: Duplicate base class `A`
--> src/mdtest_snippet.py:69:7
|
68 | # error: [duplicate-base]
69 | class D(
| _______^
70 | | A,
71 | | # error: [unused-ignore-comment]
72 | | A, # type: ignore[duplicate-base]
73 | | ): ...
| |_^
74 |
75 | # error: [duplicate-base]
|
info: The definition of class `D` will raise `TypeError` at runtime
--> src/mdtest_snippet.py:70:5
|
68 | # error: [duplicate-base]
69 | class D(
70 | A,
| - Class `A` first included in bases list here
71 | # error: [unused-ignore-comment]
72 | A, # type: ignore[duplicate-base]
| ^ Class `A` later repeated here
73 | ): ...
|
info: `lint:duplicate-base` is enabled by default
```
```
error: lint:duplicate-base: Duplicate base class `Mushrooms`
--> src/mdtest_snippet.py:30:7
|
29 | class Mushrooms: ...
30 | class Omelette(Spam, Eggs, Mushrooms, Mushrooms): ... # error: [duplicate-base]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
31 |
32 | reveal_type(Omelette.__mro__) # revealed: tuple[<class 'Omelette'>, Unknown, <class 'object'>]
|
info: The definition of class `Omelette` will raise `TypeError` at runtime
--> src/mdtest_snippet.py:30:28
|
29 | class Mushrooms: ...
30 | class Omelette(Spam, Eggs, Mushrooms, Mushrooms): ... # error: [duplicate-base]
| --------- ^^^^^^^^^ Class `Mushrooms` later repeated here
| |
| Class `Mushrooms` first included in bases list here
31 |
32 | reveal_type(Omelette.__mro__) # revealed: tuple[<class 'Omelette'>, Unknown, <class 'object'>]
|
info: `lint:duplicate-base` is enabled by default
```
```
error: lint:duplicate-base: Duplicate base class `Eggs`
--> src/mdtest_snippet.py:37:7
|
36 | # error: [duplicate-base] "Duplicate base class `Eggs`"
37 | class VeryEggyOmelette(
| _______^
38 | | Eggs,
39 | | Ham,
40 | | Spam,
41 | | Eggs,
42 | | Mushrooms,
43 | | Bar,
44 | | Eggs,
45 | | Baz,
46 | | Eggs,
47 | | ): ...
| |_^
48 |
49 | # fmt: off
|
info: The definition of class `VeryEggyOmelette` will raise `TypeError` at runtime
--> src/mdtest_snippet.py:38:5
|
36 | # error: [duplicate-base] "Duplicate base class `Eggs`"
37 | class VeryEggyOmelette(
38 | Eggs,
| ---- Class `Eggs` first included in bases list here
39 | Ham,
40 | Spam,
41 | Eggs,
| ^^^^ Class `Eggs` later repeated here
42 | Mushrooms,
43 | Bar,
44 | Eggs,
| ^^^^ Class `Eggs` later repeated here
45 | Baz,
46 | Eggs,
| ^^^^ Class `Eggs` later repeated here
47 | ): ...
|
info: `lint:duplicate-base` is enabled by default
```
```
error: lint:duplicate-base: Duplicate base class `str`
--> src/mdtest_snippet.py:3:7
|
1 | from typing_extensions import reveal_type
2 |
3 | class Foo(str, str): ... # error: [duplicate-base] "Duplicate base class `str`"
| ^^^^^^^^^^^^^
4 |
5 | reveal_type(Foo.__mro__) # revealed: tuple[<class 'Foo'>, Unknown, <class 'object'>]
|
info: The definition of class `Foo` will raise `TypeError` at runtime
--> src/mdtest_snippet.py:3:11
|
1 | from typing_extensions import reveal_type
2 |
3 | class Foo(str, str): ... # error: [duplicate-base] "Duplicate base class `str`"
| --- ^^^ Class `str` later repeated here
| |
| Class `str` first included in bases list here
4 |
5 | reveal_type(Foo.__mro__) # revealed: tuple[<class 'Foo'>, Unknown, <class 'object'>]
|
info: `lint:duplicate-base` is enabled by default
```
```
warning: lint:unused-ignore-comment
--> src/mdtest_snippet.py:72:9
|
70 | A,
71 | # error: [unused-ignore-comment]
72 | A, # type: ignore[duplicate-base]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unused blanket `type: ignore` directive
73 | ): ...
|
```
```
warning: lint:unused-ignore-comment
--> src/mdtest_snippet.py:81:13
|
79 | ):
80 | # error: [unused-ignore-comment]
81 | x: int # type: ignore[duplicate-base]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unused blanket `type: ignore` directive
82 |
83 | # fmt: on
|
```

16
crates/ty_python_semantic/src/types/class.rs
View file

@ -1799,26 +1799,32 @@ impl<'db> ClassLiteral<'db> {
}
}
/// Returns the [`Span`] of the class's "header": the class name
/// Returns a [`Span`] with the range of the class's header.
///
/// See [`Self::header_range`] for more details.
pub(super) fn header_span(self, db: &'db dyn Db) -> Span {
Span::from(self.file(db)).with_range(self.header_range(db))
}
/// Returns the range of the class's "header": the class name
/// and any arguments passed to the `class` statement. E.g.
///
/// ```ignore
/// class Foo(Bar, metaclass=Baz): ...
/// ^^^^^^^^^^^^^^^^^^^^^^^
/// ```
pub(super) fn header_span(self, db: &'db dyn Db) -> Span {
pub(super) fn header_range(self, db: &'db dyn Db) -> TextRange {
let class_scope = self.body_scope(db);
let class_node = class_scope.node(db).expect_class();
let class_name = &class_node.name;
let header_range = TextRange::new(
TextRange::new(
class_name.start(),
class_node
.arguments
.as_deref()
.map(Ranged::end)
.unwrap_or_else(|| class_name.end()),
);
Span::from(class_scope.file(db)).with_range(header_range)
)
}
}

49
crates/ty_python_semantic/src/types/diagnostic.rs
View file

@ -1,4 +1,5 @@
use super::context::InferContext;
use super::mro::DuplicateBaseError;
use super::{ClassLiteral, KnownClass};
use crate::db::Db;
use crate::declare_lint;
@ -1595,3 +1596,51 @@ pub(crate) fn report_attempted_protocol_instantiation(
);
diagnostic.sub(class_def_diagnostic);
}
pub(crate) fn report_duplicate_bases(
context: &InferContext,
class: ClassLiteral,
duplicate_base_error: &DuplicateBaseError,
bases_list: &[ast::Expr],
) {
let db = context.db();
let Some(builder) = context.report_lint(&DUPLICATE_BASE, class.header_range(db)) else {
return;
};
let DuplicateBaseError {
duplicate_base,
first_index,
later_indices,
} = duplicate_base_error;
let duplicate_name = duplicate_base.name(db);
let mut diagnostic =
builder.into_diagnostic(format_args!("Duplicate base class `{duplicate_name}`",));
let mut sub_diagnostic = SubDiagnostic::new(
Severity::Info,
format_args!(
"The definition of class `{}` will raise `TypeError` at runtime",
class.name(db)
),
);
sub_diagnostic.annotate(
Annotation::secondary(
Span::from(context.file()).with_range(bases_list[*first_index].range()),
)
.message(format_args!(
"Class `{duplicate_name}` first included in bases list here"
)),
);
for index in later_indices {
sub_diagnostic.annotate(
Annotation::primary(Span::from(context.file()).with_range(bases_list[*index].range()))
.message(format_args!("Class `{duplicate_name}` later repeated here")),
);
}
diagnostic.sub(sub_diagnostic);
}

26
crates/ty_python_semantic/src/types/infer.rs
View file

@ -72,9 +72,9 @@ use crate::types::diagnostic::{
report_invalid_attribute_assignment, report_invalid_generator_function_return_type,
report_invalid_return_type, report_possibly_unbound_attribute, TypeCheckDiagnostics,
CALL_NON_CALLABLE, CALL_POSSIBLY_UNBOUND_METHOD, CONFLICTING_DECLARATIONS,
CONFLICTING_METACLASS, CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, DUPLICATE_BASE,
INCONSISTENT_MRO, INVALID_ARGUMENT_TYPE, INVALID_ASSIGNMENT, INVALID_ATTRIBUTE_ACCESS,
INVALID_BASE, INVALID_DECLARATION, INVALID_GENERIC_CLASS, INVALID_LEGACY_TYPE_VARIABLE,
CONFLICTING_METACLASS, CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, INCONSISTENT_MRO,
INVALID_ARGUMENT_TYPE, INVALID_ASSIGNMENT, INVALID_ATTRIBUTE_ACCESS, INVALID_BASE,
INVALID_DECLARATION, INVALID_GENERIC_CLASS, INVALID_LEGACY_TYPE_VARIABLE,
INVALID_PARAMETER_DEFAULT, INVALID_TYPE_FORM, INVALID_TYPE_VARIABLE_CONSTRAINTS,
POSSIBLY_UNBOUND_IMPORT, UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE, UNRESOLVED_IMPORT,
UNSUPPORTED_OPERATOR,
@ -99,9 +99,10 @@ use crate::{Db, FxOrderSet};
use super::context::{InNoTypeCheck, InferContext};
use super::diagnostic::{
report_attempted_protocol_instantiation, report_bad_argument_to_get_protocol_members,
report_index_out_of_bounds, report_invalid_exception_caught, report_invalid_exception_cause,
report_invalid_exception_raised, report_invalid_type_checking_constant,
report_non_subscriptable, report_possibly_unresolved_reference,
report_duplicate_bases, report_index_out_of_bounds, report_invalid_exception_caught,
report_invalid_exception_cause, report_invalid_exception_raised,
report_invalid_type_checking_constant, report_non_subscriptable,
report_possibly_unresolved_reference,
report_runtime_check_against_non_runtime_checkable_protocol, report_slice_step_size_zero,
report_unresolved_reference, INVALID_METACLASS, INVALID_OVERLOAD, INVALID_PROTOCOL,
REDUNDANT_CAST, STATIC_ASSERT_ERROR, SUBCLASS_OF_FINAL_CLASS, TYPE_ASSERTION_FAILURE,
@ -855,17 +856,8 @@ impl<'db> TypeInferenceBuilder<'db> {
match mro_error.reason() {
MroErrorKind::DuplicateBases(duplicates) => {
let base_nodes = class_node.bases();
for (index, duplicate) in duplicates {
let Some(builder) = self
.context
.report_lint(&DUPLICATE_BASE, &base_nodes[*index])
else {
continue;
};
builder.into_diagnostic(format_args!(
"Duplicate base class `{}`",
duplicate.name(self.db())
));
for duplicate in duplicates {
report_duplicate_bases(&self.context, class, duplicate, base_nodes);
}
}
MroErrorKind::InvalidBases(bases) => {

60
crates/ty_python_semantic/src/types/mro.rs
View file

@ -1,7 +1,7 @@
use std::collections::VecDeque;
use std::ops::Deref;
use rustc_hash::FxHashSet;
use rustc_hash::FxHashMap;
use crate::types::class_base::ClassBase;
use crate::types::generics::Specialization;
@ -154,25 +154,40 @@ impl<'db> Mro<'db> {
);
c3_merge(seqs).ok_or_else(|| {
let mut seen_bases = FxHashSet::default();
let mut duplicate_bases = vec![];
for (index, base) in valid_bases
.iter()
.enumerate()
.filter_map(|(index, base)| Some((index, base.into_class()?)))
{
if !seen_bases.insert(base) {
let (base_class_literal, _) = base.class_literal(db);
duplicate_bases.push((index, base_class_literal));
let duplicate_bases: Box<[DuplicateBaseError<'db>]> = {
let mut base_to_indices: FxHashMap<ClassType<'db>, Vec<usize>> =
FxHashMap::default();
for (index, base) in valid_bases
.iter()
.enumerate()
.filter_map(|(index, base)| Some((index, base.into_class()?)))
{
base_to_indices.entry(base).or_default().push(index);
}
}
base_to_indices
.iter()
.filter_map(|(base, indices)| {
let (first_index, later_indices) = indices.split_first()?;
if later_indices.is_empty() {
return None;
}
Some(DuplicateBaseError {
duplicate_base: base.class_literal(db).0,
first_index: *first_index,
later_indices: later_indices.iter().copied().collect(),
})
})
.collect()
};
if duplicate_bases.is_empty() {
MroErrorKind::UnresolvableMro {
bases_list: valid_bases.into_boxed_slice(),
}
} else {
MroErrorKind::DuplicateBases(duplicate_bases.into_boxed_slice())
MroErrorKind::DuplicateBases(duplicate_bases)
}
})
}
@ -328,12 +343,8 @@ pub(super) enum MroErrorKind<'db> {
InvalidBases(Box<[(usize, Type<'db>)]>),
/// The class has one or more duplicate bases.
///
/// This variant records the indices and [`ClassLiteral`]s
/// of the duplicate bases. The indices are the indices of nodes
/// in the bases list of the class's [`StmtClassDef`](ruff_python_ast::StmtClassDef) node.
/// Each index is the index of a node representing a duplicate base.
DuplicateBases(Box<[(usize, ClassLiteral<'db>)]>),
/// See [`DuplicateBaseError`] for more details.
DuplicateBases(Box<[DuplicateBaseError<'db>]>),
/// The MRO is otherwise unresolvable through the C3-merge algorithm.
///
@ -350,6 +361,17 @@ impl<'db> MroErrorKind<'db> {
}
}
/// Error recording the fact that a class definition was found to have duplicate bases.
#[derive(Debug, PartialEq, Eq, salsa::Update)]
pub(super) struct DuplicateBaseError<'db> {
/// The base that is duplicated in the class's bases list.
pub(super) duplicate_base: ClassLiteral<'db>,
/// The index of the first occurrence of the base in the class's bases list.
pub(super) first_index: usize,
/// The indices of the base's later occurrences in the class's bases list.
pub(super) later_indices: Box<[usize]>,
}
/// Implementation of the [C3-merge algorithm] for calculating a Python class's
/// [method resolution order].
///