bitExpert / phpstan-magento

Magento specific extension for PHPStan
MIT License
134 stars 24 forks source link

Occasional Errors In Pipeline #316

Open dmanners opened 6 months ago

dmanners commented 6 months ago

While running phpstan in the context of our Magento project locally we get no errors, but occasionally with the same code base running in a ci/cd pipeline we will get errors that will either change or disappear on a re-run.

The errors that do appear seem to only occur with extension attributes.

71 Call to method getSuppliers() on an unknown class
Magento\Sales\Api\Data\OrderExtensionInterface.
💡 Learn more at https://phpstan.org/user-guide/discovering-symbols

In our pipeline and also lokally we are running

./vendor/bin/phpstan analyse --memory-limit=4G --configuration=.dev/tests/static/testsuite/Magento/Test/Php/_files/phpstan/db-systel.neon

With the config

parameters:
    level: 4
    treatPhpDocTypesAsCertain: false
    paths:
        - %currentWorkingDirectory%/app/code
        - %currentWorkingDirectory%/app/design
    magento:
        magentoRoot: %currentWorkingDirectory%
    ignoreErrors:
        -
            # reason: false positive magic methods
            message: '#Call to an undefined method [a-zA-Z0-9\\_]+::setData|hasData|unsetData|getData|toArray\(\)#'
        -
            # reason: false positive with CartInterface
            message: '#Call to an undefined method Magento\\Quote\\Api\\Data\\CartInterface::getAllVisibleItems\(\).#'
        -
            message: '#Call to an undefined method Magento\\Quote\\Api\\Data\\CartInterface::getGrandTotal\(\).#'
        -
            message: '#Call to an undefined method Magento\\Quote\\Api\\Data\\CartInterface::getPayment\(\).#'

I have tried clearing the phpstan cache both locally and in the pipeline as well as locally clearing the generated folder as in the pipeline we have no generated folder.

shochdoerfer commented 6 months ago

Thanks for reporting the issue @dmanners.

The difference between local and CI runs is that locally, PHPStan and this extension will most likely use the Magento-generated classes (if they exist), while in CI, the PHPStan extension generates the missing classes by itself with its own logic. Still, that does not explain why CI occasionally breaks.

dmanners commented 6 months ago

Yeah it's odd some runs it's fine and some the errors appear but on a rerun everything is then green. I have also tested it locally without a generated folder and it's still green locally. I will also check that we dont override a magento file with a local patch that isnt applied in the pipeline as well.

I have also tested out firstly clearing the phpstan cache folder before running the check.

dmanners commented 6 months ago

Ok after a bit more debugging I have found that disabling the parallel processing then the errors do not seem to appear.

    parallel:
        maximumNumberOfProcesses: 1

Here I have run our pipeline 3 times and had no error cases.

shochdoerfer commented 6 months ago

Uh, good catch! Fixing this issue might not be easy due to the way the autoloader & the code generation in this extension work. Will think about it.

dmanners commented 6 months ago

I will keep this updated after a week or so of pipeline runs to see if this setting helps us in the long run or just randomly today.

shochdoerfer commented 6 months ago

Sounds like a plan. Don't close the issue if the current setting works for you. I'll take this as an opportunity to discuss with Ondrej how to better hook into PHPStan to generate the classes when they do not exist. The autoloader magic isn't ideal and seems to break a few things, e.g. the Rector integration (see #297). Looks like a larger refactoring is needed to improve things.

If the setting does work for you, please consider adding a PR to the README that includes a hint on how to resolve the issue.

dmanners commented 6 months ago

Yeah this seems to have fixed the issue with us a week of pipeline runs and no false positive errors in the phpstan job.

shochdoerfer commented 2 months ago

I've discussed this issue with Ondřej the other day. If you want to run the processes in parallel, it would be best to run Magento's setup:di:compile command before running PHPStan. This way, Magento will generate all "missing" files, and the autoloaders of our extension are not executed.

I will have to check how to change the extension's architecture to allow the code generation to happen only once. Ideally, without invoking a separate CLI tool.