eclipse-ocl / org.eclipse.ocl

Eclipse Public License 2.0
0 stars 0 forks source link

[pivot] Rationalize null-free collections #2287

Open eclipse-ocl-bot opened 3 days ago

eclipse-ocl-bot commented 3 days ago

| --- | --- | | Bugzilla Link | 581851 | | Status | NEW | | Importance | P3 normal | | Reported | Apr 22, 2023 03:39 EDT | | Modified | Apr 23, 2023 06:26 EDT | | See also | 578117, 581543 | | Reporter | Ed Willink |

Description

Bug 578117 observed that the null-free-ness of Collections was not adequately propagated.

The Bug 581543 introducing FlatClass etc to eliminate the ExecutorXXX classes gives CG/non-CG consistency.

But it becomes apparent that the 'collections-are-null-free' default is hardwired in many places and in some cases redefaults rather than propagates.

Therefore make sure that isNullFree/lower/upper is always passed. Introduce PivotConstants.DEFAULT_COLLECTIONS_ARE_NULL_FREE/DEFAULT_MAP_KEYS_ARE_NULL_FREE/DEFAULT_MAP_VALUES_ARE_NULL_FREE to eliminate all hardwiring.

We gradually get to a point where DEFAULT_COLLECTIONS_ARE_NULL_FREE can be set true/false and JUnit tests pass either way. (About 10 tests that expose the cardinality in an error message deviate in the error text.)

But testStringMatches fails for DEFAULT_COLLECTIONS_ARE_NULL_FREE = false.

'the' problem is that OrderedCollectionReverseOperation has no resolveReturnType overload so that the behaviour defaults to whatever the default was when OCLstdlib was generated. Seems that an overload is mandatory for any implementation that returns a Collection/Map; the Operation.type is unsound.

Rather than 50 custom overloads, perhaps there are just 3 or 4 strategies.

eclipse-ocl-bot commented 3 days ago

By Ed Willink on Apr 23, 2023 05:51

(In reply to Ed Willink from comment #0)

Rather than 50 custom overloads, perhaps there are just 3 or 4 strategies.

AbstractIterationOrOperation.resolveXXX already defines strategies.

Seems that an overload is mandatory for any implementation

Need about 10 return returnType - the current default, about 10 return source/argument determined nulity, and a few more esoteric ones.

? re-instate the return returnType default and risk new operations being ill-considered, or mandate and leave a hidden crash opportunity?

Return type default re-instated.

eclipse-ocl-bot commented 3 days ago

By Ed Willink on Apr 23, 2023 06:08

(In reply to Ed Willink from comment #0)

We gradually get to a point where DEFAULT_COLLECTIONS_ARE_NULL_FREE can be set true/false and JUnit tests pass either way. (About 10 tests that expose the cardinality in an error message deviate in the error text.)

This is more an observation that we hae very poor test coverage rather than null-free insensitivity.

An interesting new problem shows up in testLoad_Pivot_ocl where the gratuitous OrderedSet(Parameter) let-variable declaration in OperationCallExp::ArgumentTypeIsConformant:\ let operation : Operation = self.referredOperation in\ let parameters : OrderedSet(Parameter) = operation?.ownedParameters in ...\ let parameter : Parameter = parameters?->at(i) in\ let parameterType : Type = parameter.type in\ let requiredType : Type = if parameter.isTypeof ...

is now diagnosed as a safety violation since OrderedSet(Parameter[*|?]) may have a null content even though the initializer cannot.

The equivalent @Nullable funtionality in JDT strengthens known variable content from propagated values. We could do the same in OCL, but surely the gratuitous declaration is an explicit user intent? If too strong we will diagnose it, if too weak tough, the consequences will appear downstream.

If the DEFAULT_COLLECTIONS_ARE_NULL_FREE value was true, the specific problem goes away since the shortform OrderedSet(Parameter) defaults usefully to OrderedSet(Parameter[|1]) rather than confusingly to OrderedSet(Parameter[|?]).

eclipse-ocl-bot commented 3 days ago

By Ed Willink on Apr 23, 2023 06:26

(In reply to Ed Willink from comment #2)

If the DEFAULT_COLLECTIONS_ARE_NULL_FREE value was true,

Changing to true removes the [|1] from 8 test messages and adds [|?] to 1, which seems clearer omitting clutter except where we need to stress null-free.

UML compatibility suggests a null-free default since relations are supposed to reference real objects. But multi-values datatypes could realistially be null.

Ecore compatibility is a bit mixed but again null is prohobited in reference contexts.

Test pragmatism is not a partiularly good design guide, but we do seem to have many pushes towards a null-free default rather thanthe pedantic legacy compatibility of nullable.

Design decision: DEFAULT_COLLECTIONS_ARE_NULL_FREE is TRUE.