Skip to content

Commit 3e9dd1b

Browse files
committed
[ty] diagnostic for dataclass field order
1 parent fc72ff4 commit 3e9dd1b

File tree

3 files changed

+82
-21
lines changed

3 files changed

+82
-21
lines changed

crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ If attributes without default values are declared after attributes with default
116116
@dataclass
117117
class D:
118118
x: int = 1
119-
# TODO: this should be an error: field without default defined after field with default
119+
# error: [dataclass-field-order] "Required field `y` cannot be defined after fields with default values"
120120
y: str
121121
```
122122

crates/ty_python_semantic/src/types/diagnostic.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) {
3838
registry.register_lint(&DIVISION_BY_ZERO);
3939
registry.register_lint(&DUPLICATE_BASE);
4040
registry.register_lint(&DUPLICATE_KW_ONLY);
41+
registry.register_lint(&DATACLASS_FIELD_ORDER);
4142
registry.register_lint(&INSTANCE_LAYOUT_CONFLICT);
4243
registry.register_lint(&INCONSISTENT_MRO);
4344
registry.register_lint(&INDEX_OUT_OF_BOUNDS);
@@ -337,6 +338,32 @@ declare_lint! {
337338
}
338339
}
339340

341+
declare_lint! {
342+
/// ## What it does
343+
/// Checks for dataclass definitions where required fields are defined after
344+
/// fields with default values.
345+
///
346+
/// ## Why is this bad?
347+
/// In dataclasses, all required fields (fields without default values) must be
348+
/// defined before fields with default values. This is a Python requirement that
349+
/// will raise a `TypeError` at runtime if violated.
350+
///
351+
/// ## Example
352+
/// ```python
353+
/// from dataclasses import dataclass
354+
///
355+
/// @dataclass
356+
/// class Example:
357+
/// x: int = 1 # Field with default value
358+
/// y: str # Error: Required field after field with default
359+
/// ```
360+
pub(crate) static DATACLASS_FIELD_ORDER = {
361+
summary: "detects dataclass definitions with required fields after fields with default values",
362+
status: LintStatus::preview("1.0.0"),
363+
default_level: Level::Error,
364+
}
365+
}
366+
340367
declare_lint! {
341368
/// ## What it does
342369
/// Checks for classes definitions which will fail at runtime due to

crates/ty_python_semantic/src/types/infer.rs

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -93,18 +93,19 @@ use crate::types::call::{Binding, Bindings, CallArguments, CallError, CallErrorK
9393
use crate::types::class::{CodeGeneratorKind, Field, MetaclassErrorKind, SliceLiteral};
9494
use crate::types::diagnostic::{
9595
self, CALL_NON_CALLABLE, CONFLICTING_DECLARATIONS, CONFLICTING_METACLASS,
96-
CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, DUPLICATE_KW_ONLY, INCONSISTENT_MRO,
97-
INVALID_ARGUMENT_TYPE, INVALID_ASSIGNMENT, INVALID_ATTRIBUTE_ACCESS, INVALID_BASE,
98-
INVALID_DECLARATION, INVALID_GENERIC_CLASS, INVALID_KEY, INVALID_PARAMETER_DEFAULT,
99-
INVALID_TYPE_FORM, INVALID_TYPE_GUARD_CALL, INVALID_TYPE_VARIABLE_CONSTRAINTS,
100-
IncompatibleBases, POSSIBLY_UNBOUND_IMPLICIT_CALL, POSSIBLY_UNBOUND_IMPORT,
101-
TypeCheckDiagnostics, UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE, UNRESOLVED_GLOBAL,
102-
UNRESOLVED_IMPORT, UNRESOLVED_REFERENCE, UNSUPPORTED_OPERATOR, report_implicit_return_type,
103-
report_instance_layout_conflict, report_invalid_argument_number_to_special_form,
104-
report_invalid_arguments_to_annotated, report_invalid_arguments_to_callable,
105-
report_invalid_assignment, report_invalid_attribute_assignment,
106-
report_invalid_generator_function_return_type, report_invalid_key_on_typed_dict,
107-
report_invalid_return_type, report_possibly_unbound_attribute,
96+
CYCLIC_CLASS_DEFINITION, DATACLASS_FIELD_ORDER, DIVISION_BY_ZERO, DUPLICATE_KW_ONLY,
97+
INCONSISTENT_MRO, INVALID_ARGUMENT_TYPE, INVALID_ASSIGNMENT, INVALID_ATTRIBUTE_ACCESS,
98+
INVALID_BASE, INVALID_DECLARATION, INVALID_GENERIC_CLASS, INVALID_KEY,
99+
INVALID_PARAMETER_DEFAULT, INVALID_TYPE_FORM, INVALID_TYPE_GUARD_CALL,
100+
INVALID_TYPE_VARIABLE_CONSTRAINTS, IncompatibleBases, POSSIBLY_UNBOUND_IMPLICIT_CALL,
101+
POSSIBLY_UNBOUND_IMPORT, TypeCheckDiagnostics, UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE,
102+
UNRESOLVED_GLOBAL, UNRESOLVED_IMPORT, UNRESOLVED_REFERENCE, UNSUPPORTED_OPERATOR,
103+
report_implicit_return_type, report_instance_layout_conflict,
104+
report_invalid_argument_number_to_special_form, report_invalid_arguments_to_annotated,
105+
report_invalid_arguments_to_callable, report_invalid_assignment,
106+
report_invalid_attribute_assignment, report_invalid_generator_function_return_type,
107+
report_invalid_key_on_typed_dict, report_invalid_return_type,
108+
report_possibly_unbound_attribute,
108109
};
109110
use crate::types::enums::is_enum_class;
110111
use crate::types::function::{
@@ -1356,30 +1357,38 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
13561357
}
13571358
}
13581359

1359-
// (5) Check that a dataclass does not have more than one `KW_ONLY`.
1360+
// (5) Check that a dataclass does not have more than one `KW_ONLY`
1361+
// and that required fields are defined before default fields.
13601362
if let Some(field_policy @ CodeGeneratorKind::DataclassLike) =
13611363
CodeGeneratorKind::from_class(self.db(), class)
13621364
{
13631365
let specialization = None;
13641366
let mut kw_only_field_names = vec![];
1367+
let mut required_after_default_field_names = vec![];
1368+
let mut has_seen_default_field = false;
13651369

13661370
for (
13671371
name,
13681372
Field {
13691373
declared_ty: field_ty,
1374+
default_ty,
13701375
..
13711376
},
13721377
) in class.fields(self.db(), specialization, field_policy)
13731378
{
1374-
let Some(instance) = field_ty.into_nominal_instance() else {
1375-
continue;
1376-
};
1377-
1378-
if !instance.class.is_known(self.db(), KnownClass::KwOnly) {
1379-
continue;
1379+
// Check for KW_ONLY
1380+
if let Some(instance) = field_ty.into_nominal_instance() {
1381+
if instance.class.is_known(self.db(), KnownClass::KwOnly) {
1382+
kw_only_field_names.push(name.clone());
1383+
}
13801384
}
13811385

1382-
kw_only_field_names.push(name);
1386+
// Check field ordering
1387+
if default_ty.is_some() {
1388+
has_seen_default_field = true;
1389+
} else if has_seen_default_field {
1390+
required_after_default_field_names.push(name);
1391+
}
13831392
}
13841393

13851394
if kw_only_field_names.len() > 1 {
@@ -1401,6 +1410,31 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
14011410
));
14021411
}
14031412
}
1413+
1414+
// Report field ordering violations
1415+
for name in required_after_default_field_names {
1416+
// Find the AST node for this field
1417+
if let Some(field_node) = class_node.body.iter().find_map(|stmt| {
1418+
if let ast::Stmt::AnnAssign(ann_assign) = stmt {
1419+
if let ast::Expr::Name(name_expr) = ann_assign.target.as_ref() {
1420+
if name_expr.id == *name {
1421+
return Some(ann_assign.target.as_ref());
1422+
}
1423+
}
1424+
}
1425+
None
1426+
}) {
1427+
// Report the lint error
1428+
if let Some(builder) =
1429+
self.context.report_lint(&DATACLASS_FIELD_ORDER, field_node)
1430+
{
1431+
builder.into_diagnostic(format_args!(
1432+
"Required field `{}` cannot be defined after fields with default values",
1433+
name
1434+
));
1435+
}
1436+
}
1437+
}
14041438
}
14051439
}
14061440
}

0 commit comments

Comments
 (0)