From 492134e0ed07d5a08cd19510a07421f2ae335c8a Mon Sep 17 00:00:00 2001 From: Lukas Radermacher <84092952+lukarade@users.noreply.github.com> Date: Tue, 18 Jun 2024 16:12:38 +0200 Subject: [PATCH] feat: detect instance variables generated with `@property`-functions (#281) Closes #272 ### Summary of Changes Added the detection of instance variables generated with `@property`-functions. --------- Co-authored-by: megalinter-bot <129584137+megalinter-bot@users.noreply.github.com> --- .../api/purity_analysis/_get_module_data.py | 13 ++++ .../purity_analysis/_resolve_references.py | 12 +++- .../purity_analysis/test_get_module_data.py | 60 ++++++++++++++++++- .../api/purity_analysis/test_infer_purity.py | 20 +++++++ .../test_resolve_references.py | 6 +- 5 files changed, 108 insertions(+), 3 deletions(-) diff --git a/src/library_analyzer/processing/api/purity_analysis/_get_module_data.py b/src/library_analyzer/processing/api/purity_analysis/_get_module_data.py index 51a9624a..4bdc742a 100644 --- a/src/library_analyzer/processing/api/purity_analysis/_get_module_data.py +++ b/src/library_analyzer/processing/api/purity_analysis/_get_module_data.py @@ -705,6 +705,19 @@ def enter_functiondef(self, node: astroid.FunctionDef) -> None: for decorator in node.decorators.nodes: if isinstance(decorator, astroid.Name) and decorator.name == "overload": return + elif isinstance(decorator, astroid.Name) and decorator.name == "property": + if isinstance(self.current_node_stack[-1], ClassScope) and hasattr( + self.current_node_stack[-1], + "instance_variables", + ): + self.current_node_stack[-1].instance_variables.setdefault(node.name, []).append( + InstanceVariable( + node=node, + id=NodeID.calc_node_id(node), + name=node.name, + klass=self.current_node_stack[-1].symbol.node, + ), + ) self.current_node_stack.append( FunctionScope( diff --git a/src/library_analyzer/processing/api/purity_analysis/_resolve_references.py b/src/library_analyzer/processing/api/purity_analysis/_resolve_references.py index a58dbe4f..73e33530 100644 --- a/src/library_analyzer/processing/api/purity_analysis/_resolve_references.py +++ b/src/library_analyzer/processing/api/purity_analysis/_resolve_references.py @@ -768,8 +768,18 @@ def _resolve_references(self) -> tuple[dict[str, list[ReferenceNode]], dict[Node if isinstance(referenced_symbol, GlobalVariable | ClassVariable | InstanceVariable): # Since classes and functions are defined as immutable # reading from them is not a reason for impurity. + # There is an exception to this rule for functions + # that are decorated with a '@property' decorator. These functions define an + # instance variable as a property, which can be read from. if isinstance(referenced_symbol.node, astroid.ClassDef | astroid.FunctionDef): - continue + if ( + isinstance(referenced_symbol.node, astroid.FunctionDef) + and "builtins.property" in referenced_symbol.node.decoratornames() + and isinstance(referenced_symbol, InstanceVariable) + ): + pass + else: + continue # Add the referenced symbol to the list of symbols whom are read from. if referenced_symbol not in raw_reasons[function.symbol.id].reads_from: raw_reasons[function.symbol.id].reads_from.add(referenced_symbol) diff --git a/tests/library_analyzer/processing/api/purity_analysis/test_get_module_data.py b/tests/library_analyzer/processing/api/purity_analysis/test_get_module_data.py index 7c056d97..b7c8f290 100644 --- a/tests/library_analyzer/processing/api/purity_analysis/test_get_module_data.py +++ b/tests/library_analyzer/processing/api/purity_analysis/test_get_module_data.py @@ -119,7 +119,10 @@ def transform_scope_node( super_classes_transformed = [] for child in node.instance_variables.values(): for c1 in child: - c_str = to_string_class(c1.node.node) + if isinstance(c1.node, MemberAccess): + c_str = to_string_class(c1.node.node) + else: + c_str = to_string_class(c1.node) if c_str is not None: instance_vars_transformed.append(c_str) # type: ignore[misc] # it is not possible that c_str is None @@ -1884,6 +1887,60 @@ def __post_init__(self): ), }, ), + ( # language=Python "Assign Instance Attribute via property" + """ +class A: + def __init__(self, value): + self._value = value + + def f(self): + return self.value + + @property + def value(self): + return self._value + """, # language=none + { + "A": SimpleClassScope( + "GlobalVariable.ClassDef.A", + [ + SimpleFunctionScope( + "ClassVariable.FunctionDef.__init__", + [ + SimpleScope("Parameter.AssignName.self", []), + SimpleScope("Parameter.AssignName.value", []), + SimpleScope("InstanceVariable.MemberAccess.self._value", []), + ], + ["AssignName.self", "Name.self", "AssignName.value", "MemberAccessTarget.self._value"], + ["Name.value"], + [], + ["AssignName.self", "AssignName.value"], + ), + SimpleFunctionScope( + "ClassVariable.FunctionDef.f", + [SimpleScope("Parameter.AssignName.self", [])], + ["AssignName.self"], + ["MemberAccessValue.self.value", "Name.self"], + [], + ["AssignName.self"], + ), + SimpleFunctionScope( + "ClassVariable.FunctionDef.value", + [SimpleScope("Parameter.AssignName.self", [])], + ["AssignName.self"], + ["MemberAccessValue.self._value", "Name.self"], + [], + ["AssignName.self"], + ), + ], + ["FunctionDef.__init__", "FunctionDef.f", "FunctionDef.value"], + ["AssignAttr._value", "FunctionDef.value"], + None, + "__init__", + None, + ), + }, + ), ], ids=[ "ClassDef", @@ -1898,6 +1955,7 @@ def __post_init__(self): "Multiple ClassDef", "ClassDef with super class", "ClassDef with __new__, __init__ and __post_init__", + "Assign Instance Attribute via property", ], ) def test_get_module_data_classes(code: str, expected: dict[str, SimpleClassScope]) -> None: diff --git a/tests/library_analyzer/processing/api/purity_analysis/test_infer_purity.py b/tests/library_analyzer/processing/api/purity_analysis/test_infer_purity.py index dc6eb812..ef111f29 100644 --- a/tests/library_analyzer/processing/api/purity_analysis/test_infer_purity.py +++ b/tests/library_analyzer/processing/api/purity_analysis/test_infer_purity.py @@ -471,6 +471,25 @@ def f(): "f.line2": Pure(), }, ), + ( # language=Python "Assign Instance Attribute via property" + """ +class A: + def __init__(self, value): + self._value = value + + def f(self): + return self.value + + @property + def value(self): + return self._value + """, # language=none + { + "__init__.line3": Pure(), + "f.line6": SimpleImpure({"NonLocalVariableRead.InstanceVariable.A.value"}), + "value.line10": SimpleImpure({"NonLocalVariableRead.InstanceVariable.A._value"}), + }, + ), ], ids=[ "Trivial function", @@ -496,6 +515,7 @@ def f(): "Builtins for dict", "Builtins for list", "Builtins for set", + "Assign Instance Attribute via property", ], # TODO: class inits in cycles ) def test_infer_purity_pure(code: str, expected: list[ImpurityReason]) -> None: diff --git a/tests/library_analyzer/processing/api/purity_analysis/test_resolve_references.py b/tests/library_analyzer/processing/api/purity_analysis/test_resolve_references.py index 6c33e707..f012c9e5 100644 --- a/tests/library_analyzer/processing/api/purity_analysis/test_resolve_references.py +++ b/tests/library_analyzer/processing/api/purity_analysis/test_resolve_references.py @@ -2851,7 +2851,11 @@ def f(): ReferenceTestNode( "a.state.line18", "FunctionDef.f", - ["ClassVariable.State.state.line13", "ClassVariable.State.state.line9"], + [ + "ClassVariable.State.state.line13", + "ClassVariable.State.state.line9", + "InstanceVariable.State.state.line9", + ], ), ReferenceTestNode("a.line18", "FunctionDef.f", ["LocalVariable.a.line17"]), ],