diff --git a/code_review_graph/incremental.py b/code_review_graph/incremental.py index af72d29a..c4169f68 100644 --- a/code_review_graph/incremental.py +++ b/code_review_graph/incremental.py @@ -13,7 +13,7 @@ import re import subprocess import time -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import Optional from .graph import GraphStore @@ -22,24 +22,58 @@ logger = logging.getLogger(__name__) # Default ignore patterns (in addition to .gitignore) +# +# Two pattern styles: +# 1. `**/name/**` — matches `name` as any path segment (safe-anywhere). +# Use only for directories that are NEVER legitimate source code names +# (node_modules, __pycache__, .venv, vendor, .gradle, .dart_tool, etc.). +# 2. `name/**` — matches only at repo root. +# Use for directories that MAY be valid source names in some projects +# (packages/, bin/, build/, dist/, storage/, obj/). DEFAULT_IGNORE_PATTERNS = [ + # Tool-owned (always safe anywhere) ".code-review-graph/**", - "node_modules/**", ".git/**", - "__pycache__/**", - "*.pyc", - ".venv/**", - "venv/**", + # Dependency directories — never source code, safe to match anywhere + "**/node_modules/**", + "**/__pycache__/**", + "**/.venv/**", + "**/venv/**", + "**/vendor/**", # Composer (PHP), Go modules, Ruby/Rails + "**/.bundle/**", # Ruby Bundler + "**/.gradle/**", # Gradle cache + "**/.dart_tool/**", # Dart/Flutter + "**/.pub-cache/**", # Dart/Flutter + "**/.cache/**", + # Framework build/output dirs — at repo root only to avoid matching + # legit source dirs (e.g. src/build/, packages/, etc.) + ".next/**", + ".nuxt/**", "dist/**", "build/**", - ".next/**", - "target/**", + "target/**", # Rust (also used by Maven/SBT) + "bin/**", # .NET (root only; src/bin/ not ignored) + "obj/**", # .NET + # NOTE: `packages/**` (.NET NuGet) is NOT included because it conflicts + # with npm/Lerna/Turborepo monorepos where packages/ holds source code. + # .NET users should add it via .code-review-graphignore. + "storage/**", # Laravel + "bootstrap/cache/**", # Laravel + "public/build/**", # Laravel Mix/Vite output + "coverage/**", + ".tmp/**", + "tmp/**", + # Python cache file + "*.pyc", + # Minified / generated single files "*.min.js", "*.min.css", "*.map", "*.lock", "package-lock.json", "yarn.lock", + "*.jar", # Java compiled + # Database files "*.db", "*.sqlite", "*.db-journal", @@ -116,8 +150,37 @@ def _load_ignore_patterns(repo_root: Path) -> list[str]: def _should_ignore(path: str, patterns: list[str]) -> bool: - """Check if a path matches any ignore pattern.""" - return any(fnmatch.fnmatch(path, p) for p in patterns) + """Check if a path matches any ignore pattern. + + Pattern semantics: + - ``**/name/**`` — matches ``name`` as any path segment (safe-anywhere). + Use for dirs that are never valid source code (node_modules, vendor). + - ``name/**`` — matches only at the repo root (first segment is ``name``). + Use for dirs that may be valid source names in some projects (packages, + bin, build). + - ``*.ext`` and other non-``**`` patterns fall back to ``fnmatch``. + """ + parts = PurePosixPath(path).parts + for p in patterns: + # Safe-anywhere: **/name/** + if p.startswith("**/") and p.endswith("/**"): + segment = p[3:-3] + if segment in parts: + return True + # Root-relative: name/** — matches only if first segment is `name` + elif p.endswith("/**"): + prefix = p[:-3] + # Support multi-segment prefixes like "bootstrap/cache" + prefix_parts = prefix.split("/") + if ( + len(parts) >= len(prefix_parts) + and parts[: len(prefix_parts)] == tuple(prefix_parts) + ): + return True + # Plain glob (e.g. *.pyc, *.min.js) + elif fnmatch.fnmatch(path, p): + return True + return False def _is_binary(path: Path) -> bool: diff --git a/tests/test_incremental.py b/tests/test_incremental.py index ec3184fa..4addb5de 100644 --- a/tests/test_incremental.py +++ b/tests/test_incremental.py @@ -80,9 +80,9 @@ def test_cleans_legacy_side_files(self, tmp_path): class TestIgnorePatterns: def test_default_patterns_loaded(self, tmp_path): patterns = _load_ignore_patterns(tmp_path) - assert "node_modules/**" in patterns + assert "**/node_modules/**" in patterns assert ".git/**" in patterns - assert "__pycache__/**" in patterns + assert "**/__pycache__/**" in patterns def test_custom_ignore_file(self, tmp_path): ignore = tmp_path / ".code-review-graphignore" @@ -95,12 +95,69 @@ def test_custom_ignore_file(self, tmp_path): assert "" not in patterns def test_should_ignore_matches(self): - patterns = ["node_modules/**", "*.pyc", ".git/**"] + patterns = ["**/node_modules/**", "*.pyc", ".git/**"] assert _should_ignore("node_modules/foo/bar.js", patterns) assert _should_ignore("test.pyc", patterns) assert _should_ignore(".git/HEAD", patterns) assert not _should_ignore("src/main.py", patterns) + def test_safe_anywhere_matches_nested_paths(self): + """**/name/** patterns match nested dependency dirs (monorepos).""" + patterns = ["**/node_modules/**", "**/vendor/**", "**/__pycache__/**"] + # Nested node_modules (npm workspaces, Lerna, Turborepo) + assert _should_ignore("packages/app/node_modules/react/index.js", patterns) + # Nested vendor (PHP monorepo) + assert _should_ignore("services/api/vendor/guzzlehttp/Client.php", patterns) + # Nested __pycache__ + assert _should_ignore("src/utils/__pycache__/helpers.cpython-311.pyc", patterns) + # Actual source code must not be affected + assert not _should_ignore("packages/app/src/main.ts", patterns) + assert not _should_ignore("src/vendors/custom.php", patterns) + + def test_root_relative_patterns_dont_match_nested(self): + """name/** patterns should only match at root, not nested `name/` dirs.""" + patterns = ["packages/**", "bin/**", "build/**"] + # Root-level matches + assert _should_ignore("packages/nuget-cache/lib.dll", patterns) + assert _should_ignore("bin/Debug/net8.0/app.dll", patterns) + assert _should_ignore("build/output.txt", patterns) + # Nested `packages/` in monorepo must NOT match + assert not _should_ignore("apps/web/packages/src/main.ts", patterns) + assert not _should_ignore("services/api/bin/helper.sh", patterns) + assert not _should_ignore("docs/build/page.md", patterns) + + def test_multi_segment_root_prefix(self): + """Multi-segment patterns like `bootstrap/cache/**` match only at root.""" + patterns = ["bootstrap/cache/**"] + assert _should_ignore("bootstrap/cache/packages.php", patterns) + assert not _should_ignore("src/bootstrap/cache/file.php", patterns) + + def test_should_ignore_framework_patterns(self): + """Framework-specific dirs from DEFAULT_IGNORE_PATTERNS.""" + from code_review_graph.incremental import DEFAULT_IGNORE_PATTERNS + + patterns = DEFAULT_IGNORE_PATTERNS + # Safe-anywhere: dependency dirs never used as source + assert _should_ignore("vendor/laravel/framework/src/Collection.php", patterns) + assert _should_ignore("services/api/vendor/pkg/file.go", patterns) # nested + assert _should_ignore(".dart_tool/package_config.json", patterns) + assert _should_ignore(".gradle/caches/transforms-3/file.jar", patterns) + assert _should_ignore("node_modules/react/index.js", patterns) + assert _should_ignore("apps/api/node_modules/foo.js", patterns) # nested + # Root-only: Laravel framework dirs + assert _should_ignore("storage/logs/laravel.log", patterns) + assert _should_ignore("bootstrap/cache/packages.php", patterns) + assert _should_ignore("public/build/assets/app.js", patterns) + # Root-only: .NET + assert _should_ignore("bin/Debug/net8.0/app.dll", patterns) + assert _should_ignore("obj/Release/app.assets.cache", patterns) + # Monorepo-safe: `packages/` is NOT in defaults (npm/Lerna/Turborepo + # use it for source code; .NET users must add it to .code-review-graphignore) + assert not _should_ignore("packages/app/src/main.ts", patterns) + assert not _should_ignore("packages/Newtonsoft.Json.13.0.1/lib.dll", patterns) + assert not _should_ignore("apps/web/src/main.tsx", patterns) + assert not _should_ignore("app/Http/Controllers/UserController.php", patterns) + class TestIsBinary: def test_text_file_is_not_binary(self, tmp_path):