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

review: feature: Spoon meta-model #1627

Merged
merged 4 commits into from
Oct 29, 2017
Merged

Conversation

pvojtechovsky
Copy link
Collaborator

I see that whenever one wants to write some generator of spoon helper (CloneVisitor, EqualsVisitor, ReplacementVisitor, RoleHandler #1582, CtRoleScanner #1614 , ...) s/he needs to create spoon model of spoon sources and to analyze (build some extra structure) it somehow.
I have feeling like it might be helpful to have a standard code, which can build a simple metamodel structure (first draft is in this PR) of the Spoon model.

I am really not sure how it should look like. May be we just need some wrappers around AST of Spoon sources ... do you have alternative ideas?

WDYT? Are we able to create a simple meta model which might simplify development of new spoon code generators and may be simplify writing some tests?

Note: the meta model of this PR is probably sufficient to generate: CtScanner, RoleHandler #1582, CtRoleScanner #1614, CtInheritanceScanner, ... ?

@tdurieux did you already used some metamodel to assign CtRole to spoon sources in past?

@tdurieux
Copy link
Collaborator

@tdurieux did you already used some metamodel to assign CtRole to spoon sources in past?

Not really, I used name convention and I used the roles in the Actions

@pvojtechovsky pvojtechovsky force-pushed the metamodel branch 2 times, most recently from bad27a8 to dc5a179 Compare October 22, 2017 11:41
@pvojtechovsky pvojtechovsky changed the title WiP: feature: Spoon meta-model review2: feature: Spoon meta-model Oct 22, 2017
@pvojtechovsky pvojtechovsky changed the title review2: feature: Spoon meta-model WiP: feature: Spoon meta-model Oct 22, 2017
@pvojtechovsky
Copy link
Collaborator Author

@monperrus where to put these meta model classes? I guess it belongs to tests, because it is actually used only in testing and code generation (not in spoon runtime by clients), but on the other side code generator tasks are not pure tests, so if I put the code into tests then CI complains and generator which is src/main is missing dependency to SpoonMetaModel which is in src/test ...

@tdurieux
Copy link
Collaborator

I think we have a metamodel package

@pvojtechovsky
Copy link
Collaborator Author

I think we have a metamodel package

I cannot see it.

@pvojtechovsky pvojtechovsky force-pushed the metamodel branch 2 times, most recently from 8dd6f8b to 1e25412 Compare October 24, 2017 19:09
@pvojtechovsky pvojtechovsky changed the title WiP: feature: Spoon meta-model review: feature: Spoon meta-model Oct 24, 2017
@monperrus
Copy link
Collaborator

We don't have a metamodel package but we have src/main/java/spoon/Metamodel.java.

What about merging it with SpoonMetaModel?

@pvojtechovsky
Copy link
Collaborator Author

The SpoonMetaModel is different to Metamodel class, because SpoonMetaModel needs access to java sources of Spoon library (only in test phase), while Metamodel will work in runtime too.

WDYT about access to spoon sources in runtime?

@monperrus
Copy link
Collaborator

monperrus commented Oct 24, 2017 via email

@pvojtechovsky
Copy link
Collaborator Author

How to get list Spoon model class using reflection if sources are not available? It looks like there is no easy solution. Or will we list all possible classes in an constant list?

@monperrus
Copy link
Collaborator

monperrus commented Oct 25, 2017 via email

@pvojtechovsky
Copy link
Collaborator Author

What about merging it with SpoonMetaModel?

I like this idea, but

  • spoon.Metamodel is in spoon package, which should not contain classes like MMType, MMField, ...
  • I would prefer to have MMType, MMField, in spoon.metamodel package, otherwise they will be visible for each client who looks at root spoon package - I think it would be not good
  • I cannot move model loading code into spoon.Metamodel and keep MMType, ... in spoon.metamodel, because I would like to use package protected methods.

Will we move spoon.Metamodel into spoon.metamodel.Metamodel ?

WDYT?

@monperrus
Copy link
Collaborator

Have just had a more detailed look.

It seems clear that this code will stay in src/testbecause it goes much beyond code transformation and analysis.

What you do here is creating a meta-meta-model :-) (a metamodel of the Spoon meta-model). That's fun and interesting.

Note that there exists other metametamodels, starting with EMF/Ecore

Note also that we can reuse Java/Spoon itself as a metametamodel (ie Spoon would be metacircular), in this case, we would have:

  • MMType extends CtType
  • MMField extends CtField
  • etc...

I like this solution, because we would use the Spoon API itself to model the Spoon metamodel, and this would help us to see how well it works.

WDYT?

@pvojtechovsky
Copy link
Collaborator Author

It seems clear that this code will stay in src/test because it goes much beyond code transformation and analysis.

I agree, it is not for client's. It is for spoon internal purposes: testing and generating of some spoon sources.

EMF/Ecore

This model is too abstract. I would prefer to have domain (Spoon) specific meta meta model (DSMMM), which express the internal structures of spoon model. Why?

  • in DSMMM it is more visible the purpose/semantic of elements then in abstract MMM
  • it is easier to use DSMMM and understand
  • it is visible when new concept is added into Spoon model, because we have to change DSMMM to reflect that.

We will see if current Spoon specific meta meta model (SSMMM) is good or not. I am open to discuss it deeply, because I have only little experience with meta modeling, so may be it is on wrong way. But actually I like it and current SSMMM is helpful.

we can reuse Java/Spoon itself as a metametamodel

I understand the idea. At least on high level ;-)
I would prefer to not extend SSMMM from CtType itself, because

  • it would become more abstract, then needed again - it doesn't match with main idea of DSMMM (see above).
  • I actually see no method of CtType, which would be helpful in MMType (etc.), so I do not see any reason why to extend from CtType or CtField.
  • The meaning of MMType and MMField is not well matching the meaning of CtType and CtField.
  • The SSMMM should be readonly (after it is created) - again no match with CtType, ...
  • I believe that we use SSMMM to generate CtType and CtField in future, so the meta circular dependency might be problem (may be not a problem, it is not relevant now)

But note one think, which is near to idea of using of spoon itself: SSMMM references CtType and CtMethod of Spoon model, which was build from spoon sources. So: "SSMMM is a facade above Spoon meta meta model". See MMType.modelClass, MMType.modelInteface, MMField.valueType, MMethod.ownMethods. SSMMM is NOT independent on Spoon meta meat model.

WDYT?

@pvojtechovsky
Copy link
Collaborator Author

By the way, I have committed refactored version of metamodel. This implementation is more straight forward and correctly handles generic types used in spoon.

@monperrus
Copy link
Collaborator

monperrus commented Oct 28, 2017 via email

@INRIA INRIA deleted a comment from spoon-bot Oct 28, 2017
@INRIA INRIA deleted a comment from spoon-bot Oct 28, 2017
@pvojtechovsky
Copy link
Collaborator Author

If the tests pass, I am done here. Please review

@INRIA INRIA deleted a comment from spoon-bot Oct 28, 2017
* The fact that you are presently reading this means that you have had
* knowledge of the CeCILL-C license and that you accept its terms.
*/
package spoon.metamodel;
Copy link
Collaborator

Choose a reason for hiding this comment

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

move to spoon.test.metamodel to make it clear that's for test only

@spoon-bot
Copy link
Collaborator

Revapi Analysis results

Old API: fr.inria.gforge.spoon:spoon-core:jar:6.0.0-20171028.224456-120

New API: fr.inria.gforge.spoon:spoon-core:jar:6.0.0-SNAPSHOT

Detected changes: 5.

Change 1

Name Element
Old method java.util.ArrayList org.eclipse.jdt.internal.compiler.batch.Main::handleBootclasspath(java.util.ArrayList, java.lang.String) @ spoon.support.compiler.jdt.JDTBatchCompiler
New method java.util.ArrayList<org.eclipse.jdt.internal.compiler.batch.FileSystem.Classpath> org.eclipse.jdt.internal.compiler.batch.Main::handleBootclasspath(java.util.ArrayList<java.lang.String>, java.lang.String) @ spoon.support.compiler.jdt.JDTBatchCompiler
Code java.method.returnTypeTypeParametersChanged
Description The return type changed from 'java.util.ArrayList' to 'java.util.ArrayList<org.eclipse.jdt.internal.compiler.batch.FileSystem.Classpath>'.
Breaking binary: non_breaking,

Change 2

Name Element
Old method java.util.ArrayList org.eclipse.jdt.internal.compiler.batch.Main::handleClasspath(java.util.ArrayList, java.lang.String) @ spoon.support.compiler.jdt.JDTBatchCompiler
New method java.util.ArrayList<org.eclipse.jdt.internal.compiler.batch.FileSystem.Classpath> org.eclipse.jdt.internal.compiler.batch.Main::handleClasspath(java.util.ArrayList<java.lang.String>, java.lang.String) @ spoon.support.compiler.jdt.JDTBatchCompiler
Code java.method.returnTypeTypeParametersChanged
Description The return type changed from 'java.util.ArrayList' to 'java.util.ArrayList<org.eclipse.jdt.internal.compiler.batch.FileSystem.Classpath>'.
Breaking binary: non_breaking,

Change 3

Name Element
Old method java.util.ArrayList org.eclipse.jdt.internal.compiler.batch.Main::handleEndorseddirs(java.util.ArrayList) @ spoon.support.compiler.jdt.JDTBatchCompiler
New method java.util.ArrayList<org.eclipse.jdt.internal.compiler.batch.FileSystem.Classpath> org.eclipse.jdt.internal.compiler.batch.Main::handleEndorseddirs(java.util.ArrayList<java.lang.String>) @ spoon.support.compiler.jdt.JDTBatchCompiler
Code java.method.returnTypeTypeParametersChanged
Description The return type changed from 'java.util.ArrayList' to 'java.util.ArrayList<org.eclipse.jdt.internal.compiler.batch.FileSystem.Classpath>'.
Breaking binary: non_breaking,

Change 4

Name Element
Old method java.util.ArrayList org.eclipse.jdt.internal.compiler.batch.Main::handleExtdirs(java.util.ArrayList) @ spoon.support.compiler.jdt.JDTBatchCompiler
New method java.util.ArrayList<org.eclipse.jdt.internal.compiler.batch.FileSystem.Classpath> org.eclipse.jdt.internal.compiler.batch.Main::handleExtdirs(java.util.ArrayList<java.lang.String>) @ spoon.support.compiler.jdt.JDTBatchCompiler
Code java.method.returnTypeTypeParametersChanged
Description The return type changed from 'java.util.ArrayList' to 'java.util.ArrayList<org.eclipse.jdt.internal.compiler.batch.FileSystem.Classpath>'.
Breaking binary: non_breaking,

Change 5

Name Element
Old method void org.eclipse.jdt.internal.compiler.batch.Main::setPaths(java.util.ArrayList, java.lang.String, java.util.ArrayList, java.util.ArrayList, java.util.ArrayList, java.util.ArrayList, java.lang.String) @ spoon.support.compiler.jdt.JDTBatchCompiler
New method void org.eclipse.jdt.internal.compiler.batch.Main::setPaths(java.util.ArrayList<java.lang.String>, java.lang.String, java.util.ArrayList<java.lang.String>, java.util.ArrayList<java.lang.String>, java.lang.String, java.lang.String, java.util.ArrayList<java.lang.String>, java.util.ArrayList<java.lang.String>, java.lang.String) @ spoon.support.compiler.jdt.JDTBatchCompiler
Code java.method.numberOfParametersChanged
Description The number of parameters of the method have changed.
Breaking binary: breaking,

@monperrus monperrus merged commit 2aeab1a into INRIA:master Oct 29, 2017
@monperrus
Copy link
Collaborator

thanks Pavel! I'm really looking forward to how we'll use that in the future.

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

Successfully merging this pull request may close these issues.

4 participants