Generated property getter methods don't handle the case when the property doesn't apply

[AaronGreenhouse]

Summary

When updating the FlowLatencyAnalysisSwitch to use the new generated property getter methods, I discovered that the generated property set code doesn't nicely handle the case where the property doesn't to apply to an AADL element. The underlying property lookup code throws a PropertyDoesNotApplyToHolderException. This is allowed to propagate to the caller. This would be fine if there were an easy way to test if the property applies, but there isn't one. You really just have to try it and see what happens. This exception should not be propagated, but caught internally to take advantage of the Optional class.

Expected behavior

I expected the generated property getter method to return an empty Optional instance in this case, same as when the property value is undefined.

Actual behavior

The underlying PropertyDoesNotApplyToHolderException propagates to the caller of the method.

Possible fix

I fixed this by updating CodeGenUtil.lookupProperty() on my local branch to be

    public static PropertyExpression lookupProperty(Property property, NamedElement lookupContext,
            Optional<Mode> mode) {
        try {
            Optional<PropertyExpression> modalValue = mode.map(m -> {
                PropertyAssociation association = lookupContext.getPropertyValue(property).first();
                if (association == null) {
                    PropertyExpression defaultValue = property.getDefaultValue();
                    if (defaultValue == null) {
                        throw new PropertyNotPresentException(lookupContext, property, "No property value");
                    } else {
                        return defaultValue;
                    }
                } else {
                    return association.valueInMode(m);
                }
            });
            return modalValue.orElseGet(() -> lookupContext.getNonModalPropertyValue(property));
        } catch (final PropertyDoesNotApplyToHolderException e) {
            throw new PropertyNotPresentException(lookupContext, property, "Property does not apply");
        }
    }

This seems to do what I want, but I'm not sure if there are other places I need to catch the exception in the code used by the generated method.

[AaronGreenhouse]

Discussed this with @lwrage . Probably we don't want to make this change, but we might want to add acceptsXXX() where xxx is a property name methods. Need to try to use these generated classes more.

[lwrage]

I now think we DO want the empty option if a property doesn't apply. In an analysis it is inconvenient to have to check first if a property applies and then get the value or hard code what is in the applies to clause. This is relevant in bulk operations such as getting all bindings in a model.

[jjhugues]

I understand the use case as different from one a typical user would do. But I would recommend having a safe (with exception raised) and unsafe (without exception) variants for this method. The rationale is that it may help spotting inconsistent model traversal.