Problem/Motivation
PHPStan is reporting Function t invoked with 2 parameters, 1 required errors when I run it locally on HEAD. I first noticed this when working on a change and used commit-code-check.sh. Also reported in the last full run is Ignored error pattern #^Variable \$error in empty\(\) always exists and is not falsy\.$# in path /var/www/html/core/modules/media/media.install was not matched in reported errors.
I deleted the site and started fresh. There was no change. Clearing the phpstan cache also has no effect.
I tracked it to #3376396: Fix PHPStan error in run-tests.sh. When I revert that commit I don't get the errors. However, if I keep that commit but change the fileExtensions from - sh to - .sh PHPStan doesn't report these errors. I only tried adding the "." out of desperation and it isn't a fix, just information.
I've attached a file with the output of running
php -d apc.enabled=0 -d apc.enable_cli=0 vendor/bin/phpstan analyze --no-progress --configuration=core/phpstan.neon.dist > phpstan-errors.txt
That reports errors in 203 files.
My dev environment is ddev, version v1.22.1.
Steps to reproduce
Proposed resolution
Remove redefintion of t() from update-countries.sh
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| phpstan-errors.txt | 108.91 KB | quietone |
Issue fork drupal-3384436
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:
- 3384436-phpstan-reports-errors
changes, plain diff MR !4721
Comments
Comment #2
quietone commentedComment #3
spokjeExperiencing the exact same thing as @quietone.
Local env is Windows using XAMPP and MINGW64, before #3376396: Fix PHPStan error in run-tests.sh there were no differences between running PHPStan locally or on Drupal CI
I work around the
Function tonce (which adds a lot of errors) by suppressing it inphpstan.neon.distby adding this- "#^Function t invoked with .* parameters, 1 required#"as the last line.Then I only have to worry about the locally not found
#^Variable \$error in empty\(\)Comment #6
smustgrave commentedExperienced this today myself. This solved it for me.
Comment #7
smustgrave commentedChatting with catch in https://drupal.slack.com/archives/C1BMUQ9U6/p1694113654557499 going to try and delete the redefinition
Comment #8
quietone commentedAfter reading the slack discussion I was able to track what this is doing. @smustgrave, I think we shouldn't rely on Slack history to be available or accessible to everyone.We should document on the issue.
The block of code being removed was added in #1938892: Switch from ISO-3166-1 country data to CLDR unicode data. Years later, #3151118: Include bootstrap.inc using composer removed the redefinition of t() but missed the one in update-countries.sh. And this make sense since it started when PHPStan began scanning .sh files.
I tested this by adding a comment to migrate.module and ran commit-code-check.sh. Without the change I got the PHPStan errors without I did not.
Thanks!
Comment #9
spokjeNice sleuthing @catch and @smustgrave, as @quietone already said, this gets rid of my PHPStan woes.
Comment #10
catchCommitted/pushed to 11.x and cherry-picked to 10.1.x, thanks!
Comment #11
lauriiiI think I pushed at the same time with you and your commits didn't actually push 😇