Skip descending into ignored directories in Distignore_Filter_Iterator

[BrianHenryIE]

Edit: I pointed Claude at the issue I wrote and this is what it came up with. It looks good. I tested it against the repo I was working on and everything looks right. I think the existing should exist tests are pretty comprehensive and they are passing.

Claude:

Fixes #134

Root cause

The descent-skipping optimization in Distignore_Filter_Iterator::hasChildren() (#116) never engaged. It probed a synthetic child path ($relative_filepath . '/test') to decide whether an ignored directory's children are also ignored — but that path never exists on disk, and gitignore-checker validates existence (RelativePath::setPath() throws InvalidArgumentException for nonexistent paths). The catch block then fell through to "descend to be safe", so every ignored directory was fully traversed and every file inside was individually checked against the ignore rules.

The existing unit test passed despite this because it only asserted that ignored children aren't yielded — accept() filters them one by one — it never asserted that descent didn't happen.

Fix

hasChildren() now skips descent into ignored directories, with one guard: a .distignore negation rule (leading !) might re-include a path inside an ignored directory, so might_contain_negated_path() checks for that first.

The guard is needed because the checker library reports /frontend itself as ignored under a frontend/* rule (unlike git), so unconditionally skipping descent would break the supported frontend/* + !/frontend/build/ pattern (covered by existing unit and Behat tests).

• No negation rules (the common case): descent into ignored directories is always skipped.
• Anchored negation patterns without wildcards (e.g. !/frontend/build/): compared by path prefix, so only the affected parent directory is descended into.
• Unanchored or wildcard negation patterns: conservatively force descent everywhere.

The previous top-level-only (count( $path_parts ) === 1) restriction is removed, so nested ignored directories (e.g. a deeper node_modules) are skipped too.

Per the issue's request for logs when exceptions are caught, debug messages (--debug=dist-archive) are emitted when descent is skipped and when a checker exception is swallowed.

Performance

Archiving bh-wp-bitcoin-gateway (2 GB node_modules): 8m32s → ~10s, identical 3.03 MB archive.

Tests

• New unit test test_does_not_descend_into_ignored_directories uses a recording subclass that captures every path checked against the ignore rules, proving the contents of ignored directories (top-level and nested) are never visited.
• The Behat performance scenario from Performance Issue: RecursiveIteratorIterator scans all files in ignored directories before filtering #115 now also runs with --debug=dist-archive and asserts the skip message appears and no node_modules contents are mentioned.
• Existing negation tests (frontend/* + !/frontend/build/) pass unchanged.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved .distignore handling for negation rules, preserving files that are explicitly re-included.
  • Ignored directories are now skipped without unnecessarily scanning their contents.
  • Added safeguards for nested ignored directories and more efficient directory traversal.

• Tests

  • Added coverage confirming ignored paths are checked correctly and excluded directory contents are not traversed.

[coderabbitai]

Warning

Review limit reached

@swissspidy, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a2cc3e5c-a5ee-4a55-960c-2e2077fe4c97

📥 Commits

Reviewing files that changed from the base of the PR and between eb50ef2 and 50387a1.

📒 Files selected for processing (1)

• src/Distignore_Filter_Iterator.php

📝 Walkthrough

Walkthrough

Distignore_Filter_Iterator now caches .distignore negation rules and skips ignored directory contents unless a descendant may be re-included. Tests verify that traversal does not check paths inside ignored node_modules directories.

Changes

Distignore traversal correction

Layer / File(s)	Summary
Negation-aware directory traversal src/Distignore_Filter_Iterator.php	The iterator caches negation rules, replaces the synthetic-child probe, and descends into ignored directories only when a rule may match a descendant. Child iterators share the cache.
Skipped-path regression coverage tests/Recording_Distignore_Filter_Iterator.php, tests/Distignore_Filter_Iterator_Test.php, features/distignore.feature	Tests record checked paths and verify that ignored directories are checked without checking paths inside them. The debug scenario validates skipped node_modules contents.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: swissspidy

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address issue #134 by skipping ignored directories, preserving negation support, adding exception logging, and testing traversal behavior.
Out of Scope Changes check	✅ Passed	All changes support issue #134 and the stated objectives for traversal, negation handling, logging, performance, and tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing traversal into ignored directories in Distignore_Filter_Iterator.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/134-filter-iterator-descent

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/Distignore_Filter_Iterator.php	90.90%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!