Problem/Motivation

Discovered while committing #2921133: Remove "Please" from the codebase, this would apparently be triggered by any change to core/scripts/run-tests.sh:

$ ../vendor/bin/phpstan analyze scripts/run-tests.sh 

 ------ ------------------------------------------------------------------------------------------------------ 
  Line                                                                                                         
 ------ ------------------------------------------------------------------------------------------------------ 
  1239   Static method Drupal\Component\Utility\Html::decodeEntities() invoked with 3 parameters, 1 required.  
 ------ ------------------------------------------------------------------------------------------------------

Unsure why the full run of PHPStan doesn't pick this up.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 3376396-5.patch1.19 KBlongwave
#3 3376396-3.patch696 byteslongwave

Comments

longwave created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

For some reason I can reproduce this on my core commit checkout, but not on the separate checkout I use for development work, and I am really not sure why.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
Related issues: +#3372757: Bump phpstan/phpstan and mglaman/phpstan-drupal to latest to make daily "updated deps" QA run pass again
StatusFileSize
new 3376396-3.patch696 bytes

I had not run composer install on one copy since #3372757: Bump phpstan/phpstan and mglaman/phpstan-drupal to latest to make daily "updated deps" QA run pass again, so this must be a recent fix in PHPStan or phpstan-drupal.

The attached patch ensures PHPStan runs on all the .sh scripts in the scripts directory, and should trigger the error.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Needs work

Needs work to fix that method call.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
StatusFileSize
new 3376396-5.patch1.19 KB
smustgrave’s picture

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

Seems like a simple cleanup.

  • lauriii committed cb850c3b on 11.x 
    Issue #3376396 by longwave: Fix PHPStan error in run-tests.sh
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Committed cb850c3 and pushed to 11.x. Thanks!

quietone’s picture

quietone commented
Status: Fixed » Needs work

Since this commit I am getting errors like this on .inc files when running commit-code-check --cached .

Running PHPStan on changed files.
 ------ --------------------------------------------------- 
  Line                                                      
 ------ --------------------------------------------------- 
  285    Function t invoked with 2 parameters, 1 required.  
 ------ ---------------------------------------------------

When I revert the commit I do not get the above error message.

Setting back to Needs work for now.

quietone’s picture

quietone commented

I changed commit-code-check.sh so that PHPSTAN_DIST_FILE_CHANGED=1 and thus phpstan is run on all files. I then get the same error as above but on many files, *.php, *.module, *.inc, *.theme.

Add a dot to sh in the FileExtensions seems to be working as expected.

  fileExtensions:
    - .sh

With that I am not getting the error on either a full run or just the cached files. Maybe the phpstan docs are wrong?

quietone’s picture

quietone commented
Status: Needs work » Fixed

I am still having the problem mentioned above but so far, no one else has reported they also have it. I have opened a new issue #3384436: Remove redefintion of t() from update-countries.sh. I'll set this back to Fixed.

Status: Fixed » Closed (fixed)

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