Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename refactoring misses a reference #2262

Closed
lwrage opened this issue Mar 30, 2020 · 4 comments · Fixed by #2266
Closed

Rename refactoring misses a reference #2262

lwrage opened this issue Mar 30, 2020 · 4 comments · Fixed by #2266

Comments

@lwrage
Copy link
Contributor

@lwrage lwrage commented Mar 30, 2020

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

  • OSATE Version: 2.7.0
  • Operating System: all
@lwrage
Copy link
Contributor Author

@lwrage lwrage commented Apr 2, 2020

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
Copy link
Contributor Author

@lwrage lwrage commented Apr 2, 2020

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
Copy link
Contributor

@joeseibel joeseibel commented Apr 6, 2020

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
Copy link
Contributor Author

@lwrage lwrage commented Apr 6, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants