mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-01 08:07:31 +00:00
[ty] Report when a dataclass contains more than one KW_ONLY
field (#18731)
## Summary Part of [#111](https://github.com/astral-sh/ty/issues/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.
This commit is contained in:
parent
50bf3fa45a
commit
20d73dd41c
7 changed files with 308 additions and 69 deletions
|
@ -715,6 +715,8 @@ asdict(Foo)
|
|||
|
||||
## `dataclasses.KW_ONLY`
|
||||
|
||||
<!-- snapshot-diagnostics -->
|
||||
|
||||
If an attribute is annotated with `dataclasses.KW_ONLY`, it is not added to the synthesized
|
||||
`__init__` of the class. Instead, this special marker annotation causes Python at runtime to ensure
|
||||
that all annotations following it have keyword-only parameters generated for them in the class's
|
||||
|
@ -727,6 +729,7 @@ python-version = "3.10"
|
|||
|
||||
```py
|
||||
from dataclasses import dataclass, field, KW_ONLY
|
||||
from typing_extensions import reveal_type
|
||||
|
||||
@dataclass
|
||||
class C:
|
||||
|
@ -734,6 +737,8 @@ class C:
|
|||
_: KW_ONLY
|
||||
y: str
|
||||
|
||||
reveal_type(C.__init__) # revealed: (self: C, x: int, *, y: str) -> None
|
||||
|
||||
# error: [missing-argument]
|
||||
# error: [too-many-positional-arguments]
|
||||
C(3, "")
|
||||
|
@ -746,14 +751,14 @@ runtime:
|
|||
|
||||
```py
|
||||
@dataclass
|
||||
class Fails:
|
||||
class Fails: # error: [duplicate-kw-only]
|
||||
a: int
|
||||
b: KW_ONLY
|
||||
c: str
|
||||
|
||||
# TODO: we should emit an error here
|
||||
# (two different names with `KW_ONLY` annotations in the same dataclass means the class fails at runtime)
|
||||
d: KW_ONLY
|
||||
e: bytes
|
||||
|
||||
reveal_type(Fails.__init__) # revealed: (self: Fails, a: int, *, c: str, e: bytes) -> None
|
||||
```
|
||||
|
||||
## Other special cases
|
||||
|
|
|
@ -0,0 +1,114 @@
|
|||
---
|
||||
source: crates/ty_test/src/lib.rs
|
||||
expression: snapshot
|
||||
---
|
||||
---
|
||||
mdtest name: dataclasses.md - Dataclasses - `dataclasses.KW_ONLY`
|
||||
mdtest path: crates/ty_python_semantic/resources/mdtest/dataclasses.md
|
||||
---
|
||||
|
||||
# Python source files
|
||||
|
||||
## mdtest_snippet.py
|
||||
|
||||
```
|
||||
1 | from dataclasses import dataclass, field, KW_ONLY
|
||||
2 | from typing_extensions import reveal_type
|
||||
3 |
|
||||
4 | @dataclass
|
||||
5 | class C:
|
||||
6 | x: int
|
||||
7 | _: KW_ONLY
|
||||
8 | y: str
|
||||
9 |
|
||||
10 | reveal_type(C.__init__) # revealed: (self: C, x: int, *, y: str) -> None
|
||||
11 |
|
||||
12 | # error: [missing-argument]
|
||||
13 | # error: [too-many-positional-arguments]
|
||||
14 | C(3, "")
|
||||
15 |
|
||||
16 | C(3, y="")
|
||||
17 | @dataclass
|
||||
18 | class Fails: # error: [duplicate-kw-only]
|
||||
19 | a: int
|
||||
20 | b: KW_ONLY
|
||||
21 | c: str
|
||||
22 | d: KW_ONLY
|
||||
23 | e: bytes
|
||||
24 |
|
||||
25 | reveal_type(Fails.__init__) # revealed: (self: Fails, a: int, *, c: str, e: bytes) -> None
|
||||
```
|
||||
|
||||
# Diagnostics
|
||||
|
||||
```
|
||||
info[revealed-type]: Revealed type
|
||||
--> src/mdtest_snippet.py:10:13
|
||||
|
|
||||
8 | y: str
|
||||
9 |
|
||||
10 | reveal_type(C.__init__) # revealed: (self: C, x: int, *, y: str) -> None
|
||||
| ^^^^^^^^^^ `(self: C, x: int, *, y: str) -> None`
|
||||
11 |
|
||||
12 | # error: [missing-argument]
|
||||
|
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
error[missing-argument]: No argument provided for required parameter `y`
|
||||
--> src/mdtest_snippet.py:14:1
|
||||
|
|
||||
12 | # error: [missing-argument]
|
||||
13 | # error: [too-many-positional-arguments]
|
||||
14 | C(3, "")
|
||||
| ^^^^^^^^
|
||||
15 |
|
||||
16 | C(3, y="")
|
||||
|
|
||||
info: rule `missing-argument` is enabled by default
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
error[too-many-positional-arguments]: Too many positional arguments: expected 1, got 2
|
||||
--> src/mdtest_snippet.py:14:6
|
||||
|
|
||||
12 | # error: [missing-argument]
|
||||
13 | # error: [too-many-positional-arguments]
|
||||
14 | C(3, "")
|
||||
| ^^
|
||||
15 |
|
||||
16 | C(3, y="")
|
||||
|
|
||||
info: rule `too-many-positional-arguments` is enabled by default
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
error[duplicate-kw-only]: Dataclass has more than one field annotated with `KW_ONLY`
|
||||
--> src/mdtest_snippet.py:18:7
|
||||
|
|
||||
16 | C(3, y="")
|
||||
17 | @dataclass
|
||||
18 | class Fails: # error: [duplicate-kw-only]
|
||||
| ^^^^^
|
||||
19 | a: int
|
||||
20 | b: KW_ONLY
|
||||
|
|
||||
info: `KW_ONLY` fields: `b`, `d`
|
||||
info: rule `duplicate-kw-only` is enabled by default
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
info[revealed-type]: Revealed type
|
||||
--> src/mdtest_snippet.py:25:13
|
||||
|
|
||||
23 | e: bytes
|
||||
24 |
|
||||
25 | reveal_type(Fails.__init__) # revealed: (self: Fails, a: int, *, c: str, e: bytes) -> None
|
||||
| ^^^^^^^^^^^^^^ `(self: Fails, a: int, *, c: str, e: bytes) -> None`
|
||||
|
|
||||
|
||||
```
|
|
@ -123,7 +123,7 @@ fn try_metaclass_cycle_initial<'db>(
|
|||
|
||||
/// A category of classes with code generation capabilities (with synthesized methods).
|
||||
#[derive(Clone, Copy, Debug, PartialEq)]
|
||||
enum CodeGeneratorKind {
|
||||
pub(crate) enum CodeGeneratorKind {
|
||||
/// Classes decorated with `@dataclass` or similar dataclass-like decorators
|
||||
DataclassLike,
|
||||
/// Classes inheriting from `typing.NamedTuple`
|
||||
|
@ -131,7 +131,7 @@ enum CodeGeneratorKind {
|
|||
}
|
||||
|
||||
impl CodeGeneratorKind {
|
||||
fn from_class(db: &dyn Db, class: ClassLiteral<'_>) -> Option<Self> {
|
||||
pub(crate) fn from_class(db: &dyn Db, class: ClassLiteral<'_>) -> Option<Self> {
|
||||
if CodeGeneratorKind::DataclassLike.matches(db, class) {
|
||||
Some(CodeGeneratorKind::DataclassLike)
|
||||
} else if CodeGeneratorKind::NamedTuple.matches(db, class) {
|
||||
|
@ -1322,7 +1322,7 @@ impl<'db> ClassLiteral<'db> {
|
|||
.is_some_and(|instance| instance.class.is_known(db, KnownClass::KwOnly))
|
||||
{
|
||||
// Attributes annotated with `dataclass.KW_ONLY` are not present in the synthesized
|
||||
// `__init__` method, ; they only used to indicate that the parameters after this are
|
||||
// `__init__` method; they are used to indicate that the following parameters are
|
||||
// keyword-only.
|
||||
kw_only_field_seen = true;
|
||||
continue;
|
||||
|
@ -1455,7 +1455,7 @@ impl<'db> ClassLiteral<'db> {
|
|||
/// Returns a list of all annotated attributes defined in this class, or any of its superclasses.
|
||||
///
|
||||
/// See [`ClassLiteral::own_fields`] for more details.
|
||||
fn fields(
|
||||
pub(crate) fn fields(
|
||||
self,
|
||||
db: &'db dyn Db,
|
||||
specialization: Option<Specialization<'db>>,
|
||||
|
|
|
@ -33,6 +33,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) {
|
|||
registry.register_lint(&CYCLIC_CLASS_DEFINITION);
|
||||
registry.register_lint(&DIVISION_BY_ZERO);
|
||||
registry.register_lint(&DUPLICATE_BASE);
|
||||
registry.register_lint(&DUPLICATE_KW_ONLY);
|
||||
registry.register_lint(&INCOMPATIBLE_SLOTS);
|
||||
registry.register_lint(&INCONSISTENT_MRO);
|
||||
registry.register_lint(&INDEX_OUT_OF_BOUNDS);
|
||||
|
@ -277,6 +278,38 @@ declare_lint! {
|
|||
}
|
||||
}
|
||||
|
||||
declare_lint! {
|
||||
/// ## What it does
|
||||
/// Checks for dataclass definitions with more than one field
|
||||
/// annotated with `KW_ONLY`.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// `dataclasses.KW_ONLY` is a special marker used to
|
||||
/// emulate the `*` syntax in normal signatures.
|
||||
/// It can only be used once per dataclass.
|
||||
///
|
||||
/// Attempting to annotate two different fields with
|
||||
/// it will lead to a runtime error.
|
||||
///
|
||||
/// ## Examples
|
||||
/// ```python
|
||||
/// from dataclasses import dataclass, KW_ONLY
|
||||
///
|
||||
/// @dataclass
|
||||
/// class A: # Crash at runtime
|
||||
/// b: int
|
||||
/// _1: KW_ONLY
|
||||
/// c: str
|
||||
/// _2: KW_ONLY
|
||||
/// d: bytes
|
||||
/// ```
|
||||
pub(crate) static DUPLICATE_KW_ONLY = {
|
||||
summary: "detects dataclass definitions with more than once usages of `KW_ONLY`",
|
||||
status: LintStatus::preview("1.0.0"),
|
||||
default_level: Level::Error,
|
||||
}
|
||||
}
|
||||
|
||||
declare_lint! {
|
||||
/// ## What it does
|
||||
/// Checks for classes whose bases define incompatible `__slots__`.
|
||||
|
|
|
@ -74,13 +74,13 @@ use crate::semantic_index::{
|
|||
use crate::types::call::{
|
||||
Argument, Binding, Bindings, CallArgumentTypes, CallArguments, CallError,
|
||||
};
|
||||
use crate::types::class::{MetaclassErrorKind, SliceLiteral};
|
||||
use crate::types::class::{CodeGeneratorKind, MetaclassErrorKind, SliceLiteral};
|
||||
use crate::types::diagnostic::{
|
||||
self, CALL_NON_CALLABLE, CONFLICTING_DECLARATIONS, 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_ALIAS_TYPE, INVALID_TYPE_FORM, INVALID_TYPE_GUARD_CALL,
|
||||
CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, DUPLICATE_KW_ONLY, 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_ALIAS_TYPE, INVALID_TYPE_FORM, INVALID_TYPE_GUARD_CALL,
|
||||
INVALID_TYPE_VARIABLE_CONSTRAINTS, POSSIBLY_UNBOUND_IMPLICIT_CALL, POSSIBLY_UNBOUND_IMPORT,
|
||||
TypeCheckDiagnostics, UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE, UNRESOLVED_IMPORT,
|
||||
UNRESOLVED_REFERENCE, UNSUPPORTED_OPERATOR, report_implicit_return_type,
|
||||
|
@ -1114,6 +1114,46 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
// (5) Check that a dataclass does not have more than one `KW_ONLY`.
|
||||
if let Some(field_policy @ CodeGeneratorKind::DataclassLike) =
|
||||
CodeGeneratorKind::from_class(self.db(), class)
|
||||
{
|
||||
let specialization = None;
|
||||
let mut kw_only_field_names = vec![];
|
||||
|
||||
for (name, (attr_ty, _)) in class.fields(self.db(), specialization, field_policy) {
|
||||
let Some(instance) = attr_ty.into_nominal_instance() else {
|
||||
continue;
|
||||
};
|
||||
|
||||
if !instance.class.is_known(self.db(), KnownClass::KwOnly) {
|
||||
continue;
|
||||
}
|
||||
|
||||
kw_only_field_names.push(name);
|
||||
}
|
||||
|
||||
if kw_only_field_names.len() > 1 {
|
||||
// TODO: The fields should be displayed in a subdiagnostic.
|
||||
if let Some(builder) = self
|
||||
.context
|
||||
.report_lint(&DUPLICATE_KW_ONLY, &class_node.name)
|
||||
{
|
||||
let mut diagnostic = builder.into_diagnostic(format_args!(
|
||||
"Dataclass has more than one field annotated with `KW_ONLY`"
|
||||
));
|
||||
|
||||
diagnostic.info(format_args!(
|
||||
"`KW_ONLY` fields: {}",
|
||||
kw_only_field_names
|
||||
.iter()
|
||||
.map(|name| format!("`{name}`"))
|
||||
.join(", ")
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue