scalameta / metals

Scala language server with rich IDE features 🚀
https://scalameta.org/metals/
Apache License 2.0
2.09k stars 331 forks source link

incomplete code lenses when initializing metals on a test file #5115

Closed ghostbuster91 closed 1 year ago

ghostbuster91 commented 1 year ago

Describe the bug

I noticed that if I open neovim and go straight to a scalatest file code lenses appear only on the class level until the file gets reloaded (e.g. due to some change). On the other hand, opening first a different scala file (that initializes metals) and navigating then to the scalatest file shows complete code lenses (for both the class and all the tests).

Reproduction steps:

Part 2:

Full code lenses can be obtained when make a change in the test fail and saving it like in the gif: Peek 2023-03-30 18-14

Expected behavior

The same complete code lenses should be shown regardless of when metals was attached.

Operating system

ubuntu

Version of Metals

v0.11.11

Commit of nvim-metals

51e88e4f5eeadbd92a75cae71c5cbb75f3cb6765

Additional info

Incomplete response:

[DEBUG][2023-04-01 21:02:39] .../vim/lsp/rpc.lua:387    "rpc.receive"   {  id = 7,  jsonrpc = "2.0",  result = { {      command = {        arguments = { {            data = { "com.softwaremill.bootzooka.user.UserValidatorSpec" },            dataKind = "scala-test-suites",            targets = { {                uri = "file:/home/kghost/workspace/bootzooka/backend/?id=backend-test"              } }          } },        command = "metals-run-session-start",        title = "test"      },      range = {        ["end"] = {          character = 23,          line = 5        },        start = {          character = 6,          line = 5        }      }    }, {      command = {        arguments = { {            data = { "com.softwaremill.bootzooka.user.UserValidatorSpec" },            dataKind = "scala-test-suites",            targets = { {                uri = "file:/home/kghost/workspace/bootzooka/backend/?id=backend-test"              } }          } },        command = "metals-debug-session-start",        title = "debug test"      },      range = {        ["end"] = {          character = 23,          line = 5        },        start = {          character = 6,          line = 5        }      }    } }}

Complete response:

[DEBUG][2023-04-01 21:11:32] .../vim/lsp/rpc.lua:387    "rpc.receive"   {  id = 21,  jsonrpc = "2.0",  result = { {      command = {        arguments = { {            data = { "com.softwaremill.bootzooka.user.UserValidatorSpec" },            dataKind = "scala-test-suites",            targets = { {                uri = "file:/home/kghost/workspace/bootzooka/backend/?id=backend-test"              } }          } },        command = "metals-run-session-start",        title = "test"      },      range = {        ["end"] = {          character = 23,          line = 5        },        start = {          character = 6,          line = 5        }      }    }, {      command = {        arguments = { {            data = { "com.softwaremill.bootzooka.user.UserValidatorSpec" },            dataKind = "scala-test-suites",            targets = { {                uri = "file:/home/kghost/workspace/bootzooka/backend/?id=backend-test"              } }          } },        command = "metals-debug-session-start",        title = "debug test"      },      range = {        ["end"] = {          character = 23,          line = 5        },        start = {          character = 6,          line = 5        }      }    }, {      command = {        arguments = { {            data = {              environmentVariables = {},              jvmOptions = {},              suites = { {                  className = "com.softwaremill.bootzooka.user.UserValidatorSpec",                  tests = { "validate should not accept too short login" }                } }            },            dataKind = "scala-test-suites-selection",            targets = { {                uri = "file:/home/kghost/workspace/bootzooka/backend/?id=backend-test"              } }          } },        command = "metals-run-session-start",        title = "test case"      },      range = {        ["end"] = {          character = 48,          line = 17        },        start = {          character = 2,          line = 17        }      }    }, {      command = {        arguments = { {            data = {              environmentVariables = {},              jvmOptions = {},              suites = { {                  className = "com.softwaremill.bootzooka.user.UserValidatorSpec",                  tests = { "validate should not accept too short login" }                } }            },            dataKind = "scala-test-suites-selection",            targets = { {                uri = "file:/home/kghost/workspace/bootzooka/backend/?id=backend-test"              } }          } },        command = "metals-debug-session-start",        title = "debug test case"      },      range = {        ["end"] = {          character = 48,          line = 17        },        start = {          character = 2,          line = 17        }      }    }, {      command = {        arguments = { {            data = {              environmentVariables = {},              jvmOptions = {},              suites = { {                  className = "com.softwaremill.bootzooka.user.UserValidatorSpec",                  tests = { "validate should not accept login containing only empty spaces" }                } }            },            dataKind = "scala-test-suites-selection",            targets = { {                uri = "file:/home/kghost/workspace/bootzooka/backend/?id=backend-test"              } }          } },        command = "metals-run-session-start",        title = "test case"      },      range = {        ["end"] = {          character = 67,          line = 11        },        start = {          character = 2,          line = 11        }      }    }, {      command = {        arguments = { {            data = {              environmentVariables = {},              jvmOptions = {},              suites = { {                  className = "com.softwaremill.bootzooka.user.UserValidatorSpec",                  tests = { "validate should not accept login containing only empty spaces" }                } }            },            dataKind = "scala-test-suites-selection",            targets = { {                uri = "file:/home/kghost/workspace/bootzooka/backend/?id=backend-test"              } }          } },        command = "metals-debug-session-start",        title = "debug test case"      },      range = {        ["end"] = {          character = 67,          line = 11        },        start = {          character = 2,          line = 11        }      }    } }}
ckipp01 commented 1 year ago

Thanks for the report @ghostbuster91! I just tried playing around with this and the behavior seems a little bit different from me when I do it for the first time vs when I do it a second time. For example the first time I followed you instructions and opened UserValidatorSpec.scala directly I got no lenses when the indexing finished. Once I saved the file, I got them at the class level. After saving again I got them at the individual test level. After closing and doing this all again, I got them all right away. The third time I only got the class level ones without doing anything, and then I got the individual ones.

Some of this seems like a little bit of ordering here with when things are being indexed, test being discovered, etc. It might just be that at the moment that the lens request goes to Metals it only has the class ones, and then once it's triggered again, you'll get everything. I'll need to dig in a bit more on this or maybe someone else knows a bit more about the order that tests are discovered in Metals.

ckipp01 commented 1 year ago

O, and one more things. What you have in your config will matter here as well. For example if you have what I have with:

    api.nvim_create_autocmd({ "BufEnter", "BufWritePost" }, {
      callback = vim.lsp.codelens.refresh,
      buffer = bufnr,
      group = lsp_group,
    })

Then what you wrote sort of makes sense. Right when you open the file the BufEnter is called and the response from Metals at that time may just not have any code lenses for you. However if would have entered that file later, then the autocmd is triggered at a later time, when Metals has that info.

ghostbuster91 commented 1 year ago

Thanks for taking a look at it.

Some of this seems like a little bit of ordering here with when things are being indexed, test being discovered, etc. It might just be that at the moment that the lens request goes to Metals it only has the class ones, and then once it's triggered again, you'll get everything.

Right, that makes a lot of sense.

What you have in your config will matter here as well.

Totally, however to my surprise I discovered that I don't have anything like that. My config is sort of a mess at that point but fwiw here is the link: https://github.com/ghostbuster91/dot-files/blob/master/programs/neovim/init.lua#L272 Could it be that neovim is triggering that refresh on its own?

ckipp01 commented 1 year ago

Totally, however to my surprise I discovered that I don't have anything like that. My config is sort of a mess at that point but fwiw here is the link: https://github.com/ghostbuster91/dot-files/blob/master/programs/neovim/init.lua#L272 Could it be that neovim is triggering that refresh on its own?

Ooooo, the plot thickens! This one confused me for a bit because no, by default nvim shouldn't do this. However, I think what is actually triggering it is this:

https://github.com/scalameta/nvim-metals/blob/51e88e4f5eeadbd92a75cae71c5cbb75f3cb6765/lua/metals/handlers.lua#L58-L59

My guess is that Metals is sending a metals-model-refresh which is then calling lsp.codelens.refresh() and giving you code lenses. This could in theory be why you're experiencing somewhat unexpected or odd results with code lenses since the only time the client is every requesting them is only when the model refresh comes in. My expectation is that if you added an autocmd for the code lens, then everything might be much closer to what you'd expect. Could you give that a try? I moved this over to Metals ha, but I think this might end up being nvim-metals specific after all.

LaurenceWarne commented 1 year ago

I think I've seen similar behaviour to what's described, but with Emacs, at least with test case lenses.

I've had some success calling client.refreshModel here (workspace/codeLens/refresh), since it doesn't otherwise appear clients are asked to refresh lenses when tests have been discovered and added to the index.

I'll test a bit further and maybe open a PR :+1:

Edit: Opened a PR here: https://github.com/scalameta/metals/pull/5124, any comments or testing welcome :slightly_smiling_face:

ckipp01 commented 1 year ago

Thanks for looking at this @LaurenceWarne! Now that #5124 is merged if you also add the autocmd to your config @ghostbuster91 I think you should see what you'd expect. I'm going to close this for now, but after trying the latest snapshot feel free to report back if something still isn't working as expected.