Closed ujhelyiz closed 11 years ago
These patterns did work before, and are tested at each build, so they are correct on a runtime level. The question is, how can we get them to work on the tooling level. Once again, I'll look at the query-based feature validator, but I think it did work on the snapshot queries as well last time I looked.
I have changed the validator to only signal a warning when the target has no type. However, while this may be correct in general, I think that we have to look at the type provider and find out why it doesn't find the type in the above examples.
As stated above, the correct types would be:
Ok, sorry for the delay I just forgot to work with this ticket in the last 2 days.
The second part comes with a great corner-case again!
Here is sample pattern for discussion:
pattern TestValue(Value) = { EIntegerObject(Value); EJavaObject(Value); }
Question is that what should happen now? At the moment this is an error (Ambiguous variable type defintions: [EIntegerObject, EJavaObject]), in technical reality it is as these are EDataTypes which have no connection with each other. However one should think that the Value will be an Integer, as the EJavaObject's instance class is plain Object.
I would like to offer a more detailed view of the corner case, and explain why I think that it's two versions should be handled differently.
First case: Redundant type constraints error (see below)
pattern TestValue(Value) = {
EIntegerObject(Value);
EJavaObject(Value);
}
I agree that this is an error, first because EJavaObject will match every Object in the model (that is reachable through the visitor), which is bad for performance, second because the two type constraints are redundant IF we presume that we are dealing with EMF models in Java (which we do).
Second case: Unclear type compatibility (warning)
pattern RecordedSchoolName(Name) = {
MatchRecordSubstitution.value(_MR, Name);
School.name(_S, Name);
}
While from a certain perspective this leads back to the first case, I argue that it is different. Here, we have two path expressions (transformed to feature constraints afaik) and an "equality". While it is unclear from the EMF type system, the two EDataTypes (EString and EJavaObject) are in fact compatible. However, I agree that it deserves a warning.
Final note: in this very specific example, there exists a workaround: using MatchRecordSubstitution.stringValue, which of course will only have a value if the substitution value is a String.
Correction:
EJavaObject will match every Object in the model
This is not how it works right now for EDataTypes. This construct will match the values of all EJavaObject-typed attributes. Values of an EInteger-typed attribute will not be included in the match set, nor will values of any EDatatype other than EJavaObject, even if it is associated with the same java.lang.Object instance type.
It is theoretically possible to have the same object appear as an EJavaObject and an EInteger, but only if it appears as the values of two different attributes, one of each data type. In the MatchRecord example, this might easily be the case.
Check the two instance classes:
Is someone working on this issue?
The original issue has been fixed, but we should probable move it as an issue with another title and description. Or we can just close it and get back to it if the last corner case actually comes up.
OK so @ujhelyiz could you please migrate this one to Eclipse?
Moved to Eclipse.org BugZilla https://bugs.eclipse.org/bugs/show_bug.cgi?id=398911
We have reached now the next chapter of the epic type checking/inference saga. As of now, the testing.queries project does not build because of the new query based feature validators (see #133) and the type inference engine.
The last parameters of the RecordRoleValue and SubstitutionValue patterns are checked by the new validators, they are null in the return value in line 169 of the QueryBasedFeaturePatternValidator.java file.
@okrosa, @abelhegedus, please check where lies this issue and also resolve it, as the testing.queries project is important to build.