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

Get ready for PHPStan 2.0 #8815

Open
ondrejmirtes opened this issue Sep 4, 2024 · 38 comments
Open

Get ready for PHPStan 2.0 #8815

ondrejmirtes opened this issue Sep 4, 2024 · 38 comments
Labels

Comments

@ondrejmirtes
Copy link
Contributor

Feature Request

Hi, I just started working on PHPStan 2.0 which will come with PHP-Parser 5. These are early days of the development, but you can already require phpstan/phpstan:^2.0 (with minimum-stability dev) and start working on making Rector compatible.

There will be more backward compatibility breaks coming in the next few months as I work towards to release, but you can already start getting rector-src ready for Rector 2.0 which will require PHPStan 2.0.

Please keep this issue open so we can discuss the next major PHPStan version. I'd also appreciate any feedback from your side. Thanks. 🤞

@TomasVotruba
Copy link
Member

TomasVotruba commented Sep 4, 2024

Sounds good 👍

We'll give this a priority by the end of the year and do upgrade of both PHPStan 2 and php-parser 5 at once.

@ondrejmirtes
Copy link
Contributor Author

We're now in the release candidate period phpstan/phpstan#11816

@samsonasik
Copy link
Member

rector phpstan's extensions seems need to have compatible to phpstan 2 first:

  Problem 1
    - Root composer.json requires rector/type-perfect ^0.1.6 -> satisfiable by rector/type-perfect[0.1.6, 0.1.7, 0.1.8].
    - rector/type-perfect[0.1.6, ..., 0.1.8] require phpstan/phpstan ^1.11 -> found phpstan/phpstan[1.11.0, ..., 1.12.x-dev] but it conflicts with your root composer.json require (^2.0).
  Problem 2
    - Root composer.json requires symplify/phpstan-extensions ^11.4 -> satisfiable by symplify/phpstan-extensions[11.4.0, 11.4.1, 11.4.2, 11.4.3].
    - symplify/phpstan-extensions[11.4.0, ..., 11.4.3] require phpstan/phpstan ^1.10 -> found phpstan/phpstan[1.10.0, ..., 1.12.x-dev] but it conflicts with your root composer.json require (^2.0).
  Problem 3
    - Root composer.json requires symplify/phpstan-rules ^13.0 -> satisfiable by symplify/phpstan-rules[13.0.0, 13.0.1].
    - symplify/phpstan-rules[13.0.0, ..., 13.0.1] require phpstan/phpstan ^1.10.30 -> found phpstan/phpstan[1.10.30, ..., 1.12.x-dev] but it conflicts with your root composer.json require (^2.0).
  Problem 4
    - Root composer.json requires tomasvotruba/unused-public ^0.3.10 -> satisfiable by tomasvotruba/unused-public[0.3.10, 0.3.11].
    - tomasvotruba/unused-public[0.3.10, ..., 0.3.11] require phpstan/phpstan ^1.10.19 -> found phpstan/phpstan[1.10.19, ..., 1.12.x-dev] but it conflicts with your root composer.json require (^2.0).
  Problem 5
    - Root composer.json requires phpstan/phpstan-phpunit ^1.4 -> satisfiable by phpstan/phpstan-phpunit[1.4.0, 1.4.x-dev].
    - phpstan/phpstan-phpunit 1.4.0 requires phpstan/phpstan ^1.11 -> found phpstan/phpstan[1.11.0, ..., 1.12.x-dev] but it conflicts with your root composer.json require (^2.0).
    - phpstan/phpstan-phpunit 1.4.x-dev requires phpstan/phpstan ^1.12 -> found phpstan/phpstan[1.12.0, ..., 1.12.x-dev] but it conflicts with your root composer.json require (^2.0).

@szepeviktor
Copy link
Contributor

@TomasVotruba Today is 11.11. Do you plan to make Rector PHPStan 2 compatible this week?

@samsonasik
Copy link
Member

First step: symplify/phpstan-extensions#12

@TomasVotruba
Copy link
Member

@szepeviktor Depends how many only-in-Rector BC breaks we'll have to deal with. We use lot of internal code, so it might be a challange.

@samsonasik Could you look into it? rector/rector first, so we can keep it focused on single repository

@samsonasik
Copy link
Member

@TomasVotruba sure, phpstan 2 require php-parser 5 so it will need revisit my old PR:

to be reincorporated ;)

@canvural
Copy link

I think best way to start is to enable bleeding edge and maybe install phpstan-deprecation-rules and start fixing stuff that pops up. We don't need to require 2.0 for that.

@ondrejmirtes
Copy link
Contributor Author

Exactly, as the upgrading guide says!

@samsonasik
Copy link
Member

sure, see rectorphp/rector-src#6415

@carlos-granados
Copy link

I created this PR for tomasvotruba/unused-public TomasVotruba/unused-public#135, probably complementary to TomasVotruba/unused-public#134

@carlos-granados
Copy link

Another PR for tomasvotruba/type-coverage TomasVotruba/type-coverage#46, complementary to TomasVotruba/type-coverage#45

@samsonasik
Copy link
Member

I am trying to install phpstan 2 and php-parser 5

the php-parser is patched, and currently got error on phpstan service:

44) Rector\Tests\Issues\AutoImport\AutoImportTest::test with data set #43 ('/Users/samsonasik/www/rector-...hp.inc')
_PHPStan_2a200beec\Nette\DI\ServiceCreationException: Service 'rectorParser' (type of PHPStan\Parser\RichParser): Unable to pass specified arguments to RichParser::__construct().

@samsonasik
Copy link
Member

The upgrade process has been completed 🎉 🎉 🎉

@FabianKoestring
Copy link

The upgrade process has been completed

Are there any plans for a new release?

@TomasVotruba
Copy link
Member

TomasVotruba commented Nov 22, 2024

@FabianKoestring We're testing the dev branch now in couple big projects. There are still breaks in docblocks we have to verify to avoid mess.
I expect dev tagged with rc/beta mid next week. If stable enough, we can land stable within ~week after that.

Date 12. 12. seems like best :)

If you can try out latest Rector dev and report feedback from your project, it would speedup the process 🙂

{
	"require-dev": {
        "phpstan/phpstan": "^2.0",
        "rector/rector": "dev-main"
    }
}

@tacman
Copy link

tacman commented Nov 22, 2024

I've switched to the dev branch (so that I could install phpstan 2), and haven't had any issues. Granted, my use cases are fairly straightforward and common (just adding return types is a huge help!)

@ruudk
Copy link
Contributor

ruudk commented Nov 22, 2024

There is an issue going on with conditional return types. Had a few files that failed.

@samsonasik
Copy link
Member

rectorphp/rector-src#6475

@simonschaufi
Copy link

simonschaufi commented Nov 22, 2024

A really good project to also test is TYPO3 Rector. I've tried it out with:

"require": {
        "php": "^7.4 || ^8.0",
        "ext-json": "*",
        "league/flysystem": "^2.0 || ^3.0",
        "league/flysystem-memory": "^2.0 || ^3.0",
        "nette/utils": "^3.2.10 || ^4.0.4",
        "nikic/php-parser": "^4.18.0",
-       "phpstan/phpstan": "^1.10.56",
-       "rector/rector": "^1.1.0",
+       "phpstan/phpstan": "^2.0",
+       "rector/rector": "dev-main",
        "symfony/console": "^5.4 || ^6.4 || ^7.0",
        "symfony/filesystem": "^5.4 || ^6.4 || ^7.0",
        "symfony/finder": "^5.4 || ^6.4 || ^7.0",
        "symfony/polyfill-php80": "^1.28.0",
        "symfony/polyfill-php81": "^1.28.0",
        "symfony/string": "^5.4 || ^6.4 || ^7.0",
        "webmozart/assert": "^1.11.0"
    },
    "require-dev": {
        "ergebnis/composer-normalize": "^2.42.0",
        "php-parallel-lint/php-parallel-lint": "^1.3.2",
        "phpstan/extension-installer": "^1.3.1",
-       "phpstan/phpstan-deprecation-rules": "^1.1.4",
-       "phpstan/phpstan-phpunit": "^1.3.16",
        "phpunit/phpunit": "^9.6.17 || ^10.0",
        "symfony/config": "^5.0 || ^6.0 || ^7.0",
        "symfony/dependency-injection": "^5.4.36 || ^6.4.2 || ^7.0.2",
        "symfony/http-kernel": "^5.4.37 || ^6.4.2 || ^7.0.2",
        "symplify/easy-coding-standard": "^12.1.14"
    },

and then run:

composer update
vendor/bin/rector process

and I'm getting some phpstan error:

PHP Fatal error: Uncaught _PHPStan_c684505c9\Nette\Schema\ValidationException: Unexpected item 'parameters › featureToggles › disableRuntimeReflectionProvider'. in phar:///home/xxx/sabbelasichon/typo3-rector/vendor/phpstan/phpstan/phpstan.phar/vendor/nette/schema/src/Schema/Processor.php:75

@samsonasik
Copy link
Member

@simonschaufi I created PR on typo-3 rector for that :)

@carlos-granados
Copy link

I tried the new version with our project and could not find any problems. Rector was able to find several new issues, but all these finds were correct, they were genuine problems that had not been detected before

Our project is mid sized, about 1K PHP files with about 200K lines of code. We apply a lot of Rector rules, around 300

@ruudk
Copy link
Contributor

ruudk commented Nov 23, 2024

Found another issue, but don't understand why it happens:

$ vendor/bin/phpstan analyze --debug src-dev/Rector/
/Volumes/CS/www/cosmos/src-dev/Rector/StaticMoneyConstructorRector.php
PHP Fatal error:  Dev\Rector\StaticMoneyConstructorRector::getRuleDefinition() has #[\Override] attribute, but no matching parent method exists in /Volumes/CS/www/cosmos/src-dev/Rector/StaticMoneyConstructorRector.php on line 26

Part of the rector:

<?php

declare(strict_types=1);

namespace Dev\Rector;

use Money\Currency;
use Money\Money;
use Override;
use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\New_;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Name\FullyQualified;
use PhpParser\Node\Param;
use PhpParser\Node\Scalar\LNumber;
use PhpParser\Node\Scalar\String_;
use PhpParser\NodeTraverser;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

final class StaticMoneyConstructorRector extends AbstractRector
{
    #[Override]
    public function getRuleDefinition() : RuleDefinition
    {
        return new RuleDefinition(
            'Replace `new Money(10, new Currency("EUR"))` with `Money::EUR(10)`.',
            [
                new CodeSample(
                    <<<'CODE_SAMPLE'
                        new Money(10, new Currency("EUR"));
                        CODE_SAMPLE,
                    <<<'CODE_SAMPLE'
                        Money::EUR(10);
                        CODE_SAMPLE,
                )],
        );
    }
  // ...
}

I ran this:

$ composer update -W "phpstan/*" rector/rector phpdocumentor/type-resolver phpstan/phpdoc-parser

The reason I don't understand it, is that I'm overriding this, as it's part of a higher interface, no?

When I remove the #[Override] interface, PHPStan reports an error like this:

Method Dev\Rector\StaticMoneyConstructorRector::getRuleDefinition() overrides method Symplify\RuleDocGenerator\Contract\DocumentedRuleInterface::getRuleDefinition() but is missing the #[\Override] attribute.
method.missingOverride

Which is what I expect, because we enforce #[Override]

@samsonasik
Copy link
Member

samsonasik commented Nov 23, 2024

@ruudk It because of this PR:

getRuleDefinition() no longer exists in shipped rector/rector, to ease for write custom rule without define getRuleDefinition() method.

You can remove #[Override] on getRuleDefinition() and that should working again.

@samsonasik
Copy link
Member

@ruudk I created PR for that for win win solution:

@TomasVotruba
Copy link
Member

@ruudk That's expected. See the upgrade guide, the getDefinition() is no longer needed as it was way to enforce docs for Rector rules. https://github.com/rectorphp/rector-src/blob/main/UPGRADING.md#2-abstractrector-get-focused-on-code-the-getruledefinition-is-no-longer-required

@carlos-granados
Copy link

@TomasVotruba does this mean that when we upgrade to Rector 2.0 we need to remove our getDefinition() functions or can they remain in place even if they are no longer needed?

@ruudk
Copy link
Contributor

ruudk commented Nov 23, 2024

@ruudk That's expected. See the upgrade guide, the getDefinition() is no longer needed as it was way to enforce docs for Rector rules. https://github.com/rectorphp/rector-src/blob/main/UPGRADING.md#2-abstractrector-get-focused-on-code-the-getruledefinition-is-no-longer-required

As mentioned in the PR, this is not a solution as it doesn't work:

$ vendor/bin/rector process /Volumes/CS/www/cosmos/src-dev/Rector/StaticMoneyConstructorRector.php
PHP Fatal error:  Class Dev\Rector\StaticMoneyConstructorRector contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Symplify\RuleDocGenerator\Contract\DocumentedRuleInterface::getRuleDefinition) in /Volumes/CS/www/cosmos/src-dev/Rector/StaticMoneyConstructorRector.php on line 23
Fatal error: Class Dev\Rector\StaticMoneyConstructorRector contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Symplify\RuleDocGenerator\Contract\DocumentedRuleInterface::getRuleDefinition) in /Volumes/CS/www/cosmos/src-dev/Rector/StaticMoneyConstructorRector.php on line 23

@samsonasik
Copy link
Member

@carlos-granados if on autoload, it conflict with existing package symplify/rule-doc-generator, unfortunatelly, it can't, as autoload conflict, the solution is remove #[\Override] since PR rectorphp/rector-src#6476 not accepted

@carlos-granados
Copy link

I would prefer a solution that would allow us to keep the getDefinition() function if it already exists as I like how it shows the functionality of the rule. We don't use #[\Override]

@samsonasik
Copy link
Member

samsonasik commented Nov 23, 2024

@carlos-granados add getRuleDefinition() method will still works, just remove the #[\Override] attribute if exists as it may conflict with real dependency vs rector scoped dependency.

@samsonasik
Copy link
Member

new PR to avoid conflict with real dependency of symplify/rule-doc-generator

@g5bot
Copy link

g5bot commented Nov 23, 2024

Just tried rector/dev-main on our project. Works like a charm. Migrated to v2 without any problems! Good work @samsonasik
st-universe/core#1937

@TomasVotruba
Copy link
Member

TomasVotruba commented Nov 23, 2024

@carlos-granados

@TomasVotruba does this mean that when we upgrade to Rector 2.0 we need to remove our getDefinition() functions or can they remain in place even if they are no longer needed?

For 99 % use cases, they should be removed. The code snippets should be part of tests, if you have any.

If you use them in some own tooling to generate documentation, you can implement Symplify\RuleDocGenerator\Contract\DocumentedRuleInterface on your rules.

@ruudk
Copy link
Contributor

ruudk commented Nov 23, 2024

After @samsonasik's latest fix, both Rector and PHPStan work 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests