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

Please use PHPStan #618

Closed
szepeviktor opened this issue Jul 15, 2024 · 11 comments · Fixed by #638
Closed

Please use PHPStan #618

szepeviktor opened this issue Jul 15, 2024 · 11 comments · Fixed by #638
Milestone

Comments

@szepeviktor
Copy link
Contributor

szepeviktor commented Jul 15, 2024

Describe the bug

There are nasty things in the code, e.g. return parent::__construct();

Steps to Reproduce

The simplest way.

composer require --dev szepeviktor/phpstan-wordpress
vendor/bin/phpstan analyze -c vendor/szepeviktor/phpstan-wordpress/extension.neon includes/ providers/ class-*.php two-factor.php --level=0
@dd32
Copy link
Member

dd32 commented Jul 15, 2024

Hi!

Can you double-check the command provided?

$ composer require --dev szepeviktor/phpstan-wordpress

./composer.json has been updated
Running composer update szepeviktor/phpstan-wordpress
Loading composer repositories with package information
Updating dependencies
Lock file operations: 3 installs, 0 updates, 0 removals
  - Locking php-stubs/wordpress-stubs (v6.5.3)
  - Locking phpstan/phpstan (1.11.7)
  - Locking szepeviktor/phpstan-wordpress (v1.3.5)
Writing lock file


$ vendor/bin/phpstan analyze -c vendor/szepeviktor/phpstan-wordpress/extension.neon includes/ providers/ class-*.php two-factor.php --level=0
 14/14 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 [OK] No errors

$ git remote -v
origin	[email protected]:WordPress/two-factor.git (fetch)
origin	[email protected]:WordPress/two-factor.git (push)

$ git branch
* master

@szepeviktor
Copy link
Contributor Author

This is surreal. 14/14: all files are checked 😲

I cannot find the reason why there are no errors.

@dd32
Copy link
Member

dd32 commented Jul 15, 2024

I fully suspect it's related to my environment somehow :)

Thank you for the PR, it should be enough to review the changes and have them merged without issue.

(we'll need to get the GitHub actions working prior to merge however, to ensure no regressions. I'm not requesting you do that, unless you feel like it!)

@szepeviktor
Copy link
Contributor Author

I'm contributing to PHPStan for years and I have no clue what is going on!

@kasparsd
Copy link
Collaborator

PHPstan reporting and internal cache is the only unreliably thing I haven't been able to solve.

@szepeviktor
Copy link
Contributor Author

PHPstan reporting and internal cache

Please do report your findings.
https://github.com/phpstan/phpstan/issues/new/choose
I need PHPStan to get better.

@kasparsd
Copy link
Collaborator

@szepeviktor I got the same results out of the box on my local PHP 7.4:

no-errors

Turns out it is running out of memory and exiting silently. Adding the --debug parameter disables the cache and it correctly reports memory issue:

PHPStan process crashed because it reached configured PHP memory limit: 128M
Increase your memory limit in php.ini or run PHPStan with --memory-limit CLI option.

@szepeviktor
Copy link
Contributor Author

configured PHP memory limit: 128M

@kasparsd This is shockingly low.

In Debian on PHP-CLI the default is unlimited memory for PHP.

@kasparsd
Copy link
Collaborator

@szepeviktor The core issue is that doesn't fail with that memory error on the first run. It somehow thinks every is a pass and then returns that cached result.

@szepeviktor
Copy link
Contributor Author

@ondrejmirtes Could you help us with an advice?

@ondrejmirtes
Copy link

@szepeviktor Hey, I'm not going to read the whole thread and trying to figure out what you're asking me. Please open a GitHub discussion and provide the full context. Thanks.

@jeffpaul jeffpaul added this to the 0.10.0 milestone Sep 19, 2024
@jeffpaul jeffpaul modified the milestones: 0.11.0, 0.10.0 Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants