diff --git a/news/7077.breaking.md b/news/7077.breaking.md new file mode 100644 index 00000000000..9d8c019ee20 --- /dev/null +++ b/news/7077.breaking.md @@ -0,0 +1 @@ +Declaring a substate var that shadows a var inherited from a parent state now raises `BaseVarShadowsInheritedVarError` at class creation. Such a declaration was silently ignored — reads and writes resolved to the parent's var and class-level access returned the raw default instead of a reactive `Var`. Rename the substate var to fix the error. diff --git a/packages/reflex-base/news/7077.feature.md b/packages/reflex-base/news/7077.feature.md new file mode 100644 index 00000000000..43f9c9516a6 --- /dev/null +++ b/packages/reflex-base/news/7077.feature.md @@ -0,0 +1 @@ +Add `BaseVarShadowsInheritedVarError`, raised when a substate declares a var that shadows a var inherited from a parent state. diff --git a/packages/reflex-base/src/reflex_base/utils/exceptions.py b/packages/reflex-base/src/reflex_base/utils/exceptions.py index bbf29239edf..7e44e993990 100644 --- a/packages/reflex-base/src/reflex_base/utils/exceptions.py +++ b/packages/reflex-base/src/reflex_base/utils/exceptions.py @@ -206,6 +206,10 @@ class ComputedVarShadowsBaseVarsError(ReflexError, NameError): """Raised when a computed var shadows a base var.""" +class BaseVarShadowsInheritedVarError(ReflexError, NameError): + """Raised when a base var shadows a var inherited from a parent state.""" + + class EventHandlerShadowsBuiltInStateMethodError(ReflexError, NameError): """Raised when an event handler shadows a built-in state method.""" diff --git a/reflex/state.py b/reflex/state.py index 1ea080cc863..4a95a88569e 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -40,6 +40,7 @@ ) from reflex_base.registry import RegistrationContext from reflex_base.utils.exceptions import ( + BaseVarShadowsInheritedVarError, ComputedVarShadowsBaseVarsError, ComputedVarShadowsStateVarError, DynamicComponentInvalidSignatureError, @@ -663,6 +664,9 @@ def __init_subclass__(cls, mixin: bool = False, **kwargs): if k not in own_descriptor_names } + # Base vars silently lose to an inherited var of the same name; warn about it. + cls._check_overridden_inherited_vars() + # Get computed vars. computed_vars = cls._get_computed_vars() cls._check_overridden_computed_vars() @@ -1110,6 +1114,44 @@ def _check_overridden_computed_vars(cls) -> None: msg = f"The computed var name `{cv._js_expr}` shadows a var in {cls.__module__}.{cls.__name__}; use a different name instead" raise ComputedVarShadowsStateVarError(msg) + @classmethod + def _check_overridden_inherited_vars(cls) -> None: + """Reject base vars that shadow a var inherited from a parent state. + + Such a redeclaration is dropped: the field never becomes a base var, + so reads and writes resolve to the parent's var, and the raw default left in + the class dict makes class-level access return it instead of a Var. + + A bare re-annotation leaves no class attribute, so the name keeps resolving + to the inherited Var and stays reactive — that form is inert, not a shadow. + + Raises: + BaseVarShadowsInheritedVarError: When a base var shadows an inherited var. + """ + parent_state = cls.get_parent_state() + if parent_state is None: + return + parent_fields = parent_state.get_fields() + for name, own_field in cls.get_fields().items(): + if ( + name.startswith("_") + or not own_field.is_var + or name not in cls.inherited_vars + or name not in cls.__dict__ + ): + continue + # A field redeclared on this class is a distinct object from the parent's; + # a merely inherited one is the same object. + parent_field = parent_fields.get(name) + if parent_field is None or parent_field is own_field: + continue + msg = ( + f"The var `{name}` in {cls.__module__}.{cls.__name__} shadows a var " + f"inherited from {parent_state.__module__}.{parent_state.__name__}; " + "use a different name instead" + ) + raise BaseVarShadowsInheritedVarError(msg) + @classmethod def get_skip_vars(cls) -> set[str]: """Get the vars to skip when serializing. diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 932456cd6ed..9d0b8ca7e4e 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -1779,7 +1779,6 @@ class DynamicState(State): recalculated when the dynamic route var was dirty """ - is_hydrated: bool = False loaded: int = 0 counter: int = 0 diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 37c0ed2fc4c..0e53545ba0e 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -29,6 +29,7 @@ from reflex_base.event.processor import BaseStateEventProcessor from reflex_base.utils import format, types from reflex_base.utils.exceptions import ( + BaseVarShadowsInheritedVarError, InvalidLockWarningThresholdError, LockExpiredError, ReflexRuntimeError, @@ -5380,3 +5381,84 @@ def test_setattr_alias_annotated_var(mocker: MockerFixture): state.key = 1 # pyright: ignore[reportAttributeAccessIssue] assert state.key == 1 error_mock.assert_called_once() + + +def test_base_var_shadowing_inherited_var_raises() -> None: + """A base var shadowing an inherited var raises instead of being dropped silently.""" + + class ShadowParent(BaseState): + shadowed_value: int = 1 + + with pytest.raises(BaseVarShadowsInheritedVarError, match="shadowed_value"): + + class ShadowChild(ShadowParent): + shadowed_value: str = "ninety-nine" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + +def test_base_var_shadowing_non_state_descriptor_does_not_raise() -> None: + """Re-annotating to win over a descriptor from a non-state base is not a shadow.""" + from reflex_base.vars.hybrid_property import hybrid_property + + class SharedMixin: + @hybrid_property + def descriptor_value(self) -> int: + return 1 + + class PlainBase(SharedMixin): + pass + + class OverridingState(SharedMixin, BaseState): + descriptor_value: int = 5 # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + class DescriptorChild(PlainBase, OverridingState): + descriptor_value: int # pyright: ignore[reportGeneralTypeIssues, reportIncompatibleVariableOverride] + + assert isinstance(DescriptorChild.descriptor_value, Var) + + +def test_base_var_shadowing_raises_when_descriptor_outranks_state_field() -> None: + """A descriptor closer than the state field does not exempt a dropped declaration.""" + from reflex_base.vars.hybrid_property import hybrid_property + + class CloserMixin: + @hybrid_property + def outranked_value(self) -> int: + return 1 + + class OutrankedParent(BaseState): + outranked_value: int = 1 # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + with pytest.raises(BaseVarShadowsInheritedVarError, match="outranked_value"): + + class OutrankedChild(CloserMixin, OutrankedParent): + outranked_value: str = "x" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + +def test_base_var_shadowing_raises_despite_state_field_outranking_descriptor() -> None: + """A dropped redeclaration raises even where a state field outranks a descriptor.""" + from reflex_base.vars.hybrid_property import hybrid_property + + class OutrankedMixin: + @hybrid_property + def redeclared_value(self) -> int: + return 1 + + class DescriptorOwningParent(OutrankedMixin, BaseState): + redeclared_value: int = 5 # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + with pytest.raises(BaseVarShadowsInheritedVarError, match="redeclared_value"): + + class RedeclaringChild(DescriptorOwningParent): + redeclared_value: str = "shadowed" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + +def test_base_var_bare_reannotation_does_not_raise() -> None: + """A bare re-annotation of an inherited var is inert and stays allowed.""" + + class ReannotatedParent(BaseState): + reannotated_value: int = 1 + + class ReannotatingChild(ReannotatedParent): + reannotated_value: int # pyright: ignore[reportGeneralTypeIssues] + + assert isinstance(ReannotatingChild.reannotated_value, Var)