Skip to content

Commit 49a4939

Browse files
committed
[ty] address pr feedback
1 parent 299af74 commit 49a4939

4 files changed

Lines changed: 168 additions & 58 deletions

File tree

crates/ty_python_semantic/src/types/class/enum_literal.rs

Lines changed: 18 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@ use crate::types::class::known::KnownClass;
1313
use crate::types::class::{ClassLiteral, ClassType, MemberLookupPolicy};
1414
use crate::types::class_base::ClassBase;
1515
use crate::types::member::Member;
16-
use crate::types::mro::Mro;
17-
use crate::types::todo_type;
16+
use crate::types::mro::{DynamicMroError, Mro};
1817

1918
#[salsa::interned(debug, heap_size=ruff_memory_usage::heap_size)]
2019
pub struct EnumSpec<'db> {
@@ -110,31 +109,16 @@ impl<'db> DynamicEnumLiteral<'db> {
110109
#[salsa::tracked(
111110
returns(ref),
112111
heap_size=ruff_memory_usage::heap_size,
113-
cycle_initial=dynamic_enum_mro_cycle_initial
112+
cycle_initial=dynamic_enum_try_mro_cycle_initial
114113
)]
115-
pub(crate) fn mro(self, db: &'db dyn Db) -> Mro<'db> {
116-
let self_base = ClassBase::Class(ClassType::NonGeneric(self.into()));
117-
let mixin_mro: Vec<ClassBase<'db>> = self
118-
.mixin_type(db)
119-
.and_then(|t| ClassBase::try_from_type(db, t, None))
120-
.map(|base| base.mro(db, None).collect())
121-
.unwrap_or_default();
122-
let base = self
123-
.base_class(db)
124-
.to_class_literal(db)
125-
.as_class_literal()
126-
.expect("enum base should be a class literal")
127-
.default_specialization(db);
128-
std::iter::once(self_base)
129-
.chain(mixin_mro)
130-
.chain(base.iter_mro(db))
131-
.collect()
114+
pub(crate) fn try_mro(self, db: &'db dyn Db) -> Result<Mro<'db>, DynamicMroError<'db>> {
115+
Mro::of_dynamic_enum(db, self)
132116
}
133117

134118
pub(super) fn own_class_member(self, db: &'db dyn Db, name: &str) -> Member<'db> {
135119
let spec = self.spec(db);
136120
if !spec.has_known_members(db) {
137-
return Member::definitely_declared(todo_type!("functional `Enum` syntax"));
121+
return Member::definitely_declared(Type::unknown());
138122
}
139123
if spec.members(db).iter().any(|(n, _)| n.as_str() == name) {
140124
let class_lit = ClassLiteral::DynamicEnum(self);
@@ -146,10 +130,6 @@ impl<'db> DynamicEnumLiteral<'db> {
146130
}
147131

148132
pub(crate) fn class_member(self, db: &'db dyn Db, name: &str) -> PlaceAndQualifiers<'db> {
149-
let spec = self.spec(db);
150-
if !spec.has_known_members(db) {
151-
return Place::bound(todo_type!("functional `Enum` syntax")).into();
152-
}
153133
let own = self.own_class_member(db, name);
154134
if !own.is_undefined() {
155135
return own.inner;
@@ -170,9 +150,6 @@ impl<'db> DynamicEnumLiteral<'db> {
170150
}
171151

172152
pub(crate) fn instance_member(self, db: &'db dyn Db, name: &str) -> PlaceAndQualifiers<'db> {
173-
if !self.spec(db).has_known_members(db) {
174-
return Place::bound(todo_type!("functional `Enum` syntax")).into();
175-
}
176153
if let Some(mixin) = self.mixin_type(db) {
177154
if let Some(ClassBase::Class(class)) = ClassBase::try_from_type(db, mixin, None) {
178155
let result = class.instance_member(db, name);
@@ -186,19 +163,26 @@ impl<'db> DynamicEnumLiteral<'db> {
186163
.instance_member(db, name)
187164
}
188165

189-
pub(super) fn own_instance_member(self, db: &'db dyn Db, name: &str) -> Member<'db> {
166+
pub(super) fn own_instance_member(
167+
self,
168+
db: &'db dyn Db,
169+
#[expect(unused)] name: &str,
170+
) -> Member<'db> {
190171
if !self.spec(db).has_known_members(db) {
191-
return Member::definitely_declared(todo_type!("functional `Enum` syntax"));
172+
return Member::definitely_declared(Type::unknown());
192173
}
193-
let _ = name;
194174
Member::unbound()
195175
}
196176
}
197177

198-
fn dynamic_enum_mro_cycle_initial<'db>(
178+
#[expect(clippy::unnecessary_wraps)]
179+
fn dynamic_enum_try_mro_cycle_initial<'db>(
199180
db: &'db dyn Db,
200181
_id: salsa::Id,
201182
self_: DynamicEnumLiteral<'db>,
202-
) -> Mro<'db> {
203-
Mro::from_error(db, ClassType::NonGeneric(ClassLiteral::DynamicEnum(self_)))
183+
) -> Result<Mro<'db>, DynamicMroError<'db>> {
184+
Ok(Mro::from([
185+
ClassBase::Class(ClassType::NonGeneric(self_.into())),
186+
ClassBase::object(db),
187+
]))
204188
}

crates/ty_python_semantic/src/types/infer/builder/dynamic_class.rs

Lines changed: 56 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::types::diagnostic::{
1010
};
1111
use crate::types::enums::is_enum_class_by_inheritance;
1212
use crate::types::infer::builder::TypeInferenceBuilder;
13-
use crate::types::mro::DynamicMroErrorKind;
13+
use crate::types::mro::{DynamicMroError, DynamicMroErrorKind};
1414
use crate::types::{ClassBase, KnownClass, Type, extract_fixed_length_iterable_element_types};
1515

1616
/// Whether a dynamic class is being created via `type()` or `types.new_class()`.
@@ -219,10 +219,50 @@ pub(super) fn report_dynamic_mro_errors<'db>(
219219
return true;
220220
};
221221

222-
let bases_tuple_elts = bases.as_tuple_expr().map(|tuple| tuple.elts.as_slice());
222+
let bases_display = dynamic_class
223+
.explicit_bases(db)
224+
.iter()
225+
.map(|base| base.display(db))
226+
.join(", ");
227+
report_mro_error_kind(
228+
context,
229+
error,
230+
dynamic_class.name(db),
231+
call_expr,
232+
Some(bases),
233+
Some(&bases_display),
234+
);
223235

236+
false
237+
}
238+
239+
/// Report diagnostics for a dynamic MRO error. Shared by both
240+
/// `report_dynamic_mro_errors` (for `type()` / `new_class()`) and the
241+
/// functional enum path.
242+
///
243+
/// `bases_expr` is the AST node for the bases argument (e.g. the tuple in
244+
/// `type("Foo", (A, B), {})`). When `Some`, `InvalidBases` diagnostics point
245+
/// at specific elements in the tuple. When `None` (enums), `InvalidBases`
246+
/// is skipped since enum bases are always valid.
247+
///
248+
/// `bases_display` is an optional pre-formatted string of the bases list
249+
/// (e.g. `"<class 'X'>, <class 'Y'>"`). When provided, the `UnresolvableMro`
250+
/// message includes `with bases [...]`.
251+
pub(super) fn report_mro_error_kind<'db>(
252+
context: &InferContext<'db, '_>,
253+
error: &DynamicMroError<'db>,
254+
class_name: &Name,
255+
call_expr: &ast::ExprCall,
256+
bases_expr: Option<&ast::Expr>,
257+
bases_display: Option<&str>,
258+
) {
259+
let db = context.db();
224260
match error.reason() {
225261
DynamicMroErrorKind::InvalidBases(invalid_bases) => {
262+
let Some(bases) = bases_expr else {
263+
return;
264+
};
265+
let bases_tuple_elts = bases.as_tuple_expr().map(|tuple| tuple.elts.as_slice());
226266
for (idx, base_type) in invalid_bases {
227267
let instance_of_type = KnownClass::Type.to_instance(db);
228268
let specific_base = bases_tuple_elts.and_then(|elts| elts.get(*idx));
@@ -240,8 +280,7 @@ pub(super) fn report_dynamic_mro_errors<'db>(
240280
base_type.display(db)
241281
));
242282
diagnostic.info(format_args!(
243-
"ty cannot determine a MRO for class `{}` due to this base",
244-
dynamic_class.name(db)
283+
"ty cannot determine a MRO for class `{class_name}` due to this base",
245284
));
246285
diagnostic.info("Only class objects or `Any` are supported as class bases");
247286
}
@@ -259,40 +298,35 @@ pub(super) fn report_dynamic_mro_errors<'db>(
259298
}
260299
DynamicMroErrorKind::InheritanceCycle => {
261300
if let Some(builder) = context.report_lint(&CYCLIC_CLASS_DEFINITION, call_expr) {
262-
builder.into_diagnostic(format_args!(
263-
"Cyclic definition of `{}`",
264-
dynamic_class.name(db)
265-
));
301+
builder.into_diagnostic(format_args!("Cyclic definition of `{class_name}`"));
266302
}
267303
}
268304
DynamicMroErrorKind::DuplicateBases(duplicates) => {
269305
if let Some(builder) = context.report_lint(&DUPLICATE_BASE, call_expr) {
270306
builder.into_diagnostic(format_args!(
271-
"Duplicate base class{maybe_s} {dupes} in class `{class}`",
307+
"Duplicate base class{maybe_s} {dupes} in class `{class_name}`",
272308
maybe_s = if duplicates.len() == 1 { "" } else { "es" },
273309
dupes = duplicates
274310
.iter()
275311
.map(|base: &ClassBase<'_>| base.display(db))
276312
.join(", "),
277-
class = dynamic_class.name(db),
278313
));
279314
}
280315
}
281316
DynamicMroErrorKind::UnresolvableMro => {
282317
if let Some(builder) = context.report_lint(&INCONSISTENT_MRO, call_expr) {
283-
builder.into_diagnostic(format_args!(
284-
"Cannot create a consistent method resolution order (MRO) \
285-
for class `{}` with bases `[{}]`",
286-
dynamic_class.name(db),
287-
dynamic_class
288-
.explicit_bases(db)
289-
.iter()
290-
.map(|base| base.display(db))
291-
.join(", ")
292-
));
318+
if let Some(bases) = bases_display {
319+
builder.into_diagnostic(format_args!(
320+
"Cannot create a consistent method resolution order (MRO) \
321+
for class `{class_name}` with bases `[{bases}]`",
322+
));
323+
} else {
324+
builder.into_diagnostic(format_args!(
325+
"Cannot create a consistent method resolution order (MRO) \
326+
for class `{class_name}`",
327+
));
328+
}
293329
}
294330
}
295331
}
296-
297-
false
298332
}

crates/ty_python_semantic/src/types/infer/builder/enum_call.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use crate::{
99
class::{DynamicEnumAnchor, DynamicEnumLiteral, EnumSpec},
1010
diagnostic::{INVALID_ARGUMENT_TYPE, TOO_MANY_POSITIONAL_ARGUMENTS, UNKNOWN_ARGUMENT},
1111
infer::TypeInferenceBuilder,
12+
infer::builder::dynamic_class::report_mro_error_kind,
1213
subclass_of::SubclassOfType,
1314
},
1415
};
@@ -251,6 +252,16 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
251252
};
252253

253254
let enum_lit = DynamicEnumLiteral::new(db, name, anchor, base_class, mixin_type);
255+
if let Err(error) = enum_lit.try_mro(db) {
256+
report_mro_error_kind(
257+
&self.context,
258+
error,
259+
enum_lit.name(db),
260+
call_expr,
261+
None,
262+
None,
263+
);
264+
}
254265
Some(Type::ClassLiteral(ClassLiteral::DynamicEnum(enum_lit)))
255266
}
256267

crates/ty_python_semantic/src/types/mro.rs

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use indexmap::IndexMap;
55
use rustc_hash::{FxBuildHasher, FxHashSet};
66

77
use crate::Db;
8-
use crate::types::class::DynamicClassLiteral;
8+
use crate::types::class::{DynamicClassLiteral, DynamicEnumLiteral};
99
use crate::types::class_base::ClassBase;
1010
use crate::types::generics::Specialization;
1111
use crate::types::{
@@ -421,6 +421,84 @@ impl<'db> Mro<'db> {
421421
}
422422
}
423423

424+
/// Compute the MRO of a dynamic enum (created via the functional `Enum()`/`StrEnum()` API).
425+
///
426+
/// Uses C3 linearization to correctly handle the optional `type=` mixin parameter.
427+
/// For example, `Enum("Http", {"OK": 200}, type=int)` is equivalent to
428+
/// `class Http(int, Enum)` at runtime, so the MRO must be C3-linearized from
429+
/// both bases to produce `[Http, int, Enum, object]`
430+
pub(super) fn of_dynamic_enum(
431+
db: &'db dyn Db,
432+
dynamic_enum: DynamicEnumLiteral<'db>,
433+
) -> Result<Self, DynamicMroError<'db>> {
434+
let self_base = ClassBase::Class(ClassType::NonGeneric(dynamic_enum.into()));
435+
436+
// Build the bases list: [mixin, base_class] when a `type=` mixin is present,
437+
// or just [base_class] otherwise. For example:
438+
// Enum("Color", ...) -> resolved_bases = [Enum]
439+
// Enum("Http", ..., type=int) -> resolved_bases = [int, Enum]
440+
// StrEnum("Method", ...) -> resolved_bases = [StrEnum]
441+
let mut resolved_bases: Vec<ClassBase<'db>> = Vec::with_capacity(2);
442+
if let Some(mixin) = dynamic_enum.mixin_type(db) {
443+
if let Some(base) = ClassBase::try_from_type(db, mixin, None) {
444+
resolved_bases.push(base);
445+
}
446+
}
447+
let enum_base = dynamic_enum
448+
.base_class(db)
449+
.to_class_literal(db)
450+
.as_class_literal()
451+
.expect("enum base should be a class literal")
452+
.default_specialization(db);
453+
resolved_bases.push(ClassBase::Class(enum_base));
454+
455+
// When C3 linearization fails (e.g. a bad `type=` mixin), we still need a
456+
// usable MRO for downstream type inference. Rather than falling back to the
457+
// generic `[self, Unknown, object]`, we chain the bases' MROs with
458+
// deduplication. This preserves type information from the known bases so that
459+
// member lookups can still find attributes from the mixin and enum base class.
460+
//
461+
// For example, if `Enum("Foo", ..., type=BadMixin)` fails C3, the fallback
462+
// produces `[Foo, BadMixin, ..., Enum, object]` (deduped), so lookups for
463+
// `Foo.some_method` can still resolve methods from `BadMixin` or `Enum`.
464+
// With `[Foo, Unknown, object]`, those lookups would silently return Unknown.
465+
//
466+
// This matches the `dynamic_fallback` approach used by `of_dynamic_class`.
467+
let fallback_mro = || {
468+
let mut result = vec![self_base];
469+
let mut seen = FxHashSet::default();
470+
seen.insert(self_base);
471+
for base in &resolved_bases {
472+
for item in base.mro(db, None) {
473+
if seen.insert(item) {
474+
result.push(item);
475+
}
476+
}
477+
}
478+
Self::from(result)
479+
};
480+
481+
// Standard C3 linearization: build sequences from each base's MRO, plus the
482+
// bases list itself, then merge. See `of_static_class` and `of_dynamic_class`
483+
// for the same pattern.
484+
let mut seqs = vec![VecDeque::from([self_base])];
485+
for base in &resolved_bases {
486+
if base.has_cyclic_mro(db) {
487+
return Err(DynamicMroError {
488+
kind: DynamicMroErrorKind::InheritanceCycle,
489+
fallback_mro: fallback_mro(),
490+
});
491+
}
492+
seqs.push(base.mro(db, None).collect());
493+
}
494+
seqs.push(resolved_bases.iter().copied().collect());
495+
496+
c3_merge(seqs).ok_or_else(|| DynamicMroError {
497+
kind: DynamicMroErrorKind::UnresolvableMro,
498+
fallback_mro: fallback_mro(),
499+
})
500+
}
501+
424502
/// Compute a fallback MRO for a dynamic class when `of_dynamic_class` fails.
425503
///
426504
/// Iterates over base MROs sequentially with deduplication.
@@ -577,7 +655,10 @@ impl<'db> MroIterator<'db> {
577655
full_mro_iter
578656
}
579657
ClassLiteral::DynamicEnum(literal) => {
580-
let mut full_mro_iter = literal.mro(self.db).iter();
658+
let mut full_mro_iter = match literal.try_mro(self.db) {
659+
Ok(mro) => mro.iter(),
660+
Err(error) => error.fallback_mro().iter(),
661+
};
581662
full_mro_iter.next();
582663
full_mro_iter
583664
}

0 commit comments

Comments
 (0)