From 5934335f15c6a6b293ca4223eae8566fcf2ff5a4 Mon Sep 17 00:00:00 2001 From: Remco de Boer <29308176+redeboer@users.noreply.github.com> Date: Mon, 11 Mar 2024 11:02:42 +0100 Subject: [PATCH] FIX: correctly dump pre-commit and update changelog (#321) * ENH: shorten pyproject and pre-commit changelog * FIX: assert if `ModifiablePrecommit` is modified in context * FIX: correctly merge changelog entries on `__exit__()` calls * FIX: correctly write to stream on `__exit__()` calls * MAINT: make `raise_exception` dunder --- src/compwa_policy/check_dev_files/black.py | 7 ++--- src/compwa_policy/check_dev_files/citation.py | 2 +- src/compwa_policy/check_dev_files/mypy.py | 12 ++++----- .../check_dev_files/precommit.py | 11 ++++---- src/compwa_policy/check_dev_files/pyright.py | 13 +++++----- src/compwa_policy/check_dev_files/pytest.py | 11 ++++---- src/compwa_policy/check_dev_files/ruff.py | 24 ++++++++--------- src/compwa_policy/utilities/executor.py | 4 +-- .../utilities/precommit/__init__.py | 21 ++++++++------- .../utilities/precommit/setters.py | 4 +-- .../utilities/pyproject/__init__.py | 10 +++---- tests/check_dev_files/test_cspell.py | 26 +++++++++++++++---- 12 files changed, 79 insertions(+), 66 deletions(-) diff --git a/src/compwa_policy/check_dev_files/black.py b/src/compwa_policy/check_dev_files/black.py index 75727e82..a9e601b2 100644 --- a/src/compwa_policy/check_dev_files/black.py +++ b/src/compwa_policy/check_dev_files/black.py @@ -53,10 +53,7 @@ def _remove_outdated_settings(pyproject: ModifiablePyproject) -> None: settings.pop(option) removed_options.add(option) if removed_options: - msg = ( - f"Removed {', '.join(sorted(removed_options))} option from black" - f" configuration in {CONFIG_PATH.pyproject}" - ) + msg = f"Removed {', '.join(sorted(removed_options))} option from black configuration" pyproject.append_to_changelog(msg) @@ -70,7 +67,7 @@ def _update_black_settings(pyproject: ModifiablePyproject) -> None: } if not complies_with_subset(settings, minimal_settings): settings.update(minimal_settings) - msg = f"Updated black configuration in {CONFIG_PATH.pyproject}" + msg = "Updated black configuration" pyproject.append_to_changelog(msg) diff --git a/src/compwa_policy/check_dev_files/citation.py b/src/compwa_policy/check_dev_files/citation.py index 396d724e..a4f01558 100644 --- a/src/compwa_policy/check_dev_files/citation.py +++ b/src/compwa_policy/check_dev_files/citation.py @@ -206,7 +206,7 @@ def add_json_schema_precommit(precommit: ModifiablePrecommit) -> None: existing_repos = precommit.document["repos"] repos_yaml = cast(CommentedSeq, existing_repos) repos_yaml.yaml_set_comment_before_after_key(repo_idx + 1, before="\n") - msg = f"Updated check-jsonschema hook in {CONFIG_PATH.precommit}" + msg = f"Updated pre-commit hook {repo_url}" precommit.append_to_changelog(msg) diff --git a/src/compwa_policy/check_dev_files/mypy.py b/src/compwa_policy/check_dev_files/mypy.py index f5d3f728..c2e1f769 100644 --- a/src/compwa_policy/check_dev_files/mypy.py +++ b/src/compwa_policy/check_dev_files/mypy.py @@ -38,15 +38,15 @@ def _update_vscode_settings(pyproject: Pyproject) -> None: def _merge_mypy_into_pyproject(pyproject: ModifiablePyproject) -> None: - config_path = ".mypy.ini" - if not os.path.exists(config_path): + old_config_path = ".mypy.ini" + if not os.path.exists(old_config_path): return - with open(config_path) as stream: + with open(old_config_path) as stream: original_contents = stream.read() - toml_str = Translator().translate(original_contents, profile_name=config_path) + toml_str = Translator().translate(original_contents, profile_name=old_config_path) mypy_config = rtoml.loads(toml_str) tool_table = pyproject.get_table("tool", create=True) tool_table.update(mypy_config) - os.remove(config_path) - msg = f"Moved mypy configuration to {CONFIG_PATH.pyproject}" + os.remove(old_config_path) + msg = f"Imported mypy configuration from {old_config_path}" pyproject.append_to_changelog(msg) diff --git a/src/compwa_policy/check_dev_files/precommit.py b/src/compwa_policy/check_dev_files/precommit.py index 54e1bba5..39710e72 100644 --- a/src/compwa_policy/check_dev_files/precommit.py +++ b/src/compwa_policy/check_dev_files/precommit.py @@ -10,7 +10,6 @@ from ruamel.yaml.scalarstring import DoubleQuotedScalarString from compwa_policy.errors import PrecommitError -from compwa_policy.utilities import CONFIG_PATH from compwa_policy.utilities.executor import Executor from compwa_policy.utilities.precommit.getters import find_repo from compwa_policy.utilities.pyproject import Pyproject, get_constraints_file @@ -41,7 +40,7 @@ def _sort_hooks(precommit: ModifiablePrecommit) -> None: sorted_repos = sorted(repos, key=__repo_sort_key) if sorted_repos != repos: precommit.document["repos"] = sorted_repos - msg = f"Sorted pre-commit hooks in {CONFIG_PATH.precommit}" + msg = "Sorted all pre-commit hooks" precommit.append_to_changelog(msg) @@ -80,7 +79,7 @@ def _update_precommit_ci_commit_msg(precommit: ModifiablePrecommit) -> None: autoupdate_commit_msg = precommit_ci.get(key) if autoupdate_commit_msg != expected_msg: precommit_ci[key] = expected_msg # type:ignore[literal-required] - msg = f"Updated ci.{key} in {CONFIG_PATH.precommit} to {expected_msg!r}" + msg = f"Set ci.{key} to {expected_msg!r}" precommit.append_to_changelog(msg) @@ -101,13 +100,13 @@ def _update_precommit_ci_skip(precommit: ModifiablePrecommit) -> None: existing_skips = precommit_ci.get("skip") if not expected_skips and existing_skips is not None: del precommit_ci["skip"] - msg = f"Removed redundant ci.skip section from {CONFIG_PATH.precommit}" + msg = "Removed redundant ci.skip section" precommit.append_to_changelog(msg) if existing_skips != expected_skips: precommit_ci["skip"] = sorted(expected_skips) yaml_config = cast(CommentedMap, precommit.document) yaml_config.yaml_set_comment_before_after_key("repos", before="\n") - msg = f"Updated ci.skip section in {CONFIG_PATH.precommit}" + msg = "Updated ci.skip section" precommit.append_to_changelog(msg) @@ -186,7 +185,7 @@ def _update_repo_urls(precommit: ModifiablePrecommit) -> None: repo["repo"] = new_url updated_repos.append((url, new_url)) if updated_repos: - msg = f"Updated repo urls in {CONFIG_PATH.precommit}:" + msg = "Updated repo URLs:" for url, new_url in updated_repos: msg += f"\n {url} -> {new_url}" precommit.append_to_changelog(msg) diff --git a/src/compwa_policy/check_dev_files/pyright.py b/src/compwa_policy/check_dev_files/pyright.py index 300d9b5d..f3f9dc4b 100644 --- a/src/compwa_policy/check_dev_files/pyright.py +++ b/src/compwa_policy/check_dev_files/pyright.py @@ -5,7 +5,6 @@ import json import os -from compwa_policy.utilities import CONFIG_PATH from compwa_policy.utilities.pyproject import ModifiablePyproject, complies_with_subset from compwa_policy.utilities.toml import to_toml_array @@ -17,18 +16,18 @@ def main() -> None: def _merge_config_into_pyproject(pyproject: ModifiablePyproject) -> None: - config_path = "pyrightconfig.json" # cspell:ignore pyrightconfig - if not os.path.exists(config_path): + old_config_path = "pyrightconfig.json" # cspell:ignore pyrightconfig + if not os.path.exists(old_config_path): return - with open(config_path) as stream: + with open(old_config_path) as stream: existing_config = json.load(stream) for key, value in existing_config.items(): if isinstance(value, list): existing_config[key] = to_toml_array(sorted(value)) tool_table = pyproject.get_table("tool.pyright", create=True) tool_table.update(existing_config) - os.remove(config_path) - msg = f"Moved pyright configuration to {CONFIG_PATH.pyproject}" + os.remove(old_config_path) + msg = f"Imported pyright configuration from {old_config_path}" pyproject.append_to_changelog(msg) @@ -42,5 +41,5 @@ def _update_settings(pyproject: ModifiablePyproject) -> None: } if not complies_with_subset(pyright_settings, minimal_settings): pyright_settings.update(minimal_settings) - msg = f"Updated pyright configuration in {CONFIG_PATH.pyproject}" + msg = "Updated pyright configuration" pyproject.append_to_changelog(msg) diff --git a/src/compwa_policy/check_dev_files/pytest.py b/src/compwa_policy/check_dev_files/pytest.py index 0e1665a1..a342d43d 100644 --- a/src/compwa_policy/check_dev_files/pytest.py +++ b/src/compwa_policy/check_dev_files/pytest.py @@ -39,7 +39,7 @@ def _merge_coverage_into_pyproject(pyproject: ModifiablePyproject) -> None: coverage_config[key] = [value] tool_table = pyproject.get_table("tool.coverage.run", create=True) tool_table.update(coverage_config) - msg = f"Merged Coverage.py configuration into {CONFIG_PATH.pyproject}" + msg = f"Imported Coverage.py configuration from {CONFIG_PATH.pytest_ini}" pyproject.append_to_changelog(msg) @@ -54,19 +54,20 @@ def _merge_pytest_into_pyproject(pyproject: ModifiablePyproject) -> None: tool_table = pyproject.get_table("tool", create=True) tool_table.update(pytest_config) CONFIG_PATH.pytest_ini.unlink() - msg = f"Moved pytest configuration to {CONFIG_PATH.pyproject}" + msg = f"Imported pytest configuration from {CONFIG_PATH.pytest_ini}" pyproject.append_to_changelog(msg) def _update_settings(pyproject: ModifiablePyproject) -> None: - if not pyproject.has_table("tool.pytest.ini_options"): + table_key = "tool.pytest.ini_options" + if not pyproject.has_table(table_key): return - config = pyproject.get_table("tool.pytest.ini_options") + config = pyproject.get_table(table_key) existing = config.get("addopts", "") expected = __get_expected_addopts(existing) if isinstance(existing, str) or sorted(existing) != sorted(expected): config["addopts"] = expected - msg = f"Updated tool.pytest.ini_options.addopts under {CONFIG_PATH.pyproject}" + msg = f"Updated [{table_key}]" pyproject.append_to_changelog(msg) diff --git a/src/compwa_policy/check_dev_files/ruff.py b/src/compwa_policy/check_dev_files/ruff.py index 58ce0ab9..9353beb9 100644 --- a/src/compwa_policy/check_dev_files/ruff.py +++ b/src/compwa_policy/check_dev_files/ruff.py @@ -8,7 +8,7 @@ from ruamel.yaml import YAML -from compwa_policy.utilities import CONFIG_PATH, natural_sorting, remove_configs, vscode +from compwa_policy.utilities import natural_sorting, remove_configs, vscode from compwa_policy.utilities.executor import Executor from compwa_policy.utilities.precommit.struct import Hook, Repo from compwa_policy.utilities.pyproject import ( @@ -108,7 +108,7 @@ def __remove_nbqa_option(pyproject: ModifiablePyproject, option: str) -> None: if option not in nbqa_table: return nbqa_table.pop(option) - msg = f"Removed {option!r} nbQA options from {CONFIG_PATH.pyproject}" + msg = f"Removed {option!r} nbQA options from [{table_key}]" pyproject.append_to_changelog(msg) @@ -116,7 +116,7 @@ def __remove_tool_table(pyproject: ModifiablePyproject, tool_table: str) -> None tools = pyproject._document.get("tool") if isinstance(tools, dict) and tool_table in tools: tools.pop(tool_table) - msg = f"Removed [tool.{tool_table}] section from {CONFIG_PATH.pyproject}" + msg = f"Removed [tool.{tool_table}] table" pyproject.append_to_changelog(msg) @@ -180,9 +180,7 @@ def _move_ruff_lint_config(pyproject: ModifiablePyproject) -> None: for key in lint_settings: del global_settings[key] if lint_arrays or lint_tables: - pyproject.append_to_changelog( - f"Moved linting configuration to [tool.ruff.lint] in {CONFIG_PATH.pyproject}" - ) + pyproject.append_to_changelog("Moved linting configuration to [tool.ruff.lint]") def _update_ruff_config( @@ -223,7 +221,7 @@ def __update_global_settings( minimal_settings[key] = ___merge(default_excludes, settings.get(key, [])) if not complies_with_subset(settings, minimal_settings): settings.update(minimal_settings) - msg = f"Updated Ruff configuration in {CONFIG_PATH.pyproject}" + msg = "Updated Ruff configuration" pyproject.append_to_changelog(msg) @@ -270,7 +268,7 @@ def __update_ruff_format_settings(pyproject: ModifiablePyproject) -> None: } if not complies_with_subset(settings, minimal_settings): settings.update(minimal_settings) - msg = f"Updated Ruff formatter configuration in {CONFIG_PATH.pyproject}" + msg = "Updated Ruff formatter configuration" pyproject.append_to_changelog(msg) @@ -301,7 +299,7 @@ def __update_ruff_lint_settings(pyproject: ModifiablePyproject) -> None: } if not complies_with_subset(settings, minimal_settings): settings.update(minimal_settings) - msg = f"Updated Ruff linting configuration in {CONFIG_PATH.pyproject}" + msg = "Updated Ruff linting configuration" pyproject.append_to_changelog(msg) @@ -418,7 +416,7 @@ def __update_per_file_ignores( minimal_settings[key] = ___merge_rules(default_ignores, settings.get(key, [])) if not complies_with_subset(settings, minimal_settings): settings.update(minimal_settings) - msg = f"Updated Ruff configuration in {CONFIG_PATH.pyproject}" + msg = "Updated Ruff configuration" pyproject.append_to_changelog(msg) @@ -469,7 +467,7 @@ def __update_isort_settings(pyproject: ModifiablePyproject) -> None: minimal_settings = {"split-on-trailing-comma": False} if not complies_with_subset(settings, minimal_settings): settings.update(minimal_settings) - msg = f"Updated Ruff isort settings in {CONFIG_PATH.pyproject}" + msg = "Updated Ruff isort settings" pyproject.append_to_changelog(msg) @@ -480,7 +478,7 @@ def __update_pydocstyle_settings(pyproject: ModifiablePyproject) -> None: } if not complies_with_subset(settings, minimal_settings): settings.update(minimal_settings) - msg = f"Updated Ruff configuration in {CONFIG_PATH.pyproject}" + msg = "Updated Ruff configuration" pyproject.append_to_changelog(msg) @@ -501,7 +499,7 @@ def ___remove_nbqa_settings(pyproject: ModifiablePyproject) -> None: tool_table = pyproject.get_table("tool", create=True) del tool_table["nbqa"] if nbqa_addopts: - msg = f"Removed Ruff configuration for nbQA from {CONFIG_PATH.pyproject}" + msg = "Removed Ruff configuration for nbQA" pyproject.append_to_changelog(msg) diff --git a/src/compwa_policy/utilities/executor.py b/src/compwa_policy/utilities/executor.py index cee8f8a2..fd809ce9 100644 --- a/src/compwa_policy/utilities/executor.py +++ b/src/compwa_policy/utilities/executor.py @@ -57,7 +57,7 @@ class Executor(AbstractContextManager): """ def __init__(self, raise_exception: bool = True) -> None: - self._raise_exception = raise_exception + self.__raise_exception = raise_exception self.__error_messages: list[str] = [] self.__is_in_context = False self.__execution_times: dict[str, float] = {} @@ -108,7 +108,7 @@ def __exit__( ) -> None: error_msg = self.merge_messages() if error_msg: - if self._raise_exception: + if self.__raise_exception: raise PrecommitError(error_msg) from exc_value print(error_msg) # noqa: T201 if os.getenv("COMPWA_POLICY_DEBUG") is not None: diff --git a/src/compwa_policy/utilities/precommit/__init__.py b/src/compwa_policy/utilities/precommit/__init__.py index e934ac5c..3c3ea51a 100644 --- a/src/compwa_policy/utilities/precommit/__init__.py +++ b/src/compwa_policy/utilities/precommit/__init__.py @@ -59,7 +59,8 @@ def load(cls: type[T], source: IO | Path | str = CONFIG_PATH.precommit) -> T: return cls(config, parser, source) def dumps(self) -> str: - return self.parser.dump(self.document) + with io.StringIO() as stream: + return self.parser.dump(self.document, stream) def find_repo(self, search_pattern: str) -> Repo | None: """Find pre-commit repo definition in pre-commit config.""" @@ -92,12 +93,12 @@ def __exit__( return if self.parser is None: self.dump(self.source) - msg = "Following modifications were made" + msg = "The following modifications were made" if isinstance(self.source, Path): - msg = f" to {self.source}" - msg += ":\n\n" - modifications = indent("\n".join(self.__changelog), prefix=" - ") - raise PrecommitError(modifications) + msg += f" to {self.source}" + msg += ":\n" + msg += indent("\n".join(self.__changelog), prefix=" - ") + raise PrecommitError(msg) def dump(self, target: IO | Path | str | None = None) -> None: if target is None: @@ -110,10 +111,9 @@ def dump(self, target: IO | Path | str | None = None) -> None: target.seek(0) self.parser.dump(self.document, target) target.seek(current_position) - elif isinstance(target, (Path, str)): - src = self.dumps() + elif isinstance(target, Path): with open(target, "w") as stream: - stream.write(src) + self.parser.dump(self.document, stream) else: msg = f"Target of type {type(target).__name__} is not supported" raise TypeError(msg) @@ -128,12 +128,15 @@ def __assert_is_in_context(self) -> None: raise RuntimeError(msg) def remove_hook(self, hook_id: str, repo_url: str | None = None) -> None: + self.__assert_is_in_context() remove_precommit_hook(self, hook_id, repo_url) def update_single_hook_repo(self, expected: Repo) -> None: + self.__assert_is_in_context() update_single_hook_precommit_repo(self, expected) def update_hook(self, repo_url: str, expected_hook: Hook) -> None: + self.__assert_is_in_context() update_precommit_hook(self, repo_url, expected_hook) diff --git a/src/compwa_policy/utilities/precommit/setters.py b/src/compwa_policy/utilities/precommit/setters.py index eb0e6b45..b9a7b626 100644 --- a/src/compwa_policy/utilities/precommit/setters.py +++ b/src/compwa_policy/utilities/precommit/setters.py @@ -27,7 +27,7 @@ def remove_precommit_hook( repos.pop(repo_idx) else: hooks.pop(hook_idx) - msg = f"Removed {hook_id!r} from {CONFIG_PATH.precommit}" + msg = f"Removed {hook_id!r} hook" precommit.append_to_changelog(msg) @@ -82,7 +82,7 @@ def update_single_hook_precommit_repo( repos[idx] = expected_yaml # type: ignore[assignment,call-overload] repos_map = cast(CommentedMap, repos) repos_map.yaml_set_comment_before_after_key(idx + 1, before="\n") - msg = f"Updated {hook_id} hook in {CONFIG_PATH.precommit}" + msg = f"Updated {hook_id} hook" precommit.append_to_changelog(msg) diff --git a/src/compwa_policy/utilities/pyproject/__init__.py b/src/compwa_policy/utilities/pyproject/__init__.py index 19789875..86dff787 100644 --- a/src/compwa_policy/utilities/pyproject/__init__.py +++ b/src/compwa_policy/utilities/pyproject/__init__.py @@ -164,12 +164,12 @@ def __exit__( return if self._source is None: self.dump(self._source) - msg = "Following modifications were made" + msg = "The following modifications were made" if isinstance(self._source, (Path, str)): - msg = f" to {self._source}" - msg += ":\n\n" - modifications = indent("\n".join(self._changelog), prefix=" - ") - raise PrecommitError(modifications) + msg += f" to {self._source}" + msg += ":\n" + msg += indent("\n".join(self._changelog), prefix=" - ") + raise PrecommitError(msg) def dump(self, target: IO | Path | str | None = None) -> None: if target is None and self._source is None: diff --git a/tests/check_dev_files/test_cspell.py b/tests/check_dev_files/test_cspell.py index 7bb1994a..172234b8 100644 --- a/tests/check_dev_files/test_cspell.py +++ b/tests/check_dev_files/test_cspell.py @@ -1,3 +1,4 @@ +import io from pathlib import Path import pytest @@ -7,14 +8,29 @@ from compwa_policy.utilities.precommit import ModifiablePrecommit, Precommit -def test_update_cspell_repo_url(): - test_dir = Path(__file__).parent / "cspell" +def test_update_cspell_repo_url(bad_yaml: io.StringIO, good_yaml: io.StringIO): with pytest.raises( PrecommitError, match=r"Updated cSpell pre-commit repo URL" - ), ModifiablePrecommit.load(test_dir / ".pre-commit-config-bad.yaml") as bad: + ), ModifiablePrecommit.load(bad_yaml) as bad: _update_cspell_repo_url(bad) - good_config = Precommit.load(test_dir / ".pre-commit-config-good.yaml") - imported = good_config.document["repos"][0]["repo"] + good = Precommit.load(good_yaml) + imported = good.document["repos"][0]["repo"] expected = bad.document["repos"][0]["repo"] assert imported == expected + + +@pytest.fixture(scope="module") +def bad_yaml() -> io.StringIO: + return load_config(".pre-commit-config-bad.yaml") + + +@pytest.fixture(scope="module") +def good_yaml() -> io.StringIO: + return load_config(".pre-commit-config-good.yaml") + + +def load_config(filename: str) -> io.StringIO: + path = Path(__file__).parent / "cspell" / filename + with path.open() as file: + return io.StringIO(file.read())