eclipse-ocl / org.eclipse.ocl

Eclipse Public License 2.0
0 stars 0 forks source link

[classic] Incorrect oclstdlib.ecore OrderedSet::collectedNested return #1682

Closed eclipse-ocl-bot closed 1 month ago

eclipse-ocl-bot commented 1 month ago

| --- | --- | | Bugzilla Link | 490982 | | Status | RESOLVED WONTFIX | | Importance | P3 normal | | Reported | Apr 04, 2016 04:10 EDT | | Modified | Apr 04, 2016 05:35 EDT | | See also | 350392 | | Reporter | Ed Willink |

Description

From Bug 350392#c4

This is due to the oclstdlib incorrectly describing the operation.

org.eclipse.ocl.ecore/model/oclstdlib.ecore has "OrderedSet(T)_Class.collectNested(OclExpression) : Bag(T2)". The EvaluationVisitorImpl, however, correctly uses an ArrayList (Sequence) for the result of this call.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Apr 04, 2016 05:15

How pervasive is the erroneous OrderedSet(T)_Class.collectNested return?

AbstractOCLAnalyzer special cases Sequence/OrderedSet to return Sequence: correct.

OCLStandardLibraryUtil.createOrderedSetIterators reuse createSetIterators: wrong.

OCLStandardLibraryUtil.getOrderedSetTypeResultTypeOf omits a COLLECT_NESTED case and so delegates to getSetTypeResultTypeOf: wrong.

JUnit tests - no test applicable. New tests:

assertQueryEquals(null, createSequence(true, false, true), "OrderedSet{1,2,3}->collectNested(p | (p.mod(2)) <> 0)");\ assertQueryEquals(null, false, "OrderedSet{1,2,3}->collectNested(p | (p.mod(2)) <> 0)->at(2)");

Both pass since:

EvaluationVisitorImpl.evaluateCollectNestedIterator distinguishes Bag/Set from Sequence/OrderedSet.

Therefore Evaluation ok, most parsing ok. Need a stronger test to traverse bad path.\ \ Instrumenting the existing JUnit tests to see where OCLStandardLibraryUtil.getOrderedSetTypeResultTypeOf it appears only to be called from the sysntax helper when offering completion assists.

Crippling createSetIterators to see what tests use it causes 4 failures. Three for completion assists and one checking on the available iterator count.

Checking on use of oclstdlib.ecore (oclstdlib.uml has the same bug). It is not normally used unless the user provides an OCLstdlib URI mapping. By default the in memory moderl is built programmatically using createOrderedSetIterators so either way, there is buggy return for collectNested in the library 'model'. But the library 'model' is not much used.

Difficult compromise choice:

Evaluation and non-trivial parsing is ok. Do we correct obscure inaccuracies that are unlikely to be used?

Yes, better to be correct.

No, we have no clients who have reported a functional problem. Change it and we might break someone.

(OMG. OrderedSet.collectNested was unspecified in 2.0, returns Sequence in 2.2, so OCL evaluation is 2.2. No OMG-based excuse for the model error.)

The most obvious Eclipse clients are Acceleo/QVTo, that may well serialize OrderedSet::collectNested references in its AS. A change to the referenced OCL models may affect Acceleo/QVTo validation, or a workaround for the broken model.

On balance, I'm inclined to WONTFIX. But these are outright easily fixed bugs. If we don't fix these do we ever fix anything?

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Apr 04, 2016 05:35

(In reply to Ed Willink from comment #1)

But these are outright easily fixed bugs. If we don't fix these do we ever fix anything?

As soon as I start to fix this, I spot that collect is defined to return Collection for all forms of Collection. Exactly as specified by OCL 2.2 that neglected to define the overrides, but not what is implemented by evaluateCollectIterator.

This chaos of multiple points of implementation for a single specification is one of many motivations for the Pivot model where the extensible library definition for OrderedSet contains:

iteration collect(V)(i : T[?] | lambda : Lambda T() : V[?]) : Sequence(V) => 'org.eclipse.ocl.pivot.library.iterator.CollectIteration';

iteration collectNested(V)(i : T[?] | lambda : Lambda T() : V[?]) : Sequence(V) => 'org.eclipse.ocl.pivot.library.iterator.CollectNestedIteration';

as 100% of the specification and then org.eclipse.ocl.pivot.library.iterator.CollectIteration and org.eclipse.ocl.pivot.library.iterator.CollectNestedIteration as 100% of the implementation.

It is not intended to chase down the many minor inconsistencies in Classic OCL. WONTFIX.