Problem/Motivation

If you have phpstan/extension-installer installed, upgrade_status errors due to a double include of the extensions PHPStan Drupal and Deprecation Rules. Upgrade Status could probably check to see if it is installed before trying to also include them.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#3 3175304-3.patch981 bytesmglaman

Comments

mglaman created an issue. See original summary.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Right here: https://git.drupalcode.org/project/upgrade_status/-/blob/8.x-3.x/src/Dep...

This could be conditionally skipped. If \PHPStan\ExtensionInstaller\GeneratedConfig exists, don't execute this code.

    $config .= "\nincludes:\n\t- '" .
      $this->vendorPath . "/mglaman/phpstan-drupal/extension.neon'\n\t- '" .
      $this->vendorPath . "/phpstan/phpstan-deprecation-rules/rules.neon'\n";
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Active » Needs review
StatusFileSize
new 3175304-3.patch981 bytes

This requires manual testing.

Before patch:

./vendor/bin/drush upgrade_status:analyze MODULE
composer require phpstan/extension-installer
./vendor/bin/drush upgrade_status:analyze MODULE

💥

Apply patch. No 💥, just report.

Status: Needs review » Needs work

The last submitted patch, 3: 3175304-3.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Hm, we are running phpstan through exec() so whether that class exists in the runtime Drupal environment should not mean it does not need to be added to the phpstan config we run with exec()? This seems to be confirmed by the fail. I feel like I am missing something.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Hm, we are running phpstan through exec()

But it's using the same configuration and autoloader. And the autoloader adds items to the configuration.

I'm really confused how this broke the tests when it fixed it manually. I'll do more investigations.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs work » Reviewed & tested by the community

I sent this for a re-test. Apparently the fail case a month ago was on phpstan/phpstan 0.12.48, which falls in the range of https://github.com/mglaman/phpstan-drupal/issues/143. So that was a totally unrelated fail. It runs fine now :)

  • Gábor Hojtsy committed 45399bb on 8.x-3.x 
    Issue #3175304 by mglaman: Check if phpstan/extension-installer is...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

Landed, thanks again.

Status: Fixed » Closed (fixed)

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