Regression in Nova's LSP configuration support

[reykjalin (Kristófer Reykjalín)]

After updating Nova to the latest version (currently version 9.1 Build 383286) it seems Nova no longer sends updated configurations to language servers. I noticed this because a key part of the features in the Intelephense language server extension I maintain relies on you being able to update LSP configurations, specifically being able to add stubs to support development for projects like WordPress.

It seems this is a regression introduced in v9, based on testing Nova v9 and v8.4.

Reproduction steps

1. Use the following code as a test file:

<?php

add_filter( 'test_filter', 'foo' );
intval( '1' );

2. Install the Intelephense extension from the extension library. Make sure to use the one from me, not the one from GeneaLabs, since I don’t think they’ve added any LSP configuration options. If they have I suspect you’ll run into the same problem.
3. In the Intelephense extension preferences (either in the project menu or extensions menu) add a line to the stubs list with a value of wordpress.

4. Open the test.php file that has the code above.
5. You may need to run the “Restart Intelephense” action in Extensions > Intelephense > Restart Intelephense to trigger the language server in the test file.

In Nova 8.4

You’ll see that the language server knows about both intval() and add_filter():

In Nova 9.1 / Nova 9

You’ll see that the language server only knows about intval() and not add_filter():

I hope this regression is simple to fix since it unfortunately breaks support for something essential to the projects I work on

Let me know if there’s anything else I can do to help!

[logan]

Hi Kristófer,

I am investigating this now! Hopefully it’ll be something easy.

Logan

[robb-j (Rob Anderson)]

Hey, I’d like to +1 this. It seems to be the same for nova-yaml. Yaml Extension uses "yaml.schemas" configuration to support Kubernetes schemas

[logan]

Sorry for the delay on this—it’s been in my queue for Nova 9.2 but I’ve just now been able to get to it.

In investigating, I’ve run into some questions that I’m not sure on that (either of) you might be able to shed some light on.

For Nova 9, we added proper support for the Language Server Protocol workspace/configuration request. Previously, Nova could only “push” configuration keys to the language server via the workspace/didChangeConfiguration notification. I think this is where the issue might lie, as Intelephense is now requesting configuration keys from us explicitly when it needs them.

Reading the message stream, it seems that the value for the stubs property is being set as expected when requested by Intelephense, but I was wondering if there was any reason you can think of that it would not be respecting it since this change took place.

Thank you!

[robb-j (Rob Anderson)]

From a quick look at nova-yaml, I think it is ignoring anything that is passed to workspace/didChangeConfiguration and will only use the workspace/configuration request’s response now that it is supported.

Running the examples in the repo itself, it has yaml.schemas set inside the project configuration and it looks like nova isn’t returning that in the workspace/configuration response.

This output is from me saving the .nova/configuration.json to trigger the processes

server stdin.log

{"jsonrpc":"2.0","method":"workspace\/didChangeConfiguration","params":{"settings":{}}}Content-Length: 55
{“jsonrpc”:“2.0”,“id”:6,“result”:[null,null,null,null]}

server stdout.log

{"jsonrpc":"2.0","id":6,"method":"workspace/configuration","params":{"items":[{"section":"yaml"},{"section":"http"},{"section":"[yaml]"},{"section":"editor"}]}}Content-Length: 149

The .nova/configuration.json

I think sending the workspace/didChangeConfiguration is triggering the yaml server to do a workspace/configuration request, but that isn’t returning the correct configuration?

Update — If I rework the Configuration.json to not use dot-notation, Nova does pick up the configuration and return it to the workspace/configuration response.

.nova/Configuration.json

{
  "yaml": {
    "customTags": ["!secret scalar"],
    "schemas": { ... }
}

server stdin.log

{"jsonrpc":"2.0","id":12,"result":[{"schemas":{"https:\/\/raw.githubusercontent.com\/yannh\/kubernetes-json-schema\/master\/v1.20.5-standalone-strict\/all.json":["*deployment.yml","*deploy.yml","*configmap.yml","*cm.yml","*namespace.yml","*ns.yml","*persistentvolumeclaim.yml","*pvc.yml","*pod.yml","*po.yml","*secret.yml","*service.yml","*svc.yml","*serviceaccount.yml","*sa.yml","*daemonset.yml","*ds.yml","*cronjob.yml","*cj.yml","*job.yml","*ingress.yml","*ing.yml"]},"customTags":["!secret scalar"]},null,null,null]}

I think I touched on this in WorkspaceConfiguration middleware it didn’t seem like there was a defined way to store configuration sections

[logan]

Good to know. Thank you. I should be able to provide a fix for this in our 9.2 update to property return depth-based objects for preferences via the configuration request.

[reykjalin (Kristófer Reykjalín)]

For Nova 9, we added proper support for the Language Server Protocol workspace/configuration request. Previously, Nova could only “push” configuration keys to the language server via the workspace/didChangeConfiguration notification. I think this is where the issue might lie, as Intelephense is now requesting configuration keys from us explicitly when it needs them.

Reading the message stream, it seems that the value for the stubs property is being set as expected when requested by Intelephense, but I was wondering if there was any reason you can think of that it would not be respecting it since this change took place.

I’m not sure. It’s really difficult to debug anything with the Intelephense language server since I can’t seem to get debug logs from the language server to show up in the Nova Extension Console

Maybe this provides some extra context on what’s going on:

When a workspace/didChangeConfiguration notfication is received by the server it now sends a request for the entire config using workspace/configuration . Reason being is that the intelephense.files.exclude setting was recently changed to a resource level setting and there is no way to know for which folder uri the changed config belongs to with just workspace/didChangeConfiguration

There might also be some context in this issue but I’m really not sure.

I’d be happy to open an issue in the Intelephense repository if the problem persists after you’ve applied the fix you have in mind. In that case it’d be good to have some debug logs, and I think I’d have to ask you to provide them

Happy to help more if there’s anything I can do!

Thank you for the hard work on this @logan

[reykjalin (Kristófer Reykjalín)]

Things seem to be working in version 9.2. Thank you so much @logan !

[reykjalin (Kristófer Reykjalín)]

@logan it seems I was too hasty. The stubs still aren’t working . The project I was testing in had the wordpress stubs installed in the project, which I didn’t know about. Testing with the test file above reveals that the settings aren’t working .

Is there anyway you could look into this further? Or is there anything I can do to help?

[robb-j (Rob Anderson)]

I think the new dot-notation support isn’t quite right, it’s returning the whole block including the “section” that is requested. For nova-yaml:

The request:

{
  "jsonrpc": "2.0",
  "id": 1,
  "method": "workspace/configuration",
  "params": {
    "items": [
      { "section": "yaml" },
      { "section": "http" },
      { "section": "[yaml]" },
      { "section": "editor" }
    ]
  }
}

The response:

{
  "jsonrpc": "2.0",
  "id": 1,
  "result": [
    {
      "yaml": {
        "format": { "enable": false },
        "validate": true,
        "customTags": ["!secret scalar"],
        "completion": true,
        "schemas": {
          "kubernetes": [
            "*deployment.yml",
           ...
            "*ing.yml"
          ]
        },
        "hover": true
      }
    },
    null,
    null,
    null
  ]
}

It looks like the response contains an extra object with the section name in ("yaml" in this case) at the top of each configuration item, I think each result in the array should be be the value of that object instead. That’s what the yaml server is expecting.

[logan]

Aha, great catch. Thank you! I’ll look into getting this fixed.

[reykjalin (Kristófer Reykjalín)]

@logan I’m wondering if you’ve been able to fix the issue? I’d be happy to test a dev version of Nova if you think a fix is present there. If you know roughly what time you’re aiming at for the next release that’d be great to know as well; something like Q3/Q4 2022?

If y’all at Nova have been busy or focusing on more important issues; @robb-j did you find some way to work around the problem? I’d be interested in trying to apply the same workaround in the Intelephense extension if you did

[robb-j (Rob Anderson)]

I’ve not found a work around unfortunately, I attempted to rework my need for configuration to use custom LSP requests but I couldn’t get that working either. I’ve been tracking it on GitHub.

[logan]

Hey Kristófer,

Sorry for the delay on this. This is expected to be fixed in Nova 9.4, but that release has been delayed a bit. At the moment, I’m hoping we have it out by the end of the month or shortly after.

[reykjalin (Kristófer Reykjalín)]

Awesome, thank you Logan! I’ll report back here once Nova 9.4 is out

[logan]

After entirely too long (9.4 got delayed a bit longer, sorry folks), Nova 9.4 is out now. Let me know if this fixes your issue and if you see anything else that might need fixed!

[robb-j (Rob Anderson)]

I can confirm that this is working for nova-yaml’s use case, thanks for getting this released!

[reykjalin (Kristófer Reykjalín)]

I can confirm Nova 9.4 fixed the issue for the Intelephense extension. Thank you so much for your hard work on getting Nova 9.4 out