vimeo / psalm

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

Latest Psalm release scan vendor when it should not and fails there #7332

Closed reypm closed 2 years ago

reypm commented 2 years ago

I have upgraded my Psalm to the latest 4.18 and now it scans the vendor directory when it should not as per the psalm.xml (see below) file configuration.

Issues found:

Note: the same configuration works fine with Psalm 4.16.1

The error reads as below:

>>  ../framework
Target PHP version: 8.1 (inferred from composer.json)
Scanning files...
Analyzing files...

ERROR: ParseError - vendor/doctrine/mongodb-odm/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/GraphLookup/Match.php:15:7 - Syntax error, unexpected T_MATCH, expecting T_STRING on line 15 (see https://psalm.dev/173)
class Match extends MatchStage

ERROR: ParseError - vendor/doctrine/mongodb-odm/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/GraphLookup/Match.php:29:1 - Syntax error, unexpected '}', expecting EOF on line 29 (see https://psalm.dev/173)
}

ERROR: ParseError - vendor/doctrine/mongodb-odm/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Match.php:14:7 - Syntax error, unexpected T_MATCH, expecting T_STRING on line 14 (see https://psalm.dev/173)
class Match extends MatchStage

ERROR: ParseError - vendor/doctrine/mongodb-odm/lib/Doctrine/ODM/MongoDB/Aggregation/Stage/Match.php:28:1 - Syntax error, unexpected '}', expecting EOF on line 28 (see https://psalm.dev/173)
}

ERROR: ParseError - vendor/jms/serializer/src/Annotation/ReadOnly.php:14:13 - Syntax error, unexpected T_READONLY, expecting T_STRING on line 14 (see https://psalm.dev/173)
final class ReadOnly extends ReadOnlyProperty

ERROR: ParseError - vendor/rector/rector/vendor/rector/rector-generator/templates/rules/__Package__/Rector/__Category__/__Configured__Name__.php:19:5 - Syntax error, unexpected T_STRING on line 19 (see https://psalm.dev/173)
    __ConfigurationProperties__

ERROR: ParseError - vendor/rector/rector/vendor/rector/rector-generator/templates/rules/__Package__/Rector/__Category__/__Configured__Name__.php:50:5 - Syntax error, unexpected T_STRING on line 50 (see https://psalm.dev/173)
    __ConfigureClassMethod__

------------------------------
7 errors found
------------------------------
99 other issues found.
You can display them with --show-info=true
------------------------------
Psalm can automatically fix 5 of these issues.
Run Psalm again with
--alter --issues=InvalidNullableReturnType,InvalidFalsableReturnType --dry-run
to see what it can fix.
------------------------------

Checks took 378.43 seconds and used 5,587.343MB of memory
Psalm was able to infer types for 95.2732% of the codebase

Below is some helpful information to try to fix this in the case is a bug:

PHP Information:

# php -v
PHP 8.1.1 (cli) (built: Dec 21 2021 03:37:39) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.1, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.1, Copyright (c), by Zend Technologies
    with blackfire v1.72.0~linux-x64-non_zts81, https://blackfire.io, by Blackfire

psalm.xml

<?xml version="1.0"?>
<psalm xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" errorLevel="7" totallyTyped="true"
       strictBinaryOperands="true" useDocblockPropertyTypes="true"
       xmlns="https://getpsalm.org/schema/config"
       xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd">
    <projectFiles>
        <directory name=".."/>
        <ignoreFiles>
            <directory name="*vendor"/>
            <directory name="*var"/>
            <directory name="*features/"/>
            <file name="phpinsights.php"/>
            <file name="rector.php"/>
        </ignoreFiles>
    </projectFiles>

    <plugins>
        <pluginClass class="Psalm\SymfonyPsalmPlugin\Plugin">
            <containerXml>var/cache/dev/App_KernelDevDebugContainer.xml</containerXml>
        </pluginClass>
        <pluginClass class="Weirdan\DoctrinePsalmPlugin\Plugin"/>
    </plugins>

    <issueHandlers>
        <MethodSignatureMismatch errorLevel="suppress"/>
        <UndefinedDocblockClass errorLevel="suppress"/>
        <MissingFile errorLevel="suppress"/>
        <UndefinedClass errorLevel="suppress"/>
        <PluginIssue name="RepositoryStringShortcut" errorLevel="suppress" />
    </issueHandlers>
</psalm>

Note: I have tried removing the * from the vendor path and the error is the same

composer.json

{
  "name": "xyz",
  "description": "xyz",
  "type": "project",
  "license": "proprietary",
  "prefer-stable": true,
  "require": {
    "php": "^8.1",
    "ext-json": "*",
    "ext-mongodb": "*",
    "ext-redis": "*",
    "ext-amqp": "*",
    "doctrine/mongodb-odm-bundle": "^4.3",
    "firebase/php-jwt": "^v5.5",
    "jms/serializer-bundle": "^4.0",
    "php-pm/httpkernel-adapter": "^2.0",
    "php-pm/php-pm": "^2.2",
    "ramsey/uuid": "^4.2",
    "sensio/framework-extra-bundle": "^v6.1",
    "sentry/sentry-symfony": "^4.2",
    "snc/redis-bundle": "^3.6",
    "stof/doctrine-extensions-bundle": "^v1.7",
    "symfony/console": "^5.4",
    "symfony/dotenv": "^5.4",
    "symfony/expression-language": "^5.4",
    "symfony/flex": "^v2.0",
    "symfony/framework-bundle": "^5.4",
    "symfony/messenger": "^5.4",
    "symfony/monolog-bundle": "^3.3",
    "symfony/runtime": "^5.4",
    "symfony/security-bundle": "^5.4",
    "symfony/validator": "^5.4"
  },
  "require-dev": {
    "behat/behat": "^v3.10",
    "blackfire/php-sdk": "^v1.27",
    "composer/package-versions-deprecated": "^1.11",
    "friends-of-behat/symfony-extension": "^2.3",
    "friendsofphp/php-cs-fixer": "v3.4",
    "guzzlehttp/guzzle": "^7.4",
    "insolita/unused-scanner": "^2.3",
    "mongodb/mongodb": "dev-master",
    "nunomaduro/phpinsights": "^2.0",
    "phan/phan": "^5.3",
    "phpstan/extension-installer": "^1.1",
    "phpstan/phpdoc-parser": "^1.2",
    "phpstan/phpstan": "^1.3.3",
    "phpstan/phpstan-deprecation-rules": "^1.0",
    "phpstan/phpstan-doctrine": "^1.0",
    "phpstan/phpstan-php-parser": "^1.0",
    "phpstan/phpstan-strict-rules": "^1.0",
    "phpstan/phpstan-symfony": "^1.0",
    "psalm/plugin-symfony": "^3.0",
    "rector/rector": "^0.12",
    "roave/security-advisories": "dev-latest",
    "symfony/stopwatch": "^5.4",
    "symfony/thanks": "^v1.2",
    "symplify/composer-json-manipulator": "^10.0",
    "vimeo/psalm": "^4.18",
    "weirdan/doctrine-psalm-plugin": "^1.1"
  },
  "config": {
    "preferred-install": {
      "*": "dist"
    },
    "sort-packages": true,
    "optimize-autoloader": true,
    "process-timeout": 0,
    "allow-plugins": {
      "composer/package-versions-deprecated": true,
      "symfony/flex": true,
      "symfony/thanks": true,
      "dealerdirect/phpcodesniffer-composer-installer": true,
      "phpstan/extension-installer": true,
      "symfony/runtime": true
    }
  },
  "autoload": {
    "psr-4": {
      "App\\": "src/"

    }
  },
  "autoload-dev": {
    "psr-4": {
      "Behat\\Behat\\": "features/bootstrap/"
    }
  },
  "replace": {
    "paragonie/random_compat": "2.*",
    "symfony/polyfill-ctype": "*",
    "symfony/polyfill-iconv": "*",
    "symfony/polyfill-php72": "*",
    "symfony/polyfill-php71": "*",
    "symfony/polyfill-php70": "*",
    "symfony/polyfill-php56": "*"
  },
  "scripts": {
    "psalm": "find ../* -prune -type d | while IFS= read -r DIR; do echo '>> ' ${DIR}; ./vendor/bin/psalm ${DIR}; done",
    "behat": "APP_ENV=dev ./vendor/bin/behat --suite=default --format=progress -o std -f junit -o test_results",
    "auto-scripts": {
      "cache:clear": "symfony-cmd",
      "assets:install %PUBLIC_DIR%": "symfony-cmd"
    },
    "post-install-cmd": [
      "@auto-scripts"
    ],
    "post-update-cmd": [
      "@auto-scripts"
    ]
  },
  "conflict": {
    "symfony/symfony": "*"
  },
  "extra": {
    "symfony": {
      "allow-contrib": true,
      "require": "5.4.*"
    }
  }
}
orklah commented 2 years ago

As discussed in Slack, the basic psalm command runs fine without arguments. The error happens when running the command through composer with find ../* -prune -type d | while IFS= read -r DIR; do echo '>> ' ${DIR}; ./vendor/bin/psalm ${DIR}; done

I believe this may be related to https://github.com/vimeo/psalm/pull/7210

vstm commented 2 years ago

Yes it could be related to #7210. I assume ../framework is some shell script which calls psalm? Does this shell script call psalm in a different way than the composer script does?

If it is called the same way as in the composer script it shouldn't have worked before as well (there's multiple paths and it's not . or ./ so it would have ignored the projectFiles).

The rule is basically: if you supply any paths/files via the command line the projectFiles definition in the psalm.xml (including the ignoreFiles) is not applied.

orklah commented 2 years ago

I assume ../framework is some shell script which calls psalm

I was told it's just the path. The command is the one in the composer command

The rule is basically: if you supply any paths/files via the command line the projectFiles definition in the psalm.xml (including the ignoreFiles) is not applied.

Oh right, I forgot that.

@reypm what is the goal to have the path in argument in your composer script?

vstm commented 2 years ago

Ah now I get it yes, it's the output from the composer script, and I didn't read it correctly - it calls psalm multiple times but with just one argument. So now that I'm not confused anymore I can say for sure that it's because of #7210.

The "../framework" is the . and was filtered out before 4.17 so the projectFiles of the psalm.xml was applied for this case so that's why it worked before (for the framework directory at least, it would have still scanned the vendors in the other "sibling" directories).

So yes as @orklah said, it's best to run psalm without any arguments so the composer script would just be

"psalm": "./vendor/bin/psalm"

Now if you have vendor directories in other sibling directories than just ../framework I would suggest to use this glob:

<directory name="../*/vendor"/>

This would exclude all sibling vendor directories, because the glob *vendor just excludes the vendor directory in ../framework, since the paths in projectFiles are alway relative to the path where the psalm.xml is.

reypm commented 2 years ago

@orklah, @vstm the idea with the command and the path is to move into different directories. See for example the following project structure:

my-app/
├─ .git/
├─ Dockerfiles/
├─ Folder1/
├─ framework/
│  ├─ config/
│  ├─ features/
│  ├─ src/
│  ├─ var/
│  ├─ vendor/
├─ Folder 2/
├─ .gitignore
├─ README.md

First of all, I will start saying psalm is living inside framework/vendor/bin. I would like to scan the following dirs: Folder1, framework/src, Folder2 and the only way I have found to do so is through that script line and adding the path to the psalm command.

If I run just psalm it will scan|avoid only what is inside framework and will skip everything else, I have tried this and yes, psalm works fine if no path is supplied while running the command.

Can any of you think in a different way to achieve this? 🤔

vstm commented 2 years ago

Thank you for the explanation. It should be possible to do this with just the psalm.xml config. The question is where is your psalm.xml located in the project structure, is it also in the framework folder?

All the paths in the projectFiles are relative to the psalm.xml, so that's why it's important where it is located.

So if your psalm.xml is in the framework directory it actually should work like you would expect. I tried to replicate your structure and psalm scans also in the other directories (I have included the exact xml config in the output):

➜  framework ./vendor/bin/psalm --version 
Psalm 4.18.0.0
➜  framework ./vendor/bin/psalm          
Target PHP version: 7.4 (inferred from current PHP version)
Scanning files...
Analyzing files...

ERROR: UndefinedGlobalVariable - ../Folder1/Fail.php:1:6 - Cannot find referenced variable $fail in global scope (see https://psalm.dev/127)
?php $fail->fail(); 

ERROR: UndefinedGlobalVariable - ../Folder2/Fail.php:1:6 - Cannot find referenced variable $fail in global scope (see https://psalm.dev/127)
?php $fail->fail(); 

ERROR: UndefinedGlobalVariable - src/Fail.php:1:6 - Cannot find referenced variable $fail in global scope (see https://psalm.dev/127)
?php $fail->fail(); 

------------------------------
3 errors found
------------------------------
3 other issues found.
You can display them with --show-info=true
------------------------------

Checks took 0.01 seconds and used 4.939MB of memory
No files analyzed
Psalm was able to infer types for 0.0000% of the codebase
➜  framework cat psalm.xml 
<?xml version="1.0"?>
<psalm xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" errorLevel="7" totallyTyped="true"
   strictBinaryOperands="true" useDocblockPropertyTypes="true"
   xmlns="https://getpsalm.org/schema/config"
   xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd">
    <projectFiles>
        <directory name=".." />
        <ignoreFiles>
            <directory name="../.git" />
            <directory name="../Dockerfiles" />
            <directory name="vendor" />
            <directory name="features" />
            <directory name="var" />
        </ignoreFiles>
    </projectFiles>
</psalm>
➜  framework tree  -L 3  ../../my-app
../../my-app
├── Dockerfiles
├── Folder1
│   └── Fail.php
├── Folder2
│   └── Fail.php
├── framework
│   ├── composer.json
│   ├── composer.lock
│   ├── config
│   ├── features
│   │   └── Fail.php
│   ├── psalm.xml
│   ├── src
│   │   └── Fail.php
│   ├── var
│   │   └── Fail.php
│   └── vendor
│       ├── ... omitted
└── psalm.xml
reypm commented 2 years ago

@vstm thank you a lot, gonna give this a try and will see if it works then if everything is fine will close this issue.

reypm commented 2 years ago

@vstm in my case seems not to be working ... :( I have exactly what you have and psalm seems to be scanning only what is inside framework but not outside of that directory:

root@3ff6e051f8c6:/var/www/framework# composer psalm
> ./vendor/bin/psalm
Target PHP version: 8.1 (inferred from composer.json)
Scanning files...
Analyzing files...

------------------------------

       No errors found!

------------------------------
65 other issues found.
You can display them with --show-info=true
------------------------------

Checks took 0.01 seconds and used 12.380MB of memory
No files analyzed
Psalm was able to infer types for 90.8057% of the codebase

any ideas in what could be wrong here?

vstm commented 2 years ago

Oh, at the moment I have no idea. It seems that it doesn't even scan the framework folder now (It says "No files analyzed " in the output). Can you do a cat /var/www/framework/psalm.xml and give me the output so I can see how your psalm.xml looks now?

reypm commented 2 years ago

@vstm here it is ...

<?xml version="1.0"?>
<psalm xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" errorLevel="7" totallyTyped="true"
       strictBinaryOperands="true" useDocblockPropertyTypes="true"
       xmlns="https://getpsalm.org/schema/config"
       xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd">
    <projectFiles>
        <directory name=".."/>
        <ignoreFiles>
            <directory name="vendor"/>
            <directory name="var"/>
            <directory name="features"/>
            <file name="phpinsights.php"/>
            <file name="rector.php"/>
        </ignoreFiles>
    </projectFiles>

    <plugins>
        <pluginClass class="Psalm\SymfonyPsalmPlugin\Plugin">
            <containerXml>var/cache/dev/App_KernelDevDebugContainer.xml</containerXml>
        </pluginClass>
        <pluginClass class="Weirdan\DoctrinePsalmPlugin\Plugin"/>
    </plugins>

    <issueHandlers>
        <MethodSignatureMismatch errorLevel="suppress"/>
        <UndefinedDocblockClass errorLevel="suppress"/>
        <MissingFile errorLevel="suppress"/>
        <UndefinedClass errorLevel="suppress"/>
        <PluginIssue name="RepositoryStringShortcut" errorLevel="suppress" />
    </issueHandlers>
</psalm>

I executed it one more time and now seems to be working 🤔 I haven't made any other changes and/or updates. However I am not sure what directory is Psalm scanning or files, any way to know those?

# composer psalm
> ./vendor/bin/psalm
Target PHP version: 8.1 (inferred from composer.json)
Scanning files...
Analyzing files...

░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 60 / 82 (73%)
░░░░░░░░░░░░░░░░░░░░░░

------------------------------

       No errors found!

------------------------------
65 other issues found.
You can display them with --show-info=true
------------------------------
Psalm can automatically fix 6 of these issues.
Run Psalm again with
--alter --issues=InvalidNullableReturnType,InvalidFalsableReturnType,MissingParamType --dry-run
to see what it can fix.
------------------------------

Checks took 5.08 seconds and used 237.624MB of memory
Psalm was able to infer types for 90.8057% of the codebase
vstm commented 2 years ago

Hmm maybe it was a cache-issue. To find out if it actually works you can run psalm with --debug or add an intentional issue. What I did is to just create a PHP file with the following content:

<?php $fail->fail();

Psalm should find at least one issue with that.

Otherwise you can use the --debug flag and maybe grep for the files psalm analyzes:

$ composer psalm -- --no-cache --debug 2>&1 | grep Analyzing
reypm commented 2 years ago

Is working fine now, using that last debug line I am able to see the files being analyzed. Thank you a lot both of you @vstm @orklah

orklah commented 2 years ago

Thanks @vstm !