Ignore phpstan-tmp in phpcs.xml.dist

Created on 7 August 2024, 4 months ago
Updated 27 August 2024, 3 months ago

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

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

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

πŸ“Œ Task
Status

Fixed

Version

11.0 πŸ”₯

Component
OtherΒ  β†’

Last updated about 18 hours ago

Created by

πŸ‡¬πŸ‡§United Kingdom longwave UK

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @longwave
  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    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.

  • πŸ‡¬πŸ‡§United Kingdom longwave UK
  • Merge request !9118Exclude phpstan-tmp from phpcs. β†’ (Open) created by longwave
  • Status changed to Needs review 4 months ago
  • πŸ‡¬πŸ‡§United Kingdom longwave UK
  • Pipeline finished with Failed
    4 months ago
    Total: 386s
    #247091
  • Status changed to RTBC 3 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

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

  • Status changed to Needs review 3 months ago
  • πŸ‡«πŸ‡·France nod_ Lille

    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?

  • Status changed to Needs work 3 months ago
  • πŸ‡«πŸ‡·France nod_ Lille

    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.
    
    
  • First commit to issue fork.
  • Status changed to RTBC 3 months ago
  • πŸ‡¬πŸ‡§United Kingdom catch

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

    Fixed the test - PHP orders differently from humans.

  • Pipeline finished with Success
    3 months ago
    Total: 392s
    #252568
    • 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...
  • Status changed to Fixed 3 months ago
  • πŸ‡«πŸ‡·France nod_ Lille

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

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

Production build 0.71.5 2024