Skip to content

Conversation

canvural
Copy link
Contributor

Followup to d3589dc

Currently 1.7.x-dev still does not validate the stubs loaded from the extension. AnalyseApplication was the only place I found that calls StubValidator::validate So I thought here the stub files should also be obtained from StubFilesProvider::getStubFiles

@ondrejmirtes
Copy link
Member

The reason for the original code was that PHPStan does not analyse and report bugs from 3rd party stubs. So it used stubFiles that's only directly in the project's configuration. If you have some other idea how to isolate those but include the ones from the extension, I'm all ears :)

@canvural
Copy link
Contributor Author

What are 3rd party stubs?

@canvural
Copy link
Contributor Author

Aah, you mean for example an application requires a PHPStan extension that has StubFilesExtension, when they analyze their application, those stubs would also be analyzed.

Yeah, that's a tough issue I guess.

@ondrejmirtes
Copy link
Member

Currently it's not perfect either, some users have their project configuration consisting of their own multiple neon files, like https://github.com/pmmp/PocketMine-MP/blob/39b8daeeec3fd2ad9a8593bb15edca65349948bd/phpstan.neon.dist#L1-L11.

So the question is - how to recognize project stub files from 3rd party ones? Just exclude those from $CWD/vendor directory, and analyse everything else? Might be good enough :)

@canvural
Copy link
Contributor Author

Ok, I tried something. But not sure how to test it.

I can write a unit test for the array_filter logic in DefaultStubFilesProvider. But dunno about the integration test.

@ondrejmirtes
Copy link
Member

A few things to complicate your life:

Otherwise I think it should be fine - please test this in practice in your project if it works as expected. You can grab the PHAR file artifact of this PR in Compile PHAR workflow (rename it to phpstan.phar and copy it to vendor/phpstan/phpstan) https://github.com/phpstan/phpstan-src/actions/runs/2401474675

Don't worry about phpstan-doctrine failures, I expected them 😊

@ondrejmirtes
Copy link
Member

BTW the vendor-dir approach could be fixed in more places around the codebase where "vendor" is currently hardcoded 😊

@canvural
Copy link
Contributor Author

canvural commented Jun 2, 2022

Updated to use the new ComposerHelper to determine the vendor directory. If the composer.json is not found for some reason, it still returns all the stubs because there is no path to filter with.

@ondrejmirtes
Copy link
Member

Awesome, thank you!

@ondrejmirtes ondrejmirtes merged commit 39bbdb9 into phpstan:1.7.x Jun 2, 2022
@canvural canvural deleted the validate-stubs branch June 2, 2022 15:38
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.

2 participants