Minor indentation bug with template php code where statements have comments to the right.

[stilliard]

Bug Report

Subject	Details
Rector version	v1.0.2 (maybe other versions)

Hi, this is a bit of a weird one but on the legacy code I'm upgrading there's a a lot of comments after if statements etc. for inline php in HTML templates, such as:

<ul>
    <?php if (true) { // comment ?>
    	<li><a href="/test">test</a></li>
    <?php } ?>
</ul>

Rector runs over the code fine but it changes the indentation of the closing tag for some reason.
It's to do with the comment being there as removing the comment in the code fixes it.

Resulting diff from running rector:

@@ -1,5 +1,5 @@
 <ul>
     <?php if (true) { // comment ?>
     	<li><a href="/test">test</a></li>
-    <?php } ?>
+<?php } ?>
 </ul>

Minimal PHP Code Causing Issue

Examples with each type of comment that results in indentation change:
https://getrector.com/demo/4565e8a3-8ca6-458d-bb9f-a88dc83935ee
https://getrector.com/demo/a25d56d0-c6ad-43de-b5d6-29a389e3a7e6
https://getrector.com/demo/5f58a4fc-7888-4d5c-891b-4c8735085310

Example without comment where it doesn't change indentation:
https://getrector.com/demo/b1f7573b-445c-4666-b3ce-12141dca9b50

Above examples show it happening to if statements but tested and I believe all similar block statements have this issue, such as foreach: https://getrector.com/demo/1cf98379-d3e1-4310-90fe-ca75c009588f

Expected Behaviour

Is it possible to have Rector skip this change?
Has anyone else run into a similar issue?

I can resolve by removing the comments if needed but there's many throughout this project that'd need switching to either remove or show the comment above where it's needed.

I wonder if there's a way to have Rector skip changes like this where no other issue were spotted in that diff?

Thank you.

[samsonasik]

That known drawaback https://github.com/rectorphp/rector?tab=readme-ov-file#known-drawbacks

You can skip the rule via configuration https://getrector.com/documentation/ignoring-rules-or-paths

[stilliard]

Hi @samsonasik
Thanks for your reply, which rule is causing this? I'll add it to be skipped then.

[stilliard]

@samsonasik
Looking over the available rules, I don't see any about handling of statements like this nor comments?
Which rule are you referring to?

Ref known drawbacks, are you referring to the use of the AST with issues with spaces or that it's a mix of HTML & PHP?

I'm wondering if we can resolve the issue by having rector check if rules match a block or even at a file level and if there were no matching rules then it'd not apply the indentation change?

[samsonasik]

When comparing content, php-parser print it first, and during print, the space maybe wiped on php-parser side, that unrelated with any rule, you can skip the path instead for that.

[stilliard]

@samsonasik ok I'll investigate it in the php-parser directly as I suspect it's an issue with it handling trailing comments. Adding a null; after the comment but before the closing ?> resolves it so will investigate what's possible there, ref https://getrector.com/demo/3c8259ac-e3f3-44a2-aa86-606fd619652d

Is there any way to have rector check if there were matching rules in a block or file maybe and not update the code if none matched?

I can't easily exclude these files as they too need rector rules applied.

[samsonasik]

it probably was already handled before in the history:

• [Core][e2e] Handle space before content and space before open tag on deep content on PhpFileProcessor rector-src#3418

since we don't support html + php support since long time ago, I am not sure if that actually need to bring back, as the support should be in php-parser.

but you may can try in FileProcessor https://github.com/rectorphp/rector-src/blob/main/src/Application/FileProcessor.php#L28 by verify:

    /**
     * @var string
     * @see https://regex101.com/r/xP2MGa/1
     */
    private const OPEN_TAG_SPACED_REGEX = '#^(?<open_tag_spaced>[^\S\r\n]+\<\?php)#m';

and add line after 150 after compare string equal:

https://github.com/rectorphp/rector-src/blob/c14f8d45175bd4c10fef503e350d677c4a73ce29/src/Application/FileProcessor.php#L148-L150

            if (preg_replace(self::OPEN_TAG_SPACED_REGEX, '<?php', $ltrimOriginalFileContent) === $newContent) {
                return;
            }

if that works for you, I will think if that can be back, while still will need @TomasVotruba decision.

[samsonasik]

Just checked, it seems the open tag spaced regex not working on this case if bring back, so probably need special handling, but not sure if it worths the effort.

[samsonasik]

If you're insterested to cover regex on this, you can create PR to https://github.com/rectorphp/rector-src

1. Add fixture under tests/Issues/NoNamespaced/Fixture https://github.com/rectorphp/rector-src/tree/main/tests/Issues/NoNamespaced/Fixture with php+html content you have, with name, eg: open_tag_spaced_with_comment.php.inc
2. Try regex thing like above with modified version if needed.
3. run test:

vendor/bin/phpunit tests/Issues/NoNamespaced

[stilliard]

@samsonasik thanks, appreciate your help with this, checking the rectorphp/rector-src repo out now to test with 👍

[stilliard]

@samsonasik I've submitted a PR which resolves the issue by stripping the spaces before the original and new, what do you think?
rectorphp/rector-src#5701
Thanks for your help on this btw.