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

Upgrade to PHPParser 5 and PHPStan 2 #6431

Merged
merged 250 commits into from
Nov 20, 2024

Conversation

samsonasik
Copy link
Member

@samsonasik samsonasik commented Nov 14, 2024

Require patch branch of vendor-patches

Ref rectorphp/rector#8815

@samsonasik
Copy link
Member Author

samsonasik commented Nov 14, 2024

phpstan/phpdoc-parser seems needs to be upgraded to v2.0 as well to handle error undefined constant:

[ERROR] Could not process "rules/Php81/NodeFactory/EnumFactory.php" file, due  
         to:                                                                    
         "Undefined constant                                                    
         PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprStringNode::SINGLE_QUOTED".
         On line: 434        

ref https://github.com/phpstan/phpdoc-parser/blob/c00d78fb6b29658347f9d37ebe104bffadf36299/src/Ast/ConstExpr/ConstExprStringNode.php#L19

upgrading it currently got error:

There was 1 error:

1) Rector\Tests\Issues\DoubleRun\DoubleRunTest::test with data set #0 ('/Users/samsonasik/www/rector-...hp.inc')
Illuminate\Contracts\Container\BindingResolutionException: Unresolvable dependency resolving [Parameter #0 [ <required> array $usedAttributes ]] in class PHPStan\PhpDocParser\ParserConfig

will continue trying.

@samsonasik
Copy link
Member Author

Register ParserConfig as service seems handle it d301344

@staabm
Copy link
Contributor

staabm commented Nov 14, 2024

phpstan/phpdoc-parser seems needs to be upgraded to v2.0

Correct 👍

@samsonasik samsonasik force-pushed the upgrade-to-php-parser5-and-phpstan-2 branch 4 times, most recently from 6992147 to c183f3c Compare November 15, 2024 12:16
@samsonasik
Copy link
Member Author

PHPStan RichParser seems cause error on read curly array: $string{0}

There was 1 error:

1) Rector\Tests\Php74\Rector\ArrayDimFetch\CurlyToSquareBracketArrayStringRector\CurlyToSquareBracketArrayStringRectorTest::test with data set #0 ('/Users/samsonasik/www/rector-...hp.inc')
PHPStan\Parser\ParserErrorsException: Syntax error, unexpected '{', expecting ';', Syntax error, unexpected '{', expecting ';', Syntax error, unexpected '{', expecting ';'

phar:///Users/samsonasik/www/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Parser/RichParser.php:70

I will try if it possible to use native PHPParser Parser.

@samsonasik
Copy link
Member Author

use simple parser for defaultAnalysisParser, with native parser php 7.0 469b596

Before

There was 1 error:

1) Rector\Tests\Php74\Rector\ArrayDimFetch\CurlyToSquareBracketArrayStringRector\CurlyToSquareBracketArrayStringRectorTest::test with data set #0 ('/Users/samsonasik/www/rector-...hp.inc')
PHPStan\Parser\ParserErrorsException: Syntax error, unexpected '{', expecting ';', Syntax error, unexpected '{', expecting ';', Syntax error, unexpected '{', expecting ';'

phar:///Users/samsonasik/www/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Parser/RichParser.php:70

After

➜  rector-src git:(upgrade-to-php-parser5-and-phpstan-2) ✗ vendor/bin/phpunit rules-tests/Php74/Rector/ArrayDimFetch/CurlyToSquareBracketArrayStringRector/CurlyToSquareBracketArrayStringRectorTest.php --filter=#0
PHPUnit 10.5.38 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.12
Configuration: /Users/samsonasik/www/rector-src/phpunit.xml

.                                                                   1 / 1 (100%)

Time: 00:00.316, Memory: 62.50 MB

OK (1 test, 2 assertions)

This needs to fallback to php 8.0 feature as well, I will continue :)

@samsonasik samsonasik force-pushed the upgrade-to-php-parser5-and-phpstan-2 branch 4 times, most recently from 6acb52e to 3bb03c6 Compare November 17, 2024 03:01
@samsonasik
Copy link
Member Author

I got it, using ->createForNewestSupportedVersion() is fine to be on default, then if it catch PHPStan\Parser\ParserErrorsException, fallback to use php 7 parser when doing parsing, see 3bb03c6

@samsonasik samsonasik force-pushed the upgrade-to-php-parser5-and-phpstan-2 branch from 632553a to f381808 Compare November 17, 2024 13:57
@samsonasik
Copy link
Member Author

@TomasVotruba this is so far so good :), the todo is Doctrine annotation detection, which bc break on phpstan/phpdoc-parser 2,

I will continue :)

@samsonasik
Copy link
Member Author

@TomasVotruba a first step progress on doctrine annotation 🎉 , it needs detect class name from ObjectTypeSpecifier service as it Tag value no longer get full class name, only name, so it needs to know where it belongs, eg: from use statement.

b622c84

Before

There was 1 failure:

1) Rector\Tests\Issues\FqcnAnnotationToAttribute\FqcnAnnotationToAttributeTest::test with data set #3 ('/Users/samsonasik/www/rector-...hp.inc')
Failed on fixture file "sub_namespace_from_use.php.inc"
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 
 use Doctrine\ORM\Mapping;
 
-#[Mapping\Entity]
+/**
+ * @Mapping\Entity()
+ */
 class SubNamespaceFromUse
 {
 }

After

➜  rector-src git:(upgrade-to-php-parser5-and-phpstan-2) vendor/bin/phpunit tests/Issues/FqcnAnnotationToAttribute/FqcnAnnotationToAttributeTest.php --filter=#3
PHPUnit 10.5.38 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.12
Configuration: /Users/samsonasik/www/rector-src/phpunit.xml

.                                                                   1 / 1 (100%)

Time: 00:00.219, Memory: 46.00 MB

OK (1 test, 2 assertions)

@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba I think it is ready 🎉 🎉 🎉

@TomasVotruba
Copy link
Member

Let's ship it, so we can iterate faster 👍

Thanks for pushing this forwards @samsonasik 💪

@TomasVotruba TomasVotruba merged commit d5f9216 into main Nov 20, 2024
36 checks passed
@TomasVotruba TomasVotruba deleted the upgrade-to-php-parser5-and-phpstan-2 branch November 20, 2024 15:17
@samsonasik
Copy link
Member Author

@TomasVotruba thank you for the merge, I will fix the downgrade notice :)

https://github.com/rectorphp/rector-src/actions/runs/11936299874/job/33269627769#step:10:20

@samsonasik
Copy link
Member Author

Just one error on downgrade:

Syntax error found in 1 file
------------------------------------------------------------
Parse error: rector-prefixed-downgraded/vendor/nette/utils/src/Utils/FileSystem.php:140
    138|             $counter = 0;
    139|             do {
  > 140|                 $line = Callback::invokeSafe('fgets', [$f], fn($error) => throw new Nette\IOException(\sprintf("Unable to read file '%s'. %s", self::normalizePath($file), $error)));
    141|                 if ($line === \false) {
    142|                     \fclose($f);
Unexpected 'throw' (T_THROW)

I will check

@samsonasik
Copy link
Member Author

@samsonasik
Copy link
Member Author

Downgrade succeed 🎉

Screenshot 2024-11-20 at 22 59 23

Now, goes to fix php 7 code notice e2e test on rector/rector:

class Foo
{
    public function __construct()
    {
        $bar = 'baz';
        print $bar{2};
    }
}

https://github.com/rectorphp/rector/actions/runs/11937063227/job/33272177173#step:5:13

@samsonasik
Copy link
Member Author

Ok, it seems the issue is on downgrade process, the token got array over Token object:

➜  rector-src git:(main) ✗ bin/rector process vendor/nikic/php-parser/lib/PhpParser/Lexer.php --config build/config/config-downgrade.php --dry-run --clear-cache
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
1 file with changes
===================

1) vendor/nikic/php-parser/lib/PhpParser/Lexer.php:27

    ---------- begin diff ----------
@@ @@

         $scream = ini_set('xdebug.scream', '0');

-        $tokens = @Token::tokenize($code);
+        $tokens = @token_get_all($code);
         $this->postprocessTokens($tokens, $errorHandler);

         if (false !== $scream) {
@@ @@
     }

     private function handleInvalidCharacter(Token $token, ErrorHandler $errorHandler): void {
-        $chr = $token->text;
+        $chr = is_array($token) ? $token[1] : $token;
         if ($chr === "\0") {
             // PHP cuts error message after null byte, so need special case
             $errorMsg = 'Unexpected null byte';
@@ @@

     private function isUnterminatedComment(Token $token): bool {
         return $token->is([\T_COMMENT, \T_DOC_COMMENT])
-            && substr($token->text, 0, 2) === '/*'
-            && substr($token->text, -2) !== '*/';
+            && substr(is_array($token) ? $token[1] : $token, 0, 2) === '/*'
+            && substr(is_array($token) ? $token[1] : $token, -2) !== '*/';
     }

     /**
@@ @@

         for ($i = 0; $i < $numTokens; $i++) {
             $token = $tokens[$i];
-            if ($token->id === \T_BAD_CHARACTER) {
+            if ((is_array($token) ? $token[0] : $token) === \T_BAD_CHARACTER) {
                 $this->handleInvalidCharacter($token, $errorHandler);
             }

-            if ($token->id === \ord('&')) {
+            if ((is_array($token) ? $token[0] : $token) === \ord('&')) {
                 $next = $i + 1;
-                while (isset($tokens[$next]) && $tokens[$next]->id === \T_WHITESPACE) {
+                while (isset($tokens[$next]) && (is_array($tokens[$next]) ? $tokens[$next][0] : $tokens[$next]) === \T_WHITESPACE) {
                     $next++;
                 }
                 $followedByVarOrVarArg = isset($tokens[$next]) &&
                     $tokens[$next]->is([\T_VARIABLE, \T_ELLIPSIS]);
-                $token->id = $followedByVarOrVarArg
+                is_array($token) ? $token[0] : $token = $followedByVarOrVarArg
                     ? \T_AMPERSAND_FOLLOWED_BY_VAR_OR_VARARG
                     : \T_AMPERSAND_NOT_FOLLOWED_BY_VAR_OR_VARARG;
             }
    ----------- end diff -----------

Applied rules:
 * DowngradePhpTokenRector

I will continue checking...

@samsonasik
Copy link
Member Author

samsonasik commented Nov 20, 2024

it seems php-parser internal Token polyfill needs to be skipped

@samsonasik
Copy link
Member Author

@TomasVotruba Finally 🎉 🎉 🎉 All green 🎉 🎉 🎉

Screenshot 2024-11-21 at 00 21 42

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

Successfully merging this pull request may close these issues.

5 participants