Missing instance class for CollectionTypeImpl

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 456513 | | Status | RESOLVED FIXED | | Importance | P3 normal | | Reported | Jan 02, 2015 06:39 EDT | | Modified | Jan 10, 2015 19:11 EDT | | Reporter | Christopher Gerking |

Description

Since CollectionTypeImpl has no instance class, isInstance(...) fails even when invoked with an appropriate Java collection, e.g., LinkedHashSet. That's why QVTo fails to set a Collection-typed property to any appropriate Java collection.

Could CollectionTypeImpl set its instance class to java.util.Collection?

[eclipse-ocl-bot]

By Ed Willink on Jan 02, 2015 10:53

It is far from clear, from your repro-less words, what you want but in TypesPackageImpl you will find

initEClass(collectionTypeEClass, CollectionType.class,\ "CollectionType", !IS_ABSTRACT, !IS_INTERFACE, IS_GENERATED_INSTANCE_CLASS); //$NON-NLS-1$

showing the CollectionType is initialized in traditional way with CollectionType.class.

Since this is 100% EMF standard,. I doubt this is what you want to change.

I suspect you getting in a Collection/CollectionType meta-mess.

However be aware that classic OCL has increasingly long-established behaviors that we are very reluctant to change. After all consumers such as QVTo often have workarounds for dubious behavior and so when corrected the consumers break.

This will almost certainly be WONTFIX.

[eclipse-ocl-bot]

By Christopher Gerking on Jan 02, 2015 11:13

(In reply to Ed Willink from comment #1)

It is far from clear, from your repro-less words, what you want

All concrete collection types set their instance class inside the respective constructor. Example taken from org.eclipse.ocl.ecore.impl.OrderedSetTypeImpl:

protected OrderedSetTypeImpl() {\ super();\ setInstanceClass(LinkedHashSet.class);\ }

No instance class is set in the CollectionTypeImpl constructor, where setInstanceClass(java.util.Collection.class) would help to enable a QVTo property definition such as:

property p : Collection(String) = OrderedSet{"a"};

[eclipse-ocl-bot]

By Ed Willink on Jan 02, 2015 16:57

I recalled a previous long inconclusive discussion about instanceClass in oclstdlib. That was Bug 344368 where the much harder multiple Long/long/BigDecimal/.... Integer/UnlimitedNatural ambiguities added to the complexity.

Collection is at least much simpler only 1 candidate type.

(In reply to Christopher Gerking from comment #2)

All concrete collection types set their instance class inside the respective constructor.

Well that must be a bug. Why doesn't oclstdlib.ecore provide the correct value directly? Why is oclstdlib.ecore available with a discrepant value?

There's a metalevel subtlety/horror. The EClassifier.instanceClass defines the type that GenModel synthesizes. So setting java.util.Set on SetType synthesizes SetTypeImpl as a Set sub-type.

OCLEcore does the thoroughly not-recommended trick of extending Ecore so that while CollectionTyupe is an EClassifier its instanceClass is reused with a different semantic for its instances. (NB the pivot escapes this nastiness.)

On the one hand I can't see that adding the one line setInstanceClass(Collection.class) in the CollectionTypeImpl constructor can do any harm; no JUnit tests fail.

On the other hand whenever we change things in Classic Ecore something seems to get broken.

If we fix the Java do we fix related bad declarations in oclstdlib.ecore; for Collection and OrderedSet specializations?

Trying to synthesize an OCL scenario where we could detect a Classic OCL error I try:

Set{1}->oclIsKindOf(Set)

and get a parsing error. Maybe collection type conformance is so broken it cannot be made worse.

ewillink/456513 available for review.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jan 05, 2015 08:22

Hi Ed,

Given how other collection types are instantiated, I'm not against the proposed change.

I'd only change the java code.

Please, don't forget to add the @generated NOT to the constructor comment.

Cheers,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Jan 10, 2015 19:11

Thanks. Pushed to master for M5.