Skip to content
/ rector-src Public

Add constant FilesystemIterator::SKIP_DOTS when flags parameter is used#3215

Merged
TomasVotruba merged 11 commits intorectorphp:mainfrom
jawira:filesystem_iterator_skip_dots
Jan 18, 2023
Merged

Add constant FilesystemIterator::SKIP_DOTS when flags parameter is used#3215
TomasVotruba merged 11 commits intorectorphp:mainfrom
jawira:filesystem_iterator_skip_dots

Conversation

@jawira
Copy link
Copy Markdown
Contributor

@jawira jawira commented Dec 17, 2022

I created a new rector rule to update \FilesystemIterator , this rule is supposed to be used when migrating code from php 8.1 to 8.2.

Before PHP 8.2, \FilesystemIterator::SKIP_DOTS was always set and couldn't be removed, this has been fixed in PHP 8.2 and now you have to add this constant manually if you want to maintain the same behavior. This is also explained in documentation https://www.php.net/manual/en/filesystemiterator.construct.php.

You can see an example here https://3v4l.org/HrmZ2.

Note that this rule is important only if you use the second parameter ($params) from FilesystemIterator, the reason is that \FilesystemIterator::SKIP_DOTS is part of $params default value.

// Nothing to do since \FilesystemIterator::SKIP_DOTS is part of default value.  
new \FilesystemIterator(__DIR__);

// Must add \FilesystemIterator::SKIP_DOTS to maintain same behavior before PHP 8.2
new \FilesystemIterator(__DIR__, \FilesystemIterator::KEY_AS_FILENAME);

I don't have experience creating Rector rules, so I'm open to any comment/suggestion :)

@jawira jawira requested a review from TomasVotruba as a code owner December 17, 2022 18:39
samsonasik
samsonasik reviewed Dec 22, 2022
View reviewed changes
samsonasik
samsonasik reviewed Dec 22, 2022
View reviewed changes
@jawira jawira marked this pull request as draft December 22, 2022 19:55
@jawira jawira force-pushed the filesystem_iterator_skip_dots branch from a41f412 to 923d4cd Compare December 23, 2022 13:52
@jawira jawira force-pushed the filesystem_iterator_skip_dots branch from 216f808 to 6ee659e Compare December 26, 2022 09:00
@jawira jawira marked this pull request as ready for review December 26, 2022 09:03
samsonasik
samsonasik reviewed Dec 26, 2022
View reviewed changes
samsonasik
samsonasik reviewed Jan 3, 2023
View reviewed changes
samsonasik
samsonasik approved these changes Jan 4, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@TomasVotruba
Copy link
Copy Markdown
Member

TomasVotruba commented Jan 18, 2023

Thank you 👏

@TomasVotruba TomasVotruba merged commit 72bd75f into rectorphp:main Jan 18, 2023
@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Jan 18, 2023

Downgrade build failure seems unrelated:

------------------------------------------------------------
Parse error: rector-prefixed-downgraded/vendor/nette/utils/src/Utils/Arrays.php:171
    169|      */
    170|     public static function grep(array $array, #[\JetBrains\PhpStorm\Language('RegExp')] string $pattern, int $flags = 0) : array
  > 171|     {
    172|         return Strings::pcre('preg_grep', [$pattern, $array, $flags]);
    173|     }
Unexpected '{', expecting variable (T_VARIABLE) in rector-prefixed-downgraded/vendor/nette/utils/src/Utils/Arrays.php on line 171
------------------------------------------------------------
Parse error: rector-prefixed-downgraded/vendor/nette/utils/src/Utils/Strings.php:399
    397|      */
    398|     public static function split(string $subject, #[\JetBrains\PhpStorm\Language('RegExp')] string $pattern, int $flags = 0) : array
  > 399|     {
    400|         return self::pcre('preg_split', [$pattern, $subject, -1, $flags | \PREG_SPLIT_DELIM_CAPTURE]);
    401|     }
Unexpected '{', expecting variable (T_VARIABLE) in rector-prefixed-downgraded/vendor/nette/utils/src/Utils/Strings.php on line 399

seems due to new nette/utils release https://github.com/nette/utils/releases/tag/v3.2.9

@jawira jawira deleted the filesystem_iterator_skip_dots branch January 18, 2023 11:03
@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Jan 18, 2023 •
edited
Loading

The error seems on downgrading to php 7.4 on Attribute in parameter, ref https://3v4l.org/F7j9M#v7.4.33, which may solvable to re-print the parameter to make new line instead of single line https://3v4l.org/o82Pl#v7.4.2

@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Jan 18, 2023

Actually, there is already DowngradeAttributeToAnnotationRector, since it prefixed vendor that we will not use in directly outside rector, I think we can add more register to config/set/downgrade-php80.php in rector-downgrade-php

@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Jan 18, 2023

@TomasVotruba I created PR rectorphp/rector-downgrade-php#36 for it.

@samsonasik
Copy link
Copy Markdown
Member

samsonasik commented Jan 18, 2023

@TomasVotruba I created another PR alternative specific rule to reprint the node when attribute line is equal with node line at PR rectorphp/rector-downgrade-php#37

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samsonasik samsonasik samsonasik approved these changes

@TomasVotruba TomasVotruba Awaiting requested review from TomasVotruba

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

@jawira @TomasVotruba @samsonasik