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

4.17.0 scans vendor folder even when it is in ignored files #7289

Closed
SharkMachine opened this issue Jan 4, 2022 · 9 comments
Closed

4.17.0 scans vendor folder even when it is in ignored files #7289

SharkMachine opened this issue Jan 4, 2022 · 9 comments

Comments

@SharkMachine
Copy link

How to reproduce:

  1. Add composer.json and psalm.xml to an empty folder

composer.json

{
    "require-dev": {
        "vimeo/psalm": "4.17.0"
    },
    "config": {
        "allow-plugins": {
            "composer/package-versions-deprecated": false
        }
    }
}

psalm.xml

<?xml version="1.0"?>
<psalm xmlns="https://getpsalm.org/schema/config" xmlns:xi="http://www.w3.org/2001/XInclude">
    <projectFiles>
        <ignoreFiles>
            <directory name="vendor" />
        </ignoreFiles>
    </projectFiles>
</psalm>
  1. Run composer update and ./vendor/bin/psalm ./

Result with 4.17.0
image

Result with 4.16.1
image

@psalm-github-bot
Copy link

Hey @SharkMachine, can you reproduce the issue on https://psalm.dev ?

@orklah
Copy link
Collaborator

orklah commented Jan 4, 2022

unfortunately, this is expected behavior.

Psalm will not analyze the vendor dir if you ignored it. However, it still needs to know the types from the dependencies you use.

For example if you have

$var = Dependency::getSpecialResult();
assert(is_int($var));

Psalm needs to be able to check if you fetched an int or not from the dependency.

For that, it performs a quick scan of the phpdoc and signatures of all the files you actually use as a dependency. If it can't read those, Psalm is unable to work properly (In the example above, what would you expect to have as a result of Psalm having no idea what's in the result?). So when that happens, it throws an InvalidDocblock, even in ignored files.

This is not a new behavior either. What changed is that we tweaked our own dependencies in the last release and it must have changed your own resolved dependencies by Composer and retrieved (newer and) flawed versions.

I pushed a first PR for the second error, it seems to be a typo. However, the first one is more tricky, it uses a relatively new syntax from phpstan array{} who is used to describe an empty array. Psalm can't read that for now so we're working on a new release that will be able to.

@SharkMachine
Copy link
Author

For me it looks like something else has changed as well as 4.17.0 tells me it's analyzing 1779 files and 4.16.1 tells me that no files were analyzed. Analyzing also takes over 30 seconds longer with 4.17.0 than with 4.16.1, and it takes less than a second with 4.16.1.

This is the case when the only dependency is Psalm itself as a dev dependency.

@weirdan
Copy link
Collaborator

weirdan commented Jan 4, 2022

@SharkMachine do those numbers come from a --no-cache run? No files analyzed usually means Psalm detected no changes in your codebase since the last run, so it just returns a cached report. No wonder that it's faster.

@SharkMachine
Copy link
Author

@weirdan Yes, here's the output from both runs:

4.16.1

$ ./vendor/bin/psalm --no-cache ./
Target PHP version: 8.0 (inferred from current PHP version)
Scanning files...
Analyzing files...


------------------------------
                              
       No errors found!       
                              
------------------------------

Checks took 0.53 seconds and used 39.085MB of memory
No files analyzed
Psalm was unable to infer types in the codebase

4.17.0

$ ./vendor/bin/psalm --no-cache ./
Target PHP version: 8.0 (inferred from current PHP version)
Scanning files...
Analyzing files...

                                                                                
ERROR: InvalidDocblock - vendor/composer/InstalledVersions.php:31:5 - array{root: array{name: string, version: string, reference: string, pretty_version: string, aliases: string[], dev: bool, install_path: string, type: string}, versions: array<string, array{dev_requirement: bool, pretty_version?: string, version?: string, aliases?: string[], reference?: string, replaced?: string[], provided?: string[], install_path?: string, type?: string}>}|array{}|null is not a valid type (from /home/sharkma/Documents/Projects/psalm-test/vendor/composer/InstalledVersions.php:27) (see https://psalm.dev/008)
    /**
     * @var mixed[]|null
     * @psalm-var array{root: array{name: string, version: string, reference: string, pretty_version: string, aliases: string[], dev: bool, install_path: string, type: string}, versions: array<string, array{dev_requirement: bool, pretty_version?: string, version?: string, aliases?: string[], reference?: string, replaced?: string[], provided?: string[], install_path?: string, type?: string}>}|array{}|null
     */
    private static $installed;


------------------------------
1 errors found
------------------------------

Checks took 32.08 seconds and used 644.101MB of memory
Psalm was unable to infer types in the codebase

@weirdan
Copy link
Collaborator

weirdan commented Jan 4, 2022

Mind running that without ./?

@SharkMachine
Copy link
Author

Thanks. That did the trick and 4.17 doesn't analyze vendor folder any more.

@orklah
Copy link
Collaborator

orklah commented Jan 4, 2022

My bad, my first explanation was wrong then. At least we found two errors in third packages :)

@orklah orklah closed this as completed Jan 4, 2022
@weirdan
Copy link
Collaborator

weirdan commented Jan 4, 2022

A little explanation is warranted here. When you pass a path argument, Psalm is supposed to ignore any file filters you may have in your config file. Historically we had a bug that made Psalm ignore ./ argument - it behaved as if there was no argument at all.

#7210 dropped the buggy part, so the bug was accidentally fixed (thanks @vstm).

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

No branches or pull requests

3 participants