phpstan / extension-installer

Composer plugin for automatic installation of PHPStan extensions.
MIT License
399 stars 27 forks source link

Why type: phpstan-extension is mandatory? #13

Closed kylekatarnls closed 4 years ago

kylekatarnls commented 5 years ago

Hello, I'm considering adding an extension.neon in an existing library to help recognizing methods available via __call / __callStatic PHPStan can't recognize.

Extension's composer package type has to be set to phpstan-extension for this plugin to be able to recognize it.

The library itself is not a PHPStan extension (so I can't set type to phpstan-extension). But I would like than my library users who also use PHPStan get those methods recognized by PHPStan the easier way.

Creating an other package to handle the extension.neon + 1 small file seems overkill too.

And finally, I think having a extra.phpstan property is enough to recognize a library that includes PHPStan files. Why requiring this additional phpstan-extension flag?

Thanks.

ondrejmirtes commented 4 years ago

I'm not sure. Is there a reason for the type @lookyman?

Anyway, phpstan/extension-installer can afford to be opinionated. It doesn't have to be used by everyone. You can always include your extension.neon manually in your project's phpstan.neon.

jdeniau commented 4 years ago

Hi,

I'm in the same situation here. I am trying to create a phpstan extension for mapado/rest-client-sdk, and I would prefer include the phpstan files directly in the library because they are really coupled.

I think we can replace in https://github.com/phpstan/extension-installer/blob/master/src/Plugin.php#L85

- if ($package->getType() !== 'phpstan-extension') {
+ if ($package->getType() !== 'phpstan-extension' || !isset($package->getExtra()['phpstan']['includes'])) {

I am willing to make a PR if you are OK with this.

ondrejmirtes commented 4 years ago

This condition is wrong, you need negated one.

jdeniau commented 4 years ago

This condition is wrong, you need negated one.

I updated the snippet 😉

ondrejmirtes commented 4 years ago

Please use && instaed of ||. I will accept the PR.

jdeniau commented 4 years ago

@ondrejmirtes Done

lookyman commented 4 years ago

There really isn't a reason for the package type. I think I just wanted a safeguard for the installer to kick in. I'm fine with removing it.

ondrejmirtes commented 4 years ago

I think it’s great for finding compatible packages on Packagist: https://packagist.org/explore/?type=phpstan-extension

But it should be optional in the definition.

ondrejmirtes commented 4 years ago

Solved with #16.

github-actions[bot] commented 3 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.