From 9962519dc9575d650c66a8da9c1126864b45330b Mon Sep 17 00:00:00 2001 From: Julfried Date: Wed, 30 Oct 2024 15:12:31 +0100 Subject: [PATCH 1/9] Extract return types for property decorator --- pylint/pyreverse/inspector.py | 46 +++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index 40fb8d6079..a51759e17b 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -21,6 +21,7 @@ from pylint import constants from pylint.pyreverse import utils +from pylint.pyreverse.utils import get_annotation_label _WrapperFuncT = Callable[ [Callable[[str], nodes.Module], str, bool], Optional[nodes.Module] @@ -174,6 +175,51 @@ def visit_classdef(self, node: nodes.ClassDef) -> None: if not isinstance(assignattr, nodes.Unknown): self.associations_handler.handle(assignattr, node) self.handle_assignattr_type(assignattr, node) + # resolve class properties + properties = [] + for name, member_list in node.locals.items(): + for member in member_list: + if isinstance(member, nodes.FunctionDef): + # Check if this function is decorated with @property + if any( + isinstance(decorator, nodes.Name) + and decorator.name == "property" + for decorator in ( + member.decorators.nodes if member.decorators else [] + ) + ): + properties.append(member) + + # Extract return type directly from the function definition + if member.returns: + # Use get_annotation_label to extract the type name + annotation_label = get_annotation_label(member.returns) + inferred_type = annotation_label + + # Special handling for enum types ==> name property is always a string + elif ( + any(base.name == "Enum" for base in node.ancestors()) + and name == "name" + ): + inferred_type = "str" + + # Fallback to inference (which may not always be perfect) + else: + inferred_nodes = utils.infer_node(member) + inferred_type = ( + ", ".join(str(inf) for inf in inferred_nodes if inf) + if inferred_nodes + else "Unknown" + ) + + # Assign to instance attributes with the inferred or annotated type + node.instance_attrs_type[name] = [inferred_type] + print( + f"Property {name} in class {node.name} has type {inferred_type}" + ) + + # Add the detected properties to a new attribute for the class definition + node.properties = properties def visit_functiondef(self, node: nodes.FunctionDef) -> None: """Visit an astroid.Function node. From a707767afb52e3d303ed03aa9c1d93fa0bdabd94 Mon Sep 17 00:00:00 2001 From: Julfried Date: Wed, 30 Oct 2024 15:41:51 +0100 Subject: [PATCH 2/9] Update writer logic to also add properties to the diagram --- pylint/pyreverse/writer.py | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/pylint/pyreverse/writer.py b/pylint/pyreverse/writer.py index e822f67096..6736c99e08 100644 --- a/pylint/pyreverse/writer.py +++ b/pylint/pyreverse/writer.py @@ -171,10 +171,27 @@ def get_package_properties(self, obj: PackageEntity) -> NodeProperties: def get_class_properties(self, obj: ClassEntity) -> NodeProperties: """Get label and shape for classes.""" + attrs = obj.attrs if not self.config.only_classnames else None + methods = obj.methods if not self.config.only_classnames else None + + if attrs and hasattr(obj.node, "properties"): + formatted_attrs = [] + property_names = {prop.name for prop in obj.node.properties} + + for attr in attrs: + name = attr.split(":")[0].strip() + if name in property_names: + # Get type from instance_attrs_type + prop_type = obj.node.instance_attrs_type.get(name, ["Unknown"])[0] + formatted_attrs.append(f"{name} «property»: {prop_type}") + else: + formatted_attrs.append(attr) + attrs = formatted_attrs + properties = NodeProperties( label=obj.title, - attrs=obj.attrs if not self.config.only_classnames else None, - methods=obj.methods if not self.config.only_classnames else None, + attrs=attrs, + methods=methods, fontcolor="red" if is_exception(obj.node) else "black", color=self.get_shape_color(obj) if self.config.colorized else "black", ) From a407835c2426dffb89d852b0f0729a26edd8d177 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sun, 2 Mar 2025 22:14:38 +0100 Subject: [PATCH 3/9] Revert "Update writer logic to also add properties to the diagram" This reverts commit a707767afb52e3d303ed03aa9c1d93fa0bdabd94. Revert "Extract return types for property decorator" This reverts commit 9962519dc9575d650c66a8da9c1126864b45330b. --- pylint/pyreverse/inspector.py | 46 ----------------------------------- pylint/pyreverse/writer.py | 21 ++-------------- 2 files changed, 2 insertions(+), 65 deletions(-) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index a51759e17b..40fb8d6079 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -21,7 +21,6 @@ from pylint import constants from pylint.pyreverse import utils -from pylint.pyreverse.utils import get_annotation_label _WrapperFuncT = Callable[ [Callable[[str], nodes.Module], str, bool], Optional[nodes.Module] @@ -175,51 +174,6 @@ def visit_classdef(self, node: nodes.ClassDef) -> None: if not isinstance(assignattr, nodes.Unknown): self.associations_handler.handle(assignattr, node) self.handle_assignattr_type(assignattr, node) - # resolve class properties - properties = [] - for name, member_list in node.locals.items(): - for member in member_list: - if isinstance(member, nodes.FunctionDef): - # Check if this function is decorated with @property - if any( - isinstance(decorator, nodes.Name) - and decorator.name == "property" - for decorator in ( - member.decorators.nodes if member.decorators else [] - ) - ): - properties.append(member) - - # Extract return type directly from the function definition - if member.returns: - # Use get_annotation_label to extract the type name - annotation_label = get_annotation_label(member.returns) - inferred_type = annotation_label - - # Special handling for enum types ==> name property is always a string - elif ( - any(base.name == "Enum" for base in node.ancestors()) - and name == "name" - ): - inferred_type = "str" - - # Fallback to inference (which may not always be perfect) - else: - inferred_nodes = utils.infer_node(member) - inferred_type = ( - ", ".join(str(inf) for inf in inferred_nodes if inf) - if inferred_nodes - else "Unknown" - ) - - # Assign to instance attributes with the inferred or annotated type - node.instance_attrs_type[name] = [inferred_type] - print( - f"Property {name} in class {node.name} has type {inferred_type}" - ) - - # Add the detected properties to a new attribute for the class definition - node.properties = properties def visit_functiondef(self, node: nodes.FunctionDef) -> None: """Visit an astroid.Function node. diff --git a/pylint/pyreverse/writer.py b/pylint/pyreverse/writer.py index 6736c99e08..e822f67096 100644 --- a/pylint/pyreverse/writer.py +++ b/pylint/pyreverse/writer.py @@ -171,27 +171,10 @@ def get_package_properties(self, obj: PackageEntity) -> NodeProperties: def get_class_properties(self, obj: ClassEntity) -> NodeProperties: """Get label and shape for classes.""" - attrs = obj.attrs if not self.config.only_classnames else None - methods = obj.methods if not self.config.only_classnames else None - - if attrs and hasattr(obj.node, "properties"): - formatted_attrs = [] - property_names = {prop.name for prop in obj.node.properties} - - for attr in attrs: - name = attr.split(":")[0].strip() - if name in property_names: - # Get type from instance_attrs_type - prop_type = obj.node.instance_attrs_type.get(name, ["Unknown"])[0] - formatted_attrs.append(f"{name} «property»: {prop_type}") - else: - formatted_attrs.append(attr) - attrs = formatted_attrs - properties = NodeProperties( label=obj.title, - attrs=attrs, - methods=methods, + attrs=obj.attrs if not self.config.only_classnames else None, + methods=obj.methods if not self.config.only_classnames else None, fontcolor="red" if is_exception(obj.node) else "black", color=self.get_shape_color(obj) if self.config.colorized else "black", ) From 8ff2fdd127eff527176134dd17552b6cde82ec20 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sun, 2 Mar 2025 22:45:20 +0100 Subject: [PATCH 4/9] Implement a cleaner solution for extracting the return types of properties --- pylint/pyreverse/diagrams.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/pylint/pyreverse/diagrams.py b/pylint/pyreverse/diagrams.py index 278102cab8..6db0563003 100644 --- a/pylint/pyreverse/diagrams.py +++ b/pylint/pyreverse/diagrams.py @@ -13,7 +13,7 @@ from astroid import nodes, util from pylint.checkers.utils import decorated_with_property, in_type_checking_block -from pylint.pyreverse.utils import FilterMixIn +from pylint.pyreverse.utils import FilterMixIn, get_annotation_label class Figure: @@ -121,12 +121,16 @@ def get_relationship( def get_attrs(self, node: nodes.ClassDef) -> list[str]: """Return visible attributes, possibly with class name.""" attrs = [] + + # Collect functions decorated with @property properties = { local_name: local_node for local_name, local_node in node.items() if isinstance(local_node, nodes.FunctionDef) and decorated_with_property(local_node) } + + # Add instance attributes to properties for attr_name, attr_type in list(node.locals_type.items()) + list( node.instance_attrs_type.items() ): @@ -136,9 +140,21 @@ def get_attrs(self, node: nodes.ClassDef) -> list[str]: for node_name, associated_nodes in properties.items(): if not self.show_attr(node_name): continue - names = self.class_names(associated_nodes) - if names: - node_name = f"{node_name} : {', '.join(names)}" + + # Handle property methods differently to correctly extract return type + if isinstance( + associated_nodes, nodes.FunctionDef + ) and decorated_with_property(associated_nodes): + if associated_nodes.returns: + type_annotation = get_annotation_label(associated_nodes.returns) + node_name = f"{node_name} : {type_annotation}" + + # Handle regular attributes + else: + names = self.class_names(associated_nodes) + if names: + node_name = f"{node_name} : {', '.join(names)}" + attrs.append(node_name) return sorted(attrs) From 22249a2d95ab94dc931583e3c00aff2d4e0379cb Mon Sep 17 00:00:00 2001 From: Julfried Date: Fri, 7 Mar 2025 17:14:15 +0100 Subject: [PATCH 5/9] Add functional tests for property decorator --- .../property_decorator/annotated.mmd | 4 ++++ .../property_decorator/annotated.py | 19 +++++++++++++++++++ .../property_decorator/not_annotated.mmd | 4 ++++ .../property_decorator/not_annotated.py | 19 +++++++++++++++++++ 4 files changed, 46 insertions(+) create mode 100644 tests/pyreverse/functional/class_diagrams/property_decorator/annotated.mmd create mode 100644 tests/pyreverse/functional/class_diagrams/property_decorator/annotated.py create mode 100644 tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.mmd create mode 100644 tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.py diff --git a/tests/pyreverse/functional/class_diagrams/property_decorator/annotated.mmd b/tests/pyreverse/functional/class_diagrams/property_decorator/annotated.mmd new file mode 100644 index 0000000000..40cc5e3527 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/property_decorator/annotated.mmd @@ -0,0 +1,4 @@ +classDiagram + class PropertyTest { + x : int + } diff --git a/tests/pyreverse/functional/class_diagrams/property_decorator/annotated.py b/tests/pyreverse/functional/class_diagrams/property_decorator/annotated.py new file mode 100644 index 0000000000..059ff2a691 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/property_decorator/annotated.py @@ -0,0 +1,19 @@ +class PropertyTest: + """This is a test class for property decorators with annotated return type""" + def __init__(self): + self._x = 0 + + @property + def x(self) -> int: + """This is a getter for x""" + return self._x + + @x.setter + def x(self, value): + """This is a setter for x""" + self._x = value + + @x.deleter + def x(self): + """This is a deleter for x""" + del self._x diff --git a/tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.mmd b/tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.mmd new file mode 100644 index 0000000000..bbb2994e98 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.mmd @@ -0,0 +1,4 @@ +classDiagram + class PropertyTest { + x + } diff --git a/tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.py b/tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.py new file mode 100644 index 0000000000..c6582e0a35 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.py @@ -0,0 +1,19 @@ +class PropertyTest: + """This is a test class for property decorators with annotated return type""" + def __init__(self): + self._x = 0 + + @property + def x(self): + """This is a getter for x""" + return self._x + + @x.setter + def x(self, value): + """This is a setter for x""" + self._x = value + + @x.deleter + def x(self): + """This is a deleter for x""" + del self._x From c424c3db22ba3ee295714eb9248163b13c45937d Mon Sep 17 00:00:00 2001 From: Julfried Date: Mon, 10 Mar 2025 10:11:01 +0100 Subject: [PATCH 6/9] Merge into a single file --- .../property_decorator/annotated.mmd | 4 -- .../property_decorator/annotated.py | 19 --------- .../property_decorator/not_annotated.mmd | 4 -- .../property_decorator/not_annotated.py | 19 --------- .../property_decorator/property_decorator.mmd | 7 ++++ .../property_decorator/property_decorator.py | 41 +++++++++++++++++++ 6 files changed, 48 insertions(+), 46 deletions(-) delete mode 100644 tests/pyreverse/functional/class_diagrams/property_decorator/annotated.mmd delete mode 100644 tests/pyreverse/functional/class_diagrams/property_decorator/annotated.py delete mode 100644 tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.mmd delete mode 100644 tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.py create mode 100644 tests/pyreverse/functional/class_diagrams/property_decorator/property_decorator.mmd create mode 100644 tests/pyreverse/functional/class_diagrams/property_decorator/property_decorator.py diff --git a/tests/pyreverse/functional/class_diagrams/property_decorator/annotated.mmd b/tests/pyreverse/functional/class_diagrams/property_decorator/annotated.mmd deleted file mode 100644 index 40cc5e3527..0000000000 --- a/tests/pyreverse/functional/class_diagrams/property_decorator/annotated.mmd +++ /dev/null @@ -1,4 +0,0 @@ -classDiagram - class PropertyTest { - x : int - } diff --git a/tests/pyreverse/functional/class_diagrams/property_decorator/annotated.py b/tests/pyreverse/functional/class_diagrams/property_decorator/annotated.py deleted file mode 100644 index 059ff2a691..0000000000 --- a/tests/pyreverse/functional/class_diagrams/property_decorator/annotated.py +++ /dev/null @@ -1,19 +0,0 @@ -class PropertyTest: - """This is a test class for property decorators with annotated return type""" - def __init__(self): - self._x = 0 - - @property - def x(self) -> int: - """This is a getter for x""" - return self._x - - @x.setter - def x(self, value): - """This is a setter for x""" - self._x = value - - @x.deleter - def x(self): - """This is a deleter for x""" - del self._x diff --git a/tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.mmd b/tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.mmd deleted file mode 100644 index bbb2994e98..0000000000 --- a/tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.mmd +++ /dev/null @@ -1,4 +0,0 @@ -classDiagram - class PropertyTest { - x - } diff --git a/tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.py b/tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.py deleted file mode 100644 index c6582e0a35..0000000000 --- a/tests/pyreverse/functional/class_diagrams/property_decorator/not_annotated.py +++ /dev/null @@ -1,19 +0,0 @@ -class PropertyTest: - """This is a test class for property decorators with annotated return type""" - def __init__(self): - self._x = 0 - - @property - def x(self): - """This is a getter for x""" - return self._x - - @x.setter - def x(self, value): - """This is a setter for x""" - self._x = value - - @x.deleter - def x(self): - """This is a deleter for x""" - del self._x diff --git a/tests/pyreverse/functional/class_diagrams/property_decorator/property_decorator.mmd b/tests/pyreverse/functional/class_diagrams/property_decorator/property_decorator.mmd new file mode 100644 index 0000000000..af55f86849 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/property_decorator/property_decorator.mmd @@ -0,0 +1,7 @@ +classDiagram + class AnnotatedPropertyTest { + x : int + } + class NonAnnotatedPropertyTest { + x + } diff --git a/tests/pyreverse/functional/class_diagrams/property_decorator/property_decorator.py b/tests/pyreverse/functional/class_diagrams/property_decorator/property_decorator.py new file mode 100644 index 0000000000..7a87e6b5b9 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/property_decorator/property_decorator.py @@ -0,0 +1,41 @@ +# property_decorator.py +class AnnotatedPropertyTest: + """Test class for property decorators with annotated return type""" + def __init__(self): + self._x = 0 + + @property + def x(self) -> int: + """This is a getter for x""" + return self._x + + @x.setter + def x(self, value): + """This is a setter for x""" + self._x = value + + @x.deleter + def x(self): + """This is a deleter for x""" + del self._x + + +class NonAnnotatedPropertyTest: + """Test class for property decorators without annotated return type""" + def __init__(self): + self._x = 0 + + @property + def x(self): + """This is a getter for x""" + return self._x + + @x.setter + def x(self, value): + """This is a setter for x""" + self._x = value + + @x.deleter + def x(self): + """This is a deleter for x""" + del self._x From 5d558e91495fcc209b65a6394ad5c9565cade72c Mon Sep 17 00:00:00 2001 From: Julfried Date: Mon, 10 Mar 2025 10:22:48 +0100 Subject: [PATCH 7/9] Add newsfragment --- doc/whatsnew/fragments/10057.feature.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 doc/whatsnew/fragments/10057.feature.rst diff --git a/doc/whatsnew/fragments/10057.feature.rst b/doc/whatsnew/fragments/10057.feature.rst new file mode 100644 index 0000000000..1b6256dc7c --- /dev/null +++ b/doc/whatsnew/fragments/10057.feature.rst @@ -0,0 +1,4 @@ +Enhanced support for @property decorator in pyreverse to correctly display return types of annotated properties when generating class diagrams. + +Closes #9212 +Closes #7644 From e9a3af0a863a23c0651fb699b23492bab65688b0 Mon Sep 17 00:00:00 2001 From: Julfried Date: Mon, 10 Mar 2025 10:26:21 +0100 Subject: [PATCH 8/9] Fix newsfragment --- doc/whatsnew/fragments/10057.feature.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/whatsnew/fragments/10057.feature.rst b/doc/whatsnew/fragments/10057.feature.rst index 1b6256dc7c..41fd94e5b7 100644 --- a/doc/whatsnew/fragments/10057.feature.rst +++ b/doc/whatsnew/fragments/10057.feature.rst @@ -1,4 +1,3 @@ Enhanced support for @property decorator in pyreverse to correctly display return types of annotated properties when generating class diagrams. -Closes #9212 -Closes #7644 +Closes #10057 From 52880d382d19900e3dc8ff77f538338d7d914c41 Mon Sep 17 00:00:00 2001 From: Julfried Date: Mon, 10 Mar 2025 10:36:04 +0100 Subject: [PATCH 9/9] Fix file name of newsframent --- doc/whatsnew/fragments/{10057.feature.rst => 10057.feature} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename doc/whatsnew/fragments/{10057.feature.rst => 10057.feature} (100%) diff --git a/doc/whatsnew/fragments/10057.feature.rst b/doc/whatsnew/fragments/10057.feature similarity index 100% rename from doc/whatsnew/fragments/10057.feature.rst rename to doc/whatsnew/fragments/10057.feature