vimeo / psalm

A static analysis tool for finding errors in PHP applications
https://psalm.dev
MIT License
5.57k stars 659 forks source link

Vendor directory being scanned on every run + other vendor bugs #6751

Closed Stevemoretz closed 3 years ago

Stevemoretz commented 3 years ago

Here's my config:

<?xml version="1.0"?>
<psalm
        autoloader="wp-content/mu-plugins/fast/Lumen/vendor/autoload.php"
>
    <projectFiles>
        <directory name="wp-content/mu-plugins/fast/Lumen/packages/stevemoretz/tools/src/Utils/MultiDrivers/Managers/" />
        <ignoreFiles>
            <directory name="wp-content/mu-plugins/fast/Lumen/vendor"/>
            <directory name="vendor"/>
        </ignoreFiles>
    </projectFiles>
</psalm>

I use a custom autoloader which I have defined in the xml under the main entry. Let's see the output :

steve@MacBook-Pro wordpress-clean % vendor/bin/psalm --config=psalm.xml --debug --threads=1 
Composer lockfile change detected, clearing cache
Scanning files...
Registering autoloaded files
   /Volumes/HDD/Websites/wordpress-clean/vendor/autoload.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/composer/autoload_real.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/composer/ClassLoader.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/composer/autoload_static.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/symfony/polyfill-ctype/bootstrap.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/phpstan/phpstan/bootstrap.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/symfony/polyfill-php80/bootstrap.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/symfony/polyfill-mbstring/bootstrap.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/amphp/amp/lib/functions.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/amphp/amp/lib/Internal/functions.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/symfony/polyfill-intl-normalizer/bootstrap.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/symfony/polyfill-intl-grapheme/bootstrap.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/symfony/deprecation-contracts/function.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/symfony/polyfill-php73/bootstrap.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/symfony/string/Resources/functions.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/amphp/byte-stream/lib/functions.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/vimeo/psalm/src/functions.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/vimeo/psalm/src/spl_object_id.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/composer/package-versions-deprecated/src/PackageVersions/Versions.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/composer/InstalledVersions.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/composer/installed.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/composer/autoload_files.php

First of all it is still using /Volumes/HDD/Websites/wordpress-clean/vendor instead of wp-content/mu-plugins/fast/Lumen/vendor.

A little down the logs:

Deep scanning /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/autoload.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/composer/autoload_real.php
Deep scanning /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/composer/autoload_real.php

It starts scanning the right folder. But the main problem is that it does this every time, this makes my development very slow because I use IntellijIdea with Jetbrain's Psalm plugin, which runs the same command and it takes the same amount of time for every Ctrl + S to show the errors and warnings.

Why would everything be scanned again, I thought this has a cache, if it doesn't work what's the point of cache?

Notice I'm only including one directory which has a very simple file in it, I tried another directory which has a bunch of classes that have extend and oop structure a lot and in there it takes Checks took 45.73 seconds and used 536.583MB of memory. which is 9 times worse.

Stevemoretz commented 3 years ago

Changing the directory to the second one now cache happens, but the weird problem is that if I edit any included files (only one file and a small change for example) it would have to rescan everything again (of course including vendors).

Wish that could recognize the change and only react on the changed files.

Is that possible?

orklah commented 3 years ago

Psalm needs to analyze files you included in your autoload file. If it didn't, it would miss possibly structural files from your project due to them being included in the execution before the beginning of the analysis

Secondly, the files from /Volumes are files your autoloader included, I would not expect Psalm to go look for them elsewhere. Maybe change the way your autoloader works if you want to change paths?

Finally, Psalm will perform a light scan on any file you depend in your code to retrieve types (Otherwise, how do you expect it to know that "ThirdPartyLib::anyMethod" takes an int and return a string?)

Now, about cache, you'll notice your log begins with Composer lockfile change detected, clearing cache. Psalm has a safety that clears cache when dependencies are updated or changed. This is know to cause issues for non-standard installation with composer. I suggested a course of action here: https://github.com/vimeo/psalm/issues/6664. If you're up for a PR, you should look that way

Stevemoretz commented 3 years ago

Thank you for the response.

Psalm needs to analyze files you included in your autoload file. If it didn't, it would miss possibly structural files from your project due to them being included in the execution before the beginning of the analysis

I didn't say it should never do it, I just mean why should it do over and over on a single file change, why doesn't it just cache it, if it's smart enough to know the composer.lock changes.

Secondly, the files from /Volumes are files your autoloader included, I would not expect Psalm to go look for them elsewhere. Maybe change the way your autoloader works if you want to change paths?

I have a bunch of autoloaders, the defined one in xml is : autoloader="wp-content/mu-plugins/fast/Lumen/vendor/autoload.php" but clearly psalm is also autoloading the wp-content/mu-plugins/vendor/autoload.php. Sounds like a bug to me...

And I can't change the way my autoloader works, it's a complex project based on a combination of laravel and wordpress and wordpress plugins also have their own autoloaders, so I need to use multiple autoloaders. (which is another issue, I don't know how to do that?)

Finally, Psalm will perform a light scan on any file you depend in your code to retrieve types (Otherwise, how do you expect it to know that "ThirdPartyLib::anyMethod" takes an int and return a string?)

Sure it must do that, and that wasn't my concern.

Now, about cache, you'll notice your log begins with Composer lockfile change detected, clearing cache. Psalm has a safety that clears cache when dependencies are updated or changed. This is know to cause issues for non-standard installation with composer. I suggested a course of action here: #6664. If you're up for a PR, you should look that way

True I didn't notice that lockfile change, but let me show you another example.


Here's the output when no files change, which goes ahead and shows the errors great so caching works for this purpose.

steve@MacBook-Pro wordpress-clean % vendor/bin/psalm --config=psalm.xml --debug --threads=1
Scanning files...
0 changed files: 

Analyzing files...
Checking class references

ERROR: ImplementedReturnTypeMismatch - wp-content/mu-plugins/fast/Lumen/app/Models/Guest.php:10:19 - The inherited return type 'Illuminate\Database\Eloquent\Builder' for Illuminate\Database\Eloquent\Model::newQuery is different to the implemented return type for Illuminate\Database\Eloquent\Model::newquery 'App\Models\Guest|Illuminate\Database\Eloquent\Builder' (see https://psalm.dev/123)
/**
 * App\Guest
 *

I put one space in that file let's look at the results:

steve@MacBook-Pro wordpress-clean % vendor/bin/psalm --config=psalm.xml --debug --threads=1
Scanning files...
1 changed files: 
    /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/app/Models/Guest.php
Registering autoloaded files
   /Volumes/HDD/Websites/wordpress-clean/vendor/autoload.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/composer/autoload_real.php
   /Volumes/HDD/Websites/wordpress-clean/vendor/composer/ClassLoader.php
....

So far we see even though it knows the composer.lock hasn't changed and only 1 file has changed it is however Registering autoloaded files which sounds to me, an extra step because composer.lock is not changed so why do that again?

then this happens

Deep scanning /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/composer/ClassLoader.php
Using reflection to get metadata for InvalidArgumentException
Using reflection to get metadata for Closure
Using composer to locate file for Symfony\Polyfill\Ctype\Ctype
Using reflection to get metadata for Exception
Using composer to locate file for Symfony\Polyfill\Php80\Php80
Using composer to locate file for Symfony\Polyfill\Mbstring\Mbstring
Using reflection to get metadata for Generator
Using composer to locate file for Amp\Promise
Using reflection to get metadata for Throwable
Using composer to locate file for Amp\Failure
Using composer to locate file for Amp\Coroutine
... A million more of these
ClassLikeStorage is populating
Have populated ComposerAutoloaderInitf32e81cd9df0d6bb5bda1181a7fcb0d9
Have populated Composer\Autoload\ClassLoader
Have populated Composer\Autoload\ComposerStaticInitf32e81cd9df0d6bb5bda1181a7fcb0d9
Have populated PHPStan\PharAutoloader
... A million more of these
ClassLikeStorage is populated
FileStorage is populating
FileStorage is populated
Finished registering stub files

Finally

Analyzing files...
Getting /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/app/Models/Guest.php
Analyzing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/app/Models/Guest.php
Skipping analysis of pre-analyzed method App\Models\Guest::notifications
Skipping analysis of pre-analyzed method App\Models\Guest::readnotifications
Skipping analysis of pre-analyzed method App\Models\Guest::unreadnotifications
Skipping analysis of pre-analyzed method App\Models\Guest::notify
Skipping analysis of pre-analyzed method App\Models\Guest::notifynow
Skipping analysis of pre-analyzed method App\Models\Guest::routenotificationfor
Skipping analysis of pre-analyzed method App\Models\Guest::pushsubscriptions
Skipping analysis of pre-analyzed method App\Models\Guest::updatepushsubscription
Skipping analysis of pre-analyzed method App\Models\Guest::ownspushsubscription
Skipping analysis of pre-analyzed method App\Models\Guest::deletepushsubscription
Skipping analysis of pre-analyzed method App\Models\Guest::routenotificationforwebpush
Skipping analysis of pre-analyzed method App\Models\Guest::now
Skipping analysis of pre-analyzed method App\Models\Guest::eee
Skipping analysis of pre-analyzed method App\Models\Guest::id
Skipping analysis of pre-analyzed method App\Models\Guest::nowid
Skipping analysis of pre-analyzed method App\Models\Guest::getauthidentifier
Skipping analysis of pre-analyzed method App\Models\Guest::getauthidentifiername
Skipping analysis of pre-analyzed method App\Models\Guest::getauthpassword
Skipping analysis of pre-analyzed method App\Models\Guest::getremembertoken
Skipping analysis of pre-analyzed method App\Models\Guest::setremembertoken
Skipping analysis of pre-analyzed method App\Models\Guest::getremembertokenname
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Model.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Model.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/HasEvents.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Model.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/HasAttributes.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Model.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/GuardsAttributes.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/GuardsAttributes.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/GuardsAttributes.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/HasAttributes.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/HasAttributes.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Model.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Model.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/HasEvents.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Model.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/HasAttributes.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Model.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/GuardsAttributes.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/GuardsAttributes.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/GuardsAttributes.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/HasAttributes.php
Parsing /Volumes/HDD/Websites/wordpress-clean/wp-content/mu-plugins/fast/Lumen/vendor/illuminate/database/Eloquent/Concerns/HasAttributes.php
Checking class references

ERROR: ImplementedReturnTypeMismatch - wp-content/mu-plugins/fast/Lumen/app/Models/Guest.php:10:19 - The inherited return type 'Illuminate\Database\Eloquent\Builder' for Illuminate\Database\Eloquent\Model::newQuery is different to the implemented return type for Illuminate\Database\Eloquent\Model::newquery 'App\Models\Guest|Illuminate\Database\Eloquent\Builder' (see https://psalm.dev/123)
/**

Too many extra things happened because of the autoloader being scanned again, without composer.lock being changed. That is what I'm concerned about!

orklah commented 3 years ago

Those checks should be pretty quick. Can you tell us what the time was when you just changed a space? Can you try to profile a psalm run (you can use blackfire for that) to see what takes time?

Stevemoretz commented 3 years ago

Those checks should be pretty quick. Can you tell us what the time was when you just changed a space? Can you try to profile a psalm run (you can use blackfire for that) to see what takes time?

Sure it takes 5-10 seconds, which is not bad at all if you run it once you wanna run it, the problem is that JetBrains has this tool integrated with their ide, and there it becomes slow.

Anyways I changed my source folder again to a bigger portion and now it doesn't generate autoloaders everytime which is perfect!

Any idea why it was doing that? The previous folder which only had one file in it, is now a subset of the selected folders and surprisingly instead of getting slower, it got a lot better and generation of autoloaders don't happen on file changes anymore.( I'm changing the exact same file with adding and removing spaces again)

Now it takes 1-2 seconds instead of 5-10.

Fantastic, thank you so much! I think there is a bug somewhere, when you select only one folder with one file? I'm not sure I'm just happy that wasn't intended to happen.


PS: about what you said, no those checks actually get the most time, I could see it in the terminal, because I'm using laravel under the hood and it has a big vendor folder.

orklah commented 3 years ago

Not sure if that was intended and you found a way around that or if it was a bug. If your issue is fixed, I'll close this :)