composer: Raising the PHPStan level to 6

I have laid down the ground work for reaching level 6 by typing all class properties and interface methods throughout the codebase, which should already help ensure that types are correct when they get added/copied to getters and such.

I definitely could use help though in reaching level 6, because it’s a ton of work left (1465 errors for src/, 400 errors for tests/).

If you are interested in helping, here is how to go about it:

Setup a Composer environment first if you haven’t

  • git clone https://github.com/composer/composer
  • cd composer && wget https://getcomposer.org/composer.phar && php composer.phar install

Setup the env for PHPStan-usage

This requires PHP 7.4 (it won’t run with php8 currently due to reasons, don’t try to make it do that)

  • php7.4 composer.phar phpstan-setup to get phpstan installed in the project - make sure you have a composer.phar or some global composer install to run this command with, you can’t run phpstan-setup with bin/composer as it replaces parts of the vendor dir and it’ll break the install

Running PHPStan

  • Pick a single sub-namespace and focus work on one area, otherwise it’s too many errors and also we risk having multiple people doing the same work. Ideally you can comment here to claim a namespace if you’re gonna work on it.
  • Change level from 5 to 6 in phpstan/config.neon
  • Run it with the sub-path you are interested in e.g. php7.4 vendor/bin/phpstan analyse --configuration=phpstan/config.neon src/Composer/Package/

Keep in mind

  • If you are unsure about some type, rather leave it undefined/erroring than use mixed or something equally vague, leave it for someone else to fill the gap later, because reviewing the whole code to find inappropriate mixed-usage is going to be near impossible.
  • The main focus is src/ IMO. Typing the tests can’t hurt, but it might be a mess in places as some of that code is very old and using php 5.3-compatible patterns.

Thanks in advance to any volunteer 😃

Status

Pick one that’s not checked, and claim below in comments please.

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Reactions: 5
  • Comments: 38 (36 by maintainers)

Commits related to this issue

Most upvoted comments

Alright we are done here, level 6 is now enabled! 🥳

Many many thanks to everyone involved here writing and reviewing PRs, that was a great help!

Now only 770 errors left to level 7 😉 But that’ll require some more code cleanups I think and will have to wait a little.

+6
Seldaek on Nov 2, 2021

BTW for everyone working on tests, if you see dataProvider methods which PHPStan complains about, I’d say instead of adding types feel free to rename them to provideFoo or fooProvider, which will then make PHPStan ignore the errors. I don’t see much point in typing those return values as they’re usually super weird nested arrays and not directly called anywhere.

@megubyte all yours, I marked them claimed now.

+3
Seldaek on Oct 26, 2021

Down to 286 errors in Composer/* and Composer/Command/* (@samfelgar as you still working on this one or can someone else tackle it?) 👍🏻

I’ll finish it today!

+1
samfelgar on Oct 25, 2021

Down to 286 errors in Composer/* and Composer/Command/* (@samfelgar as you still working on this one or can someone else tackle it?) 👍🏻

+1
Seldaek on Oct 25, 2021

Well, when the PR is referenced, github shows whether it is merged or no (the icon is different). And we cannot have multiple checkboxes per item in a GFM task list.

+1
stof on Oct 18, 2021

@stof do you mean PHP 7.4 style types properties? Because that’s a long way out…

As for var annotations as i wrote in the op i did spend time typing all properties throughout the codebase, which should be enough for static analysis and tools like rector. But it seems to do too much magic with other things which isn’t desirable IMO. Maybe I’ll try to run it again only with one or two very specific rules.

+1
Seldaek on Oct 15, 2021

Ah I wasn’t aware it did type inference too, thanks, will definitely try that before wasting anyone’s time doing it by hand 😃

+1
Seldaek on Oct 15, 2021
Read more comments on GitHub
← composer: `psalm/phar` broken on composer 2.2
composer: [Regression] class loading does not work since 2.1.0-RC1 →