Fix PHPStan violations (happened because PHPStan code checks stopped running…)

updated output

I started the 3342120-fix-some-phpstan because are some clear things we can fix. If someone wants to take this up there are more to fix

Do I need to fix all of this problems + every other problem in package_manager (phpstan has problme with every use of \Drupal) I am getting on local? Also I need help with package_manager/tests/src/Kernel/SymlinkValidatorTest.php problem that $this->reveal() Call to an undefined method Drupal\Tests\package_manager\Kernel\SymlinkValidatorTest::reveal().

Yes, we should fix everything.

(I'm assuming you get the same output locally as @tedbow reported above?)

Hi @tedbow, I am not able to reproduce auto_updates/src/Validator/StagedProjectsValidator.php
locally

see:

Am I missing something? Other than that other errors are fixed taking consideration core/phpstan.neon.dist

Hi @tedbow, I am not able to reproduce auto_updates/src/Validator/StagedProjectsValidator.php
locally

see:

Am I missing something? Other than that other errors are fixed taking consideration core/phpstan.neon.dist

You are missing something. You continued Ted's MR. Ted had already fixed this in his first commit to the MR: https://git.drupalcode.org/project/automatic_updates/-/merge_requests/70... …

Clarifying after discussing with @tedbow and @yash.rode :)

See #3343430-4: Commit our copied and edited version of commit-code-check.sh → , I am pretty sure that Drupal core was not running these checks anymore either!

This has been unblocked since 📌 Stop reusing core's commit-code-check.sh in favor of 4 simple commands Fixed landed last night!

phpstan.neon.dist excludes error pattern #^Class .* extends @internal class#
but we are getting an error messages

Anonymous class extends @internal class                      
         Drupal\automatic_updates\Validator\CronFrequencyValidator

As discussed in the call just now, I'll be digging into this — you fixed all the things, the only remaining thing is battling phpstan 👍🤓

🥳 PHPStan is passing now!

But … a million tests are failing.

Because this was added

assert($this instanceof ContainerBuilder);

to \Drupal\Tests\package_manager\Traits\ValidationTestTrait::resolvePlaceholdersInArrayValuesWithRealPaths().

That of course makes no sense. But specifying $this->container just triggers another PHPStan error.

Fixed.

There was a similar bug introduced by @yash.rode in FormTestTrait. Fixed that too. Now doing a proper review…

I reverted 99% of @yash.rode's 1a4a9768be59ccb95d8d48c15e791e4589b9ea28. See https://git.drupalcode.org/project/automatic_updates/-/merge_requests/70....

🚢

I have some questions...

Answered them all.

Assigning to myself to fix the one outstanding thing that I don't agree with.

🥳 Thanks!

Will be great to finally have phpstan running again, and this time we'll know for sure we won't regress! 😊

Based on the sheer number of tests that will have to become more complicated … do you really think 4d58dd79 is worth it, @phenaproxima? That seems like a pretty painful change for DX… 😳

Re #26: I concur. I'd still like to not redefine the property unless we absolutely have to. And if we absolutely have to, then it will need a comment to explain it.

That said, it looks like asserting that $this is a KernelTestBase or BrowserTestBase (both of which have $this->container) was sufficient. Restoring RTBC and will merge when tests pass.

Nice to get this merged in. I think PHPStan will catch a lot of little logic errors and nitpicks that we'd usually find in code review. Hopefully this makes development a little easier and keeps us closer to core standards!

Automatically closed - issue fixed for 2 weeks with no activity.