Skip to content

Pyreverse: Use dashed lines for type-checking imports #8824

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions pylint/pyreverse/diagrams.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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."""
Expand All @@ -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:
continue
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two lines are uncovered, and I don't know how to cover them. Any ideas? They could also be cut, or just ignore the coverage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check is definitely required. But I can't figure out exactly what triggers it.

self.add_relationship(package_obj, dep, "type_depends")
1 change: 1 addition & 0 deletions pylint/pyreverse/dot_printer.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class HTMLLabels(Enum):
"style": "solid",
},
EdgeType.USES: {"arrowtail": "none", "arrowhead": "open"},
EdgeType.TYPE_USES: {"arrowtail": "none", "arrowhead": "open", "style": "dashed"},
}


Expand Down
1 change: 1 addition & 0 deletions pylint/pyreverse/inspector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
1 change: 1 addition & 0 deletions pylint/pyreverse/mermaidjs_printer.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class MermaidJSPrinter(Printer):
EdgeType.ASSOCIATION: "--*",
EdgeType.AGGREGATION: "--o",
EdgeType.USES: "-->",
EdgeType.TYPE_USES: "-.->",
}

def _open_graph(self) -> None:
Expand Down
1 change: 1 addition & 0 deletions pylint/pyreverse/printer.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class EdgeType(Enum):
ASSOCIATION = "association"
AGGREGATION = "aggregation"
USES = "uses"
TYPE_USES = "type_uses"


class Layout(Enum):
Expand Down
7 changes: 7 additions & 0 deletions pylint/pyreverse/writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_USES,
)

def write_classes(self, diagram: ClassDiagram) -> None:
"""Write a class diagram."""
# sorted to get predictable (hence testable) results
Expand Down
7 changes: 7 additions & 0 deletions tests/pyreverse/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ def no_standalone_dot_config() -> PyreverseConfig:
)


@pytest.fixture()
def type_check_imports_dot_config() -> PyreverseConfig:
return PyreverseConfig(
output_format="dot",
)


@pytest.fixture()
def puml_config() -> PyreverseConfig:
return PyreverseConfig(
Expand Down
4 changes: 4 additions & 0 deletions tests/pyreverse/data/classes_type_check_imports.dot
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
digraph "classes_type_check_imports" {
rankdir=BT
charset="utf-8"
}
13 changes: 13 additions & 0 deletions tests/pyreverse/data/packages_type_check_imports.dot
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
digraph "packages_type_check_imports" {
rankdir=BT
charset="utf-8"
"package_diagrams" [color="black", label=<package_diagrams>, shape="box", style="solid"];
"package_diagrams.type_check_imports" [color="black", label=<package_diagrams.type_check_imports>, shape="box", style="solid"];
"package_diagrams.type_check_imports.mod_a" [color="black", label=<package_diagrams.type_check_imports.mod_a>, shape="box", style="solid"];
"package_diagrams.type_check_imports.mod_b" [color="black", label=<package_diagrams.type_check_imports.mod_b>, shape="box", style="solid"];
"package_diagrams.type_check_imports.mod_c" [color="black", label=<package_diagrams.type_check_imports.mod_c>, shape="box", style="solid"];
"package_diagrams.type_check_imports.mod_d" [color="black", label=<package_diagrams.type_check_imports.mod_d>, 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"];
}
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Int = int

List = list
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from typing import Any

from mod_a import Int


def is_int(x) -> bool:
return isinstance(x, Int)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from typing import TYPE_CHECKING

if TYPE_CHECKING:
from mod_a import Int

def some_int() -> Int:
return 5
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from typing import TYPE_CHECKING, Any

from mod_a import Int

if TYPE_CHECKING:
from mod_a import List

def list_int(x: Any) -> List[Int]:
return [x] if isinstance(x, Int) else []
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

23 changes: 23 additions & 0 deletions tests/pyreverse/test_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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(
type_check_imports_dot_config: PyreverseConfig, get_project: GetProjectCallable
) -> Iterator[None]:
writer = DiagramWriter(type_check_imports_dot_config)
project = get_project(
os.path.join(os.path.dirname(__file__), "functional", "package_diagrams"),
name="type_check_imports",
)

yield from _setup(project, type_check_imports_dot_config, writer)


@pytest.fixture()
def setup_puml(
puml_config: PyreverseConfig, get_project: GetProjectCallable
Expand Down Expand Up @@ -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:
Expand Down