Skip to content

Commit

Permalink
Forbid Computed var shadowing (#3843)
Browse files Browse the repository at this point in the history
  • Loading branch information
benedikt-bartscher authored Sep 10, 2024
1 parent a5c73ad commit d672c64
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 4 deletions.
21 changes: 19 additions & 2 deletions reflex/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ def __init_subclass__(cls, mixin: bool = False, **kwargs):
cls._check_overridden_methods()

# Computed vars should not shadow builtin state props.
cls._check_overriden_basevars()
cls._check_overridden_basevars()

# Reset subclass tracking for this class.
cls.class_subclasses = set()
Expand Down Expand Up @@ -505,6 +505,7 @@ def __init_subclass__(cls, mixin: bool = False, **kwargs):

# Get computed vars.
computed_vars = cls._get_computed_vars()
cls._check_overridden_computed_vars()

new_backend_vars = {
name: value
Expand Down Expand Up @@ -718,7 +719,7 @@ def _check_overridden_methods(cls):
)

@classmethod
def _check_overriden_basevars(cls):
def _check_overridden_basevars(cls):
"""Check for shadow base vars and raise error if any.
Raises:
Expand All @@ -730,6 +731,22 @@ def _check_overriden_basevars(cls):
f"The computed var name `{computed_var_._var_name}` shadows a base var in {cls.__module__}.{cls.__name__}; use a different name instead"
)

@classmethod
def _check_overridden_computed_vars(cls) -> None:
"""Check for shadow computed vars and raise error if any.
Raises:
NameError: When a computed var shadows another.
"""
for name, cv in cls.__dict__.items():
if not isinstance(cv, (ComputedVar, ImmutableComputedVar)):
continue
name = cv._var_name
if name in cls.inherited_vars or name in cls.inherited_backend_vars:
raise NameError(
f"The computed var name `{cv._var_name}` shadows a var in {cls.__module__}.{cls.__name__}; use a different name instead"
)

@classmethod
def get_skip_vars(cls) -> set[str]:
"""Get the vars to skip when serializing.
Expand Down
32 changes: 30 additions & 2 deletions tests/test_var.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ def ChildState(ParentState, TestObj):
class ChildState(ParentState):
@immutable_computed_var
def var_without_annotation(self):
"""This shadows ParentState.var_without_annotation.
Returns:
TestObj: Test object.
"""
return TestObj

return ChildState
Expand All @@ -89,6 +94,11 @@ def GrandChildState(ChildState, TestObj):
class GrandChildState(ChildState):
@immutable_computed_var
def var_without_annotation(self):
"""This shadows ChildState.var_without_annotation.
Returns:
TestObj: Test object.
"""
return TestObj

return GrandChildState
Expand Down Expand Up @@ -738,8 +748,6 @@ def test_var_unsupported_indexing_dicts(var, index):
"fixture",
[
"ParentState",
"ChildState",
"GrandChildState",
"StateWithAnyVar",
],
)
Expand All @@ -761,6 +769,26 @@ def test_computed_var_without_annotation_error(request, fixture):
)


@pytest.mark.parametrize(
"fixture",
[
"ChildState",
"GrandChildState",
],
)
def test_shadow_computed_var_error(request: pytest.FixtureRequest, fixture: str):
"""Test that a name error is thrown when an attribute of a computed var is
shadowed by another attribute.
Args:
request: Fixture Request.
fixture: The state fixture.
"""
with pytest.raises(NameError):
state = request.getfixturevalue(fixture)
state.var_without_annotation.foo


@pytest.mark.parametrize(
"fixture",
[
Expand Down

0 comments on commit d672c64

Please sign in to comment.