Improve clang hook error diagnostics

shenxianpeng · shenxianpeng · commit fbc6270bbd38 · 2026-05-21T05:50:35.000+03:00
diff --git a/cpp_linter_hooks/clang_format.py b/cpp_linter_hooks/clang_format.py
@@ -3,20 +3,23 @@
 from argparse import ArgumentParser
 from typing import Tuple
 
-from cpp_linter_hooks.util import resolve_install, DEFAULT_CLANG_FORMAT_VERSION
+from cpp_linter_hooks.util import resolve_install_with_diagnostics
 
 
 parser = ArgumentParser()
-parser.add_argument("--version", default=DEFAULT_CLANG_FORMAT_VERSION)
+parser.add_argument("--version", default=None)
 parser.add_argument(
     "-v", "--verbose", action="store_true", help="Enable verbose output"
 )
 
 
 def run_clang_format(args=None) -> Tuple[int, str]:
     hook_args, other_args = parser.parse_known_args(args)
-    if hook_args.version:
-        resolve_install("clang-format", hook_args.version)
+    _, version_error = resolve_install_with_diagnostics(
+        "clang-format", hook_args.version, hook_args.verbose
+    )
+    if version_error is not None:
+        return 1, version_error
     command = ["clang-format", "-i"]
 
     # Add verbose flag if requested
diff --git a/cpp_linter_hooks/clang_tidy.py b/cpp_linter_hooks/clang_tidy.py
@@ -5,7 +5,7 @@
 from pathlib import Path
 from typing import List, Optional, Tuple
 
-from cpp_linter_hooks.util import resolve_install, DEFAULT_CLANG_TIDY_VERSION
+from cpp_linter_hooks.util import resolve_install_with_diagnostics
 
 COMPILE_DB_SEARCH_DIRS = ["build", "out", "cmake-build-debug", "_build"]
 SOURCE_FILE_SUFFIXES = {
@@ -28,6 +28,18 @@
     ".tpp",
     ".txx",
 }
+COMPILE_COMMANDS_HINT = """\
+Generate compile_commands.json with one of:
+  CMake: cmake -S . -B build -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
+  Meson: meson setup builddir
+Then run clang-tidy with --compile-commands=build or --compile-commands=builddir."""
+
+MSVC_HINT = """\
+Windows/MSVC clang-tidy hints:
+  - Run from a Visual Studio Developer Command Prompt, or call vcvars64.bat first.
+  - Make sure the Windows SDK and MSVC include paths are visible in that shell.
+  - For MSVC-style compile databases, try --extra-arg-before=--driver-mode=cl.
+  - If using CMake, generate compile_commands.json from the same toolchain."""
 
 
 def _positive_int(value: str) -> int:
@@ -38,7 +50,7 @@ def _positive_int(value: str) -> int:
 
 
 parser = ArgumentParser()
-parser.add_argument("--version", default=DEFAULT_CLANG_TIDY_VERSION)
+parser.add_argument("--version", default=None)
 parser.add_argument("--compile-commands", default=None, dest="compile_commands")
 parser.add_argument(
     "--no-compile-commands", action="store_true", dest="no_compile_commands"
@@ -55,6 +67,17 @@ def _find_compile_commands() -> Optional[str]:
     return None
 
 
+def _compile_commands_not_found_message(path: Optional[str] = None) -> str:
+    if path is None:
+        return "No compile_commands.json was found in common build directories.\n\n" + (
+            COMPILE_COMMANDS_HINT
+        )
+    return (
+        f"--compile-commands: no compile_commands.json in '{path}'.\n\n"
+        f"{COMPILE_COMMANDS_HINT}"
+    )
+
+
 def _resolve_compile_db(
     hook_args, other_args
 ) -> Tuple[Optional[str], Optional[Tuple[int, str]]]:
@@ -79,8 +102,7 @@ def _resolve_compile_db(
         if not p.is_dir() or not (p / "compile_commands.json").exists():
             return None, (
                 1,
-                f"--compile-commands: no compile_commands.json"
-                f" in '{hook_args.compile_commands}'",
+                _compile_commands_not_found_message(hook_args.compile_commands),
             )
         return hook_args.compile_commands, None
 
@@ -90,13 +112,68 @@ def _resolve_compile_db(
     return None, None
 
 
+def _looks_like_compile_db_error(output: str) -> bool:
+    lower_output = output.lower()
+    compile_db_error = "compile_commands.json" in lower_output and any(
+        pattern in lower_output
+        for pattern in (
+            "not found",
+            "no such file",
+            "missing",
+            "error",
+            "could not",
+        )
+    )
+    return compile_db_error or any(
+        pattern in lower_output
+        for pattern in (
+            "error while trying to load a compilation database",
+            "could not auto-detect compilation database",
+            "no compilation database found",
+        )
+    )
+
+
+def _looks_like_msvc_error(output: str) -> bool:
+    lower_output = output.lower()
+    cl_driver_error = "cl.exe" in lower_output and any(
+        pattern in lower_output
+        for pattern in ("not found", "doesn't exist", "unable to execute")
+    )
+    msvc_patterns = (
+        "unable to find a visual studio installation",
+        "visual studio installation",
+        "vcruntime.h",
+        "windows.h' file not found",
+        "sal.h' file not found",
+        "msvc",
+        "unknown argument: '/",
+        "unsupported option '/",
+        "argument unused during compilation: '/",
+    )
+    return cl_driver_error or any(pattern in lower_output for pattern in msvc_patterns)
+
+
+def _append_guidance(output: str) -> str:
+    hints: List[str] = []
+    if _looks_like_compile_db_error(output) and COMPILE_COMMANDS_HINT not in output:
+        hints.append(COMPILE_COMMANDS_HINT)
+    if _looks_like_msvc_error(output) and MSVC_HINT not in output:
+        hints.append(MSVC_HINT)
+    if not hints:
+        return output
+    separator = "\n\n" if output.rstrip("\n") else ""
+    return output.rstrip("\n") + separator + "\n\n".join(hints)
+
+
 def _exec_clang_tidy(command) -> Tuple[int, str]:
     """Run clang-tidy and return (retval, output)."""
     try:
         sp = subprocess.run(
             command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8"
         )
         output = (sp.stdout or "") + (sp.stderr or "")
+        output = _append_guidance(output)
         retval = (
             1 if sp.returncode != 0 or "warning:" in output or "error:" in output else 0
         )
@@ -139,8 +216,11 @@ def run_file(source_file: str) -> Tuple[int, str]:
 
 def run_clang_tidy(args=None) -> Tuple[int, str]:
     hook_args, other_args = parser.parse_known_args(args)
-    if hook_args.version:
-        resolve_install("clang-tidy", hook_args.version)
+    _, version_error = resolve_install_with_diagnostics(
+        "clang-tidy", hook_args.version, hook_args.verbose
+    )
+    if version_error is not None:
+        return 1, version_error
 
     compile_db_path, error = _resolve_compile_db(hook_args, other_args)
     if error is not None:
@@ -152,6 +232,10 @@ def run_clang_tidy(args=None) -> Tuple[int, str]:
                 f"Using compile_commands.json from: {compile_db_path}", file=sys.stderr
             )
         other_args = ["-p", compile_db_path] + other_args
+    elif hook_args.verbose and not hook_args.no_compile_commands:
+        has_p = any(a == "-p" or a.startswith("-p=") for a in other_args)
+        if not has_p:
+            print(_compile_commands_not_found_message(), file=sys.stderr)
 
     clang_tidy_args, source_files = _split_source_files(other_args)
 
diff --git a/cpp_linter_hooks/util.py b/cpp_linter_hooks/util.py
@@ -3,7 +3,7 @@
 import subprocess
 from pathlib import Path
 import logging
-from typing import Optional, List
+from typing import Optional, List, Tuple
 
 if sys.version_info >= (3, 11):
     import tomllib
@@ -34,6 +34,23 @@ def get_version_from_dependency(tool: str) -> Optional[str]:
 DEFAULT_CLANG_TIDY_VERSION = CLANG_TIDY_VERSIONS[-1]  # latest from versions.py
 
 
+def _versions_for_tool(tool: str) -> List[str]:
+    return CLANG_FORMAT_VERSIONS if tool == "clang-format" else CLANG_TIDY_VERSIONS
+
+
+def _default_version_for_tool(tool: str) -> Optional[str]:
+    return (
+        DEFAULT_CLANG_FORMAT_VERSION
+        if tool == "clang-format"
+        else DEFAULT_CLANG_TIDY_VERSION
+    )
+
+
+def _supported_versions_message(tool: str) -> str:
+    versions = ", ".join(_versions_for_tool(tool))
+    return f"Supported {tool} wheel versions: {versions}"
+
+
 def _resolve_version(versions: List[str], user_input: Optional[str]) -> Optional[str]:
     """Resolve the latest matching version based on user input and available versions."""
     if user_input is None:
@@ -59,6 +76,23 @@ def parse_version(v: str):
         return None
 
 
+def resolve_tool_version(
+    tool: str, version: Optional[str]
+) -> Tuple[Optional[str], Optional[str]]:
+    """Resolve a requested tool version or return a user-facing error message."""
+    if version is None:
+        return _default_version_for_tool(tool), None
+
+    resolved = _resolve_version(_versions_for_tool(tool), version)
+    if resolved is None:
+        return (
+            None,
+            f"Unsupported {tool} version '{version}'.\n"
+            f"{_supported_versions_message(tool)}",
+        )
+    return resolved, None
+
+
 def _is_version_installed(tool: str, version: str) -> Optional[Path]:
     """Return the tool path if the installed version matches, otherwise None."""
     existing = shutil.which(tool)
@@ -85,19 +119,39 @@ def _install_tool(tool: str, version: str) -> Optional[Path]:
     return None
 
 
-def resolve_install(tool: str, version: Optional[str]) -> Optional[Path]:
-    """Resolve the installation of a tool, checking for version and installing if necessary."""
-    user_version = _resolve_version(
-        CLANG_FORMAT_VERSIONS if tool == "clang-format" else CLANG_TIDY_VERSIONS,
-        version,
+def resolve_install_with_diagnostics(
+    tool: str, version: Optional[str], verbose: bool = False
+) -> Tuple[Optional[Path], Optional[str]]:
+    """Resolve/install a tool, returning a user-facing error for bad versions."""
+    user_version, error = resolve_tool_version(tool, version)
+    if error is not None:
+        return None, error
+
+    if verbose:
+        if version is None:
+            print(
+                f"Using default {tool} Python wheel version {user_version}",
+                file=sys.stderr,
+            )
+        elif version == user_version:
+            print(f"Using {tool} Python wheel version {user_version}", file=sys.stderr)
+        else:
+            print(
+                f"Resolved {tool} --version={version} to Python wheel version "
+                f"{user_version}",
+                file=sys.stderr,
+            )
+
+    return (
+        _is_version_installed(tool, user_version)
+        or _install_tool(tool, user_version),
+        None,
     )
-    if user_version is None:
-        user_version = (
-            DEFAULT_CLANG_FORMAT_VERSION
-            if tool == "clang-format"
-            else DEFAULT_CLANG_TIDY_VERSION
-        )
 
-    return _is_version_installed(tool, user_version) or _install_tool(
-        tool, user_version
-    )
+
+def resolve_install(tool: str, version: Optional[str]) -> Optional[Path]:
+    """Resolve/install a tool, logging bad-version diagnostics."""
+    path, error = resolve_install_with_diagnostics(tool, version)
+    if error is not None:
+        LOG.error(error)
+    return path
diff --git a/tests/test_clang_format.py b/tests/test_clang_format.py
@@ -1,5 +1,6 @@
 import pytest
 from pathlib import Path
+from unittest.mock import patch
 
 from cpp_linter_hooks.clang_format import run_clang_format
 
@@ -103,3 +104,35 @@ def test_run_clang_format_verbose_error(tmp_path):
     assert ret != 0
     # Should have error message in output
     assert "Invalid value for -style" in output
+
+
+def test_run_clang_format_invalid_version_returns_supported_versions():
+    with patch(
+        "cpp_linter_hooks.clang_format.resolve_install_with_diagnostics",
+        return_value=(
+            None,
+            "Unsupported clang-format version '99'.\nSupported versions",
+        ),
+    ):
+        ret, output = run_clang_format(["--version=99", "dummy.cpp"])
+
+    assert ret == 1
+    assert "Unsupported clang-format version '99'" in output
+    assert "Supported versions" in output
+
+
+def test_run_clang_format_verbose_passes_version_diagnostics():
+    with (
+        patch(
+            "cpp_linter_hooks.clang_format.resolve_install_with_diagnostics",
+            return_value=(None, None),
+        ) as mock_resolve,
+        patch("cpp_linter_hooks.clang_format.subprocess.run") as mock_run,
+    ):
+        mock_run.return_value.returncode = 0
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = ""
+        ret, output = run_clang_format(["--verbose", "--version=21", "dummy.cpp"])
+
+    assert (ret, output) == (0, "")
+    mock_resolve.assert_called_once_with("clang-format", "21", True)
diff --git a/tests/test_clang_tidy.py b/tests/test_clang_tidy.py
diff --git a/tests/test_util.py b/tests/test_util.py
