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

Fix missing stmt on adding a new node #6383

Merged
merged 3 commits into from
May 7, 2021
Merged

Conversation

TomasVotruba
Copy link
Member

No description provided.

reset($all);
if (($inputArgument = isset($all[$key = key($all)]) ? $all[$key = key($all)] : null) && 'command' === $inputArgument->getName()) {
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samsonasik BTW, today I find one nasty bug in Rector.
This might be one reason why the downgrade rules were not working sometimes.

This code previously failed and missed all the removed types. Causing missmatch:

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomasVotruba thank you 👍

Comment on lines +145 to +149
$currentStmt = $node->getAttribute(AttributeKey::CURRENT_STATEMENT);
if ($currentStmt instanceof Stmt) {
return spl_object_hash($currentStmt);
}

Copy link
Member Author

@TomasVotruba TomasVotruba May 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix that helped. It might require more parent node/stmt connecting fixes though

@TomasVotruba TomasVotruba changed the title downgrade covariant mix bug Fix missing stmt on adding a new node May 7, 2021
Comment on lines -141 to -142
if ($foundNode === null) {
$foundNode = $node;
Copy link
Member Author

@TomasVotruba TomasVotruba May 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why I put this here (probably not knowing difference between Stmt and Expr in that time), but it was asking for a bug 🤔

If it's null, it has to fail

@TomasVotruba TomasVotruba enabled auto-merge (squash) May 7, 2021 19:33
@TomasVotruba TomasVotruba merged commit b1b5f95 into main May 7, 2021
@TomasVotruba TomasVotruba deleted the downgrade-covariant-mix-bug branch May 7, 2021 19:33
TomasVotruba added a commit that referenced this pull request Oct 15, 2024
rectorphp/rector-src@7be438b [DeadCode] Skip non FullyQualified property type on RemoveTypedPropertyNonMockDocblockRector (#6383)
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.

2 participants