tests: add testcase that shows that pyreverse will not extract the inheritance link for a flat folder as described in #7686

[Hannoma]

Type of Changes

	Type
✓	🐛 Bug fix

Description

As described in #7686, pyreverse does not extract the inheritance link for classes living in different files. In this PR I haved added the possibility to easily write testcases for pyreverse to generate a diagram for a complete directory / module. As far as I can tell this was not possible before just using the functional tests.

I also have a possible fix in mind for this issue, but am not sure, if I should add it in this PR or open another one. As @DudeNr33 suggested, I started with writing the test to reproduce the described issue.

Refs #7686

[Hannoma]

What I have found out while debugging:
When the node.ancestors(recurs=False) function is called in diagrams.py lines 213-219

            # inheritance link
            for par_node in node.ancestors(recurs=False):
                try:
                    par_obj = self.object_from_node(par_node)
                    self.add_relationship(obj, par_obj, "specialization")
                except KeyError:
                    continue

the ancestors are not extracted correctly as the cached InferenceContext maps the statement of the name of the base classes to "Uninferable"

[Hannoma]

As I really do not know what is going on there in astroid, my quick and dirty solution would be to extract the correct base classes via their name:

            # inheritance links
            for par_node in node.bases:
                # Name nodes are used for simple types, e.g. class A(B):
                if isinstance(par_node, astroid.Name):
                    try:
                        par_obj = self.classe(par_node.name)
                        self.add_relationship(obj, par_obj, "specialization")
                    except KeyError:
                        continue
                # Subscript is used for generic types, e.g. class A(Generic[T]):
                if isinstance(par_node, astroid.Subscript):
                    try:
                        par_obj = self.classe(par_node.value.name)
                        self.add_relationship(obj, par_obj, "specialization")
                    except KeyError:
                        continue

I have pushed this simple fix to https://github.com/ANTICIPATE-GmbH/pylint/tree/fix/7686/extract-inheritance-relations-correctly. Note that with this fix, all test cases complete without issues.

[DudeNr33]

Thanks for taking this up. I have not found the time yet to check if the problem itself could/should be fixed in astroid rather than pylint, so I will just focus on the testing extension for now:

I would indeed suggest to keep the testing extension and the concrete fix as two separate merge requests. I left some first thoughts.

It would be great if this could be used for functional tests of package diagrams as well, though this could be added later in a separate MR.

[github-actions]

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit d99f984

[DudeNr33]

I tried it out locally and I quite like it.
As mentioned I would like to expand on this to also include a functional test harness for package diagrams.
There already exists some test data under tests/pyreverse/functional/package_diagrams, which however does not build upon test_pyreverse_functional.py to collect the test data.

With your change and some very minor modifications, we can move these files into the new tests/pyreverse/functional/packages directory (we can do that in a follow up PR, just want to note down what I did locally):

diff --git a/pylint/testutils/pyreverse.py b/pylint/testutils/pyreverse.py
index 9e6647183..03840944e 100644
--- a/pylint/testutils/pyreverse.py
+++ b/pylint/testutils/pyreverse.py
@@ -72,6 +72,7 @@ class TestFileOptions(TypedDict):
     source_roots: list[str]
     output_formats: list[str]
     command_line_args: list[str]
+    diagram_type: str
 
 
 class FunctionalPyreverseTestfile(NamedTuple):
@@ -150,4 +151,5 @@ def _read_config(config_file: Path) -> TestFileOptions:
         "command_line_args": shlex.split(
             config.get("testoptions", "command_line_args", fallback="")
         ),
+        "diagram_type": config.get("testoptions", "diagram_type", fallback="classes"),
     }
diff --git a/tests/pyreverse/test_pyreverse_functional.py b/tests/pyreverse/test_pyreverse_functional.py
index beeb0330b..e02574463 100644
--- a/tests/pyreverse/test_pyreverse_functional.py
+++ b/tests/pyreverse/test_pyreverse_functional.py
@@ -86,5 +86,5 @@ def test_packages(test_package: FunctionalPyreverseTestfile, tmp_path: Path) ->
             Run(args)
         assert sys_exit.value.code == 0
         assert output_file.read_text(encoding="utf8") == (
-            tmp_path / f"classes.{output_format}"
+            tmp_path / f"{test_package.options['diagram_type']}.{output_format}"
         ).read_text(encoding="utf8")

This essentially just adds an option to the .rc file so that the author can specify if his tests generate a class or a package diagram.

As a next step regarding this MR, I think we can simplify the code a bit. For example, the get_functional_test_files() and get_functional_test_packages() in the testutils package share some common logic to handle the .rc files, and the test_class_diagrams() and test_packages() in test_pyreverse_functional as well.

For the suggested fix, I am hesitant to judge if this is the best way to fix it, as I currently don't find the time to work myself back into astroid and debug this issue to understand what is going on. Maybe one of the other maintainers has time to take a look (maybe @nickdrozd?) , otherwise I have to ask for your patience until I can take the time to properly look at it.