Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHPStan configuration update to fix ModuleHandler->loadInclude() (v2) #6122

Open
wants to merge 3 commits into
base: 13.x
Choose a base branch
from

Conversation

weitzman
Copy link
Member

Continued from #6121

@weitzman
Copy link
Member Author

3 PHPStan errors are persisting. They are about loadInclude(). Help wanted. You can still push to the source branch @andyg5000.

P.S. Sorry for the awkward branch naming - my gh pr checkout tool is not behaving for some reason.

@andyg5000
Copy link

Hey @weitzman I think the CI pipeline has a cached phpstan index or something. Have you tried to run this locally using the debug flag?

./vendor/bin/phpstan --debug

You should only see the 2 errors that are mentioned in #6120 (comment). Those will need code updates to use the correct logger call and likely remove the empty check since it will never be empty.

Let me know what you find out regarding CI and running the tests.

@andyg5000
Copy link

Full output from my test run for reference. I mention the cache because Matt Glaman and I determined that the phpstan cache doesn't invalidate when changing the configuration file (for some reason). Not sure what else would be causing it, but we saw the same behavior at the table during DrupalCon

61e0eb0b8fd5:/var/www/html/drush-dev/drush$ ./vendor/bin/phpstan --configuration="./phpstan.neon.dist"
 210/210 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ -----------------------------------------------------------------
  Line   Commands/core/LanguageCommands.php
 ------ -----------------------------------------------------------------
  61     Call to an undefined method Psr\Log\LoggerInterface::success().
         ✏️  Commands/core/LanguageCommands.php
 ------ -----------------------------------------------------------------

 ------ ------------------------------------------------------------------
  Line   Commands/core/LocaleCommands.php
 ------ ------------------------------------------------------------------
  488    Variable $reader_item in empty() always exists and is not falsy.
         ✏️  Commands/core/LocaleCommands.php
 ------ ------------------------------------------------------------------

@weitzman
Copy link
Member Author

Those 2 fixes are already in the PR.

I am seeing the same failures locally. See below. And I dont see caching that is setup for this job in CI.

Line   Commands/core/LanguageCommands.php
 ------ ---------------------------------------------------------------------
  160    Function _locale_translation_default_update_options not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
         ✏️  Commands/core/LanguageCommands.php
  163    Function locale_translation_batch_update_build not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
         ✏️  Commands/core/LanguageCommands.php
  165    Function locale_config_batch_update_components not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
         ✏️  Commands/core/LanguageCommands.php
 ------ ---------------------------------------------------------------------

 ------ ---------------------------------------------------------------------
  Line   Commands/core/LocaleCommands.php
 ------ ---------------------------------------------------------------------
  131    Function _locale_translation_default_update_options not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
         ✏️  Commands/core/LocaleCommands.php
  139    Function locale_translation_batch_update_build not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
         ✏️  Commands/core/LocaleCommands.php
  146    Function locale_config_batch_update_components not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
         ✏️  Commands/core/LocaleCommands.php
  230    Function _locale_translation_default_update_options not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
         ✏️  Commands/core/LocaleCommands.php
  270    Function locale_config_batch_update_components not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
         ✏️  Commands/core/LocaleCommands.php
  313    Function _locale_translation_default_update_options not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
         ✏️  Commands/core/LocaleCommands.php
  330    Function locale_config_batch_update_components not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
         ✏️  Commands/core/LocaleCommands.php
 ------ ---------------------------------------------------------------------


 [ERROR] Found 10 errors

@weitzman
Copy link
Member Author

Maybe retry with the surce branch of this new PR - andyg5000/13.x (sorry for the confusing name). Otherwise I dont know why this passes for you and fails for me and Circle.

@andyg5000
Copy link

Ok I was seeing the same with the new branch and without the --debug flag. After running with the --debug flag it's 0 errors all green and subsequent runs with or without --debug are all green too.

I'm on the train now, but when I get stable internet I'll do some research on the cache or hashing checks that phpstan does and report back. In the meantime, how are your results with --debug?

@weitzman
Copy link
Member Author

The last results I pasted were with --debug. Each filename was output before the report that I pasted above.

@weitzman
Copy link
Member Author

I deleted my DDEV build and started from scratch - same errors. I would be very surprised if this is a caching issue.

@weitzman
Copy link
Member Author

I ran with --xdebug and I saw the includes getting require_once() successfully but still those symbols are not found. Its a mystery.

@andyg5000
Copy link

andyg5000 commented Oct 1, 2024

Ok yes this is definitely a mystery. When I run with --debug it passes even without it next time. When I run clear-result-cache and run it fails.

@mglaman help! :)


195d218b8efa:/var/www/html/drush-dev/drush3$ ./vendor/bin/phpstan --configuration="phpstan.neon.dist" --debug
... 

 [OK] No errors



195d218b8efa:/var/www/html/drush-dev/drush3$ ./vendor/bin/phpstan clear-result-cache
Note: Using configuration file /var/www/html/drush-dev/drush3/phpstan.neon.dist.
Result cache cleared from directory: /tmp/phpstan

195d218b8efa:/var/www/html/drush-dev/drush3$ ./vendor/bin/phpstan --configuration="phpstan.neon.dist"
 210/210 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ---------------------------------------------------------------------
  Line   Commands/core/LanguageCommands.php
 ------ ---------------------------------------------------------------------
  160    Function _locale_translation_default_update_options not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
         ✏️  Commands/core/LanguageCommands.php
  163    Function locale_translation_batch_update_build not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
         ✏️  Commands/core/LanguageCommands.php
  165    Function locale_config_batch_update_components not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
         ✏️  Commands/core/LanguageCommands.php
 ------ ---------------------------------------------------------------------

@mglaman
Copy link
Contributor

mglaman commented Oct 6, 2024

I'm so confused. I wish I knew. Maybe it's something to do with the way the includes work. I'd have to run through with Xdebug.

So it always fails when cache disabled (--debug) correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants