mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-11-25 22:29:02 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[ty] Detect NamedTuple classes where fields without default values follow fields with default values (#19945)

Browse source
This commit is contained in:
Alex Waygood 2025-08-19 09:56:08 +01:00 • committed by GitHub
parent c20d906503
commit 4242905b36
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 358 additions and 70 deletions
Download patch file Download diff file Expand all files Collapse all files

22
crates/ty_python_semantic/resources/mdtest/named_tuple.md
View file

@ -102,15 +102,33 @@ reveal_type(alice2.name) # revealed: @Todo(functional `NamedTuple` syntax)
### Definition
TODO: Fields without default values should come before fields with.
<!-- snapshot-diagnostics -->
Fields without default values must come before fields with.
```py
from typing import NamedTuple
class Location(NamedTuple):
altitude: float = 0.0
latitude: float # this should be an error
# error: [invalid-named-tuple] "NamedTuple field without default value cannot follow field(s) with default value(s): Field `latitude` defined here without a default value"
latitude: float
# error: [invalid-named-tuple] "NamedTuple field without default value cannot follow field(s) with default value(s): Field `longitude` defined here without a default value"
longitude: float
class StrangeLocation(NamedTuple):
altitude: float
altitude: float = 0.0
altitude: float
altitude: float = 0.0
latitude: float # error: [invalid-named-tuple]
longitude: float # error: [invalid-named-tuple]
class VeryStrangeLocation(NamedTuple):
altitude: float = 0.0
latitude: float # error: [invalid-named-tuple]
longitude: float # error: [invalid-named-tuple]
altitude: float = 0.0
```
### Multiple Inheritance

140
crates/ty_python_semantic/resources/mdtest/snapshots/named_tuple.md_-_`NamedTuple`_-_`typing.NamedTuple`_-_Definition_(bbf79630502e65e9).snap Normal file
View file

@ -0,0 +1,140 @@
---
source: crates/ty_test/src/lib.rs
expression: snapshot
---
---
mdtest name: named_tuple.md - `NamedTuple` - `typing.NamedTuple` - Definition
mdtest path: crates/ty_python_semantic/resources/mdtest/named_tuple.md
---
# Python source files
## mdtest_snippet.py
```
1 | from typing import NamedTuple
2 |
3 | class Location(NamedTuple):
4 | altitude: float = 0.0
5 | # error: [invalid-named-tuple] "NamedTuple field without default value cannot follow field(s) with default value(s): Field `latitude` defined here without a default value"
6 | latitude: float
7 | # error: [invalid-named-tuple] "NamedTuple field without default value cannot follow field(s) with default value(s): Field `longitude` defined here without a default value"
8 | longitude: float
9 |
10 | class StrangeLocation(NamedTuple):
11 | altitude: float
12 | altitude: float = 0.0
13 | altitude: float
14 | altitude: float = 0.0
15 | latitude: float # error: [invalid-named-tuple]
16 | longitude: float # error: [invalid-named-tuple]
17 |
18 | class VeryStrangeLocation(NamedTuple):
19 | altitude: float = 0.0
20 | latitude: float # error: [invalid-named-tuple]
21 | longitude: float # error: [invalid-named-tuple]
22 | altitude: float = 0.0
```
# Diagnostics
```
error[invalid-named-tuple]: NamedTuple field without default value cannot follow field(s) with default value(s)
--> src/mdtest_snippet.py:4:5
|
3 | class Location(NamedTuple):
4 | altitude: float = 0.0
| --------------------- Earlier field `altitude` defined here with a default value
5 | # error: [invalid-named-tuple] "NamedTuple field without default value cannot follow field(s) with default value(s): Field `latitud…
6 | latitude: float
| ^^^^^^^^ Field `latitude` defined here without a default value
7 | # error: [invalid-named-tuple] "NamedTuple field without default value cannot follow field(s) with default value(s): Field `longitu…
8 | longitude: float
|
info: rule `invalid-named-tuple` is enabled by default
```
```
error[invalid-named-tuple]: NamedTuple field without default value cannot follow field(s) with default value(s)
--> src/mdtest_snippet.py:4:5
|
3 | class Location(NamedTuple):
4 | altitude: float = 0.0
| --------------------- Earlier field `altitude` defined here with a default value
5 | # error: [invalid-named-tuple] "NamedTuple field without default value cannot follow field(s) with default value(s): Field `latitu…
6 | latitude: float
7 | # error: [invalid-named-tuple] "NamedTuple field without default value cannot follow field(s) with default value(s): Field `longit…
8 | longitude: float
| ^^^^^^^^^ Field `longitude` defined here without a default value
9 |
10 | class StrangeLocation(NamedTuple):
|
info: rule `invalid-named-tuple` is enabled by default
```
```
error[invalid-named-tuple]: NamedTuple field without default value cannot follow field(s) with default value(s)
--> src/mdtest_snippet.py:14:5
|
12 | altitude: float = 0.0
13 | altitude: float
14 | altitude: float = 0.0
| --------------------- Earlier field `altitude` defined here with a default value
15 | latitude: float # error: [invalid-named-tuple]
| ^^^^^^^^ Field `latitude` defined here without a default value
16 | longitude: float # error: [invalid-named-tuple]
|
info: rule `invalid-named-tuple` is enabled by default
```
```
error[invalid-named-tuple]: NamedTuple field without default value cannot follow field(s) with default value(s)
--> src/mdtest_snippet.py:14:5
|
12 | altitude: float = 0.0
13 | altitude: float
14 | altitude: float = 0.0
| --------------------- Earlier field `altitude` defined here with a default value
15 | latitude: float # error: [invalid-named-tuple]
16 | longitude: float # error: [invalid-named-tuple]
| ^^^^^^^^^ Field `longitude` defined here without a default value
17 |
18 | class VeryStrangeLocation(NamedTuple):
|
info: rule `invalid-named-tuple` is enabled by default
```
```
error[invalid-named-tuple]: NamedTuple field without default value cannot follow field(s) with default value(s)
--> src/mdtest_snippet.py:20:5
|
18 | class VeryStrangeLocation(NamedTuple):
19 | altitude: float = 0.0
20 | latitude: float # error: [invalid-named-tuple]
| ^^^^^^^^ Field `latitude` defined here without a default value
21 | longitude: float # error: [invalid-named-tuple]
22 | altitude: float = 0.0
|
info: Earlier field `altitude` was defined with a default value
info: rule `invalid-named-tuple` is enabled by default
```
```
error[invalid-named-tuple]: NamedTuple field without default value cannot follow field(s) with default value(s)
--> src/mdtest_snippet.py:21:5
|
19 | altitude: float = 0.0
20 | latitude: float # error: [invalid-named-tuple]
21 | longitude: float # error: [invalid-named-tuple]
| ^^^^^^^^^ Field `longitude` defined here without a default value
22 | altitude: float = 0.0
|
info: Earlier field `altitude` was defined with a default value
info: rule `invalid-named-tuple` is enabled by default
```

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

@ -11,8 +11,11 @@ use super::{
use crate::FxOrderMap;
use crate::module_resolver::KnownModule;
use crate::semantic_index::definition::{Definition, DefinitionState};
use crate::semantic_index::place::ScopedPlaceId;
use crate::semantic_index::scope::NodeWithScopeKind;
use crate::semantic_index::{DeclarationWithConstraint, SemanticIndex, attribute_declarations};
use crate::semantic_index::{
BindingWithConstraints, DeclarationWithConstraint, SemanticIndex, attribute_declarations,
};
use crate::types::context::InferContext;
use crate::types::diagnostic::{INVALID_LEGACY_TYPE_VARIABLE, INVALID_TYPE_ALIAS_TYPE};
use crate::types::enums::enum_metadata;
@ -2982,6 +2985,54 @@ impl<'db> ClassLiteral<'db> {
.unwrap_or_else(|| class_name.end()),
)
}
pub(super) fn declarations_of_name(
self,
db: &'db dyn Db,
name: &str,
index: &'db SemanticIndex<'db>,
) -> Option<impl Iterator<Item = DeclarationWithConstraint<'db>>> {
let class_body_scope = self.body_scope(db).file_scope_id(db);
let symbol_id = index.place_table(class_body_scope).symbol_id(name)?;
let use_def = index.use_def_map(class_body_scope);
Some(use_def.end_of_scope_declarations(ScopedPlaceId::Symbol(symbol_id)))
}
pub(super) fn first_declaration_of_name(
self,
db: &'db dyn Db,
name: &str,
index: &'db SemanticIndex<'db>,
) -> Option<DeclarationWithConstraint<'db>> {
self.declarations_of_name(db, name, index)
.into_iter()
.flatten()
.next()
}
pub(super) fn bindings_of_name(
self,
db: &'db dyn Db,
name: &str,
index: &'db SemanticIndex<'db>,
) -> Option<impl Iterator<Item = BindingWithConstraints<'db, 'db>>> {
let class_body_scope = self.body_scope(db).file_scope_id(db);
let symbol_id = index.place_table(class_body_scope).symbol_id(name)?;
let use_def = index.use_def_map(class_body_scope);
Some(use_def.end_of_scope_bindings(ScopedPlaceId::Symbol(symbol_id)))
}
pub(super) fn first_binding_of_name(
self,
db: &'db dyn Db,
name: &str,
index: &'db SemanticIndex<'db>,
) -> Option<BindingWithConstraints<'db, 'db>> {
self.bindings_of_name(db, name, index)
.into_iter()
.flatten()
.next()
}
}
impl<'db> From<ClassLiteral<'db>> for Type<'db> {

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

@ -6,6 +6,7 @@ use super::{
add_inferred_python_version_hint_to_diagnostic,
};
use crate::lint::{Level, LintRegistryBuilder, LintStatus};
use crate::semantic_index::SemanticIndex;
use crate::suppression::FileSuppressionId;
use crate::types::LintDiagnosticGuard;
use crate::types::class::{Field, SolidBase, SolidBaseKind};
@ -2676,6 +2677,60 @@ pub(crate) fn report_invalid_key_on_typed_dict<'db>(
}
}
pub(super) fn report_namedtuple_field_without_default_after_field_with_default<'db>(
context: &InferContext<'db, '_>,
class: ClassLiteral<'db>,
index: &'db SemanticIndex<'db>,
field_name: &str,
field_with_default: &str,
) {
let db = context.db();
let module = context.module();
let diagnostic_range = class
.first_declaration_of_name(db, field_name, index)
.and_then(|definition| definition.declaration.definition())
.map(|definition| definition.kind(db).full_range(module))
.unwrap_or_else(|| class.header_range(db));
let Some(builder) = context.report_lint(&INVALID_NAMED_TUPLE, diagnostic_range) else {
return;
};
let mut diagnostic = builder.into_diagnostic(format_args!(
"NamedTuple field without default value cannot follow field(s) with default value(s)",
));
diagnostic.set_primary_message(format_args!(
"Field `{field_name}` defined here without a default value"
));
let Some(field_with_default_range) = class
.first_binding_of_name(db, field_with_default, index)
.and_then(|definition| definition.binding.definition())
.map(|definition| definition.kind(db).full_range(module))
else {
return;
};
// If the end-of-scope definition in the class scope of the field-with-a-default-value
// occurs after the range of the field-without-a-default-value,
// avoid adding a subdiagnostic that points to the definition of the
// field-with-a-default-value. It's confusing to talk about a field "before" the
// field without the default value but then point to a definition that actually
// occurs after the field without-a-default-value.
if field_with_default_range.end() < diagnostic_range.start() {
diagnostic.annotate(
Annotation::secondary(context.span(field_with_default_range)).message(format_args!(
"Earlier field `{field_with_default}` defined here with a default value",
)),
);
} else {
diagnostic.info(format_args!(
"Earlier field `{field_with_default}` was defined with a default value"
));
}
}
/// This function receives an unresolved `from foo import bar` import,
/// where `foo` can be resolved to a module but that module does not
/// have a `bar` member or submodule.

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

@ -105,6 +105,7 @@ use crate::types::diagnostic::{
report_invalid_arguments_to_callable, report_invalid_assignment,
report_invalid_attribute_assignment, report_invalid_generator_function_return_type,
report_invalid_key_on_typed_dict, report_invalid_return_type,
report_namedtuple_field_without_default_after_field_with_default,
report_possibly_unbound_attribute,
};
use crate::types::enums::is_enum_class;
@ -1110,11 +1111,34 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
continue;
}
let is_protocol = class.is_protocol(self.db());
let is_named_tuple = CodeGeneratorKind::NamedTuple.matches(self.db(), class);
// (2) If it's a `NamedTuple` class, check that no field without a default value
// appears after a field with a default value.
if is_named_tuple {
let mut field_with_default_encountered = None;
for (field_name, field) in class.own_fields(self.db(), None) {
if field.default_ty.is_some() {
field_with_default_encountered = Some(field_name);
} else if let Some(field_with_default) = field_with_default_encountered.as_ref()
{
report_namedtuple_field_without_default_after_field_with_default(
&self.context,
class,
self.index,
&field_name,
field_with_default,
);
}
}
}
let is_protocol = class.is_protocol(self.db());
let mut solid_bases = IncompatibleBases::default();
// (2) Iterate through the class's explicit bases to check for various possible errors:
// (3) Iterate through the class's explicit bases to check for various possible errors:
// - Check for inheritance from plain `Generic`,
// - Check for inheritance from a `@final` classes
// - If the class is a protocol class: check for inheritance from a non-protocol class
@ -1208,7 +1232,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
}
}
// (3) Check that the class's MRO is resolvable
// (4) Check that the class's MRO is resolvable
match class.try_mro(self.db(), None) {
Err(mro_error) => match mro_error.reason() {
MroErrorKind::DuplicateBases(duplicates) => {
@ -1279,7 +1303,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
}
}
// (4) Check that the class's metaclass can be determined without error.
// (5) Check that the class's metaclass can be determined without error.
if let Err(metaclass_error) = class.try_metaclass(self.db()) {
match metaclass_error.reason() {
MetaclassErrorKind::Cycle => {
@ -1376,7 +1400,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
}
}
// (5) Check that a dataclass does not have more than one `KW_ONLY`.
// (6) 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)
{