Skip to content

Conversation

@Wohlie
Copy link
Contributor

@Wohlie Wohlie commented Nov 26, 2022

Fix rectorphp/rector#7592

The issue rectorphp/rector#7570 can be changed to improvement, see expected result.

@Wohlie Wohlie requested a review from TomasVotruba as a code owner November 26, 2022 21:06
@Wohlie Wohlie force-pushed the fix-array-key-evaluation-on-foreach-to-array-filter-rector branch from 24de7a4 to 22ab55d Compare November 26, 2022 21:12
Comment on lines 143 to 148
$parentArrayDimFetch = $keyVarUsage[0]->getAttribute(AttributeKey::PARENT_NODE);
if (! $parentArrayDimFetch instanceof ArrayDimFetch) {
return true;
}

return $parentArrayDimFetch->dim !== $keyVarUsage[0];
Copy link
Member

@samsonasik samsonasik Nov 28, 2022

Choose a reason for hiding this comment

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

I tried locally, you can actually use ReadExprAnalyzer service:

Suggested change
$parentArrayDimFetch = $keyVarUsage[0]->getAttribute(AttributeKey::PARENT_NODE);
if (! $parentArrayDimFetch instanceof ArrayDimFetch) {
return true;
}
return $parentArrayDimFetch->dim !== $keyVarUsage[0];
return ! $this->readExprAnalyzer->isExprRead(current($keyVarUsage));

You need to inject the constructor with Rector\ReadWrite\NodeAnalyzer\ReadExprAnalyzer service.

Copy link
Member

Choose a reason for hiding this comment

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

@Wohlie I updated to use ReadExprAnalyzer service to your branch 76fa681

@samsonasik samsonasik force-pushed the fix-array-key-evaluation-on-foreach-to-array-filter-rector branch from 22ab55d to 76fa681 Compare December 3, 2022 06:52
@samsonasik samsonasik enabled auto-merge (squash) December 3, 2022 06:52
@samsonasik
Copy link
Member

Thank you @Wohlie

@samsonasik samsonasik merged commit 139c19c into rectorphp:main Dec 3, 2022
@Wohlie
Copy link
Contributor Author

Wohlie commented Dec 3, 2022

Thanks @samsonasik for fixing!

@Wohlie Wohlie deleted the fix-array-key-evaluation-on-foreach-to-array-filter-rector branch December 3, 2022 11:36
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.

Incorrect behavior of SimplifyForeachToArrayFilterRector

2 participants