Skip to content

Added failling RemoveUnusedPrivatePropertyRector test-case#3539

Closed
staabm wants to merge 1 commit intorectorphp:mainfrom
staabm:patch-1
Closed

Added failling RemoveUnusedPrivatePropertyRector test-case#3539
staabm wants to merge 1 commit intorectorphp:mainfrom
staabm:patch-1

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented Mar 30, 2023

rector moved a comment related to a propery beeing removed to a different property.
I think the comment should also be removed when on the same line as the property beeing removed

from https://getrector.com/demo/0772a67b-072b-47c0-b268-db16502bca38

@staabm staabm requested a review from TomasVotruba as a code owner March 30, 2023 08:29
Copy link
Copy Markdown
Member

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

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

comment is belong to next stmt, so the change is expected, if it need to be resolved, it may need to be resolved in php-parser itself.

@samsonasik
Copy link
Copy Markdown
Member

It possibly can be tweaked by check there is a Nop node after a Property with same line number, then remove it as well.

@samsonasik
Copy link
Copy Markdown
Member

@staabm I chery-picked your commit at PR #3547

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