Diagnostics do not update on save

[jakcharvat]

It appears my project doesn't recompile when I save the source files in an sbt project. As a result, only treesitter errors seem to update with the source, but not build diagnostics.

Reproduction steps:

• Create a new scala 3 project with sbt new scala/scala3.g8

• Open the project in zed, and open Main.scala in the editor

• Delete the msg def

  • Diagnostics do not update

• Restart the language server using editor: restart language server

  • bloop: Not found: msg diagnostic appears as expected

• Bring def msg = ... back

  • The project is compilable and runnable with sbt, but the bloop: Not found: msg error remains until the next language server restart

---

Looking at lsp and bsp traces from metals-nvim, it appears to send a buildTarget/compile request on file saves, which eventually results in a build/publishDiagnostics response. The zed extension seems to send no such request, and diagnostics only update on language server (re)start.

It seems like I am able to force recompile the project using the "clean compile" button in the web interface of the server, or by manually restarting the language server using zed's editor: restart language server command, but neither of those is very ergonomic.

Am I the only one experiencing this issue? I have metals set up through coursier, using sbt to create projects. Zed metals settings only enable http server:

"metals": {
  "binary": {
    "arguments": ["-Dmetals.http=on"]
  },
  "initialization_options": {
    "isHttpEnabled": true
  }
}

Doctor output:

[tgodzik]

We should be getting didSave notifications when saving the file in Zed, which in turn should invoke the compilation. Are you able to check the .metals/lsp.trace.json file if that happens? If you create the file it should get polluted with traces

[jakcharvat]

Actually I'm not getting any traces there. When I make a bsp.trace.json I get a single build/initialize request with no response, but the lsp.trace.json file stays empty. Maybe zed intercepts them somewhere? When I use metals from neovim they get populated as I would expect.

The LSP features like goto definition, hovers, etc all work though, and I see their messages in the zed LSP Logs / RPC messages view. No didSave notifications there either though.

This is the only entry I get in bsp.trace.json:

[Trace - 05:27:01 PM] Sending request 'build/initialize - (1)'
Params: {
  "displayName": "Metals",
  "version": "1.3.5",
  "bspVersion": "2.2.0-M2",
  "rootUri": "file:///.../testproj/",
  "capabilities": {
    "languageIds": [
      "scala",
      "java"
    ],
    "jvmCompileClasspathReceiver": true
  },
  "data": {
    "javaSemanticdbVersion": "0.10.0",
    "semanticdbVersion": "4.9.9",
    "supportedScalaVersions": [
      "2.13.14",
      "2.12.19",
      "2.12.18",
      "2.12.17",
      "2.12.16",
      "2.13.11",
      "2.13.12",
      "2.13.13",
      "2.11.12"
    ],
    "enableBestEffortMode": false
  }
}

[jakcharvat]

Looks like zed was somehow interfering with the log files, just watching it with tail I do get messages in the lsp trace, but no didSave notifications.

Here's the trace for opening a file, removing the `msg` method, saving and closing the file from the reproduction steps in the original post

I have format on save enabled, so the format notification just before the close is when I saved the file.

[Trace - 05:40:20 PM] Received notification 'textDocument/didOpen'
Params: {
  "textDocument": {
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala",
    "languageId": "scala",
    "version": 0,
    "text": "@main def hello(): Unit \u003d\n  println(\"Hello world!\")\n  println(msg)\n\ndef msg: String \u003d \"Hello, World!\"\n"
  }
}


[Trace - 05:40:20 PM] Received request 'textDocument/inlayHint - (121)'
Params: {
  "textDocument": {
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala"
  },
  "range": {
    "start": {
      "line": 0,
      "character": 0
    },
    "end": {
      "line": 5,
      "character": 0
    }
  }
}


[Trace - 05:40:20 PM] Received request 'textDocument/documentHighlight - (122)'
Params: {
  "textDocument": {
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala"
  },
  "position": {
    "line": 0,
    "character": 0
  }
}


[Trace - 05:40:20 PM] Sending response 'textDocument/documentHighlight - (122)'. Processing request took 7ms
Result: [
  {
    "range": {
      "start": {
        "line": 0,
        "character": 0
      },
      "end": {
        "line": 0,
        "character": 5
      }
    },
    "kind": 2
  }
]


[Trace - 05:40:20 PM] Received request 'textDocument/codeAction - (123)'
Params: {
  "textDocument": {
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala"
  },
  "range": {
    "start": {
      "line": 0,
      "character": 0
    },
    "end": {
      "line": 0,
      "character": 0
    }
  },
  "context": {
    "diagnostics": [],
    "only": [
      "quickfix",
      "refactor",
      "source.organizeImports"
    ]
  }
}


[Trace - 05:40:20 PM] Sending response 'textDocument/codeAction - (123)'. Processing request took 38ms
Result: [
  {
    "title": "Organize imports",
    "kind": "source.organizeImports",
    "diagnostics": [],
    "edit": {
      "changes": {
        "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala": []
      }
    }
  }
]


[Trace - 05:40:21 PM] Received request 'textDocument/documentHighlight - (124)'
Params: {
  "textDocument": {
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala"
  },
  "position": {
    "line": 4,
    "character": 23
  }
}


[Trace - 05:40:21 PM] Sending response 'textDocument/documentHighlight - (124)'. Processing request took 3ms
Result: []


[Trace - 05:40:21 PM] Received request 'textDocument/documentHighlight - (125)'
Params: {
  "textDocument": {
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala"
  },
  "position": {
    "line": 4,
    "character": 24
  }
}


[Trace - 05:40:21 PM] Sending response 'textDocument/documentHighlight - (125)'. Processing request took 3ms
Result: []


[Trace - 05:40:21 PM] Received request 'textDocument/documentHighlight - (126)'
Params: {
  "textDocument": {
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala"
  },
  "position": {
    "line": 3,
    "character": 0
  }
}


[Trace - 05:40:21 PM] Sending response 'textDocument/documentHighlight - (126)'. Processing request took 2ms
Result: []


[Trace - 05:40:21 PM] Received request 'textDocument/codeAction - (127)'
Params: {
  "textDocument": {
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala"
  },
  "range": {
    "start": {
      "line": 3,
      "character": 0
    },
    "end": {
      "line": 4,
      "character": 24
    }
  },
  "context": {
    "diagnostics": [],
    "only": [
      "quickfix",
      "refactor",
      "source.organizeImports"
    ]
  }
}


[Trace - 05:40:21 PM] Sending response 'textDocument/codeAction - (127)'. Processing request took 25ms
Result: [
  {
    "title": "Organize imports",
    "kind": "source.organizeImports",
    "diagnostics": [],
    "edit": {
      "changes": {
        "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala": []
      }
    }
  }
]


[Trace - 05:40:22 PM] Received notification 'textDocument/didChange'
Params: {
  "textDocument": {
    "version": 1,
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala"
  },
  "contentChanges": [
    {
      "text": "@main def hello(): Unit \u003d\n  println(\"Hello world!\")\n  println(msg)\n"
    }
  ]
}


[Trace - 05:40:22 PM] Sending notification 'textDocument/publishDiagnostics'
Params: {
  "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala",
  "diagnostics": []
}


[Trace - 05:40:22 PM] Received request 'textDocument/documentHighlight - (128)'
Params: {
  "textDocument": {
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala"
  },
  "position": {
    "line": 3,
    "character": 0
  }
}


[Trace - 05:40:22 PM] Sending response 'textDocument/documentHighlight - (128)'. Processing request took 8ms
Result: []


[Trace - 05:40:22 PM] Received request 'textDocument/codeAction - (129)'
Params: {
  "textDocument": {
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala"
  },
  "range": {
    "start": {
      "line": 3,
      "character": 0
    },
    "end": {
      "line": 3,
      "character": 0
    }
  },
  "context": {
    "diagnostics": [],
    "only": [
      "quickfix",
      "refactor",
      "source.organizeImports"
    ]
  }
}


[Trace - 05:40:22 PM] Sending response 'textDocument/codeAction - (129)'. Processing request took 39ms
Result: [
  {
    "title": "Organize imports",
    "kind": "source.organizeImports",
    "diagnostics": [],
    "edit": {
      "changes": {
        "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala": []
      }
    }
  }
]


[Trace - 05:40:22 PM] Received request 'textDocument/formatting - (130)'
Params: {
  "textDocument": {
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala"
  },
  "options": {
    "tabSize": 2,
    "insertSpaces": true,
    "trimTrailingWhitespace": true,
    "insertFinalNewline": true,
    "trimFinalNewlines": true
  }
}


[Trace - 05:40:22 PM] Sending response 'textDocument/formatting - (130)'. Processing request took 12ms
Result: []


[Trace - 05:40:22 PM] Received request 'textDocument/inlayHint - (131)'
Params: {
  "textDocument": {
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala"
  },
  "range": {
    "start": {
      "line": 0,
      "character": 0
    },
    "end": {
      "line": 3,
      "character": 0
    }
  }
}


[Trace - 05:40:22 PM] Received notification '$/cancelRequest'
Params: {
  "id": 121
}


[Trace - 05:40:22 PM] Sending response 'textDocument/inlayHint - (121)'. Processing request took 2693ms
Result: null


[Trace - 05:40:25 PM] Received notification '$/cancelRequest'
Params: {
  "id": 131
}


[Trace - 05:40:25 PM] Sending response 'textDocument/inlayHint - (131)'. Processing request took 2741ms
Result: null


[Trace - 05:40:25 PM] Received notification 'textDocument/didClose'
Params: {
  "textDocument": {
    "uri": "file:///Users/jakcharvat/FIT/OOP/testproj/src/main/scala/Main.scala"
  }
}

[tgodzik]

That's unexpected, any idea if that is on purpose? we do depend on didSave notification

[jakcharvat]

Could it be related to zed-industries/zed#14412?

I'll see if I can figure out whether the metals server declares support for textDocumentSync.save...

https://github.com/ckipp01/metals/blob/260e9b90b46ba4e5fc790e63467f5c668c1a7bfc/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala#L1205
Looks like it should, is there a way I can query my metals binary to see what features it declares support for?

[WeetHet]

It seems like zed thinks that watched paths for Metals are empty, weird

[WeetHet]

Oh, there seem to be two issues here, first, metals sends watchers with file://, which zed doesn't handle well. It's a relatively easy fix, strip file:// in zed. I can implement that. The second issue, though, is that metals doesn't send any watchers that match src/main/Scala/*.scala. For me the watchers sent are:

[crates/project/src/lsp_store.rs:4546:63] params.watchers = [
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/*.sbt",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/pom.xml",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/*.sc",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/*?.gradle",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/*.gradle.kts",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/project/*.{scala,sbt}",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/project/project/*.{scala,sbt}",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/project/build.properties",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/.metals/.reports/bloop/*/*",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/**/.bsp/*.json",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/**/BUILD",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/**/BUILD.bazel",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/WORKSPACE",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/WORKSPACE.bazel",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/**/*.bzl",
        ),
        kind: None,
    },
    FileSystemWatcher {
        glob_pattern: String(
            "file:///Users/weethet/Projects/tmp/kyo-pl/*.bazelproject",
        ),
        kind: None,
    },
]

[tgodzik]

Och, we do use our own watchers for most of the stuff, since we had worse experience with in build VS Code ones previously. We could add all Scala paths for Zed at least, though not sure we want to add them in general, which is a much bigger change.

Though I don't believe only watched files should be sent if they are indeed open. At least this is not the experience in other editors,

[filipwiech]

Hey, I'm terribly sorry, due to the lack of time I have completely forgot to link this issue here:
zed-industries/zed#18636
You can find more details there and a linked change that explains the source of the problem. We need 2 things to fix this:

• First and foremost, Metals has to register more globs for file watching, basically to over all the Scala and Java source code in the workspace, not only a few special build files.
• We have to switch the default glob syntax in this plugin to a VS Code one to get rid of the file:// prefix. One can already do it with a custom Zed config like this one:

"lsp": {
  "metals": {
    "initialization_options": {
      "globSyntax": "vscode"
    }
  }
}

Hope that helps. 👍

[WeetHet]

I've looked a bit into didSave, and I'm unsure if textDocument/didSave should only be sent to the language servers watching the saved file. The spec is unclear, and, for example, helix sends it to everyone

If someone is better than me at reading VSCode files, here's the impl for VSCode as far as I see it also send the notification to all servers, so metals might just be correct here

[tgodzik]

We also have didChangeWatchedFiles notification which is normally what I assumed was decided by the globs.

[tgodzik]

I commented a bit on the issue in Zed and I feel like they arbitrarily decided that LSP spec is saying one thing without consulting anyone, which seems to be the root of the problem. We could try and switch file watching for Scala/Java files to use the default file watching, but I can't promise when it will be. I need to work on a release first

[WeetHet]

I commented a bit on the issue in Zed and I feel like they arbitrarily decided that LSP spec is saying one thing without consulting anyone, which seems to be the root of the problem. We could try and switch file watching for Scala/Java files to use the default file watching, but I can't promise when it will be. I need to work on a release first

I've created a PR reverting the changes to Zed as they seem incorrect, let's see if it goes through

[tgodzik]

At a minimum I think we would be able to force a setting.

Overall, the option to declare files we are interested in would be good to have, but it is simply not there yet.

[WeetHet]

Overall, the option to declare files we are interested in would be good to have, but it is simply not there yet.

I'd prefer that to be a part of the LSP and not a Zed-specific feature

[tgodzik]

Looks like it was fixed upstream