From e3206959a1d402175053a2bab6de5a4c5a918211 Mon Sep 17 00:00:00 2001 From: Madison Steiner Date: Wed, 26 Aug 2026 15:30:46 +0000 Subject: [PATCH] fix: satisfy ruff's expanded default rule set, and cap the version `ruff check ./pre_commit_hooks_tests/ ./pre_commit_hooks` has failed on every pull request since ruff 0.16 shipped. It last passed in March. The six findings are all pre-existing code that earlier ruff releases did not look at: requirements-dev.txt pins nothing, so CI installs whatever is current, and 0.16 expanded which rules are on by default. Reproduced on an unmodified main with ruff 0.16.4, which is what CI installed: two I001 (import blocks unsorted), one UP035 (`Sequence` from `collections.abc`), one UP007 (`X | Y` in an annotation) and two SIM117 (nested `with`). `from __future__ import annotations` is added to pre_commit_hooks/cfn_guard.py first, and it is what makes the rest safe rather than being cosmetic. setup.cfg declares `python_requires = >=3.8`; both `Sequence[str] | None` and a subscripted `collections.abc.Sequence` are runtime errors before 3.10 and 3.9 respectively without it. Applying ruff's suggestions without that import would have made the published hook fail to import on the Pythons this package says it supports. pre_commit_hooks_tests/test_cfn_guard.py already carries the same import. The two SIM117 sites combine two context managers into one `with`, kept on a single line: the parenthesized multi-line form needs 3.10. requirements-dev.txt caps ruff below 0.17 rather than leaving it open. The cap is the part that stops this recurring: raising it becomes a deliberate change, with whatever new findings it introduces visible in the same diff instead of appearing on an unrelated pull request. black, mypy and pytest are also unpinned and CI has drifted to major versions of all three -- black 26, mypy 2, pytest 9 -- which currently pass. Capping those too is a broader decision and is left alone here. Verified with CI's own four commands: black, ruff and mypy exit 0. The two remaining pytest failures reproduce identically on an unmodified main, are `assert 126 == ...` from the downloaded binary not being executable in a sandbox, and pass in CI. --- pre_commit_hooks/cfn_guard.py | 23 +++++++++++------------ pre_commit_hooks_tests/test_cfn_guard.py | 4 ++-- requirements-dev.txt | 2 +- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/pre_commit_hooks/cfn_guard.py b/pre_commit_hooks/cfn_guard.py index edfee3fc7..af1737a7f 100644 --- a/pre_commit_hooks/cfn_guard.py +++ b/pre_commit_hooks/cfn_guard.py @@ -2,6 +2,9 @@ This module contains the logic for the cfn-guard pre-commit hook """ +from __future__ import annotations + +import argparse import os import platform import shutil @@ -9,9 +12,8 @@ import sys import tarfile import tempfile -import argparse +from collections.abc import Sequence from pathlib import Path -from typing import Sequence, Union from urllib.request import Request, urlopen BIN_NAME = "cfn-guard" @@ -84,9 +86,8 @@ def install_cfn_guard(): if current_os in supported_oses: url = release_urls_dict[current_os].replace("TAG", GUARD_BINARY_VERSION) # Download tarball of release from Github - with tempfile.NamedTemporaryFile(delete=False) as temp_file: - with urlopen(url) as response: - shutil.copyfileobj(response, temp_file) + with tempfile.NamedTemporaryFile(delete=False) as temp_file, urlopen(url) as response: + shutil.copyfileobj(response, temp_file) # Create the install_dir if it doesn't exist os.makedirs(install_dir, exist_ok=True) @@ -100,12 +101,10 @@ def install_cfn_guard(): filename = os.path.basename(member.name) # Join the install_dir path and the filename to get the full target path file_path = os.path.join(install_dir, filename) - # Open the archived file - with tar.extractfile(member) as source: - # Create a new file using the file_path with write binary mode - with open(file_path, "wb") as target: - # Copy the contents of the archived file(s) to the target file - shutil.copyfileobj(source, target) + # Open the archived file, and a new file at file_path in write binary mode + with tar.extractfile(member) as source, open(file_path, "wb") as target: + # Copy the contents of the archived file(s) to the target file + shutil.copyfileobj(source, target) binary_path = os.path.join(install_dir, binary_name) os.chmod(binary_path, 0o755) @@ -135,7 +134,7 @@ def run_cfn_guard(args: str) -> int: return run_cfn_guard(args) -def main(argv: Union[Sequence[str], None] = None) -> int: +def main(argv: Sequence[str] | None = None) -> int: """Entry point for the pre-commit hook""" if argv is None: argv = sys.argv[1:] diff --git a/pre_commit_hooks_tests/test_cfn_guard.py b/pre_commit_hooks_tests/test_cfn_guard.py index 8d7eedbf8..10648f3e0 100644 --- a/pre_commit_hooks_tests/test_cfn_guard.py +++ b/pre_commit_hooks_tests/test_cfn_guard.py @@ -5,10 +5,10 @@ from __future__ import annotations -from pre_commit_hooks.cfn_guard import main - import os.path +from pre_commit_hooks.cfn_guard import main + def get_guard_resource_path(relative_path): return os.path.join(os.path.abspath(__file__ + "/../../")) + "/guard/resources" + relative_path diff --git a/requirements-dev.txt b/requirements-dev.txt index b3bc21d7d..169308711 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,4 +1,4 @@ pytest mypy black -ruff +ruff>=0.16.4,<0.17