From d5e0f2a3c286419d871c4a756ff56658d741d9cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Mond=C3=A9jar=20Rubio?= Date: Tue, 22 Jun 2021 13:05:40 +0200 Subject: [PATCH] Fix error adding hooks with 'add-pre-commit-hook' when repository is already defined in configuration file. --- .bumpversion.cfg | 2 +- .pre-commit-config.yaml | 6 +- README.md | 2 +- hooks/add_pre_commit_hook.py | 84 ++++++++++++++++++++++++---- setup.cfg | 2 +- tests/test_add_pre_commit_hook.py | 92 +++++++++++++++++++++++-------- 6 files changed, 145 insertions(+), 43 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index 811c332..010f0a5 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 1.1.2 +current_version = 1.1.3 [bumpversion:file:setup.cfg] diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d80c49e..c9fd093 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -4,7 +4,7 @@ repos: hooks: - id: setup-cfg-fmt - repo: https://github.com/mondeja/pre-commit-hooks - rev: v1.0.0 + rev: v1.1.2 hooks: - id: dev-extras-required - repo: https://github.com/asottile/pyupgrade @@ -56,7 +56,3 @@ repos: args: - -config=https://github.com/mondeja/repo-stream-config - -updater=upstream - - repo: https://github.com/mondeja/pre-commit-hooks - rev: v1.1.2 - hooks: - - id: dev-extras-required diff --git a/README.md b/README.md index dd4b5f9..d4087fb 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ ```yaml - repo: https://github.com/mondeja/pre-commit-hooks - rev: v1.1.2 + rev: v1.1.3 hooks: - id: dev-extras-required ``` diff --git a/hooks/add_pre_commit_hook.py b/hooks/add_pre_commit_hook.py index d9e8fef..c33cdd2 100644 --- a/hooks/add_pre_commit_hook.py +++ b/hooks/add_pre_commit_hook.py @@ -7,6 +7,60 @@ import yaml +def repeated_definitions_of_repo_in_config(config, repo): + """Check if there are multiple definitions of the same repository in a + pre-commit configuration object. + + Parameters + ---------- + + config : dict + Pre-commit configuration dictionary. + + repo : str + Repository to check for multiple definitions. + + Returns + ------- + + bool : ``True`` if there are more than one definition of the passed + repository in the configuration dictionary, ``False`` otherwise. + """ + return len([_repo for _repo in config["repos"] if _repo["repo"] == repo]) > 1 + + +def is_repo_in_config(config, repo, rev, hook_id): + """Get if a repository is defined in a pre-commit configuration. + + Parameters + ---------- + + config : dict + Pre-commit configuration dictionary. + + repo : str + Repository to search. + + rev : str + Repository tag revision. + + hook_id : Hook identifier. + + Returns + ------- + + dict : Information about if the repository and the hook have been found. + """ + response = {"repo_found": False, "hook_found": False, "same_rev": False} + for repo_ in config["repos"]: + if repo_["repo"] == repo: + response["repo_found"] = True + response["hook_found"] = hook_id in [hook["id"] for hook in repo_["hooks"]] + response["same_rev"] = repo_["rev"] == rev + break + return response + + def add_pre_commit_hook(repo, rev, hook_id, quiet=False, dry_run=False): """Add a pre-commit hook configuration to a pre-commit configuration file. @@ -33,7 +87,6 @@ def add_pre_commit_hook(repo, rev, hook_id, quiet=False, dry_run=False): ------- int: 1 if the pre-commit configuration file has been changed, 0 otherwise. - """ pre_commit_config_path = ".pre-commit-config.yaml" if not os.path.isfile(pre_commit_config_path): @@ -46,15 +99,9 @@ def add_pre_commit_hook(repo, rev, hook_id, quiet=False, dry_run=False): if "repos" not in config or not config["repos"]: return 0 - _repo_found, _hook_found, _same_rev = (False, False, False) - for repo_ in config["repos"]: - if repo_["repo"] == repo: - _repo_found = True - _hook_found = hook_id in [hook["id"] for hook in repo_["hooks"]] - _same_rev = repo_["rev"] == rev - break + repo_in_config = is_repo_in_config(config, repo, rev, hook_id) - if not _repo_found or not _same_rev: + if not repo_in_config["repo_found"]: _repo_indentation = 2 config_lines = config_content.splitlines(keepends=True) for line in config_lines: @@ -87,10 +134,20 @@ def add_pre_commit_hook(repo, rev, hook_id, quiet=False, dry_run=False): return 1 - if not _hook_found and _same_rev: + # repo in configuration multiple times + if repeated_definitions_of_repo_in_config(config, repo): + sys.stderr.write( + f"Multiple definitions of repository '{repo}' in configuration" + " file '.pre-commit-config.yaml'. You must determine manually one" + " of them.\n" + ) + return 1 + + if not repo_in_config["hook_found"]: config_lines = config_content.splitlines(keepends=True) _inside_repo, _inside_hooks, _hooks_indent = (False, False, None) + _rev_line = None for i, line in enumerate(config_lines): if not _inside_repo: if line.lstrip().startswith("- repo:") and repo in line.replace( @@ -107,10 +164,15 @@ def add_pre_commit_hook(repo, rev, hook_id, quiet=False, dry_run=False): else: if line.lstrip().startswith("hooks:"): _inside_hooks = True + elif line.lstrip().startswith("rev:"): + _rev_line = i new_lines = [] for n, line in enumerate(config_lines): - new_lines.append(line) + if _rev_line is not None and not repo_in_config["same_rev"]: + new_lines.append(line.split(":")[0] + f": {rev}\n") + else: + new_lines.append(line) if n == i: if not new_lines[-1].strip(): new_lines = new_lines[:-1] diff --git a/setup.cfg b/setup.cfg index 597520d..d877de1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = mondeja_pre_commit_hooks -version = 1.1.2 +version = 1.1.3 description = My own useful pre-commit hooks long_description = file: README.md long_description_content_type = text/markdown diff --git a/tests/test_add_pre_commit_hook.py b/tests/test_add_pre_commit_hook.py index ed13489..6af7f75 100644 --- a/tests/test_add_pre_commit_hook.py +++ b/tests/test_add_pre_commit_hook.py @@ -15,7 +15,15 @@ "dry_run", (False, True), ids=("dry_run=False", "dry_run=True") ) @pytest.mark.parametrize( - ("repo", "rev", "hook_id", "input_content", "expected_result", "expected_exitcode"), + ( + "repo", + "rev", + "hook_id", + "input_content", + "expected_result", + "expected_exitcode", + "expected_stderr", + ), ( pytest.param( "https://github.com/mondeja/pre-commit-hooks", @@ -39,6 +47,7 @@ - id: dev-extras-required """, 1, + None, id="add-repo", ), pytest.param( @@ -60,6 +69,7 @@ - id: dev-extras-required """, 1, + None, id="add-hook", ), pytest.param( @@ -79,7 +89,32 @@ - id: dev-extras-required """, 0, - id="add-hook", + None, + id="dont-add-hook", + ), + pytest.param( + "https://github.com/mondeja/pre-commit-hooks", + "v1.1.2", + "foo-hook-id", + r"""repos: + - repo: https://github.com/mondeja/pre-commit-hooks + rev: v1.0.0 + hooks: + - id: dev-extras-required + - repo: https://github.com/mondeja/pre-commit-hooks + rev: v1.1.2 + hooks: + - id: dev-extras-required +""", + None, + 1, + ( + "Multiple definitions of repository" + " 'https://github.com/mondeja/pre-commit-hooks' in" + " configuration file '.pre-commit-config.yaml'." + " You must determine manually one of them.\n" + ), + id="multiple-definitions", ), ), ) @@ -90,36 +125,45 @@ def test_add_pre_commit_hook( input_content, expected_result, expected_exitcode, + expected_stderr, quiet, dry_run, ): - previous_cwd = os.getcwd() + try: + previous_cwd = os.getcwd() + + with tempfile.TemporaryDirectory() as dirname: + filepath = os.path.join(dirname, ".pre-commit-config.yaml") + if os.path.isfile(filepath): + os.remove(filepath) - with tempfile.TemporaryDirectory() as dirname: - filepath = os.path.join(dirname, ".pre-commit-config.yaml") - if os.path.isfile(filepath): - os.remove(filepath) + with open(filepath, "w") as f: + f.write(input_content) - with open(filepath, "w") as f: - f.write(input_content) + os.chdir(dirname) - os.chdir(dirname) + stderr = io.StringIO() - stderr = io.StringIO() + with contextlib.redirect_stderr(stderr): + exitcode = add_pre_commit_hook( + repo, + rev, + hook_id, + quiet=quiet, + dry_run=dry_run, + ) - with contextlib.redirect_stderr(stderr): - exitcode = add_pre_commit_hook( - repo, - rev, - hook_id, - quiet=quiet, - dry_run=dry_run, - ) + assert exitcode == expected_exitcode - assert exitcode == expected_exitcode + if expected_stderr is not None: + assert stderr.getvalue() == expected_stderr + else: - if not dry_run: - with open(filepath) as f: - assert f.read() == expected_result + if not dry_run: + with open(filepath) as f: + assert f.read() == expected_result - os.chdir(previous_cwd) + except Exception: + raise + finally: + os.chdir(previous_cwd)