diff options
author | Alvaro Frias <alvaro.frias@eclypsium.com> | 2022-11-27 12:18:43 -0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-11-27 16:18:43 +0100 |
commit | b950567b872c7177f976066e9b8982f08e62eb6c (patch) | |
tree | ee0a1c30ded4eb4ce2267adcc52b9dcfb9865039 | |
parent | 978d1ab95603fa337e686aac8956366556c23080 (diff) | |
download | pylint-git-b950567b872c7177f976066e9b8982f08e62eb6c.tar.gz |
Feature: distinct Composition and Aggregation arrows (#7836)
* Update Linker to add aggregations_type and associations_type to nodes
Update logic of nodes to check what kind of relationship does nodes have (association, aggregation)
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Update ClassDiagram's extrac_relationship method to add aggregations links
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Update DiagramWriter to generate AGGREGATION edges
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Update printers to show aggregations
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Update tests with changes on aggregations
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Update Linker to add aggregations_type and associations_type to nodes
Update logic of nodes to check what kind of relationship does nodes have (association, aggregation)
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Update ClassDiagram's extrac_relationship method to add aggregations links
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Update DiagramWriter to generate AGGREGATION edges
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Update tests with changes on aggregations
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Apply pylint pre-commit correction
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Apply mypy corrections
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Add towrncrier fragment
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Update towncrier fragment
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Update towncrier fragment
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Update doc/whatsnew/fragments/6543.feature
Co-authored-by: Andreas Finkler <3929834+DudeNr33@users.noreply.github.com>
* Add documentation
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* fix typo
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Add type hints
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* Update pylint/pyreverse/diagrams.py
Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Update type hints
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Update fragment
* Update fragment
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Andreas Finkler <3929834+DudeNr33@users.noreply.github.com>
Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>
-rw-r--r-- | doc/whatsnew/fragments/6543.feature | 5 | ||||
-rw-r--r-- | pylint/pyreverse/diagrams.py | 37 | ||||
-rw-r--r-- | pylint/pyreverse/dot_printer.py | 6 | ||||
-rw-r--r-- | pylint/pyreverse/inspector.py | 67 | ||||
-rw-r--r-- | pylint/pyreverse/mermaidjs_printer.py | 1 | ||||
-rw-r--r-- | pylint/pyreverse/plantuml_printer.py | 1 | ||||
-rw-r--r-- | pylint/pyreverse/printer.py | 1 | ||||
-rw-r--r-- | pylint/pyreverse/vcg_printer.py | 5 | ||||
-rw-r--r-- | pylint/pyreverse/writer.py | 8 | ||||
-rw-r--r-- | tests/pyreverse/data/classes_No_Name.dot | 2 | ||||
-rw-r--r-- | tests/pyreverse/data/classes_No_Name.html | 2 | ||||
-rw-r--r-- | tests/pyreverse/data/classes_No_Name.mmd | 2 | ||||
-rw-r--r-- | tests/pyreverse/data/classes_No_Name.puml | 2 | ||||
-rw-r--r-- | tests/pyreverse/data/classes_colorized.dot | 2 | ||||
-rw-r--r-- | tests/pyreverse/data/classes_colorized.puml | 2 | ||||
-rw-r--r-- | tests/pyreverse/test_diadefs.py | 2 |
16 files changed, 127 insertions, 18 deletions
diff --git a/doc/whatsnew/fragments/6543.feature b/doc/whatsnew/fragments/6543.feature new file mode 100644 index 000000000..0ebb9b19f --- /dev/null +++ b/doc/whatsnew/fragments/6543.feature @@ -0,0 +1,5 @@ +Update ``pyreverse`` to differentiate between aggregations and compositions. +``pyreverse`` checks if it's an Instance or a Call of an object via method parameters (via type hints) +to decide if it's a composition or an aggregation. + +Refs #6543 diff --git a/pylint/pyreverse/diagrams.py b/pylint/pyreverse/diagrams.py index a9dcd1c5e..382d76bf7 100644 --- a/pylint/pyreverse/diagrams.py +++ b/pylint/pyreverse/diagrams.py @@ -214,20 +214,35 @@ class ClassDiagram(Figure, FilterMixIn): self.add_relationship(obj, impl_obj, "implements") except KeyError: continue - # associations link - for name, values in list(node.instance_attrs_type.items()) + list( + + # associations & aggregations links + for name, values in list(node.aggregations_type.items()): + for value in values: + self.assign_association_relationship( + value, obj, name, "aggregation" + ) + + for name, values in list(node.associations_type.items()) + list( node.locals_type.items() ): + for value in values: - if value is astroid.Uninferable: - continue - if isinstance(value, astroid.Instance): - value = value._proxied - try: - associated_obj = self.object_from_node(value) - self.add_relationship(associated_obj, obj, "association", name) - except KeyError: - continue + self.assign_association_relationship( + value, obj, name, "association" + ) + + def assign_association_relationship( + self, value: astroid.NodeNG, obj: ClassEntity, name: str, type_relationship: str + ) -> None: + if value is astroid.Uninferable: + return + if isinstance(value, astroid.Instance): + value = value._proxied + try: + associated_obj = self.object_from_node(value) + self.add_relationship(associated_obj, obj, type_relationship, name) + except KeyError: + return class PackageDiagram(ClassDiagram): diff --git a/pylint/pyreverse/dot_printer.py b/pylint/pyreverse/dot_printer.py index 7579d2877..1d5f2c32b 100644 --- a/pylint/pyreverse/dot_printer.py +++ b/pylint/pyreverse/dot_printer.py @@ -39,6 +39,12 @@ ARROWS: dict[EdgeType, dict[str, str]] = { "arrowhead": "diamond", "style": "solid", }, + EdgeType.AGGREGATION: { + "fontcolor": "green", + "arrowtail": "none", + "arrowhead": "odiamond", + "style": "solid", + }, EdgeType.USES: {"arrowtail": "none", "arrowhead": "open"}, } diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index 042d3845e..8c403ffc6 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -13,6 +13,7 @@ import collections import os import traceback import warnings +from abc import ABC, abstractmethod from collections.abc import Generator from typing import Any, Callable, Optional @@ -123,6 +124,12 @@ class Linker(IdGeneratorMixIn, utils.LocalsVisitor): * instance_attrs_type as locals_type but for klass member attributes (only on astroid.Class) + * associations_type + as instance_attrs_type but for association relationships + + * aggregations_type + as instance_attrs_type but for aggregations relationships + * implements, list of implemented interface _objects_ (only on astroid.Class nodes) """ @@ -134,6 +141,8 @@ class Linker(IdGeneratorMixIn, utils.LocalsVisitor): self.tag = tag # visited project self.project = project + self.associations_handler = AggregationsHandler() + self.associations_handler.set_next(OtherAssociationsHandler()) def visit_project(self, node: Project) -> None: """Visit a pyreverse.utils.Project node. @@ -178,9 +187,12 @@ class Linker(IdGeneratorMixIn, utils.LocalsVisitor): baseobj.specializations = specializations # resolve instance attributes node.instance_attrs_type = collections.defaultdict(list) + node.aggregations_type = collections.defaultdict(list) + node.associations_type = collections.defaultdict(list) 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.handle_assignattr_type(assignattr, node) # resolve implemented interface try: @@ -313,6 +325,61 @@ class Linker(IdGeneratorMixIn, utils.LocalsVisitor): mod_paths.append(mod_path) +class AssociationHandlerInterface(ABC): + @abstractmethod + def set_next( + self, handler: AssociationHandlerInterface + ) -> AssociationHandlerInterface: + pass + + @abstractmethod + def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: + pass + + +class AbstractAssociationHandler(AssociationHandlerInterface): + """ + Chain of Responsibility for handling types of association, useful + to expand in the future if we want to add more distinct associations. + + 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`. + + The default chaining behavior is implemented inside the base handler + class. + """ + + _next_handler: AssociationHandlerInterface + + def set_next( + self, handler: AssociationHandlerInterface + ) -> AssociationHandlerInterface: + self._next_handler = handler + return handler + + @abstractmethod + def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: + if self._next_handler: + self._next_handler.handle(node, parent) + + +class AggregationsHandler(AbstractAssociationHandler): + def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: + if isinstance(node.parent.value, astroid.node_classes.Name): + current = set(parent.aggregations_type[node.attrname]) + parent.aggregations_type[node.attrname] = list( + current | utils.infer_node(node) + ) + else: + super().handle(node, parent) + + +class OtherAssociationsHandler(AbstractAssociationHandler): + def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None: + current = set(parent.associations_type[node.attrname]) + parent.associations_type[node.attrname] = list(current | utils.infer_node(node)) + + def project_from_files( files: list[str], func_wrapper: _WrapperFuncT = _astroid_wrapper, diff --git a/pylint/pyreverse/mermaidjs_printer.py b/pylint/pyreverse/mermaidjs_printer.py index 07cad97be..a8f3c576b 100644 --- a/pylint/pyreverse/mermaidjs_printer.py +++ b/pylint/pyreverse/mermaidjs_printer.py @@ -24,6 +24,7 @@ class MermaidJSPrinter(Printer): EdgeType.INHERITS: "--|>", EdgeType.IMPLEMENTS: "..|>", EdgeType.ASSOCIATION: "--*", + EdgeType.AGGREGATION: "--o", EdgeType.USES: "-->", } diff --git a/pylint/pyreverse/plantuml_printer.py b/pylint/pyreverse/plantuml_printer.py index f3f639f5c..56463165d 100644 --- a/pylint/pyreverse/plantuml_printer.py +++ b/pylint/pyreverse/plantuml_printer.py @@ -24,6 +24,7 @@ class PlantUmlPrinter(Printer): EdgeType.INHERITS: "--|>", EdgeType.IMPLEMENTS: "..|>", EdgeType.ASSOCIATION: "--*", + EdgeType.AGGREGATION: "--o", EdgeType.USES: "-->", } diff --git a/pylint/pyreverse/printer.py b/pylint/pyreverse/printer.py index 55ce2c8b1..cdbf7e3c8 100644 --- a/pylint/pyreverse/printer.py +++ b/pylint/pyreverse/printer.py @@ -25,6 +25,7 @@ class EdgeType(Enum): INHERITS = "inherits" IMPLEMENTS = "implements" ASSOCIATION = "association" + AGGREGATION = "aggregation" USES = "uses" diff --git a/pylint/pyreverse/vcg_printer.py b/pylint/pyreverse/vcg_printer.py index 6f28a24e8..b9e2e94f3 100644 --- a/pylint/pyreverse/vcg_printer.py +++ b/pylint/pyreverse/vcg_printer.py @@ -177,6 +177,11 @@ ARROWS: dict[EdgeType, dict[str, str | int]] = { "backarrowstyle": "none", "textcolor": "green", }, + EdgeType.AGGREGATION: { + "arrowstyle": "solid", + "backarrowstyle": "none", + "textcolor": "green", + }, } ORIENTATION: dict[Layout, str] = { Layout.LEFT_TO_RIGHT: "left_to_right", diff --git a/pylint/pyreverse/writer.py b/pylint/pyreverse/writer.py index b19286a87..68a49eea1 100644 --- a/pylint/pyreverse/writer.py +++ b/pylint/pyreverse/writer.py @@ -120,6 +120,14 @@ class DiagramWriter: label=rel.name, type_=EdgeType.ASSOCIATION, ) + # generate aggregations + for rel in diagram.get_relationships("aggregation"): + self.printer.emit_edge( + rel.from_object.fig_id, + rel.to_object.fig_id, + label=rel.name, + type_=EdgeType.AGGREGATION, + ) def set_printer(self, file_name: str, basename: str) -> None: """Set printer.""" diff --git a/tests/pyreverse/data/classes_No_Name.dot b/tests/pyreverse/data/classes_No_Name.dot index 01e3619d6..a598ab6d9 100644 --- a/tests/pyreverse/data/classes_No_Name.dot +++ b/tests/pyreverse/data/classes_No_Name.dot @@ -13,5 +13,5 @@ charset="utf-8" "data.clientmodule_test.Ancestor" -> "data.suppliermodule_test.Interface" [arrowhead="empty", arrowtail="node", style="dashed"]; "data.suppliermodule_test.DoNothing" -> "data.clientmodule_test.Ancestor" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="cls_member", style="solid"]; "data.suppliermodule_test.DoNothing" -> "data.clientmodule_test.Specialization" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="relation", style="solid"]; -"data.suppliermodule_test.DoNothing2" -> "data.clientmodule_test.Specialization" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="relation2", style="solid"]; +"data.suppliermodule_test.DoNothing2" -> "data.clientmodule_test.Specialization" [arrowhead="odiamond", arrowtail="none", fontcolor="green", label="relation2", style="solid"]; } diff --git a/tests/pyreverse/data/classes_No_Name.html b/tests/pyreverse/data/classes_No_Name.html index 8ec309d33..602f2e3b7 100644 --- a/tests/pyreverse/data/classes_No_Name.html +++ b/tests/pyreverse/data/classes_No_Name.html @@ -43,7 +43,7 @@ Ancestor ..|> Interface DoNothing --* Ancestor : cls_member DoNothing --* Specialization : relation - DoNothing2 --* Specialization : relation2 + DoNothing2 --o Specialization : relation2 </div> </body> diff --git a/tests/pyreverse/data/classes_No_Name.mmd b/tests/pyreverse/data/classes_No_Name.mmd index 9b8397206..1db88b2ae 100644 --- a/tests/pyreverse/data/classes_No_Name.mmd +++ b/tests/pyreverse/data/classes_No_Name.mmd @@ -38,4 +38,4 @@ classDiagram Ancestor ..|> Interface DoNothing --* Ancestor : cls_member DoNothing --* Specialization : relation - DoNothing2 --* Specialization : relation2 + DoNothing2 --o Specialization : relation2 diff --git a/tests/pyreverse/data/classes_No_Name.puml b/tests/pyreverse/data/classes_No_Name.puml index ee3fa2bd4..837e6865c 100644 --- a/tests/pyreverse/data/classes_No_Name.puml +++ b/tests/pyreverse/data/classes_No_Name.puml @@ -39,5 +39,5 @@ data.clientmodule_test.Specialization --|> data.clientmodule_test.Ancestor data.clientmodule_test.Ancestor ..|> data.suppliermodule_test.Interface data.suppliermodule_test.DoNothing --* data.clientmodule_test.Ancestor : cls_member data.suppliermodule_test.DoNothing --* data.clientmodule_test.Specialization : relation -data.suppliermodule_test.DoNothing2 --* data.clientmodule_test.Specialization : relation2 +data.suppliermodule_test.DoNothing2 --o data.clientmodule_test.Specialization : relation2 @enduml diff --git a/tests/pyreverse/data/classes_colorized.dot b/tests/pyreverse/data/classes_colorized.dot index 09f72b782..4ff12a819 100644 --- a/tests/pyreverse/data/classes_colorized.dot +++ b/tests/pyreverse/data/classes_colorized.dot @@ -13,5 +13,5 @@ charset="utf-8" "data.clientmodule_test.Ancestor" -> "data.suppliermodule_test.Interface" [arrowhead="empty", arrowtail="node", style="dashed"]; "data.suppliermodule_test.DoNothing" -> "data.clientmodule_test.Ancestor" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="cls_member", style="solid"]; "data.suppliermodule_test.DoNothing" -> "data.clientmodule_test.Specialization" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="relation", style="solid"]; -"data.suppliermodule_test.DoNothing2" -> "data.clientmodule_test.Specialization" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="relation2", style="solid"]; +"data.suppliermodule_test.DoNothing2" -> "data.clientmodule_test.Specialization" [arrowhead="odiamond", arrowtail="none", fontcolor="green", label="relation2", style="solid"]; } diff --git a/tests/pyreverse/data/classes_colorized.puml b/tests/pyreverse/data/classes_colorized.puml index 2b450207a..7398ee60f 100644 --- a/tests/pyreverse/data/classes_colorized.puml +++ b/tests/pyreverse/data/classes_colorized.puml @@ -39,5 +39,5 @@ data.clientmodule_test.Specialization --|> data.clientmodule_test.Ancestor data.clientmodule_test.Ancestor ..|> data.suppliermodule_test.Interface data.suppliermodule_test.DoNothing --* data.clientmodule_test.Ancestor : cls_member data.suppliermodule_test.DoNothing --* data.clientmodule_test.Specialization : relation -data.suppliermodule_test.DoNothing2 --* data.clientmodule_test.Specialization : relation2 +data.suppliermodule_test.DoNothing2 --o data.clientmodule_test.Specialization : relation2 @enduml diff --git a/tests/pyreverse/test_diadefs.py b/tests/pyreverse/test_diadefs.py index 9a550c5db..da16eea33 100644 --- a/tests/pyreverse/test_diadefs.py +++ b/tests/pyreverse/test_diadefs.py @@ -99,9 +99,9 @@ def test_default_values() -> None: class TestDefaultDiadefGenerator: _should_rels = [ + ("aggregation", "DoNothing2", "Specialization"), ("association", "DoNothing", "Ancestor"), ("association", "DoNothing", "Specialization"), - ("association", "DoNothing2", "Specialization"), ("implements", "Ancestor", "Interface"), ("specialization", "Specialization", "Ancestor"), ] |