pyreverse: better error messages for unsupported file formats (#5951)

* Pyreverse: better error messages for unsupported file formats

* Apply suggestions from code review.

Author: DudeNr33

ChangeLog

@@ -17,6 +17,10 @@ Release date: TBA
   Ref PyCQA/astroid#1360
   Closes #4826
 
+* Output better error message if unsupported file formats are used with ``pyreverse``.
+
+  Closes #5950
+
 * Fix pyreverse diagrams type hinting for classmethods and staticmethods.
 
 * Fix pyreverse diagrams type hinting for methods returning None.

doc/whatsnew/2.13.rst

@@ -515,3 +515,7 @@ Other Changes
   Closes #5333
 
 * Fix type hints in class diagrams generated by pyreverse for class methods and methods returning None.
+
+* Output better error message if unsupported file formats are used with ``pyreverse``.
+
+  Closes #5950

pylint/pyreverse/dot_printer.py

@@ -19,7 +19,7 @@
 from astroid import nodes
 
 from pylint.pyreverse.printer import EdgeType, Layout, NodeProperties, NodeType, Printer
-from pylint.pyreverse.utils import check_graphviz_availability, get_annotation_label
+from pylint.pyreverse.utils import get_annotation_label
 
 ALLOWED_CHARSETS: FrozenSet[str] = frozenset(("utf-8", "iso-8859-1", "latin1"))
 SHAPES: Dict[NodeType, str] = {
@@ -152,7 +152,6 @@ def generate(self, outputfile: str) -> None:
         with open(dot_sourcepath, "w", encoding="utf8") as outfile:
             outfile.writelines(self.lines)
         if target not in graphviz_extensions:
-            check_graphviz_availability()
             use_shell = sys.platform == "win32"
             subprocess.call(
                 ["dot", "-T", target, dot_sourcepath, "-o", outputfile],

pylint/pyreverse/main.py

@@ -32,7 +32,20 @@
 from pylint.pyreverse import writer
 from pylint.pyreverse.diadefslib import DiadefsHandler
 from pylint.pyreverse.inspector import Linker, project_from_files
-from pylint.pyreverse.utils import check_graphviz_availability, insert_default_options
+from pylint.pyreverse.utils import (
+    check_graphviz_availability,
+    check_if_graphviz_supports_format,
+    insert_default_options,
+)
+
+DIRECTLY_SUPPORTED_FORMATS = (
+    "dot",
+    "vcg",
+    "puml",
+    "plantuml",
+    "mmd",
+    "html",
+)
 
 OPTIONS = (
     (
@@ -139,7 +152,10 @@
             action="store",
             default="dot",
             metavar="<format>",
-            help="create a *.<format> output file if format available.",
+            help=(
+                f"create a *.<format> output file if format is available. Available formats are: {', '.join(DIRECTLY_SUPPORTED_FORMATS)}. "
+                f"Any other format will be tried to create by means of the 'dot' command line tool, which requires a graphviz installation."
+            ),
         ),
     ),
     (
@@ -205,15 +221,12 @@ def __init__(self, args: Iterable[str]):
         super().__init__(usage=__doc__)
         insert_default_options()
         args = self.load_command_line_configuration(args)
-        if self.config.output_format not in (
-            "dot",
-            "vcg",
-            "puml",
-            "plantuml",
-            "mmd",
-            "html",
-        ):
+        if self.config.output_format not in DIRECTLY_SUPPORTED_FORMATS:
             check_graphviz_availability()
+            print(
+                f"Format {self.config.output_format} is not supported natively. Pyreverse will try to generate it using Graphviz..."
+            )
+            check_if_graphviz_supports_format(self.config.output_format)
 
         sys.exit(self.run(args))
 

pylint/pyreverse/utils.py

@@ -23,6 +23,7 @@
 import os
 import re
 import shutil
+import subprocess
 import sys
 from typing import Optional, Union
 
@@ -290,9 +291,33 @@ def check_graphviz_availability():
     from *.dot or *.gv into the final output format.
     """
     if shutil.which("dot") is None:
+        print("'Graphviz' needs to be installed for your chosen output format.")
+        sys.exit(32)
+
+
+def check_if_graphviz_supports_format(output_format: str) -> None:
+    """Check if the ``dot`` command supports the requested output format.
+
+    This is needed if image output is desired and ``dot`` is used to convert
+    from *.gv into the final output format.
+    """
+    dot_output = subprocess.run(
+        ["dot", "-T?"], capture_output=True, check=False, encoding="utf-8"
+    )
+    match = re.match(
+        pattern=r".*Use one of: (?P<formats>(\S*\s?)+)",
+        string=dot_output.stderr.strip(),
+    )
+    if not match:
+        print(
+            "Unable to determine Graphviz supported output formats. "
+            "Pyreverse will continue, but subsequent error messages "
+            "regarding the output format may come from Graphviz directly."
+        )
+        return
+    supported_formats = match.group("formats")
+    if output_format not in supported_formats.split():
         print(
-            "The requested output format is currently not available.\n"
-            "Please install 'Graphviz' to have other output formats "
-            "than 'dot' or 'vcg'."
+            f"Format {output_format} is not supported by Graphviz. It supports: {supported_formats}"
         )
         sys.exit(32)

tests/pyreverse/test_main.py

@@ -2,15 +2,39 @@
 import os
 import sys
 from typing import Iterator
+from unittest import mock
 
 import pytest
 
 from pylint.lint import fix_import_path
+from pylint.pyreverse import main
 
 TEST_DATA_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "data"))
 PROJECT_ROOT_DIR = os.path.abspath(os.path.join(TEST_DATA_DIR, ".."))
 
 
+@pytest.fixture(name="mock_subprocess")
+def mock_utils_subprocess():
+    with mock.patch("pylint.pyreverse.utils.subprocess") as mock_subprocess:
+        yield mock_subprocess
+
+
+@pytest.fixture
+def mock_graphviz(mock_subprocess):
+    mock_subprocess.run.return_value = mock.Mock(
+        stderr=(
+            'Format: "XYZ" not recognized. Use one of: '
+            "bmp canon cgimage cmap cmapx cmapx_np dot dot_json eps exr fig gd "
+            "gd2 gif gv icns ico imap imap_np ismap jp2 jpe jpeg jpg json json0 "
+            "mp pct pdf pic pict plain plain-ext png pov ps ps2 psd sgi svg svgz "
+            "tga tif tiff tk vdx vml vmlz vrml wbmp webp xdot xdot1.2 xdot1.4 xdot_json"
+        )
+    )
+    with mock.patch("pylint.pyreverse.utils.shutil") as mock_shutil:
+        mock_shutil.which.return_value = "/usr/bin/dot"
+        yield
+
+
 @pytest.fixture(params=[PROJECT_ROOT_DIR, TEST_DATA_DIR])
 def setup_path(request) -> Iterator:
     current_sys_path = list(sys.path)
@@ -29,3 +53,64 @@ def test_project_root_in_sys_path():
     """
     with fix_import_path([TEST_DATA_DIR]):
         assert sys.path == [PROJECT_ROOT_DIR]
+
+
+@mock.patch("pylint.pyreverse.main.Linker", new=mock.MagicMock())
+@mock.patch("pylint.pyreverse.main.DiadefsHandler", new=mock.MagicMock())
+@mock.patch("pylint.pyreverse.main.writer")
+@pytest.mark.usefixtures("mock_graphviz")
+def test_graphviz_supported_image_format(mock_writer, capsys):
+    """Test that Graphviz is used if the image format is supported."""
+    with pytest.raises(SystemExit) as wrapped_sysexit:
+        # we have to catch the SystemExit so the test execution does not stop
+        main.Run(["-o", "png", TEST_DATA_DIR])
+    # Check that the right info message is shown to the user
+    assert (
+        "Format png is not supported natively. Pyreverse will try to generate it using Graphviz..."
+        in capsys.readouterr().out
+    )
+    # Check that pyreverse actually made the call to create the diagram and we exit cleanly
+    mock_writer.DiagramWriter().write.assert_called_once()
+    assert wrapped_sysexit.value.code == 0
+
+
+@mock.patch("pylint.pyreverse.main.Linker", new=mock.MagicMock())
+@mock.patch("pylint.pyreverse.main.DiadefsHandler", new=mock.MagicMock())
+@mock.patch("pylint.pyreverse.main.writer")
+@pytest.mark.usefixtures("mock_graphviz")
+def test_graphviz_cant_determine_supported_formats(
+    mock_writer, mock_subprocess, capsys
+):
+    """Test that Graphviz is used if the image format is supported."""
+    mock_subprocess.run.return_value.stderr = "..."
+    with pytest.raises(SystemExit) as wrapped_sysexit:
+        # we have to catch the SystemExit so the test execution does not stop
+        main.Run(["-o", "png", TEST_DATA_DIR])
+    # Check that the right info message is shown to the user
+    assert (
+        "Unable to determine Graphviz supported output formats."
+        in capsys.readouterr().out
+    )
+    # Check that pyreverse actually made the call to create the diagram and we exit cleanly
+    mock_writer.DiagramWriter().write.assert_called_once()
+    assert wrapped_sysexit.value.code == 0
+
+
+@mock.patch("pylint.pyreverse.main.Linker", new=mock.MagicMock())
+@mock.patch("pylint.pyreverse.main.DiadefsHandler", new=mock.MagicMock())
+@mock.patch("pylint.pyreverse.main.writer", new=mock.MagicMock())
+@pytest.mark.usefixtures("mock_graphviz")
+def test_graphviz_unsupported_image_format(capsys):
+    """Test that Graphviz is used if the image format is supported."""
+    with pytest.raises(SystemExit) as wrapped_sysexit:
+        # we have to catch the SystemExit so the test execution does not stop
+        main.Run(["-o", "somethingElse", TEST_DATA_DIR])
+    # Check that the right info messages are shown to the user
+    stdout = capsys.readouterr().out
+    assert (
+        "Format somethingElse is not supported natively. Pyreverse will try to generate it using Graphviz..."
+        in stdout
+    )
+    assert "Format somethingElse is not supported by Graphviz. It supports:" in stdout
+    # Check that we exited with the expected error code
+    assert wrapped_sysexit.value.code == 32