[LSP] Xtext Grammar loaded twice when another Xtext grammar depends on it

[Mehmetcanss]

When the house.xtext grammar is dependent on another grammar person.xtext the person grammar is loaded twice:

1) Once from the Xtext bin files (org.eclipse.xtext.ISetup File) 2) A second load via the PersonStandaloneSetup.doSetup() call inside the HouseStandaloneSetupGenerated.createInjectorAndDoEMFRegistration() Method

A consequence of this is that the Content Assist for the Person Grammar does not work.

Below a sample projects and reproduction steps:

1) checkout the project => git clone git@github.com:Mehmetcanss/example_xtext_language_server_vscode.git (Repository url: https://github.com/Mehmetcanss/example_xtext_language_server_vscode) 2) change directory to parent folder => cd example_xtext_language_server_vscode/org.xtext.example.mydsl.parent/ 3) generate xtext artifacts and build the language server => ./gradlew clean org.xtext.example.mydsl.vscode:installLanguageServerDist 4) Open a visual studio code instance, contest assist works for the house grammar but not for the person grammar. (use the Extension run configuration) 5) You can use the RunServer class at the ide project to debug (You have to use the Extension Running Server Run Configuration in VS-Code)

[cdietrich]

i think we need a special standalone setup that can handle this and creates injectors only once / does only metamodel registrations for parent grammar and not injector creation as well

[cdietrich]

cc @kittaakos @svenefftinge

[soerendomroes]

I had the same problem and solved it by making the injector returned by the doSetup() method Singleton and registering the parent grammar before the child grammar. This way the content assist still works.

[cdietrich]

@soerendomroes could you document your workaround here

[cdietrich]

it also looks like the order in

*ide/src/main/xtext-gen/META-INF/services/org.eclipse.xtext.ISetup

plays a role too. maybe it should be

• subgrammar
• supergrammar

@szarnekow any ideas/hints on this?

(wont solve the problem if dsl are separate though)

[cdietrich]

see also https://www.eclipse.org/forums/index.php/t/1099693/

[soerendomroes]

@cdietrich I do the following: For other subgrammars I only use a singleton injector, since they may not be instantiated by another bundle.

class MyLangStandaloneSetup extends MyLangStandaloneSetupGenerated {

    protected static var Injector injector

    /**
     * Used by LS to override previously created injector, if the current injector does not contain an MyLangIdeModule
     */
    public static var boolean force
    def static doSetup() {
        if (injector === null || force) {
            injector = new MyLangStandaloneSetup().createInjectorAndDoEMFRegistration()
        }
        return injector
    }
}

A simple singleton is not enough. It can happen that some other bundle calls do setup first, but does not bind MyLangIdeModule. Therefore, I have a singleton that I can override if it some other bundle already created a wrong injector.

[JanKoehnlein]

Stumbled upon this as well.

It seems bogus that HouseStandaloneSetupGenerated#createInjectorAndDoEMFRegistration() calls the static of the PersonStandaloneSetup.doSetup(). This way Xtext registers the wrong injector (runtime instead of IDE) in the LS scenario. This can be very hard to track down. It actually took me a few hours, because it only caused a linking issue in live-scope scenarios).

My current workaround is to just override Person.doSetup() to register ecore, xmi, xtextbin and the XtextPackage but nothing else. It is called by the sublanguage only which relies on these being set.

As I fix, I am inclined to change StandaloneSetupGenerated to

• not call superlang.doSetup()
• always generate the guarded registrations for ecore, xmi, xtextbin and the XtextPackage in createInjectorAndDoEMFRegistration and StandaloneSetup to
• no longer generate doSetup() Unfortunately, this would break some clients that relied on the superlanguage to also be registered automatically in the runtime scenario.

The combination of static methods and generated-once files makes it hard to find a non-breaking solution. An alternative would be to introduce an ISetupExtension interface with a different method to call in the ide scenario, but existing users wouldn't get the fix then, as the <lang>IdeStandaloneSetup is generate once.

[cdietrich]

@JanKoehnlein wont we be able to add the new IF/Method to the abstract class? the IF even could delegate to the current method in a default impl

[cdietrich]

@szarnekow what do you think?

[szarnekow]

I assume we'll still need the injector of the base languages being created to make sure that validators for the EPackages are registered. So skipping the base language is probably not a general solution to the problem. From the top of my head I don't see how we can use a standalone setup in a reasonable way in the IDE bundles with the shape of the code generation that we currently use.

It would probably help to extract a method setupBaseLanguage in the XYZStandaloneSetupGenerated and override setupBaseLanguage in an abstract XYZIdeSetupGenerated that should become the new base class for XYZIdeSetup. I'm not sure yet how this could be done in a backwards compatible way.

[cdietrich]

if you use e.g. xbase the service loader will also call the registration for the baselanguage itself. but i assume it has to be called first.

[cdietrich]

is there anything that speaks against completely separating standalone and ide setups and let backwards compatibility be backwards compatibility?

[szarnekow]

is there anything that speaks against completely separating standalone and ide setups and let backwards compatibility be backwards compatibility?

Since it's currently just very broken, I'd think backwards compatibility is not a reason to keep it as is. It'd be great of the ISetup service files would remain stable though.

[cdietrich]

is there anything that speaks against making the injector instances static singletons?

[szarnekow]

is there anything that speaks against making the injector instances static singletons?

Testability comes to my mind, but usually the InjectorProvider for tests does the caching anyhow.