No effects from "Extract" code action

[ckipp01]

Describe the bug

Lately when triggering code actions I see a lot of messages that are unclear related to the "Extract ... as value" code action. For example I just opened Metals and triggered this in a few places and while I can't get it to happen all the time I often see code actions that look like this:

I'll try to get a more minimal reproduction, but I'm getting this often enough to just create this.

Expected behavior

I'd never expect to see a code action like this. When selected it also jumps you to a random place in your file. There are times that it will offer me this, and then after a bit on the same place it won't offer it to me anymore.

Here is part of the response that is relevant

[Trace - 06:47:13 pm] Sending response 'textDocument/codeAction - (25)'. Processing request took 9ms
Result: [
  {
    "title": "Extract `{\n    val ` ... as value",
    "kind": "refactor.extract",
    "edit": {
      "changes": {
        "file:///Users/ckipp/Documents/scala-workspace/scalameta-org/metals/metals/src/main/scala/scala/meta/internal/metals/codeactions/ExtractValueCodeAction.scala": [
          {
            "range": {
              "start": {
                "line": 30,
                "character": 40
              },
              "end": {
                "line": 90,
                "character": 3
              }
            },
            "newText": "(newValue)"
          },
          {
            "range": {
              "start": {
                "line": 30,
                "character": 32
              },
              "end": {
                "line": 30,
                "character": 33
              }
            },

Operating system

macOS

Editor/Extension

Nvim (nvim-metals)

Version of Metals

Latest snapshot

[tgodzik]

Thanks for reporting! I think we have 3 different issues here (discussed it a bit offline with @jkciesluk ):

• the title - we should make it more meaningful, we shouldn't show empty lines and in case of longer expressions maybe a range? This might require some tinkering.
• Maybe we shouldn't extract a single apply with {}, since that is usually some kind of Future or Task and extracting value would change the logic. Instead Extract function should work here,
• Extracting in the case @ckipp01 found is not working at all, but it's only needed to fix if we don't change the second point.

[kpodsiad]

Small reproduction for people without imagination (me 😄 )

    val x = Try {
      Some(new Exception)
    }

• Maybe we shouldn't extract a single apply with {}, since that is usually some kind of Future or Task and extracting value would change the logic. Instead Extract function should work here,

This can be also just a plain value and sometimes it can be helpful to extract it.

[tanishiking]

Maybe we shouldn't extract a single apply with {}, since that is usually some kind of Future or Task and extracting value would change the logic. Instead Extract function should work here,

Oh, sorry, I missed the context about this and rejected the change in the PR https://github.com/scalameta/metals/pull/4274 However, I'm basically against extracting block apply as a function. I understand extracting code in Future { }, Task { }, or, IO { } as value would change the semantics by this refactoring, but if we extract Try { } as function, the refacctored code will be a bit odd.

In my opinion, this extract code action should be a syntactic refactoring, and preserving the semantics should be a users' responsibility (as renaming symbols refactoring may break the source code).

[tanishiking]

Closing for https://github.com/scalameta/metals/pull/4274 :tada:

[tgodzik]

@tanishiking to we have a separate issue for the bug also reported by @ckipp01 ?

[tanishiking]

When selected it also jumps you to a random place in your file.

Extracting in the case @ckipp01 found is not working at all, but it's only needed to fix if we don't change the second point.

Oh, this one?

Sorry, maybe I didn't fully understand the issue. The thing https://github.com/scalameta/metals/pull/4274 fixed was about the code action title, another things haven't fixed. Let's re-open the issue. Can you rename the title of the issue? I actually fully understand the problem 😅

[tgodzik]

I changed the name, if anyone has a better idea for the detail please rename it :sweat_smile: