eclipse-xtext / xtext

Eclipse Xtext™ is a language development framework
http://www.eclipse.org/Xtext
Eclipse Public License 2.0
771 stars 321 forks source link

Renaming misses some qualified names in special cases #2474

Open msujew opened 3 years ago

msujew commented 3 years ago

Examine the following DSL structure that is split over three files:

package pack {
    extern class foo;
}
class pack.foo {
    extern method bar;
}
method pack.foo.bar { 
    ... 
}

pack.foo and pack.foo.bar are dot separated references to the respective external class or method definition, each defined in another file

When renaming foo or pack via the language server, the fully qualified method reference pack.foo.bar never changes. While debugging the issue I noticed that the framework never notices the fully qualified name should change. It doesn't even pick up the appropriate Resource for the recording mechanism of the ChangeSerializer. However, renaming pack correctly changes the pack.foo external class reference. Meaning, the reason behind this issue is somewhere in the recursive referencing nature of the grammar.

Instead of trying to change anything inside the likes of the ChangeSerializer, we instead opted to override the RenameService. There, we simply added our own code that operated directly on the WorkspaceEdit.

We don't think that this use case needs a fix within the framework. Instead I would like to propose an overridable method that is called after all other renaming changes have been applied. I'll complement this issue with a PR, where discussion about this proposal is appreciated.

szarnekow commented 3 years ago

Not sure about the semantics of your language and how your references are implemented, but if find-references finds the usages of pack in pack.foo, it should certainly rename these, too.

msujew commented 3 years ago

References are implemented via these rules:

ExternalMethodImplementation:
    'method' method=[Method|QualifiedName] ...

QualifiedName:
    ID ('.' ID)*;

When searching for references of pack, only itself appears. When searching for foo only its definition as well as its reference class pack.foo appears. This is expected, as pack.foo.bar is only a qualified name, and not an actual reference to any package or class rule. Hence the issue of it not being picked up by the ChangeSerializer.