Merge pull request #10538 from pradyunsg/diagnostic-errors

Author: pradyunsg

src/pip/_internal/cli/base_command.py

@@ -22,6 +22,7 @@
 from pip._internal.exceptions import (
     BadCommand,
     CommandError,
+    DiagnosticPipError,
     InstallationError,
     NetworkConnectionError,
     PreviousBuildDirError,
@@ -169,6 +170,11 @@ def exc_logging_wrapper(*args: Any) -> int:
                     logger.debug("Exception information:", exc_info=True)
 
                     return PREVIOUS_BUILD_DIR_ERROR
+                except DiagnosticPipError as exc:
+                    logger.critical(str(exc))
+                    logger.debug("Exception information:", exc_info=True)
+
+                    return ERROR
                 except (
                     InstallationError,
                     UninstallationError,

src/pip/_internal/exceptions.py

@@ -1,23 +1,102 @@
 """Exceptions used throughout package"""
 
 import configparser
+import re
 from itertools import chain, groupby, repeat
-from typing import TYPE_CHECKING, Dict, List, Optional, Union
+from typing import TYPE_CHECKING, Dict, Iterator, List, Optional, Union
 
 from pip._vendor.pkg_resources import Distribution
 from pip._vendor.requests.models import Request, Response
 
 if TYPE_CHECKING:
     from hashlib import _Hash
+    from typing import Literal
 
     from pip._internal.metadata import BaseDistribution
     from pip._internal.req.req_install import InstallRequirement
 
 
+#
+# Scaffolding
+#
+def _is_kebab_case(s: str) -> bool:
+    return re.match(r"^[a-z]+(-[a-z]+)*$", s) is not None
+
+
+def _prefix_with_indent(prefix: str, s: str, indent: Optional[str] = None) -> str:
+    if indent is None:
+        indent = " " * len(prefix)
+    else:
+        assert len(indent) == len(prefix)
+    message = s.replace("\n", "\n" + indent)
+    return f"{prefix}{message}\n"
+
+
 class PipError(Exception):
-    """Base pip exception"""
+    """The base pip error."""
+
 
+class DiagnosticPipError(PipError):
+    """A pip error, that presents diagnostic information to the user.
 
+    This contains a bunch of logic, to enable pretty presentation of our error
+    messages. Each error gets a unique reference. Each error can also include
+    additional context, a hint and/or a note -- which are presented with the
+    main error message in a consistent style.
+    """
+
+    reference: str
+
+    def __init__(
+        self,
+        *,
+        message: str,
+        context: Optional[str],
+        hint_stmt: Optional[str],
+        attention_stmt: Optional[str] = None,
+        reference: Optional[str] = None,
+        kind: 'Literal["error", "warning"]' = "error",
+    ) -> None:
+
+        # Ensure a proper reference is provided.
+        if reference is None:
+            assert hasattr(self, "reference"), "error reference not provided!"
+            reference = self.reference
+        assert _is_kebab_case(reference), "error reference must be kebab-case!"
+
+        super().__init__(f"{reference}: {message}")
+
+        self.kind = kind
+        self.message = message
+        self.context = context
+
+        self.reference = reference
+        self.attention_stmt = attention_stmt
+        self.hint_stmt = hint_stmt
+
+    def __str__(self) -> str:
+        return "".join(self._string_parts())
+
+    def _string_parts(self) -> Iterator[str]:
+        # Present the main message, with relevant context indented.
+        yield f"{self.message}\n"
+        if self.context is not None:
+            yield f"\n{self.context}\n"
+
+        # Space out the note/hint messages.
+        if self.attention_stmt is not None or self.hint_stmt is not None:
+            yield "\n"
+
+        if self.attention_stmt is not None:
+            yield _prefix_with_indent("Note: ", self.attention_stmt)
+
+        if self.hint_stmt is not None:
+            yield _prefix_with_indent("Hint: ", self.hint_stmt)
+
+
+#
+# Actual Errors
+#
 class ConfigurationError(PipError):
     """General exception in configuration"""
 
@@ -30,6 +109,45 @@ class UninstallationError(PipError):
     """General exception during uninstallation"""
 
 
+class MissingPyProjectBuildRequires(DiagnosticPipError):
+    """Raised when pyproject.toml has `build-system`, but no `build-system.requires`."""
+
+    reference = "missing-pyproject-build-system-requires"
+
+    def __init__(self, *, package: str) -> None:
+        super().__init__(
+            message=f"Can not process {package}",
+            context=(
+                "This package has an invalid pyproject.toml file.\n"
+                "The [build-system] table is missing the mandatory `requires` key."
+            ),
+            attention_stmt=(
+                "This is an issue with the package mentioned above, not pip."
+            ),
+            hint_stmt="See PEP 518 for the detailed specification.",
+        )
+
+
+class InvalidPyProjectBuildRequires(DiagnosticPipError):
+    """Raised when pyproject.toml an invalid `build-system.requires`."""
+
+    reference = "invalid-pyproject-build-system-requires"
+
+    def __init__(self, *, package: str, reason: str) -> None:
+        super().__init__(
+            message=f"Can not process {package}",
+            context=(
+                "This package has an invalid `build-system.requires` key in "
+                "pyproject.toml.\n"
+                f"{reason}"
+            ),
+            hint_stmt="See PEP 518 for the detailed specification.",
+            attention_stmt=(
+                "This is an issue with the package mentioned above, not pip."
+            ),
+        )
+
+
 class NoneMetadataError(PipError):
     """
     Raised when accessing "METADATA" or "PKG-INFO" metadata for a

src/pip/_internal/pyproject.py

@@ -5,7 +5,11 @@
 from pip._vendor import tomli
 from pip._vendor.packaging.requirements import InvalidRequirement, Requirement
 
-from pip._internal.exceptions import InstallationError
+from pip._internal.exceptions import (
+    InstallationError,
+    InvalidPyProjectBuildRequires,
+    MissingPyProjectBuildRequires,
+)
 
 
 def _is_list_of_str(obj: Any) -> bool:
@@ -119,47 +123,28 @@ def load_pyproject_toml(
 
     # Ensure that the build-system section in pyproject.toml conforms
     # to PEP 518.
-    error_template = (
-        "{package} has a pyproject.toml file that does not comply "
-        "with PEP 518: {reason}"
-    )
 
     # Specifying the build-system table but not the requires key is invalid
     if "requires" not in build_system:
-        raise InstallationError(
-            error_template.format(
-                package=req_name,
-                reason=(
-                    "it has a 'build-system' table but not "
-                    "'build-system.requires' which is mandatory in the table"
-                ),
-            )
-        )
+        raise MissingPyProjectBuildRequires(package=req_name)
 
     # Error out if requires is not a list of strings
     requires = build_system["requires"]
     if not _is_list_of_str(requires):
-        raise InstallationError(
-            error_template.format(
-                package=req_name,
-                reason="'build-system.requires' is not a list of strings.",
-            )
+        raise InvalidPyProjectBuildRequires(
+            package=req_name,
+            reason="It is not a list of strings.",
         )
 
     # Each requirement must be valid as per PEP 508
     for requirement in requires:
         try:
             Requirement(requirement)
-        except InvalidRequirement:
-            raise InstallationError(
-                error_template.format(
-                    package=req_name,
-                    reason=(
-                        "'build-system.requires' contains an invalid "
-                        "requirement: {!r}".format(requirement)
-                    ),
-                )
-            )
+        except InvalidRequirement as error:
+            raise InvalidPyProjectBuildRequires(
+                package=req_name,
+                reason=f"It contains an invalid requirement: {requirement!r}",
+            ) from error
 
     backend = build_system.get("build-backend")
     backend_path = build_system.get("backend-path", [])

tests/functional/test_install.py

@@ -108,7 +108,12 @@ def test_pep518_refuses_invalid_requires(script, data, common_wheels):
         expect_error=True,
     )
     assert result.returncode == 1
-    assert "does not comply with PEP 518" in result.stderr
+
+    # Ensure the relevant things are mentioned.
+    assert "PEP 518" in result.stderr
+    assert "not a list of strings" in result.stderr
+    assert "build-system.requires" in result.stderr
+    assert "pyproject.toml" in result.stderr
 
 
 def test_pep518_refuses_invalid_build_system(script, data, common_wheels):
@@ -120,7 +125,12 @@ def test_pep518_refuses_invalid_build_system(script, data, common_wheels):
         expect_error=True,
     )
     assert result.returncode == 1
-    assert "does not comply with PEP 518" in result.stderr
+
+    # Ensure the relevant things are mentioned.
+    assert "PEP 518" in result.stderr
+    assert "mandatory `requires` key" in result.stderr
+    assert "[build-system] table" in result.stderr
+    assert "pyproject.toml" in result.stderr
 
 
 def test_pep518_allows_missing_requires(script, data, common_wheels):
@@ -132,7 +142,7 @@ def test_pep518_allows_missing_requires(script, data, common_wheels):
         expect_stderr=True,
     )
     # Make sure we don't warn when this occurs.
-    assert "does not comply with PEP 518" not in result.stderr
+    assert "PEP 518" not in result.stderr
 
     # We want it to go through isolation for now.
     assert "Installing build dependencies" in result.stdout, result.stdout