refactor new changes

shenxianpeng · shenxianpeng · commit 48b848bcc6a9 · 2025-07-04T00:26:30.000+03:00
diff --git a/cpp_linter_hooks/clang_format.py b/cpp_linter_hooks/clang_format.py
@@ -3,7 +3,12 @@
 from argparse import ArgumentParser
 from typing import Tuple
 
-from .util import ensure_installed, DEFAULT_CLANG_VERSION
+from .util import (
+    ensure_installed,
+    DEFAULT_CLANG_VERSION,
+    debug_print,
+    run_subprocess_with_logging,
+)
 
 
 parser = ArgumentParser()
@@ -21,71 +26,17 @@ def run_clang_format(args=None) -> Tuple[int, str]:
 
     verbose = hook_args.verbose
 
-    if verbose:
-        print(f"[DEBUG] clang-format command: {' '.join(command)}", file=sys.stderr)
-        print(f"[DEBUG] clang-format version: {hook_args.version}", file=sys.stderr)
-        print(f"[DEBUG] clang-format executable: {path}", file=sys.stderr)
-
-    retval = 0
-    output = ""
-    stderr_output = ""
-
-    try:
-        if "--dry-run" in command:
-            sp = subprocess.run(
-                command,
-                stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE,
-                encoding="utf-8",
-            )
-            retval = -1  # Not a fail just identify it's a dry-run.
-            output = sp.stdout
-            stderr_output = sp.stderr
-
-            if verbose:
-                print("[DEBUG] Dry-run mode detected", file=sys.stderr)
-                print(
-                    f"[DEBUG] Exit code: {sp.returncode} (converted to -1 for dry-run)",
-                    file=sys.stderr,
-                )
-        else:
-            sp = subprocess.run(
-                command,
-                stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE,
-                encoding="utf-8",
-            )
-            retval = sp.returncode
-            output = sp.stdout
-            stderr_output = sp.stderr
-
-            if verbose:
-                print(f"[DEBUG] Exit code: {retval}", file=sys.stderr)
-
-        # Combine stdout and stderr for comprehensive output
-        combined_output = ""
-        if output:
-            combined_output += output
-        if stderr_output:
-            if combined_output:
-                combined_output += "\n"
-            combined_output += stderr_output
-
-        if verbose and (output or stderr_output):
-            print(f"[DEBUG] stdout: {repr(output)}", file=sys.stderr)
-            print(f"[DEBUG] stderr: {repr(stderr_output)}", file=sys.stderr)
-
-        return retval, combined_output
-
-    except FileNotFoundError as error:
-        retval = 1
-        error_msg = f"clang-format executable not found: {error}"
+    # Log initial debug information
+    debug_print(f"clang-format version: {hook_args.version}", verbose)
+    debug_print(f"clang-format executable: {path}", verbose)
 
-        if verbose:
-            print(f"[DEBUG] FileNotFoundError: {error}", file=sys.stderr)
-            print(f"[DEBUG] Command attempted: {' '.join(command)}", file=sys.stderr)
+    # Check for dry-run mode
+    dry_run = "--dry-run" in command
 
-        return retval, error_msg
+    # Use the utility function for subprocess execution
+    return run_subprocess_with_logging(
+        command=command, tool_name="clang-format", verbose=verbose, dry_run=dry_run
+    )
 
 
 def main() -> int:
diff --git a/cpp_linter_hooks/clang_tidy.py b/cpp_linter_hooks/clang_tidy.py
@@ -3,7 +3,12 @@
 from argparse import ArgumentParser
 from typing import Tuple
 
-from .util import ensure_installed, DEFAULT_CLANG_VERSION
+from .util import (
+    ensure_installed,
+    DEFAULT_CLANG_VERSION,
+    debug_print,
+    run_subprocess_with_logging,
+)
 
 
 parser = ArgumentParser()
@@ -21,57 +26,14 @@ def run_clang_tidy(args=None) -> Tuple[int, str]:
 
     verbose = hook_args.verbose
 
-    if verbose:
-        print(f"[DEBUG] clang-tidy command: {' '.join(command)}", file=sys.stderr)
-        print(f"[DEBUG] clang-tidy version: {hook_args.version}", file=sys.stderr)
-        print(f"[DEBUG] clang-tidy executable: {path}", file=sys.stderr)
-
-    retval = 0
-    output = ""
-    stderr_output = ""
-
-    try:
-        sp = subprocess.run(
-            command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8"
-        )
-        retval = sp.returncode
-        output = sp.stdout
-        stderr_output = sp.stderr
-
-        if verbose:
-            print(f"[DEBUG] Exit code: {retval}", file=sys.stderr)
-            print(f"[DEBUG] stdout: {repr(output)}", file=sys.stderr)
-            print(f"[DEBUG] stderr: {repr(stderr_output)}", file=sys.stderr)
-
-        # Combine stdout and stderr for comprehensive output
-        combined_output = ""
-        if output:
-            combined_output += output
-        if stderr_output:
-            if combined_output:
-                combined_output += "\n"
-            combined_output += stderr_output
-
-        # Check for warnings or errors in the combined output
-        if "warning:" in combined_output or "error:" in combined_output:
-            retval = 1
-            if verbose:
-                print(
-                    "[DEBUG] Found warnings/errors, setting exit code to 1",
-                    file=sys.stderr,
-                )
-
-        return retval, combined_output
-
-    except FileNotFoundError as error:
-        retval = 1
-        error_msg = f"clang-tidy executable not found: {error}"
-
-        if verbose:
-            print(f"[DEBUG] FileNotFoundError: {error}", file=sys.stderr)
-            print(f"[DEBUG] Command attempted: {' '.join(command)}", file=sys.stderr)
+    # Log initial debug information
+    debug_print(f"clang-tidy version: {hook_args.version}", verbose)
+    debug_print(f"clang-tidy executable: {path}", verbose)
 
-        return retval, error_msg
+    # Use the utility function for subprocess execution
+    return run_subprocess_with_logging(
+        command=command, tool_name="clang-tidy", verbose=verbose, dry_run=False
+    )
 
 
 def main() -> int:
diff --git a/cpp_linter_hooks/util.py b/cpp_linter_hooks/util.py
@@ -1,7 +1,8 @@
 import sys
 from pathlib import Path
 import logging
-from typing import Optional
+import subprocess
+from typing import Optional, Tuple, List
 
 from clang_tools.util import Version
 from clang_tools.install import is_installed as _is_installed, install_tool
@@ -13,6 +14,80 @@
 DEFAULT_CLANG_VERSION = "18"  # Default version for clang tools, can be overridden
 
 
+def debug_print(message: str, verbose: bool = False) -> None:
+    """Print debug message to stderr if verbose mode is enabled."""
+    if verbose:
+        print(f"[DEBUG] {message}", file=sys.stderr)
+
+
+def run_subprocess_with_logging(
+    command: List[str], tool_name: str, verbose: bool = False, dry_run: bool = False
+) -> Tuple[int, str]:
+    """
+    Run subprocess with comprehensive logging and error handling.
+
+    Args:
+        command: Command list to execute
+        tool_name: Name of the tool (for logging)
+        verbose: Enable verbose debug output
+        dry_run: Whether this is a dry run (affects return code for clang-format)
+
+    Returns:
+        Tuple of (return_code, combined_output)
+    """
+    debug_print(f"{tool_name} command: {' '.join(command)}", verbose)
+
+    retval = 0
+    output = ""
+    stderr_output = ""
+
+    try:
+        sp = subprocess.run(
+            command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8"
+        )
+
+        output = sp.stdout
+        stderr_output = sp.stderr
+        retval = sp.returncode
+
+        if dry_run and tool_name == "clang-format":
+            debug_print("Dry-run mode detected", verbose)
+            debug_print(f"Exit code: {retval} (converted to -1 for dry-run)", verbose)
+            retval = -1  # Special handling for clang-format dry-run
+        else:
+            debug_print(f"Exit code: {retval}", verbose)
+
+        # Log outputs if verbose
+        if verbose and (output or stderr_output):
+            debug_print(f"stdout: {repr(output)}", verbose)
+            debug_print(f"stderr: {repr(stderr_output)}", verbose)
+
+        # Combine stdout and stderr for comprehensive output
+        combined_output = ""
+        if output:
+            combined_output += output
+        if stderr_output:
+            if combined_output:
+                combined_output += "\n"
+            combined_output += stderr_output
+
+        # Special handling for clang-tidy warnings/errors
+        if tool_name == "clang-tidy" and (
+            "warning:" in combined_output or "error:" in combined_output
+        ):
+            if retval == 0:  # Only override if it was successful
+                retval = 1
+                debug_print("Found warnings/errors, setting exit code to 1", verbose)
+
+        return retval, combined_output
+
+    except FileNotFoundError as error:
+        error_msg = f"{tool_name} executable not found: {error}"
+        debug_print(f"FileNotFoundError: {error}", verbose)
+        debug_print(f"Command attempted: {' '.join(command)}", verbose)
+        return 1, error_msg
+
+
 def is_installed(tool_name: str, version: str) -> Optional[Path]:
     """Check if tool is installed.
 
diff --git a/tests/test_clang_format.py b/tests/test_clang_format.py
@@ -90,8 +90,10 @@ def test_verbose_output(tmp_path, capsys):
 
     with patch("cpp_linter_hooks.clang_format.ensure_installed") as mock_ensure:
         mock_ensure.return_value = "/fake/clang-format"
-        with patch("subprocess.run") as mock_run:
-            mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="")
+        with patch(
+            "cpp_linter_hooks.clang_format.run_subprocess_with_logging"
+        ) as mock_run:
+            mock_run.return_value = (0, "")
 
             # Test verbose mode
             retval, output = run_clang_format(
@@ -100,10 +102,14 @@ def test_verbose_output(tmp_path, capsys):
 
             # Check that debug messages were printed to stderr
             captured = capsys.readouterr()
-            assert "[DEBUG] clang-format command:" in captured.err
             assert "[DEBUG] clang-format version:" in captured.err
             assert "[DEBUG] clang-format executable:" in captured.err
-            assert "[DEBUG] Exit code:" in captured.err
+
+            # Verify that run_subprocess_with_logging was called correctly
+            mock_run.assert_called_once()
+            call_args = mock_run.call_args
+            assert call_args[1]["tool_name"] == "clang-format"
+            assert call_args[1]["verbose"] == True
 
 
 def test_verbose_with_error(tmp_path, capsys):
@@ -112,10 +118,10 @@ def test_verbose_with_error(tmp_path, capsys):
 
     with patch("cpp_linter_hooks.clang_format.ensure_installed") as mock_ensure:
         mock_ensure.return_value = "/fake/clang-format"
-        with patch("subprocess.run") as mock_run:
-            mock_run.return_value = MagicMock(
-                returncode=1, stdout="error output", stderr="error in stderr"
-            )
+        with patch(
+            "cpp_linter_hooks.clang_format.run_subprocess_with_logging"
+        ) as mock_run:
+            mock_run.return_value = (1, "error output\nerror in stderr")
 
             # Test verbose mode with error
             retval, output = run_clang_format(
@@ -127,11 +133,11 @@ def test_verbose_with_error(tmp_path, capsys):
             assert "error output" in output
             assert "error in stderr" in output
 
-            # Check debug output
-            captured = capsys.readouterr()
-            assert "[DEBUG] Exit code: 1" in captured.err
-            assert "[DEBUG] stdout:" in captured.err
-            assert "[DEBUG] stderr:" in captured.err
+            # Verify that run_subprocess_with_logging was called correctly
+            mock_run.assert_called_once()
+            call_args = mock_run.call_args
+            assert call_args[1]["tool_name"] == "clang-format"
+            assert call_args[1]["verbose"] == True
 
 
 def test_verbose_dry_run(tmp_path, capsys):
@@ -141,10 +147,10 @@ def test_verbose_dry_run(tmp_path, capsys):
 
     with patch("cpp_linter_hooks.clang_format.ensure_installed") as mock_ensure:
         mock_ensure.return_value = "/fake/clang-format"
-        with patch("subprocess.run") as mock_run:
-            mock_run.return_value = MagicMock(
-                returncode=0, stdout="dry run output", stderr=""
-            )
+        with patch(
+            "cpp_linter_hooks.clang_format.run_subprocess_with_logging"
+        ) as mock_run:
+            mock_run.return_value = (-1, "dry run output")
 
             # Test verbose dry-run mode
             retval, output = run_clang_format(
@@ -155,7 +161,9 @@ def test_verbose_dry_run(tmp_path, capsys):
             assert retval == -1
             assert "dry run output" in output
 
-            # Check debug output
-            captured = capsys.readouterr()
-            assert "[DEBUG] Dry-run mode detected" in captured.err
-            assert "[DEBUG] Exit code: 0 (converted to -1 for dry-run)" in captured.err
+            # Verify that run_subprocess_with_logging was called correctly
+            mock_run.assert_called_once()
+            call_args = mock_run.call_args
+            assert call_args[1]["tool_name"] == "clang-format"
+            assert call_args[1]["verbose"] == True
+            assert call_args[1]["dry_run"] == True
diff --git a/tests/test_clang_tidy.py b/tests/test_clang_tidy.py
