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

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

Merged
merged 11 commits into from Jan 18, 2023

Conversation

jawira
Copy link
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 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
Copy link
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
Member

Thank you 👏

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

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
Member

samsonasik commented Jan 18, 2023

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
Member

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
Member

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

@samsonasik
Copy link
Member

@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
Labels
None yet
Projects
None yet
3 participants