Skip to content

Conversation

@szepeviktor
Copy link

@szepeviktor szepeviktor commented Nov 10, 2022

All Submissions:

Signed-off-by: Viktor Szépe <viktor@szepe.net>
@mscurtescu
Copy link
Contributor

Thanks again @szepeviktor

Merging here on dev, will later merge into main.

@mscurtescu mscurtescu merged commit 7936b6e into hellocoop:dev Nov 13, 2022
@szepeviktor szepeviktor deleted the patch-3 branch November 13, 2022 17:52
@mscurtescu
Copy link
Contributor

mscurtescu commented Apr 26, 2023

Hi @szepeviktor ,

I finally merged your simplification into main and started using it, thanks again.

Running into an issue with phpstan that I cannot figure, as far as I can tell it properly pulls in the WordPress stubs but it seems to be confused about number of parameters:

  236    Function site_url invoked with 1 parameter, 0 required.   
  337    Function admin_url invoked with 1 parameter, 0 required.  
  573    Function get_bloginfo invoked with 1 parameter, 0 required.                           
  574    Function esc_attr invoked with 1 parameter, 0 required.                               

It's not complaining about all WordPress functions, only some.

For now I am ignoring these specific errors so I can see the real issues being brought up.

Any idea where is this coming from? Some extra libraries being imported unnecessarily? Thanks in advance

Link to phpstan.neon.dist

@szepeviktor
Copy link
Author

Investigating 🏃🏻‍♂️

@szepeviktor
Copy link
Author

On PHP 7.4 I get

 -- -----------------------------------------------------------------------------------------------------------
     Error
 -- -----------------------------------------------------------------------------------------------------------
     Ignored error pattern #^Function [^ ]+ invoked with [1234] parameters?, 0 required\.$# was not matched in
     reported errors.
 -- -----------------------------------------------------------------------------------------------------------

@szepeviktor szepeviktor mentioned this pull request Apr 26, 2023
@mscurtescu
Copy link
Contributor

I was getting the errors when running locally, either:

grunt shell:phpstan

or directly:

./vendor/bin/phpstan analyze

@szepeviktor
Copy link
Author

szepeviktor commented Apr 27, 2023

Try clearing composer cache, removing vendor/ and doing a composer update.

@mscurtescu
Copy link
Contributor

clearing the cache made no difference :-(

@szepeviktor
Copy link
Author

szepeviktor commented Apr 27, 2023

I see.

All I can see static analysis here on GitHub Actions recognizes those functions.
(and on my Debian/Linux computer)

@mscurtescu
Copy link
Contributor

Thanks, your help is much appreciated. Will try on a different computer with a clean state.

@szepeviktor
Copy link
Author

szepeviktor commented Apr 27, 2023

I wish you to keep all of your computers clean all the time.

@mscurtescu
Copy link
Contributor

On a clean computer it works as expected, thanks for your help. Yes, I have to figure what exactly is messing with phpstan.

@mscurtescu
Copy link
Contributor

mscurtescu commented May 3, 2023

Really puzzling, still running into this error on one computer:

  • run ./vendor/bin/phpstan clear-result-cache (this is not documented when running phpstan -h)
  • run composer clear-cache
  • brand new git clone in a new folder

Since I am seeing the error in a brand new git clone the cause must reside outside of the git repo folder. Beyond phpstan and composer caches not sure what it can be. I also nuked the npm cache, but I don't see how that would be related anyhow.

Also, using --debug with phpstan is not providing much insight, it only shows what files are being analysed and what configuration file was used. Where are the WordPress stubs loaded from? I assume through composer from the vendor subfolder, but deleting that folder made no difference.

@szepeviktor
Copy link
Author

Where are the WordPress stubs loaded from?

From the extension's configuration: https://github.com/szepeviktor/phpstan-wordpress/blob/master/extension.neon#L116

@mscurtescu
Copy link
Contributor

Where are the WordPress stubs loaded from?

From the extension's configuration: https://github.com/szepeviktor/phpstan-wordpress/blob/master/extension.neon#L116

So that's under vendor, a brand new git clone, or removing the vendor folder, should take care of a bad cache, and that's not the case. Is there any chance that stubs are also loaded from some other location?

@szepeviktor
Copy link
Author

Is there any chance that stubs are also loaded from some other location?

No, it is loaded from the extension.

@mscurtescu
Copy link
Contributor

Maybe I am looking at it the wrong way and it's not a caching issue, what would generate an error like:

Function admin_url invoked with 1 parameter, 0 required

How does phpstan determine that admin_url requires 0 parameters? I checked the stub and the stub has the expected signature which is:

function admin_url($path = '', $scheme = 'admin')
{
}

@szepeviktor
Copy link
Author

Search for function admin_url in every file.
grep -rFn 'function admin_url'
If there is another definition than that is the cause.
Otherwise it is a PHPStan error.

@mscurtescu
Copy link
Contributor

$ find . -name \*.php -exec grep -iHn "function admin_url" {} \;
./vendor/php-stubs/wordpress-stubs/wordpress-stubs.php:110462:    function admin_url($path = '', $scheme = 'admin')
./wordpress/wp-includes/link-template.php:3522:function admin_url( $path = '', $scheme = 'admin' ) {
./wordpress/wp-admin/includes/noop.php:80:function admin_url() {}

This is the suspicious one: ./wordpress/wp-admin/includes/noop.php:80:function admin_url() {}

I deleted the whole wordpress folder, now phpstan runs as expected.

Questions:

  1. Why is phpstan loading this folder?
  2. Checking what is different on my other computer where it worked from the beginning...

@mscurtescu
Copy link
Contributor

Answering question 2, on the machine where phpstan worked, there is also the exact same ./wordpress/wp-admin/includes/noop.php:80:function admin_url() {}, mind bugling

@szepeviktor
Copy link
Author

szepeviktor commented May 4, 2023

PHPStan should be instructed to scan that file in phpstan.neon.dist

@szepeviktor
Copy link
Author

Here is our perpetrator!!!

"wordpress/"

@mscurtescu
Copy link
Contributor

The content of phpstan.neon.dist:

parameters:
  level: 5
  inferPrivatePropertyTypeFromConstructor: true
  bootstrapFiles:
    - tests/phpstan-bootstrap.php
  paths:
    - hello-login.php
    - includes/

@szepeviktor
Copy link
Author

The cause is in the previous comment!

@mscurtescu
Copy link
Contributor

Here is our perpetrator!!!

"wordpress/"

So on some machines classes are loaded in some order and on other machines in a different one?

@szepeviktor
Copy link
Author

So on some machines classes are loaded in some order and on other machines in a different one?

  • composer install
  • composer install --no-dev

That makes the difference.

@mscurtescu
Copy link
Contributor

The cause is in the previous comment!

Yes, removing autoload-dev for now, it was inherited from the parent project (the one this project forked). Thanks.

@mscurtescu
Copy link
Contributor

So on some machines classes are loaded in some order and on other machines in a different one?

  • composer install
  • composer install --no-dev

That makes the difference.

I don't think it's that, running composer install --no-dev will not download and install the wordpress folder, that definitely helps. but on my other machine the wordpress folder was installed and it still works, so it must be class loading order.

@mscurtescu
Copy link
Contributor

adding:

  excludePaths:
  	- wordpress/

also works, yay!

@mscurtescu
Copy link
Contributor

köszönöm szépen

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 this pull request may close these issues.

2 participants