extension-installer icon indicating copy to clipboard operation
extension-installer copied to clipboard

Published 20 hours ago • verified publisher icon phpstan

Install phpstan extension also for root package

Open mvorisek opened this issue 2 years ago • 7 comments

fixes https://github.com/phpstan/extension-installer/issues/36

mvorisek avatar Nov 26 '21 13:11 mvorisek

PR is done, Build / PHPStan (8.0, highest) CI error seems unrelated

mvorisek avatar Nov 26 '21 14:11 mvorisek

@ondrejmirtes can you please review this PR?

mvorisek avatar Dec 15 '21 09:12 mvorisek

  1. This PR does too many unrelated changes
  2. I'm not even sure that https://github.com/phpstan/extension-installer/issues/36 is a real problem that needs solving. The whole point of this package is that it saves a few lines in your phpstan.neon.

ondrejmirtes avatar Dec 15 '21 14:12 ondrejmirtes

  1. This PR does too many unrelated changes

I have separated the verbose logging to https://github.com/phpstan/extension-installer/pull/42, all other changes are related

  1. I'm not even sure that composer.json's extra section must be honored also for root-package #36 is a real problem that needs solving. The whole point of this package is that it saves a few lines in your phpstan.neon.

Exactly that is the point. When phpstan config is added to composer.json, it must always apply.

Currently, for the root project, all configs must be included on two locations - like in https://github.com/atk4/data/blob/b0b50a0f0ae5ce280a180beb0dfd63f08100e02f/phpstan.neon.dist#L1-L3 . And if such config is then run from another project, it will even fail as included twice, steps to reproduce:

  1. create some repo with this ext, require package x/y with phpstan config phpstan-ext.neon specified in composer.json and also include that config in phpstan.neon
  2. run composer install
  3. run phpstan from the "some repo" on vendor/x/y
  4. currently, phpstan will fail as phpstan-ext.neon is loaded twice
  5. with this PR, phpstan-ext.neon is enough to be specified in a single place - composer.json, phpstan always finish no matter if run from x/y project directly or from root project

mvorisek avatar Dec 15 '21 15:12 mvorisek

@ondrejmirtes can you please review?

mvorisek avatar Jun 03 '22 12:06 mvorisek

I'm still not sure about the implications and the usefulness. I'd rather not do it.

ondrejmirtes avatar Jun 03 '22 12:06 ondrejmirtes

yes, repos including some extra conf manually need to be updated (otherwise phpstan will throw for config included twice), but this PR unifies how the composer config behaves - like for ex. autoload config - it works for root project and also when included

other than this change needed, do you have any other implication in head?

to maintain compatibility with existing projects, what about releasing this as 2.x version?

mvorisek avatar Jun 03 '22 12:06 mvorisek

I feel like there isn't any demand for this, and I feel against it too. So closing. Thanks for understanding.

ondrejmirtes avatar Jan 17 '23 10:01 ondrejmirtes