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

Xbase proposal: Add features to express the expected semantics of supertypes #2880

Closed
LorenzoBettini opened this issue Dec 20, 2023 · 7 comments · Fixed by #2924
Closed

Xbase proposal: Add features to express the expected semantics of supertypes #2880

LorenzoBettini opened this issue Dec 20, 2023 · 7 comments · Fixed by #2924
Milestone

Comments

@LorenzoBettini
Copy link
Contributor

To properly and cleanly implement #2349, it would be nice to have in the types model proper types for a Jvm class and Jvm interface to distinguish between the intended extended class and implemented interfaces (for classes) and extended interfaces (for interfaces).
Actually, the JvmGenericInterface would not be strictly necessary since we can assume that all the set supertypes are meant to be interfaces.
On the contrary, JvmGenericClass would be necessary (see #2349 (comment)).

My proposed extension is meant to be backward compatible. Speaking about JvmGenericClass, I planned to add two fields, extendedClass (singular) and implementedInterfaces (multiple), and let the inherited superTypes be consistent with them. Similarly, for the methods getExtendedClass and getImplementedInterfaces.

I could provide a PR against the main branch (and then use it in #2349).

@szarnekow, what do you think?

@LorenzoBettini
Copy link
Contributor Author

@cdietrich if I wanted to try to generate the EMF classes of common.types, which run configuration should I use? I see this one

but it is not in a source folder.

@cdietrich
Copy link
Member

no idea. side effect of mig?

@LorenzoBettini
Copy link
Contributor Author

@cdietrich the same holds for the Xbase generator. I created this #2881

@szarnekow
Copy link
Contributor

Actually, the JvmGenericInterface would not be strictly necessary since we can assume that all the set supertypes are meant to be interfaces.

I don't think that's actually true, since interfaces like Serializable, e.g., interface without a super interface, do have Object as their superclass.

@LorenzoBettini
Copy link
Contributor Author

Right! So I guess it's even more important to separate the intentions and validate them.

@LorenzoBettini
Copy link
Contributor Author

@szarnekow I started to work on that in my fork, and I might need some early feedback, please.

Note that these branches are currently based on #2882 to have proper generation of common types.

The currently working solution is in the branch https://github.com/LorenzoBettini/xtext/tree/lb_2880-2

In this branch, I haven't introduced JvmGenericClass and JvmGenericInterface. Instead, I enriched JvmGenericType.
The new features are classToExtend and the (non-containment) lists interfacesToImplement and interfacesToExtend.
I chose new names not to mix them with the get methods like getExtendedClass. In fact, these features are meant for expressing the intentions (class to extend and interfaces to implement for classes and interfaces to extend for interfaces) to better perform validation (#2349).

The main commit is LorenzoBettini@0cb0120

the getter and setter for classToExtend and the getters for the lists are customized to keep the new features consistent with what getSuperTypes returns. This way, this extension should be backward compatible. It is not a complete synchronization, see the Javadoc LorenzoBettini@0cb0120#diff-0243bd309c39347e67704b49bdd4b374c1a1a475fefb6beb2ceb1268c5f8a273R20
but it should be enough for the proposed extension.
In particular, adding something directly to getSuperTypes does not change the new features because there would be no way to know which one to update.
On the contrary, adding a type with the new features will update the list returned by getSuperTypes.
Removing that type from getSuperTypes would remove it from the original place as well.
I have added several tests in that respect in JvmGenericTypeTest.

I then applied these new features in the XtendJvmModelInferrer LorenzoBettini@08c9989 and the build is still green.

I added a few more tests to JvmModelGeneratorTest as well LorenzoBettini@f620b2a

Finally, I applied that to Domainmodel example and added a test (we were never testing with a superclass) LorenzoBettini@bad8527

Unfortunately, I could not add a check for proper usages of the new features: classToExtend and interfacesToImplement should not be used when isInterface is true; accordingly for interfacesToExtend.
Actually, I had added such checks but had to revert them LorenzoBettini@bad8527 because such a check would break code that scans all the features, e.g.,
org.eclipse.xtext.findReferences.ReferenceFinder.findLocalReferencesFromElement(Predicate,
EObject, Resource, Acceptor).

On a separate branch https://github.com/LorenzoBettini/xtext/commits/lb_2880-1/ I tried adding JvmGenericClass and JvmGenericInterface with the same functionality. However, when I applied that to XtendJvmModelInferrer I have several test failures. From what I understand, this is due to many parts in Xtend and Xbase performing checks of the shape

type.eClass() == TypesPackage.Literals.JVM_GENERIC_TYPE

which would return false for the new types.
(e.g., this fixes a few tests LorenzoBettini@e83d91d).
A proper check for all the places where this occurs and replacement with something like

TypesPackage.Literals.JVM_GENERIC_TYPE.isSuperTypeOf(type.eClass())

would be required; however, I wanted to first know whether you think it's worthwhile or whether the first proposal is enough.

@LorenzoBettini LorenzoBettini changed the title Xbase proposal: add JvmGenericClass and JvmGenericInterface to types model Xbase proposal: Add features to JvmGenericType to express the expected semantics of supertypes Jan 9, 2024
@LorenzoBettini LorenzoBettini changed the title Xbase proposal: Add features to JvmGenericType to express the expected semantics of supertypes Xbase proposal: Add features to express the expected semantics of supertypes Feb 4, 2024
@LorenzoBettini LorenzoBettini added this to the Release_2.34 milestone Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment