Skip to content

Add heredoc test to StringReplaceRector#32

Open
koertho wants to merge 1 commit intocontao:mainfrom
koertho:feature/string_replace_rector_test_heredoc
Open

Add heredoc test to StringReplaceRector#32
koertho wants to merge 1 commit intocontao:mainfrom
koertho:feature/string_replace_rector_test_heredoc

Conversation

@koertho
Copy link
Copy Markdown

@koertho koertho commented Jun 13, 2025

This PR add a heredoc test to StringReplaceRector to catch the case in #31
I hope I got it right.

Copy link
Copy Markdown
Member

@zoglo zoglo left a comment

Choose a reason for hiding this comment

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

It's not exactly what I meant by #31 (comment).

The test itself is just something to validate the Rector Rule and has no effect on the outcome of the refactoring.

@zoglo
Copy link
Copy Markdown
Member

zoglo commented Jun 13, 2025

What I meant is that we can explicitly skip the DOC types or we could include them by not rewriting the node but actually changing the value and thus triggering a repaint of the $node itself (keep in mind that changing attributes would need a manual repaint but that's not needed in this case).

Refactoring the string with the indent can be tricky and needs to be checked if it's possible. I do know that there is a DOC_INDENTATION attribute but that one is bound to the identifier, not the string itself.

TL;DR:
Basically using $node->value and returning the original node instead of returning a new string within the StringReplaceRector.

@koertho
Copy link
Copy Markdown
Author

koertho commented Jun 13, 2025

then we misunderstood each other :D
I will have a look into it next week.

@aschempp
Copy link
Copy Markdown
Member

ping @koertho

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.

3 participants