Problem/Motivation

When running PHPStan check I'm getting the following error:

Child process error (exit code 255): PHP Warning: file_get_contents(phar:///var/www/html/vendor/phpstan/phpstan/phpstan/vendor/phpunit/phpunit/src/Framework/TestCase.php): failed to open stream: phar error: "vendor/phpunit/phpunit/src/Framework/TestCase.php" is not a file in phar "/var/www/html/vendor/phpstan/phpstan/phpstan" in /var/www/html/core/tests/Drupal/TestTools/PhpUnitCompatibility/PhpUnit8/ClassWriter.php on line 43 Warning: file_get_contents(phar:///var/www/html/vendor/phpstan/phpstan/phpstan/vendor/phpunit/phpunit/src/Framework/TestCase.php): failed to open stream: phar error: "vendor/phpunit/phpunit/src/Framework/TestCase.php" is not a file in phar "/var/www/html/vendor/phpstan/phpstan/phpstan" in /var/www/html/core/tests/Drupal/TestTools/PhpUnitCompatibility/PhpUnit8/ClassWriter.php on line 43 PHP Fatal error: Declaration of Drupal\Tests\UnitTestCase::setUp() must be compatible with PHPUnit\Framework\TestCase::setUp(): void in /var/www/html/core/tests/Drupal/Tests/UnitTestCase.php on line 41 Fatal error: Declaration of Drupal\Tests\UnitTestCase::setUp() must be compatible with PHPUnit\Framework\TestCase::setUp(): void in /var/www/html/core/tests/Drupal/Tests/UnitTestCase.php on line 41

See https://dispatcher.drupalci.org/job/drupal8_contrib_patches/36806/Phpstan/

The same happens locally

Seems like the following code:

$reflector = new \ReflectionClass($autoloader);
$vendor_dir = dirname($reflector->getFileName(), 2);

does not work when executed within the other reflection (by PHPStan) - my assumption needs to be verified.

Proposed resolution

Implement alternative PHPUnit source code directory detection

Remaining tasks

Provide patch & review

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#8 interdiff-3143604-2-8.txt1.3 KBchr.fritsch
#8 3143604-8.patch1.77 KBchr.fritsch
#2 3143604-2.patch1.82 KBTaran2L
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Taran2L created an issue. See original summary.

Taran2L’s picture

Taran2L
Primary language Ukrainian
Location Lviv
CreditAttribution: Taran2L at Lemberg Solutions, Drupal Ukraine Community commented
FileSize
3143604-2.patch1.82 KB
Taran2L’s picture

Taran2L
Primary language Ukrainian
Location Lviv
CreditAttribution: Taran2L at Lemberg Solutions, Drupal Ukraine Community commented
Status: Active » Needs review
Taran2L’s picture

Taran2L
Primary language Ukrainian
Location Lviv
CreditAttribution: Taran2L at Lemberg Solutions, Drupal Ukraine Community commented

With this patch, everything works as expected https://dispatcher.drupalci.org/job/drupal8_contrib_patches/36849/Phpstan/

dungahk’s picture

dungahk
he/him/his
Primary language Portuguese, Brazil
Location Hertfordshire
CreditAttribution: dungahk at Zoocha commented

I can confirm those changes fixes the issue. I'm running PHPStan with a Drupal 9.0.0-rc1 website. I will not say the changes are correct because they are out of my expertise at the moment.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave as a volunteer commented
Status: Needs review » Reviewed & tested by the community

This passes the DrupalCI tests and so the change is fine for there, as reported above this works for PHPStan now as well, so I think this is good to go - I don't think we could or should add a separate test for this specific case in core itself.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/core/tests/Drupal/TestTools/PhpUnitCompatibility/PhpUnit8/ClassWriter.php
@@ -36,13 +36,11 @@ final class ClassWriter {
-    // Inspired by Symfony's simple-phpunit remove typehints from TestCase.

    Please don't removed this line. I think it is important to document where this idea came from. 

  2. +++ b/core/tests/Drupal/TestTools/PhpUnitCompatibility/PhpUnit8/ClassWriter.php
@@ -36,13 +36,11 @@ final class ClassWriter {
+    $phpunit_dir = dirname($autoloader->getClassMap()['PHPUnit\Framework\TestCase'], 3);

    Let's make less of an assumption about how the class is being autoloaded.

    We can do 

        $alteredFile = $autoloader->findFile('PHPUnit\Framework\TestCase');
    $phpunit_dir = dirname($alteredFile, 3);
    // Mutate TestCase code to make it compatible with Drupal 8 and 9 tests.
    $alteredCode = file_get_contents($alteredFile);
    $alteredCode = preg_replace('/^    ((?:protected|public)(?: static)? function \w+\(\)): void/m', '    $1', $alteredCode);
    $alteredCode = str_replace("__DIR__ . '/../Util/", "'$phpunit_dir/src/Util/", $alteredCode);

    instead. If getClassMap() works then findFile() should too.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
CreditAttribution: chr.fritsch at Thunder commented
Status: Needs work » Needs review
FileSize
3143604-8.patch1.77 KB
interdiff-3143604-2-8.txt1.3 KB

Thanks for reviewing, @alexpott.

I applied your suggestions.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave as a volunteer commented

Can someone test #8 with PHPStan, if that works this can be marked RTBC again?

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
CreditAttribution: chr.fritsch at Thunder commented

This CI run (https://travis-ci.org/github/BurdaMagazinOrg/module-update_helper/builds...) uses the patch and PHPStan is now working.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Version: 9.1.x-dev » 9.0.x-dev

Committed 15b39f0 and pushed to 9.1.x. Thanks!

Will backport to 9.0.x once that is open for commits.

  • alexpott committed 15b39f0 on 9.1.x 
    Issue #3143604 by chr.fritsch, Taran2L, longwave, alexpott, dungahk:...

  • alexpott committed 756fe35 on 9.0.x 
    Issue #3143604 by chr.fritsch, Taran2L, longwave, alexpott, dungahk:...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

9.0.x is now open for bugfixes.

yang_yi_cn’s picture

yang_yi_cn CreditAttribution: yang_yi_cn at New/Mode commented

I'm not sure if the fix is good?

After applying the patch I got a bunch of phpstan errors like:

  85     Function user_load_by_name not found.                                        
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols           
  87     Call to static method create() on an unknown class Drupal\user\Entity\User.  
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols   
...
         Reflection error: Drupal\rest\Plugin\ResourceBase not found.        
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols

Prior to this patch I didn't get those errors, and I believe the functions still exist in D9 but somehow can't be found after the patch.

Is it due to the changes in the patch about autoloader and reflector?

yang_yi_cn’s picture

yang_yi_cn CreditAttribution: yang_yi_cn at New/Mode commented
Status: Fixed » Needs review
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Needs review » Fixed

@yang_yi_cn this change doesn't affect autoloading of class in PHPStan - there's something else going on. This change was about how we detect the location of the TestCase class.

Status: Fixed » Closed (fixed)

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