From 23fa20bf70225ba9a8675e32994c7ce0774ea8fb Mon Sep 17 00:00:00 2001 From: blockgroot <170620375+blockgroot@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:24:41 +0530 Subject: [PATCH 1/4] Warn when a base var shadows a var inherited from a parent state A substate field whose name matches an inherited base var is dropped silently: get_skip_vars() includes inherited_vars, so base_vars filters the field out. _init_var never runs for it, class access returns the raw default instead of a Var, and reads and writes are delegated to the parent. A component built from it renders a static value rather than a reactive binding, with no diagnostic anywhere. Emit a deduped console.warn at class creation naming the var and both states. A redeclaration whose purpose is to win over a descriptor reached through a non-state base is exempt, since re-annotating is how that MRO conflict is resolved (see test_hybrid_property_shadowed_by_closer_base_stays_a_field). Warning rather than raising, per the repository's policy of not breaking downstream users; the shadowing computed-var guards raise, so this can be escalated later if preferred. Also drops a redundant `is_hydrated: bool = False` from DynamicState in tests/units/test_app.py: it shadowed the identical root State default, so it was already a no-op. Fixes #7074 --- news/7074.bugfix.md | 1 + reflex/state.py | 69 +++++++++++++++++++++++++++++++++++++-- tests/units/test_app.py | 1 - tests/units/test_state.py | 50 ++++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 4 deletions(-) create mode 100644 news/7074.bugfix.md diff --git a/news/7074.bugfix.md b/news/7074.bugfix.md new file mode 100644 index 00000000000..17fd03f35e5 --- /dev/null +++ b/news/7074.bugfix.md @@ -0,0 +1 @@ +Warn when a substate declares a var whose name is already a var on a parent state. Such a declaration is ignored — reads and writes resolve to the parent's var and class-level access returns the raw default instead of a reactive `Var` — and previously produced no diagnostic at all. diff --git a/reflex/state.py b/reflex/state.py index 1ea080cc863..97d884fb4f1 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -344,7 +344,7 @@ def _has_data_descriptor(cls: type, name: str) -> bool: return False -def _is_user_descriptor(value: Any) -> bool: +def _is_user_descriptor(value: Any, *, include_properties: bool = False) -> bool: """Whether a class attribute is a user-defined descriptor. Excludes framework-recognized callables and var types so user-defined @@ -353,6 +353,7 @@ def _is_user_descriptor(value: Any) -> bool: Args: value: The class attribute value to check. + include_properties: Whether property-like descriptors also count. Returns: True if the value is a custom descriptor. @@ -365,14 +366,16 @@ def _is_user_descriptor(value: Any) -> bool: FunctionType, classmethod, staticmethod, - property, - functools.cached_property, EventHandler, Var, Field, ), ): return False + if not include_properties and isinstance( + value, (property, functools.cached_property) + ): + return False return not is_computed_var(value) @@ -663,6 +666,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 +1116,63 @@ 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 _shadows_non_state_descriptor(cls, name: str) -> bool: + """Whether a base outside the state hierarchy defines name as a descriptor. + + Args: + name: The var name to look up. + + Returns: + True if a non-state base in the MRO defines name as a user descriptor. + """ + return any( + not issubclass(base, BaseState) + and _is_user_descriptor(base.__dict__[name], include_properties=True) + for base in cls.__mro__ + if name in base.__dict__ + ) + + @classmethod + def _check_overridden_inherited_vars(cls) -> None: + """Warn about base vars that shadow a var inherited from a parent state. + + Such a redeclaration is dropped silently: the field never becomes a base var, + so reads and writes resolve to the parent's var and class-level access returns + the raw default instead of a Var. + + A redeclaration that exists to win over a descriptor reached through a + non-state base is left alone, since re-annotating is how that MRO conflict + is resolved. + """ + 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 + ): + 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 + or cls._shadows_non_state_descriptor(name) + ): + continue + console.warn( + f"The var `{name}` in {cls.__module__}.{cls.__name__} shadows a var " + f"inherited from {parent_state.__module__}.{parent_state.__name__} and " + "is ignored: reads and writes resolve to the parent's var. Use a " + "different name instead.", + dedupe=True, + ) + @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..b5e942b6ab0 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -5380,3 +5380,53 @@ 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_warns(mocker: MockerFixture) -> None: + """A base var shadowing an inherited var warns instead of being dropped silently. + + Args: + mocker: Pytest mock fixture. + """ + warn_mock = mocker.patch("reflex.state.console.warn") + + class ShadowParent(BaseState): + shadowed_value: int = 1 + + class ShadowChild(ShadowParent): + shadowed_value: str = "ninety-nine" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + assert any("shadowed_value" in call.args[0] for call in warn_mock.call_args_list), ( + "expected a warning naming the shadowed var" + ) + + +def test_base_var_shadowing_non_state_descriptor_does_not_warn( + mocker: MockerFixture, +) -> None: + """Re-annotating to win over a descriptor from a non-state base is not a shadow. + + Args: + mocker: Pytest mock fixture. + """ + from reflex_base.vars.hybrid_property import hybrid_property + + warn_mock = mocker.patch("reflex.state.console.warn") + + 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 not [ + call for call in warn_mock.call_args_list if "descriptor_value" in call.args[0] + ], "re-annotation resolving a descriptor MRO conflict must not warn" From b4c2d768c7d29b4970a1e3e1887f0d3b4d31c8d2 Mon Sep 17 00:00:00 2001 From: blockgroot <170620375+blockgroot@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:37:59 +0530 Subject: [PATCH 2/4] Narrow the descriptor exemption to where the state field outranks it The first version exempted any redeclaration whose name appeared as a descriptor on a non-state base anywhere in the MRO. That is broader than the pattern it exists for: in Child(DescriptorMixin, ParentState) the descriptor precedes the state field, the child's declaration is still discarded, and class access still returns the raw default rather than a Var -- yet the warning was suppressed. Exempt only when a state base declaring the name precedes the same-named non-state descriptor, which is the ordering in which the field already wins and the re-annotation is therefore inert. Both conditions are required: with no descriptor in the MRO at all, an ordinary redeclaration still warns. This tracks whether the declaration actually breaks class access: warnings now fire exactly in the cases where the shadowed name no longer resolves to a Var. Adds test_base_var_shadowing_warns_when_descriptor_outranks_state_field. --- reflex/state.py | 28 ++++++++++++++++++---------- tests/units/test_state.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index 97d884fb4f1..cee25d4024a 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1117,21 +1117,29 @@ def _check_overridden_computed_vars(cls) -> None: raise ComputedVarShadowsStateVarError(msg) @classmethod - def _shadows_non_state_descriptor(cls, name: str) -> bool: - """Whether a base outside the state hierarchy defines name as a descriptor. + def _state_field_precedes_descriptor(cls, name: str) -> bool: + """Whether a state base declaring name outranks a same-named descriptor. + + Re-annotating is how a state field that already wins over a descriptor on a + non-state base is kept, so that redeclaration is inert rather than a mistake. + A descriptor that instead outranks the state field does not make the + redeclaration take effect, so it is not exempt. Args: name: The var name to look up. Returns: - True if a non-state base in the MRO defines name as a user descriptor. + True if a state base declaring name precedes a non-state descriptor. """ - return any( - not issubclass(base, BaseState) - and _is_user_descriptor(base.__dict__[name], include_properties=True) - for base in cls.__mro__ - if name in base.__dict__ - ) + state_first = False + for base in cls.__mro__[1:]: + if name not in base.__dict__: + continue + if issubclass(base, BaseState): + state_first = True + elif _is_user_descriptor(base.__dict__[name], include_properties=True): + return state_first + return False @classmethod def _check_overridden_inherited_vars(cls) -> None: @@ -1162,7 +1170,7 @@ def _check_overridden_inherited_vars(cls) -> None: if ( parent_field is None or parent_field is own_field - or cls._shadows_non_state_descriptor(name) + or cls._state_field_precedes_descriptor(name) ): continue console.warn( diff --git a/tests/units/test_state.py b/tests/units/test_state.py index b5e942b6ab0..4801cc340df 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -5430,3 +5430,34 @@ class DescriptorChild(PlainBase, OverridingState): assert not [ call for call in warn_mock.call_args_list if "descriptor_value" in call.args[0] ], "re-annotation resolving a descriptor MRO conflict must not warn" + + +def test_base_var_shadowing_warns_when_descriptor_outranks_state_field( + mocker: MockerFixture, +) -> None: + """A descriptor closer than the state field does not exempt a dropped declaration. + + Args: + mocker: Pytest mock fixture. + """ + from reflex_base.vars.hybrid_property import hybrid_property + + warn_mock = mocker.patch("reflex.state.console.warn") + + class CloserMixin: + @hybrid_property + def outranked_value(self) -> int: + return 1 + + class OutrankedParent(BaseState): + outranked_value: int = 1 # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + class OutrankedChild(CloserMixin, OutrankedParent): + outranked_value: str = "x" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + assert "outranked_value" not in OutrankedChild.base_vars, ( + "declaration is still dropped, so it must not be treated as effective" + ) + assert any( + "outranked_value" in call.args[0] for call in warn_mock.call_args_list + ), "expected a warning when the descriptor outranks the state field" From 469b68d18c0270547037cee1d4cc4cf1700d709f Mon Sep 17 00:00:00 2001 From: Farhan Date: Tue, 15 Sep 2026 20:20:38 +0500 Subject: [PATCH 3/4] Discriminate shadows by the leftover class attribute, not MRO order A dropped redeclaration is observable directly: the assignment leaves a raw default in the class dict, which is exactly what breaks class-level Var access. A bare re-annotation leaves no class attribute, so the name keeps resolving to the inherited Var and stays reactive. Checking cls.__dict__ instead of walking the MRO fixes both residual misfires of the ordering heuristic: a redeclaration with an assignment in a state-field-outranks-descriptor hierarchy now warns (it was silently dropped), and an inert bare re-annotation with no descriptor in play no longer warns. _state_field_precedes_descriptor and the include_properties kwarg on _is_user_descriptor are deleted. The warning goes through logger.warning with the pipeline's dedupe extra; console.warn is deprecated since 0.9.9. --- reflex/state.py | 54 ++++++-------------------- tests/units/test_state.py | 81 ++++++++++++++++++++++++++++++--------- 2 files changed, 74 insertions(+), 61 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index cee25d4024a..54b423fa5f2 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -344,7 +344,7 @@ def _has_data_descriptor(cls: type, name: str) -> bool: return False -def _is_user_descriptor(value: Any, *, include_properties: bool = False) -> bool: +def _is_user_descriptor(value: Any) -> bool: """Whether a class attribute is a user-defined descriptor. Excludes framework-recognized callables and var types so user-defined @@ -353,7 +353,6 @@ def _is_user_descriptor(value: Any, *, include_properties: bool = False) -> bool Args: value: The class attribute value to check. - include_properties: Whether property-like descriptors also count. Returns: True if the value is a custom descriptor. @@ -366,16 +365,14 @@ def _is_user_descriptor(value: Any, *, include_properties: bool = False) -> bool FunctionType, classmethod, staticmethod, + property, + functools.cached_property, EventHandler, Var, Field, ), ): return False - if not include_properties and isinstance( - value, (property, functools.cached_property) - ): - return False return not is_computed_var(value) @@ -1116,42 +1113,16 @@ 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 _state_field_precedes_descriptor(cls, name: str) -> bool: - """Whether a state base declaring name outranks a same-named descriptor. - - Re-annotating is how a state field that already wins over a descriptor on a - non-state base is kept, so that redeclaration is inert rather than a mistake. - A descriptor that instead outranks the state field does not make the - redeclaration take effect, so it is not exempt. - - Args: - name: The var name to look up. - - Returns: - True if a state base declaring name precedes a non-state descriptor. - """ - state_first = False - for base in cls.__mro__[1:]: - if name not in base.__dict__: - continue - if issubclass(base, BaseState): - state_first = True - elif _is_user_descriptor(base.__dict__[name], include_properties=True): - return state_first - return False - @classmethod def _check_overridden_inherited_vars(cls) -> None: """Warn about base vars that shadow a var inherited from a parent state. Such a redeclaration is dropped silently: the field never becomes a base var, - so reads and writes resolve to the parent's var and class-level access returns - the raw default instead of a 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 redeclaration that exists to win over a descriptor reached through a - non-state base is left alone, since re-annotating is how that MRO conflict - is resolved. + 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. """ parent_state = cls.get_parent_state() if parent_state is None: @@ -1162,23 +1133,20 @@ def _check_overridden_inherited_vars(cls) -> None: 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 - or cls._state_field_precedes_descriptor(name) - ): + if parent_field is None or parent_field is own_field: continue - console.warn( + logger.warning( f"The var `{name}` in {cls.__module__}.{cls.__name__} shadows a var " f"inherited from {parent_state.__module__}.{parent_state.__name__} and " "is ignored: reads and writes resolve to the parent's var. Use a " "different name instead.", - dedupe=True, + extra={"dedupe": True}, ) @classmethod diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 4801cc340df..852c7ebddc1 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -5382,13 +5382,14 @@ def test_setattr_alias_annotated_var(mocker: MockerFixture): error_mock.assert_called_once() -def test_base_var_shadowing_inherited_var_warns(mocker: MockerFixture) -> None: +def test_base_var_shadowing_inherited_var_warns( + caplog: pytest.LogCaptureFixture, +) -> None: """A base var shadowing an inherited var warns instead of being dropped silently. Args: - mocker: Pytest mock fixture. + caplog: Pytest log capture fixture. """ - warn_mock = mocker.patch("reflex.state.console.warn") class ShadowParent(BaseState): shadowed_value: int = 1 @@ -5396,23 +5397,21 @@ class ShadowParent(BaseState): class ShadowChild(ShadowParent): shadowed_value: str = "ninety-nine" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] - assert any("shadowed_value" in call.args[0] for call in warn_mock.call_args_list), ( + assert any("shadowed_value" in r.getMessage() for r in caplog.records), ( "expected a warning naming the shadowed var" ) def test_base_var_shadowing_non_state_descriptor_does_not_warn( - mocker: MockerFixture, + caplog: pytest.LogCaptureFixture, ) -> None: """Re-annotating to win over a descriptor from a non-state base is not a shadow. Args: - mocker: Pytest mock fixture. + caplog: Pytest log capture fixture. """ from reflex_base.vars.hybrid_property import hybrid_property - warn_mock = mocker.patch("reflex.state.console.warn") - class SharedMixin: @hybrid_property def descriptor_value(self) -> int: @@ -5427,23 +5426,21 @@ class OverridingState(SharedMixin, BaseState): class DescriptorChild(PlainBase, OverridingState): descriptor_value: int # pyright: ignore[reportGeneralTypeIssues, reportIncompatibleVariableOverride] - assert not [ - call for call in warn_mock.call_args_list if "descriptor_value" in call.args[0] - ], "re-annotation resolving a descriptor MRO conflict must not warn" + assert not [r for r in caplog.records if "descriptor_value" in r.getMessage()], ( + "re-annotation resolving a descriptor MRO conflict must not warn" + ) def test_base_var_shadowing_warns_when_descriptor_outranks_state_field( - mocker: MockerFixture, + caplog: pytest.LogCaptureFixture, ) -> None: """A descriptor closer than the state field does not exempt a dropped declaration. Args: - mocker: Pytest mock fixture. + caplog: Pytest log capture fixture. """ from reflex_base.vars.hybrid_property import hybrid_property - warn_mock = mocker.patch("reflex.state.console.warn") - class CloserMixin: @hybrid_property def outranked_value(self) -> int: @@ -5458,6 +5455,54 @@ class OutrankedChild(CloserMixin, OutrankedParent): assert "outranked_value" not in OutrankedChild.base_vars, ( "declaration is still dropped, so it must not be treated as effective" ) - assert any( - "outranked_value" in call.args[0] for call in warn_mock.call_args_list - ), "expected a warning when the descriptor outranks the state field" + assert any("outranked_value" in r.getMessage() for r in caplog.records), ( + "expected a warning when the descriptor outranks the state field" + ) + + +def test_base_var_shadowing_warns_despite_state_field_outranking_descriptor( + caplog: pytest.LogCaptureFixture, +) -> None: + """A dropped redeclaration warns even where a state field outranks a descriptor. + + Args: + caplog: Pytest log capture fixture. + """ + 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] + + class RedeclaringChild(DescriptorOwningParent): + redeclared_value: str = "shadowed" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + assert not isinstance(RedeclaringChild.redeclared_value, Var) + assert any("redeclared_value" in r.getMessage() for r in caplog.records), ( + "the raw default breaks class-level Var access, so it must warn" + ) + + +def test_base_var_bare_reannotation_does_not_warn( + caplog: pytest.LogCaptureFixture, +) -> None: + """A bare re-annotation of an inherited var is inert and stays silent. + + Args: + caplog: Pytest log capture fixture. + """ + + class ReannotatedParent(BaseState): + reannotated_value: int = 1 + + class ReannotatingChild(ReannotatedParent): + reannotated_value: int # pyright: ignore[reportGeneralTypeIssues] + + assert isinstance(ReannotatingChild.reannotated_value, Var) + assert not [r for r in caplog.records if "reannotated_value" in r.getMessage()], ( + "a bare re-annotation keeps resolving to the inherited Var and must not warn" + ) From 4153b762be6f9c45dc0662c39014ce7e12576d8b Mon Sep 17 00:00:00 2001 From: Farhan Date: Tue, 15 Sep 2026 20:55:06 +0500 Subject: [PATCH 4/4] Raise BaseVarShadowsInheritedVarError instead of warning The issue asks for the same treatment a shadowing computed var already gets: reject the collision at class-creation time. A substate field that shadows an inherited var now raises BaseVarShadowsInheritedVarError, a NameError like its ComputedVarShadows* siblings. The inert bare re-annotation form stays allowed. The news fragment moves to breaking accordingly, and reflex-base gets a fragment for the new exception. --- news/7074.bugfix.md | 1 - news/7077.breaking.md | 1 + packages/reflex-base/news/7077.feature.md | 1 + .../src/reflex_base/utils/exceptions.py | 4 + reflex/state.py | 17 ++-- tests/units/test_state.py | 86 +++++-------------- 6 files changed, 37 insertions(+), 73 deletions(-) delete mode 100644 news/7074.bugfix.md create mode 100644 news/7077.breaking.md create mode 100644 packages/reflex-base/news/7077.feature.md diff --git a/news/7074.bugfix.md b/news/7074.bugfix.md deleted file mode 100644 index 17fd03f35e5..00000000000 --- a/news/7074.bugfix.md +++ /dev/null @@ -1 +0,0 @@ -Warn when a substate declares a var whose name is already a var on a parent state. Such a declaration is ignored — reads and writes resolve to the parent's var and class-level access returns the raw default instead of a reactive `Var` — and previously produced no diagnostic at all. 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 54b423fa5f2..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, @@ -1115,14 +1116,17 @@ def _check_overridden_computed_vars(cls) -> None: @classmethod def _check_overridden_inherited_vars(cls) -> None: - """Warn about base vars that shadow a var inherited from a parent state. + """Reject base vars that shadow a var inherited from a parent state. - Such a redeclaration is dropped silently: the field never becomes a base var, + 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: @@ -1141,13 +1145,12 @@ def _check_overridden_inherited_vars(cls) -> None: parent_field = parent_fields.get(name) if parent_field is None or parent_field is own_field: continue - logger.warning( + msg = ( f"The var `{name}` in {cls.__module__}.{cls.__name__} shadows a var " - f"inherited from {parent_state.__module__}.{parent_state.__name__} and " - "is ignored: reads and writes resolve to the parent's var. Use a " - "different name instead.", - extra={"dedupe": True}, + 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]: diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 852c7ebddc1..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, @@ -5382,34 +5383,20 @@ def test_setattr_alias_annotated_var(mocker: MockerFixture): error_mock.assert_called_once() -def test_base_var_shadowing_inherited_var_warns( - caplog: pytest.LogCaptureFixture, -) -> None: - """A base var shadowing an inherited var warns instead of being dropped silently. - - Args: - caplog: Pytest log capture fixture. - """ +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 - class ShadowChild(ShadowParent): - shadowed_value: str = "ninety-nine" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] - - assert any("shadowed_value" in r.getMessage() for r in caplog.records), ( - "expected a warning naming the shadowed var" - ) + 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_warn( - caplog: pytest.LogCaptureFixture, -) -> None: - """Re-annotating to win over a descriptor from a non-state base is not a shadow. - Args: - caplog: Pytest log capture fixture. - """ +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: @@ -5426,19 +5413,11 @@ class OverridingState(SharedMixin, BaseState): class DescriptorChild(PlainBase, OverridingState): descriptor_value: int # pyright: ignore[reportGeneralTypeIssues, reportIncompatibleVariableOverride] - assert not [r for r in caplog.records if "descriptor_value" in r.getMessage()], ( - "re-annotation resolving a descriptor MRO conflict must not warn" - ) + assert isinstance(DescriptorChild.descriptor_value, Var) -def test_base_var_shadowing_warns_when_descriptor_outranks_state_field( - caplog: pytest.LogCaptureFixture, -) -> None: - """A descriptor closer than the state field does not exempt a dropped declaration. - - Args: - caplog: Pytest log capture fixture. - """ +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: @@ -5449,25 +5428,14 @@ def outranked_value(self) -> int: class OutrankedParent(BaseState): outranked_value: int = 1 # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] - class OutrankedChild(CloserMixin, OutrankedParent): - outranked_value: str = "x" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + with pytest.raises(BaseVarShadowsInheritedVarError, match="outranked_value"): - assert "outranked_value" not in OutrankedChild.base_vars, ( - "declaration is still dropped, so it must not be treated as effective" - ) - assert any("outranked_value" in r.getMessage() for r in caplog.records), ( - "expected a warning when the descriptor outranks the state field" - ) + class OutrankedChild(CloserMixin, OutrankedParent): + outranked_value: str = "x" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] -def test_base_var_shadowing_warns_despite_state_field_outranking_descriptor( - caplog: pytest.LogCaptureFixture, -) -> None: - """A dropped redeclaration warns even where a state field outranks a descriptor. - - Args: - caplog: Pytest log capture fixture. - """ +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: @@ -5478,23 +5446,14 @@ def redeclared_value(self) -> int: class DescriptorOwningParent(OutrankedMixin, BaseState): redeclared_value: int = 5 # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] - class RedeclaringChild(DescriptorOwningParent): - redeclared_value: str = "shadowed" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + with pytest.raises(BaseVarShadowsInheritedVarError, match="redeclared_value"): - assert not isinstance(RedeclaringChild.redeclared_value, Var) - assert any("redeclared_value" in r.getMessage() for r in caplog.records), ( - "the raw default breaks class-level Var access, so it must warn" - ) + class RedeclaringChild(DescriptorOwningParent): + redeclared_value: str = "shadowed" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] -def test_base_var_bare_reannotation_does_not_warn( - caplog: pytest.LogCaptureFixture, -) -> None: - """A bare re-annotation of an inherited var is inert and stays silent. - - Args: - caplog: Pytest log capture fixture. - """ +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 @@ -5503,6 +5462,3 @@ class ReannotatingChild(ReannotatedParent): reannotated_value: int # pyright: ignore[reportGeneralTypeIssues] assert isinstance(ReannotatingChild.reannotated_value, Var) - assert not [r for r in caplog.records if "reannotated_value" in r.getMessage()], ( - "a bare re-annotation keeps resolving to the inherited Var and must not warn" - )