Problem/Motivation

commit-code-check.sh runs PHPStan, that now creates a cache in core/phpstan-tmp.

If phpcs.xml.dist is modified, it then runs phpcs across the entire codebase - which currently does not exclude core/phpstan-tmp - and so the commit errors with things like

FILE: ./core/phpstan-tmp/cache/PHPStan/7d/6e/7d6e5b8369b9e39306b75313ec87286641509818.php
-------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 23 ERRORS AFFECTING 14 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------------
  1 | ERROR | [x] Missing file doc comment (Drupal.Commenting.FileComment.Missing)
...

Steps to reproduce

Proposed resolution

Either configure core/phpstan-tmp to be ignored in phpcs.xml.dist

Or remove the PHPStan tmpDir setting:

By default, PHPStan stores its cache files in sys_get_temp_dir() . '/phpstan' (usually /tmp/phpstan

This seems fine for our needs, and avoids cluttering the core directory.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3466689

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

longwave created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Title: Ignore phpstan-tmp in phpcs, or change PHPStan tmpDir » Ignore phpstan-tmp in phpcs.xml.dist
Issue summary: View changes

Oh, we can't remove it, because we did this so GitLab CI can access the cache as an artifact. So let's just ignore it for phpcs.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Issue summary: View changes

longwave opened merge request !9118

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Seems pretty straight forward. Can't re-run failing unit tests but highly doubt this change broke those.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Reviewed & tested by the community » Needs review

we're also creating: 

core/.cspellcache
core/.eslintcache
core/.stylelintcache

Should they be excluded as well or doesn't matter since they don't have php inthem?

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Needs work

test failure is legit

Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Fail      Other      phpunit-440.xml      0 Drupal\Tests\PhpCs\SortTest        
    PHPUnit Test failed to complete; Error: PHPUnit 10.5.29 by Sebastian
    Bergmann and contributors.
    Runtime:       PHP 8.3.10
    Configuration: /builds/project/drupal/core/phpunit.xml.dist
    .F                                                                  2 / 2
    (100%)
    Time: 00:00.025, Memory: 8.00 MB
    There was 1 failure:
    1) Drupal\Tests\PhpCs\SortTest::testSorted
    Failed asserting that two arrays are equal.
    --- Expected
    +++ Actual
    @@ @@
         1 => '*/node_modules/*'
         2 => './assets/vendor/*'
         3 => './core/.phpstan-baseline.php'
    -    4 => './core/phpstan-tmp/*'
    -    5 => './core/lib/Drupal/Component/Diff/'
    +    4 => './core/lib/Drupal/Component/Diff/'
    +    5 => './core/phpstan-tmp/*'
         6 => './core/tests/Drupal/Tests/Com...trine/'
         7 => './modules/system/tests/fixtur...ssTest'
     )
    /builds/project/drupal/core/tests/Drupal/Tests/PhpCs/SortTest.php:98
    /builds/project/drupal/core/tests/Drupal/Tests/PhpCs/SortTest.php:56
    FAILURES!
    Tests: 2, Assertions: 7, Failures: 1.

catch made their first commit to this issue’s fork.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Reviewed & tested by the community

I think #7 doesn't matter - we can always add those in a follow-up.

Fixed the test - PHP orders differently from humans.

  • nod_ committed 63980670 on 11.0.x 
    Issue #3466689 by longwave, catch, nod_, smustgrave: Ignore phpstan-tmp...

  • nod_ committed 576253ca on 11.x 
    Issue #3466689 by longwave, catch, nod_, smustgrave: Ignore phpstan-tmp...
nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 11.x-dev » 11.0.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 576253ca19 to 11.x and 63980670bb to 11.0.x. Thanks!

Status: Fixed » Closed (fixed)

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