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

rector adding php open/close tags in view files #6836

Closed
staabm opened this issue Nov 26, 2021 · 3 comments · Fixed by rectorphp/rector-src#1329
Closed

rector adding php open/close tags in view files #6836

staabm opened this issue Nov 26, 2021 · 3 comments · Fixed by rectorphp/rector-src#1329
Labels

Comments

@staabm
Copy link
Contributor

staabm commented Nov 26, 2021

given a view file with contents

	<div>
	<?php echo check_box($auktion, 'skipnetzprovider', '1', "class=checkbox onclick=\"getAngebotHTML($('angebot').value);\""); ?>
	<b class="caption">Provider und Netz nicht anzeigen</b> (Im gesamten Template)
	<br><br>
	</div>

running rector over it produces "unnecessary" changes.
rector adds php starting/ending tags in this files.

this unnecessary changes create noise in bigger PRs

I created a repo which reproduces this case

Steps

  • gh repo clone staabm/rector-repro1
  • composer install
  • vendor/bin/rector process -n

expected result

  • rector might change whitespaces / intentation.. AFAIK its technically hard to keep all whitespaces -> thats fine
  • rector should not add <?php ?> at the beginning of the file
  • rector should not add a trailling <?php

actual result

+<?php
+
+?>

        <div>
-       <?php echo check_box($auktion, 'skipnetzprovider', '1', "class=checkbox onclick=\"getAngebotHTML($('angebot').value);\""); ?>
+       <?php
+echo check_box($auktion, 'skipnetzprovider', '1', "class=checkbox onclick=\"getAngebotHTML($('angebot').value);\"");
+?>
        <b class="caption">Provider und Netz nicht anzeigen</b> (Im gesamten Template)
        <br><br>
-       </div>
+       </div><?php
@staabm staabm added the bug label Nov 26, 2021
@staabm
Copy link
Contributor Author

staabm commented Nov 27, 2021

In case someone could give me some pointers on where to start analyzing, I could try tominvestigate myself

@TomasVotruba
Copy link
Member

TomasVotruba commented Nov 27, 2021

The service responsible for printing is BetterStandardPrinter. It will be in one of its print*() methods.

Tip: Analyse just one file, it will make debugging much easier:

vendor/bin/rector process src/SingleFile.php --debug

@staabm
Copy link
Contributor Author

staabm commented Nov 28, 2021

just leaving some notes here of my ongoing analysis

php-parser already contains code, which should prevent the php tags which are added in the repro case
https://github.com/nikic/PHP-Parser/blob/f09f22760ef1b292c3312105532c8e84e80cce19/lib/PhpParser/PrettyPrinterAbstract.php#L228-L235

it seems this code is only used when using pretty-print but rector is using formatPreserving printing.


rector is processing my view file 4 times. in the first view steps the html is still look like I expect but, in the process the first AST-node is turned into a Rector\Core\PhpParser\Node\CustomNode\FileWithoutNamespace and since then, the open/close tags show up in the printed content-strings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants