Allow higher versions of phpstan to be installed #30581

hostep · 2020-10-21T10:39:03Z

Description (*)

See discussions in bitExpert/phpstan-magento#42 and bitExpert/phpstan-magento#43

Basically it boils down to:

bitexpert/phpstan-magento need phpstan/phpstan 0.12.26 or higher
this is possible in Magento 2.3.6: https://github.com/magento/magento2/blob/2.3.6/composer.json#L122
this is not possible in Magento 2.4.0 or 2.4.1: https://github.com/magento/magento2/blob/2.4.0/composer.json#L95
problem was introduced in 700de11
as far as I know, phpstan is developed with semantic versioning in mind, so if they add a breaking change, it will be released as 0.13.0 and then the contraint ^0.12.3 will prevent Magento to use this newer version.

Related Pull Requests

https://github.com/magento/partners-magento2ee/pull/462

Fixed Issues (if relevant)

Fixes Mismatch between requirements bitExpert/phpstan-magento#42: Mismatch between requirements
Fixes Update phpstan/phpstan dependency to PHP 8 compatible version #31176

Manual testing scenarios (*)

Somehow, tell phpstan to run on the Magento 2 source code and it should run without problems (it might detect more/different issues, and that is expected)

Questions or comments

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds are green)

m2-assistant · 2020-10-21T10:39:06Z

Hi @hostep. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

hostep · 2020-10-21T10:44:37Z

@magento run all tests

(this probably doesn't make much sense, because phpstan is executed on changed source code files and none have been changed in this PR)

magento-engcom-team · 2020-10-21T13:25:40Z

Hi @ihor-sviziev, thank you for the review.
ENGCOM-8363 has been created to process this Pull Request
✳️ @ihor-sviziev, could you please add one of the following labels to the Pull Request?

Label	Description
`Auto-Tests: Covered`	All changes in Pull Request is covered by auto-tests
`Auto-Tests: Not Covered`	Changes in Pull Request requires coverage by auto-tests
`Auto-Tests: Not Required`	Changes in Pull Request does not require coverage by auto-tests

ihor-sviziev · 2020-10-21T13:26:51Z

This was introduced in 700de11
I don't really see any reasons for that, but that need to be checked additionally during a testing of this PR.

BTW from https://getcomposer.org/doc/articles/versions.md#caret-version-range- - so such dependence should be fine:

For pre-1.0 versions it also acts with safety in mind and treats ^0.3 as >=0.3.0 <0.4.0.

ihor-sviziev · 2020-10-21T13:32:48Z

I'm pretty sure failed tests not related to changes in this PR, but let's restart tests
@magento run Functional Tests CE, Functional Tests EE

ihor-sviziev · 2020-11-04T07:41:52Z

For QA - unfortunately all the time someone changes composer.json - we have a conflict in composer.lock in "hash".
Unfortunately processing PR is quite slow process, so it's almost impossible to test just merged version of composer.lock.
Could you fix the conflict yourself by accepting any of versions of "hash" and then running composer update --lock?

hws47a · 2020-11-17T19:45:03Z

I had a case when PHPStan was failing with Magento core configuration with PHPStan version >=0.12.29 and definitely on 0.12.32. So please be careful with the update as it can break tests. I'm sure there was a reason why the higher band was introduced.

ihor-sviziev · 2020-11-17T19:55:35Z

@hws47a if you could give us steps to reproduce - it will be really helpful

hostep · 2020-12-08T07:15:43Z

@fascinosum: do you happen to remember why the upper constraint for version 0.12.23 was added in 700de11? Thanks!

hostep · 2020-12-18T07:05:26Z

Rebased on top of 2.4-develop, solved conflicts, further updated phpstan/phpstan to 0.12.63 and force pushed my commit to keep a clean history.

@magento run all tests

ihor-sviziev · 2021-01-25T11:51:54Z

@magento run all tests

sivaschenko · 2021-01-26T13:55:27Z

@hostep php8-develop branches update is in progress now

hostep · 2021-01-26T20:22:23Z

Rebased on latest php8-develop branch, hopefully tests will run more stable now (but I still believe they are irrelevant in scope of this PR and this should get tested in another way).

@magento run all tests

ihor-sviziev · 2021-01-27T07:55:44Z

@magento run Functional Tests B2B, Functional Tests CE, Functional Tests EE

sivaschenko · 2021-01-27T13:53:19Z

Added related ee pull request

sivaschenko · 2021-01-27T13:53:27Z

@magento run all tests

ihor-sviziev · 2021-01-27T14:45:54Z

@sivaschenko, could you also resolve the conflict by accepting any version of "hash" and executing composer update --lock (it will generate a new hash)?

…-phpstan-constraint

sivaschenko · 2021-01-27T15:39:08Z

@magento run all tests

m2-assistant · 2021-01-27T18:12:27Z

Hi @hostep, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

fascinosum · 2021-02-16T17:26:36Z

hi @hostep , @ihor-sviziev
see steps to reproduce below (tested on 2.3 and 2.4)

Run the command in console
echo 'setup/src/Magento/Setup/Test/Unit/Console/Style/MagentoStyleTest.php' >> $magento_DIR$/dev/tests/static/testsuite/Magento/Test/_files/changed_files_local.txt
Replace $magento_DIR$ with the real path to the Magento installation directory
run
composer require --dev phpstan/phpstan:0.12.24 or just use some other later version
run \Magento\Test\Php\LiveCodeTest::testPhpStan

cc: @sivaschenko
Internal ticket was created MC-40914

hostep · 2021-02-16T17:55:40Z

Thanks @fascinosum !

Can you tell us how we need to execute \Magento\Test\Php\LiveCodeTest::testPhpStan ?
Is there an easy way to do this?

fascinosum · 2021-02-16T18:21:23Z

@hostep, just run it on your local
https://github.com/magento-commerce/magento2ce/blob/10a70656165765af1f9a2277eb91410c79eacd33/dev/tests/static/testsuite/Magento/Test/Php/LiveCodeTest.php#L449 in PHPStorm
using https://github.com/magento-commerce/magento2ce/blob/2.4-develop/dev/tests/static/phpunit.xml.dist configuration for PHPUnit. This is the easiest way how to run and debug
If you are not using PHPStorm, you can run a CLI command for PHPUnit

hostep · 2021-02-16T20:15:15Z

Ok great!

So something like this on the CLI I believe:

vendor/bin/phpunit -c dev/tests/static/phpunit.xml.dist --testsuite 'PHP Coding Standard Verification' --filter testPhpStan

However, I'm still unable to trigger any kind of error:

$ composer require --dev phpstan/phpstan:0.12.24
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 0 installs, 1 update, 0 removals
  - Updating phpstan/phpstan (0.12.23 => 0.12.24): Downloading (100%)
...

$ echo 'setup/src/Magento/Setup/Test/Unit/Console/Style/MagentoStyleTest.php' > ./dev/tests/static/testsuite/Magento/Test/_files/changed_files_local.txt

$ vendor/bin/phpunit -c dev/tests/static/phpunit.xml.dist --testsuite 'PHP Coding Standard Verification' --filter testPhpStan
PHPUnit 9.1.5 by Sebastian Bergmann and contributors.

.                                                                                                                                                                                                                                 1 / 1 (100%)

Time: 00:00.215, Memory: 4.00 MB

OK (1 test, 1 assertion)

Am I still doing something wrong @fascinosum?

hostep · 2021-02-16T20:34:28Z

Ok, I'm able to trigger it, finally.
I had to hardcode the method Magento\Test\Php\LiveCodeTest::getWhitelist to return:

return ['setup/src/Magento/Setup/Test/Unit/Console/Style/MagentoStyleTest.php'];

After that I can trigger the test, but my god, this is such an awful way to run static tests, I really hope something gets done to this in the future so we have a much more user-friendly way of triggering these, and then also finally document how you should do it.

Anyway, the results are in:

phpstan 0.12.23:

$ vendor/bin/phpunit -c dev/tests/static/phpunit.xml.dist --testsuite 'PHP Coding Standard Verification' --filter testPhpStan
PHPUnit 9.1.5 by Sebastian Bergmann and contributors.

.                                                                                                                                                                                                                                 1 / 1 (100%)

Time: 00:02.147, Memory: 4.00 MB

OK (1 test, 1 assertion)

phpstan 0.12.24:

$ vendor/bin/phpunit -c dev/tests/static/phpunit.xml.dist --testsuite 'PHP Coding Standard Verification' --filter testPhpStan
PHPUnit 9.1.5 by Sebastian Bergmann and contributors.


In Resolver.php line 402:

  Service 'errorFormatter.filtered' (type of Magento\PhpStan\Formatters\FilteredErrorFormatter): Unable to pass specified arguments to PHPStan\Command\ErrorFormatter\TableErrorFormatter::__construct().


analyse [--paths-file PATHS-FILE] [-c|--configuration CONFIGURATION] [-l|--level LEVEL] [--no-progress] [--debug] [-a|--autoload-file AUTOLOAD-FILE] [--error-format ERROR-FORMAT] [--generate-baseline [GENERATE-BASELINE]] [--memory-limit MEMORY-LIMIT] [--xdebug] [--] [<paths>...]

F                                                                                                                                                                                                                                 1 / 1 (100%)

Time: 00:00.902, Memory: 6.00 MB

There was 1 failure:

1) Magento\Test\Php\LiveCodeTest::testPhpStan
PHPStan command run failed.
Failed asserting that 1 matches expected 0.

dev/tests/static/testsuite/Magento/Test/Php/LiveCodeTest.php:478

FAILURES!
Tests: 1, Assertions: 1, Failures: 1.

phpstan 0.12.69 (which was used and approved in this PR):

$ vendor/bin/phpunit -c dev/tests/static/phpunit.xml.dist --testsuite 'PHP Coding Standard Verification' --filter testPhpStan
PHPUnit 9.1.5 by Sebastian Bergmann and contributors.


In Resolver.php line 442:

  Service 'errorFormatter.github' (type of PHPStan\Command\ErrorFormatter\GithubErrorFormatter): Multiple services of type PHPStan\Command\ErrorFormatter\TableErrorFormatter found: errorFormatter.filtered, errorFormatter.table (needed
  by $tableErrorformatter in __construct())


In Autowiring.php line 50:

  Multiple services of type PHPStan\Command\ErrorFormatter\TableErrorFormatter found: errorFormatter.filtered, errorFormatter.table


analyse [--paths-file PATHS-FILE] [-c|--configuration CONFIGURATION] [-l|--level LEVEL] [--no-progress] [--debug] [-a|--autoload-file AUTOLOAD-FILE] [--error-format ERROR-FORMAT] [--generate-baseline [GENERATE-BASELINE]] [--memory-limit MEMORY-LIMIT] [--xdebug] [--fix] [--watch] [--pro] [--] [<paths>...]

F                                                                                                                                                                                                                                 1 / 1 (100%)

Time: 00:01.177, Memory: 6.00 MB

There was 1 failure:

1) Magento\Test\Php\LiveCodeTest::testPhpStan
PHPStan command run failed.
Failed asserting that 1 matches expected 0.

dev/tests/static/testsuite/Magento/Test/Php/LiveCodeTest.php:478

FAILURES!
Tests: 1, Assertions: 1, Failures: 1.

So indeed, something will need to be done inside Magento's code of how it calls phpstan to make it compatible again somehow. I'm not going to put time in this in the next few days I think and we should probably now also put #31972 on hold until a solution is found for this.

hostep · 2021-02-16T20:49:38Z

The easiest way to trigger the newer phpstan version incompatibility is by simply executing:

$ ./vendor/bin/phpstan analyse --level 1 --no-progress --memory-limit=4G --configuration ./dev/tests/static/testsuite/Magento/Test/Php/_files/phpstan/phpstan.neon setup/src/Magento/Setup/Test/Unit/Console/Style/MagentoStyleTest.php

In Resolver.php line 442:

  Service 'errorFormatter.github' (type of PHPStan\Command\ErrorFormatter\GithubErrorFormatter): Multiple services of type PHPStan\Command\ErrorFormatter\TableErrorFormatter found: errorFormatter.filtered, errorFormatter.table (needed
  by $tableErrorformatter in __construct())


In Autowiring.php line 50:

  Multiple services of type PHPStan\Command\ErrorFormatter\TableErrorFormatter found: errorFormatter.filtered, errorFormatter.table


analyse [--paths-file PATHS-FILE] [-c|--configuration CONFIGURATION] [-l|--level LEVEL] [--no-progress] [--debug] [-a|--autoload-file AUTOLOAD-FILE] [--error-format ERROR-FORMAT] [--generate-baseline [GENERATE-BASELINE]] [--memory-limit MEMORY-LIMIT] [--xdebug] [--fix] [--watch] [--pro] [--] [<paths>...]

That way you don't need to go through this phpunit mess.

fascinosum · 2021-02-16T20:52:51Z

thank you @hostep,
the CLI command from PHPStorm is
/usr/local/bin/php $magento_DIR$/vendor/phpunit/phpunit/phpunit --configuration $magento_DIR$/dev/tests/static/phpunit.xml.dist --filter "/::testPhpStan( .*)?$/" --test-suffix LiveCodeTest.php $magento_DIR$/dev/tests/static/testsuite/Magento/Test/Php --teamcity
All paths are absolute
JFYI: we only run static tests on change files, so you need this hack with changed_files_local.txt. But it should work without mocked methods

ihor-sviziev · 2021-02-17T07:29:29Z

It would be good to create a new issue for fixing this new behavior after the phpstan update as it was recently merged to the php8 branch.

hostep · 2021-02-17T19:45:51Z

I'm assuming a core Magento dev wil pick this up since internal ticket MC-40914 was created. It has something to do with that custom FilteredErrorFormatter class which is custom written for Magento

sivaschenko · 2021-02-18T12:03:53Z

@hostep @ihor-sviziev @fascinosum can you please take a look at this proposed fix: #32215

ihor-sviziev · 2021-02-18T12:14:24Z

@sivaschenko, TBH, I didn't work enough with phpstan to check if your changes are good enough, but from the first look - looks good.

hws47a · 2021-02-18T20:11:08Z

Good job @sivaschenko

ghost added the Progress: pending review label Oct 21, 2020

magento-engcom-team added the Release Line: 2.4 label Oct 21, 2020

hostep mentioned this pull request Oct 21, 2020

Mismatch between requirements bitExpert/phpstan-magento#42

Closed

ihor-sviziev approved these changes Oct 21, 2020

View reviewed changes

ghost assigned ihor-sviziev Oct 21, 2020

ghost added Progress: ready for testing and removed Progress: pending review labels Oct 21, 2020

ihor-sviziev added Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests Cleanup Severity: S4 Affects aesthetics, professional look and feel, “quality” or “usability”. labels Oct 21, 2020

sidolov added the Priority: P4 No current plan to fix. Fixing can be deferred as a logical part of more important work. label Oct 21, 2020

ihor-sviziev mentioned this pull request Dec 17, 2020

Update phpstan/phpstan dependency to PHP 8 compatible version #31176

Closed

hostep mentioned this pull request Dec 17, 2020

Allowed php 8 compatible phpstan version #31303

Closed

4 tasks

hostep force-pushed the relax-phpstan-constraint branch from 3258dc4 to 57c136b Compare December 18, 2020 07:03

sivaschenko added the Project: PHP8 label Dec 18, 2020

sivaschenko changed the base branch from 2.4-develop to php8-develop December 18, 2020 15:29

Allow higher versions of phpstan to be installed.

075a1b2

hostep force-pushed the relax-phpstan-constraint branch from f303bad to 075a1b2 Compare January 26, 2021 20:21

sivaschenko added 2 commits January 27, 2021 15:29

Merge branch 'php8-develop' of github.com:magento/magento2 into relax…

ccf86af

…-phpstan-constraint

magento#31176: Updated composer.lock

e355503

sivaschenko removed the Cleanup label Jan 27, 2021

sivaschenko merged commit 0bd5384 into magento:php8-develop Jan 27, 2021

hostep mentioned this pull request Feb 4, 2021

Allow higher versions of phpstan to be installed (Magento 2.4 edition) #31972

Closed

4 tasks

hostep mentioned this pull request Nov 6, 2021

Upgraded phpstan dependency to 1.1 #34555

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow higher versions of phpstan to be installed #30581

Allow higher versions of phpstan to be installed #30581

hostep commented Oct 21, 2020 •

edited by sivaschenko

Loading

m2-assistant bot commented Oct 21, 2020

hostep commented Oct 21, 2020

magento-engcom-team commented Oct 21, 2020

ihor-sviziev commented Oct 21, 2020 •

edited

Loading

ihor-sviziev commented Oct 21, 2020

ihor-sviziev commented Nov 4, 2020

hws47a commented Nov 17, 2020

ihor-sviziev commented Nov 17, 2020

hostep commented Dec 8, 2020

hostep commented Dec 18, 2020

ihor-sviziev commented Jan 25, 2021

sivaschenko commented Jan 26, 2021

hostep commented Jan 26, 2021 •

edited

Loading

ihor-sviziev commented Jan 27, 2021

sivaschenko commented Jan 27, 2021

sivaschenko commented Jan 27, 2021

ihor-sviziev commented Jan 27, 2021

sivaschenko commented Jan 27, 2021

m2-assistant bot commented Jan 27, 2021

fascinosum commented Feb 16, 2021 •

edited

Loading

hostep commented Feb 16, 2021

fascinosum commented Feb 16, 2021

hostep commented Feb 16, 2021

hostep commented Feb 16, 2021

hostep commented Feb 16, 2021

fascinosum commented Feb 16, 2021 •

edited

Loading

ihor-sviziev commented Feb 17, 2021 •

edited

Loading

hostep commented Feb 17, 2021

sivaschenko commented Feb 18, 2021

ihor-sviziev commented Feb 18, 2021

hws47a commented Feb 18, 2021

Allow higher versions of phpstan to be installed #30581

Allow higher versions of phpstan to be installed #30581

Conversation

hostep commented Oct 21, 2020 • edited by sivaschenko Loading

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

m2-assistant bot commented Oct 21, 2020

hostep commented Oct 21, 2020

magento-engcom-team commented Oct 21, 2020

ihor-sviziev commented Oct 21, 2020 • edited Loading

ihor-sviziev commented Oct 21, 2020

ihor-sviziev commented Nov 4, 2020

hws47a commented Nov 17, 2020

ihor-sviziev commented Nov 17, 2020

hostep commented Dec 8, 2020

hostep commented Dec 18, 2020

ihor-sviziev commented Jan 25, 2021

sivaschenko commented Jan 26, 2021

hostep commented Jan 26, 2021 • edited Loading

ihor-sviziev commented Jan 27, 2021

sivaschenko commented Jan 27, 2021

sivaschenko commented Jan 27, 2021

ihor-sviziev commented Jan 27, 2021

sivaschenko commented Jan 27, 2021

m2-assistant bot commented Jan 27, 2021

fascinosum commented Feb 16, 2021 • edited Loading

hostep commented Feb 16, 2021

fascinosum commented Feb 16, 2021

hostep commented Feb 16, 2021

hostep commented Feb 16, 2021

hostep commented Feb 16, 2021

fascinosum commented Feb 16, 2021 • edited Loading

ihor-sviziev commented Feb 17, 2021 • edited Loading

hostep commented Feb 17, 2021

sivaschenko commented Feb 18, 2021

ihor-sviziev commented Feb 18, 2021

hws47a commented Feb 18, 2021

hostep commented Oct 21, 2020 •

edited by sivaschenko

Loading

ihor-sviziev commented Oct 21, 2020 •

edited

Loading

hostep commented Jan 26, 2021 •

edited

Loading

fascinosum commented Feb 16, 2021 •

edited

Loading

fascinosum commented Feb 16, 2021 •

edited

Loading

ihor-sviziev commented Feb 17, 2021 •

edited

Loading