The code field in the Diagnostics structure is unexpectedly converted to a String

[Markiewic]

I'm working on an Angular extension. I'm currently solving issue nathansbradshaw/zed-angular#16, which says that Angular Language Server's Code Actions don't work in Zed.

After checking, I found that Zed converts the code field to a string in messages with the "textDocument/codeAction" method, which is a problem for Angular, since it uses strict error code comparison when looking for Code Actions. Here's an example of the message Zed gets when asking for diagnostics, and here's what it sends later.

Examples

Pay attention to the path params.context.diagnostics[*].code.

Received from ALS:

{
  "jsonrpc": "2.0",
  "method": "textDocument/publishDiagnostics",
  "params": {
    "uri": "file:///Users/a.khodov/vscodeprojects/leak-test/src/app/app.component.html",
    "diagnostics": [
      {
        "range": {
          "start": { "line": 5, "character": 2 },
          "end": { "line": 5, "character": 22 }
        },
        "message": "'app-data-carrier' is not a known element:\n1. If 'app-data-carrier' is an Angular component, then verify that it is included in the '@Component.imports' of this component.\n2. If 'app-data-carrier' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@Component.schemas' of this component to suppress this message.",
        "severity": 1,
        "code": -998001,
        "source": "ngtsc",
        "relatedInformation": [
          {
            "location": {
              "uri": "file:///Users/a.khodov/vscodeprojects/leak-test/src/app/app.component.ts",
              "range": {
                "start": { "line": 8, "character": 24 },
                "end": { "line": 8, "character": 36 }
              }
            },
            "message": "Error occurs in the template of component AppComponent."
          }
        ]
      },
      {
        "range": {
          "start": { "line": 8, "character": 7 },
          "end": { "line": 8, "character": 32 }
        },
        "message": "In the two-way binding syntax the parentheses should be inside the brackets, ex. '[(ngModel)]=\"(someValue)\"'.\n        Find more at https://angular.dev/guide/templates/two-way-binding",
        "severity": 2,
        "code": -998101,
        "source": "ngtsc",
        "relatedInformation": [
          {
            "location": {
              "uri": "file:///Users/a.khodov/vscodeprojects/leak-test/src/app/app.component.ts",
              "range": {
                "start": { "line": 8, "character": 24 },
                "end": { "line": 8, "character": 36 }
              }
            },
            "message": "Error occurs in the template of component AppComponent."
          }
        ]
      }
    ]
  }
}

Sent to ALS:

{
  "jsonrpc": "2.0",
  "id": 86,
  "method": "textDocument/codeAction",
  "params": {
    "textDocument": {
      "uri": "file:///Users/a.khodov/vscodeprojects/leak-test/src/app/app.component.html"
    },
    "range": {
      "start": { "line": 5, "character": 22 },
      "end": { "line": 5, "character": 22 }
    },
    "context": {
      "diagnostics": [
        {
          "range": {
            "start": { "line": 5, "character": 2 },
            "end": { "line": 5, "character": 22 }
          },
          "severity": 1,
          "code": "-998001",
          "source": "ngtsc",
          "message": "'app-data-carrier' is not a known element:\n1. If 'app-data-carrier' is an Angular component, then verify that it is included in the '@Component.imports' of this component.\n2. If 'app-data-carrier' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@Component.schemas' of this component to suppress this message."
        }
      ],
      "only": ["quickfix"]
    }
  }
}

Most likely, the unconditional conversion to String happens here. Compared to VS Code, it doesn't do that, it passes the code as it received it.

I intervened inside ALS and checked if this is really the reason - it turned out that it is (details in the attached issue).

I'm looking for a way to intervene in the RPC messages with an extension and convert the code back to a number. Or some other way to make sure the code's type doesn't change.

The LSP spec allows both a string and a number in this field. I believe that in this case, it is the language server that has the right to choose in what format it should accept and return numbers, and the editor should take into account either format.

[SomeoneToIgnore]

Nice catch.

To note, the real code -> String conversion happens here:

zed/crates/project/src/lsp_store.rs

Lines 7369 to 7372 in 5bd7eaa

	let code = diagnostic.code.as_ref().map(|code| match code {
	lsp::NumberOrString::Number(code) => code.to_string(),
	lsp::NumberOrString::String(code) => code.clone(),
	});

and most likely we have to adjust this across the Zed's code base, as there's no good way to understand whether the original was a String or a Number, to "intervene and convert it back".

Something similar happened with RequestId

zed/crates/context_server/src/client.rs

Lines 40 to 43 in 5bd7eaa

	pub enum RequestId {
	Int(i32),
	Str(String),
	}

some time ago, we can at least reuse the conversion code as the first step.

[Markiewic]

Thank you for reply. Should I create an issue out of this discussion?

[SomeoneToIgnore]

Sure, as that looks like a bug and has a good context.

I cannot guarantee it won't converted back though, as other team members might have other opinion on that.

[Markiewic]

I created an issue from this discussion but it didn't get marked as a bug. Could you please mark it as bug manually? #24084

[SomeoneToIgnore]

Fixed in #24347

[Markiewic]

Great news! Thank you all