Skip to content

[CodeQuality][EarlyReturn] Handle crash Replace Stmt to Expr on FlipTypeControlToUseExclusiveTypeRector+FlipTypeControlToUseExclusiveTypeRector+ReturnBinaryOrToEarlyReturnRector#4474

Merged
samsonasik merged 6 commits intomainfrom
crash-replace-stmt-to-expr
Jul 11, 2023

Conversation

@samsonasik
Copy link
Copy Markdown
Member

…ypeControlToUseExclusiveTypeRector+FlipTypeControlToUseExclusiveTypeRector+ReturnBinaryOrToEarlyReturnRector
@samsonasik samsonasik requested a review from TomasVotruba as a code owner July 10, 2023 23:39
@TomasVotruba
Copy link
Copy Markdown
Member

Thanks for the test fixture 👍

Seems the fix commit is missing. Or still WIP? :)

@samsonasik
Copy link
Copy Markdown
Member Author

I am still looking for the solution 👍

@TomasVotruba
Copy link
Copy Markdown
Member

TomasVotruba commented Jul 11, 2023

I can check if you want. It will be some rule returning expr instead of stmt. Not directly in the refactor() method, but in one of nested nodes IMO.

@samsonasik
Copy link
Copy Markdown
Member Author

That seems due to refactor return array of nodes, which inside the nodes doesn't have origNode yet/overlapped on multi rules

@samsonasik
Copy link
Copy Markdown
Member Author

It seems can be tweaked on RectifiedAnalyzer, wait a minute ...

@TomasVotruba
Copy link
Copy Markdown
Member

That seems due to refactor return array of nodes, which inside the nodes doesn't have origNode yet/overlapped on multi rules

That should be ok, untill all those arrays contain only stmts.

@samsonasik
Copy link
Copy Markdown
Member Author

Fixed with verify on re-print has empty origNode, that's means the node is on nested Expr d0e6a73

@samsonasik
Copy link
Copy Markdown
Member Author

All checks have passed 🎉 @TomasVotruba let's give it a try to have faster feedback to test ;)

@samsonasik samsonasik merged commit e45e65c into main Jul 11, 2023
@samsonasik samsonasik deleted the crash-replace-stmt-to-expr branch July 11, 2023 09:42
@samsonasik
Copy link
Copy Markdown
Member Author

It seems cause error on downgrade:



------------------------------------------------------------
Parse error: rector-prefixed-downgraded/packages/BetterPhpDocParser/PhpDocParser/BetterPhpDocParser.php:55
    53|     public function __construct(TypeParser $typeParser, ConstExprParser $constExprParser, CurrentNodeProvider $currentNodeProvider, TokenIteratorFactory $tokenIteratorFactory, iterable $phpDocNodeDecorators, PrivatesAccessor $privatesAccessor = null)
    54|     {
  > 55|         $privatesAccessor ??= new PrivatesAccessor();
    56|         $this->currentNodeProvider = $currentNodeProvider;
    57|         $this->tokenIteratorFactory = $tokenIteratorFactory;
Unexpected '=' in rector-prefixed-downgraded/packages/BetterPhpDocParser/PhpDocParser/BetterPhpDocParser.php on line 55
------------------------------------------------------------
Parse error: rector-prefixed-downgraded/vendor/rector/rector-doctrine/src/NodeManipulator/ColumnPropertyTypeResolver.php:59
    57|     public function __construct(PhpDocInfoFactory $phpDocInfoFactory, TypeFactory $typeFactory, AttributeFinder $attributeFinder, array $doctrineTypeToScalarType = null)
    58|     {
  > 59|         $doctrineTypeToScalarType ??= [
    60|             'tinyint' => new BooleanType(),
    61|             'boolean' => new BooleanType(),
Unexpected '=' in rector-prefixed-downgraded/vendor/rector/rector-doctrine/src/NodeManipulator/ColumnPropertyTypeResolver.php on line 59

https://github.com/rectorphp/rector-src/actions/runs/5518361429/jobs/10062204998#step:15:23

I will check 👍

@canvural
Copy link
Copy Markdown
Contributor

Thank you @samsonasik 🎉

BTW I also get a similar error Trying to replace statement (Stmt_If) with expression (Expr_Instanceof). Are you missing a Stmt_Expression wrapper?

But I could not reproduce this one in the playground. Just writing it here in case it rings a bell for you. Or maybe it'll also be fixed with this PR because both are Stmt

@samsonasik
Copy link
Copy Markdown
Member Author

@canvural yes, that should be resolved in core instead of individual rule to avoid similar issue happen again.

I am looking into it, as current solution cause error on downgrade

@TomasVotruba
Copy link
Copy Markdown
Member

TomasVotruba commented Jul 11, 2023

I'll tag a new version 👍

@samsonasik
Copy link
Copy Markdown
Member Author

@TomasVotruba it not yet resolved, so new tag won't be applied to downgrade as downgrade build error

@TomasVotruba
Copy link
Copy Markdown
Member

TomasVotruba commented Jul 11, 2023

Ah, I see :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

System error: "Trying to replace statement (Stmt_Return) with expression (Expr_BooleanNot). Are you missing a Stmt_Expression wrapper?

4 participants