Corner Case got FeatureGroupTypeImpl::getAllFeatures()

[philip-alldredge]

I believe there is an unhandled corner case in FeatureGroupTypeImpl::getAllFeatures(). I believe it returns the incorrect features when called on a feature group type that: -Does not define its own features -Extends another feature group type that is the inverse of a feature group type and explicitly defined features(which are the complement of the other feature group type's features). -Is declared as the inverse of another feature group type

AADL Example:

package issue
public

system TestSys
features
    fg1: feature group TestExtendedFeatureGroup;
    fg2: feature group TestInvExtendedFeatureGroup;
end TestSys;

system TestSubSys
features
    in1: in feature;
    out1: out feature;
end TestSubSys;

system implementation TestSys.Impl
subcomponents
    ss1: system TestSubSys;
connections
    c1: feature fg1.in1 -> ss1.in1;
    c2: feature ss1.out1 -> fg2.out1;
end TestSys.Impl;

feature group TestFeatureGroup
features
    in1: in feature;
end TestFeatureGroup;

feature group TestExtendedFeatureGroup extends TestFeatureGroup
end TestExtendedFeatureGroup;

feature group TestInvFeatureGroup
features
    out1: out feature;
inverse  of TestFeatureGroup
end TestInvFeatureGroup;

feature group TestInvExtendedFeatureGroup extends TestInvFeatureGroup
inverse of TestExtendedFeatureGroup
end TestInvExtendedFeatureGroup;

end issue;

If getAllFeatures() is called on TestInvExtendedFeatureGroup, the feature "in1" is returned. I would expect "out1" to be returned. Below is code that implements an alternative version of getAllFeatures() that behaves according to my expectations. I currently have not intregrated into the class itself, but it could be easily adapted if the behavior of getAllFeatures is decided to be incorrect.

/**
 * Returns all the features owned by the feature group type or the type it extends. This differs from FeatureGroupType's getAllFeatures because it does not 
 * return features from the inverse.
 */
public static EList<Feature> getAllOwnedFeatures(final FeatureGroupType fgt) {
    final EList<Feature> features = new BasicEList<Feature>();
    FeatureGroupType temp = fgt;
    while(temp != null) {
        features.addAll(temp.getOwnedFeatures());
        temp = temp.getExtended();
    }

    return features;
}

public static EList<Feature> getAllFeatures(final FeatureGroupType fgt) {
    final EList<Feature> owned = getAllOwnedFeatures(fgt);
    final FeatureGroupType inverseFgt = fgt.getInverse();
    if (owned.isEmpty() && !Aadl2Util.isNull(inverseFgt)) {
        return getAllOwnedFeatures(inverseFgt);
    }

    return owned;
}

[reteprelief]

The feature group stuff is complicated enough that I have a proposal to get rid of the inverse of between feature group types. I believe that being able to declare the feature group as the inverse of a feature group type is sufficient (we did not have that in V1).

I think there is a legality rule in V2.1 that does not allow you to extend another FGT that itself is an inverse. Let me take a closer look and then I’ll adapt your fix.

[philip-alldredge]

I agree it's complicated. It makes things like determining which direction a feature inside of a nested feature group even more complicated than it would be with just the inverse of on the feature groups themselves.

I don't think I have a copy of the finalized V2.1 standard(I've been working off one of the drafts). Unless it changed in a later version, section 8.2, L3 and L5 seem to allow it.

[lwrage]

Fixed, commit refers to #227