Problem/Motivation

Committing https://www.drupal.org/project/drupal/issues/3624887 is hard because commit-code-check.sh fails because the file that's changed is excluded from PHPStan.

Steps to reproduce

Use commit code check on an issue which only changes .api.php file.

Proposed resolution

There's no easy way to ignore this error. We have to catch the error and do a text match.

Remaining tasks

User interface changes

None

Introduced terminology

None

API changes

None

Data model changes

None

Release notes snippet

N/a

Generated with the help of an LLM.

I used AI to help code the fix.

Issue fork drupal-3636099

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

alexpott created an issue. See original summary.

alexpott opened merge request !17727

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Tested locally - works a charm...

$ git commit -m "Test"
4.18.0
CSpell: Files checked: 1 (1 from cache), skipped: 1, Issues found: 0 in 0 files.

CSpell: passed

----------------------------------------------------------------------------------------------------

Running PHPStan on changed files.

 [ERROR] No files found to analyse.

PHPStan: passed

----------------------------------------------------------------------------------------------------
Time: 620ms; Memory: 22MB

PHPCS: passed

----------------------------------------------------------------------------------------------------
 ______________________________________________________________________________________________________________________
< Committed <a href="https://git.drupalcode.org/project/drupal/commit/778fe08">778fe08</a> and pushed to main. Thanks! >
 ----------------------------------------------------------------------------------------------------------------------
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

*** Does it need a change record? ***
*** Should it be in the release notes? ***
[main 778fe083f3a] Test
 1 file changed, 41 deletions(-)
xjm’s picture

xjm
she/her
Primary language English
commented

Not to scope-creep, but I think PHPCS has the same issue? There are a number of recent changes related to PHPStan that I was only able to commit with --no-verify.

xjm’s picture

xjm
she/her
Primary language English
commented

I confirmed that #3624887: Remove obsolete hook_extension() and hook_render_template() documentation fails with:

Running PHPStan on changed files.

                                                                                
 [ERROR] No files found to analyse.                                             
                                                                                


PHPStan: failed

...and passes with the diff from this issue committed first.

Examples of issues that needed a --no-verify for PHPCS:

Not sure if we want to file a separate issue for that (everyone claims it exists but I never found one) or make some sort of change to how the script works that would resolve this for PHPCS as well.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Needs work

@xjm let's fix the PHPCS thing here too... seems like a good thing to do

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review

Actaully the PHPCS thing was fixed by #3574117: Improve commit-code-check.sh so we're all good here.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Ah I see why the PHPCS is not 100% fixed... it's core/.baseline.php ending up in $PHP_FILES

mstrelan’s picture

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

Tested this in three scenarios, when there are no files to scan, when there are no violations and when there are violations. All seemed fine, only one cosmetic problem, if there are no files to scan it outputs [ERROR] No files found to analyse followed by PHPStan: passed, which I think is good enough.

quietone’s picture

quietone commented

@mstrelan, beat me to this. Nonetheless here is what I did.

I applied the diff, made a change to core.api.php and ran the commit-code-check. There were no failures
I removed those changes and added a blank line to core/.phpstan-baseline.php and ran the commit-code-check. There were no failures.
I then add a PHPCS error to Concat.php and ran commit-code-check. PHPCS reported the error, so nothing broken there.

This is really nice to have fixed, Thanks!

  • catch committed fe3a9de6 on 11.5.x 
    fix: #3636099 commit-code-check.sh fails if PHPStan ignores all the...

  • catch committed d4987d00 on 11.x 
    fix: #3636099 commit-code-check.sh fails if PHPStan ignores all the...

  • catch committed 04671a87 on 12.0.x 
    fix: #3636099 commit-code-check.sh fails if PHPStan ignores all the...

  • catch committed af90f36e on main 
    fix: #3636099 commit-code-check.sh fails if PHPStan ignores all the...
catch’s picture

catch
he/him
Primary language English
commented
Version: main » 11.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to main and cherry-picked back through to 11.5.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.