From 4dfad38c753e28ff1fca4c961097abcb9f19d4e1 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 13 Dec 2025 20:49:40 +0000 Subject: [PATCH] fancier diagnostics --- .../mdtest/dataclasses/dataclasses.md | 51 +++++- ...rozen__non-frozen_in…_(9af2ab07b8e829e).snap | 154 ++++++++++++++++++ crates/ty_python_semantic/src/types.rs | 6 + crates/ty_python_semantic/src/types/class.rs | 22 +++ .../src/types/diagnostic.rs | 90 +++++++++- .../src/types/infer/builder.rs | 88 ++++------ 6 files changed, 348 insertions(+), 63 deletions(-) create mode 100644 crates/ty_python_semantic/resources/mdtest/snapshots/dataclasses.md_-_Dataclasses_-_Other_dataclass_para…_-_frozen__non-frozen_in…_(9af2ab07b8e829e).snap diff --git a/crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md b/crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md index 54942f4a2e..60bfd36176 100644 --- a/crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md +++ b/crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md @@ -521,7 +521,14 @@ frozen = MyFrozenChildClass() del frozen.x # TODO this should emit an [invalid-assignment] ``` -A diagnostic is emitted if a non-frozen dataclass inherits from a frozen dataclass: +### frozen/non-frozen inheritance + +If a non-frozen dataclass inherits from a frozen dataclass, an exception is raised at runtime. We +catch this error: + + + +`a.py`: ```py from dataclasses import dataclass @@ -530,13 +537,15 @@ from dataclasses import dataclass class FrozenBase: x: int -@dataclass -class Child(FrozenBase): # error: [invalid-frozen-dataclass-subclass] "A non-frozen class `Child` cannot inherit from a class `FrozenBase` that is frozen" - +@dataclass +# error: [invalid-frozen-dataclass-subclass] "Non-frozen dataclass `Child` cannot inherit from frozen dataclass `FrozenBase`" +class Child(FrozenBase): y: int ``` -A diagnostic is emitted if a frozen dataclass inherits from a non-frozen dataclass: +Frozen dataclasses inheriting from non-frozen dataclasses are also illegal: + +`b.py`: ```py from dataclasses import dataclass @@ -546,11 +555,39 @@ class Base: x: int @dataclass(frozen=True) -class FrozenChild(Base): # error: [invalid-frozen-dataclass-subclass] "A frozen class `FrozenChild` cannot inherit from a class `Base` that is not frozen" - +# error: [invalid-frozen-dataclass-subclass] "Frozen dataclass `FrozenChild` cannot inherit from non-frozen dataclass `Base`" +class FrozenChild(Base): y: int ``` +Example of diagnostics when there are multiple files involved: + +`module.py`: + +```py +import dataclasses + +@dataclasses.dataclass(frozen=False) +class NotFrozenBase: + x: int +``` + +`main.py`: + +```py +from functools import total_ordering +from typing import final +from dataclasses import dataclass + +from module import NotFrozenBase + +@final +@dataclass(frozen=True) +@total_ordering +class FrozenChild(NotFrozenBase): # error: [invalid-frozen-dataclass-subclass] + y: str +``` + ### `match_args` If `match_args` is set to `True` (the default), the `__match_args__` attribute is a tuple created diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/dataclasses.md_-_Dataclasses_-_Other_dataclass_para…_-_frozen__non-frozen_in…_(9af2ab07b8e829e).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/dataclasses.md_-_Dataclasses_-_Other_dataclass_para…_-_frozen__non-frozen_in…_(9af2ab07b8e829e).snap new file mode 100644 index 0000000000..3b3a006b6e --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/dataclasses.md_-_Dataclasses_-_Other_dataclass_para…_-_frozen__non-frozen_in…_(9af2ab07b8e829e).snap @@ -0,0 +1,154 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: dataclasses.md - Dataclasses - Other dataclass parameters - frozen/non-frozen inheritance +mdtest path: crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md +--- + +# Python source files + +## a.py + +``` + 1 | from dataclasses import dataclass + 2 | + 3 | @dataclass(frozen=True) + 4 | class FrozenBase: + 5 | x: int + 6 | + 7 | @dataclass + 8 | # error: [invalid-frozen-dataclass-subclass] "Non-frozen dataclass `Child` cannot inherit from frozen dataclass `FrozenBase`" + 9 | class Child(FrozenBase): +10 | y: int +``` + +## b.py + +``` + 1 | from dataclasses import dataclass + 2 | + 3 | @dataclass + 4 | class Base: + 5 | x: int + 6 | + 7 | @dataclass(frozen=True) + 8 | # error: [invalid-frozen-dataclass-subclass] "Frozen dataclass `FrozenChild` cannot inherit from non-frozen dataclass `Base`" + 9 | class FrozenChild(Base): +10 | y: int +``` + +## module.py + +``` +1 | import dataclasses +2 | +3 | @dataclasses.dataclass(frozen=False) +4 | class NotFrozenBase: +5 | x: int +``` + +## main.py + +``` + 1 | from functools import total_ordering + 2 | from typing import final + 3 | from dataclasses import dataclass + 4 | + 5 | from module import NotFrozenBase + 6 | + 7 | @final + 8 | @dataclass(frozen=True) + 9 | @total_ordering +10 | class FrozenChild(NotFrozenBase): # error: [invalid-frozen-dataclass-subclass] +11 | y: str +``` + +# Diagnostics + +``` +error[invalid-frozen-dataclass-subclass]: Non-frozen dataclass cannot inherit from frozen dataclass + --> src/a.py:7:1 + | + 5 | x: int + 6 | + 7 | @dataclass + | ---------- `Child` dataclass parameters + 8 | # error: [invalid-frozen-dataclass-subclass] "Non-frozen dataclass `Child` cannot inherit from frozen dataclass `FrozenBase`" + 9 | class Child(FrozenBase): + | ^^^^^^----------^ Subclass `Child` is not frozen but base class `FrozenBase` is +10 | y: int + | +info: This causes the class creation to fail +info: Base class definition + --> src/a.py:3:1 + | +1 | from dataclasses import dataclass +2 | +3 | @dataclass(frozen=True) + | ----------------------- `FrozenBase` dataclass parameters +4 | class FrozenBase: + | ^^^^^^^^^^ `FrozenBase` definition +5 | x: int + | +info: rule `invalid-frozen-dataclass-subclass` is enabled by default + +``` + +``` +error[invalid-frozen-dataclass-subclass]: Frozen dataclass cannot inherit from non-frozen dataclass + --> src/b.py:7:1 + | + 5 | x: int + 6 | + 7 | @dataclass(frozen=True) + | ----------------------- `FrozenChild` dataclass parameters + 8 | # error: [invalid-frozen-dataclass-subclass] "Frozen dataclass `FrozenChild` cannot inherit from non-frozen dataclass `Base`" + 9 | class FrozenChild(Base): + | ^^^^^^^^^^^^----^ Subclass `FrozenChild` is frozen but base class `Base` is not +10 | y: int + | +info: This causes the class creation to fail +info: Base class definition + --> src/b.py:3:1 + | +1 | from dataclasses import dataclass +2 | +3 | @dataclass + | ---------- `Base` dataclass parameters +4 | class Base: + | ^^^^ `Base` definition +5 | x: int + | +info: rule `invalid-frozen-dataclass-subclass` is enabled by default + +``` + +``` +error[invalid-frozen-dataclass-subclass]: Frozen dataclass cannot inherit from non-frozen dataclass + --> src/main.py:8:1 + | + 7 | @final + 8 | @dataclass(frozen=True) + | ----------------------- `FrozenChild` dataclass parameters + 9 | @total_ordering +10 | class FrozenChild(NotFrozenBase): # error: [invalid-frozen-dataclass-subclass] + | ^^^^^^^^^^^^-------------^ Subclass `FrozenChild` is frozen but base class `NotFrozenBase` is not +11 | y: str + | +info: This causes the class creation to fail +info: Base class definition + --> src/module.py:3:1 + | +1 | import dataclasses +2 | +3 | @dataclasses.dataclass(frozen=False) + | ------------------------------------ `NotFrozenBase` dataclass parameters +4 | class NotFrozenBase: + | ^^^^^^^^^^^^^ `NotFrozenBase` definition +5 | x: int + | +info: rule `invalid-frozen-dataclass-subclass` is enabled by default + +``` diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index e9311547d7..726decfc07 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -677,6 +677,12 @@ bitflags! { } } +impl DataclassFlags { + pub(crate) const fn is_frozen(self) -> bool { + self.contains(Self::FROZEN) + } +} + pub(crate) const DATACLASS_FLAGS: &[(&str, DataclassFlags)] = &[ ("init", DataclassFlags::INIT), ("repr", DataclassFlags::REPR), diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 32962ea128..022e63fe33 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -1856,6 +1856,28 @@ impl<'db> ClassLiteral<'db> { .filter_map(|decorator| decorator.known(db)) } + /// Iterate through the decorators on this class, returning the position of the first one + /// that matches the given predicate. + pub(super) fn find_decorator_position( + self, + db: &'db dyn Db, + predicate: impl Fn(Type<'db>) -> bool, + ) -> Option { + self.decorators(db) + .iter() + .position(|decorator| predicate(*decorator)) + } + + /// Iterate through the decorators on this class, returning the index of the first one + /// that is either `@dataclass` or `@dataclass(...)`. + pub(super) fn find_dataclass_decorator_position(self, db: &'db dyn Db) -> Option { + self.find_decorator_position(db, |ty| match ty { + Type::FunctionLiteral(function) => function.is_known(db, KnownFunction::Dataclass), + Type::DataclassDecorator(_) => true, + _ => false, + }) + } + /// Is this class final? pub(super) fn is_final(self, db: &'db dyn Db) -> bool { self.known_function_decorators(db) diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index abd39ee2b4..3acd7b0a64 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -30,7 +30,7 @@ use crate::types::{ ProtocolInstanceType, SpecialFormType, SubclassOfInner, Type, TypeContext, binding_type, protocol_class::ProtocolClass, }; -use crate::types::{KnownInstanceType, MemberLookupPolicy}; +use crate::types::{DataclassFlags, KnownInstanceType, MemberLookupPolicy}; use crate::{Db, DisplaySettings, FxIndexMap, Module, ModuleName, Program, declare_lint}; use itertools::Itertools; use ruff_db::{ @@ -4308,6 +4308,94 @@ fn report_unsupported_binary_operation_impl<'a>( Some(diagnostic) } +pub(super) fn report_bad_frozen_dataclass_inheritance<'db>( + context: &InferContext<'db, '_>, + class: ClassLiteral<'db>, + class_node: &ast::StmtClassDef, + base_class: ClassLiteral<'db>, + base_class_node: &ast::Expr, + base_class_params: DataclassFlags, +) { + let db = context.db(); + + let Some(builder) = + context.report_lint(&INVALID_FROZEN_DATACLASS_SUBCLASS, class.header_range(db)) + else { + return; + }; + + let mut diagnostic = if base_class_params.is_frozen() { + let mut diagnostic = + builder.into_diagnostic("Non-frozen dataclass cannot inherit from frozen dataclass"); + diagnostic.set_concise_message(format_args!( + "Non-frozen dataclass `{}` cannot inherit from frozen dataclass `{}`", + class.name(db), + base_class.name(db) + )); + diagnostic.set_primary_message(format_args!( + "Subclass `{}` is not frozen but base class `{}` is", + class.name(db), + base_class.name(db) + )); + diagnostic + } else { + let mut diagnostic = + builder.into_diagnostic("Frozen dataclass cannot inherit from non-frozen dataclass"); + diagnostic.set_concise_message(format_args!( + "Frozen dataclass `{}` cannot inherit from non-frozen dataclass `{}`", + class.name(db), + base_class.name(db) + )); + diagnostic.set_primary_message(format_args!( + "Subclass `{}` is frozen but base class `{}` is not", + class.name(db), + base_class.name(db) + )); + diagnostic + }; + + diagnostic.annotate(context.secondary(base_class_node)); + + if let Some(position) = class.find_dataclass_decorator_position(db) { + diagnostic.annotate( + context + .secondary(&class_node.decorator_list[position]) + .message(format_args!("`{}` dataclass parameters", class.name(db))), + ); + } + diagnostic.info("This causes the class creation to fail"); + + if let Some(decorator_position) = base_class.find_dataclass_decorator_position(db) { + let mut sub = SubDiagnostic::new( + SubDiagnosticSeverity::Info, + format_args!("Base class definition"), + ); + sub.annotate( + Annotation::primary(base_class.header_span(db)) + .message(format_args!("`{}` definition", base_class.name(db))), + ); + + let base_class_file = base_class.file(db); + let module = parsed_module(db, base_class_file).load(db); + + let decorator_range = base_class + .body_scope(db) + .node(db) + .expect_class() + .node(&module) + .decorator_list[decorator_position] + .range(); + + sub.annotate( + Annotation::secondary(Span::from(base_class_file).with_range(decorator_range)).message( + format_args!("`{}` dataclass parameters", base_class.name(db)), + ), + ); + + diagnostic.sub(sub); + } +} + /// 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. diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index b6ed9aa2c6..43940a4d88 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -59,25 +59,26 @@ use crate::types::diagnostic::{ self, CALL_NON_CALLABLE, CONFLICTING_DECLARATIONS, CONFLICTING_METACLASS, CYCLIC_CLASS_DEFINITION, CYCLIC_TYPE_ALIAS_DEFINITION, DIVISION_BY_ZERO, DUPLICATE_KW_ONLY, INCONSISTENT_MRO, INVALID_ARGUMENT_TYPE, INVALID_ASSIGNMENT, INVALID_ATTRIBUTE_ACCESS, - INVALID_BASE, INVALID_DECLARATION, INVALID_FROZEN_DATACLASS_SUBCLASS, INVALID_GENERIC_CLASS, - INVALID_KEY, INVALID_LEGACY_TYPE_VARIABLE, INVALID_METACLASS, INVALID_NAMED_TUPLE, - INVALID_NEWTYPE, INVALID_OVERLOAD, INVALID_PARAMETER_DEFAULT, INVALID_PARAMSPEC, - INVALID_PROTOCOL, INVALID_TYPE_ARGUMENTS, INVALID_TYPE_FORM, INVALID_TYPE_GUARD_CALL, + INVALID_BASE, INVALID_DECLARATION, INVALID_GENERIC_CLASS, INVALID_KEY, + INVALID_LEGACY_TYPE_VARIABLE, INVALID_METACLASS, INVALID_NAMED_TUPLE, INVALID_NEWTYPE, + INVALID_OVERLOAD, INVALID_PARAMETER_DEFAULT, INVALID_PARAMSPEC, INVALID_PROTOCOL, + INVALID_TYPE_ARGUMENTS, INVALID_TYPE_FORM, INVALID_TYPE_GUARD_CALL, INVALID_TYPE_VARIABLE_CONSTRAINTS, IncompatibleBases, NON_SUBSCRIPTABLE, POSSIBLY_MISSING_ATTRIBUTE, POSSIBLY_MISSING_IMPLICIT_CALL, POSSIBLY_MISSING_IMPORT, SUBCLASS_OF_FINAL_CLASS, UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE, UNRESOLVED_GLOBAL, UNRESOLVED_IMPORT, UNRESOLVED_REFERENCE, UNSUPPORTED_OPERATOR, USELESS_OVERLOAD_BODY, hint_if_stdlib_attribute_exists_on_other_versions, hint_if_stdlib_submodule_exists_on_other_versions, report_attempted_protocol_instantiation, - report_bad_dunder_set_call, report_cannot_pop_required_field_on_typed_dict, - report_duplicate_bases, report_implicit_return_type, report_index_out_of_bounds, - report_instance_layout_conflict, report_invalid_arguments_to_annotated, - report_invalid_assignment, report_invalid_attribute_assignment, - report_invalid_exception_caught, report_invalid_exception_cause, - report_invalid_exception_raised, report_invalid_exception_tuple_caught, - report_invalid_generator_function_return_type, report_invalid_key_on_typed_dict, - report_invalid_or_unsupported_base, report_invalid_return_type, - report_invalid_type_checking_constant, report_named_tuple_field_with_leading_underscore, + report_bad_dunder_set_call, report_bad_frozen_dataclass_inheritance, + report_cannot_pop_required_field_on_typed_dict, report_duplicate_bases, + report_implicit_return_type, report_index_out_of_bounds, report_instance_layout_conflict, + report_invalid_arguments_to_annotated, report_invalid_assignment, + report_invalid_attribute_assignment, report_invalid_exception_caught, + report_invalid_exception_cause, report_invalid_exception_raised, + report_invalid_exception_tuple_caught, report_invalid_generator_function_return_type, + report_invalid_key_on_typed_dict, report_invalid_or_unsupported_base, + report_invalid_return_type, report_invalid_type_checking_constant, + report_named_tuple_field_with_leading_underscore, report_namedtuple_field_without_default_after_field_with_default, report_non_subscriptable, report_possibly_missing_attribute, report_possibly_unresolved_reference, report_rebound_typevar, report_slice_step_size_zero, report_unsupported_augmented_assignment, @@ -104,15 +105,14 @@ use crate::types::typed_dict::{ use crate::types::visitor::any_over_type; use crate::types::{ BoundTypeVarInstance, CallDunderError, CallableBinding, CallableType, CallableTypeKind, - ClassLiteral, ClassType, DataclassFlags, DataclassParams, DynamicType, InternedType, - IntersectionBuilder, IntersectionType, KnownClass, KnownInstanceType, KnownUnion, - LintDiagnosticGuard, MemberLookupPolicy, MetaclassCandidate, PEP695TypeAliasType, - ParamSpecAttrKind, Parameter, ParameterForm, Parameters, Signature, SpecialFormType, - SubclassOfType, TrackedConstraintSet, Truthiness, Type, TypeAliasType, TypeAndQualifiers, - TypeContext, TypeQualifiers, TypeVarBoundOrConstraints, TypeVarBoundOrConstraintsEvaluation, - TypeVarDefaultEvaluation, TypeVarIdentity, TypeVarInstance, TypeVarKind, TypeVarVariance, - TypedDictType, UnionBuilder, UnionType, UnionTypeInstance, binding_type, infer_scope_types, - todo_type, + ClassLiteral, ClassType, DataclassParams, DynamicType, InternedType, IntersectionBuilder, + IntersectionType, KnownClass, KnownInstanceType, KnownUnion, LintDiagnosticGuard, + MemberLookupPolicy, MetaclassCandidate, PEP695TypeAliasType, ParamSpecAttrKind, Parameter, + ParameterForm, Parameters, Signature, SpecialFormType, SubclassOfType, TrackedConstraintSet, + Truthiness, Type, TypeAliasType, TypeAndQualifiers, TypeContext, TypeQualifiers, + TypeVarBoundOrConstraints, TypeVarBoundOrConstraintsEvaluation, TypeVarDefaultEvaluation, + TypeVarIdentity, TypeVarInstance, TypeVarKind, TypeVarVariance, TypedDictType, UnionBuilder, + UnionType, UnionTypeInstance, binding_type, infer_scope_types, todo_type, }; use crate::types::{CallableTypes, overrides}; use crate::types::{ClassBase, add_inferred_python_version_hint_to_diagnostic}; @@ -763,40 +763,18 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { base_class_literal.dataclass_params(self.db()), class.dataclass_params(self.db()), ) { - let base_is_frozen = base_params - .flags(self.db()) - .contains(DataclassFlags::FROZEN); + let base_params = base_params.flags(self.db()); + let class_is_frozen = class_params.flags(self.db()).is_frozen(); - let class_is_frozen = class_params - .flags(self.db()) - .contains(DataclassFlags::FROZEN); - - match (base_is_frozen, class_is_frozen) { - (true, false) => { - if let Some(builder) = self.context.report_lint( - &INVALID_FROZEN_DATACLASS_SUBCLASS, - &class_node.bases()[i], - ) { - builder.into_diagnostic(format_args!( - "A non-frozen class `{}` cannot inherit from a class `{}` that is frozen", - class.name(self.db()), - base_class.name(self.db()), - )); - } - } - (false, true) => { - if let Some(builder) = self.context.report_lint( - &INVALID_FROZEN_DATACLASS_SUBCLASS, - &class_node.bases()[i], - ) { - builder.into_diagnostic(format_args!( - "A frozen class `{}` cannot inherit from a class `{}` that is not frozen", - class.name(self.db()), - base_class.name(self.db()), - )); - } - } - _ => {} + if base_params.is_frozen() != class_is_frozen { + report_bad_frozen_dataclass_inheritance( + &self.context, + class, + class_node, + base_class_literal, + &class_node.bases()[i], + base_params, + ); } } }