From 6b3342a93182ffc624a8d2e1ba772cd2a857d816 Mon Sep 17 00:00:00 2001 From: Ralf Grubenmann Date: Thu, 22 Dec 2022 10:15:25 +0100 Subject: [PATCH 1/4] validate extras against dependencies and in schema --- src/poetry/core/factory.py | 14 ++++++++++++++ src/poetry/core/json/schemas/poetry-schema.json | 3 ++- .../pyproject.toml | 3 +++ tests/json/test_poetry_schema.py | 10 ++++++++++ tests/test_factory.py | 8 ++++++++ 5 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index 1d56d78c7..740c4d74e 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -409,6 +409,20 @@ def validate( 'Use "allow-prereleases" instead.' ) + if "extras" in config: + for extra_name, requirements in config["extras"].items(): + extra_name = canonicalize_name(extra_name) + + for req in requirements: + for dependency in config["dependencies"].keys(): + if req == dependency: + break + else: + result["warnings"].append( + f'Cannot find dependency "{req}" for extra ' + f'"{extra_name}" in main dependencies.' + ) + # Checking for scripts with extras if "scripts" in config: scripts = config["scripts"] diff --git a/src/poetry/core/json/schemas/poetry-schema.json b/src/poetry/core/json/schemas/poetry-schema.json index 8ff976f5b..195f466ee 100644 --- a/src/poetry/core/json/schemas/poetry-schema.json +++ b/src/poetry/core/json/schemas/poetry-schema.json @@ -155,7 +155,8 @@ "^[a-zA-Z-_.0-9]+$": { "type": "array", "items": { - "type": "string" + "type": "string", + "pattern": "^[a-zA-Z-_.0-9]+$" } } } diff --git a/tests/fixtures/project_failing_strict_validation/pyproject.toml b/tests/fixtures/project_failing_strict_validation/pyproject.toml index 6d282ba97..3fc8d4fbb 100644 --- a/tests/fixtures/project_failing_strict_validation/pyproject.toml +++ b/tests/fixtures/project_failing_strict_validation/pyproject.toml @@ -5,6 +5,9 @@ readme = ["README.rst", "README_WITH_ANOTHER_EXTENSION.md"] python = "*" pathlib2 = { version = "^2.2", python = "3.7", allows-prereleases = true } +[tool.poetry.extras] +some_extras = ["missing_extra", "another_missing_extra"] + [tool.poetry.scripts] a_script_with_unknown_extra = { reference = "a_script_with_unknown_extra.py", type = "file", extras = ["foo"] } a_script_without_extras = { reference = "a_script_without_extras.py", type = "file" } diff --git a/tests/json/test_poetry_schema.py b/tests/json/test_poetry_schema.py index 06f5e343e..de3900921 100644 --- a/tests/json/test_poetry_schema.py +++ b/tests/json/test_poetry_schema.py @@ -57,3 +57,13 @@ def test_multiline_description(base_object: dict[str, Any]) -> None: errors = validate_object(base_object, "poetry-schema") assert len(errors) == 1 assert errors[0] == f"[description] {bad_description!r} does not match '^[^\\n]*$'" + + +def test_bad_extra(base_object: dict[str, Any]) -> None: + bad_extra = "a{[*+" + base_object["extras"] = {} + base_object["extras"]["test"] = [bad_extra] + + errors = validate_object(base_object, "poetry-schema") + assert len(errors) == 1 + assert errors[0] == f"[extras.test.0] {bad_extra!r} does not match '^[a-zA-Z-_.0-9]+$'" diff --git a/tests/test_factory.py b/tests/test_factory.py index af4101dd0..e1280c54d 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -250,6 +250,14 @@ def test_validate_strict_fails_strict_and_non_strict() -> None: 'The "pathlib2" dependency specifies the "allows-prereleases" property,' ' which is deprecated. Use "allow-prereleases" instead.' ), + ( + 'Cannot find dependency "missing_extra" for extra "some-extras" in ' + 'main dependencies.' + ), + ( + 'Cannot find dependency "another_missing_extra" for extra ' + '"some-extras" in main dependencies.' + ) ], } From 98c4eb2642691e2799dda8e9e05fd6ae836c0688 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 22 Dec 2022 09:27:13 +0000 Subject: [PATCH 2/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/json/test_poetry_schema.py | 4 +++- tests/test_factory.py | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/json/test_poetry_schema.py b/tests/json/test_poetry_schema.py index de3900921..f6ba78f3a 100644 --- a/tests/json/test_poetry_schema.py +++ b/tests/json/test_poetry_schema.py @@ -66,4 +66,6 @@ def test_bad_extra(base_object: dict[str, Any]) -> None: errors = validate_object(base_object, "poetry-schema") assert len(errors) == 1 - assert errors[0] == f"[extras.test.0] {bad_extra!r} does not match '^[a-zA-Z-_.0-9]+$'" + assert ( + errors[0] == f"[extras.test.0] {bad_extra!r} does not match '^[a-zA-Z-_.0-9]+$'" + ) diff --git a/tests/test_factory.py b/tests/test_factory.py index e1280c54d..3b2d0c939 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -252,12 +252,12 @@ def test_validate_strict_fails_strict_and_non_strict() -> None: ), ( 'Cannot find dependency "missing_extra" for extra "some-extras" in ' - 'main dependencies.' + "main dependencies." ), ( 'Cannot find dependency "another_missing_extra" for extra ' '"some-extras" in main dependencies.' - ) + ), ], } From b583f255cf5bd1e9a29b697f3f0edd8fc63d4560 Mon Sep 17 00:00:00 2001 From: Ralf Grubenmann Date: Tue, 3 Jan 2023 09:15:21 +0100 Subject: [PATCH 3/4] address PR comments --- src/poetry/core/factory.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index 740c4d74e..e6bf4fa56 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -414,8 +414,10 @@ def validate( extra_name = canonicalize_name(extra_name) for req in requirements: + req_name = canonicalize_name(req) for dependency in config["dependencies"].keys(): - if req == dependency: + dep_name = canonicalize_name(dependency) + if req_name == dep_name: break else: result["warnings"].append( From 017df47dcdd76083db84ef2242355c7b724b19cf Mon Sep 17 00:00:00 2001 From: Ralf Grubenmann Date: Sun, 8 Jan 2023 11:50:16 +0100 Subject: [PATCH 4/4] always check extras and make it an error --- src/poetry/core/factory.py | 30 +++++++++++++++--------------- tests/test_factory.py | 16 ++++++++-------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index e6bf4fa56..0a776659c 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -409,21 +409,21 @@ def validate( 'Use "allow-prereleases" instead.' ) - if "extras" in config: - for extra_name, requirements in config["extras"].items(): - extra_name = canonicalize_name(extra_name) - - for req in requirements: - req_name = canonicalize_name(req) - for dependency in config["dependencies"].keys(): - dep_name = canonicalize_name(dependency) - if req_name == dep_name: - break - else: - result["warnings"].append( - f'Cannot find dependency "{req}" for extra ' - f'"{extra_name}" in main dependencies.' - ) + if "extras" in config: + for extra_name, requirements in config["extras"].items(): + extra_name = canonicalize_name(extra_name) + + for req in requirements: + req_name = canonicalize_name(req) + for dependency in config.get("dependencies", {}).keys(): + dep_name = canonicalize_name(dependency) + if req_name == dep_name: + break + else: + result["errors"].append( + f'Cannot find dependency "{req}" for extra ' + f'"{extra_name}" in main dependencies.' + ) # Checking for scripts with extras if "scripts" in config: diff --git a/tests/test_factory.py b/tests/test_factory.py index 3b2d0c939..1a5f23091 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -232,6 +232,14 @@ def test_validate_strict_fails_strict_and_non_strict() -> None: "'version' is a required property", "'description' is a required property", "'authors' is a required property", + ( + 'Cannot find dependency "missing_extra" for extra "some-extras" in ' + "main dependencies." + ), + ( + 'Cannot find dependency "another_missing_extra" for extra ' + '"some-extras" in main dependencies.' + ), ( 'Script "a_script_with_unknown_extra" requires extra "foo" which is not' " defined." @@ -250,14 +258,6 @@ def test_validate_strict_fails_strict_and_non_strict() -> None: 'The "pathlib2" dependency specifies the "allows-prereleases" property,' ' which is deprecated. Use "allow-prereleases" instead.' ), - ( - 'Cannot find dependency "missing_extra" for extra "some-extras" in ' - "main dependencies." - ), - ( - 'Cannot find dependency "another_missing_extra" for extra ' - '"some-extras" in main dependencies.' - ), ], }