shipmonk-rnd / composer-dependency-analyser

🚀 Fast detection of composer dependency issues (unused dependencies, shadow dependencies, misplaced dependencies)
MIT License
332 stars 8 forks source link

PHPStan Extension wrapper #152

Closed pb30 closed 1 week ago

pb30 commented 1 week ago

I've created a simple PHPStan extension that wraps this package and displays any errors as standard PHPStan errors. This provides the following benefits:

https://github.com/pb30/phpstan-composer-analysis

It does not (yet) support all of the configuration options as this package, but the missing "Ignore" configuration can be handled through PHPStan ignores functionality.

Wanted to share this here to see if there was any interest in folding the PHPStan extension into this repo (I can do a PR), or if you'd like the project under your Org, or no interest at all.

(This is my first PHPStan extension, so not 100% sure it's the "right" way, but I've used this on a handful of projects of varying sizes without issue)

Thanks for your work!

janedbal commented 1 week ago

Hello!

Thanks for the notice!

Wanted to share this here to see if there was any interest in folding the PHPStan extension into this repo (I can do a PR), or if you'd like the project under your Org

I think it is absolutely fine as separate package. No need to move it here.

This is my first PHPStan extension, so not 100% sure it's the "right" way

My suggestion here is to ensure that composer dependency analyser runs only once per PHPStan analysis, not once per process (which is currently the case imo). It should be enough to run it only in the Rule (as CollectedData rules are executed only once in the main process).


Also, since public api of composer dependency analyser is only the Configuration class and the cli api, and you are using its internal classes, there is no guarrantee that it wont break in future releases. I hope you are aware.

pb30 commented 1 week ago

My suggestion here is to ensure that composer dependency analyser runs only once per PHPStan analysis, not once per process (which is currently the case imo). It should be enough to run it only in the Rule (as CollectedData rules are executed only once in the main process).

I believe it should only be running once per analysis (at least based on my testing of adding some debug code in Analyser::run()). I had started with a Rule, but couldn't determine how to target an AST node so it only ran once.

Understood on API.

Thanks!

janedbal commented 1 week ago

I believe it should only be running once per analysis

Are you sure? Child processes wont output to your terminal. Try debug write to files, e.g. pid.

pb30 commented 1 week ago

On this line I put line below and got only one file:

file_put_contents('/path/to/myproject/' . uniqid(), '1');
pb30 commented 1 week ago

Ah PHPstan result cache was interfering, looks like it may run multiple times. Will look into that at some point.