[Generator] Refactor the Metamodel generator to not use Ring anymore and simplify it - #1122
Merged
Conversation
… it returns ring elements, not real classes...)
This makes some code a little more complex, but this code should vanish when we finish to cut the dependency to ring
…are not compatible so a test will fail)
…any impact until the end of the collection in order to manage removals later
…amodel generation
Next step: remove the previous generator
Properties do not need the builder
Member
Author
|
This PR should now be done. There are still work to do but I'll not have the time to do it now I think. Maybe I'll find some personal time to fix the last bug I detected that is that remote methods are not removed. But the PR can be reviewed and merged now |
This is fixed via an update of Fame but I'm adding a regression test in Famix
This will simplify the fact that multiple visitors (for multiple languages) will be able to visit the same entities without a clash
…els and to avoid double visit of some traits
ClotildeToullec
approved these changes
May 28, 2026
This was referenced May 28, 2026
Contributor
|
merci pour la review Clotilde |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ring removal
This is a big change. This PR removes the usage of Ring in the Metamodel builder.
Before the process was:
Execute the generator to create an intermediate model -> Generate ring entities -> Filter ring entities to remove what would have no impact -> create changes -> generate the real code -> build a Fame model to use as descriptions
Now the process is:
Execute the generator to create an intermediate model -> Generate changes -> Filter the changes -> generate the code -> build the Fame model to use as descriptions
This is still complex but it's a big step to simplify things.
Using Ring had a lot of cons such as:
This change fixes multiple bugs:
Fixes #983
Fixes #1135
Refactoring of the visitor generator
I did multiple changes in the visitor generator to make it more usable.
Simplifications
Simplify relations
I also shipped some simplifications with this PR. The main one is the merging of FmxMBRelation and FmxMBRelationSide.
FmxMBRelation was saving 2 relations sides. FmxMBBehavior was saving both the relation in #relations and the sides in #properties.
I renamed all the API related to properties and relations of FmxMBBehavior to ensure we know if we have properties, relations or both.
Now we only save the relations sides. I removed the FmxMBRelation. Instead a relation side knows its opposite directly.
In FmxMBRelation we had a concept of direction. A relation was from a side to another. But this is wrong, a relation is bidirectional. What defines the direction of a relation should not be the relation itself, but the behavioral holding the relation.
Fixes #1124
Simplify Relation accessors generation
Until now, we saved a relation generation strategy in FmxMBRelationSide. This strategy was set manually when creating a relation side.
The name of the strategy did not help to understand them and they had no comment.
I did multiple improvements:
Rework visitors and introduce a visitor to generate the MM code
I did improvements of the visitors and with those changes I updated the way to generate the code.
Instead of having all the code generation in the FmxMB model classes I moved it to a visitor. I have one for the normal changes and one for the remote subbuilders.
I also introduced a FmxMBMethodFactory to simplify the generation code. The generation code can be quite verbose. The introduction of this class group the methods to generate and simplify the generation code.
Other
Future works
Following this change, there is still some work to do: