Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[make:crud|voter] generate classes with final keyword #1539

Merged
merged 4 commits into from
Aug 29, 2024

Conversation

jrushlow
Copy link
Collaborator

@jrushlow jrushlow commented May 6, 2024

  • introduces 2 new configuration values:

    • generate_final_classes: Defaults to true - When true, all non-entity classes generated by MakerBundle will be final.
    • generate_final_entities: Defaults to false - When true, all entity classes generated by MakerBundle will be final.
  • introduces an internal ClassData object that represents all of the meta data needed to generate a class from a template. It combines parts of ClassNameDetails & UseStatementGenerator, while adding logic to generate a class declaration conditionally including the finalkeyword &extends` class.

  • ClassData is configured by TemplateComponentGenerator to handle root_namespace, generate_final_classes, & generate_final_entities if passed as a template class_data variable to Generator::createClass(). The idea being we don't have to modify the constructor for the "non-internal" Generator::class

  • reduces the complexity of our .tpl.php templates. The ClassData object is accessible from within the templates (Crud & Voter for now). $class_data->getClassDeclaration() generates final class SomeClass extends XyzClass

  • To keep the PR's "reviewable" this PR only adds the new functionality for make:crud & make:voter. To be implemented in additional makers in subsequent PR's.

  • fix test fixtures to use final

  • update this description to include all of the proposed changes

fixes #1536


I think instead of one massive PR, we will merge this PR first to include make:voter & make:crud -> create subsequent PR's for makers that touch entities (1 pr) and the rest of the makers (1 pr). I say this because this PR is actually adding a handful of new features (mostly internal) in order to generate final classes.

@jrushlow jrushlow added Feature New Feature Status: Needs Review Needs to be reviewed labels May 6, 2024
@seb-jean
Copy link
Contributor

seb-jean commented May 6, 2024

Thanks Jesse

@jrushlow
Copy link
Collaborator Author

jrushlow commented May 6, 2024

Hmm. So I don't think we can final classes by default without causing confusion amongst the consumers of MakerBundle. I'm not 100% on my thought process here, but doctrine orm is using lazy ghosts by default - atleast that's what we're doing in the DoctrineBundleRecipe -> https://github.com/symfony/recipes/blob/8cba886fb716a49e654845980cba33e211288a9b/doctrine/doctrine-bundle/2.9/config/packages/doctrine.yaml#L11

Like the failed tests for this PR, wouldn't we trigger logic exceptions everywhere in userland code similar to:

Symfony\Component\VarExporter\Exception\LogicException: Cannot generate lazy ghost: class "Symfony\Bundle\MakerBundle\Tests\tmp\current_project\src\Entity\User" is final.

if all the generated entities were final, we'd end up forcing the user to either a) remove the final keyword everytime they make:user|entity|security etc... || b) set orm.enable_lazy_ghost_objects: false. Both of those options are not viable.

I think the only way we can implement the final keyword for generated entities would be on a opt-in basis with a --is-final flag. We should be able to implement the final keyword for non-entity classes by default.

If I'm missing something obvious - please let me know.. Thoughts @symfony/mergers ?

@jrushlow jrushlow added this to the v1.60.0 milestone May 6, 2024
@jrushlow jrushlow force-pushed the feature/final-classes branch from 37bcfbd to e9ee254 Compare May 14, 2024 14:50
@jrushlow jrushlow force-pushed the feature/final-classes branch from 8d97b3b to ee87da1 Compare May 16, 2024 10:42
@jrushlow jrushlow mentioned this pull request May 16, 2024
1 task
@jrushlow jrushlow added Status: Needs Work Additional work is needed and removed Status: Needs Review Needs to be reviewed labels May 16, 2024
@jrushlow jrushlow force-pushed the feature/final-classes branch from 522869b to 26d91de Compare May 19, 2024 17:44
@jrushlow jrushlow changed the title [make:*] generate all classes with final keyword [make:crud|voter] generate classes with final keyword May 19, 2024
@jrushlow jrushlow added Status: Needs Review Needs to be reviewed and removed Status: Needs Work Additional work is needed labels May 19, 2024
@jrushlow jrushlow force-pushed the feature/final-classes branch from 26d91de to 2d0e7d0 Compare May 28, 2024 14:21
@jrushlow jrushlow modified the milestones: v1.60.0, v1.61.0 Jun 10, 2024
@jrushlow jrushlow force-pushed the feature/final-classes branch from 2d0e7d0 to 7ec0685 Compare August 29, 2024 15:32
@jrushlow jrushlow force-pushed the feature/final-classes branch from 7ec0685 to 988f765 Compare August 29, 2024 15:54
@jrushlow jrushlow merged commit f0d1574 into symfony:main Aug 29, 2024
7 of 8 checks passed
@jrushlow jrushlow deleted the feature/final-classes branch August 29, 2024 16:20
@jrushlow jrushlow mentioned this pull request Aug 29, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use final classes
2 participants