Skip to content

Filtering default stub files on Windows#2406

Merged
ondrejmirtes merged 2 commits intophpstan:1.10.xfrom
maryo:patch-1
Jul 6, 2023
Merged

Filtering default stub files on Windows#2406
ondrejmirtes merged 2 commits intophpstan:1.10.xfrom
maryo:patch-1

Conversation

@maryo
Copy link
Copy Markdown
Contributor

@maryo maryo commented May 16, 2023

ComposerHelper::getVendorDirFromComposerConfig($this->currentWorkingDirectory, $composerConfig)
returns path separated by / ($root . '/' . trim($vendorDirectory, '/')) and $composerConfig['config']['vendor-dir'] could contain both separators.

On Windows, $this->getStubFiles() returns paths separated by \. So the vendor stub files are never filtered out on Windows.

getVendorDirFromComposerConfig could already return it normalized but I was not brave enough to change it.

@phpstan-bot
Copy link
Copy Markdown
Collaborator

You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.

@maryo maryo changed the base branch from 1.11.x to 1.10.x May 16, 2023 19:39
@maryo maryo marked this pull request as draft May 16, 2023 20:47
@maryo maryo marked this pull request as ready for review May 16, 2023 20:59
@phpstan-bot
Copy link
Copy Markdown
Collaborator

This pull request has been marked as ready for review.

@ondrejmirtes
Copy link
Copy Markdown
Member

This change needs a test (that fails when the changes in src/ are removed).

@maryo maryo force-pushed the patch-1 branch 3 times, most recently from d3d57d2 to 483609f Compare May 17, 2023 18:25
@maryo
Copy link
Copy Markdown
Contributor Author

maryo commented May 17, 2023

I see. Some tests added.
testGetProjectStubFilesWhenPathContainsWindowsSeparator fails without the changes

@ondrejmirtes ondrejmirtes merged commit e6976b6 into phpstan:1.10.x Jul 6, 2023
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants