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

RichParser error on dev-main on PHP 7.4 #6779

Closed
samsonasik opened this issue Nov 1, 2021 · 25 comments · Fixed by rectorphp/rector-src#1184
Closed

RichParser error on dev-main on PHP 7.4 #6779

samsonasik opened this issue Nov 1, 2021 · 25 comments · Fixed by rectorphp/rector-src#1184
Labels

Comments

@samsonasik
Copy link
Member

samsonasik commented Nov 1, 2021

Bug Report

Subject Details
Rector version dev-main

@TomasVotruba I tried rector dev-main on codeigniter4/CodeIgniter4#5269 to test the compatibility with PHPStan 1.0, and this only show this error on PHP 7.4, ref https://github.com/codeigniter4/CodeIgniter4/runs/4066534747?check_suite_focus=true#step:10:5

➜  CodeIgniter4 git:(upgrade-to-phpstan1) ✗ /opt/homebrew/Cellar/[email protected]/7.4.24_1/bin/php vendor/bin/rector process tests/system/View/TableTest.php --clear-cache -vvv
[parsing] tests/system/View/TableTest.php

In RichParser.php line 38:
                                                                                                                                                                                         
  [PHPStan\Parser\ParserErrorsException]                                                                                                                                                 
  Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE on line 117, Syntax error, unexpected '}', expecting T_VARIABLE on line 144, Syntax error, unexpected '}', expecting T_VARI  
  ABLE on line 167, Syntax error, unexpected '}', expecting T_VARIABLE on line 818, Syntax error, unexpected T_PUBLIC on line 1018     

while it is working ok on PHP 8.0. It seems there is invalid syntax in PHP 7.4 for PHPStan.

/cc @ondrejmirtes do you think it is PHPStan issue on downgrading syntax ?

The trace exception message are like this:

[parsing] tests/system/View/TableTest.php

In RichParser.php line 38:
                                                                                                                                                                                         
  [PHPStan\Parser\ParserErrorsException]                                                                                                                                                 
  Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE on line 117, Syntax error, unexpected '}', expecting T_VARIABLE on line 144, Syntax error, unexpected '}', expecting T_VARI  
  ABLE on line 167, Syntax error, unexpected '}', expecting T_VARIABLE on line 818, Syntax error, unexpected T_PUBLIC on line 1018                                                       
                                                                                                                                                                                         

Exception trace:
  at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Parser/RichParser.php:38
 PHPStan\Parser\RichParser->parseFile() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/Php/PhpMethodReflection.php:217
 PHPStan\Reflection\Php\PhpMethodReflection->isVariadic() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/Php/PhpMethodReflection.php:183
 PHPStan\Reflection\Php\PhpMethodReflection->getVariants() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/Php/UniversalObjectCratesClassReflectionExtension.php:50
 PHPStan\Reflection\Php\UniversalObjectCratesClassReflectionExtension->getProperty() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/ClassReflection.php:377
 PHPStan\Reflection\ClassReflection->getProperty() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Type/ObjectType.php:121
 PHPStan\Type\ObjectType->getUnresolvedPropertyPrototype() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Type/ObjectType.php:98
 PHPStan\Type\ObjectType->getProperty() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:2624
 PHPStan\Analyser\NodeScopeResolver->processAssignVar() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:1353
 PHPStan\Analyser\NodeScopeResolver->processExprNode() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:451
 PHPStan\Analyser\NodeScopeResolver->processStmtNode() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:249
 PHPStan\Analyser\NodeScopeResolver->processStmtNodes() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:414
 PHPStan\Analyser\NodeScopeResolver->processStmtNode() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:249
 PHPStan\Analyser\NodeScopeResolver->processStmtNodes() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:498
 PHPStan\Analyser\NodeScopeResolver->processStmtNode() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:249
 PHPStan\Analyser\NodeScopeResolver->processStmtNodes() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:464
 PHPStan\Analyser\NodeScopeResolver->processStmtNode() at phar:///Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:217
 PHPStan\Analyser\NodeScopeResolver->processNodes() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:152
 Rector\NodeTypeResolver\PHPStan\Scope\PHPStanNodeScopeResolver->processNodesWithMixinHandling() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:140
 Rector\NodeTypeResolver\PHPStan\Scope\PHPStanNodeScopeResolver->processNodes() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/packages/NodeTypeResolver/NodeScopeAndMetadataDecorator.php:81
 Rector\NodeTypeResolver\NodeScopeAndMetadataDecorator->decorateNodesFromFile() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/src/Application/FileProcessor.php:44
 Rector\Core\Application\FileProcessor->parseFileInfoToLocalCache() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/src/Application/FileProcessor/PhpFileProcessor.php:80
 Rector\Core\Application\FileProcessor\PhpFileProcessor->Rector\Core\Application\FileProcessor\{closure}() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/src/Application/FileProcessor/PhpFileProcessor.php:144
 Rector\Core\Application\FileProcessor\PhpFileProcessor->tryCatchWrapper() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/src/Application/FileProcessor/PhpFileProcessor.php:81
 Rector\Core\Application\FileProcessor\PhpFileProcessor->process() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/src/Application/ApplicationFileProcessor.php:76
 Rector\Core\Application\ApplicationFileProcessor->processFiles() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/src/Application/ApplicationFileProcessor.php:57
 Rector\Core\Application\ApplicationFileProcessor->run() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/src/Console/Command/ProcessCommand.php:152
 Rector\Core\Console\Command\ProcessCommand->execute() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/symfony/console/Command/Command.php:274
 RectorPrefix20211031\Symfony\Component\Console\Command\Command->run() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/symfony/console/Application.php:870
 RectorPrefix20211031\Symfony\Component\Console\Application->doRunCommand() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/symfony/console/Application.php:266
 RectorPrefix20211031\Symfony\Component\Console\Application->doRun() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/src/Console/ConsoleApplication.php:71
 Rector\Core\Console\ConsoleApplication->doRun() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/vendor/symfony/console/Application.php:162
 RectorPrefix20211031\Symfony\Component\Console\Application->run() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/bin/rector.php:63
 require_once() at /Users/samsonasik/www/CodeIgniter4/vendor/rector/rector/bin/rector:5

process [-n|--dry-run] [-a|--autoload-file AUTOLOAD-FILE] [-o|--output-format [OUTPUT-FORMAT]] [--no-progress-bar] [--no-diffs] [--clear-cache] [--] [<source>...]
@samsonasik samsonasik added the bug label Nov 1, 2021
@TomasVotruba
Copy link
Member

I'm not sure why it can't parse PHP 7.4.

What exact parsed code is causing it?

@samsonasik
Copy link
Member Author

samsonasik commented Nov 1, 2021

@ondrejmirtes
Copy link
Contributor

Hi, I think the issue is that you're running on PHP 7.4 but you're trying to parse code for PHP 8. While the code of the project might not use PHP 8 syntax, something in Rector uses it, most likely the thing that parses jetbrains/phpstorm-stubs.

PHPStan uses a different parser for this scenario. phpstorm-stubs are always parsed with a parser that contains the Emulative lexer.

I think that if you actually tried to test rector-src on PHP 7.4, it would fail in a similar manner. You're "supporting" PHP 7.x, but you're not running most tests on these supported versions. That should be fixed.

@samsonasik
Copy link
Member Author

Just note for step to reproduce:

git clone [email protected]:samsonasik/CodeIgniter4.git
cd CodeIgniter4 
git checkout upgrade-to-phpstan1
composer update

Use PHP 7.4 to run rector:

/opt/homebrew/Cellar/[email protected]/7.4.24_1/bin/php vendor/bin/rector process --clear-cache --dry-run

@samsonasik
Copy link
Member Author

Another note: the error seems still happen even all services and set disabled:

/opt/homebrew/Cellar/[email protected]/7.4.24_1/bin/php vendor/bin/rector process --clear-cache --dry-run |more

 673/673 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
 [ERROR] Could not process "app/Config/Boot/development.php" file, due to:                                              
         "Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE:206, Syntax error, unexpected T_FUNCTION, expecting 
         T_VARIABLE:467". On line: 38                                                                                   

 [ERROR] Could not process "app/Config/Boot/production.php" file, due to:                                               
         "Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE:206, Syntax error, unexpected T_FUNCTION, expecting 
         T_VARIABLE:467". On line: 38                                                                                   

 [ERROR] Could not process "app/Config/Boot/testing.php" file, due to:                                                  
         "Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE:206, Syntax error, unexpected T_FUNCTION, expecting 
         T_VARIABLE:467". On line: 38                                                                                   

 [ERROR] Could not process "app/Config/Events.php" file, due to:                                                        
         "Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE:318, Syntax error, unexpected T_FUNCTION, expecting 
         T_VARIABLE:385, Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE:427, Syntax error, unexpected '}',   
         expecting T_VARIABLE:610, Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE:682, Syntax error,         
         unexpected T_FUNCTION, expecting T_VARIABLE:744, Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE:817,
         Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE:836, Syntax error, unexpected T_FUNCTION, expecting  
         T_VARIABLE:884, Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE:1181, Syntax error, unexpected       
         T_FUNCTION, expecting T_VARIABLE:1217, Syntax error, unexpected '}', expecting T_VARIABLE:1236, Syntax error,  
         unexpected T_FUNCTION, expecting T_VARIABLE:1414, Syntax error, unexpected T_FUNCTION, expecting               
         T_VARIABLE:1481, Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE:1517, Syntax error, unexpected      
         T_FUNCTION, expecting T_VARIABLE:1585, Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE:1657, Syntax  

@samsonasik
Copy link
Member Author

samsonasik commented Nov 3, 2021

thank you @ondrejmirtes for the hint, @TomasVotruba it probably related with PHPStanServicesFactory::createEmulativeLexer() ?

https://github.com/rectorphp/rector-src/blob/6cbc5d4caf64061bd69834a394573f78520f3dd1/packages/NodeTypeResolver/DependencyInjection/PHPStanServicesFactory.php#L62-L65

The $this->container->getService('currentPhpVersionLexer') returns PhpParser\Lexer.

Looking at the Lexer directory, there is Emulative lexer that can choose which target php version to use:

https://github.com/nikic/PHP-Parser/blob/6a21234e58a65a9254178503f2ee6a26d554e7bf/lib/PhpParser/Lexer/Emulative.php#L44

@samsonasik
Copy link
Member Author

samsonasik commented Nov 3, 2021

@TomasVotruba I created repository example with minimal code that reproduce it https://github.com/samsonasik/rector-issue-php74

https://github.com/samsonasik/rector-issue-php74/blob/252543826acf69e223ba93d43ab49ef818aa7bbe/src/Test.php#L3-L5

It has Github action to show the error at https://github.com/samsonasik/rector-issue-php74/runs/4097757321?check_suite_focus=true#step:5:6

show error:

 vendor/bin/rector process src/Test.php --dry-run
  shell: /usr/bin/bash -e {0}
 0/1 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░]   0%

Error: ] Could not process "src/Test.php" file, due to:                         
         "Syntax error, unexpected T_FUNCTION, expecting T_VARIABLE:1043". On   
         line: 38       

Step to reproduce locally:

git clone [email protected]:samsonasik/rector-issue-php74.git
cd rector-issue-php74
composer update

# run with php 7.4
/opt/homebrew/Cellar/[email protected]/7.4.24_1/bin/php vendor/bin/rector process src/Test.php

@samsonasik
Copy link
Member Author

@samsonasik
Copy link
Member Author

@TomasVotruba I added e2e/define-constant failing test case per rectorphp/rector-src#1139 that displayed error on:

https://github.com/rectorphp/rector/runs/4106986477?check_suite_focus=true#step:5:7

for PHP 7.x

@TomasVotruba
Copy link
Member

It seems the provided lexer is using different PHP that current active one:
https://github.com/phpstan/phpstan-src/blob/49a77b6e1726dd5363c067ae4c6ba8dcce45ba0b/src/Parser/LexerFactory.php#L21

@TomasVotruba
Copy link
Member

See

https://github.com/phpstan/phpstan-src/blob/49a77b6e1726dd5363c067ae4c6ba8dcce45ba0b/src/Php/PhpVersionFactory.php#L23-L37

The solution might be to override this lexer service to work with our PHP version somehow

@samsonasik
Copy link
Member Author

@TomasVotruba I tried to override that, but it seems the PHPStan\Php\PhpVersion is never exists yet, so this code doesn't has any effect:

// services.php
    $services->set(\Rector\Php\PhpVersion::class)
        ->factory([service(\Rector\Core\PHPStan\RectorPhpVersionFactory::class), 'create']);

    $services->alias(\PHPStan\Php\PhpVersion::class, \Rector\Php\PhpVersion::class);
<?php
// src/PHPStan/RectorPhpVersionFactory.php
declare (strict_types=1);

namespace Rector\Core\PHPStan;

final class RectorPhpVersionFactory
{
    public function __construct(private \Rector\Core\Php\PhpVersionProvider $phpVersionProvider)
    {
    }

    public function create(): \PHPStan\Php\PhpVersion
    {
        $factory = new \PHPStan\Php\PhpVersionFactory(null, $this->phpVersionProvider->resolve());
        return $factory->create();
    }
}

@samsonasik
Copy link
Member Author

that's still not works even moved to PHPStanServicesFactory:

final class PHPStanServicesFactory
{
    // ...
    public function createPHPStanPhpVersion(): \PHPStan\Php\PhpVersion
    {
        $phpVersionProvider = $this->container->gerService(\Rector\Core\Php\PhpVersionProvider::class);

        $factory = new \PHPStan\Php\PhpVersionFactory(null, $phpVersionProvider->resolve());
        return $factory->create();
    }
}

with services:

    $services->set(\PHPStan\Php\PhpVersion::class)
        ->factory([service(PHPStanServicesFactory::class), 'createPHPStanPhpVersion']);

as it seems the \PHPStan\Php\PhpVersion::class is never called actually.

@TomasVotruba
Copy link
Member

This must happen in config that is used by PHPStan, e.g. here:
https://github.com/rectorphp/rector-src/blob/main/config/phpstan/parser.neon

@samsonasik
Copy link
Member Author

@TomasVotruba I tried register the service with:

services:
    defaultAnalysisParser:
            factory: @currentPhpVersionRichParser
            arguments!: []
    -
        class: PHPStan\Php\PhpVersion
        factory: @Rector\NodeTypeResolver\DependencyInjection\PHPStanServicesFactory::createPHPStanPhpVersion

and got error:

Next _PHPStan_bb49b3bd7\Nette\DI\ServiceCreationException: 

Service of type PHPStan\Parser\LexerFactory: Multiple services of type PHPStan\Php\PhpVersion found : 07, 0242 (needed by $phpVersion in __construct()) in phar:///Users/samsonasik/www/rector-src/vendor/phpstan/phpstan/phpstan.phar/vendor/nette/di/src/DI/Resolver.php:442

any way to set alias for it?

@TomasVotruba
Copy link
Member

TomasVotruba commented Nov 5, 2021

@samsonasik I think the previous must be overriden. Not sure if NEON allows it, in case of anonymous service:

https://github.com/phpstan/phpstan-src/blob/194ff6e8546b7922bb1f579975f268d1e9822a69/conf/config.neon#L337-L339

cc @ondrejmirtes Any ideas how to give PHPStan different PHP version?

@ondrejmirtes
Copy link
Contributor

@TomasVotruba Hi, I think you're taking the wrong approach here and doing the wrong thing. I'd much rather have a phone call with you again and solved it together :)

@TomasVotruba
Copy link
Member

@ondrejmirtes Hey, that would be the best :)
Could you drop me 2 terms on Sunday?

@ondrejmirtes
Copy link
Contributor

@TomasVotruba I'm busy this weekend, what about Monday morning?

@TomasVotruba
Copy link
Member

Sounds good. 10 AM?

@ondrejmirtes
Copy link
Contributor

Alright 😊

@ondrejmirtes
Copy link
Contributor

Just submitted a fix for this: rectorphp/rector-src#1184

The problem was that Rector uses (based on my advice which I haven't realized might be wrong for this use-case) "current version PHP parser". And that doesn't work when phpstorm-stubs need to be parsed.

PHPStan doesn't have this problem thanks to "PathRoutingParser": https://github.com/phpstan/phpstan-src/blob/master/src/Parser/PathRoutingParser.php (PHPStan wants to get parse errors when user writes PHP 8 code on PHP 7.x but still needs to parse phpstorm-stubs with PHP 8 parser).

I think for Rector it's best to just always use PHP 8 parser.

@TomasVotruba
Copy link
Member

TomasVotruba commented Nov 8, 2021

Thank you for the explanation and solution Ondra 👍

Rector tries to parse any code use has and then handle the refactoring that user wants. Parse error and current version are not relevant in this process.

Maybe the naming is confusing to me, but out of curiosity, is PHP 8 parser able to parse PHP 5-only code?

@ondrejmirtes
Copy link
Contributor

Maybe the naming is confusing to me, but out of curiosity, is PHP 8 parser able to parse PHP 5-only code?

Yeah, I think so, there's no syntax in PHP 5 that was removed later.

@TomasVotruba
Copy link
Member

TomasVotruba commented Dec 13, 2021

Parsing stubs and user's code with same parser version causes bug in #6799

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

Successfully merging a pull request may close this issue.

3 participants