Skip to content
/ rector-src Public

Skip files with short <?= PHP tag as leads to invalid changes#6068

Merged
TomasVotruba merged 3 commits intomainfrom
tv-skip-short-tags-as-crashing
Jun 28, 2024
Merged

Skip files with short <?= PHP tag as leads to invalid changes#6068
TomasVotruba merged 3 commits intomainfrom
tv-skip-short-tags-as-crashing

Conversation

@TomasVotruba
Copy link
Copy Markdown
Member

@TomasVotruba TomasVotruba commented Jun 28, 2024 •
edited
Loading

Currently, PHP allows to use <?= tags intead of typical <?php ...: https://www.php.net/manual/en/language.basic-syntax.phptags.php

Yet, such tags would lead to breaking changes like:

+<?php
+
+declare(strict_types=1);
 <?= $something ?>

We currently skip this on importing files, but we should not process such files at all, to avoid breaking code.


Use php-cs-fixer rule https://cs.symfony.com/doc/rules/php_tag/echo_tag_syntax.html to migrate your short tags. Then use Rector safely 👍

@TomasVotruba TomasVotruba force-pushed the tv-skip-short-tags-as-crashing branch from 063b699 to bdd783c Compare June 28, 2024 14:48
@TomasVotruba TomasVotruba force-pushed the tv-skip-short-tags-as-crashing branch from bdd783c to 58957d5 Compare June 28, 2024 14:49
@TomasVotruba TomasVotruba changed the title Skip files with echo PHP tags as leads to invalid changes Skip files with short <?= PHP tag as leads to invalid changes Jun 28, 2024
@TomasVotruba TomasVotruba force-pushed the tv-skip-short-tags-as-crashing branch from d043a71 to 34ab183 Compare June 28, 2024 14:54
samsonasik
samsonasik reviewed Jun 28, 2024
View reviewed changes
@TomasVotruba
Copy link
Copy Markdown
Member Author

TomasVotruba commented Jun 28, 2024

Checking <?= in /vendor, there are only dev packages with string match:

Screenshot from 2024-06-28 16-58-06

samsonasik
samsonasik reviewed Jun 28, 2024
View reviewed changes
@TomasVotruba TomasVotruba force-pushed the tv-skip-short-tags-as-crashing branch from 34ab183 to 642ca88 Compare June 28, 2024 15:03
@TomasVotruba
Copy link
Copy Markdown
Member Author

TomasVotruba commented Jun 28, 2024 •
edited
Loading

Seem broken symlinks does not allow content filter on Finder on Windows:
Screenshot from 2024-06-28 17-04-20

I don't see any option than to other skip this test on Windows, as bug in symfony/finder.

If someone is using Windows, a fix would be great :)

@TomasVotruba TomasVotruba merged commit a77b00d into main Jun 28, 2024
@TomasVotruba TomasVotruba deleted the tv-skip-short-tags-as-crashing branch June 28, 2024 15:11
@TomasVotruba
Copy link
Copy Markdown
Member Author

TomasVotruba commented Jun 28, 2024 •
edited
Loading

Let's see how downgrade goes ...

Edit: all good ✔️
rectorphp/rector@bcb11a7

@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Jun 28, 2024

@staabm @paulbalandan are using windows iirc, could you give some insight? Thank you.

@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Jun 28, 2024

The release note should include the recommendation for migrating to long php tag,

Also probably better, add warning that list of files will not be processed, so user can know that the code should be migrated to long php tag first when running it ...

@TomasVotruba
Copy link
Copy Markdown
Member Author

TomasVotruba commented Jun 28, 2024

Also probably better, add warning that list of files will not be processed, so user can know that the code should be migrated to long php tag first when running it ...

That would be helpful indeed. Could you add this?

@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Jun 28, 2024

I will try

@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Jun 28, 2024 •
edited
Loading

Windows filter files symlink fixed at PR:

@samsonasik samsonasik mentioned this pull request Jun 28, 2024
Merged
@staabm
Copy link
Copy Markdown
Contributor

staabm commented Jun 28, 2024

What exactly needs to be tested/fixed in windows?

@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Jun 28, 2024

@staabm on windows already fixed at PR:

@TomasVotruba TomasVotruba mentioned this pull request Jun 28, 2024
Closed
@samsonasik samsonasik mentioned this pull request Jun 29, 2024
Merged
@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Jun 29, 2024

I created PR to report files start with <?=

@samsonasik samsonasik mentioned this pull request Sep 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samsonasik samsonasik samsonasik left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@TomasVotruba @samsonasik @staabm