Skip to content

Commit 20d73dd

Browse files
[ty] Report when a dataclass contains more than one KW_ONLY field (#18731)
## Summary Part of [#111](astral-sh/ty#111). After this change, dataclasses with two or more `KW_ONLY` field will be reported as invalid. The duplicate fields will simply be ignored when computing `__init__`'s signature. ## Test Plan Markdown tests.
1 parent 50bf3fa commit 20d73dd

File tree

7 files changed

+308
-69
lines changed

7 files changed

+308
-69
lines changed

crates/ty/docs/rules.md

Lines changed: 93 additions & 56 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/ty_python_semantic/resources/mdtest/dataclasses.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,8 @@ asdict(Foo)
715715

716716
## `dataclasses.KW_ONLY`
717717

718+
<!-- snapshot-diagnostics -->
719+
718720
If an attribute is annotated with `dataclasses.KW_ONLY`, it is not added to the synthesized
719721
`__init__` of the class. Instead, this special marker annotation causes Python at runtime to ensure
720722
that all annotations following it have keyword-only parameters generated for them in the class's
@@ -727,13 +729,16 @@ python-version = "3.10"
727729

728730
```py
729731
from dataclasses import dataclass, field, KW_ONLY
732+
from typing_extensions import reveal_type
730733

731734
@dataclass
732735
class C:
733736
x: int
734737
_: KW_ONLY
735738
y: str
736739

740+
reveal_type(C.__init__) # revealed: (self: C, x: int, *, y: str) -> None
741+
737742
# error: [missing-argument]
738743
# error: [too-many-positional-arguments]
739744
C(3, "")
@@ -746,14 +751,14 @@ runtime:
746751

747752
```py
748753
@dataclass
749-
class Fails:
754+
class Fails: # error: [duplicate-kw-only]
750755
a: int
751756
b: KW_ONLY
752757
c: str
753-
754-
# TODO: we should emit an error here
755-
# (two different names with `KW_ONLY` annotations in the same dataclass means the class fails at runtime)
756758
d: KW_ONLY
759+
e: bytes
760+
761+
reveal_type(Fails.__init__) # revealed: (self: Fails, a: int, *, c: str, e: bytes) -> None
757762
```
758763

759764
## Other special cases
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
---
2+
source: crates/ty_test/src/lib.rs
3+
expression: snapshot
4+
---
5+
---
6+
mdtest name: dataclasses.md - Dataclasses - `dataclasses.KW_ONLY`
7+
mdtest path: crates/ty_python_semantic/resources/mdtest/dataclasses.md
8+
---
9+
10+
# Python source files
11+
12+
## mdtest_snippet.py
13+
14+
```
15+
1 | from dataclasses import dataclass, field, KW_ONLY
16+
2 | from typing_extensions import reveal_type
17+
3 |
18+
4 | @dataclass
19+
5 | class C:
20+
6 | x: int
21+
7 | _: KW_ONLY
22+
8 | y: str
23+
9 |
24+
10 | reveal_type(C.__init__) # revealed: (self: C, x: int, *, y: str) -> None
25+
11 |
26+
12 | # error: [missing-argument]
27+
13 | # error: [too-many-positional-arguments]
28+
14 | C(3, "")
29+
15 |
30+
16 | C(3, y="")
31+
17 | @dataclass
32+
18 | class Fails: # error: [duplicate-kw-only]
33+
19 | a: int
34+
20 | b: KW_ONLY
35+
21 | c: str
36+
22 | d: KW_ONLY
37+
23 | e: bytes
38+
24 |
39+
25 | reveal_type(Fails.__init__) # revealed: (self: Fails, a: int, *, c: str, e: bytes) -> None
40+
```
41+
42+
# Diagnostics
43+
44+
```
45+
info[revealed-type]: Revealed type
46+
--> src/mdtest_snippet.py:10:13
47+
|
48+
8 | y: str
49+
9 |
50+
10 | reveal_type(C.__init__) # revealed: (self: C, x: int, *, y: str) -> None
51+
| ^^^^^^^^^^ `(self: C, x: int, *, y: str) -> None`
52+
11 |
53+
12 | # error: [missing-argument]
54+
|
55+
56+
```
57+
58+
```
59+
error[missing-argument]: No argument provided for required parameter `y`
60+
--> src/mdtest_snippet.py:14:1
61+
|
62+
12 | # error: [missing-argument]
63+
13 | # error: [too-many-positional-arguments]
64+
14 | C(3, "")
65+
| ^^^^^^^^
66+
15 |
67+
16 | C(3, y="")
68+
|
69+
info: rule `missing-argument` is enabled by default
70+
71+
```
72+
73+
```
74+
error[too-many-positional-arguments]: Too many positional arguments: expected 1, got 2
75+
--> src/mdtest_snippet.py:14:6
76+
|
77+
12 | # error: [missing-argument]
78+
13 | # error: [too-many-positional-arguments]
79+
14 | C(3, "")
80+
| ^^
81+
15 |
82+
16 | C(3, y="")
83+
|
84+
info: rule `too-many-positional-arguments` is enabled by default
85+
86+
```
87+
88+
```
89+
error[duplicate-kw-only]: Dataclass has more than one field annotated with `KW_ONLY`
90+
--> src/mdtest_snippet.py:18:7
91+
|
92+
16 | C(3, y="")
93+
17 | @dataclass
94+
18 | class Fails: # error: [duplicate-kw-only]
95+
| ^^^^^
96+
19 | a: int
97+
20 | b: KW_ONLY
98+
|
99+
info: `KW_ONLY` fields: `b`, `d`
100+
info: rule `duplicate-kw-only` is enabled by default
101+
102+
```
103+
104+
```
105+
info[revealed-type]: Revealed type
106+
--> src/mdtest_snippet.py:25:13
107+
|
108+
23 | e: bytes
109+
24 |
110+
25 | reveal_type(Fails.__init__) # revealed: (self: Fails, a: int, *, c: str, e: bytes) -> None
111+
| ^^^^^^^^^^^^^^ `(self: Fails, a: int, *, c: str, e: bytes) -> None`
112+
|
113+
114+
```

crates/ty_python_semantic/src/types/class.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,15 +123,15 @@ fn try_metaclass_cycle_initial<'db>(
123123

124124
/// A category of classes with code generation capabilities (with synthesized methods).
125125
#[derive(Clone, Copy, Debug, PartialEq)]
126-
enum CodeGeneratorKind {
126+
pub(crate) enum CodeGeneratorKind {
127127
/// Classes decorated with `@dataclass` or similar dataclass-like decorators
128128
DataclassLike,
129129
/// Classes inheriting from `typing.NamedTuple`
130130
NamedTuple,
131131
}
132132

133133
impl CodeGeneratorKind {
134-
fn from_class(db: &dyn Db, class: ClassLiteral<'_>) -> Option<Self> {
134+
pub(crate) fn from_class(db: &dyn Db, class: ClassLiteral<'_>) -> Option<Self> {
135135
if CodeGeneratorKind::DataclassLike.matches(db, class) {
136136
Some(CodeGeneratorKind::DataclassLike)
137137
} else if CodeGeneratorKind::NamedTuple.matches(db, class) {
@@ -1322,7 +1322,7 @@ impl<'db> ClassLiteral<'db> {
13221322
.is_some_and(|instance| instance.class.is_known(db, KnownClass::KwOnly))
13231323
{
13241324
// Attributes annotated with `dataclass.KW_ONLY` are not present in the synthesized
1325-
// `__init__` method, ; they only used to indicate that the parameters after this are
1325+
// `__init__` method; they are used to indicate that the following parameters are
13261326
// keyword-only.
13271327
kw_only_field_seen = true;
13281328
continue;
@@ -1455,7 +1455,7 @@ impl<'db> ClassLiteral<'db> {
14551455
/// Returns a list of all annotated attributes defined in this class, or any of its superclasses.
14561456
///
14571457
/// See [`ClassLiteral::own_fields`] for more details.
1458-
fn fields(
1458+
pub(crate) fn fields(
14591459
self,
14601460
db: &'db dyn Db,
14611461
specialization: Option<Specialization<'db>>,

crates/ty_python_semantic/src/types/diagnostic.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) {
3333
registry.register_lint(&CYCLIC_CLASS_DEFINITION);
3434
registry.register_lint(&DIVISION_BY_ZERO);
3535
registry.register_lint(&DUPLICATE_BASE);
36+
registry.register_lint(&DUPLICATE_KW_ONLY);
3637
registry.register_lint(&INCOMPATIBLE_SLOTS);
3738
registry.register_lint(&INCONSISTENT_MRO);
3839
registry.register_lint(&INDEX_OUT_OF_BOUNDS);
@@ -277,6 +278,38 @@ declare_lint! {
277278
}
278279
}
279280

281+
declare_lint! {
282+
/// ## What it does
283+
/// Checks for dataclass definitions with more than one field
284+
/// annotated with `KW_ONLY`.
285+
///
286+
/// ## Why is this bad?
287+
/// `dataclasses.KW_ONLY` is a special marker used to
288+
/// emulate the `*` syntax in normal signatures.
289+
/// It can only be used once per dataclass.
290+
///
291+
/// Attempting to annotate two different fields with
292+
/// it will lead to a runtime error.
293+
///
294+
/// ## Examples
295+
/// ```python
296+
/// from dataclasses import dataclass, KW_ONLY
297+
///
298+
/// @dataclass
299+
/// class A: # Crash at runtime
300+
/// b: int
301+
/// _1: KW_ONLY
302+
/// c: str
303+
/// _2: KW_ONLY
304+
/// d: bytes
305+
/// ```
306+
pub(crate) static DUPLICATE_KW_ONLY = {
307+
summary: "detects dataclass definitions with more than once usages of `KW_ONLY`",
308+
status: LintStatus::preview("1.0.0"),
309+
default_level: Level::Error,
310+
}
311+
}
312+
280313
declare_lint! {
281314
/// ## What it does
282315
/// Checks for classes whose bases define incompatible `__slots__`.

crates/ty_python_semantic/src/types/infer.rs

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,13 @@ use crate::semantic_index::{
7474
use crate::types::call::{
7575
Argument, Binding, Bindings, CallArgumentTypes, CallArguments, CallError,
7676
};
77-
use crate::types::class::{MetaclassErrorKind, SliceLiteral};
77+
use crate::types::class::{CodeGeneratorKind, MetaclassErrorKind, SliceLiteral};
7878
use crate::types::diagnostic::{
7979
self, CALL_NON_CALLABLE, CONFLICTING_DECLARATIONS, CONFLICTING_METACLASS,
80-
CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, INCONSISTENT_MRO, INVALID_ARGUMENT_TYPE,
81-
INVALID_ASSIGNMENT, INVALID_ATTRIBUTE_ACCESS, INVALID_BASE, INVALID_DECLARATION,
82-
INVALID_GENERIC_CLASS, INVALID_LEGACY_TYPE_VARIABLE, INVALID_PARAMETER_DEFAULT,
83-
INVALID_TYPE_ALIAS_TYPE, INVALID_TYPE_FORM, INVALID_TYPE_GUARD_CALL,
80+
CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, DUPLICATE_KW_ONLY, INCONSISTENT_MRO,
81+
INVALID_ARGUMENT_TYPE, INVALID_ASSIGNMENT, INVALID_ATTRIBUTE_ACCESS, INVALID_BASE,
82+
INVALID_DECLARATION, INVALID_GENERIC_CLASS, INVALID_LEGACY_TYPE_VARIABLE,
83+
INVALID_PARAMETER_DEFAULT, INVALID_TYPE_ALIAS_TYPE, INVALID_TYPE_FORM, INVALID_TYPE_GUARD_CALL,
8484
INVALID_TYPE_VARIABLE_CONSTRAINTS, POSSIBLY_UNBOUND_IMPLICIT_CALL, POSSIBLY_UNBOUND_IMPORT,
8585
TypeCheckDiagnostics, UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE, UNRESOLVED_IMPORT,
8686
UNRESOLVED_REFERENCE, UNSUPPORTED_OPERATOR, report_implicit_return_type,
@@ -1114,6 +1114,46 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
11141114
}
11151115
}
11161116
}
1117+
1118+
// (5) Check that a dataclass does not have more than one `KW_ONLY`.
1119+
if let Some(field_policy @ CodeGeneratorKind::DataclassLike) =
1120+
CodeGeneratorKind::from_class(self.db(), class)
1121+
{
1122+
let specialization = None;
1123+
let mut kw_only_field_names = vec![];
1124+
1125+
for (name, (attr_ty, _)) in class.fields(self.db(), specialization, field_policy) {
1126+
let Some(instance) = attr_ty.into_nominal_instance() else {
1127+
continue;
1128+
};
1129+
1130+
if !instance.class.is_known(self.db(), KnownClass::KwOnly) {
1131+
continue;
1132+
}
1133+
1134+
kw_only_field_names.push(name);
1135+
}
1136+
1137+
if kw_only_field_names.len() > 1 {
1138+
// TODO: The fields should be displayed in a subdiagnostic.
1139+
if let Some(builder) = self
1140+
.context
1141+
.report_lint(&DUPLICATE_KW_ONLY, &class_node.name)
1142+
{
1143+
let mut diagnostic = builder.into_diagnostic(format_args!(
1144+
"Dataclass has more than one field annotated with `KW_ONLY`"
1145+
));
1146+
1147+
diagnostic.info(format_args!(
1148+
"`KW_ONLY` fields: {}",
1149+
kw_only_field_names
1150+
.iter()
1151+
.map(|name| format!("`{name}`"))
1152+
.join(", ")
1153+
));
1154+
}
1155+
}
1156+
}
11171157
}
11181158
}
11191159

ty.schema.json

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)