Skip to content

Conversation

@kenjis
Copy link
Member

@kenjis kenjis commented Dec 21, 2023

Important

All refactoring will be performed just prior to the release of v4.5.0.

Description
Supersedes #7964
Closes #6921

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added refactor Pull requests that refactor code 4.5 labels Dec 21, 2023
@kenjis kenjis marked this pull request as draft December 21, 2023 02:45
@ddevsr
Copy link
Collaborator

ddevsr commented Dec 21, 2023

@kenjis I suggest rules ChangeSwitchToMatchRector by Rector\Php80\Rector\Switch_\ChangeSwitchToMatchRector

@kenjis
Copy link
Member Author

kenjis commented Dec 21, 2023

Doesn't it included in LevelSetList::UP_TO_PHP_81?

@ddevsr
Copy link
Collaborator

ddevsr commented Dec 21, 2023

Doesn't it included in LevelSetList::UP_TO_PHP_81?

Sorry i didnt read it

@kenjis
Copy link
Member Author

kenjis commented Dec 21, 2023

@samsonasik When rector is executed on this branch, rector enters an infinite loop, because of the following files:

            __DIR__ . '/system/Common.php',
            __DIR__ . '/system/Helpers/filesystem_helper.php',
            __DIR__ . '/system/Helpers/text_helper.php',
            __DIR__ . '/system/Helpers/form_helper.php',
            __DIR__ . '/system/Helpers/html_helper.php',
            __DIR__ . '/system/Helpers/number_helper.php',

Rector adds too many declare(strict_types=1); lines to them.
Is this a bug?

@samsonasik
Copy link
Member

@kenjis probably, could you create a reproducible at https://getrector.com/demo and report at https://github.com/rectorphp/rector/ ? Thank you.

@github-actions github-actions bot added the stale Pull requests with conflicts label Dec 22, 2023
@github-actions
Copy link

👋 Hi, @kenjis!

We detected conflicts in your PR against the base branch 🙊
You may want to sync 🔄 your branch with upstream!

Ref: Syncing Your Branch

@kenjis kenjis mentioned this pull request Dec 26, 2023
4 tasks
@kenjis kenjis force-pushed the rector-upgrade-php81-CI45 branch from 4a389b8 to f962917 Compare February 2, 2024 12:04
@kenjis kenjis removed the stale Pull requests with conflicts label Feb 2, 2024
@kenjis kenjis mentioned this pull request Feb 3, 2024
5 tasks
@github-actions github-actions bot added the stale Pull requests with conflicts label Feb 8, 2024
@github-actions
Copy link

github-actions bot commented Feb 8, 2024

👋 Hi, @kenjis!

We detected conflicts in your PR against the base branch 🙊
You may want to sync 🔄 your branch with upstream!

Ref: Syncing Your Branch

@kenjis kenjis force-pushed the rector-upgrade-php81-CI45 branch from f962917 to 9561166 Compare February 25, 2024 03:02
@kenjis kenjis removed the stale Pull requests with conflicts label Feb 25, 2024
@kenjis
Copy link
Member Author

kenjis commented Feb 25, 2024

@samsonasik These files with too many declare(strict_types=1) are all functions, not class files.
But I cannot make a simple reproducible sample code.

@samsonasik
Copy link
Member

@kenjis the DeclareStrictTypesRector can be skipped if it make too many changes, and create separate PR to enable that?

@samsonasik
Copy link
Member

Actually, the DeclareStrictTypesRector is not part of any set list now, so should not apply the changes except registered, I can't see any addition of declare(strict_types=1) now in github diff

@kenjis kenjis force-pushed the rector-upgrade-php81-CI45 branch from 6bd5ebf to b788264 Compare February 25, 2024 10:00
@github-actions github-actions bot added the stale Pull requests with conflicts label Feb 26, 2024
@github-actions
Copy link

👋 Hi, @kenjis!

We detected conflicts in your PR against the base branch 🙊
You may want to sync 🔄 your branch with upstream!

Ref: Syncing Your Branch

@kenjis kenjis force-pushed the rector-upgrade-php81-CI45 branch from cabf2ed to a2bee68 Compare February 26, 2024 01:15
@kenjis kenjis removed the stale Pull requests with conflicts label Feb 26, 2024
@kenjis kenjis force-pushed the rector-upgrade-php81-CI45 branch from a2bee68 to ef8f4c2 Compare February 26, 2024 01:47
@github-actions github-actions bot added the stale Pull requests with conflicts label Mar 9, 2024
@github-actions
Copy link

github-actions bot commented Mar 9, 2024

👋 Hi, @kenjis!

We detected conflicts in your PR against the base branch 🙊
You may want to sync 🔄 your branch with upstream!

Ref: Syncing Your Branch

@kenjis kenjis force-pushed the rector-upgrade-php81-CI45 branch from ef8f4c2 to 39aea7c Compare March 9, 2024 23:51
@kenjis kenjis removed the stale Pull requests with conflicts label Mar 9, 2024
@kenjis kenjis force-pushed the rector-upgrade-php81-CI45 branch from 39aea7c to 2766e4c Compare March 10, 2024 10:01
@samsonasik
Copy link
Member

@kenjis could you rebase and retry add DeclareStrictTypesRector, run composer update, and verify if no longer infinite loop on latest rector 1.0.3? Thank you.

@kenjis kenjis force-pushed the rector-upgrade-php81-CI45 branch from 2766e4c to 557b355 Compare March 16, 2024 01:14
@kenjis
Copy link
Member Author

kenjis commented Mar 16, 2024

@samsonasik Thank you! It worked fine.

[OK] 148 files have been changed by Rector  

Dropped 1ecb049

@kenjis kenjis force-pushed the rector-upgrade-php81-CI45 branch from 557b355 to 531f9c7 Compare March 16, 2024 01:28
@kenjis kenjis force-pushed the rector-upgrade-php81-CI45 branch 4 times, most recently from 85d4956 to 57bee92 Compare April 2, 2024 22:55
@kenjis kenjis marked this pull request as ready for review April 2, 2024 23:45
@kenjis kenjis force-pushed the rector-upgrade-php81-CI45 branch from 57bee92 to aaa37ad Compare April 6, 2024 00:24
@kenjis kenjis merged commit 77cbf2c into codeigniter4:4.5 Apr 7, 2024
@kenjis kenjis deleted the rector-upgrade-php81-CI45 branch April 7, 2024 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Pull requests that refactor code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants