Use sebastian/file-filter in SourceFilter::includes() for issue trigger identification#6535
Draft
sebastianbergmann wants to merge 5 commits intomainfrom
Draft
Use sebastian/file-filter in SourceFilter::includes() for issue trigger identification#6535sebastianbergmann wants to merge 5 commits intomainfrom
sebastian/file-filter in SourceFilter::includes() for issue trigger identification#6535sebastianbergmann wants to merge 5 commits intomainfrom
Conversation
SourceFilter::includes() and SourceMapper::map() with sebastian/file-filter for issue trigger identificationsebastian/file-filter in SourceFilter::includes() for issue trigger identification
3edfe71 to
0f585a7
Compare
This was referenced Mar 8, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6535 +/- ##
=========================================
Coverage 95.95% 95.96%
- Complexity 7513 7520 +7
=========================================
Files 815 816 +1
Lines 23202 23225 +23
=========================================
+ Hits 22264 22287 +23
Misses 938 938 ☔ View full report in Codecov by Sentry. |
0f585a7 to
ae5c6d3
Compare
dantleech
reviewed
Mar 8, 2026
| public function map(Source $source): FileFilter | ||
| { | ||
| return (new FilterBuilder)->build( | ||
| $this->directories($source->includeDirectories()), |
Contributor
There was a problem hiding this comment.
it seems the include directories are not "normalized" - when the config is in a sub-drectory (e.g. config/phpunit.xml):
<source ignoreSelfDeprecations="true">
<include>
<directory suffix=".php">../src/</directory>
</include>
</source>
Results in the regex:
#^/home/daniel/www/dantleech/phpunit\-6111/config/\.\./src/(?:/|$)#
But the path needs to be normalized from phpunit-6111/config/../src tophpunit-6111/src in order for it to match the source paths.
(see Symfony's Path component for an example)
ae5c6d3 to
3584d21
Compare
When phpunit.xml is in a subdirectory and uses relative paths with ".." (e.g. ../src/), the path was passed to the file filter without canonicalization, causing the generated regex to include literal ".." segments that never match resolved file paths. Apply realpath() in FileFilterMapper to normalize directory and file paths before building the filter.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
FileFilterMapper::map()maps aPHPUnit\TextUI\Configuration\Sourceobject to aSebastianBergmann\FileFilter\Filterobject.SebastianBergmann\FileFilter\Filter\SourceFilternow uses thisSebastianBergmann\FileFilter\Filterobject.Backward Compatibility
The changes proposed here mostly preserve backward compatibility. The following sections describe backward compatibility breaks in detail.
SourceFilterpass, including the additional tests that were implemented onmainwhile working on these changesI think the backward compatibility breaks discussed below are acceptable, even in a minor version such as PHPUnit 13.x, considering the performance improvement.
The only way to improve performance of issue trigger identification I see is implemented by these changes. We could keep the old implementation, introduce the new implementation as an alternative that can be opted-in using the XML configuration file. In the future, we could then deprecate the old implementation and later remove it. However, I want to avoid having to maintain two implementations.
Symlinks
The old implementation resolved symlinks and relative paths via
realpath(). The new implementation does string-based matching only.Hidden directories
The old implementation excluded hidden directories as a side effect of
SebastianBergmann\FileIterator\Facadeskipping hidden directories during filesystem traversal. This only happened on non-Windows platforms.The new implementation excludes hidden directories on all platforms, including Windows. This is a minor behavioral change that only affects Windows users who have source code in hidden directories. The new behavior is arguably more consistent across platforms.