From 6d4c48754d23e902edcaba1295fe500ae33c5ce6 Mon Sep 17 00:00:00 2001 From: Julfried Date: Sun, 18 May 2025 23:36:44 +0200 Subject: [PATCH 01/39] Correct the test output to follow UML semantics --- .../class_diagrams/aggregation/fields.mmd | 16 ++++++++-------- .../class_diagrams/aggregation/fields.py | 10 +++++----- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd b/tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd index 9901b175c8..245c62cb61 100644 --- a/tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd +++ b/tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd @@ -1,23 +1,23 @@ classDiagram class A { - x + x : P } class B { - x + x : P } class C { - x + x : P } class D { - x + x : P } class E { - x + x : P } class P { } - P --* A : x - P --* C : x + P --o A : x + P --o B : x + P --o C : x P --* D : x P --* E : x - P --o B : x diff --git a/tests/pyreverse/functional/class_diagrams/aggregation/fields.py b/tests/pyreverse/functional/class_diagrams/aggregation/fields.py index a2afb89913..7e6e8597a1 100644 --- a/tests/pyreverse/functional/class_diagrams/aggregation/fields.py +++ b/tests/pyreverse/functional/class_diagrams/aggregation/fields.py @@ -4,24 +4,24 @@ class P: pass class A: - x: P + x: P # can't tell, so default to aggregation class B: def __init__(self, x: P): - self.x = x + self.x = x # not instantiated, so aggregation class C: x: P def __init__(self, x: P): - self.x = x + self.x = x # not instantiated, so aggregation class D: x: P def __init__(self): - self.x = P() + self.x = P() # instantiated, so composition class E: def __init__(self): - self.x = P() + self.x = P() # instantiated, so composition From 54fd26b9a767b40b357ba229056faafcd002eb0a Mon Sep 17 00:00:00 2001 From: Julfried Date: Mon, 19 May 2025 00:16:47 +0200 Subject: [PATCH 02/39] Introduce composition --- pylint/pyreverse/diagrams.py | 11 +++++-- pylint/pyreverse/inspector.py | 56 ++++++++++++++++++++++++++++++----- pylint/pyreverse/printer.py | 1 + pylint/pyreverse/writer.py | 8 +++++ 4 files changed, 66 insertions(+), 10 deletions(-) diff --git a/pylint/pyreverse/diagrams.py b/pylint/pyreverse/diagrams.py index a4fb8ce130..7746c34630 100644 --- a/pylint/pyreverse/diagrams.py +++ b/pylint/pyreverse/diagrams.py @@ -234,7 +234,14 @@ def extract_relationships(self) -> None: except KeyError: continue - # associations & aggregations links + # Composition links + for name, values in list(node.compositions_type.items()): + for value in values: + self.assign_association_relationship( + value, obj, name, "composition" + ) + + # Aggregation links for name, values in list(node.aggregations_type.items()): for value in values: if not self.show_attr(name): @@ -244,8 +251,8 @@ def extract_relationships(self) -> None: value, obj, name, "aggregation" ) + # Association links associations = node.associations_type.copy() - for name, values in node.locals_type.items(): if name not in associations: associations[name] = values diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index 8e69e94470..f168e92f52 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -122,8 +122,14 @@ def __init__(self, project: Project, tag: bool = False) -> None: self.tag = tag # visited project self.project = project - self.associations_handler = AggregationsHandler() - self.associations_handler.set_next(OtherAssociationsHandler()) + + # Chain: Composition → Aggregation → Association + self.associations_handler = CompositionsHandler() + aggregation_handler = AggregationsHandler() + association_handler = AssociationsHandler() + + self.associations_handler.set_next(aggregation_handler) + aggregation_handler.set_next(association_handler) def visit_project(self, node: Project) -> None: """Visit a pyreverse.utils.Project node. @@ -167,6 +173,7 @@ def visit_classdef(self, node: nodes.ClassDef) -> None: specializations.append(node) baseobj.specializations = specializations # resolve instance attributes + node.compositions_type = collections.defaultdict(list) node.instance_attrs_type = collections.defaultdict(list) node.aggregations_type = collections.defaultdict(list) node.associations_type = collections.defaultdict(list) @@ -327,16 +334,39 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: self._next_handler.handle(node, parent) +class CompositionsHandler(AbstractAssociationHandler): + """Handle composition relationships where parent creates child objects.""" + + def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: + if not isinstance(node.parent, (nodes.AnnAssign, nodes.Assign)): + super().handle(node, parent) + return + + value = node.parent.value + + # Composition: parent creates child (self.x = P()) + if isinstance(value, nodes.Call): + current = set(parent.compositions_type[node.attrname]) + parent.compositions_type[node.attrname] = list( + current | utils.infer_node(node) + ) + return + + # Not a composition, pass to next handler + super().handle(node, parent) + + class AggregationsHandler(AbstractAssociationHandler): + """Handle aggregation relationships where parent receives child objects.""" + def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: - # Check if we're not in an assignment context if not isinstance(node.parent, (nodes.AnnAssign, nodes.Assign)): super().handle(node, parent) return value = node.parent.value - # Handle direct name assignments + # Aggregation: parent receives child (self.x = x) if isinstance(value, astroid.node_classes.Name): current = set(parent.aggregations_type[node.attrname]) parent.aggregations_type[node.attrname] = list( @@ -344,11 +374,10 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: ) return - # Handle comprehensions + # Aggregation: comprehensions (self.x = [P() for ...]) if isinstance( value, (nodes.ListComp, nodes.DictComp, nodes.SetComp, nodes.GeneratorExp) ): - # Determine the type of the element in the comprehension if isinstance(value, nodes.DictComp): element_type = safe_infer(value.value) else: @@ -358,12 +387,23 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: parent.aggregations_type[node.attrname] = list(current | {element_type}) return - # Fallback to parent handler + # Type annotation only (x: P) defaults to aggregation + if isinstance(node.parent, nodes.AnnAssign) and node.parent.value is None: + current = set(parent.aggregations_type[node.attrname]) + parent.aggregations_type[node.attrname] = list( + current | utils.infer_node(node) + ) + return + + # Not an aggregation, pass to next handler super().handle(node, parent) -class OtherAssociationsHandler(AbstractAssociationHandler): +class AssociationsHandler(AbstractAssociationHandler): + """Handle regular association relationships.""" + def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: + # Everything else is a regular association current = set(parent.associations_type[node.attrname]) parent.associations_type[node.attrname] = list(current | utils.infer_node(node)) diff --git a/pylint/pyreverse/printer.py b/pylint/pyreverse/printer.py index caa7917ca0..3ec1804897 100644 --- a/pylint/pyreverse/printer.py +++ b/pylint/pyreverse/printer.py @@ -22,6 +22,7 @@ class NodeType(Enum): class EdgeType(Enum): INHERITS = "inherits" + COMPOSITION = "composition" ASSOCIATION = "association" AGGREGATION = "aggregation" USES = "uses" diff --git a/pylint/pyreverse/writer.py b/pylint/pyreverse/writer.py index e822f67096..28fb7ea095 100644 --- a/pylint/pyreverse/writer.py +++ b/pylint/pyreverse/writer.py @@ -146,6 +146,14 @@ def write_classes(self, diagram: ClassDiagram) -> None: label=rel.name, type_=EdgeType.ASSOCIATION, ) + # generate compositions + for rel in diagram.get_relationships("composition"): + self.printer.emit_edge( + rel.from_object.fig_id, + rel.to_object.fig_id, + label=rel.name, + type_=EdgeType.COMPOSITION, + ) # generate aggregations for rel in diagram.get_relationships("aggregation"): if rel.to_object.fig_id in associations[rel.from_object.fig_id]: From 19ae4248f653854bb7eaf7d6f82ef054456bb4f8 Mon Sep 17 00:00:00 2001 From: Julfried Date: Mon, 19 May 2025 00:24:06 +0200 Subject: [PATCH 03/39] Update all the printers to emit the right arrow types --- pylint/pyreverse/dot_printer.py | 8 +++++++- pylint/pyreverse/mermaidjs_printer.py | 3 ++- pylint/pyreverse/plantuml_printer.py | 3 ++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/pylint/pyreverse/dot_printer.py b/pylint/pyreverse/dot_printer.py index 4baed6c3c2..2a6ddd8888 100644 --- a/pylint/pyreverse/dot_printer.py +++ b/pylint/pyreverse/dot_printer.py @@ -30,12 +30,18 @@ class HTMLLabels(Enum): # pylint: disable-next=consider-using-namedtuple-or-dataclass ARROWS: dict[EdgeType, dict[str, str]] = { EdgeType.INHERITS: {"arrowtail": "none", "arrowhead": "empty"}, - EdgeType.ASSOCIATION: { + EdgeType.COMPOSITION: { "fontcolor": "green", "arrowtail": "none", "arrowhead": "diamond", "style": "solid", }, + EdgeType.ASSOCIATION: { + "fontcolor": "green", + "arrowtail": "none", + "arrowhead": "none", + "style": "solid", + }, EdgeType.AGGREGATION: { "fontcolor": "green", "arrowtail": "none", diff --git a/pylint/pyreverse/mermaidjs_printer.py b/pylint/pyreverse/mermaidjs_printer.py index 0f1ebd04f0..45ad91f763 100644 --- a/pylint/pyreverse/mermaidjs_printer.py +++ b/pylint/pyreverse/mermaidjs_printer.py @@ -21,7 +21,8 @@ class MermaidJSPrinter(Printer): } ARROWS: dict[EdgeType, str] = { EdgeType.INHERITS: "--|>", - EdgeType.ASSOCIATION: "--*", + EdgeType.COMPOSITION: "--*", + EdgeType.ASSOCIATION: "-->", EdgeType.AGGREGATION: "--o", EdgeType.USES: "-->", EdgeType.TYPE_DEPENDENCY: "..>", diff --git a/pylint/pyreverse/plantuml_printer.py b/pylint/pyreverse/plantuml_printer.py index 379d57a4c6..98013224c4 100644 --- a/pylint/pyreverse/plantuml_printer.py +++ b/pylint/pyreverse/plantuml_printer.py @@ -21,7 +21,8 @@ class PlantUmlPrinter(Printer): } ARROWS: dict[EdgeType, str] = { EdgeType.INHERITS: "--|>", - EdgeType.ASSOCIATION: "--*", + EdgeType.ASSOCIATION: "-->", + EdgeType.COMPOSITION: "--*", EdgeType.AGGREGATION: "--o", EdgeType.USES: "-->", EdgeType.TYPE_DEPENDENCY: "..>", From 9f48c879752b58233c2235d7fd84a5d3909b8735 Mon Sep 17 00:00:00 2001 From: Julfried Date: Mon, 19 May 2025 00:46:44 +0200 Subject: [PATCH 04/39] Update docstring --- pylint/pyreverse/inspector.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index f168e92f52..ea8723998d 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -113,6 +113,9 @@ class Linker(IdGeneratorMixIn, utils.LocalsVisitor): * aggregations_type as instance_attrs_type but for aggregations relationships + + * compositions_type + as instance_attrs_type but for compositions relationships """ def __init__(self, project: Project, tag: bool = False) -> None: From a561cee2b59ed42df1f761ebf6dcb2d71b930b68 Mon Sep 17 00:00:00 2001 From: Julfried Date: Mon, 19 May 2025 00:51:34 +0200 Subject: [PATCH 05/39] Remove type annotations from test output --- .../functional/class_diagrams/aggregation/fields.mmd | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd b/tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd index 245c62cb61..1935ab8a90 100644 --- a/tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd +++ b/tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd @@ -1,18 +1,18 @@ classDiagram class A { - x : P + x } class B { - x : P + x } class C { - x : P + x } class D { - x : P + x } class E { - x : P + x } class P { } From b52676474483e2571397a9f50ef9ed5ac0966d12 Mon Sep 17 00:00:00 2001 From: Julfried Date: Mon, 19 May 2025 01:06:34 +0200 Subject: [PATCH 06/39] Avoid processing duplicate relationships --- pylint/pyreverse/diagrams.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pylint/pyreverse/diagrams.py b/pylint/pyreverse/diagrams.py index 7746c34630..93e5896b36 100644 --- a/pylint/pyreverse/diagrams.py +++ b/pylint/pyreverse/diagrams.py @@ -226,6 +226,7 @@ def extract_relationships(self) -> None: obj.attrs = self.get_attrs(node) obj.methods = self.get_methods(node) obj.shape = "class" + # inheritance link for par_node in node.ancestors(recurs=False): try: @@ -234,12 +235,16 @@ def extract_relationships(self) -> None: except KeyError: continue + # Track processed attributes to avoid duplicates + processed_attrs = set() + # Composition links for name, values in list(node.compositions_type.items()): for value in values: self.assign_association_relationship( value, obj, name, "composition" ) + processed_attrs.add(name) # Aggregation links for name, values in list(node.aggregations_type.items()): From d5938bf8cf2a957549a6cf51b22817ecdefa755b Mon Sep 17 00:00:00 2001 From: Julfried Date: Mon, 19 May 2025 01:22:35 +0200 Subject: [PATCH 07/39] Update expected files again -> defaults to association --- .../functional/class_diagrams/aggregation/fields.mmd | 6 +++--- .../functional/class_diagrams/aggregation/fields.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd b/tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd index 1935ab8a90..96e0defda1 100644 --- a/tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd +++ b/tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd @@ -16,8 +16,8 @@ classDiagram } class P { } - P --o A : x - P --o B : x - P --o C : x + P --> A : x P --* D : x P --* E : x + P --o B : x + P --o C : x diff --git a/tests/pyreverse/functional/class_diagrams/aggregation/fields.py b/tests/pyreverse/functional/class_diagrams/aggregation/fields.py index 7e6e8597a1..dd812c5b7b 100644 --- a/tests/pyreverse/functional/class_diagrams/aggregation/fields.py +++ b/tests/pyreverse/functional/class_diagrams/aggregation/fields.py @@ -4,7 +4,7 @@ class P: pass class A: - x: P # can't tell, so default to aggregation + x: P # just type hint, no ownership, soassociation class B: def __init__(self, x: P): From ff9d93dea883b6658a35002fa76bd0c6b05d0c74 Mon Sep 17 00:00:00 2001 From: Julfried Date: Mon, 19 May 2025 01:49:15 +0200 Subject: [PATCH 08/39] change arrowhead for dot language for association relationsships --- pylint/pyreverse/dot_printer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/pyreverse/dot_printer.py b/pylint/pyreverse/dot_printer.py index 2a6ddd8888..b4aba94ea5 100644 --- a/pylint/pyreverse/dot_printer.py +++ b/pylint/pyreverse/dot_printer.py @@ -39,7 +39,7 @@ class HTMLLabels(Enum): EdgeType.ASSOCIATION: { "fontcolor": "green", "arrowtail": "none", - "arrowhead": "none", + "arrowhead": "normal", "style": "solid", }, EdgeType.AGGREGATION: { From ec949f27f620c3c6c6f96c5b76bba714cf4e08e9 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sat, 14 Jun 2025 10:01:44 +0200 Subject: [PATCH 09/39] Update comment --- tests/pyreverse/functional/class_diagrams/aggregation/fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pyreverse/functional/class_diagrams/aggregation/fields.py b/tests/pyreverse/functional/class_diagrams/aggregation/fields.py index dd812c5b7b..cbf334b298 100644 --- a/tests/pyreverse/functional/class_diagrams/aggregation/fields.py +++ b/tests/pyreverse/functional/class_diagrams/aggregation/fields.py @@ -4,7 +4,7 @@ class P: pass class A: - x: P # just type hint, no ownership, soassociation + x: P # just type hint, no ownership, so association class B: def __init__(self, x: P): From 74ebf7fc6b4a9817e49cf06bd4eb466451c37e90 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sat, 28 Jun 2025 16:47:55 +0200 Subject: [PATCH 10/39] Update comments in test file --- .../functional/class_diagrams/aggregation/fields.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/pyreverse/functional/class_diagrams/aggregation/fields.py b/tests/pyreverse/functional/class_diagrams/aggregation/fields.py index cbf334b298..9e35885a9e 100644 --- a/tests/pyreverse/functional/class_diagrams/aggregation/fields.py +++ b/tests/pyreverse/functional/class_diagrams/aggregation/fields.py @@ -4,24 +4,22 @@ class P: pass class A: - x: P # just type hint, no ownership, so association + x: P # just type hint, no ownership → Association class B: def __init__(self, x: P): - self.x = x # not instantiated, so aggregation + self.x = x # receives object, not created → Aggregation class C: x: P - def __init__(self, x: P): - self.x = x # not instantiated, so aggregation + self.x = x # receives object, not created → Aggregation class D: x: P - def __init__(self): - self.x = P() # instantiated, so composition + self.x = P() # creates object → Composition class E: def __init__(self): - self.x = P() # instantiated, so composition + self.x = P() # creates object → Composition From e7bd7ca374dbfc84eb31a71f9436498b593b92ce Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sat, 28 Jun 2025 16:49:22 +0200 Subject: [PATCH 11/39] rename test folders for better clarity --- .../{aggregation => associations}/comprehensions.mmd | 0 .../{aggregation => associations}/comprehensions.py | 0 .../class_diagrams/{aggregation => associations}/fields.mmd | 0 .../class_diagrams/{aggregation => associations}/fields.py | 0 .../{aggregation_filtering => associations_filtering}/all.mmd | 0 .../{aggregation_filtering => associations_filtering}/all.py | 0 .../{aggregation_filtering => associations_filtering}/all.rc | 0 .../{aggregation_filtering => associations_filtering}/other.mmd | 0 .../{aggregation_filtering => associations_filtering}/other.py | 0 .../{aggregation_filtering => associations_filtering}/other.rc | 0 .../pub_only.mmd | 0 .../{aggregation_filtering => associations_filtering}/pub_only.py | 0 .../{aggregation_filtering => associations_filtering}/pub_only.rc | 0 .../{aggregation_filtering => associations_filtering}/special.mmd | 0 .../{aggregation_filtering => associations_filtering}/special.py | 0 .../{aggregation_filtering => associations_filtering}/special.rc | 0 16 files changed, 0 insertions(+), 0 deletions(-) rename tests/pyreverse/functional/class_diagrams/{aggregation => associations}/comprehensions.mmd (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation => associations}/comprehensions.py (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation => associations}/fields.mmd (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation => associations}/fields.py (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation_filtering => associations_filtering}/all.mmd (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation_filtering => associations_filtering}/all.py (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation_filtering => associations_filtering}/all.rc (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation_filtering => associations_filtering}/other.mmd (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation_filtering => associations_filtering}/other.py (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation_filtering => associations_filtering}/other.rc (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation_filtering => associations_filtering}/pub_only.mmd (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation_filtering => associations_filtering}/pub_only.py (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation_filtering => associations_filtering}/pub_only.rc (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation_filtering => associations_filtering}/special.mmd (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation_filtering => associations_filtering}/special.py (100%) rename tests/pyreverse/functional/class_diagrams/{aggregation_filtering => associations_filtering}/special.rc (100%) diff --git a/tests/pyreverse/functional/class_diagrams/aggregation/comprehensions.mmd b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation/comprehensions.mmd rename to tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd diff --git a/tests/pyreverse/functional/class_diagrams/aggregation/comprehensions.py b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation/comprehensions.py rename to tests/pyreverse/functional/class_diagrams/associations/comprehensions.py diff --git a/tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd b/tests/pyreverse/functional/class_diagrams/associations/fields.mmd similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation/fields.mmd rename to tests/pyreverse/functional/class_diagrams/associations/fields.mmd diff --git a/tests/pyreverse/functional/class_diagrams/aggregation/fields.py b/tests/pyreverse/functional/class_diagrams/associations/fields.py similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation/fields.py rename to tests/pyreverse/functional/class_diagrams/associations/fields.py diff --git a/tests/pyreverse/functional/class_diagrams/aggregation_filtering/all.mmd b/tests/pyreverse/functional/class_diagrams/associations_filtering/all.mmd similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation_filtering/all.mmd rename to tests/pyreverse/functional/class_diagrams/associations_filtering/all.mmd diff --git a/tests/pyreverse/functional/class_diagrams/aggregation_filtering/all.py b/tests/pyreverse/functional/class_diagrams/associations_filtering/all.py similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation_filtering/all.py rename to tests/pyreverse/functional/class_diagrams/associations_filtering/all.py diff --git a/tests/pyreverse/functional/class_diagrams/aggregation_filtering/all.rc b/tests/pyreverse/functional/class_diagrams/associations_filtering/all.rc similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation_filtering/all.rc rename to tests/pyreverse/functional/class_diagrams/associations_filtering/all.rc diff --git a/tests/pyreverse/functional/class_diagrams/aggregation_filtering/other.mmd b/tests/pyreverse/functional/class_diagrams/associations_filtering/other.mmd similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation_filtering/other.mmd rename to tests/pyreverse/functional/class_diagrams/associations_filtering/other.mmd diff --git a/tests/pyreverse/functional/class_diagrams/aggregation_filtering/other.py b/tests/pyreverse/functional/class_diagrams/associations_filtering/other.py similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation_filtering/other.py rename to tests/pyreverse/functional/class_diagrams/associations_filtering/other.py diff --git a/tests/pyreverse/functional/class_diagrams/aggregation_filtering/other.rc b/tests/pyreverse/functional/class_diagrams/associations_filtering/other.rc similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation_filtering/other.rc rename to tests/pyreverse/functional/class_diagrams/associations_filtering/other.rc diff --git a/tests/pyreverse/functional/class_diagrams/aggregation_filtering/pub_only.mmd b/tests/pyreverse/functional/class_diagrams/associations_filtering/pub_only.mmd similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation_filtering/pub_only.mmd rename to tests/pyreverse/functional/class_diagrams/associations_filtering/pub_only.mmd diff --git a/tests/pyreverse/functional/class_diagrams/aggregation_filtering/pub_only.py b/tests/pyreverse/functional/class_diagrams/associations_filtering/pub_only.py similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation_filtering/pub_only.py rename to tests/pyreverse/functional/class_diagrams/associations_filtering/pub_only.py diff --git a/tests/pyreverse/functional/class_diagrams/aggregation_filtering/pub_only.rc b/tests/pyreverse/functional/class_diagrams/associations_filtering/pub_only.rc similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation_filtering/pub_only.rc rename to tests/pyreverse/functional/class_diagrams/associations_filtering/pub_only.rc diff --git a/tests/pyreverse/functional/class_diagrams/aggregation_filtering/special.mmd b/tests/pyreverse/functional/class_diagrams/associations_filtering/special.mmd similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation_filtering/special.mmd rename to tests/pyreverse/functional/class_diagrams/associations_filtering/special.mmd diff --git a/tests/pyreverse/functional/class_diagrams/aggregation_filtering/special.py b/tests/pyreverse/functional/class_diagrams/associations_filtering/special.py similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation_filtering/special.py rename to tests/pyreverse/functional/class_diagrams/associations_filtering/special.py diff --git a/tests/pyreverse/functional/class_diagrams/aggregation_filtering/special.rc b/tests/pyreverse/functional/class_diagrams/associations_filtering/special.rc similarity index 100% rename from tests/pyreverse/functional/class_diagrams/aggregation_filtering/special.rc rename to tests/pyreverse/functional/class_diagrams/associations_filtering/special.rc From 421654591985b0cbd60fad0cc038bed5b3c16152 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sat, 28 Jun 2025 18:07:39 +0200 Subject: [PATCH 12/39] Enhance composition and aggregation handling in AST node processing so that the differentation between aggregation and composition is correct according to UMLEnhance composition and aggregation handling in AST node processing so that the differentation between aggregation and composition is correct according to UML --- pylint/pyreverse/inspector.py | 61 +++++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 17 deletions(-) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index ea8723998d..2dd1cad4be 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -347,7 +347,7 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: value = node.parent.value - # Composition: parent creates child (self.x = P()) + # Composition: direct object creation (self.x = P()) if isinstance(value, nodes.Call): current = set(parent.compositions_type[node.attrname]) parent.compositions_type[node.attrname] = list( @@ -355,6 +355,23 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: ) return + # Composition: comprehensions with object creation (self.x = [P() for ...]) + if isinstance( + value, (nodes.ListComp, nodes.DictComp, nodes.SetComp, nodes.GeneratorExp) + ): + if isinstance(value, nodes.DictComp): + element = value.value + else: + element = value.elt + + # If the element is a Call (object creation), it's composition + if isinstance(element, nodes.Call): + current = set(parent.compositions_type[node.attrname]) + parent.compositions_type[node.attrname] = list( + current | utils.infer_node(node) + ) + return + # Not a composition, pass to next handler super().handle(node, parent) @@ -377,26 +394,27 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: ) return - # Aggregation: comprehensions (self.x = [P() for ...]) + # Aggregation: comprehensions without object creation (self.x = [existing_obj for ...]) if isinstance( value, (nodes.ListComp, nodes.DictComp, nodes.SetComp, nodes.GeneratorExp) ): if isinstance(value, nodes.DictComp): - element_type = safe_infer(value.value) + element = value.value else: - element_type = safe_infer(value.elt) - if element_type: - current = set(parent.aggregations_type[node.attrname]) - parent.aggregations_type[node.attrname] = list(current | {element_type}) - return - - # Type annotation only (x: P) defaults to aggregation - if isinstance(node.parent, nodes.AnnAssign) and node.parent.value is None: - current = set(parent.aggregations_type[node.attrname]) - parent.aggregations_type[node.attrname] = list( - current | utils.infer_node(node) - ) - return + element = value.elt + + # If the element is NOT a Call (no object creation), it's aggregation + if not isinstance(element, nodes.Call): + if isinstance(value, nodes.DictComp): + element_type = safe_infer(value.value) + else: + element_type = safe_infer(value.elt) + if element_type: + current = set(parent.aggregations_type[node.attrname]) + parent.aggregations_type[node.attrname] = list( + current | {element_type} + ) + return # Not an aggregation, pass to next handler super().handle(node, parent) @@ -406,7 +424,16 @@ class AssociationsHandler(AbstractAssociationHandler): """Handle regular association relationships.""" def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: - # Everything else is a regular association + # Type annotation only (x: P) -> Association + # BUT only if there's no actual assignment (to avoid duplicates) + if isinstance(node.parent, nodes.AnnAssign) and node.parent.value is None: + current = set(parent.associations_type[node.attrname]) + parent.associations_type[node.attrname] = list( + current | utils.infer_node(node) + ) + return + + # Everything else is also association (fallback) current = set(parent.associations_type[node.attrname]) parent.associations_type[node.attrname] = list(current | utils.infer_node(node)) From 6ee8117cf85111da4cc27db8bf2b2f2daae52516 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sat, 28 Jun 2025 18:08:35 +0200 Subject: [PATCH 13/39] Update relationship extraction to avoid duplicate entries --- pylint/pyreverse/diagrams.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/pylint/pyreverse/diagrams.py b/pylint/pyreverse/diagrams.py index 93e5896b36..ad074b2b77 100644 --- a/pylint/pyreverse/diagrams.py +++ b/pylint/pyreverse/diagrams.py @@ -238,35 +238,38 @@ def extract_relationships(self) -> None: # Track processed attributes to avoid duplicates processed_attrs = set() - # Composition links + # Process in priority order: Composition > Aggregation > Association + + # 1. Composition links (highest priority) for name, values in list(node.compositions_type.items()): + if not self.show_attr(name): + continue for value in values: self.assign_association_relationship( value, obj, name, "composition" ) processed_attrs.add(name) - # Aggregation links + # 2. Aggregation links (medium priority) for name, values in list(node.aggregations_type.items()): + if not self.show_attr(name) or name in processed_attrs: + continue for value in values: - if not self.show_attr(name): - continue - self.assign_association_relationship( value, obj, name, "aggregation" ) + processed_attrs.add(name) - # Association links + # 3. Association links (lowest priority) associations = node.associations_type.copy() for name, values in node.locals_type.items(): if name not in associations: associations[name] = values for name, values in associations.items(): + if not self.show_attr(name) or name in processed_attrs: + continue for value in values: - if not self.show_attr(name): - continue - self.assign_association_relationship( value, obj, name, "association" ) From 96776662e7ca475ed731d8e38e7d4db2a5be049b Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sat, 28 Jun 2025 18:26:16 +0200 Subject: [PATCH 14/39] Correctly infer the node type for Composition --- pylint/pyreverse/inspector.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index 2dd1cad4be..46844dfc90 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -366,11 +366,13 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: # If the element is a Call (object creation), it's composition if isinstance(element, nodes.Call): - current = set(parent.compositions_type[node.attrname]) - parent.compositions_type[node.attrname] = list( - current | utils.infer_node(node) - ) - return + element_type = safe_infer(element) + if element_type: + current = set(parent.compositions_type[node.attrname]) + parent.compositions_type[node.attrname] = list( + current | {element_type} + ) + return # Not a composition, pass to next handler super().handle(node, parent) From f3e7c23e3a3fec664032fe30eda5f2afe25aea98 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sat, 28 Jun 2025 18:51:39 +0200 Subject: [PATCH 15/39] Update the functional test for comprehensions aswell --- .../associations/comprehensions.mmd | 30 ++++++++++++++--- .../associations/comprehensions.py | 32 +++++++++++++++---- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd index 6994d91cbb..d2a35a4f3b 100644 --- a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd +++ b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd @@ -2,13 +2,33 @@ classDiagram class Component { name : str } - class Container { + class AssociationContainer { + components : list[Component] component_dict : dict[int, Component] + components_set : set[Component] + lazy_components : Generator[Component] + } + class AggregationContainer { components : list[Component] + component_dict : dict[str, Component] + components_set : set[Component] + lazy_components : Generator[Component] + } + class CompositionContainer { + components : list[Component] + component_dict : dict[int, Component] components_set : set[Component] lazy_components : Generator[Component] } - Component --o Container : components - Component --o Container : component_dict - Component --o Container : components_set - Component --o Container : lazy_components + Component --> AssociationContainer : components + Component --> AssociationContainer : component_dict + Component --> AssociationContainer : components_set + Component --> AssociationContainer : lazy_components + Component --o AggregationContainer : components + Component --o AggregationContainer : component_dict + Component --o AggregationContainer : components_set + Component --o AggregationContainer : lazy_components + Component --* CompositionContainer : components + Component --* CompositionContainer : component_dict + Component --* CompositionContainer : components_set + Component --* CompositionContainer : lazy_components diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py index 7d83430d87..551a72241d 100644 --- a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py +++ b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py @@ -8,10 +8,30 @@ def __init__(self, name: str): self.name = name -class Container: - """A container class that uses comprehension to create components.""" +class AssociationContainer: + """Type hints only - no ownership.""" + # Association: just type hints, no actual assignment + components: list[Component] + component_dict: dict[int, Component] + components_set: set[Component] + lazy_components: Generator[Component] + + +class AggregationContainer: + """Comprehensions using existing objects - aggregation.""" + def __init__(self, existing_components: list[Component]): + # Aggregation: comprehensions using existing objects (not creating) + self.components: list[Component] = [comp for comp in existing_components] + self.component_dict: dict[str, Component] = {f"key_{i}": comp for i, comp in enumerate(existing_components)} + self.components_set: set[Component] = {comp for comp in existing_components} + self.lazy_components: Generator[Component] = (comp for comp in existing_components) + + +class CompositionContainer: + """Comprehensions creating new objects - composition.""" def __init__(self): - self.components: list[Component] = [Component(f"component_{i}") for i in range(3)] # list - self.component_dict: dict[int, Component] = {i: Component(f"dict_component_{i}") for i in range(2)} # dict - self.components_set: set[Component] = {Component(f"set_component_{i}") for i in range(2)} # set - self.lazy_components: Generator[Component] = (Component(f"lazy_{i}") for i in range(2)) # generator + # Composition: comprehensions creating new objects + self.components: list[Component] = [Component(f"component_{i}") for i in range(3)] + self.component_dict: dict[int, Component] = {i: Component(f"dict_component_{i}") for i in range(2)} + self.components_set: set[Component] = {Component(f"set_component_{i}") for i in range(2)} + self.lazy_components: Generator[Component] = (Component(f"lazy_{i}") for i in range(2)) From 02e2024cad609d39adbee2fede0e13686686b48a Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sat, 28 Jun 2025 19:47:41 +0200 Subject: [PATCH 16/39] Instead of checking not call node check for name node ==> more explicit --- pylint/pyreverse/inspector.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index 46844dfc90..3d8e2bb5a4 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -388,8 +388,8 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: value = node.parent.value - # Aggregation: parent receives child (self.x = x) - if isinstance(value, astroid.node_classes.Name): + # Aggregation: direct assignment (self.x = x) + if isinstance(value, nodes.Name): current = set(parent.aggregations_type[node.attrname]) parent.aggregations_type[node.attrname] = list( current | utils.infer_node(node) @@ -405,8 +405,8 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: else: element = value.elt - # If the element is NOT a Call (no object creation), it's aggregation - if not isinstance(element, nodes.Call): + # If the element is a Name, it means it's an existing object, so it's aggregation + if isinstance(element, nodes.Name): if isinstance(value, nodes.DictComp): element_type = safe_infer(value.value) else: From 2d157d872d9b0ee8a794bb936216e043cb290d95 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sun, 29 Jun 2025 11:29:28 +0200 Subject: [PATCH 17/39] Remove redundant checks in AggregationHandler --- pylint/pyreverse/inspector.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index 3d8e2bb5a4..b5ce5f8c5e 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -407,10 +407,7 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: # If the element is a Name, it means it's an existing object, so it's aggregation if isinstance(element, nodes.Name): - if isinstance(value, nodes.DictComp): - element_type = safe_infer(value.value) - else: - element_type = safe_infer(value.elt) + element_type = safe_infer(element) if element_type: current = set(parent.aggregations_type[node.attrname]) parent.aggregations_type[node.attrname] = list( From 195a6f7e54ecde19786bc10eb821f83c594a479c Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sun, 29 Jun 2025 12:01:59 +0200 Subject: [PATCH 18/39] Add todo note because infering type in Aggregation comprehensions is unreliable for now --- pylint/pyreverse/inspector.py | 1 + .../associations/comprehensions.mmd | 12 +-------- .../associations/comprehensions.py | 26 ++++++------------- 3 files changed, 10 insertions(+), 29 deletions(-) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index b5ce5f8c5e..201c91b9f0 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -397,6 +397,7 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: return # Aggregation: comprehensions without object creation (self.x = [existing_obj for ...]) + # TODO: Currently inferring type of existing_obj is not reliable ==> improve once astroid supports it if isinstance( value, (nodes.ListComp, nodes.DictComp, nodes.SetComp, nodes.GeneratorExp) ): diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd index d2a35a4f3b..198ed7c9c5 100644 --- a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd +++ b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd @@ -8,12 +8,6 @@ classDiagram components_set : set[Component] lazy_components : Generator[Component] } - class AggregationContainer { - components : list[Component] - component_dict : dict[str, Component] - components_set : set[Component] - lazy_components : Generator[Component] - } class CompositionContainer { components : list[Component] component_dict : dict[int, Component] @@ -24,11 +18,7 @@ classDiagram Component --> AssociationContainer : component_dict Component --> AssociationContainer : components_set Component --> AssociationContainer : lazy_components - Component --o AggregationContainer : components - Component --o AggregationContainer : component_dict - Component --o AggregationContainer : components_set - Component --o AggregationContainer : lazy_components Component --* CompositionContainer : components Component --* CompositionContainer : component_dict Component --* CompositionContainer : components_set - Component --* CompositionContainer : lazy_components + Component --* CompositionContainer : lazy_components diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py index 551a72241d..35d90643ad 100644 --- a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py +++ b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py @@ -1,5 +1,6 @@ # Test for https://github.com/pylint-dev/pylint/issues/10236 from collections.abc import Generator +from dataclasses import dataclass class Component: @@ -7,25 +8,14 @@ class Component: def __init__(self, name: str): self.name = name - class AssociationContainer: """Type hints only - no ownership.""" - # Association: just type hints, no actual assignment - components: list[Component] - component_dict: dict[int, Component] - components_set: set[Component] - lazy_components: Generator[Component] - - -class AggregationContainer: - """Comprehensions using existing objects - aggregation.""" - def __init__(self, existing_components: list[Component]): - # Aggregation: comprehensions using existing objects (not creating) - self.components: list[Component] = [comp for comp in existing_components] - self.component_dict: dict[str, Component] = {f"key_{i}": comp for i, comp in enumerate(existing_components)} - self.components_set: set[Component] = {comp for comp in existing_components} - self.lazy_components: Generator[Component] = (comp for comp in existing_components) - + def __init__(self): + # Association: just type hints, no actual assignment + self.components: list[Component] + self.component_dict: dict[int, Component] + self.components_set: set[Component] + self.lazy_components: Generator[Component] class CompositionContainer: """Comprehensions creating new objects - composition.""" @@ -34,4 +24,4 @@ def __init__(self): self.components: list[Component] = [Component(f"component_{i}") for i in range(3)] self.component_dict: dict[int, Component] = {i: Component(f"dict_component_{i}") for i in range(2)} self.components_set: set[Component] = {Component(f"set_component_{i}") for i in range(2)} - self.lazy_components: Generator[Component] = (Component(f"lazy_{i}") for i in range(2)) + self.lazy_components: Generator[Component] = (Component(f"lazy_{i}") for i in range(2)) From 31d93ebbaf37e9530631e3354b888a25b8abc9ec Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sun, 29 Jun 2025 14:54:09 +0200 Subject: [PATCH 19/39] Enhance type resolution in AssociationsHandler and add utility functions for extracting element types and resolving class definitionsEnhance type resolution in AssociationsHandler and add utility functions for extracting element types and resolving class definitions --- pylint/pyreverse/inspector.py | 58 ++++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index 201c91b9f0..b10a06ddbf 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -18,6 +18,7 @@ import astroid from astroid import nodes +from astroid.typing import InferenceResult from pylint import constants from pylint.checkers.utils import safe_infer @@ -427,15 +428,64 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: # Type annotation only (x: P) -> Association # BUT only if there's no actual assignment (to avoid duplicates) if isinstance(node.parent, nodes.AnnAssign) and node.parent.value is None: + inferred_types = utils.infer_node(node) + element_types = extract_element_types(inferred_types) + + # Resolve nodes to actual class definitions + resolved_types = resolve_to_class_def(element_types) + current = set(parent.associations_type[node.attrname]) - parent.associations_type[node.attrname] = list( - current | utils.infer_node(node) - ) + parent.associations_type[node.attrname] = list(current | resolved_types) return # Everything else is also association (fallback) current = set(parent.associations_type[node.attrname]) - parent.associations_type[node.attrname] = list(current | utils.infer_node(node)) + inferred_types = utils.infer_node(node) + element_types = extract_element_types(inferred_types) + + # Resolve Name nodes to actual class definitions + resolved_types = resolve_to_class_def(element_types) + parent.associations_type[node.attrname] = list(current | resolved_types) + + +def resolve_to_class_def(types: set[nodes.NodeNG]) -> set[nodes.ClassDef]: + """Resolve a set of nodes to ClassDef nodes.""" + class_defs = set() + for node in types: + if isinstance(node, nodes.ClassDef): + class_defs.add(node) + elif isinstance(node, nodes.Name): + inferred = safe_infer(node) + if isinstance(inferred, nodes.ClassDef): + class_defs.add(inferred) + return class_defs + + +def extract_element_types(inferred_types: set[InferenceResult]) -> set[nodes.NodeNG]: + """Extract element types in case the inferred type is a container. + + This function checks if the inferred type is a container type (like list, dict, etc.) + and extracts the element type(s) from it. If the inferred type is a direct type (like a class), + it adds that type directly to the set of element types it returns. + """ + element_types = set() + + for inferred_type in inferred_types: + if isinstance(inferred_type, nodes.Subscript): + slice_node = inferred_type.slice + + # Handle both Tuple (dict[K,V]) and single element (list[T]) + elements = ( + slice_node.elts if isinstance(slice_node, nodes.Tuple) else [slice_node] + ) + + for elt in elements: + if isinstance(elt, (nodes.Name, nodes.ClassDef)): + element_types.add(elt) + else: + element_types.add(inferred_type) + + return element_types def project_from_files( From 892c02e6e1c548b113a16ee43353c9eb1d7176b0 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sun, 29 Jun 2025 14:55:28 +0200 Subject: [PATCH 20/39] Fix order so that tests pass --- .../class_diagrams/associations/comprehensions.mmd | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd index 198ed7c9c5..016f2fcb7d 100644 --- a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd +++ b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd @@ -1,16 +1,16 @@ classDiagram - class Component { - name : str - } class AssociationContainer { - components : list[Component] component_dict : dict[int, Component] + components : list[Component] components_set : set[Component] lazy_components : Generator[Component] } + class Component { + name : str + } class CompositionContainer { - components : list[Component] component_dict : dict[int, Component] + components : list[Component] components_set : set[Component] lazy_components : Generator[Component] } From bc8b028c24b13c6367c798f251ce5602260405a5 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sun, 29 Jun 2025 15:37:50 +0200 Subject: [PATCH 21/39] Update the functional test files for attribute annotation, because now pyreverse correctly extracts Dummy as Association --- .../class_diagrams/annotations/attributes_annotation.dot | 1 + .../class_diagrams/annotations/attributes_annotation.mmd | 1 + .../class_diagrams/annotations/attributes_annotation.puml | 1 + 3 files changed, 3 insertions(+) diff --git a/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.dot b/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.dot index 6b0287c4a4..0a74cc3911 100644 --- a/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.dot +++ b/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.dot @@ -3,4 +3,5 @@ rankdir=BT charset="utf-8" "attributes_annotation.Dummy" [color="black", fontcolor="black", label=<{Dummy|
|}>, shape="record", style="solid"]; "attributes_annotation.Dummy2" [color="black", fontcolor="black", label=<{Dummy2|alternative_optional : int \| None
alternative_optional_swapped : None \| int
alternative_union_syntax : str \| int
class_attr : list[Dummy]
optional : Optional[Dummy]
optional_union : Optional[int \| str]
param : str
union : Union[int, str]
|}>, shape="record", style="solid"]; +"attributes_annotation.Dummy" -> "attributes_annotation.Dummy2" [arrowhead="normal", arrowtail="none", fontcolor="green", label="optional", style="solid"]; } diff --git a/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.mmd b/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.mmd index aff946e7a8..e10a62cc61 100644 --- a/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.mmd +++ b/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.mmd @@ -11,3 +11,4 @@ classDiagram param : str union : Union[int, str] } + Dummy --> Dummy2 : optional diff --git a/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.puml b/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.puml index 65bbb3755a..54ca1db05b 100644 --- a/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.puml +++ b/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.puml @@ -12,4 +12,5 @@ class "Dummy2" as attributes_annotation.Dummy2 { param : str union : Union[int, str] } +attributes_annotation.Dummy --> attributes_annotation.Dummy2 : optional @enduml From 36fe5348b20324c1b283283d5481648d5aa2b3e3 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sun, 29 Jun 2025 17:05:50 +0200 Subject: [PATCH 22/39] Use the new utility function for the other handlers aswell --- pylint/pyreverse/inspector.py | 52 +++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index b10a06ddbf..90664407f3 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -350,10 +350,14 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: # Composition: direct object creation (self.x = P()) if isinstance(value, nodes.Call): + inferred_types = utils.infer_node(node) + element_types = extract_element_types(inferred_types) + + # Resolve nodes to actual class definitions + resolved_types = resolve_to_class_def(element_types) + current = set(parent.compositions_type[node.attrname]) - parent.compositions_type[node.attrname] = list( - current | utils.infer_node(node) - ) + parent.compositions_type[node.attrname] = list(current | resolved_types) return # Composition: comprehensions with object creation (self.x = [P() for ...]) @@ -367,13 +371,15 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: # If the element is a Call (object creation), it's composition if isinstance(element, nodes.Call): - element_type = safe_infer(element) - if element_type: - current = set(parent.compositions_type[node.attrname]) - parent.compositions_type[node.attrname] = list( - current | {element_type} - ) - return + inferred_types = utils.infer_node(node) + element_types = extract_element_types(inferred_types) + + # Resolve nodes to actual class definitions + resolved_types = resolve_to_class_def(element_types) + + current = set(parent.compositions_type[node.attrname]) + parent.compositions_type[node.attrname] = list(current | resolved_types) + return # Not a composition, pass to next handler super().handle(node, parent) @@ -391,10 +397,14 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: # Aggregation: direct assignment (self.x = x) if isinstance(value, nodes.Name): + inferred_types = utils.infer_node(node) + element_types = extract_element_types(inferred_types) + + # Resolve nodes to actual class definitions + resolved_types = resolve_to_class_def(element_types) + current = set(parent.aggregations_type[node.attrname]) - parent.aggregations_type[node.attrname] = list( - current | utils.infer_node(node) - ) + parent.aggregations_type[node.attrname] = list(current | resolved_types) return # Aggregation: comprehensions without object creation (self.x = [existing_obj for ...]) @@ -409,13 +419,15 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: # If the element is a Name, it means it's an existing object, so it's aggregation if isinstance(element, nodes.Name): - element_type = safe_infer(element) - if element_type: - current = set(parent.aggregations_type[node.attrname]) - parent.aggregations_type[node.attrname] = list( - current | {element_type} - ) - return + inferred_types = utils.infer_node(node) + element_types = extract_element_types(inferred_types) + + # Resolve nodes to actual class definitions + resolved_types = resolve_to_class_def(element_types) + + current = set(parent.aggregations_type[node.attrname]) + parent.aggregations_type[node.attrname] = list(current | resolved_types) + return # Not an aggregation, pass to next handler super().handle(node, parent) From 780740b2aec09e5036e2376d626a94c57813caad Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sun, 29 Jun 2025 17:24:12 +0200 Subject: [PATCH 23/39] Fix regression that did not correctly detect Composition in fields.py --- pylint/pyreverse/inspector.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index 90664407f3..6b3e77f80c 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -470,6 +470,9 @@ def resolve_to_class_def(types: set[nodes.NodeNG]) -> set[nodes.ClassDef]: inferred = safe_infer(node) if isinstance(inferred, nodes.ClassDef): class_defs.add(inferred) + elif isinstance(node, astroid.Instance): + # Instance of a class -> get the actual class + class_defs.add(node._proxied) return class_defs From aef07caa71a5c3c7f6d5eb0c6d874ccb43a774a7 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sun, 29 Jun 2025 17:25:56 +0200 Subject: [PATCH 24/39] Revert functional test for comprehension ==> this now works with new util functions --- .../class_diagrams/associations/comprehensions.mmd | 10 ++++++++++ .../class_diagrams/associations/comprehensions.py | 9 +++++++++ 2 files changed, 19 insertions(+) diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd index 016f2fcb7d..21d22c7798 100644 --- a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd +++ b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd @@ -1,4 +1,10 @@ classDiagram + class AggregationContainer { + component_dict : dict[str, Component] + components : list[Component] + components_set : set[Component] + lazy_components : Generator[Component] + } class AssociationContainer { component_dict : dict[int, Component] components : list[Component] @@ -22,3 +28,7 @@ classDiagram Component --* CompositionContainer : component_dict Component --* CompositionContainer : components_set Component --* CompositionContainer : lazy_components + Component --o AggregationContainer : components + Component --o AggregationContainer : component_dict + Component --o AggregationContainer : components_set + Component --o AggregationContainer : lazy_components diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py index 35d90643ad..4f088a6a9d 100644 --- a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py +++ b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py @@ -17,6 +17,15 @@ def __init__(self): self.components_set: set[Component] self.lazy_components: Generator[Component] +class AggregationContainer: + """Comprehensions using existing objects - aggregation.""" + def __init__(self, existing_components: list[Component]): + # Aggregation: comprehensions using existing objects (not creating) + self.components: list[Component] = [comp for comp in existing_components] + self.component_dict: dict[str, Component] = {f"key_{i}": comp for i, comp in enumerate(existing_components)} + self.components_set: set[Component] = {comp for comp in existing_components} + self.lazy_components: Generator[Component] = (comp for comp in existing_components) + class CompositionContainer: """Comprehensions creating new objects - composition.""" def __init__(self): From 87d308f17f135f3688e3df5540de6ad1122bba75 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sun, 29 Jun 2025 17:48:12 +0200 Subject: [PATCH 25/39] Add correct arrow type for dot printer --- pylint/pyreverse/dot_printer.py | 2 +- .../class_diagrams/annotations/attributes_annotation.dot | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pylint/pyreverse/dot_printer.py b/pylint/pyreverse/dot_printer.py index b4aba94ea5..331a61431e 100644 --- a/pylint/pyreverse/dot_printer.py +++ b/pylint/pyreverse/dot_printer.py @@ -39,7 +39,7 @@ class HTMLLabels(Enum): EdgeType.ASSOCIATION: { "fontcolor": "green", "arrowtail": "none", - "arrowhead": "normal", + "arrowhead": "vee", "style": "solid", }, EdgeType.AGGREGATION: { diff --git a/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.dot b/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.dot index 0a74cc3911..aeb167b0f0 100644 --- a/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.dot +++ b/tests/pyreverse/functional/class_diagrams/annotations/attributes_annotation.dot @@ -3,5 +3,5 @@ rankdir=BT charset="utf-8" "attributes_annotation.Dummy" [color="black", fontcolor="black", label=<{Dummy|
|}>, shape="record", style="solid"]; "attributes_annotation.Dummy2" [color="black", fontcolor="black", label=<{Dummy2|alternative_optional : int \| None
alternative_optional_swapped : None \| int
alternative_union_syntax : str \| int
class_attr : list[Dummy]
optional : Optional[Dummy]
optional_union : Optional[int \| str]
param : str
union : Union[int, str]
|}>, shape="record", style="solid"]; -"attributes_annotation.Dummy" -> "attributes_annotation.Dummy2" [arrowhead="normal", arrowtail="none", fontcolor="green", label="optional", style="solid"]; +"attributes_annotation.Dummy" -> "attributes_annotation.Dummy2" [arrowhead="vee", arrowtail="none", fontcolor="green", label="optional", style="solid"]; } From 8f59271ab3a4ff1e5139bf2aaca84a58672e6fc3 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sun, 29 Jun 2025 18:03:02 +0200 Subject: [PATCH 26/39] Update functional tests to include all file formats --- .../associations/comprehensions.dot | 20 +++++++++++ .../associations/comprehensions.puml | 36 +++++++++++++++++++ .../associations/comprehensions.rc | 2 ++ .../class_diagrams/associations/fields.dot | 15 ++++++++ .../class_diagrams/associations/fields.mmd | 20 +++++------ .../class_diagrams/associations/fields.puml | 25 +++++++++++++ .../class_diagrams/associations/fields.py | 10 +++--- .../class_diagrams/associations/fields.rc | 2 ++ 8 files changed, 115 insertions(+), 15 deletions(-) create mode 100644 tests/pyreverse/functional/class_diagrams/associations/comprehensions.dot create mode 100644 tests/pyreverse/functional/class_diagrams/associations/comprehensions.puml create mode 100644 tests/pyreverse/functional/class_diagrams/associations/comprehensions.rc create mode 100644 tests/pyreverse/functional/class_diagrams/associations/fields.dot create mode 100644 tests/pyreverse/functional/class_diagrams/associations/fields.puml create mode 100644 tests/pyreverse/functional/class_diagrams/associations/fields.rc diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.dot b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.dot new file mode 100644 index 0000000000..a191dd0a64 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.dot @@ -0,0 +1,20 @@ +digraph "classes" { +rankdir=BT +charset="utf-8" +"comprehensions.AggregationContainer" [color="black", fontcolor="black", label=<{AggregationContainer|component_dict : dict[str, Component]
components : list[Component]
components_set : set[Component]
lazy_components : Generator[Component]
|}>, shape="record", style="solid"]; +"comprehensions.AssociationContainer" [color="black", fontcolor="black", label=<{AssociationContainer|component_dict : dict[int, Component]
components : list[Component]
components_set : set[Component]
lazy_components : Generator[Component]
|}>, shape="record", style="solid"]; +"comprehensions.Component" [color="black", fontcolor="black", label=<{Component|name : str
|}>, shape="record", style="solid"]; +"comprehensions.CompositionContainer" [color="black", fontcolor="black", label=<{CompositionContainer|component_dict : dict[int, Component]
components : list[Component]
components_set : set[Component]
lazy_components : Generator[Component]
|}>, shape="record", style="solid"]; +"comprehensions.Component" -> "comprehensions.AssociationContainer" [arrowhead="vee", arrowtail="none", fontcolor="green", label="components", style="solid"]; +"comprehensions.Component" -> "comprehensions.AssociationContainer" [arrowhead="vee", arrowtail="none", fontcolor="green", label="component_dict", style="solid"]; +"comprehensions.Component" -> "comprehensions.AssociationContainer" [arrowhead="vee", arrowtail="none", fontcolor="green", label="components_set", style="solid"]; +"comprehensions.Component" -> "comprehensions.AssociationContainer" [arrowhead="vee", arrowtail="none", fontcolor="green", label="lazy_components", style="solid"]; +"comprehensions.Component" -> "comprehensions.CompositionContainer" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="components", style="solid"]; +"comprehensions.Component" -> "comprehensions.CompositionContainer" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="component_dict", style="solid"]; +"comprehensions.Component" -> "comprehensions.CompositionContainer" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="components_set", style="solid"]; +"comprehensions.Component" -> "comprehensions.CompositionContainer" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="lazy_components", style="solid"]; +"comprehensions.Component" -> "comprehensions.AggregationContainer" [arrowhead="odiamond", arrowtail="none", fontcolor="green", label="components", style="solid"]; +"comprehensions.Component" -> "comprehensions.AggregationContainer" [arrowhead="odiamond", arrowtail="none", fontcolor="green", label="component_dict", style="solid"]; +"comprehensions.Component" -> "comprehensions.AggregationContainer" [arrowhead="odiamond", arrowtail="none", fontcolor="green", label="components_set", style="solid"]; +"comprehensions.Component" -> "comprehensions.AggregationContainer" [arrowhead="odiamond", arrowtail="none", fontcolor="green", label="lazy_components", style="solid"]; +} diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.puml b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.puml new file mode 100644 index 0000000000..2398f6633d --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.puml @@ -0,0 +1,36 @@ +@startuml classes +set namespaceSeparator none +class "AggregationContainer" as comprehensions.AggregationContainer { + component_dict : dict[str, Component] + components : list[Component] + components_set : set[Component] + lazy_components : Generator[Component] +} +class "AssociationContainer" as comprehensions.AssociationContainer { + component_dict : dict[int, Component] + components : list[Component] + components_set : set[Component] + lazy_components : Generator[Component] +} +class "Component" as comprehensions.Component { + name : str +} +class "CompositionContainer" as comprehensions.CompositionContainer { + component_dict : dict[int, Component] + components : list[Component] + components_set : set[Component] + lazy_components : Generator[Component] +} +comprehensions.Component --> comprehensions.AssociationContainer : components +comprehensions.Component --> comprehensions.AssociationContainer : component_dict +comprehensions.Component --> comprehensions.AssociationContainer : components_set +comprehensions.Component --> comprehensions.AssociationContainer : lazy_components +comprehensions.Component --* comprehensions.CompositionContainer : components +comprehensions.Component --* comprehensions.CompositionContainer : component_dict +comprehensions.Component --* comprehensions.CompositionContainer : components_set +comprehensions.Component --* comprehensions.CompositionContainer : lazy_components +comprehensions.Component --o comprehensions.AggregationContainer : components +comprehensions.Component --o comprehensions.AggregationContainer : component_dict +comprehensions.Component --o comprehensions.AggregationContainer : components_set +comprehensions.Component --o comprehensions.AggregationContainer : lazy_components +@enduml diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.rc b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.rc new file mode 100644 index 0000000000..9e2ff3d953 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/associations/comprehensions.rc @@ -0,0 +1,2 @@ +[testoptions] +output_formats=mmd,dot,puml diff --git a/tests/pyreverse/functional/class_diagrams/associations/fields.dot b/tests/pyreverse/functional/class_diagrams/associations/fields.dot new file mode 100644 index 0000000000..dda2320d65 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/associations/fields.dot @@ -0,0 +1,15 @@ +digraph "classes" { +rankdir=BT +charset="utf-8" +"fields.Aggregation1" [color="black", fontcolor="black", label=<{Aggregation1|x
|}>, shape="record", style="solid"]; +"fields.Aggregation2" [color="black", fontcolor="black", label=<{Aggregation2|x
|}>, shape="record", style="solid"]; +"fields.Association" [color="black", fontcolor="black", label=<{Association|x
|}>, shape="record", style="solid"]; +"fields.Composition1" [color="black", fontcolor="black", label=<{Composition1|x
|}>, shape="record", style="solid"]; +"fields.Composition2" [color="black", fontcolor="black", label=<{Composition2|x
|}>, shape="record", style="solid"]; +"fields.P" [color="black", fontcolor="black", label=<{P|
|}>, shape="record", style="solid"]; +"fields.P" -> "fields.Association" [arrowhead="vee", arrowtail="none", fontcolor="green", label="x", style="solid"]; +"fields.P" -> "fields.Composition1" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="x", style="solid"]; +"fields.P" -> "fields.Composition2" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="x", style="solid"]; +"fields.P" -> "fields.Aggregation1" [arrowhead="odiamond", arrowtail="none", fontcolor="green", label="x", style="solid"]; +"fields.P" -> "fields.Aggregation2" [arrowhead="odiamond", arrowtail="none", fontcolor="green", label="x", style="solid"]; +} diff --git a/tests/pyreverse/functional/class_diagrams/associations/fields.mmd b/tests/pyreverse/functional/class_diagrams/associations/fields.mmd index 96e0defda1..5a2a70002a 100644 --- a/tests/pyreverse/functional/class_diagrams/associations/fields.mmd +++ b/tests/pyreverse/functional/class_diagrams/associations/fields.mmd @@ -1,23 +1,23 @@ classDiagram - class A { + class Aggregation1 { x } - class B { + class Aggregation2 { x } - class C { + class Association { x } - class D { + class Composition1 { x } - class E { + class Composition2 { x } class P { } - P --> A : x - P --* D : x - P --* E : x - P --o B : x - P --o C : x + P --> Association : x + P --* Composition1 : x + P --* Composition2 : x + P --o Aggregation1 : x + P --o Aggregation2 : x diff --git a/tests/pyreverse/functional/class_diagrams/associations/fields.puml b/tests/pyreverse/functional/class_diagrams/associations/fields.puml new file mode 100644 index 0000000000..1fd32259a1 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/associations/fields.puml @@ -0,0 +1,25 @@ +@startuml classes +set namespaceSeparator none +class "Aggregation1" as fields.Aggregation1 { + x +} +class "Aggregation2" as fields.Aggregation2 { + x +} +class "Association" as fields.Association { + x +} +class "Composition1" as fields.Composition1 { + x +} +class "Composition2" as fields.Composition2 { + x +} +class "P" as fields.P { +} +fields.P --> fields.Association : x +fields.P --* fields.Composition1 : x +fields.P --* fields.Composition2 : x +fields.P --o fields.Aggregation1 : x +fields.P --o fields.Aggregation2 : x +@enduml diff --git a/tests/pyreverse/functional/class_diagrams/associations/fields.py b/tests/pyreverse/functional/class_diagrams/associations/fields.py index 9e35885a9e..be8b5c1fd9 100644 --- a/tests/pyreverse/functional/class_diagrams/associations/fields.py +++ b/tests/pyreverse/functional/class_diagrams/associations/fields.py @@ -3,23 +3,23 @@ class P: pass -class A: +class Association: x: P # just type hint, no ownership → Association -class B: +class Aggregation1: def __init__(self, x: P): self.x = x # receives object, not created → Aggregation -class C: +class Aggregation2: x: P def __init__(self, x: P): self.x = x # receives object, not created → Aggregation -class D: +class Composition1: x: P def __init__(self): self.x = P() # creates object → Composition -class E: +class Composition2: def __init__(self): self.x = P() # creates object → Composition diff --git a/tests/pyreverse/functional/class_diagrams/associations/fields.rc b/tests/pyreverse/functional/class_diagrams/associations/fields.rc new file mode 100644 index 0000000000..9e2ff3d953 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/associations/fields.rc @@ -0,0 +1,2 @@ +[testoptions] +output_formats=mmd,dot,puml From 4362e85bb8ccc6719ecb952108f94ddb05962a97 Mon Sep 17 00:00:00 2001 From: Julfried Date: Sun, 29 Jun 2025 19:54:22 +0200 Subject: [PATCH 27/39] Fix diadefs tests (DoNothing now is correctly detected as Composition instead of assoiciation) --- tests/pyreverse/test_diadefs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pyreverse/test_diadefs.py b/tests/pyreverse/test_diadefs.py index 5da1aa1e7f..e3e4833b33 100644 --- a/tests/pyreverse/test_diadefs.py +++ b/tests/pyreverse/test_diadefs.py @@ -182,7 +182,7 @@ class TestDefaultDiadefGenerator: _should_rels = [ ("aggregation", "DoNothing2", "Specialization"), ("association", "DoNothing", "Ancestor"), - ("association", "DoNothing", "Specialization"), + ("composition", "DoNothing", "Specialization"), ("specialization", "Specialization", "Ancestor"), ] From 6e35d4bacccacd202b7528decde61071bd22d90f Mon Sep 17 00:00:00 2001 From: Julfried Date: Sun, 29 Jun 2025 20:28:41 +0200 Subject: [PATCH 28/39] Remove TODO since this now works --- pylint/pyreverse/inspector.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index 6b3e77f80c..1c564e378f 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -408,7 +408,6 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: return # Aggregation: comprehensions without object creation (self.x = [existing_obj for ...]) - # TODO: Currently inferring type of existing_obj is not reliable ==> improve once astroid supports it if isinstance( value, (nodes.ListComp, nodes.DictComp, nodes.SetComp, nodes.GeneratorExp) ): From d1fd43c9701a19f5fe5ea6c46870798790a481ab Mon Sep 17 00:00:00 2001 From: Julfried Date: Sun, 29 Jun 2025 20:32:05 +0200 Subject: [PATCH 29/39] Rename functional test files to relationships for better clarity --- .../{associations => relationships}/comprehensions.dot | 0 .../{associations => relationships}/comprehensions.mmd | 0 .../{associations => relationships}/comprehensions.puml | 0 .../{associations => relationships}/comprehensions.py | 0 .../{associations => relationships}/comprehensions.rc | 0 .../class_diagrams/{associations => relationships}/fields.dot | 0 .../class_diagrams/{associations => relationships}/fields.mmd | 0 .../class_diagrams/{associations => relationships}/fields.puml | 0 .../class_diagrams/{associations => relationships}/fields.py | 0 .../class_diagrams/{associations => relationships}/fields.rc | 0 .../{associations_filtering => relationships_filtering}/all.mmd | 0 .../{associations_filtering => relationships_filtering}/all.py | 0 .../{associations_filtering => relationships_filtering}/all.rc | 0 .../{associations_filtering => relationships_filtering}/other.mmd | 0 .../{associations_filtering => relationships_filtering}/other.py | 0 .../{associations_filtering => relationships_filtering}/other.rc | 0 .../pub_only.mmd | 0 .../pub_only.py | 0 .../pub_only.rc | 0 .../special.mmd | 0 .../special.py | 0 .../special.rc | 0 22 files changed, 0 insertions(+), 0 deletions(-) rename tests/pyreverse/functional/class_diagrams/{associations => relationships}/comprehensions.dot (100%) rename tests/pyreverse/functional/class_diagrams/{associations => relationships}/comprehensions.mmd (100%) rename tests/pyreverse/functional/class_diagrams/{associations => relationships}/comprehensions.puml (100%) rename tests/pyreverse/functional/class_diagrams/{associations => relationships}/comprehensions.py (100%) rename tests/pyreverse/functional/class_diagrams/{associations => relationships}/comprehensions.rc (100%) rename tests/pyreverse/functional/class_diagrams/{associations => relationships}/fields.dot (100%) rename tests/pyreverse/functional/class_diagrams/{associations => relationships}/fields.mmd (100%) rename tests/pyreverse/functional/class_diagrams/{associations => relationships}/fields.puml (100%) rename tests/pyreverse/functional/class_diagrams/{associations => relationships}/fields.py (100%) rename tests/pyreverse/functional/class_diagrams/{associations => relationships}/fields.rc (100%) rename tests/pyreverse/functional/class_diagrams/{associations_filtering => relationships_filtering}/all.mmd (100%) rename tests/pyreverse/functional/class_diagrams/{associations_filtering => relationships_filtering}/all.py (100%) rename tests/pyreverse/functional/class_diagrams/{associations_filtering => relationships_filtering}/all.rc (100%) rename tests/pyreverse/functional/class_diagrams/{associations_filtering => relationships_filtering}/other.mmd (100%) rename tests/pyreverse/functional/class_diagrams/{associations_filtering => relationships_filtering}/other.py (100%) rename tests/pyreverse/functional/class_diagrams/{associations_filtering => relationships_filtering}/other.rc (100%) rename tests/pyreverse/functional/class_diagrams/{associations_filtering => relationships_filtering}/pub_only.mmd (100%) rename tests/pyreverse/functional/class_diagrams/{associations_filtering => relationships_filtering}/pub_only.py (100%) rename tests/pyreverse/functional/class_diagrams/{associations_filtering => relationships_filtering}/pub_only.rc (100%) rename tests/pyreverse/functional/class_diagrams/{associations_filtering => relationships_filtering}/special.mmd (100%) rename tests/pyreverse/functional/class_diagrams/{associations_filtering => relationships_filtering}/special.py (100%) rename tests/pyreverse/functional/class_diagrams/{associations_filtering => relationships_filtering}/special.rc (100%) diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.dot b/tests/pyreverse/functional/class_diagrams/relationships/comprehensions.dot similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations/comprehensions.dot rename to tests/pyreverse/functional/class_diagrams/relationships/comprehensions.dot diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd b/tests/pyreverse/functional/class_diagrams/relationships/comprehensions.mmd similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations/comprehensions.mmd rename to tests/pyreverse/functional/class_diagrams/relationships/comprehensions.mmd diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.puml b/tests/pyreverse/functional/class_diagrams/relationships/comprehensions.puml similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations/comprehensions.puml rename to tests/pyreverse/functional/class_diagrams/relationships/comprehensions.puml diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.py b/tests/pyreverse/functional/class_diagrams/relationships/comprehensions.py similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations/comprehensions.py rename to tests/pyreverse/functional/class_diagrams/relationships/comprehensions.py diff --git a/tests/pyreverse/functional/class_diagrams/associations/comprehensions.rc b/tests/pyreverse/functional/class_diagrams/relationships/comprehensions.rc similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations/comprehensions.rc rename to tests/pyreverse/functional/class_diagrams/relationships/comprehensions.rc diff --git a/tests/pyreverse/functional/class_diagrams/associations/fields.dot b/tests/pyreverse/functional/class_diagrams/relationships/fields.dot similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations/fields.dot rename to tests/pyreverse/functional/class_diagrams/relationships/fields.dot diff --git a/tests/pyreverse/functional/class_diagrams/associations/fields.mmd b/tests/pyreverse/functional/class_diagrams/relationships/fields.mmd similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations/fields.mmd rename to tests/pyreverse/functional/class_diagrams/relationships/fields.mmd diff --git a/tests/pyreverse/functional/class_diagrams/associations/fields.puml b/tests/pyreverse/functional/class_diagrams/relationships/fields.puml similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations/fields.puml rename to tests/pyreverse/functional/class_diagrams/relationships/fields.puml diff --git a/tests/pyreverse/functional/class_diagrams/associations/fields.py b/tests/pyreverse/functional/class_diagrams/relationships/fields.py similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations/fields.py rename to tests/pyreverse/functional/class_diagrams/relationships/fields.py diff --git a/tests/pyreverse/functional/class_diagrams/associations/fields.rc b/tests/pyreverse/functional/class_diagrams/relationships/fields.rc similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations/fields.rc rename to tests/pyreverse/functional/class_diagrams/relationships/fields.rc diff --git a/tests/pyreverse/functional/class_diagrams/associations_filtering/all.mmd b/tests/pyreverse/functional/class_diagrams/relationships_filtering/all.mmd similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations_filtering/all.mmd rename to tests/pyreverse/functional/class_diagrams/relationships_filtering/all.mmd diff --git a/tests/pyreverse/functional/class_diagrams/associations_filtering/all.py b/tests/pyreverse/functional/class_diagrams/relationships_filtering/all.py similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations_filtering/all.py rename to tests/pyreverse/functional/class_diagrams/relationships_filtering/all.py diff --git a/tests/pyreverse/functional/class_diagrams/associations_filtering/all.rc b/tests/pyreverse/functional/class_diagrams/relationships_filtering/all.rc similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations_filtering/all.rc rename to tests/pyreverse/functional/class_diagrams/relationships_filtering/all.rc diff --git a/tests/pyreverse/functional/class_diagrams/associations_filtering/other.mmd b/tests/pyreverse/functional/class_diagrams/relationships_filtering/other.mmd similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations_filtering/other.mmd rename to tests/pyreverse/functional/class_diagrams/relationships_filtering/other.mmd diff --git a/tests/pyreverse/functional/class_diagrams/associations_filtering/other.py b/tests/pyreverse/functional/class_diagrams/relationships_filtering/other.py similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations_filtering/other.py rename to tests/pyreverse/functional/class_diagrams/relationships_filtering/other.py diff --git a/tests/pyreverse/functional/class_diagrams/associations_filtering/other.rc b/tests/pyreverse/functional/class_diagrams/relationships_filtering/other.rc similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations_filtering/other.rc rename to tests/pyreverse/functional/class_diagrams/relationships_filtering/other.rc diff --git a/tests/pyreverse/functional/class_diagrams/associations_filtering/pub_only.mmd b/tests/pyreverse/functional/class_diagrams/relationships_filtering/pub_only.mmd similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations_filtering/pub_only.mmd rename to tests/pyreverse/functional/class_diagrams/relationships_filtering/pub_only.mmd diff --git a/tests/pyreverse/functional/class_diagrams/associations_filtering/pub_only.py b/tests/pyreverse/functional/class_diagrams/relationships_filtering/pub_only.py similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations_filtering/pub_only.py rename to tests/pyreverse/functional/class_diagrams/relationships_filtering/pub_only.py diff --git a/tests/pyreverse/functional/class_diagrams/associations_filtering/pub_only.rc b/tests/pyreverse/functional/class_diagrams/relationships_filtering/pub_only.rc similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations_filtering/pub_only.rc rename to tests/pyreverse/functional/class_diagrams/relationships_filtering/pub_only.rc diff --git a/tests/pyreverse/functional/class_diagrams/associations_filtering/special.mmd b/tests/pyreverse/functional/class_diagrams/relationships_filtering/special.mmd similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations_filtering/special.mmd rename to tests/pyreverse/functional/class_diagrams/relationships_filtering/special.mmd diff --git a/tests/pyreverse/functional/class_diagrams/associations_filtering/special.py b/tests/pyreverse/functional/class_diagrams/relationships_filtering/special.py similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations_filtering/special.py rename to tests/pyreverse/functional/class_diagrams/relationships_filtering/special.py diff --git a/tests/pyreverse/functional/class_diagrams/associations_filtering/special.rc b/tests/pyreverse/functional/class_diagrams/relationships_filtering/special.rc similarity index 100% rename from tests/pyreverse/functional/class_diagrams/associations_filtering/special.rc rename to tests/pyreverse/functional/class_diagrams/relationships_filtering/special.rc From c21e64c09e606c8281cc71dd6d9894beacf1ae2b Mon Sep 17 00:00:00 2001 From: Julfried Date: Sun, 29 Jun 2025 20:34:32 +0200 Subject: [PATCH 30/39] rename to compositionshandler for better clarity --- pylint/pyreverse/inspector.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index 1c564e378f..352e0cc009 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -128,11 +128,11 @@ def __init__(self, project: Project, tag: bool = False) -> None: self.project = project # Chain: Composition → Aggregation → Association - self.associations_handler = CompositionsHandler() + self.compositions_handler = CompositionsHandler() aggregation_handler = AggregationsHandler() association_handler = AssociationsHandler() - self.associations_handler.set_next(aggregation_handler) + self.compositions_handler.set_next(aggregation_handler) aggregation_handler.set_next(association_handler) def visit_project(self, node: Project) -> None: @@ -184,7 +184,7 @@ def visit_classdef(self, node: nodes.ClassDef) -> None: for assignattrs in tuple(node.instance_attrs.values()): for assignattr in assignattrs: if not isinstance(assignattr, nodes.Unknown): - self.associations_handler.handle(assignattr, node) + self.compositions_handler.handle(assignattr, node) self.handle_assignattr_type(assignattr, node) def visit_functiondef(self, node: nodes.FunctionDef) -> None: From 5852435fd8bb798cc23c99c0c698093b86b2b544 Mon Sep 17 00:00:00 2001 From: Julfried Date: Sun, 29 Jun 2025 20:43:02 +0200 Subject: [PATCH 31/39] Rename association-related classes to relationship aswell for improved clarity --- pylint/pyreverse/inspector.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index 352e0cc009..857b711d2f 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -300,11 +300,11 @@ def _imported_module( mod_paths.append(mod_path) -class AssociationHandlerInterface(ABC): +class RelationshipHandlerInterface(ABC): @abstractmethod def set_next( - self, handler: AssociationHandlerInterface - ) -> AssociationHandlerInterface: + self, handler: RelationshipHandlerInterface + ) -> RelationshipHandlerInterface: pass @abstractmethod @@ -312,23 +312,23 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: pass -class AbstractAssociationHandler(AssociationHandlerInterface): +class AbstractRelationshipHandler(RelationshipHandlerInterface): """ - Chain of Responsibility for handling types of association, useful - to expand in the future if we want to add more distinct associations. + Chain of Responsibility for handling types of relationships, useful + to expand in the future if we want to add more distinct relationships. - Every link of the chain checks if it's a certain type of association. - If no association is found it's set as a generic association in `associations_type`. + Every link of the chain checks if it's a certain type of relationship. + If no relationship is found it's set as a generic relationship in `relationships_type`. The default chaining behavior is implemented inside the base handler class. """ - _next_handler: AssociationHandlerInterface + _next_handler: RelationshipHandlerInterface def set_next( - self, handler: AssociationHandlerInterface - ) -> AssociationHandlerInterface: + self, handler: RelationshipHandlerInterface + ) -> RelationshipHandlerInterface: self._next_handler = handler return handler @@ -338,7 +338,7 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: self._next_handler.handle(node, parent) -class CompositionsHandler(AbstractAssociationHandler): +class CompositionsHandler(AbstractRelationshipHandler): """Handle composition relationships where parent creates child objects.""" def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: @@ -385,7 +385,7 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: super().handle(node, parent) -class AggregationsHandler(AbstractAssociationHandler): +class AggregationsHandler(AbstractRelationshipHandler): """Handle aggregation relationships where parent receives child objects.""" def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: @@ -432,7 +432,7 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: super().handle(node, parent) -class AssociationsHandler(AbstractAssociationHandler): +class AssociationsHandler(AbstractRelationshipHandler): """Handle regular association relationships.""" def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: From 70492f68cdd6030931027a44b0cbe0a1364bba59 Mon Sep 17 00:00:00 2001 From: Julfried Date: Sun, 29 Jun 2025 21:04:22 +0200 Subject: [PATCH 32/39] Try to fix the failing tests by processing instance attributes and class level attributes separate --- pylint/pyreverse/inspector.py | 63 +++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 17 deletions(-) diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index 857b711d2f..6cc63634b2 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -187,6 +187,14 @@ def visit_classdef(self, node: nodes.ClassDef) -> None: self.compositions_handler.handle(assignattr, node) self.handle_assignattr_type(assignattr, node) + # Process class attributes + for local_nodes in node.locals.values(): + for local_node in local_nodes: + if isinstance(local_node, nodes.AssignName) and isinstance( + local_node.parent, nodes.Assign + ): + self.compositions_handler.handle(local_node, node) + def visit_functiondef(self, node: nodes.FunctionDef) -> None: """Visit an astroid.Function node. @@ -308,7 +316,9 @@ def set_next( pass @abstractmethod - def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: + def handle( + self, node: nodes.AssignAttr | nodes.AssignName, parent: nodes.ClassDef + ) -> None: pass @@ -333,7 +343,9 @@ def set_next( return handler @abstractmethod - def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: + def handle( + self, node: nodes.AssignAttr | nodes.AssignName, parent: nodes.ClassDef + ) -> None: if self._next_handler: self._next_handler.handle(node, parent) @@ -341,13 +353,19 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: class CompositionsHandler(AbstractRelationshipHandler): """Handle composition relationships where parent creates child objects.""" - def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: + def handle( + self, node: nodes.AssignAttr | nodes.AssignName, parent: nodes.ClassDef + ) -> None: + # If the node is not part of an assignment, pass to next handler if not isinstance(node.parent, (nodes.AnnAssign, nodes.Assign)): super().handle(node, parent) return value = node.parent.value + # Extract the name to handle both AssignAttr and AssignName nodes + name = node.attrname if isinstance(node, nodes.AssignAttr) else node.name + # Composition: direct object creation (self.x = P()) if isinstance(value, nodes.Call): inferred_types = utils.infer_node(node) @@ -356,8 +374,8 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: # Resolve nodes to actual class definitions resolved_types = resolve_to_class_def(element_types) - current = set(parent.compositions_type[node.attrname]) - parent.compositions_type[node.attrname] = list(current | resolved_types) + current = set(parent.compositions_type[name]) + parent.compositions_type[name] = list(current | resolved_types) return # Composition: comprehensions with object creation (self.x = [P() for ...]) @@ -377,8 +395,8 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: # Resolve nodes to actual class definitions resolved_types = resolve_to_class_def(element_types) - current = set(parent.compositions_type[node.attrname]) - parent.compositions_type[node.attrname] = list(current | resolved_types) + current = set(parent.compositions_type[name]) + parent.compositions_type[name] = list(current | resolved_types) return # Not a composition, pass to next handler @@ -388,13 +406,19 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: class AggregationsHandler(AbstractRelationshipHandler): """Handle aggregation relationships where parent receives child objects.""" - def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: + def handle( + self, node: nodes.AssignAttr | nodes.AssignName, parent: nodes.ClassDef + ) -> None: + # If the node is not part of an assignment, pass to next handler if not isinstance(node.parent, (nodes.AnnAssign, nodes.Assign)): super().handle(node, parent) return value = node.parent.value + # Extract the name to handle both AssignAttr and AssignName nodes + name = node.attrname if isinstance(node, nodes.AssignAttr) else node.name + # Aggregation: direct assignment (self.x = x) if isinstance(value, nodes.Name): inferred_types = utils.infer_node(node) @@ -403,8 +427,8 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: # Resolve nodes to actual class definitions resolved_types = resolve_to_class_def(element_types) - current = set(parent.aggregations_type[node.attrname]) - parent.aggregations_type[node.attrname] = list(current | resolved_types) + current = set(parent.aggregations_type[name]) + parent.aggregations_type[name] = list(current | resolved_types) return # Aggregation: comprehensions without object creation (self.x = [existing_obj for ...]) @@ -424,8 +448,8 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: # Resolve nodes to actual class definitions resolved_types = resolve_to_class_def(element_types) - current = set(parent.aggregations_type[node.attrname]) - parent.aggregations_type[node.attrname] = list(current | resolved_types) + current = set(parent.aggregations_type[name]) + parent.aggregations_type[name] = list(current | resolved_types) return # Not an aggregation, pass to next handler @@ -435,7 +459,12 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: class AssociationsHandler(AbstractRelationshipHandler): """Handle regular association relationships.""" - def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: + def handle( + self, node: nodes.AssignAttr | nodes.AssignName, parent: nodes.ClassDef + ) -> None: + # Extract the name to handle both AssignAttr and AssignName nodes + name = node.attrname if isinstance(node, nodes.AssignAttr) else node.name + # Type annotation only (x: P) -> Association # BUT only if there's no actual assignment (to avoid duplicates) if isinstance(node.parent, nodes.AnnAssign) and node.parent.value is None: @@ -445,18 +474,18 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: # Resolve nodes to actual class definitions resolved_types = resolve_to_class_def(element_types) - current = set(parent.associations_type[node.attrname]) - parent.associations_type[node.attrname] = list(current | resolved_types) + current = set(parent.associations_type[name]) + parent.associations_type[name] = list(current | resolved_types) return # Everything else is also association (fallback) - current = set(parent.associations_type[node.attrname]) + current = set(parent.associations_type[name]) inferred_types = utils.infer_node(node) element_types = extract_element_types(inferred_types) # Resolve Name nodes to actual class definitions resolved_types = resolve_to_class_def(element_types) - parent.associations_type[node.attrname] = list(current | resolved_types) + parent.associations_type[name] = list(current | resolved_types) def resolve_to_class_def(types: set[nodes.NodeNG]) -> set[nodes.ClassDef]: From f3f4364fcb876a5f7a0d71d795a3af444fd8a801 Mon Sep 17 00:00:00 2001 From: Julfried Date: Sun, 29 Jun 2025 21:06:36 +0200 Subject: [PATCH 33/39] Fix diadefs_test again (cls_member is now also correctly identified as composition instead of association) --- tests/pyreverse/test_diadefs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pyreverse/test_diadefs.py b/tests/pyreverse/test_diadefs.py index e3e4833b33..ce39cb5503 100644 --- a/tests/pyreverse/test_diadefs.py +++ b/tests/pyreverse/test_diadefs.py @@ -181,7 +181,7 @@ class CustomError(Exception): class TestDefaultDiadefGenerator: _should_rels = [ ("aggregation", "DoNothing2", "Specialization"), - ("association", "DoNothing", "Ancestor"), + ("composition", "DoNothing", "Ancestor"), ("composition", "DoNothing", "Specialization"), ("specialization", "Specialization", "Ancestor"), ] From e62636fc89f4d43f19142cb4cf8ea83219ccf7bb Mon Sep 17 00:00:00 2001 From: Julfried Date: Sun, 29 Jun 2025 21:30:12 +0200 Subject: [PATCH 34/39] Also consider composition when when filtering for --no-standalone ==> fixes last failing test --- pylint/pyreverse/writer.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pylint/pyreverse/writer.py b/pylint/pyreverse/writer.py index 28fb7ea095..7544be69e1 100644 --- a/pylint/pyreverse/writer.py +++ b/pylint/pyreverse/writer.py @@ -119,7 +119,12 @@ def write_classes(self, diagram: ClassDiagram) -> None: if self.config.no_standalone and not any( obj in (rel.from_object, rel.to_object) - for rel_type in ("specialization", "association", "aggregation") + for rel_type in ( + "specialization", + "association", + "aggregation", + "composition", + ) for rel in diagram.get_relationships(rel_type) ): continue From 9a3382f2b3b27b9ce16c1428afd79b181653fbf5 Mon Sep 17 00:00:00 2001 From: Julfried Date: Sun, 29 Jun 2025 22:12:30 +0200 Subject: [PATCH 35/39] Add newsfragment --- doc/whatsnew/fragments/9045.feature | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 doc/whatsnew/fragments/9045.feature diff --git a/doc/whatsnew/fragments/9045.feature b/doc/whatsnew/fragments/9045.feature new file mode 100644 index 0000000000..9a96f1b083 --- /dev/null +++ b/doc/whatsnew/fragments/9045.feature @@ -0,0 +1,3 @@ +Enhanced pyreverse to properly distinguish between UML relationship types (association, aggregation, composition) based on object ownership semantics. Type annotations without assignment are now treated as associations, parameter assignments as aggregations, and object instantiation as compositions. + +Closes #9045 \ No newline at end of file From 86c92e2c52b3b622d9da3eab83bbcd7958a7c717 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 29 Jun 2025 20:13:20 +0000 Subject: [PATCH 36/39] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- doc/whatsnew/fragments/9045.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whatsnew/fragments/9045.feature b/doc/whatsnew/fragments/9045.feature index 9a96f1b083..b3cd6c5a07 100644 --- a/doc/whatsnew/fragments/9045.feature +++ b/doc/whatsnew/fragments/9045.feature @@ -1,3 +1,3 @@ Enhanced pyreverse to properly distinguish between UML relationship types (association, aggregation, composition) based on object ownership semantics. Type annotations without assignment are now treated as associations, parameter assignments as aggregations, and object instantiation as compositions. -Closes #9045 \ No newline at end of file +Closes #9045 From c727e14860307b35d36962b3591a04cab95c33d0 Mon Sep 17 00:00:00 2001 From: Julfried Date: Mon, 30 Jun 2025 19:25:24 +0200 Subject: [PATCH 37/39] Add functional tests --- .../class_diagrams/attributes/duplicates_9267.mmd | 9 +++++++++ .../class_diagrams/attributes/duplicates_9267.py | 12 ++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 tests/pyreverse/functional/class_diagrams/attributes/duplicates_9267.mmd create mode 100644 tests/pyreverse/functional/class_diagrams/attributes/duplicates_9267.py diff --git a/tests/pyreverse/functional/class_diagrams/attributes/duplicates_9267.mmd b/tests/pyreverse/functional/class_diagrams/attributes/duplicates_9267.mmd new file mode 100644 index 0000000000..5e451ec3af --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/attributes/duplicates_9267.mmd @@ -0,0 +1,9 @@ +classDiagram + class A { + var : int + } + class B { + a_obj + func() + } + A --* B : a_obj diff --git a/tests/pyreverse/functional/class_diagrams/attributes/duplicates_9267.py b/tests/pyreverse/functional/class_diagrams/attributes/duplicates_9267.py new file mode 100644 index 0000000000..73d8138646 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/attributes/duplicates_9267.py @@ -0,0 +1,12 @@ +# Test for https://github.com/pylint-dev/pylint/issues/9267 +class A: + def __init__(self) -> None: + self.var = 2 + +class B: + def __init__(self) -> None: + self.a_obj = A() + + def func(self): + self.a_obj = A() + self.a_obj = A() From 07cda64b679f090e0a48e54a634ac9cdfa47d397 Mon Sep 17 00:00:00 2001 From: Julfried Date: Mon, 30 Jun 2025 19:27:25 +0200 Subject: [PATCH 38/39] Update newsfragement --- doc/whatsnew/fragments/9045.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/whatsnew/fragments/9045.feature b/doc/whatsnew/fragments/9045.feature index b3cd6c5a07..62b593cb7a 100644 --- a/doc/whatsnew/fragments/9045.feature +++ b/doc/whatsnew/fragments/9045.feature @@ -1,3 +1,4 @@ Enhanced pyreverse to properly distinguish between UML relationship types (association, aggregation, composition) based on object ownership semantics. Type annotations without assignment are now treated as associations, parameter assignments as aggregations, and object instantiation as compositions. Closes #9045 +Closes #9267 \ No newline at end of file From d275c2f7e3fe161275cef0c8acec8ba8dfde4b1f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 30 Jun 2025 17:28:35 +0000 Subject: [PATCH 39/39] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- doc/whatsnew/fragments/9045.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whatsnew/fragments/9045.feature b/doc/whatsnew/fragments/9045.feature index 62b593cb7a..c460b4c4c2 100644 --- a/doc/whatsnew/fragments/9045.feature +++ b/doc/whatsnew/fragments/9045.feature @@ -1,4 +1,4 @@ Enhanced pyreverse to properly distinguish between UML relationship types (association, aggregation, composition) based on object ownership semantics. Type annotations without assignment are now treated as associations, parameter assignments as aggregations, and object instantiation as compositions. Closes #9045 -Closes #9267 \ No newline at end of file +Closes #9267