From ecb9f141e4029903e95de284fef92b180844fe0a Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 26 Sep 2024 08:38:21 +0000 Subject: [PATCH 01/12] Setting State Vars that are not defined should raise an error --- reflex/state.py | 3 +++ tests/units/test_state.py | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/reflex/state.py b/reflex/state.py index cda36a0a91..d7cfcf18bc 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1277,6 +1277,9 @@ def __setattr__(self, name: str, value: Any): self._mark_dirty() return + if not name in self.vars and not name in self._computed_var_dependencies and not name in self.get_skip_vars(): + raise AttributeError(f"The state var '{name}' has not been defined in '{type(self).__name__}'. All state vars must be declared before they can be set.") + # Set the attribute. super().__setattr__(name, value) diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 205162b9f2..b85116101d 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -3262,3 +3262,26 @@ def test_child_mixin_state() -> None: assert "computed" in ChildUsesMixinState.inherited_vars assert "computed" not in ChildUsesMixinState.computed_vars + + +def test_assignment_to_undeclared_vars(): + """Test that an attribute error is thrown when undeclared vars are set""" + class State(BaseState): + val: str + + def handle(self): + self.num = 5 + + class Substate(State): + + def handle_var(self): + self.value = 20 + + state = State() + sub_state = Substate() + + with pytest.raises(AttributeError): + state.handle() + + with pytest.raises(AttributeError): + sub_state.handle() From b34a5d3b5455ec642f0a875145b9ecaaebb27001 Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 26 Sep 2024 11:44:56 +0000 Subject: [PATCH 02/12] `self.var` contains all the info we need --- reflex/state.py | 6 ++++-- tests/units/test_state.py | 7 +++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index d7cfcf18bc..ab22fd6886 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1277,8 +1277,10 @@ def __setattr__(self, name: str, value: Any): self._mark_dirty() return - if not name in self.vars and not name in self._computed_var_dependencies and not name in self.get_skip_vars(): - raise AttributeError(f"The state var '{name}' has not been defined in '{type(self).__name__}'. All state vars must be declared before they can be set.") + if name not in self.vars and name not in self.get_skip_vars(): + raise AttributeError( + f"The state var '{name}' has not been defined in '{type(self).__name__}'. All state vars must be declared before they can be set." + ) # Set the attribute. super().__setattr__(name, value) diff --git a/tests/units/test_state.py b/tests/units/test_state.py index b85116101d..d4ece2b812 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -3265,7 +3265,8 @@ def test_child_mixin_state() -> None: def test_assignment_to_undeclared_vars(): - """Test that an attribute error is thrown when undeclared vars are set""" + """Test that an attribute error is thrown when undeclared vars are set.""" + class State(BaseState): val: str @@ -3273,7 +3274,6 @@ def handle(self): self.num = 5 class Substate(State): - def handle_var(self): self.value = 20 @@ -3285,3 +3285,6 @@ def handle_var(self): with pytest.raises(AttributeError): sub_state.handle() + + with pytest.raises(AttributeError): + sub_state.handle_var() From 07a0560145a4438903cd9aa5c434b60ae1833a35 Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 26 Sep 2024 12:01:00 +0000 Subject: [PATCH 03/12] tests fix --- reflex/state.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index ab22fd6886..22d79120e8 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1277,9 +1277,15 @@ def __setattr__(self, name: str, value: Any): self._mark_dirty() return - if name not in self.vars and name not in self.get_skip_vars(): + if ( + not name.startswith("__") + and name not in self.vars + and name not in self.get_skip_vars() + ): + available_vars = ", ".join(self.vars) or "None" raise AttributeError( - f"The state var '{name}' has not been defined in '{type(self).__name__}'. All state vars must be declared before they can be set." + f"The state variable '{name}' has not been defined in '{type(self).__name__}'. " + f"All state variables must be declared before they can be set. Available vars: {available_vars}" ) # Set the attribute. From c3a2688948303e43ac6d6f9b3e67687654090078 Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 26 Sep 2024 15:04:25 +0000 Subject: [PATCH 04/12] tests fix --- reflex/state.py | 3 +-- tests/units/test_state.py | 7 ++----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index 22d79120e8..515370be08 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1282,10 +1282,9 @@ def __setattr__(self, name: str, value: Any): and name not in self.vars and name not in self.get_skip_vars() ): - available_vars = ", ".join(self.vars) or "None" raise AttributeError( f"The state variable '{name}' has not been defined in '{type(self).__name__}'. " - f"All state variables must be declared before they can be set. Available vars: {available_vars}" + f"All state variables must be declared before they can be set." ) # Set the attribute. diff --git a/tests/units/test_state.py b/tests/units/test_state.py index d4ece2b812..1f5c6a70c2 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -3277,14 +3277,11 @@ class Substate(State): def handle_var(self): self.value = 20 - state = State() - sub_state = Substate() + state = State() # type: ignore + sub_state = Substate() # type: ignore with pytest.raises(AttributeError): state.handle() - with pytest.raises(AttributeError): - sub_state.handle() - with pytest.raises(AttributeError): sub_state.handle_var() From dbf43b1b09cf939ab7d0e376e486762b3f44a1e5 Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 26 Sep 2024 15:17:37 +0000 Subject: [PATCH 05/12] precommit fix --- reflex/state.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/reflex/state.py b/reflex/state.py index 515370be08..cfe8201414 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1260,6 +1260,9 @@ def __setattr__(self, name: str, value: Any): Args: name: The name of the attribute. value: The value of the attribute. + + Raises: + AttributeError: If a value of a var is set without first defining it. """ if isinstance(value, MutableProxy): # unwrap proxy objects when assigning back to the state From 15af1cade4117a8bb590b2ecade626aca74bd226 Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 26 Sep 2024 17:28:37 +0000 Subject: [PATCH 06/12] restrict this only to frontend vars for now. --- reflex/state.py | 4 +++- tests/units/test_state.py | 26 ++++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index cfe8201414..95a767feab 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1281,7 +1281,9 @@ def __setattr__(self, name: str, value: Any): return if ( - not name.startswith("__") + not name.startswith( + "_" + ) # TODO: skipping backend vars and vars with double leading underscores for now. They should be supported, however. and name not in self.vars and name not in self.get_skip_vars() ): diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 1f5c6a70c2..06362f4435 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -3269,10 +3269,23 @@ def test_assignment_to_undeclared_vars(): class State(BaseState): val: str + _val: str + __val: str - def handle(self): + def handle_supported_regular_vars(self): + self.val = "no underscore" + self._val = "single leading underscore" + self.__val = "double leading undercore" + + def handle_regular_var(self): self.num = 5 + def handle_backend_var(self): + self._num = 5 + + def handle_non_var(self): + self.__num = 5 + class Substate(State): def handle_var(self): self.value = 20 @@ -3281,7 +3294,16 @@ def handle_var(self): sub_state = Substate() # type: ignore with pytest.raises(AttributeError): - state.handle() + state.handle_regular_var() with pytest.raises(AttributeError): sub_state.handle_var() + + # TODO: uncomment this if the case of backend vars are supported. + # with pytest.raises(AttributeError): + # state.handle_backend_var() + # + # with pytest.raises(AttributeError): + # state.handle_non_var() + + state.handle_supported_regular_vars() From 28f210c5193eaf7f067ec5b59d4de5128dfc63f6 Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 26 Sep 2024 17:32:01 +0000 Subject: [PATCH 07/12] pyright fix --- tests/units/test_state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 06362f4435..d3612231c5 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -3270,7 +3270,7 @@ def test_assignment_to_undeclared_vars(): class State(BaseState): val: str _val: str - __val: str + __val: str # type: ignore def handle_supported_regular_vars(self): self.val = "no underscore" From cd91136de67bc62faf56a99f17ff60f3ef9be1e2 Mon Sep 17 00:00:00 2001 From: Elijah Date: Wed, 2 Oct 2024 17:27:41 +0000 Subject: [PATCH 08/12] use custom exception --- reflex/state.py | 5 +++-- reflex/utils/exceptions.py | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index 95a767feab..5e97832730 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -74,6 +74,7 @@ EventHandlerShadowsBuiltInStateMethod, ImmutableStateError, LockExpiredError, + SetUndefinedStateVarError, ) from reflex.utils.exec import is_testing_env from reflex.utils.serializers import serializer @@ -1262,7 +1263,7 @@ def __setattr__(self, name: str, value: Any): value: The value of the attribute. Raises: - AttributeError: If a value of a var is set without first defining it. + SetUndefinedStateVar: If a value of a var is set without first defining it. """ if isinstance(value, MutableProxy): # unwrap proxy objects when assigning back to the state @@ -1287,7 +1288,7 @@ def __setattr__(self, name: str, value: Any): and name not in self.vars and name not in self.get_skip_vars() ): - raise AttributeError( + raise SetUndefinedStateVarError( f"The state variable '{name}' has not been defined in '{type(self).__name__}'. " f"All state variables must be declared before they can be set." ) diff --git a/reflex/utils/exceptions.py b/reflex/utils/exceptions.py index 7c35328612..9c79a387ae 100644 --- a/reflex/utils/exceptions.py +++ b/reflex/utils/exceptions.py @@ -115,3 +115,7 @@ class PrimitiveUnserializableToJSON(ReflexError, ValueError): class InvalidLifespanTaskType(ReflexError, TypeError): """Raised when an invalid task type is registered as a lifespan task.""" + + +class SetUndefinedStateVarError(ReflexError, AttributeError): + """Raised when setting the value of a var without first declaring it.""" From bb8d7c5d15a57a328289f11abe506728f43feec5 Mon Sep 17 00:00:00 2001 From: Elijah Date: Wed, 2 Oct 2024 22:20:52 +0000 Subject: [PATCH 09/12] fix darglint --- reflex/state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reflex/state.py b/reflex/state.py index 5e97832730..041fa04aeb 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1263,7 +1263,7 @@ def __setattr__(self, name: str, value: Any): value: The value of the attribute. Raises: - SetUndefinedStateVar: If a value of a var is set without first defining it. + SetUndefinedStateVarError: If a value of a var is set without first defining it. """ if isinstance(value, MutableProxy): # unwrap proxy objects when assigning back to the state From c51d6752c67f42bd8bc5152e0556b6fccb09d58f Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 3 Oct 2024 08:35:00 +0000 Subject: [PATCH 10/12] backend vars logic is now supported --- reflex/state.py | 5 ++--- tests/units/test_state.py | 14 ++++++-------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index 041fa04aeb..bd3cc79eaa 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1282,9 +1282,8 @@ def __setattr__(self, name: str, value: Any): return if ( - not name.startswith( - "_" - ) # TODO: skipping backend vars and vars with double leading underscores for now. They should be supported, however. + not name.startswith("__") + and not name.startswith(f"_{type(self).__name__}__") and name not in self.vars and name not in self.get_skip_vars() ): diff --git a/tests/units/test_state.py b/tests/units/test_state.py index d3612231c5..5bfac76282 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -41,6 +41,7 @@ ) from reflex.testing import chdir from reflex.utils import format, prerequisites, types +from reflex.utils.exceptions import SetUndefinedStateVarError from reflex.utils.format import json_dumps from reflex.vars.base import ComputedVar, Var from tests.units.states.mutation import MutableSQLAModel, MutableTestState @@ -3293,17 +3294,14 @@ def handle_var(self): state = State() # type: ignore sub_state = Substate() # type: ignore - with pytest.raises(AttributeError): + with pytest.raises(SetUndefinedStateVarError): state.handle_regular_var() - with pytest.raises(AttributeError): + with pytest.raises(SetUndefinedStateVarError): sub_state.handle_var() - # TODO: uncomment this if the case of backend vars are supported. - # with pytest.raises(AttributeError): - # state.handle_backend_var() - # - # with pytest.raises(AttributeError): - # state.handle_non_var() + with pytest.raises(SetUndefinedStateVarError): + state.handle_backend_var() state.handle_supported_regular_vars() + state.handle_non_var() From fa8f7be00c90ea92fa8bcb78153432a5cde91eeb Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 3 Oct 2024 16:58:27 +0000 Subject: [PATCH 11/12] reorder if checks --- reflex/state.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index bd3cc79eaa..d3002e04a5 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1282,9 +1282,9 @@ def __setattr__(self, name: str, value: Any): return if ( - not name.startswith("__") + name not in self.vars + and not name.startswith("__") and not name.startswith(f"_{type(self).__name__}__") - and name not in self.vars and name not in self.get_skip_vars() ): raise SetUndefinedStateVarError( From 3d07437b63f161a15555e08bde03b30df4bf570c Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 3 Oct 2024 17:00:55 +0000 Subject: [PATCH 12/12] final reorder --- reflex/state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reflex/state.py b/reflex/state.py index d3002e04a5..64ea960e10 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1283,9 +1283,9 @@ def __setattr__(self, name: str, value: Any): if ( name not in self.vars + and name not in self.get_skip_vars() and not name.startswith("__") and not name.startswith(f"_{type(self).__name__}__") - and name not in self.get_skip_vars() ): raise SetUndefinedStateVarError( f"The state variable '{name}' has not been defined in '{type(self).__name__}'. "