Repository navigation
Keep an optional path as written where a plain one would be - #6452
Merged
Merged
Conversation
staabm
reviewed
Sep 16, 2026
SanderMuller
force-pushed
the
optional-path-relative
branch
from
September 16, 2026 09:20
59233c4 to
055ee44
Compare
staabm
reviewed
Sep 16, 2026
Comment on lines
+13
to
+15
| excludePaths: | ||
| analyse: | ||
| - src/NotThere (?) |
Contributor
There was a problem hiding this comment.
do we need another test in which a file which is configured optional exists?
Contributor
Author
There was a problem hiding this comment.
Added one. optional-src/ now exists, holds a file with a level 8 error, and is excluded optionally alongside the missing path.
It does not change how the path is stored. createOptionalPath() never touches the filesystem, and I checked that an existing and a missing entry store identically. What it adds is an assertion the missing-path version could not make: that the entry still excludes. I confirmed that by pointing the entry at a typo, which makes the analyse exit 1 on the error in that file.
`- path (?)` in excludePaths was expanded and absolutized whatever the loader was asked to expand. A plain entry under the same key is only touched when that key is listed in expandRelativePaths. CommandHelper reads the project config for the result cache metadata with a list holding just paths and tmpDir. So a relative optional path was stored absolute in the cache metadata while the required entry beside it stayed as written. Moving the anchor directory - a fresh CI checkout, a git worktree - then changed projectConfig and discarded the whole cache. The three shapes an entry can take now store the same way as a plain one: relative stays relative, absolute stays absolute, and a %placeholder% stays unexpanded. The container load is unaffected, because every excludePaths key is in its expandRelativePaths list. The e2e scenario reuses the existing sources through a second config file, so what the first one covers is unchanged. That config carries its own tmpDir: two configs sharing one write the same %tmpDir%/resultCache.php and discard each other's cache, which is a separate problem. Closes phpstan/phpstan#15238 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SanderMuller
force-pushed
the
optional-path-relative
branch
from
September 16, 2026 10:30
055ee44 to
620510a
Compare
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.
- path (?)inexcludePathswas expanded and absolutized whatever the loader was asked to expand. A plain entry under the same key is only touched when that key is listed inexpandRelativePaths.CommandHelperreads the project config for the result cache metadata with a list holding justpathsandtmpDir.So the required entry stayed as written and the optional one beside it turned absolute. Moving the anchor directory then changed
projectConfigand threw the whole cache away. Same project, two paths, before:After, both:
All three shapes an entry can take now store the way a plain one does. Relative stays relative, absolute stays absolute, and a
%placeholder%stays unexpanded. The container load is unaffected, because everyexcludePathskey is in itsexpandRelativePathslist. I checked that rather than assumed it: an error in each excluded directory, and the reported set is identical before and after.The e2e fixture gains one optional
excludePathsentry, so the scenario's existing "no checkout prefix anywhere in the cache" assertion covers this. It fails on the base and passes with the change.Closes phpstan/phpstan#15238