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

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

Merged
merged 3 commits into from
Jun 28, 2024

Conversation

TomasVotruba
Copy link
Member

@TomasVotruba TomasVotruba commented Jun 28, 2024

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
@TomasVotruba
Copy link
Member Author

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

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

@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
Member Author

TomasVotruba commented Jun 28, 2024

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
33 checks passed
@TomasVotruba TomasVotruba deleted the tv-skip-short-tags-as-crashing branch June 28, 2024 15:11
@TomasVotruba
Copy link
Member Author

TomasVotruba commented Jun 28, 2024

Let's see how downgrade goes ...

Edit: all good ✔️
rectorphp/rector@bcb11a7

@samsonasik
Copy link
Member

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

@samsonasik
Copy link
Member

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
Member Author

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
Member

I will try

@samsonasik
Copy link
Member

samsonasik commented Jun 28, 2024

Windows filter files symlink fixed at PR:

@staabm
Copy link
Contributor

staabm commented Jun 28, 2024

What exactly needs to be tested/fixed in windows?

@samsonasik
Copy link
Member

@samsonasik
Copy link
Member

I created PR to report files start with <?=

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