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

Enhancement: Add rector config file for easy migration #542

Merged
merged 21 commits into from Dec 13, 2022

Conversation

OskarStark
Copy link

@OskarStark OskarStark commented Dec 5, 2022

This PR is based on the work of https://github.com/thecodingmachine/safe

I have a lot of projects where I want to run a continuous automated refactoring for properties to methods, so I ended up repeating the following code over and over again:

$rectorConfig->ruleWithConfiguration(
    PropertyFetchToMethodCallRector::class,
    [
        new PropertyFetchToMethodCall(Generator::class, 'boolean', 'boolean'),
        new PropertyFetchToMethodCall(Generator::class, 'city', 'city'),
        new PropertyFetchToMethodCall(Generator::class, 'dateTime', 'dateTime'),
        new PropertyFetchToMethodCall(Generator::class, 'dateTimeThisYear', 'dateTimeThisYear'),
        new PropertyFetchToMethodCall(Generator::class, 'email', 'email'),
        new PropertyFetchToMethodCall(Generator::class, 'firstName', 'firstName'),
        new PropertyFetchToMethodCall(Generator::class, 'firstNameMale', 'firstNameMale'),
        new PropertyFetchToMethodCall(Generator::class, 'lastName', 'lastName'),
        new PropertyFetchToMethodCall(Generator::class, 'name', 'name'),
        new PropertyFetchToMethodCall(Generator::class, 'password', 'password'),
        new PropertyFetchToMethodCall(Generator::class, 'phoneNumber', 'phoneNumber'),
        new PropertyFetchToMethodCall(Generator::class, 'sentence', 'sentence'),
        new PropertyFetchToMethodCall(Generator::class, 'sha256', 'sha256'),
        new PropertyFetchToMethodCall(Generator::class, 'slug', 'slug'),
        new PropertyFetchToMethodCall(Generator::class, 'text', 'text'),
        new PropertyFetchToMethodCall(Generator::class, 'url', 'url'),
        new PropertyFetchToMethodCall(Generator::class, 'word', 'word'),
        new PropertyFetchToMethodCall(Generator::class, 'words', 'words'),
    ]);

I think it is better to provide this file via the vendor itself.
Check the README for usage details

Todos

  • complete the list of deprecated properties

WDYT?

friendly ping @silasjoisten @mvhirsch @localheinz

rector-migrate.php Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@pimjansen
Copy link

I dont think this should be part of Faker itself. It would mean Faker has a locked dependency on Rector which is not what you want.

Just publish it as separate package? I think this would be the right thing to go here though

@pierredup
Copy link

It would mean Faker has a locked dependency on Rector which is not what you want.

There is no locked dependency, it is an optional extra file. If you use Rector in your project, you can use this additional config file, but if you don't use Rector, it won't be part of any of your dependencies

README.md Outdated Show resolved Hide resolved
@OskarStark
Copy link
Author

There is no locked dependency, it is an optional extra file. If you use Rector in your project, you can use this additional config file, but if you don't use Rector, it won't be part of any of your dependencies

Exactly 👍

Co-authored-by: Pierre du Plessis <pierre@pcservice.co.za>
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
rector-migrate.php Outdated Show resolved Hide resolved
rector-migrate.php Outdated Show resolved Hide resolved
rector-migrate.php Outdated Show resolved Hide resolved
OskarStark and others added 9 commits December 5, 2022 15:54
Co-authored-by: Andreas Möller <am@localheinz.com>
Co-authored-by: Andreas Möller <am@localheinz.com>
Co-authored-by: Andreas Möller <am@localheinz.com>
rector-migrate.php Outdated Show resolved Hide resolved
@OskarStark
Copy link
Author

Can someone please approve the new workflow to run? Thanks

@bram-pkg
Copy link
Member

bram-pkg commented Dec 5, 2022

Should be running now.

rector-migrate.php Outdated Show resolved Hide resolved
@localheinz localheinz added the enhancement New feature or request label Dec 5, 2022
rector-migrate.php Outdated Show resolved Hide resolved
rector-migrate.php Outdated Show resolved Hide resolved
Copy link
Member

@localheinz localheinz left a comment

Choose a reason for hiding this comment

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

👍

@localheinz localheinz requested review from pierredup and bram-pkg and removed request for pierredup and bram-pkg December 13, 2022 09:55
@localheinz localheinz merged commit cbe48f7 into FakerPHP:main Dec 13, 2022
@localheinz
Copy link
Member

Thank you, @bram-pkg, @OskarStark, @pierredup, and @pimjansen!

@OskarStark OskarStark deleted the feature/rector branch December 13, 2022 10:06
@OskarStark
Copy link
Author

Thank you, can I expect a release soon? 🙏

@localheinz localheinz mentioned this pull request Dec 13, 2022
5 tasks
@DvDty
Copy link

DvDty commented Dec 13, 2022

What is the point of running rector on each PR, if the report is ignored?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants