[clang-tidy] Include sdkconfig.defaults in hash calculation (#11091)

.clang-tidy.hash

@@ -1 +1 @@
-499db61c1aa55b98b6629df603a56a1ba7aff5a9a7c781a5c1552a9dcd186c08
+ab49c22900dd39c004623e450a1076b111d6741f31967a637ab6e0e3dd2e753e

.github/workflows/ci-clang-tidy-hash.yml

@@ -6,6 +6,7 @@ on:
       - ".clang-tidy"
       - "platformio.ini"
       - "requirements_dev.txt"
+      - "sdkconfig.defaults"
       - ".clang-tidy.hash"
       - "script/clang_tidy_hash.py"
       - ".github/workflows/ci-clang-tidy-hash.yml"

script/clang_tidy_hash.py

@@ -48,9 +48,10 @@ def parse_requirement_line(line: str) -> tuple[str, str] | None:
     return None
 
 
-def get_clang_tidy_version_from_requirements() -> str:
+def get_clang_tidy_version_from_requirements(repo_root: Path | None = None) -> str:
     """Get clang-tidy version from requirements_dev.txt"""
-    requirements_path = Path(__file__).parent.parent / "requirements_dev.txt"
+    repo_root = _ensure_repo_root(repo_root)
+    requirements_path = repo_root / "requirements_dev.txt"
     lines = read_file_lines(requirements_path)
 
     for line in lines:
@@ -68,30 +69,49 @@ def read_file_bytes(path: Path) -> bytes:
         return f.read()
 
 
-def calculate_clang_tidy_hash() -> str:
+def get_repo_root() -> Path:
+    """Get the repository root directory."""
+    return Path(__file__).parent.parent
+
+
+def _ensure_repo_root(repo_root: Path | None) -> Path:
+    """Ensure repo_root is a Path, using default if None."""
+    return repo_root if repo_root is not None else get_repo_root()
+
+
+def calculate_clang_tidy_hash(repo_root: Path | None = None) -> str:
     """Calculate hash of clang-tidy configuration and version"""
+    repo_root = _ensure_repo_root(repo_root)
+
     hasher = hashlib.sha256()
 
     # Hash .clang-tidy file
-    clang_tidy_path = Path(__file__).parent.parent / ".clang-tidy"
+    clang_tidy_path = repo_root / ".clang-tidy"
     content = read_file_bytes(clang_tidy_path)
     hasher.update(content)
 
     # Hash clang-tidy version from requirements_dev.txt
-    version = get_clang_tidy_version_from_requirements()
+    version = get_clang_tidy_version_from_requirements(repo_root)
     hasher.update(version.encode())
 
     # Hash the entire platformio.ini file
-    platformio_path = Path(__file__).parent.parent / "platformio.ini"
+    platformio_path = repo_root / "platformio.ini"
     platformio_content = read_file_bytes(platformio_path)
     hasher.update(platformio_content)
 
+    # Hash sdkconfig.defaults file
+    sdkconfig_path = repo_root / "sdkconfig.defaults"
+    if sdkconfig_path.exists():
+        sdkconfig_content = read_file_bytes(sdkconfig_path)
+        hasher.update(sdkconfig_content)
+
     return hasher.hexdigest()
 
 
-def read_stored_hash() -> str | None:
+def read_stored_hash(repo_root: Path | None = None) -> str | None:
     """Read the stored hash from file"""
-    hash_file = Path(__file__).parent.parent / ".clang-tidy.hash"
+    repo_root = _ensure_repo_root(repo_root)
+    hash_file = repo_root / ".clang-tidy.hash"
     if hash_file.exists():
         lines = read_file_lines(hash_file)
         return lines[0].strip() if lines else None
@@ -104,9 +124,10 @@ def write_file_content(path: Path, content: str) -> None:
         f.write(content)
 
 
-def write_hash(hash_value: str) -> None:
+def write_hash(hash_value: str, repo_root: Path | None = None) -> None:
     """Write hash to file"""
-    hash_file = Path(__file__).parent.parent / ".clang-tidy.hash"
+    repo_root = _ensure_repo_root(repo_root)
+    hash_file = repo_root / ".clang-tidy.hash"
     # Strip any trailing newlines to ensure consistent formatting
     write_file_content(hash_file, hash_value.strip() + "\n")
 
@@ -134,8 +155,28 @@ def main() -> None:
     stored_hash = read_stored_hash()
 
     if args.check:
-        # Exit 0 if full scan needed (hash changed or no hash file)
-        sys.exit(0 if current_hash != stored_hash else 1)
+        # Check if hash changed OR if .clang-tidy.hash was updated in this PR
+        # This is used in CI to determine if a full clang-tidy scan is needed
+        hash_changed = current_hash != stored_hash
+
+        # Lazy import to avoid requiring dependencies that aren't needed for other modes
+        from helpers import changed_files  # noqa: E402
+
+        hash_file_updated = ".clang-tidy.hash" in changed_files()
+
+        # Exit 0 if full scan needed
+        sys.exit(0 if (hash_changed or hash_file_updated) else 1)
+
+    elif args.verify:
+        # Verify that hash file is up to date with current configuration
+        # This is used in pre-commit and CI checks to ensure hash was updated
+        if current_hash != stored_hash:
+            print("ERROR: Clang-tidy configuration has changed but hash not updated!")
+            print(f"Expected: {current_hash}")
+            print(f"Found: {stored_hash}")
+            print("\nPlease run: script/clang_tidy_hash.py --update")
+            sys.exit(1)
+        print("Hash verification passed")
 
     elif args.update:
         write_hash(current_hash)
@@ -151,15 +192,6 @@ def main() -> None:
             print("Clang-tidy hash unchanged")
             sys.exit(0)
 
-    elif args.verify:
-        if current_hash != stored_hash:
-            print("ERROR: Clang-tidy configuration has changed but hash not updated!")
-            print(f"Expected: {current_hash}")
-            print(f"Found: {stored_hash}")
-            print("\nPlease run: script/clang_tidy_hash.py --update")
-            sys.exit(1)
-        print("Hash verification passed")
-
     else:
         print(f"Current hash: {current_hash}")
         print(f"Stored hash: {stored_hash}")

tests/script/test_clang_tidy_hash.py

@@ -44,37 +44,53 @@ def test_get_clang_tidy_version_from_requirements(
     assert result == expected
 
 
-def test_calculate_clang_tidy_hash() -> None:
-    """Test calculating hash from all configuration sources."""
+def test_calculate_clang_tidy_hash_with_sdkconfig(tmp_path: Path) -> None:
+    """Test calculating hash from all configuration sources including sdkconfig.defaults."""
     clang_tidy_content = b"Checks: '-*,readability-*'\n"
     requirements_version = "clang-tidy==18.1.5"
     platformio_content = b"[env:esp32]\nplatform = espressif32\n"
+    sdkconfig_content = b"CONFIG_AUTOSTART_ARDUINO=y\n"
+    requirements_content = "clang-tidy==18.1.5\n"
+
+    # Create temporary files
+    (tmp_path / ".clang-tidy").write_bytes(clang_tidy_content)
+    (tmp_path / "platformio.ini").write_bytes(platformio_content)
+    (tmp_path / "sdkconfig.defaults").write_bytes(sdkconfig_content)
+    (tmp_path / "requirements_dev.txt").write_text(requirements_content)
 
     # Expected hash calculation
     expected_hasher = hashlib.sha256()
     expected_hasher.update(clang_tidy_content)
     expected_hasher.update(requirements_version.encode())
     expected_hasher.update(platformio_content)
+    expected_hasher.update(sdkconfig_content)
     expected_hash = expected_hasher.hexdigest()
 
-    # Mock the dependencies
-    with (
-        patch("clang_tidy_hash.read_file_bytes") as mock_read_bytes,
-        patch(
-            "clang_tidy_hash.get_clang_tidy_version_from_requirements",
-            return_value=requirements_version,
-        ),
-    ):
-        # Set up mock to return different content based on the file being read
-        def read_file_mock(path: Path) -> bytes:
-            if ".clang-tidy" in str(path):
-                return clang_tidy_content
-            if "platformio.ini" in str(path):
-                return platformio_content
-            return b""
+    result = clang_tidy_hash.calculate_clang_tidy_hash(repo_root=tmp_path)
 
-        mock_read_bytes.side_effect = read_file_mock
-        result = clang_tidy_hash.calculate_clang_tidy_hash()
+    assert result == expected_hash
+
+
+def test_calculate_clang_tidy_hash_without_sdkconfig(tmp_path: Path) -> None:
+    """Test calculating hash without sdkconfig.defaults file."""
+    clang_tidy_content = b"Checks: '-*,readability-*'\n"
+    requirements_version = "clang-tidy==18.1.5"
+    platformio_content = b"[env:esp32]\nplatform = espressif32\n"
+    requirements_content = "clang-tidy==18.1.5\n"
+
+    # Create temporary files (without sdkconfig.defaults)
+    (tmp_path / ".clang-tidy").write_bytes(clang_tidy_content)
+    (tmp_path / "platformio.ini").write_bytes(platformio_content)
+    (tmp_path / "requirements_dev.txt").write_text(requirements_content)
+
+    # Expected hash calculation (no sdkconfig)
+    expected_hasher = hashlib.sha256()
+    expected_hasher.update(clang_tidy_content)
+    expected_hasher.update(requirements_version.encode())
+    expected_hasher.update(platformio_content)
+    expected_hash = expected_hasher.hexdigest()
+
+    result = clang_tidy_hash.calculate_clang_tidy_hash(repo_root=tmp_path)
 
     assert result == expected_hash
 
@@ -85,67 +101,63 @@ def test_read_stored_hash_exists(tmp_path: Path) -> None:
     hash_file = tmp_path / ".clang-tidy.hash"
     hash_file.write_text(f"{stored_hash}\n")
 
-    with (
-        patch("clang_tidy_hash.Path") as mock_path_class,
-        patch("clang_tidy_hash.read_file_lines", return_value=[f"{stored_hash}\n"]),
-    ):
-        # Mock the path calculation and exists check
-        mock_hash_file = Mock()
-        mock_hash_file.exists.return_value = True
-        mock_path_class.return_value.parent.parent.__truediv__.return_value = (
-            mock_hash_file
-        )
-
-        result = clang_tidy_hash.read_stored_hash()
+    result = clang_tidy_hash.read_stored_hash(repo_root=tmp_path)
 
     assert result == stored_hash
 
 
-def test_read_stored_hash_not_exists() -> None:
+def test_read_stored_hash_not_exists(tmp_path: Path) -> None:
     """Test reading hash when file doesn't exist."""
-    with patch("clang_tidy_hash.Path") as mock_path_class:
-        # Mock the path calculation and exists check
-        mock_hash_file = Mock()
-        mock_hash_file.exists.return_value = False
-        mock_path_class.return_value.parent.parent.__truediv__.return_value = (
-            mock_hash_file
-        )
-
-        result = clang_tidy_hash.read_stored_hash()
+    result = clang_tidy_hash.read_stored_hash(repo_root=tmp_path)
 
     assert result is None
 
 
-def test_write_hash() -> None:
+def test_write_hash(tmp_path: Path) -> None:
     """Test writing hash to file."""
     hash_value = "abc123def456"
+    hash_file = tmp_path / ".clang-tidy.hash"
 
-    with patch("clang_tidy_hash.write_file_content") as mock_write:
-        clang_tidy_hash.write_hash(hash_value)
+    clang_tidy_hash.write_hash(hash_value, repo_root=tmp_path)
 
-        # Verify write_file_content was called with correct parameters
-        mock_write.assert_called_once()
-        args = mock_write.call_args[0]
-        assert str(args[0]).endswith(".clang-tidy.hash")
-        assert args[1] == hash_value.strip() + "\n"
+    assert hash_file.exists()
+    assert hash_file.read_text() == hash_value.strip() + "\n"
 
 
 @pytest.mark.parametrize(
-    ("args", "current_hash", "stored_hash", "expected_exit"),
+    ("args", "current_hash", "stored_hash", "hash_file_in_changed", "expected_exit"),
     [
-        (["--check"], "abc123", "abc123", 1),  # Hashes match, no scan needed
-        (["--check"], "abc123", "def456", 0),  # Hashes differ, scan needed
-        (["--check"], "abc123", None, 0),  # No stored hash, scan needed
+        (["--check"], "abc123", "abc123", False, 1),  # Hashes match, no scan needed
+        (["--check"], "abc123", "def456", False, 0),  # Hashes differ, scan needed
+        (["--check"], "abc123", None, False, 0),  # No stored hash, scan needed
+        (
+            ["--check"],
+            "abc123",
+            "abc123",
+            True,
+            0,
+        ),  # Hash file updated in PR, scan needed
     ],
 )
 def test_main_check_mode(
-    args: list[str], current_hash: str, stored_hash: str | None, expected_exit: int
+    args: list[str],
+    current_hash: str,
+    stored_hash: str | None,
+    hash_file_in_changed: bool,
+    expected_exit: int,
 ) -> None:
     """Test main function in check mode."""
+    changed = [".clang-tidy.hash"] if hash_file_in_changed else []
+
+    # Create a mock module that can be imported
+    mock_helpers = Mock()
+    mock_helpers.changed_files = Mock(return_value=changed)
+
     with (
         patch("sys.argv", ["clang_tidy_hash.py"] + args),
         patch("clang_tidy_hash.calculate_clang_tidy_hash", return_value=current_hash),
         patch("clang_tidy_hash.read_stored_hash", return_value=stored_hash),
+        patch.dict("sys.modules", {"helpers": mock_helpers}),
         pytest.raises(SystemExit) as exc_info,
     ):
         clang_tidy_hash.main()