microsoft / vscode-languageserver-node

Language server protocol implementation for VSCode. This allows implementing language services in JS/TS running on node.js
MIT License
1.47k stars 324 forks source link

Semantic Tokens LSP Specification Violation #780

Closed KenDJohnson closed 3 years ago

KenDJohnson commented 3 years ago

Using node module vscode-languageclient version 7.1.0-next4 (tried with 7.0.0, and 6.x as well) VSCode: 1.57.1 OS: Darwin x64 20.5.0 / macOS 11.4

The provided implementation for Semantic Tokens Client Capabilities does not appear to adhere to the published LSP specification for version 3.16.

The spec for SemanticTokensClientCapabilities states that the requests field is mandatory, as an object with two optional fields, but it seems that the implementation omits the mandatory field when both the range and full fields are empty. This breaks libraries that adhere to the specification by treating a missing requests field as an error (observed in the latest release of the lsp-types Rust crate).

For reference, the initialization request sent is:

{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":83550,"clientInfo":{"name":"vscode","version":"1.57.1"},"rootPath":"/Users/kjohnson/nessus-plugins","rootUri":"...","capabilities":{"workspace":{"applyEdit":true,"workspaceEdit":{"documentChanges":true,"resourceOperations":["create","rename","delete"],"failureHandling":"textOnlyTransactional"},"didChangeConfiguration":{"dynamicRegistration":true},"didChangeWatchedFiles":{"dynamicRegistration":true},"symbol":{"dynamicRegistration":true,"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]}},"executeCommand":{"dynamicRegistration":true},"configuration":true,"workspaceFolders":true},"textDocument":{"publishDiagnostics":{"relatedInformation":true,"versionSupport":false,"tagSupport":{"valueSet":[1,2]}},"synchronization":{"dynamicRegistration":true,"willSave":true,"willSaveWaitUntil":true,"didSave":true},"completion":{"dynamicRegistration":true,"contextSupport":true,"completionItem":{"snippetSupport":true,"commitCharactersSupport":true,"documentationFormat":["markdown","plaintext"],"deprecatedSupport":true,"preselectSupport":true,"tagSupport":{"valueSet":[1]}},"completionItemKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25]}},"hover":{"dynamicRegistration":true,"contentFormat":["markdown","plaintext"]},"signatureHelp":{"dynamicRegistration":true,"signatureInformation":{"documentationFormat":["markdown","plaintext"],"parameterInformation":{"labelOffsetSupport":true}},"contextSupport":true},"definition":{"dynamicRegistration":true,"linkSupport":true},"references":{"dynamicRegistration":true},"documentHighlight":{"dynamicRegistration":true},"documentSymbol":{"dynamicRegistration":true,"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]},"hierarchicalDocumentSymbolSupport":true},"codeAction":{"dynamicRegistration":true,"isPreferredSupport":true,"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["","quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}}},"codeLens":{"dynamicRegistration":true},"formatting":{"dynamicRegistration":true},"rangeFormatting":{"dynamicRegistration":true},"onTypeFormatting":{"dynamicRegistration":true},"rename":{"dynamicRegistration":true,"prepareSupport":true},"documentLink":{"dynamicRegistration":true,"tooltipSupport":true},"typeDefinition":{"dynamicRegistration":true,"linkSupport":true},"implementation":{"dynamicRegistration":true,"linkSupport":true},"colorProvider":{"dynamicRegistration":true},"foldingRange":{"dynamicRegistration":true,"rangeLimit":5000,"lineFoldingOnly":true},"declaration":{"dynamicRegistration":true,"linkSupport":true},"selectionRange":{"dynamicRegistration":true},"callHierarchy":{"dynamicRegistration":true},"semanticTokens":{"dynamicRegistration":true,"tokenTypes":["comment","keyword","number","regexp","operator","namespace","type","struct","class","interface","enum","typeParameter","function","member","macro","variable","parameter","property","label"],"tokenModifiers":["declaration","documentation","static","abstract","deprecated","async","readonly"]}},"window":{"workDoneProgress":true}},"initializationOptions":{...}}
rcjsuen commented 3 years ago

The spec for SemanticTokensClientCapabilities states that the requests field is mandatory, as an object with two optional fields, but it seems that the implementation omits the mandatory field when both the range and full fields are empty.

I'm not sure I understand this bit. When are those fields "empty"? From a quick glance of the code, it's not clear to me how requests would be missing...

https://github.com/microsoft/vscode-languageserver-node/blob/6eef6d21ec50399a67d80442c624344870f683c8/client/src/common/semanticTokens.ts#L98-L103

dbaeumer commented 3 years ago

That looks really strange. Need to investigate why this would be lost.

rcjsuen commented 3 years ago

Upon a closer inspection of the provided JSON, looks like four fields are missing (requests and three others).

https://github.com/microsoft/vscode-languageserver-node/blob/6eef6d21ec50399a67d80442c624344870f683c8/client/src/common/semanticTokens.ts#L97-L105

KenDJohnson commented 3 years ago

Looks like requests and formats to me. Spec seems to have multilineTokenSupport and overlappingTokenSupport as optional, so omitting those should be fine. I changed the Rust library's structures to make the requests and formats fields optional, and everything deserialized properly.

I should note, this only seemed to happen through the registerProposedFeatures call. Without that it didn't ever send semantic tokens caps. It's possible I didn't call that function correctly, this is the first TypeScript I've written, but my use seemed copacetic from the docs.

rcjsuen commented 3 years ago

Looks like requests and formats to me. Spec seems to have multilineTokenSupport and overlappingTokenSupport as optional, so omitting those should be fine.

Right, but I mean it's being set in the code so it should be sent...irregardless of the fact that it is technically optional.

rcjsuen commented 3 years ago

I added print statements to my code and it looks fine to me. I'm on vscode-languageclient 7.0.0.

{
  publishDiagnostics: {
    relatedInformation: true,
    versionSupport: false,
    tagSupport: { valueSet: [Array] },
    codeDescriptionSupport: true,
    dataSupport: true
  },
  synchronization: {
    dynamicRegistration: true,
    willSave: true,
    willSaveWaitUntil: true,
    didSave: true
  },
  completion: {
    dynamicRegistration: true,
    contextSupport: true,
    completionItem: {
      snippetSupport: true,
      commitCharactersSupport: true,
      documentationFormat: [Array],
      deprecatedSupport: true,
      preselectSupport: true,
      tagSupport: [Object],
      insertReplaceSupport: true,
      resolveSupport: [Object],
      insertTextModeSupport: [Object]
    },
    completionItemKind: { valueSet: [Array] }
  },
  hover: {
    dynamicRegistration: true,
    contentFormat: [ 'markdown', 'plaintext' ]
  },
  signatureHelp: {
    dynamicRegistration: true,
    signatureInformation: {
      documentationFormat: [Array],
      parameterInformation: [Object],
      activeParameterSupport: true
    },
    contextSupport: true
  },
  definition: { dynamicRegistration: true, linkSupport: true },
  references: { dynamicRegistration: true },
  documentHighlight: { dynamicRegistration: true },
  documentSymbol: {
    dynamicRegistration: true,
    symbolKind: { valueSet: [Array] },
    hierarchicalDocumentSymbolSupport: true,
    tagSupport: { valueSet: [Array] },
    labelSupport: true
  },
  codeAction: {
    dynamicRegistration: true,
    isPreferredSupport: true,
    disabledSupport: true,
    dataSupport: true,
    resolveSupport: { properties: [Array] },
    codeActionLiteralSupport: { codeActionKind: [Object] },
    honorsChangeAnnotations: false
  },
  codeLens: { dynamicRegistration: true },
  formatting: { dynamicRegistration: true },
  rangeFormatting: { dynamicRegistration: true },
  onTypeFormatting: { dynamicRegistration: true },
  rename: {
    dynamicRegistration: true,
    prepareSupport: true,
    prepareSupportDefaultBehavior: 1,
    honorsChangeAnnotations: true
  },
  documentLink: { dynamicRegistration: true, tooltipSupport: true },
  typeDefinition: { dynamicRegistration: true, linkSupport: true },
  implementation: { dynamicRegistration: true, linkSupport: true },
  colorProvider: { dynamicRegistration: true },
  foldingRange: {
    dynamicRegistration: true,
    rangeLimit: 5000,
    lineFoldingOnly: true
  },
  declaration: { dynamicRegistration: true, linkSupport: true },
  selectionRange: { dynamicRegistration: true },
  callHierarchy: { dynamicRegistration: true },
  semanticTokens: {
    dynamicRegistration: true,
    tokenTypes: [
      'namespace',     'type',
      'class',         'enum',
      'interface',     'struct',
      'typeParameter', 'parameter',
      'variable',      'property',
      'enumMember',    'event',
      'function',      'method',
      'macro',         'keyword',
      'modifier',      'comment',
      'string',        'number',
      'regexp',        'operator'
    ],
    tokenModifiers: [
      'declaration',
      'definition',
      'readonly',
      'static',
      'deprecated',
      'abstract',
      'async',
      'modification',
      'documentation',
      'defaultLibrary'
    ],
    formats: [ 'relative' ],
    requests: { range: true, full: [Object] },
    multilineTokenSupport: false,
    overlappingTokenSupport: false
  },
  linkedEditingRange: { dynamicRegistration: true }
}
dbaeumer commented 3 years ago

Semantic tokens moved out of the proposed feature space when we shipped 3.16 (e.g. LSP client 7.0.0).

Since things are fine for @rcjsuen can you (@KenDJohnson) please provide us with a test case (GitHub repro to clone) that demos what you are seeing.

KenDJohnson commented 3 years ago

Since things are fine for @rcjsuen can you (@KenDJohnson) please provide us with a test case (GitHub repro to clone) that demos what you are seeing.

Yeah I can put something together. The current code for this is not public, so I can't point you to that. I'm out at the moment, but I should be able to get a minimal extension with a boilerplate language server that shows the issue I'm running into.

Semantic tokens moved out of the proposed feature space when we shipped 3.16 (e.g. LSP client 7.0.0). I was confused by this as well. I actually wasn't trying to use the semantic tokens, I was trying to get the call tree stuff working. Without the registerProposedFeatures call the incoming/outgoing calls menu option did not show up in the UI. Once I registered those it did show up, but I got the errors from the semantic tokens fields missing. The call tree should have been moved out of proposed as well, so this could have been PEBCAK; I've only used TS/npm a couple of times, so perhaps when I updated the version in project.json, I didn't wind up pulling in the 7.x version.

KenDJohnson commented 3 years ago

I just cleared things out and built from scratch, no issues with the 7.0 release. Looks like that was just with ~6.1