From c1d176129434bb5889cd6aabbc5d38148e818e08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Mond=C3=A9jar?= Date: Thu, 10 Mar 2022 22:51:09 +0100 Subject: [PATCH] Add development extras check for `setup.py` files (#16) * Checkpoint * Complete implementation * Admit constants * Add test * Bump version * Remove file * Compatibility with Python < 3.8 * Try with other * Fix error * Fix compat * Debug * Skip test in Python < 3.8 * Dont support Python < 3.8 * Fix error in test --- .bumpversion.cfg | 2 +- .pre-commit-hooks.yaml | 2 +- README.md | 9 +- hooks/dev_extras_required.py | 142 +++++++++++- setup.cfg | 2 +- tests/test_dev_extras_required.py | 345 +++++++++++++++++++++++------- 6 files changed, 416 insertions(+), 86 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index 23159e1..46d8bd0 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 1.6.0 +current_version = 1.7.0 [bumpversion:file:setup.cfg] diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index 446f900..c9afd94 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -29,7 +29,7 @@ name: dev-extras-required entry: dev-extras-required-hook description: Check if your development dependencies contains all other extras requirements - files: '(setup\.cfg)|(pyproject\.toml)' + files: '(setup\.cfg)|(pyproject\.toml)|(setup\.py)' language: python - id: nameservers-endswith name: nameservers-endswith diff --git a/README.md b/README.md index fd03730..c67ef51 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ ```yaml - repo: https://github.com/mondeja/pre-commit-hooks - rev: v1.5.3 + rev: v1.7.0 hooks: - id: dev-extras-required - id: root-editorconfig-required @@ -41,10 +41,8 @@ Add a pre-commit hook to your configuration file if is not already defined. ### **`dev-extras-required`** -> - Doesn't support `setup.py` files. Please, [migrate your setup configuration - to `setup.cfg` format][setup-py-upgrade-link]. -> - Support for `pyproject.toml` files is limited to printing errors, automatic - file rewriting is not performed. +> - Support for `pyproject.toml` and `setup.py` files is limited to + printing errors, automatic file rewriting is not performed. Check if your development dependencies contains all other extras requirements. If an extra requirement is defined in other extra group than your development @@ -157,6 +155,5 @@ You need to install [cloudflare-apikey-link]: https://support.cloudflare.com/hc/en-us/articles/200167836-Managing-API-Tokens-and-Keys [gh-pages-link]: https://pages.github.com [pre-commit-po-hooks-link]: https://github.com/mondeja/pre-commit-po-hooks -[setup-py-upgrade-link]: https://github.com/asottile/setup-py-upgrade [repo-stream-link]: https://github.com/mondeja/repo-stream diff --git a/hooks/dev_extras_required.py b/hooks/dev_extras_required.py index 6d093bc..7202c52 100644 --- a/hooks/dev_extras_required.py +++ b/hooks/dev_extras_required.py @@ -61,7 +61,7 @@ def check_setup_cfg( if dry_run: if not quiet: sys.stderr.write( - f"Requirement '{requirement}' would be added to" + f"Requirement '{requirement}' must be added to" f" '{dev_extra_name}' extra group at '{filename}'\n" ) else: @@ -148,7 +148,7 @@ def update_pyproject_toml_if_needed(build_system): # if dry_run: if not quiet: sys.stderr.write( - f"Requirement '{requirement}' would be added to" + f"Requirement '{requirement}' must be added to" f" '{dev_extra_name}' extra group at '{filename}'\n" ) """ @@ -183,6 +183,122 @@ def update_pyproject_toml_if_needed(build_system): return exitcode +def check_setup_py( + filename="pyproject.toml", + dev_extra_name="dev", + quiet=False, +): + """Check that all other extra requirements than development ones are + included in development extra group in a ``setup.py`` configuration + file. Only works if the groups are explicitly defined constants. + + Parameters + ---------- + + filename : str, optional + Path to the file to check. + + dev_extra_name : str, optional + Development extra requirements group name. + + quiet : bool, optional + Enabled, don't print output to stderr when a requirement is not + defined in development extra requirements group. + """ + import ast + + # Not compatible with Python < 3.8 + if sys.version_info < (3, 8): # pragma: no cover + sys.stderr.write( + "'dev-extras-required' hook for 'setup.py' files only supports" + " Python >= 3.8\n" + ) + return 1 + + with open(filename) as f: + content = f.read() + + class ExtraRequirementsExtractor(ast.NodeVisitor): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + self.extras = None + self.exitcode = 0 + + def generic_visit(self, node): + if isinstance(node, ast.keyword) and node.arg == "extras_require": + if node.value: + self.extras = self._check_extras(self._extract_extras(node)) + ast.NodeVisitor.generic_visit(self, node) + + def _extract_extras(self, node): + extras = {} + + keys = [const.value for const in node.value.keys] + for i in range(len(keys)): + values = [] + for elt in node.value.values[i].elts: + if isinstance(elt, ast.Constant): + values.append(elt.value) + else: + req = elt.id if isinstance(elt, ast.Name) else str(elt) + values.append(req) + extras[keys[i]] = values + return extras + + def _check_extras(self, extras): + if self.extras is not None: + if not quiet: + sys.stderr.write( + "Multiple 'extras_require' keywords found" + f" in '{filename}'. Impossible to resolve" + " extras groups.\n" + ) + self.exitcode = 1 + elif not extras: + sys.stderr.write( + f"Empty extra requirements group found in file '{filename}'\n" + ) + self.exitcode = 1 + + return extras + + setup_py_tree = ast.parse(ast.parse(content)) + visitor = ExtraRequirementsExtractor() + + # Useful for debugging (Python >= 3.9 needed) + # print(ast.dump(setup_py_tree, indent=4)) + + visitor.visit(setup_py_tree) + + if visitor.exitcode == 1: + return visitor.exitcode + elif visitor.extras is None: + sys.stderr.write(f"Extra requirements not found in file '{filename}'\n") + return 1 + + dev_extra_requirements, other_extra_requirements = ([], []) + + for extra, requirements in visitor.extras.items(): + if extra == dev_extra_name: + dev_extra_requirements.extend(requirements) + else: + other_extra_requirements.extend(requirements) + + exitcode = 0 + + for requirement in other_extra_requirements: + if requirement not in dev_extra_requirements: + exitcode = 1 + + if not quiet: + sys.stderr.write( + f"Requirement '{requirement}' must be added to" + f" '{dev_extra_name}' extra group at '{filename}'\n" + ) + return exitcode + + def main(): parser = argparse.ArgumentParser() parser.add_argument("filenames", nargs="*") @@ -202,7 +318,7 @@ def main(): required=False, default="dev", dest="extra_name", - help="Path to the 'setup.cfg' file.", + help="Development extra group name.", ) parser.add_argument( "-setup-cfg", @@ -224,9 +340,19 @@ def main(): dest="pyproject_toml", help="Path to the 'pyproject.toml' file.", ) + parser.add_argument( + "-setup-py", + "--setup-py", + type=str, + metavar="FILEPATH", + required=False, + default="setup.py", + dest="setup_py", + help="Path to the 'setup.py' file.", + ) args = parser.parse_args() - filenames = (args.setup_cfg, args.pyproject_toml) + filenames = (args.setup_cfg, args.pyproject_toml, args.setup_py) if not any([os.path.isfile(filename) for filename in filenames]): for filename in filenames: sys.stderr.write(f"'{filename}' file not found\n") @@ -239,12 +365,18 @@ def main(): quiet=args.quiet, dry_run=args.dry_run, ) - else: + elif os.path.isfile(args.pyproject_toml): exitcode = check_pyproject_toml( filename=args.pyproject_toml, dev_extra_name=args.extra_name, quiet=args.quiet, ) + else: + exitcode = check_setup_py( + filename=args.setup_py, + dev_extra_name=args.extra_name, + quiet=args.quiet, + ) return exitcode diff --git a/setup.cfg b/setup.cfg index 1220440..811f6b4 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = mondeja_pre_commit_hooks -version = 1.6.0 +version = 1.7.0 description = My own useful pre-commit hooks long_description = file: README.md long_description_content_type = text/markdown diff --git a/tests/test_dev_extras_required.py b/tests/test_dev_extras_required.py index d37a973..ec4e8c9 100644 --- a/tests/test_dev_extras_required.py +++ b/tests/test_dev_extras_required.py @@ -3,11 +3,64 @@ import contextlib import io import os +import sys import tempfile import pytest -from hooks.dev_extras_required import check_pyproject_toml, check_setup_cfg +from hooks.dev_extras_required import ( + check_pyproject_toml, + check_setup_cfg, + check_setup_py, +) + + +def _assert_implementation( + input_content, + expected_exitcode, + expected_stderr_lines, + dev_extra_name, + filename, + quiet, + check_function, + dry_run=None, + expected_result=None, +): + with tempfile.TemporaryDirectory() as dirname: + filename = os.path.join(dirname, filename) + + with open(filename, "w") as f: + f.write(input_content.replace("{dev_extra_name}", dev_extra_name)) + + stderr = io.StringIO() + with contextlib.redirect_stderr(stderr): + kwargs = {} if dry_run is None else {"dry_run": dry_run} + + exitcode = check_function( + filename=filename, + dev_extra_name=dev_extra_name, + quiet=quiet, + **kwargs, + ) + + assert exitcode == expected_exitcode + + if dry_run in {True, None}: + if not quiet: + expected_stderr_lines = [ + line.replace("{dev_extra_name}", dev_extra_name).replace( + "{filename}", filename + ) + for line in expected_stderr_lines + ] + + for line in stderr.getvalue().splitlines(): + assert line in expected_stderr_lines + else: + with open(filename) as f: + result = f.read() + + assert result == expected_result.replace("{dev_extra_name}", dev_extra_name) @pytest.mark.parametrize( @@ -116,15 +169,15 @@ 1, [ ( - "Requirement 'pytest==6.2.0' would be added to" + "Requirement 'pytest==6.2.0' must be added to" " '{dev_extra_name}' extra group at '{filename}'" ), ( - "Requirement 'pytest-cov==2.12.1' would be added to" + "Requirement 'pytest-cov==2.12.1' must be added to" " '{dev_extra_name}' extra group at '{filename}'" ), ( - "Requirement 'waves==0.2.4' would be added to" + "Requirement 'waves==0.2.4' must be added to" " '{dev_extra_name}' extra group at '{filename}'" ), ], @@ -212,7 +265,7 @@ 1, [ ( - "Requirement 'pytest==6.2.0' would be added to" + "Requirement 'pytest==6.2.0' must be added to" " '{dev_extra_name}' extra group at '{filename}'" ), ], @@ -309,44 +362,22 @@ def test_check_setup_cfg( expected_result, expected_exitcode, expected_stderr_lines, - filename, dev_extra_name, + filename, dry_run, quiet, ): - with tempfile.TemporaryDirectory() as dirname: - filename = os.path.join(dirname, filename) - - with open(filename, "w") as f: - f.write(input_content.replace("{dev_extra_name}", dev_extra_name)) - - stderr = io.StringIO() - with contextlib.redirect_stderr(stderr): - exitcode = check_setup_cfg( - filename=filename, - dev_extra_name=dev_extra_name, - quiet=quiet, - dry_run=dry_run, - ) - - assert exitcode == expected_exitcode - - if dry_run: - if not quiet: - expected_stderr_lines = [ - line.replace("{dev_extra_name}", dev_extra_name).replace( - "{filename}", filename - ) - for line in expected_stderr_lines - ] - - for line in stderr.getvalue().splitlines(): - assert line in expected_stderr_lines - else: - with open(filename) as f: - result = f.read() - - assert result == expected_result.replace("{dev_extra_name}", dev_extra_name) + _assert_implementation( + input_content, + expected_exitcode, + expected_stderr_lines, + dev_extra_name, + filename, + quiet, + check_function=check_setup_cfg, + dry_run=dry_run, + expected_result=expected_result, + ) @pytest.mark.parametrize( @@ -414,31 +445,31 @@ def test_check_setup_cfg( 1, [ ( - "Requirement 'pytest' would be added to" + "Requirement 'pytest' must be added to" " '{dev_extra_name}' extra group at '{filename}'" ), ( - "Requirement 'pytest-cov' would be added to" + "Requirement 'pytest-cov' must be added to" " '{dev_extra_name}' extra group at '{filename}'" ), ( - "Requirement 'pytest-xdist' would be added to" + "Requirement 'pytest-xdist' must be added to" " '{dev_extra_name}' extra group at '{filename}'" ), ( - "Requirement 'myst-parser' would be added to" + "Requirement 'myst-parser' must be added to" " '{dev_extra_name}' extra group at '{filename}'" ), ( - "Requirement 'sphinx-copybutton' would be added to" + "Requirement 'sphinx-copybutton' must be added to" " '{dev_extra_name}' extra group at '{filename}'" ), ( - "Requirement 'sphinx-inline-tabs' would be added to" + "Requirement 'sphinx-inline-tabs' must be added to" " '{dev_extra_name}' extra group at '{filename}'" ), ( - "Requirement 'docutils != 0.17' would be added to" + "Requirement 'docutils != 0.17' must be added to" " '{dev_extra_name}' extra group at '{filename}'" ), ], @@ -509,11 +540,11 @@ def test_check_setup_cfg( 1, [ ( - "Requirement 'mysqlclient' would be added to" + "Requirement 'mysqlclient' must be added to" " '{dev_extra_name}' extra group at '{filename}'" ), ( - "Requirement 'psycopg2' would be added to" + "Requirement 'psycopg2' must be added to" " '{dev_extra_name}' extra group at '{filename}'" ), ], @@ -551,29 +582,199 @@ def test_check_pyproject_toml( filename, quiet, ): - with tempfile.TemporaryDirectory() as dirname: - filename = os.path.join(dirname, filename) - - with open(filename, "w") as f: - f.write(input_content.replace("{dev_extra_name}", dev_extra_name)) - - stderr = io.StringIO() - with contextlib.redirect_stderr(stderr): - exitcode = check_pyproject_toml( - filename=filename, - dev_extra_name=dev_extra_name, - quiet=quiet, - ) - - assert exitcode == expected_exitcode + _assert_implementation( + input_content, + expected_exitcode, + expected_stderr_lines, + dev_extra_name, + filename, + quiet, + check_function=check_pyproject_toml, + ) + + +@pytest.mark.skipif( + sys.version_info < (3, 8), + reason="'dev-extras-required' hook with setup.py files only supports Python >= 3.8", +) +@pytest.mark.parametrize( + "filename", + ("setup.py", "_setup.py"), + ids=("filename=setup.py", "filename=_setup.py"), +) +@pytest.mark.parametrize("quiet", (False, True), ids=("quiet=False", "quiet=True")) +@pytest.mark.parametrize( + "dev_extra_name", + ("dev", "development"), + ids=("dev_extra_name=dev", "dev_extra_name=development"), +) +@pytest.mark.parametrize( + ("input_content", "expected_exitcode", "expected_stderr_lines"), + ( + pytest.param( + """from setuptools import setup + +setup( + version="0.0.1", + name="foo", + zip_safe=False, + extras_require={ + "{dev_extra_name}": ["foo==4.9.5a10", "bar", "baz==3.0.2"], + "pgsql": ["psycopg2"], + "test": ["pytest", "pytest-cov", "pytest-xdist"], + }, + setup_requires=["Cython"] +) +""", + 1, + [ + ( + "Requirement 'pytest' must be added to" + " '{dev_extra_name}' extra group at '{filename}'" + ), + ( + "Requirement 'pytest-cov' must be added to" + " '{dev_extra_name}' extra group at '{filename}'" + ), + ( + "Requirement 'pytest-xdist' must be added to" + " '{dev_extra_name}' extra group at '{filename}'" + ), + ( + "Requirement 'psycopg2' must be added to" + " '{dev_extra_name}' extra group at '{filename}'" + ), + ], + id="test + pgsql -> dev", + ), + pytest.param( + """from setuptools import setup + +setup( + version="0.0.1", + name="foo", + zip_safe=False, + extras_require={}, + setup_requires=["Cython"] +) +""", + 1, + [ + ("Empty extra requirements group found in file '{filename}'"), + ], + id="empty extras_require", + ), + pytest.param( + """from setuptools import setup - if not quiet: - expected_stderr_lines = [ - line.replace("{dev_extra_name}", dev_extra_name).replace( - "{filename}", filename +setup( + version="0.0.1", + name="foo", + zip_safe=False, + setup_requires=["Cython"] +) +""", + 1, + [ + "Extra requirements not found in file '{filename}'", + ], + id="not extras_require", + ), + pytest.param( + """from setuptools import setup + +setup( + version="0.0.1", + name="foo", + zip_safe=False, + setup_requires=["Cython"], + extras_require={ + "{dev_extra_name}": [ + "foo==4.9.5a10", + "bar", + "baz==3.0.2", + "psycopg2==6.7.8", + "pytest", + "pytest-cov", + "pytest-xdist", + ], + "pgsql": ["psycopg2==6.7.8"], + "test": ["pytest", "pytest-cov", "pytest-xdist"], + }, +) +""", + 0, + [], + id="correct", + ), + pytest.param( + """from setuptools import setup + +EXTRA = "pytest" + +setup( + version="0.0.1", + name="foo", + zip_safe=False, + setup_requires=["Cython"], + extras_require={ + "{dev_extra_name}": [ + EXTRA, + "pytest-cov", + "pytest-xdist", + ], + "test": [EXTRA, "pytest-cov", "pytest-xdist"], + }, +) +""", + 0, + [], + id="not constant (correct)", + ), + pytest.param( + """from setuptools import setup + +EXTRA = "pytest" + +setup( + version="0.0.1", + name="foo", + zip_safe=False, + setup_requires=["Cython"], + extras_require={ + "{dev_extra_name}": [ + "pytest-cov", + "pytest-xdist", + ], + "test": [EXTRA, "pytest-cov", "pytest-xdist"], + }, +) +""", + 1, + [ + ( + "Requirement 'EXTRA' must be added to '{dev_extra_name}'" + " extra group at '{filename}'" ) - for line in expected_stderr_lines - ] - - for line in stderr.getvalue().splitlines(): - assert line in expected_stderr_lines + ], + id="not constant (not found)", + ), + ), +) +def test_check_setup_py( + input_content, + expected_exitcode, + expected_stderr_lines, + dev_extra_name, + filename, + quiet, +): + _assert_implementation( + input_content, + expected_exitcode, + expected_stderr_lines, + dev_extra_name, + filename, + quiet, + check_function=check_setup_py, + )