[pivot] Validation discovers undeclared extensin properties

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 571074 | | Status | NEW | | Importance | P3 normal | | Reported | Feb 09, 2021 19:30 EDT | | Modified | Feb 23, 2021 05:57 EDT | | See also | 571079, 571083, 571389, 571407, 571005 | | Reporter | Deniz Eren |

Description

Created attachment 285498\ Bug demo profile file

Find attached problem model ValidationProblem.profile.uml tested using ocl-branch-tests #251 (6.14.0.v20210207-1253).

Changed the GenModel GenAnnotation:\

\ to DISCARD.

Created FarmElement Stereotype that extends UML::Element and acts as a base type for all Stereotypes used in this test example.

Any Stereotype that requires to extend other metamodel types such as UML::NamedElement and UML::Dependency add FarmElement::base_Element as a redefinedProperty of their respective base_NamedElement or base_Dependency properties.

When you generate the Java code you will notice the following Java errors, which are easily fixed by add respective type casts such as "(NamedElement)" or "(Dependency)":

Java Error:\ Type mismatch: cannot convert from Element to Dependency

Complains about line:\ final /@NonInvalid/ Dependency base_Dependency = _1.getBase_Element();

Java Error:\ Type mismatch: cannot convert from Element to Dependency

Complains about line:\ final /@NonInvalid/ Dependency base_Dependency_0 = _1_0.getBase_Element();

Java Error:\ Type mismatch: cannot convert from Element to NamedElement

Complains about line:\ final /@NonInvalid/ NamedElement base_NamedElement = this.getBase_Element();

Java Error:\ Type mismatch: cannot convert from Element to NamedElement

Complains about line:\ final /@NonInvalid/ NamedElement base_NamedElement = this.getBase_Element();

:notepad_spiral: ValidationProblem.profile.uml

[eclipse-ocl-bot]

By Deniz Eren on Feb 09, 2021 20:01

Please also see extension issue, regarding missing "import" in Java code: Bug 571075

[eclipse-ocl-bot]

By Ed Willink on Feb 10, 2021 03:50

Once incorporated into a test harness, the attched gives an UnsupportedOperationException during code generation.

Debugging, this appears to be a downstream diagnostic from an earlier implicit-collect of an extension property.

Has .allInstances()\ ->select(base_Dependency.client\ ->includes(self.base_NamedElement)) .base_Dependency .supplier .extension_Animal->asSet())

whose AST serializes to

ValidationProblem::Has.allInstances()\ ->select(1 : ValidationProblem::Has[1] | \ 1.base_Dependency.client\ ->includes(self.baseNamedElement))\ ->collect(1 : ValidationProblem::Has[1] |\ 1_.baseDependency)\ ->collect(1 : UML::Dependency[?]\ | 1.supplier)\ ->collect(1 : UML::NamedElement[1] |\ 1_.oclBadProperty)->asSet()

showing that the extension_Animal has failed even though OCL->Validate in the UML Model Editor has no complaints.

The UOE can probably be improved as a more readable diagnosis of the oclBadProperty parse fail place holder.

[eclipse-ocl-bot]

By Deniz Eren on Feb 10, 2021 04:51

Ed if you have an early fix I can test it with a more complex example on my side - let me know.

[eclipse-ocl-bot]

By Ed Willink on Feb 10, 2021 05:08

(In reply to Ed Willink from comment #2)

showing that the extension_Animal has failed

The extra properties of NamedElement are:

UML::NamedElement::extension_Chicken,\ UML::NamedElement::extension_Farm,\ UML::NamedElement::extension_Duck,\ UML::NamedElement::Farm

but not extension_Animal since there is no E_Animal_NamedElement rather an inherited E_FarmElement_Element for which Farm::base_NamedElement redefines FarmElement::base_Element without an equivalent Extension redefinition.

The OCL parsing failure therefore seems correct, but badly diagnosed.

even though OCL->Validate in the UML Model Editor has no complaints.

The downstream parsing failure (during GenModel Validate) really should be diagnosed by Validate.

The missing E_FarmElement_NamedElement is technically only poor style (you don't have to fully imolement the idiom) but since anything that deviates from the idiom is a user nightmare UML really should diagnose it. Bug 571079 raised for a possible UI solution.

[eclipse-ocl-bot]

By Ed Willink on Feb 10, 2021 05:34

(In reply to Ed Willink from comment #4)

The extra properties of NamedElement are:

UML::NamedElement::extension_Chicken, UML::NamedElement::extension_Farm, UML::NamedElement::extension_Duck, UML::NamedElement::Farm

Bug 571083 raised because the UML::NamedElement::Farm is omitted when OCL->Validate is debugged.

[eclipse-ocl-bot]

By Ed Willink on Feb 10, 2021 07:31

This is primarily a user problem triggering secondary tooling validation issues. This is easily seen by opening the *.profile.uml with a text editor.

Search for extension_Duck. One hit:

<ownedEnd xmi:type="uml:ExtensionEnd" xmi:id="_EGRFsGhjEeuxTMF8JHSJ-w" name="extension_Duck" type="_OjZdAGYgEeuLiryWOk9R1g" aggregation="composite" association="_EGQeoGhjEeuxTMF8JHSJ-w"/>

=> there is an ExtensionEnd names extension_Duck. Not used by OCL.

Search for extension_Animal. One hit:\

Has.allInstances()->select(base_Dependency.client->includes(self.base_NamedElement)).base_Dependency.supplier.extension_Animal->asSet()

=> There is an OCL usage of a property that has not been declared.

The problem is that the NamedElement base is redefined in Duck and Chicken, but this does not make it visible for their Animal general.

---

The user report is therefore INVALID, but the poor tooling diagnoses need investigating here and in Bug 571083 and Bug 571079.

[eclipse-ocl-bot]

By Deniz Eren on Feb 11, 2021 00:58

Created attachment 285520 Bug demo profile file2

ValidationProblem.profile.uml

[eclipse-ocl-bot]

By Deniz Eren on Feb 11, 2021 01:00

Ed, see attached model with fixed OCL Expression:

Has.allInstances()->select(base_Dependency.client->includes(self.base_NamedElement)).base_Dependency.supplier.oclAsType(UML::Element).extension_FarmElement->asSet()

Now I reference extension_FarmElement, in Element, which is upcast from supplier, which is NamedElement, but I still get the same Java errors:

Type mismatch: cannot convert from Element to Dependency\ Type mismatch: cannot convert from Element to NamedElement

Unless I'm missing something, the issue I reported about casting requirement, could be unrelated to the OCL expression error? I do however acknowledge your findings and NamedElement definitely shouldn't have an extension_Animal... but after fixing the OCL expression still getting the casting issue...

[eclipse-ocl-bot]

By Deniz Eren on Feb 11, 2021 20:32

Created attachment 285529 Bug demo profile file

I simplified the model, removed unnecessary items to focus on the issue I'm replicating at much as possible.

Firstly type Farm inherits FarmElement, which extends UML::Element, thus has base_Element property. Farm itself then extends UML::NamedElement, thus has base_NamedElement property, which then has Redefined property base_Element from FarmElement.

From within the properties of Farm, when referencing to self.base_NamedElement, we get the Java error in the compiled code, requiring manual casting of an expression to NamedElement type:

+/animal_fails: FarmElement[*] -> Has.allInstances()->select( base_Dependency.client->includes( self.base_NamedElement ) ).base_Dependency.supplier.extension_FarmElement->asSet()

Java Error: Type mismatch: cannot convert from Element to NamedElement

Code: final /@NonInvalid/ NamedElement base_NamedElement = this.getBase_Element();

Instead when we refer to self.base_Element in the same expression there are no Java Errors requiring casting to NamedElement etc:

+/animal_works: FarmElement[*] -> Has.allInstances()->select( base_Dependency.client->includes( self.base_Element ) ).base_Dependency.supplier.extension_FarmElement->asSet()

I then tried to narrow down this issue with simpler OCL expressions by defining derived properties called animal1, animal2 and animal3, but they all worked fine, so it's a mystery to me why animal_fails property fails...

+/animal1: FarmElement[*] -> Has.allInstances().base_Dependency.supplier.extension_FarmElement->asSet()

+/animal2: FarmElement[*] -> UML::NamedElement.allInstances().extension_FarmElement->asSet()

+/animal3: FarmElement[*] -> UML::Element.allInstances().extension_FarmElement->asSet()

:notepad_spiral: ValidationProblem.profile.uml

[eclipse-ocl-bot]

By Ed Willink on Feb 12, 2021 04:05

Your earlier problem gave me an adequate repro. I don't yet need another.

Since the bug is ultimately a 'generous' failure to diagnose a bad user model rather than bad OCL code, other bugs have higher priority.

[eclipse-ocl-bot]

By Deniz Eren on Feb 12, 2021 04:11

Ok sounds good, please let me know if you need anything else.

[eclipse-ocl-bot]

By Deniz Eren on Feb 16, 2021 00:09

Ed, there was some instability with this feature, but the latest build you have made "ocl-branch-tests #252" is completely stable so far and works.

[eclipse-ocl-bot]

By Ed Willink on Feb 16, 2021 02:46

(In reply to Deniz Eren from comment #12)

Ed, there was some instability with this feature, but the latest build you have made "ocl-branch-tests #252" is completely stable so far.

Thanks, encouraging. That has the new caches, but not yet the ThreadLocal executor to avoid churning caches. Even on small examples like yours the number of churned caches/model analyses drops from 20 to 1. Eliminating the eccentric GlonalEnvironmentFactory is also good, but there is now a memory leak to track down.

and works

I'm particularly interested in any feedback on Papyrus UX regressions, since that is not part of my JUnit testing. I hope that you will just find that with larger models it is noticeably faster.

[eclipse-ocl-bot]

By Deniz Eren on Feb 16, 2021 03:52

Yes definitely noticeably faster and more reliable (less unexpected exceptions) when using Papyrus UI

[eclipse-ocl-bot]

By Ed Willink on Feb 21, 2021 03:26

The philosophy change from staching the (re-)discovered OCL context as a ResourceSet adapter to a ThreadLocal is quite dramatic behind the scenes and offers some nice simplifications such as no longer needing a special Global OCL for installed not-compiled OCL. But it all has to be backward compatible and fixing the leaks is challenging. Hope to have a branch test that works today. If you are able to exercise it on Papyrus that would be great so that it can go into M3 tomorrow.

I think Papyrus starts again each time you do an OCL validate so it should be easy, but there is a risk of a leak / re-use of stale models on subsequent validates.

[eclipse-ocl-bot]

By Deniz Eren on Feb 21, 2021 06:45

Hi Ed,\ Yes for sure, as soon as you have a test build ready, let me know and I will test it.

[eclipse-ocl-bot]

By Deniz Eren on Feb 21, 2021 06:48

Is this version containing all the changes you mention?\ https://ci.eclipse.org/ocl/job/ocl-branch-tests/255/

[eclipse-ocl-bot]

By Ed Willink on Feb 21, 2021 07:14

Yes. For now the LeakTests are commented out.

[eclipse-ocl-bot]

By Deniz Eren on Feb 21, 2021 07:58

Hi Ed,

It's all working quite fine in Papyrus. I regenerated the java code and then ran the profiles in a model that uses it. As previously reported I need to manually cast types as a result of complex redefined property chains, but not such a problem. After upgrading to this recent version I noticed the generated Java code only change in the "*Tables.java" files for example:

public static final /*@NonInvalid*/ NsURIPackageId PACKid_http_c_s_s_www_mtlcpp_com_s_projectml_s_1_0_s_ProjectML_s_CMake = IdManager.getNsURIPackageId("http://www.mtlcpp.com/projectml/1.0/ProjectML/CMake", "CMake", CMakePackage.eINSTANCE);\
public static final /*@NonInvalid*/ NsURIPackageId PACKid_http_c_s_s_www_mtlcpp_com_s_projectml_s_1_0_s_ProjectML_s_Cxx = IdManager.getNsURIPackageId("http://www.mtlcpp.com/projectml/1.0/ProjectML/Cxx", "Cxx", CxxPackage.eINSTANCE);\
public static final /*@NonInvalid*/ NsURIPackageId PACKid_http_c_s_s_www_mtlcpp_com_s_projectml_s_1_0_s_ProjectML_s_General = IdManager.getNsURIPackageId("http://www.mtlcpp.com/projectml/1.0/ProjectML/General", "General", GeneralPackage.eINSTANCE);

Changed to be:

public static final /*@NonInvalid*/ NsURIPackageId PACKid_http_c_s_s_www_mtlcpp_com_s_projectml_s_1_0_s_ProjectML_s_CMake = IdManager.getNsURIPackageId("http://www.mtlcpp.com/projectml/1.0/ProjectML/CMake", null, CMakePackage.eINSTANCE);\
public static final /*@NonInvalid*/ NsURIPackageId PACKid_http_c_s_s_www_mtlcpp_com_s_projectml_s_1_0_s_ProjectML_s_Cxx = IdManager.getNsURIPackageId("http://www.mtlcpp.com/projectml/1.0/ProjectML/Cxx", null, CxxPackage.eINSTANCE);\
public static final /*@NonInvalid*/ NsURIPackageId PACKid_http_c_s_s_www_mtlcpp_com_s_projectml_s_1_0_s_ProjectML_s_General = IdManager.getNsURIPackageId("http://www.mtlcpp.com/projectml/1.0/ProjectML/General", null, GeneralPackage.eINSTANCE);

The strings "CMake", "Cxx" and "General" become null. However like I said this new version also works quite good in Papyrus and I have no idea what impact this string to null would cause; definitely not presenting as a problem.

I think it's good to include in M3 anyway. Thank you for all your work.

[eclipse-ocl-bot]

By Ed Willink on Feb 21, 2021 10:17

(In reply to Deniz Eren from comment #19)

The strings "CMake", "Cxx" and "General" become null.

This is the nsPrefix argument that is very little used but there for completeness.

Bug 571389 raised.

[eclipse-ocl-bot]

By Ed Willink on Feb 21, 2021 16:45

(In reply to Deniz Eren from comment #19)

As previously reported I need to manually cast types as a result of complex redefined property chains

I'm not aware of your repro for anything that produces bad OCL-derived code for 'normal' UML/OCL i.e. I am only aware of an outstanding UnlimitedNatural issue. One redefinition problem was an invalid model. The UML2 multi-inheritance is fixable by careful inheritance ordering.

[eclipse-ocl-bot]

By Deniz Eren on Feb 21, 2021 21:51

Hi Ed,

Two issues remaining are: Bug 571198

And see my comment above with demo repo attached, which demonstrates the Java generated code failing to compile because static type down casting is needed. This is easy to fix manually however if the base class property type for base_* is of lower derived types, such as base class has base_Atrifact but derived class has base_NamedElement then a simple static cast doesn’t do the trick. I’m not sure how to fix such cases besides trying to avoid them in artificially in design:\ https://bugs.eclipse.org/bugs/show_bug.cgi?id=571074#c9

Anyway hoping if the base issue in that Comment 9 is fixed then other complex cases also get addressed by the fix?

[eclipse-ocl-bot]

By Ed Willink on Feb 22, 2021 10:35

(In reply to Deniz Eren from comment #9)

Created attachment 285529 [details] Bug demo profile file

I simplified the model, removed unnecessary items to focus on the issue I'm replicating at much as possible.

Firstly type Farm inherits FarmElement, which extends UML::Element, thus has base_Element property. Farm itself then extends UML::NamedElement, thus has base_NamedElement property, which then has Redefined property base_Element from FarmElement.

This bug is about synthesis of spurious extensions; an analysis generosity not bad code generation.

Bug 571407 raised for this issue.

[eclipse-ocl-bot]

By Deniz Eren on Feb 23, 2021 00:33

(In reply to Deniz Eren from comment #22)

The UML2 multi-inheritance is fixable by careful inheritance ordering.

Is this also true when there are complex inheritance such as those that exist in UML, where multiple inceptions of diamond inheritance exist? I anyway haven't been able to untangle and make the Java generation work without manual editing. Then further, when it comes to editing base_* type differences in some cases I haven't been able to resolve the issues by manual editing.

I would say at the moment it seems only simple inheritance models are supported by the Java generator. I will continue playing, and see if I can formulate a working process for working with complex inheritance architectures.

I can see the difficulty faced regarding the use of "extends" and "implements" as a result of the underlying Java limitation of single inheritance. At the moment the situation is that the modelling needs to account for the limitations of the Java generator as a result.

[eclipse-ocl-bot]

By Ed Willink on Feb 23, 2021 05:14

(In reply to Deniz Eren from comment #24)

(In reply to Deniz Eren from comment #22)

The UML2 multi-inheritance is fixable by careful inheritance ordering.

Is this also true when there are complex inheritance

The earlier workaround should be useable, but the recent OCL2Java CG fixes that recognize that same-named redefinitions should use the redefined feature while new-named redefinitions should use the redefining festure may just fix this. Maybe yet a further discrimination can enable the OCL2Java CG to emulate/re-use the UML2Ecore algorithms.

I can see the difficulty faced regarding the use of "extends" and "implements" as a result of the underlying Java limitation of single inheritance. At the moment the situation is that the modelling needs to account for the limitations of the Java generator as a result.

Not true for Ecore where Ecore2Java supports arbitrary interface level inheritance and synthesizes duplicate implementations for the limitations of single inheritance. The inconvenience is that if you add manual Java implementation code you may need to repeat the manual addition in multiple *Impls.

If it's ok for Ecore it may be ok for UML2. UML2Ecore seems to do something plausible for multiple inheritance; it's just that the irregular naming has outwitted OCL2Java.

[eclipse-ocl-bot]

By Deniz Eren on Feb 23, 2021 05:35

(In reply to Ed Willink from comment #25)

The earlier workaround should be useable, but the recent OCL2Java CG fixes that recognize that same-named redefinitions should use the redefined feature while new-named redefinitions should use the redefining festure may just fix this. Maybe yet a further discrimination can enable the OCL2Java CG to emulate/re-use the UML2Ecore algorithms.

Both with same-named redefinitions and different named ones I need to make manual changes, including the manual casting I mentioned. I make the least changes using different named redefinitions, but OCL expressions that include selectByKind and oclIsKindOf fail for some reason - I keep getting empty sets when I try to filter sets using those calls.

For example I have lots of places where the following changes need to be manually made:

public NamedElement getBase_NamedElement() {\ if (base_NamedElement != null && base_NamedElement.eIsProxy()) {\ InternalEObject oldBase_NamedElement = (InternalEObject)base_NamedElement;\ base_NamedElement = (NamedElement)eResolveProxy(oldBase_NamedElement);\ if (base_NamedElement != oldBase_NamedElement) {\ if (eNotificationRequired())\ eNotify(new ENotificationImpl(this, Notification.RESOLVE, CMakePackage.CMAKE_BOOLEAN_CHECK__BASE_NAMED_ELEMENT, oldBase_NamedElement, base_NamedElement));\ }\ }\ return base_NamedElement;\ }

I need to change all the ocurences of base_NamedElement to base_BaseTypeName_NamedElement manually.

[eclipse-ocl-bot]

By Ed Willink on Feb 23, 2021 05:57

NB. This OCL bug is about spurious extra extension properties.

Bug 571005 is the UML2 bug about incorrect Java code for anarchic multiple inheritance. Pending a fix of Bug 571005, follow the workarounds suggested there, or discuss workarounds there. (It appears that Bug 571005 just needs UML2 to apply the same fix as Bug 571407 does for OCL.)