Implement review suggestions and fix leftovers

Author: MaxymVlasov

.gitignore

@@ -177,3 +177,5 @@ pyrightconfig.json
 # End of https://www.toptal.com/developers/gitignore/api/python
 
 tests/results/*
+.python-version
+uv.lock

.pre-commit-config.yaml

@@ -105,7 +105,6 @@ repos:
     args:
     - --fix
   - id: ruff-format
-    exclude: /terraform_docs_replace(_test)?\.py$
 
 - repo: https://github.com/pre-commit/mirrors-mypy.git
   rev: v1.15.0

ruff.toml

@@ -18,7 +18,8 @@ convention = "pep257"
 select = ["ALL"]
 preview = true
 ignore = [
-  "CPY001", # Missing copyright notice at top of file
+  "CPY001", # Skip copyright notice requirement at top of files
+  "S404",   # Allow importing 'subprocess' module to call external tools needed by these hooks
 ]
 
 [lint.isort]
@@ -29,11 +30,10 @@ lines-after-imports = 2
 parametrize-values-type = "tuple"
 
 [lint.per-file-ignores]
-# Ignore in the `tests/` directory.
+# Exceptions for test files
 "tests/**.py" = [
-  "S101", # Use of `assert` detected
-  "PLC2701", # We need import marked as internal files for testing
+  "S101",    # Allow use of `assert` in test files
+  "PLC2701", # Allow importing internal files needed for testing
+  "PLR6301", # Allow 'self' parameter in method definitions (required for test stubs)
+  "ARG002",  # Allow unused arguments in instance methods (required for test stubs)
 ]
-
-"src/pre_commit_terraform/terraform_docs_replace.py" = ["ALL"] # Deprecated hook
-"tests/pytest/terraform_docs_replace_test.py" = ["ALL"] # Tests for deprecated hook

src/pre_commit_terraform/_cli.py

@@ -28,7 +28,7 @@ def invoke_cli_app(cli_args: list[str]) -> ReturnCodeType:
     root_cli_parser = initialize_argument_parser()
     parsed_cli_args = root_cli_parser.parse_args(cli_args)
     invoke_cli_app = cast_to(
-        # FIXME: attempt typing per https://stackoverflow.com/a/75666611/595220  # noqa: TD001, TD002, TD003, FIX001, E501
+        # FIXME: attempt typing per https://stackoverflow.com/a/75666611/595220  # noqa: TD001, TD002, TD003, FIX001, E501 All these suppressions caused by "FIXME" comment
         'CLIAppEntryPointCallableType',
         parsed_cli_args.invoke_cli_app,
     )

src/pre_commit_terraform/_errors.py

@@ -12,5 +12,8 @@ class PreCommitTerraformRuntimeError(
     """An exception representing a runtime error condition."""
 
 
-class PreCommitTerraformExit(PreCommitTerraformBaseError, SystemExit):  # noqa: N818 FIXME
+# N818 - The name mimics the built-in SystemExit and is meant to have exactly
+# the same semantics. For this reason, it shouldn't have Error in the name to
+# maintain resemblance.
+class PreCommitTerraformExit(PreCommitTerraformBaseError, SystemExit):  # noqa: N818
     """An exception for terminating execution from deep app layers."""

src/pre_commit_terraform/terraform_docs_replace.py

@@ -1,7 +1,14 @@
+"""Terraform Docs Replace Hook.
+
+This hook is deprecated and will be removed in the future.
+Please, use 'terraform_docs' hook instead.
+"""
+
 import os
 import subprocess
 import warnings
 from argparse import ArgumentParser, Namespace
+from pathlib import Path
 from typing import cast as cast_to
 
 from ._structs import ReturnCode
@@ -68,7 +75,7 @@ def invoke_cli_app(parsed_cli_args: Namespace) -> ReturnCodeType:
         if os.path.realpath(filename) not in dirs and (
             filename.endswith(('.tf', '.tfvars'))
         ):
-            dirs.append(os.path.dirname(filename))
+            dirs.append(str(Path(filename).parent))
 
     retval = ReturnCode.OK
 
@@ -89,8 +96,13 @@ def invoke_cli_app(parsed_cli_args: Namespace) -> ReturnCodeType:
                     ),
                 ),
             )
-            subprocess.check_call(' '.join(proc_args), shell=True)
-        except subprocess.CalledProcessError as e:
-            print(e)
+            # S602 - 'shell=True' is insecure, but this hook is deprecated and
+            # we don't want to spent time on testing fixes for it
+            subprocess.check_call(' '.join(proc_args), shell=True)  # noqa: S602
+        # PERF203 - try-except shouldn't be in a loop, but it's deprecated
+        # hook, so leave as is
+        except subprocess.CalledProcessError as e:  # noqa: PERF203
+            # T201 - Leave print statement as is, as this is deprecated hook
+            print(e)  # noqa: T201
             retval = ReturnCode.ERROR
     return retval

tests/pytest/_cli_test.py

@@ -51,13 +51,13 @@ def test_known_interrupts(
     class CustomCmdStub:
         CLI_SUBCOMMAND_NAME = 'sentinel'
 
-        def populate_argument_parser(  # noqa: PLR6301
+        def populate_argument_parser(
             self,
-            subcommand_parser: ArgumentParser,  # noqa: ARG002
+            subcommand_parser: ArgumentParser,
         ) -> None:
             return None
 
-        def invoke_cli_app(self, parsed_cli_args: Namespace) -> ReturnCodeType:  # noqa: PLR6301, ARG002
+        def invoke_cli_app(self, parsed_cli_args: Namespace) -> ReturnCodeType:
             raise raised_error
 
     monkeypatch.setattr(
@@ -81,16 +81,13 @@ def test_app_exit(
     class CustomCmdStub:
         CLI_SUBCOMMAND_NAME = 'sentinel'
 
-        def populate_argument_parser(  # noqa: PLR6301
+        def populate_argument_parser(
             self,
-            subcommand_parser: ArgumentParser,  # noqa: ARG002
+            subcommand_parser: ArgumentParser,
         ) -> None:
             return None
 
-        def invoke_cli_app(
-            self,
-            parsed_cli_args: Namespace,  # noqa: ARG002
-        ) -> ReturnCodeType:
+        def invoke_cli_app(self, parsed_cli_args: Namespace) -> ReturnCodeType:
             raise PreCommitTerraformExit(self.CLI_SUBCOMMAND_NAME)
 
     monkeypatch.setattr(

tests/pytest/terraform_docs_replace_test.py

@@ -120,5 +120,6 @@ def test_control_flow_negative(
     )
 
     assert invoke_cli_app(parsed_cli_args) == ReturnCode.ERROR
-
-    check_call_mock.assert_called_once_with(expected_cmd, shell=True)
+    # S604 - 'shell=True' is insecure, but this hook is deprecated and we don't
+    # want to spent time on testing fixes for it
+    check_call_mock.assert_called_once_with(expected_cmd, shell=True)  # noqa: S604