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

Refactor Symfony route names and paths to snake_case with Rector #8868

Open
ahmed-bhs opened this issue Oct 22, 2024 · 7 comments
Open

Refactor Symfony route names and paths to snake_case with Rector #8868

ahmed-bhs opened this issue Oct 22, 2024 · 7 comments
Labels

Comments

@ahmed-bhs
Copy link

Hi Rector team,

Following the recent acceptance of the proposal to use snake_case for routes in Symfony, as detailed in this Symfony documentation pull request, I’d like to suggest adding a Rector rule that automates the refactoring of route names and paths to snake_case.

Why is this useful?

  1. Consistency: Symfony is moving towards snake_case for route naming and paths, as it’s more readable and aligns with common practices in other frameworks and languages like Python and Ruby.
  2. Industry standards: Major companies like Google and Facebook use snake_case in their APIs. Aligning Symfony routes with this standard will help developers transitioning from other ecosystems.
  3. Automation: Having a Rector rule will make it easier to apply these changes across projects, ensuring uniformity without manual intervention.

Next Steps:

  • Would this be considered useful by the Rector team?
  • If so, could you guide me on how to proceed with implementing or contributing to this rule? I'd love to contribute and collaborate to make this happen.

Looking forward to your feedback and guidance!

Best regards,

@TomasVotruba
Copy link
Member

TomasVotruba commented Oct 22, 2024

Hi, thanks for proposal.

Sounds good to me. Feel free to send the rule 👍

The best way to start is with the docs for custom rules: https://getrector.com/documentation/custom-rule

@ahmed-bhs
Copy link
Author

ahmed-bhs commented Oct 22, 2024

Hi @TomasVotruba,
Thank you for considering the idea. I appreciate your attention to this matter.

Here’s the plan:

  1. Objective: Create a custom Rector class that renames all route names and paths to snake_case if they are currently in camelCase, kebab-case, or any other format.

  2. Implementation Steps:

    • Utilize the PhpParser to access the route attribute/annotation definitions.
    • Retrieve all route attributes defined in PHP classes.
    • Use the Symfony String component to convert names and paths to snake_case.
    • Implement logic to also search for route definitions in Twig, YAML, and automatically rename them.
    • Ensure the new naming conventions are applied consistently across all identified routes.
  3. Class Structure:

    • Create a class class SnakeCaseRouteRector that extends AbstractRector.
    • Implement the refactor method to iterate over the routes.
    • Define a private method to rename each route and convert it to snake_case.
    • Implement logic for handling Twig, YAML files for renaming routes.
  4. Testing:

    • Thoroughly test the Rector class to ensure it correctly renames routes without breaking functionality.
  5. Documentation:

    • Document the new features and usage of the custom Rector class.

If you have any comments or concerns about anything, just let me know. I’m eager to hear your thoughts on this plan!

@TomasVotruba
Copy link
Member

Looks good. Let's start with the Rector rule 👍

@ahmed-bhs
Copy link
Author

Hi @TomasVotruba,

I’ve successfully implemented the rule to modify all the annotations and attributes in Symfony routes. However, I'm now focusing on how to apply these modifications to Twig templates while considering dry-run mode.

I started with the following code to update the Twig files:

private function updateTwigFiles(string $oldRouteName, string $newRouteName): void
{
    $finder = new Finder();
    $directory = 'templates'; // Adjust the path as necessary

    // Find all .twig files recursively
    $finder->files()->in($directory)->name('*.twig');

    foreach ($finder as $file) {
        $content = file_get_contents($file->getRealPath());
        $content = str_replace($oldRouteName, $newRouteName, $content);
        file_put_contents($file->getRealPath(), $content);
    }
}

The problem is that this approach modifies the files directly, which isn’t suitable for dry-run functionality.

Could you suggest a way to simulate these changes in the Twig templates without applying them immediately? Any insights or recommendations would be greatly appreciated!

Thank you!

@TomasVotruba
Copy link
Member

Rector can only handle PHP files. We've tried changing non-PHP files before, but it only lead to problems like you describe:
https://getrector.com/blog/rector-018-refocus-on-php

We can either limit the rule to handle PHP, or better create a custom tool to handle your use case.

@JoolsMcFly
Copy link

Hi.

Just a thought but couldn't dry-run just output a diff?
So instead of writing changes back to the Twig file you could output them (using xdiff_string_diff I suppose) so it would result in:

templates/some/file.html.twig
- path("myRoute")
+ path("my_route")

My 2 cents.

@ahmed-bhs
Copy link
Author

Hey @JoolsMcFly,

We could actually use sebastianbergmann/diff, which is already included in Rector. The catch is that we need to connect the rule logic with the ApplicationFileProcessor. It seems like this has already been tried, but it caused some issues, as mentioned by Tomas Votruba. So, for now, I think I’ll keep the rule focused solely on PHP classes.

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

No branches or pull requests

3 participants