From dd1245859e8a2349d26d66a0ebdcb82ff3fc09db Mon Sep 17 00:00:00 2001 From: David Hotham Date: Mon, 31 Oct 2022 13:40:08 +0000 Subject: [PATCH] clearer error handling (#514) - raise and catch more precise errors - version-parsing errors on a package are caught and re-raised with text saying what the package is --- src/poetry/core/constraints/generic/parser.py | 3 +- src/poetry/core/constraints/version/parser.py | 34 +++++++++++++++---- src/poetry/core/packages/package.py | 23 +++++++++---- src/poetry/core/version/markers.py | 2 +- tests/packages/test_package.py | 19 +++++++++++ 5 files changed, 66 insertions(+), 15 deletions(-) diff --git a/src/poetry/core/constraints/generic/parser.py b/src/poetry/core/constraints/generic/parser.py index 7ef2b1a29..abfba8eb5 100644 --- a/src/poetry/core/constraints/generic/parser.py +++ b/src/poetry/core/constraints/generic/parser.py @@ -7,6 +7,7 @@ from poetry.core.constraints.generic.any_constraint import AnyConstraint from poetry.core.constraints.generic.constraint import Constraint from poetry.core.constraints.generic.union_constraint import UnionConstraint +from poetry.core.constraints.version.exceptions import ParseConstraintError if TYPE_CHECKING: @@ -61,4 +62,4 @@ def parse_single_constraint(constraint: str) -> Constraint: return Constraint(version, op) - raise ValueError(f"Could not parse version constraint: {constraint}") + raise ParseConstraintError(f"Could not parse version constraint: {constraint}") diff --git a/src/poetry/core/constraints/version/parser.py b/src/poetry/core/constraints/version/parser.py index 6e55b984d..118a78cc3 100644 --- a/src/poetry/core/constraints/version/parser.py +++ b/src/poetry/core/constraints/version/parser.py @@ -4,6 +4,9 @@ from typing import TYPE_CHECKING +from poetry.core.constraints.version.exceptions import ParseConstraintError +from poetry.core.version.exceptions import InvalidVersion + if TYPE_CHECKING: from poetry.core.constraints.version.version_constraint import VersionConstraint @@ -66,7 +69,13 @@ def parse_single_constraint(constraint: str) -> VersionConstraint: # Tilde range m = TILDE_CONSTRAINT.match(constraint) if m: - version = Version.parse(m.group("version")) + try: + version = Version.parse(m.group("version")) + except InvalidVersion as e: + raise ParseConstraintError( + f"Could not parse version constraint: {constraint}" + ) from e + high = version.stable.next_minor() if version.release.precision == 1: high = version.stable.next_major() @@ -76,7 +85,13 @@ def parse_single_constraint(constraint: str) -> VersionConstraint: # PEP 440 Tilde range (~=) m = TILDE_PEP440_CONSTRAINT.match(constraint) if m: - version = Version.parse(m.group("version")) + try: + version = Version.parse(m.group("version")) + except InvalidVersion as e: + raise ParseConstraintError( + f"Could not parse version constraint: {constraint}" + ) from e + if version.release.precision == 2: high = version.stable.next_major() else: @@ -87,7 +102,12 @@ def parse_single_constraint(constraint: str) -> VersionConstraint: # Caret range m = CARET_CONSTRAINT.match(constraint) if m: - version = Version.parse(m.group("version")) + try: + version = Version.parse(m.group("version")) + except InvalidVersion as e: + raise ParseConstraintError( + f"Could not parse version constraint: {constraint}" + ) from e return VersionRange(version, version.next_breaking(), include_min=True) @@ -127,8 +147,10 @@ def parse_single_constraint(constraint: str) -> VersionConstraint: try: version = Version.parse(version_string) - except ValueError: - raise ValueError(f"Could not parse version constraint: {constraint}") + except InvalidVersion as e: + raise ParseConstraintError( + f"Could not parse version constraint: {constraint}" + ) from e if op == "<": return VersionRange(max=version) @@ -142,6 +164,4 @@ def parse_single_constraint(constraint: str) -> VersionConstraint: return VersionUnion(VersionRange(max=version), VersionRange(min=version)) return version - from poetry.core.constraints.version.exceptions import ParseConstraintError - raise ParseConstraintError(f"Could not parse version constraint: {constraint}") diff --git a/src/poetry/core/packages/package.py b/src/poetry/core/packages/package.py index 1d2efa87c..1651ca6b6 100644 --- a/src/poetry/core/packages/package.py +++ b/src/poetry/core/packages/package.py @@ -12,9 +12,11 @@ from typing import TypeVar from poetry.core.constraints.version import parse_constraint +from poetry.core.constraints.version.exceptions import ParseConstraintError from poetry.core.packages.dependency_group import MAIN_GROUP from poetry.core.packages.specification import PackageSpecification from poetry.core.packages.utils.utils import create_nested_marker +from poetry.core.version.exceptions import InvalidVersion from poetry.core.version.markers import parse_marker @@ -215,11 +217,15 @@ def _set_version( from poetry.core.constraints.version import Version if not isinstance(version, Version): - self._version = Version.parse(version) - self._pretty_version = pretty_version or version - else: - self._version = version - self._pretty_version = pretty_version or self._version.text + try: + version = Version.parse(version) + except InvalidVersion: + raise InvalidVersion( + f"Invalid version '{version}' on package {self.name}" + ) + + self._version = version + self._pretty_version = pretty_version or version.text def _get_author(self) -> dict[str, str | None]: if not self._authors: @@ -261,8 +267,13 @@ def python_versions(self) -> str: @python_versions.setter def python_versions(self, value: str) -> None: + try: + constraint = parse_constraint(value) + except ParseConstraintError: + raise ParseConstraintError(f"Invalid python versions '{value}' on {self}") + self._python_versions = value - self._python_constraint = parse_constraint(value) + self._python_constraint = constraint self._python_marker = parse_marker( create_nested_marker("python_version", self._python_constraint) ) diff --git a/src/poetry/core/version/markers.py b/src/poetry/core/version/markers.py index cb2d4fcb5..7f1b642ab 100644 --- a/src/poetry/core/version/markers.py +++ b/src/poetry/core/version/markers.py @@ -199,7 +199,7 @@ def __init__( # Extract operator and value m = self._CONSTRAINT_RE.match(constraint_string) if m is None: - raise ValueError(f"Invalid marker '{constraint_string}'") + raise InvalidMarker(f"Invalid marker '{constraint_string}'") self._operator = m.group(1) if self._operator is None: diff --git a/tests/packages/test_package.py b/tests/packages/test_package.py index 1f08bb627..998b25e22 100644 --- a/tests/packages/test_package.py +++ b/tests/packages/test_package.py @@ -9,11 +9,13 @@ import pytest from poetry.core.constraints.version import Version +from poetry.core.constraints.version.exceptions import ParseConstraintError from poetry.core.factory import Factory from poetry.core.packages.dependency import Dependency from poetry.core.packages.dependency_group import DependencyGroup from poetry.core.packages.package import Package from poetry.core.packages.project_package import ProjectPackage +from poetry.core.version.exceptions import InvalidVersion if TYPE_CHECKING: @@ -660,3 +662,20 @@ def test_project_package_hash_not_changed_when_version_is_changed() -> None: assert hash(package) == package_hash, "Hash must not change!" assert hash(package_clone) == package_hash assert package != package_clone + + +def test_package_invalid_version() -> None: + with pytest.raises(InvalidVersion) as exc_info: + Package("foo", "1.2.3.bogus") + + expected = "Invalid version '1.2.3.bogus' on package foo" + assert str(exc_info.value) == expected + + +def test_package_invalid_python_versions() -> None: + package = Package("foo", "1.2.3") + with pytest.raises(ParseConstraintError) as exc_info: + package.python_versions = ">=3.6.y" + + expected = "Invalid python versions '>=3.6.y' on foo (1.2.3)" + assert str(exc_info.value) == expected