From ffe1e714a3cfe83b5a627ade6d723489f2e10621 Mon Sep 17 00:00:00 2001 From: Nick Drozd Date: Tue, 11 Jul 2023 03:02:40 -0400 Subject: [PATCH] Pyreverse: Use dashed lines for type-checking imports (#8824) --- doc/whatsnew/fragments/8112.feature | 3 +++ pylint/pyreverse/diagrams.py | 21 +++++++++++++---- pylint/pyreverse/dot_printer.py | 5 ++++ pylint/pyreverse/inspector.py | 1 + pylint/pyreverse/mermaidjs_printer.py | 1 + pylint/pyreverse/plantuml_printer.py | 1 + pylint/pyreverse/printer.py | 1 + pylint/pyreverse/writer.py | 7 ++++++ .../data/classes_type_check_imports.dot | 4 ++++ .../data/packages_type_check_imports.dot | 13 +++++++++++ .../functional/package_diagrams/__init__.py | 0 .../type_check_imports/__init__.py | 0 .../type_check_imports/mod_a.py | 3 +++ .../type_check_imports/mod_b.py | 7 ++++++ .../type_check_imports/mod_c.py | 7 ++++++ .../type_check_imports/mod_d.py | 11 +++++++++ .../type_check_imports/packages.mmd | 14 +++++++++++ tests/pyreverse/test_writer.py | 23 +++++++++++++++++++ 18 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 doc/whatsnew/fragments/8112.feature create mode 100644 tests/pyreverse/data/classes_type_check_imports.dot create mode 100644 tests/pyreverse/data/packages_type_check_imports.dot create mode 100644 tests/pyreverse/functional/package_diagrams/__init__.py create mode 100644 tests/pyreverse/functional/package_diagrams/type_check_imports/__init__.py create mode 100644 tests/pyreverse/functional/package_diagrams/type_check_imports/mod_a.py create mode 100644 tests/pyreverse/functional/package_diagrams/type_check_imports/mod_b.py create mode 100644 tests/pyreverse/functional/package_diagrams/type_check_imports/mod_c.py create mode 100644 tests/pyreverse/functional/package_diagrams/type_check_imports/mod_d.py create mode 100644 tests/pyreverse/functional/package_diagrams/type_check_imports/packages.mmd diff --git a/doc/whatsnew/fragments/8112.feature b/doc/whatsnew/fragments/8112.feature new file mode 100644 index 0000000000..cee1f37a86 --- /dev/null +++ b/doc/whatsnew/fragments/8112.feature @@ -0,0 +1,3 @@ +In Pyreverse package dependency diagrams, show when a module imports another only for type-checking. + +Closes #8112 diff --git a/pylint/pyreverse/diagrams.py b/pylint/pyreverse/diagrams.py index bc2ba18c5b..41db5ff5a9 100644 --- a/pylint/pyreverse/diagrams.py +++ b/pylint/pyreverse/diagrams.py @@ -12,7 +12,7 @@ import astroid from astroid import nodes, util -from pylint.checkers.utils import decorated_with_property +from pylint.checkers.utils import decorated_with_property, in_type_checking_block from pylint.pyreverse.utils import FilterMixIn @@ -281,9 +281,15 @@ def get_module(self, name: str, node: nodes.Module) -> PackageEntity: def add_from_depend(self, node: nodes.ImportFrom, from_module: str) -> None: """Add dependencies created by from-imports.""" mod_name = node.root().name - obj = self.module(mod_name) - if from_module not in obj.node.depends: - obj.node.depends.append(from_module) + package = self.module(mod_name).node + + if from_module in package.depends: + return + + if not in_type_checking_block(node): + package.depends.append(from_module) + elif from_module not in package.type_depends: + package.type_depends.append(from_module) def extract_relationships(self) -> None: """Extract relationships between nodes in the diagram.""" @@ -304,3 +310,10 @@ def extract_relationships(self) -> None: except KeyError: continue self.add_relationship(package_obj, dep, "depends") + + for dep_name in package_obj.node.type_depends: + try: + dep = self.get_module(dep_name, package_obj.node) + except KeyError: # pragma: no cover + continue + self.add_relationship(package_obj, dep, "type_depends") diff --git a/pylint/pyreverse/dot_printer.py b/pylint/pyreverse/dot_printer.py index edaea2384e..ddd895d309 100644 --- a/pylint/pyreverse/dot_printer.py +++ b/pylint/pyreverse/dot_printer.py @@ -43,6 +43,11 @@ class HTMLLabels(Enum): "style": "solid", }, EdgeType.USES: {"arrowtail": "none", "arrowhead": "open"}, + EdgeType.TYPE_DEPENDENCY: { + "arrowtail": "none", + "arrowhead": "open", + "style": "dashed", + }, } diff --git a/pylint/pyreverse/inspector.py b/pylint/pyreverse/inspector.py index 830479219a..a00a330993 100644 --- a/pylint/pyreverse/inspector.py +++ b/pylint/pyreverse/inspector.py @@ -139,6 +139,7 @@ def visit_module(self, node: nodes.Module) -> None: return node.locals_type = collections.defaultdict(list) node.depends = [] + node.type_depends = [] if self.tag: node.uid = self.generate_id() diff --git a/pylint/pyreverse/mermaidjs_printer.py b/pylint/pyreverse/mermaidjs_printer.py index 61d0d79486..b0b64e0b43 100644 --- a/pylint/pyreverse/mermaidjs_printer.py +++ b/pylint/pyreverse/mermaidjs_printer.py @@ -24,6 +24,7 @@ class MermaidJSPrinter(Printer): EdgeType.ASSOCIATION: "--*", EdgeType.AGGREGATION: "--o", EdgeType.USES: "-->", + EdgeType.TYPE_DEPENDENCY: "-.->", } def _open_graph(self) -> None: diff --git a/pylint/pyreverse/plantuml_printer.py b/pylint/pyreverse/plantuml_printer.py index 5f703b62eb..379d57a4c6 100644 --- a/pylint/pyreverse/plantuml_printer.py +++ b/pylint/pyreverse/plantuml_printer.py @@ -24,6 +24,7 @@ class PlantUmlPrinter(Printer): EdgeType.ASSOCIATION: "--*", EdgeType.AGGREGATION: "--o", EdgeType.USES: "-->", + EdgeType.TYPE_DEPENDENCY: "..>", } def _open_graph(self) -> None: diff --git a/pylint/pyreverse/printer.py b/pylint/pyreverse/printer.py index f08c746024..caa7917ca0 100644 --- a/pylint/pyreverse/printer.py +++ b/pylint/pyreverse/printer.py @@ -25,6 +25,7 @@ class EdgeType(Enum): ASSOCIATION = "association" AGGREGATION = "aggregation" USES = "uses" + TYPE_DEPENDENCY = "type_dependency" class Layout(Enum): diff --git a/pylint/pyreverse/writer.py b/pylint/pyreverse/writer.py index 91190c0669..5ef5420496 100644 --- a/pylint/pyreverse/writer.py +++ b/pylint/pyreverse/writer.py @@ -76,6 +76,13 @@ def write_packages(self, diagram: PackageDiagram) -> None: type_=EdgeType.USES, ) + for rel in diagram.get_relationships("type_depends"): + self.printer.emit_edge( + rel.from_object.fig_id, + rel.to_object.fig_id, + type_=EdgeType.TYPE_DEPENDENCY, + ) + def write_classes(self, diagram: ClassDiagram) -> None: """Write a class diagram.""" # sorted to get predictable (hence testable) results diff --git a/tests/pyreverse/data/classes_type_check_imports.dot b/tests/pyreverse/data/classes_type_check_imports.dot new file mode 100644 index 0000000000..592d9716ca --- /dev/null +++ b/tests/pyreverse/data/classes_type_check_imports.dot @@ -0,0 +1,4 @@ +digraph "classes_type_check_imports" { +rankdir=BT +charset="utf-8" +} diff --git a/tests/pyreverse/data/packages_type_check_imports.dot b/tests/pyreverse/data/packages_type_check_imports.dot new file mode 100644 index 0000000000..e44ed82444 --- /dev/null +++ b/tests/pyreverse/data/packages_type_check_imports.dot @@ -0,0 +1,13 @@ +digraph "packages_type_check_imports" { +rankdir=BT +charset="utf-8" +"package_diagrams" [color="black", label=, shape="box", style="solid"]; +"package_diagrams.type_check_imports" [color="black", label=, shape="box", style="solid"]; +"package_diagrams.type_check_imports.mod_a" [color="black", label=, shape="box", style="solid"]; +"package_diagrams.type_check_imports.mod_b" [color="black", label=, shape="box", style="solid"]; +"package_diagrams.type_check_imports.mod_c" [color="black", label=, shape="box", style="solid"]; +"package_diagrams.type_check_imports.mod_d" [color="black", label=, shape="box", style="solid"]; +"package_diagrams.type_check_imports.mod_b" -> "package_diagrams.type_check_imports.mod_a" [arrowhead="open", arrowtail="none"]; +"package_diagrams.type_check_imports.mod_d" -> "package_diagrams.type_check_imports.mod_a" [arrowhead="open", arrowtail="none"]; +"package_diagrams.type_check_imports.mod_c" -> "package_diagrams.type_check_imports.mod_a" [arrowhead="open", arrowtail="none", style="dashed"]; +} diff --git a/tests/pyreverse/functional/package_diagrams/__init__.py b/tests/pyreverse/functional/package_diagrams/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/pyreverse/functional/package_diagrams/type_check_imports/__init__.py b/tests/pyreverse/functional/package_diagrams/type_check_imports/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/pyreverse/functional/package_diagrams/type_check_imports/mod_a.py b/tests/pyreverse/functional/package_diagrams/type_check_imports/mod_a.py new file mode 100644 index 0000000000..bceec8d697 --- /dev/null +++ b/tests/pyreverse/functional/package_diagrams/type_check_imports/mod_a.py @@ -0,0 +1,3 @@ +Int = int + +List = list diff --git a/tests/pyreverse/functional/package_diagrams/type_check_imports/mod_b.py b/tests/pyreverse/functional/package_diagrams/type_check_imports/mod_b.py new file mode 100644 index 0000000000..2fdc98b54c --- /dev/null +++ b/tests/pyreverse/functional/package_diagrams/type_check_imports/mod_b.py @@ -0,0 +1,7 @@ +from typing import Any + +from mod_a import Int + + +def is_int(x) -> bool: + return isinstance(x, Int) diff --git a/tests/pyreverse/functional/package_diagrams/type_check_imports/mod_c.py b/tests/pyreverse/functional/package_diagrams/type_check_imports/mod_c.py new file mode 100644 index 0000000000..bf4a46dcb1 --- /dev/null +++ b/tests/pyreverse/functional/package_diagrams/type_check_imports/mod_c.py @@ -0,0 +1,7 @@ +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from mod_a import Int + +def some_int() -> Int: + return 5 diff --git a/tests/pyreverse/functional/package_diagrams/type_check_imports/mod_d.py b/tests/pyreverse/functional/package_diagrams/type_check_imports/mod_d.py new file mode 100644 index 0000000000..af61065db1 --- /dev/null +++ b/tests/pyreverse/functional/package_diagrams/type_check_imports/mod_d.py @@ -0,0 +1,11 @@ +from typing import TYPE_CHECKING + +from mod_a import Int + +if TYPE_CHECKING: + from typing import Any + + from mod_a import List + +def list_int(x: Any) -> List[Int]: + return [x] if isinstance(x, Int) else [] diff --git a/tests/pyreverse/functional/package_diagrams/type_check_imports/packages.mmd b/tests/pyreverse/functional/package_diagrams/type_check_imports/packages.mmd new file mode 100644 index 0000000000..91f0e65f73 --- /dev/null +++ b/tests/pyreverse/functional/package_diagrams/type_check_imports/packages.mmd @@ -0,0 +1,14 @@ +classDiagram + class type_check_imports { + } + class mod_a { + } + class mod_b { + } + class mod_c { + } + class mod_d { + } + mod_b --> mod_a + mod_d --> mod_a + mod_c -.-> mod_a diff --git a/tests/pyreverse/test_writer.py b/tests/pyreverse/test_writer.py index ed0f5b31ee..d3a2ce5aae 100644 --- a/tests/pyreverse/test_writer.py +++ b/tests/pyreverse/test_writer.py @@ -49,6 +49,10 @@ MMD_FILES = ["packages_No_Name.mmd", "classes_No_Name.mmd"] HTML_FILES = ["packages_No_Name.html", "classes_No_Name.html"] NO_STANDALONE_FILES = ["classes_no_standalone.dot", "packages_no_standalone.dot"] +TYPE_CHECK_IMPORTS_FILES = [ + "packages_type_check_imports.dot", + "classes_type_check_imports.dot", +] class Config: @@ -105,6 +109,19 @@ def setup_no_standalone_dot( yield from _setup(project, no_standalone_dot_config, writer) +@pytest.fixture() +def setup_type_check_imports_dot( + default_config: PyreverseConfig, get_project: GetProjectCallable +) -> Iterator[None]: + writer = DiagramWriter(default_config) + project = get_project( + os.path.join(os.path.dirname(__file__), "functional", "package_diagrams"), + name="type_check_imports", + ) + + yield from _setup(project, default_config, writer) + + @pytest.fixture() def setup_puml( puml_config: PyreverseConfig, get_project: GetProjectCallable @@ -173,6 +190,12 @@ def test_no_standalone_dot_files(generated_file: str) -> None: _assert_files_are_equal(generated_file) +@pytest.mark.usefixtures("setup_type_check_imports_dot") +@pytest.mark.parametrize("generated_file", TYPE_CHECK_IMPORTS_FILES) +def test_type_check_imports_dot_files(generated_file: str) -> None: + _assert_files_are_equal(generated_file) + + @pytest.mark.usefixtures("setup_puml") @pytest.mark.parametrize("generated_file", PUML_FILES) def test_puml_files(generated_file: str) -> None: