osate / osate2

Open Source AADL2 Tool Environment
http://osate.org
Eclipse Public License 2.0
36 stars 8 forks source link

Rename refactoring misses a reference #2262

Closed lwrage closed 4 years ago

lwrage commented 4 years ago

Summary

There is a case where renaming a subcomponent does not rename a reference to this subcomponent in a property association.

Expected and Current Behavior

The model below has two references to bus subcomponent theBus. When renaming theBus to, for example, theBus1, it is renamed in the properties section but not in the property association at connection conni. Both should be renamed.

Note that highlighting references works.

Steps to Reproduce

  1. Load the following mode
  2. Use rename refactoring on theBus in top.i
package Issue2259
public
    bus MyBus
    end MyBus;

    system S1
        features
            out1: out data port;
    end S1;

    system S2
        features
            in1: in data port;
    end S2;

    -- assembled system
    system top
    end top;

    system implementation top.i
        subcomponents
            sub1: system s1;
            sub2: system s2;
            theBus: bus MyBus;
        connections
            conn1: port sub1.out1 -> sub2.in1 {
                Actual_Connection_Binding => (reference (theBus));
            };
        properties
            -- Bind the connections
            Actual_Connection_Binding => (reference (theBus)) applies to conn1;
    end top.i;
end Issue2259;

Environment

lwrage commented 4 years ago

Renaming calls Aadl2SerializerScopeProvider to get the scope for the reference to theBus (in org.eclipse.xtext.ui.refactoring.impl.RefactoringCrossReferenceSerializer.getCrossRefText()). When processing the property value at the port connection the scope is empty. This is wrong.

lwrage commented 4 years ago

The bug is in PropertiesScopeProvider.namespaceForPropertyAssociation(). It does not return the classifier for local property associations of connections (and other elements that do not reference a classifier). It returns the connection (or other element), instead.

joeseibel commented 4 years ago

I'm not convinced that this is a bug in the scope provider. Is it instead a bug that the linking service is resolving the reference to theBus in the property association on conn1? In other words, is it legal to have a reference value in a property association that is not a direct child of a classifier, feature group, or subcomponent?

Naming rule N6 of section 11.4 says, "If a reference property is associated with a core AADL model element, then the contained model element path of a reference term can only refer to a core AADL model element. The first identifier in the path must be defined in the namespace of the directly enclosing component that contains the property association or the classifier of the subcomponent when associated with a subcomponent."

In this case, the property association does not have a directly enclosing component, since it is indirectly enclosed by the component implementation via the connection and the connection has no namespace.

lwrage commented 4 years ago

It should probably read closest enclosing component or so. This was discussed a couple of years ago: the goal was to allow local property associations for Actual_Connection_Binding. The linking service is correct, the standard still a bit unclear, and the scoping clearly wrong.