From cc804ee890aff1d2f64bd737a1430055c059ccd9 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Tue, 20 Oct 2026 14:01:22 -0400 Subject: [PATCH] Require hook name with `rapids-pre-commit-hooks:` directives Allowing the hook name to be omitted with these directives was a terrible idea. Require it, and update the tests and documentation accordingly. At the time of this PR, there are very few places where the directive is used without the check name, and this change would not disrupt any of them. --- README.md | 5 +- .../hardcoded_version.py | 2 +- src/rapids_pre_commit_hooks/lint.py | 10 +- .../rapids_pre_commit_hooks/test_copyright.py | 25 +-- .../test_hardcoded_version.py | 5 +- tests/rapids_pre_commit_hooks/test_lint.py | 142 ++++++++++++------ 6 files changed, 123 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index 209c85e..02497b1 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,10 @@ Copy the following into your repository's `.pre-commit-config.yaml`: Some checks are prone to false positives, particularly `verify-hardcoded-version`. In such cases, you can use `rapids-pre-commit-hooks: disable`, `rapids-pre-commit-hooks: enable`, and -`rapids-pre-commit-hooks: disable-next-line` directives: +`rapids-pre-commit-hooks: disable-next-line` directives. Each directive must include a +non-empty, comma-separated list of check names in brackets, such as +`disable[verify-hardcoded-version,verify-copyright]`. Directives without a check list +are ignored. Only the listed checks are affected: ```python def deprecated_function(): diff --git a/src/rapids_pre_commit_hooks/hardcoded_version.py b/src/rapids_pre_commit_hooks/hardcoded_version.py index 5cd5234..afd6520 100644 --- a/src/rapids_pre_commit_hooks/hardcoded_version.py +++ b/src/rapids_pre_commit_hooks/hardcoded_version.py @@ -252,7 +252,7 @@ def check_hardcoded_version( match.span("full"), "if this is intentional (as part of a docstring or deprecation " "notice), suppress it with rapids-pre-commit-hooks: " - "disable-next-line - see " + "disable-next-line[verify-hardcoded-version] - see " "https://github.com/rapidsai/pre-commit-hooks/blob/main/README.md" "#suppressing-false-positives for details", ) diff --git a/src/rapids_pre_commit_hooks/lint.py b/src/rapids_pre_commit_hooks/lint.py index ad32c89..bc975d1 100644 --- a/src/rapids_pre_commit_hooks/lint.py +++ b/src/rapids_pre_commit_hooks/lint.py @@ -139,8 +139,8 @@ class Linter: _DISABLE_ENABLE_DIRECTIVE_RE: re.Pattern = re.compile( r"\brapids-pre-commit-hooks: *" r"(?Penable|disable)" - r"(?:-(?Pnext-line))?(?P\b)?" - r"(?: *\[(?P[\w-]+(?:,[\w-]+)*)\])?" + r"(?:-(?Pnext-line))?" + r" *\[(?P[\w-]+(?:,[\w-]+)*)\]" ) def __init__(self, filename: str, content: str, warning_name: str) -> None: @@ -335,11 +335,7 @@ def handle_end(end: int) -> "Generator[tuple[Span, bool]]": for m in Linter._DISABLE_ENABLE_DIRECTIVE_RE.finditer( lines.content ): - if m.group("word_boundary") is None: - continue - if m.group("warning_names") and warning_name not in m.group( - "warning_names" - ).split(","): + if warning_name not in m.group("warning_names").split(","): continue directive_is_enable = m.group("directive_name") == "enable" diff --git a/tests/rapids_pre_commit_hooks/test_copyright.py b/tests/rapids_pre_commit_hooks/test_copyright.py index e9f971a..a7d28d2 100644 --- a/tests/rapids_pre_commit_hooks/test_copyright.py +++ b/tests/rapids_pre_commit_hooks/test_copyright.py @@ -461,36 +461,41 @@ def test_match_copyright(content): f"""\ + rapids-pre-commit-hooks: {"disable"} + Copyright (c) 2021 NVIDIA CORPORATION - + rapids-pre-commit-hooks: {"enable"} - + Copyright (c) 2022 NVIDIA CORPORATION : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~0.span : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~0.full_copyright_text_span : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~0.nvidia_copyright_text_span : ~~~~0.years_span : ~~~~0.first_year_span - + rapids-pre-commit-hooks: {"disable[verify-copyright]"} - + Copyright (c) 2023 NVIDIA CORPORATION - + rapids-pre-commit-hooks: {"enable[verify-copyright]"} - + Copyright (c) 2024 NVIDIA CORPORATION + + rapids-pre-commit-hooks: {"enable"} + + Copyright (c) 2022 NVIDIA CORPORATION : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~1.span : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~1.full_copyright_text_span : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~1.nvidia_copyright_text_span : ~~~~1.years_span : ~~~~1.first_year_span - + rapids-pre-commit-hooks: {"disable[verify-codeowners]"} - + Copyright (c) 2025 NVIDIA CORPORATION + + rapids-pre-commit-hooks: {"disable[verify-copyright]"} + + Copyright (c) 2023 NVIDIA CORPORATION + + rapids-pre-commit-hooks: {"enable[verify-copyright]"} + + Copyright (c) 2024 NVIDIA CORPORATION : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~2.span : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~2.full_copyright_text_span : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~2.nvidia_copyright_text_span : ~~~~2.years_span : ~~~~2.first_year_span - + rapids-pre-commit-hooks: {"enable[verify-codeowners]"} - + Copyright (c) 2026 NVIDIA CORPORATION + + rapids-pre-commit-hooks: {"disable[verify-codeowners]"} + + Copyright (c) 2025 NVIDIA CORPORATION : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~3.span : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~3.full_copyright_text_span : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~3.nvidia_copyright_text_span : ~~~~3.years_span : ~~~~3.first_year_span + + rapids-pre-commit-hooks: {"enable[verify-codeowners]"} + + Copyright (c) 2026 NVIDIA CORPORATION + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~4.span + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~4.full_copyright_text_span + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~4.nvidia_copyright_text_span + : ~~~~4.years_span + : ~~~~4.first_year_span """, id="disable-enable-dirctives", ), diff --git a/tests/rapids_pre_commit_hooks/test_hardcoded_version.py b/tests/rapids_pre_commit_hooks/test_hardcoded_version.py index f81beba..59a25d3 100644 --- a/tests/rapids_pre_commit_hooks/test_hardcoded_version.py +++ b/tests/rapids_pre_commit_hooks/test_hardcoded_version.py @@ -1,4 +1,4 @@ -# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION. +# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 import contextlib @@ -1236,7 +1236,8 @@ def test_check_hardcoded_version( s, "if this is intentional (as part of a docstring or " "deprecation notice), suppress it with " - "rapids-pre-commit-hooks: disable-next-line - see " + "rapids-pre-commit-hooks: " + "disable-next-line[verify-hardcoded-version] - see " "https://github.com/rapidsai/pre-commit-hooks/blob/main/" "README.md#suppressing-false-positives for details", ) diff --git a/tests/rapids_pre_commit_hooks/test_lint.py b/tests/rapids_pre_commit_hooks/test_lint.py index a6a0225..4309f12 100644 --- a/tests/rapids_pre_commit_hooks/test_lint.py +++ b/tests/rapids_pre_commit_hooks/test_lint.py @@ -422,7 +422,7 @@ def test_fix(self): def test_fix_disabled(self): content, spans = parse_named_spans( """\ - + # rapids-pre-commit-hooks: disable + + # rapids-pre-commit-hooks: disable[test] + Hello world! : ~shout """ @@ -497,27 +497,27 @@ def test_sort_and_check_replacements_for_conflicts(self): ), pytest.param( """\ - + # rapids-pre-commit-hooks:disable + + # rapids-pre-commit-hooks:disable[other,test] : ~~0 - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~1 + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~1 + Hello : ~~~~~~1 """, "test", [True, False], - id="single-unfiltered-disable", + id="single-multiple-checks-disable", ), pytest.param( """\ - + # rapids-pre-commit-hooks: enable + + # rapids-pre-commit-hooks: enable[other,test] : ~~0 - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~1 + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~1 + Hello : ~~~~~~1 """, "test", [True, True], - id="single-unfiltered-enable", + id="single-multiple-checks-enable", ), pytest.param( """\ @@ -567,27 +567,27 @@ def test_sort_and_check_replacements_for_conflicts(self): ), pytest.param( """\ - + # rapids-pre-commit-hooks: disable-next-line - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~0 + + # rapids-pre-commit-hooks: disable-next-line[other,test] + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~0 + Hello : ~~~~~1 : ~2 """, "test", [True, False, True], - id="single-unfiltered-disable-next-line", + id="single-multiple-checks-disable-next-line", ), pytest.param( """\ - + # rapids-pre-commit-hooks: enable-next-line - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~0 + + # rapids-pre-commit-hooks: enable-next-line[other,test] + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~0 + Hello : ~~~~~1 : ~2 """, "test", [True, True, True], - id="single-unfiltered-enable-next-line", + id="single-multiple-checks-enable-next-line", ), pytest.param( """\ @@ -637,15 +637,15 @@ def test_sort_and_check_replacements_for_conflicts(self): ), pytest.param( """\ - + # rapids-pre-commit-hooks: enable-next-line rapids-pre-commit-hooks: disable-next-line rapids-pre-commit-hooks: enable - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~0 - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~1 - + # rapids-pre-commit-hooks: enable rapids-pre-commit-hooks: disable - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~2 - : ~3 - + # rapids-pre-commit-hooks: enable + + # rapids-pre-commit-hooks: enable-next-line[test] rapids-pre-commit-hooks: disable-next-line[test] rapids-pre-commit-hooks: enable[test] + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~0 + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~1 + + # rapids-pre-commit-hooks: enable[test] rapids-pre-commit-hooks: disable[test] + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~2 + : ~3 + + # rapids-pre-commit-hooks: enable[test] : ~~3 - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~4 + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~4 """, # noqa: E501 "test", [True, True, False, False, True], @@ -653,10 +653,10 @@ def test_sort_and_check_replacements_for_conflicts(self): ), pytest.param( """\ - + # prapids-pre-commit-hooks: enable + + # prapids-pre-commit-hooks: enable[test] : >0 - + # rapids-pre-commit-hooks: enabled - + # rapids-pre-commit-hooks: enable-next-lines + + # rapids-pre-commit-hooks: enabled[test] + + # rapids-pre-commit-hooks: enable-next-lines[test] + Hello : !0 """, @@ -666,13 +666,13 @@ def test_sort_and_check_replacements_for_conflicts(self): ), pytest.param( """\ - + # rapids-pre-commit-hooks: disable-next-line - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~0 + + # rapids-pre-commit-hooks: disable-next-line[test] + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~0 + Hello : ~~~~~1 : ~2 - + # rapids-pre-commit-hooks: disable-next-line - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~2 + + # rapids-pre-commit-hooks: disable-next-line[test] + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~2 + Hello : ~~~~~3 : ~4 @@ -691,6 +691,58 @@ def test_get_disabled_enabled_boundaries( Lines(content), warning_name ) == list(zip(spans, expected_boundaries, strict=True)) + @pytest.mark.parametrize( + ["directive"], + [ + pytest.param("enable", id="enable-missing-checks"), + pytest.param("enable[]", id="enable-empty-checks"), + pytest.param("enable[test,]", id="enable-malformed-checks"), + pytest.param("disable", id="disable-missing-checks"), + pytest.param("disable[]", id="disable-empty-checks"), + pytest.param("disable[test,]", id="disable-malformed-checks"), + pytest.param( + "enable-next-line", id="enable-next-line-missing-checks" + ), + pytest.param( + "enable-next-line[]", id="enable-next-line-empty-checks" + ), + pytest.param( + "enable-next-line[test,]", + id="enable-next-line-malformed-checks", + ), + pytest.param( + "disable-next-line", id="disable-next-line-missing-checks" + ), + pytest.param( + "disable-next-line[]", id="disable-next-line-empty-checks" + ), + pytest.param( + "disable-next-line[test,]", + id="disable-next-line-malformed-checks", + ), + ], + ) + def test_directives_require_checks(self, directive): + content, spans = parse_named_spans( + f"""\ + + # rapids-pre-commit-hooks: {directive} + + Hello + : ~~~~~warning + """ + ) + for prefix, expected_enabled in ( + ("", True), + ("# rapids-pre-commit-hooks: disable[test]\n", False), + ): + boundaries = Linter.get_disabled_enabled_boundaries( + Lines(prefix + content), "test" + ) + warning_span = tuple(pos + len(prefix) for pos in spans["warning"]) + assert ( + Linter.is_warning_span_enabled(boundaries, warning_span) + == expected_enabled + ) + @pytest.mark.parametrize( ["content", "expected_enabled"], [ @@ -733,10 +785,10 @@ def test_get_disabled_enabled_boundaries( ), pytest.param( """\ - + # rapids-pre-commit-hooks: enable - + # rapids-pre-commit-hooks: disable - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~warning - + # rapids-pre-commit-hooks: enable + + # rapids-pre-commit-hooks: enable[relevant] + + # rapids-pre-commit-hooks: disable[relevant] + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~warning + + # rapids-pre-commit-hooks: enable[relevant] : ~~warning """, False, @@ -744,10 +796,10 @@ def test_get_disabled_enabled_boundaries( ), pytest.param( """\ - + # rapids-pre-commit-hooks: enable - + # rapids-pre-commit-hooks: disable - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~warning - + # rapids-pre-commit-hooks: enable + + # rapids-pre-commit-hooks: enable[relevant] + + # rapids-pre-commit-hooks: disable[relevant] + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~warning + + # rapids-pre-commit-hooks: enable[relevant] : ~warning """, False, @@ -755,10 +807,10 @@ def test_get_disabled_enabled_boundaries( ), pytest.param( """\ - + # rapids-pre-commit-hooks: enable - + # rapids-pre-commit-hooks: disable - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~warning - + # rapids-pre-commit-hooks: enable + + # rapids-pre-commit-hooks: enable[relevant] + + # rapids-pre-commit-hooks: disable[relevant] + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~warning + + # rapids-pre-commit-hooks: enable[relevant] : ~~warning """, True, @@ -766,10 +818,10 @@ def test_get_disabled_enabled_boundaries( ), pytest.param( """\ - + # rapids-pre-commit-hooks: enable - + # rapids-pre-commit-hooks: disable - : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~warning - + # rapids-pre-commit-hooks: enable + + # rapids-pre-commit-hooks: enable[relevant] + + # rapids-pre-commit-hooks: disable[relevant] + : ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~warning + + # rapids-pre-commit-hooks: enable[relevant] : ~~~warning """, True, @@ -880,7 +932,7 @@ def disabled_file(self, file_with_spans): return file_with_spans( "disabled.txt", """\ - + # rapids-pre-commit-hooks: disable + + # rapids-pre-commit-hooks: disable[test] + Hello! : ~shout """,