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

Provide a JvmGenericTypeValidator #2349

Closed
LorenzoBettini opened this issue Nov 5, 2019 · 9 comments · Fixed by #3031
Closed

Provide a JvmGenericTypeValidator #2349

LorenzoBettini opened this issue Nov 5, 2019 · 9 comments · Fixed by #3031
Assignees
Milestone

Comments

@LorenzoBettini
Copy link
Contributor

I'd like to propose an idea I've been thinking of for a while.

When implementing an Xbase language you have to repeat several typical checks, like no duplicate fields, no duplicate methods (according to Java overriding and overloading rules), cyclic hierarchy, etc., like we recently did in eclipse/xtext-eclipse#1205 and eclipse/xtext-eclipse#1205 . I seem to understand that the best way of doing that is by performing checks directly on the inferred JvmModel, like we did in the issues above and like Xtend itself does. After all, it does not make sense to have an inferred JvmModel which then produces incorrect Java code...

So why not providing some kind of JvmGenericTypeValidator that does all these checks automatically on the inferred JvmModel and then generating issues the original elements in the program (by means of IJvmModelAssociations)? With that respect the starting point could be the XtendValidator itself, from where we can extract most of the logic of typical JvmModel validations in the reusable JvmGenericTypeValidator (and then XtendValidator could be refactored accordingly). The JvmGenericTypeValidator could follow the same strategy of JvmTypeReferencesValidator which is automatically enabled (by relying on getEPackages).

All Xbase languages would then benefit from such automatic validations.

@szarnekow do you think it is feasible, makes sense, could be useful?

@szarnekow
Copy link
Contributor

@LorenzoBettini Yes that does make a lot of sense. Having sensible and efficient impl for this and relieving clients from the burden to implement these manually sounds very good.
One caveat: Some Xbase languages may abuse the JVM model to infer class-like structures for other languages with other constraints. There should be an easy way to opt out from the validation.
Also the error messages must be easy to customise since languages often don't want to expose the JVM terminology but the DSL concepts to the end user.

@LorenzoBettini
Copy link
Contributor Author

@szarnekow thank you for the feedback!
I'll try to start working on some prototype soon (hopefully).
Do you have any suggestion to opt out? simple runtimemodule approach?
Moreover, is there an existing test Xbase language that I can use for working on this? I'd need at least the concepts of mapped classes, interface and methods. The Domainmodel could be a good candidate, but I'd rather use some Xbase language that's part of the test suite.

@szarnekow
Copy link
Contributor

Seems the only things we have are in org.eclipse.xtext.xbase.testlanguages.
And Xtend of course.

@szarnekow
Copy link
Contributor

Do you have any suggestion to opt out? simple runtimemodule approach?

The validators should be language specific (as in: not registered only for the EPackage but also enabled for the language at hand). See AbstractInjectableValidator.isLanguageSpecific()

@miklossy miklossy added this to the Release_2.21 milestone Nov 24, 2019
@cdietrich cdietrich removed this from the Release_2.21 milestone Feb 19, 2020
@cdietrich cdietrich transferred this issue from eclipse/xtext-extras Apr 17, 2023
@LorenzoBettini
Copy link
Contributor Author

@szarnekow I started to work on this; I was planning to factor many parts of XtendValidator into an Xbase JvmGenericTypeValidator. Then, I would use XtendValidatorTest to check whether errors are still generated as expected, of course, after adapting a few things.

That's the very first experimental commit on my fork: LorenzoBettini@c490251

I set JvmGenericTypeValidator as a composite validator in XtendValidator.

However, by debugging, my validator never kicks in for inferred elements like JvmGenericType. Am I doing anything wrong?
How would such a validator be supposed to be configured so that it can inspect inferred types in a derived state resource?

@LorenzoBettini
Copy link
Contributor Author

@szarnekow, I pushed an updated commit

LorenzoBettini@7c9717b

tests are green.

It looks like a small hack because I need to intercept an EObject, which is the root of the Resource's contents, because, as you said, only the first element of the Resource is currently validated.

The abstract declarative validator does not provide anything else to override, at least, in my understanding.
On the other hand, composed validators must be abstract declarative validators.

@LorenzoBettini
Copy link
Contributor Author

@szarnekow Going back to what we discussed, JvmGenericType, getExtendClass, and getExtendedInterfaces: this would not work for what we need. Looking at the implementation

if (candidate.getType() instanceof JvmGenericType && !((JvmGenericType) candidate.getType()).isInterface())

The method filters the supertypes based on whether the referenced type is an interface or not.
It returns the first one matching.
This is useful if you know that the JvmGenericType is already valid.

However, in our case, during validation, we need to make sure that what is specified as a superclass is indeed a class and not an interface.

For example, in the following invalid program

class Foo extends Serializable implements Object

getExtendedClass would return Object. Similarly, getImplementedInterfaces would return Serializable.

Thus, besides my current solution based on the multiplicity of the containing feature:
https://github.com/LorenzoBettini/xtext/blob/61771e21ad32b6b042797f278e5fc91428c1068d/org.eclipse.xtext.xbase/src/org/eclipse/xtext/xbase/validation/JvmGenericTypeValidator.java#L101
I wouldn't know how to check that.

@LorenzoBettini
Copy link
Contributor Author

@szarnekow for some Xtend-specific validations, I was thinking of letting the JvmGenericTypeValidator be inherited and some methods extended. I checked the implementation of scanning of Check methods in Composed validators, and it considers the class hierarchy so there should be no problem. However, I seem to remember that you did not like clients to extend validators to be composed. If I recall correctly, why is it so?

@szarnekow
Copy link
Contributor

Oh I think I meant the opposite (might have been confusing rambling from my end). If the composed injectors are instantiated from the correct injector, inheriting from them and overriding some rules is absolutely ok.

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 a pull request may close this issue.

4 participants