Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LSP messaging does not properly handle trailing .0 on integer values #2420

Closed
IsaacShelton opened this issue Feb 13, 2024 · 7 comments
Closed

Comments

@IsaacShelton
Copy link

Describe the bug

LSP messaging does not properly handle trailing .0 on integer values as per JSON spec.

To Reproduce

Send a server initialization response such as:

{
  "jsonrpc": "2.0",
  "id": 1.0,
  "result": {
    "capabilities": {
      "hoverProvider": true,
      "definitionProvider": true,
      "textDocumentSync": {
        "openClose": true,
        "change": 1.0
      }
    },
    "serverInfo": {
      "name": "adeptls",
      "version": "2.8"
    }
  }
}

Expected behavior

For the above response message, textDocument/didChange should be set to TextDocumentSyncKind.Full, but instead is ignored because 1.0 is not recognized as an integer.

Logs

Message sent by server:

[sending] {"jsonrpc":"2.0","id":1.0,"result":{"capabilities":{"hoverProvider":true,"definitionProvider":true,"textDocumentSync":{"openClose":true,"change":1.0},"completionProvider":{"triggerCharacters":["\\"]}},"serverInfo":{"name":"adeptls","version":"2.8"}}}

Client Log:

:: [10:17:06.975] --> adeptls initialize (1): {'initializationOptions': {}, 'processId': 72053, 'capabilities': {'textDocument': {'synchronization': {'didSave': True, 'willSave': True, 'dynamicRegistration': True, 'willSaveWaitUntil': True}, 'documentLink': {'dynamicRegistration': True, 'tooltipSupport': True}, 'documentHighlight': {'dynamicRegistration': True}, 'hover': {'contentFormat': ['markdown', 'plaintext'], 'dynamicRegistration': True}, 'diagnostic': {'relatedDocumentSupport': True, 'dynamicRegistration': True}, 'documentSymbol': {'tagSupport': {'valueSet': [1]}, 'symbolKind': {'valueSet': [10, 12, 5, 3, 13, 15, 17, 11, 4, 20, 7, 21, 8, 6, 18, 25, 24, 2, 16, 22, 23, 26, 9, 14, 19, 1]}, 'dynamicRegistration': True, 'hierarchicalDocumentSymbolSupport': True}, 'rename': {'prepareSupportDefaultBehavior': 1, 'dynamicRegistration': True, 'prepareSupport': True}, 'selectionRange': {'dynamicRegistration': True}, 'semanticTokens': {'dynamicRegistration': True, 'tokenModifiers': ['async', 'declaration', 'documentation', 'deprecated', 'static', 'abstract', 'definition', 'modification', 'defaultLibrary', 'readonly'], 'requests': {'full': {'delta': True}, 'range': True}, 'augmentsSyntaxTokens': True, 'multilineTokenSupport': True, 'formats': ['relative'], 'overlappingTokenSupport': False, 'tokenTypes': ['enum', 'function', 'macro', 'event', 'namespace', 'variable', 'comment', 'class', 'interface', 'struct', 'modifier', 'string', 'method', 'operator', 'property', 'number', 'enumMember', 'typeParameter', 'type', 'parameter', 'keyword', 'regexp', 'decorator']}, 'implementation': {'dynamicRegistration': True, 'linkSupport': True}, 'foldingRange': {'foldingRangeKind': {'valueSet': ['comment', 'imports', 'region']}, 'dynamicRegistration': True}, 'typeDefinition': {'dynamicRegistration': True, 'linkSupport': True}, 'completion': {'completionItemKind': {'valueSet': [13, 3, 7, 16, 6, 18, 8, 10, 5, 2, 24, 23, 9, 22, 17, 20, 25, 4, 11, 21, 19, 1, 15, 14, 12]}, 'completionItem': {'insertTextModeSupport': {'valueSet': [2]}, 'documentationFormat': ['markdown', 'plaintext'], 'tagSupport': {'valueSet': [1]}, 'snippetSupport': True, 'labelDetailsSupport': True, 'resolveSupport': {'properties': ['detail', 'documentation', 'additionalTextEdits']}, 'deprecatedSupport': True, 'insertReplaceSupport': True}, 'completionList': {'itemDefaults': ['editRange', 'insertTextFormat', 'data']}, 'dynamicRegistration': True, 'insertTextMode': 2}, 'colorProvider': {'dynamicRegistration': True}, 'callHierarchy': {'dynamicRegistration': True}, 'publishDiagnostics': {'tagSupport': {'valueSet': [2, 1]}, 'versionSupport': True, 'relatedInformation': True, 'dataSupport': True, 'codeDescriptionSupport': True}, 'declaration': {'dynamicRegistration': True, 'linkSupport': True}, 'codeLens': {'dynamicRegistration': True}, 'references': {'dynamicRegistration': True}, 'codeAction': {'codeActionLiteralSupport': {'codeActionKind': {'valueSet': ['quickfix', 'refactor', 'refactor.extract', 'refactor.inline', 'refactor.rewrite', 'source.fixAll', 'source.organizeImports']}}, 'resolveSupport': {'properties': ['edit']}, 'dynamicRegistration': True, 'dataSupport': True, 'isPreferredSupport': True}, 'typeHierarchy': {'dynamicRegistration': True}, 'formatting': {'dynamicRegistration': True}, 'signatureHelp': {'signatureInformation': {'documentationFormat': ['markdown', 'plaintext'], 'activeParameterSupport': True, 'parameterInformation': {'labelOffsetSupport': True}}, 'dynamicRegistration': True, 'contextSupport': True}, 'definition': {'dynamicRegistration': True, 'linkSupport': True}, 'inlayHint': {'resolveSupport': {'properties': ['textEdits', 'label.command']}, 'dynamicRegistration': True}, 'rangeFormatting': {'dynamicRegistration': True, 'rangesSupport': True}}, 'general': {'markdown': {'parser': 'Python-Markdown', 'version': '3.2.2'}, 'regularExpressions': {'engine': 'ECMAScript'}}, 'workspace': {'diagnostics': {'refreshSupport': True}, 'executeCommand': {}, 'applyEdit': True, 'workspaceEdit': {'failureHandling': 'abort', 'documentChanges': True}, 'configuration': True, 'symbol': {'resolveSupport': {'properties': ['location.range']}, 'symbolKind': {'valueSet': [10, 12, 5, 3, 13, 15, 17, 11, 4, 20, 7, 21, 8, 6, 18, 25, 24, 2, 16, 22, 23, 26, 9, 14, 19, 1]}, 'dynamicRegistration': True, 'tagSupport': {'valueSet': [1]}}, 'didChangeConfiguration': {'dynamicRegistration': True}, 'inlayHint': {'refreshSupport': True}, 'codeLens': {'refreshSupport': True}, 'workspaceFolders': True, 'semanticTokens': {'refreshSupport': True}}, 'window': {'showMessage': {'messageActionItem': {'additionalPropertiesSupport': True}}, 'workDoneProgress': True, 'showDocument': {'support': True}}}, 'rootPath': None, 'clientInfo': {'version': '1.28.0', 'name': 'Sublime Text LSP'}, 'rootUri': None, 'workspaceFolders': None}
:: [10:17:07.127] <<< adeptls (1) (duration: 151ms): {'serverInfo': {'version': '2.8', 'name': 'adeptls'}, 'capabilities': {'textDocumentSync': {'didOpen': {}, 'didClose': {}}, 'definitionProvider': True, 'hoverProvider': True, 'completionProvider': {'triggerCharacters': ['\\']}}}
:: [10:17:07.127]  -> adeptls initialized: {}
:: [10:17:07.129]  -> adeptls textDocument/didOpen: <params with 12090 characters>
:: [10:17:07.182] <-  adeptls textDocument/publishDiagnostics: {'version': 0.0, 'diagnostics': [], 'uri': 'file:///Users/isaac/AdeptProjects/AdeptLSP/src/main.adept'}
@rchl
Copy link
Member

rchl commented Feb 13, 2024

JSONRPC spec says:

id
An identifier established by the Client that MUST contain a String, Number, or NULL value if included. If it is not included it is assumed to be a notification. The value SHOULD normally not be Null [1] and Numbers SHOULD NOT contain fractional parts [2]

It doesn't say "MUST NOT" but still.

@IsaacShelton
Copy link
Author

It's not only with the id of the response, it's also that a value of 1.0 for textDocumentSync.change is not recognized.

@predragnikolic
Copy link
Member

The LSP spec says https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocumentSyncKind

The LSP spec doesn't have fractional parts for textDocumentSyncKind

@IsaacShelton
Copy link
Author

The LSP spec says nothing about fractional parts, the messaging is in JSON, and by the JSON spec, 1.0 is the same as 1.

Also, the LSP spec defines integers as JavaScript to be number

Do what you like though

@rchl
Copy link
Member

rchl commented Feb 13, 2024

The place you've linked defines integer, uinteger and decimal explicitly. All are mapped to number, I guess due to it being based on typescript typing system but that doesn't mean that decimal values are allowed in place of integer and uinteger. And for sure not in place of enum values which might technically be numbers under the hood in typescript case but spec doesn't specify that.

@IsaacShelton
Copy link
Author

They are specified as JSON, and JSON does not have the concept of integers, only numbers

In JSON, there is no difference between 1 and 1.0.

The entire protocol is in JSON. What is there to argue here?

I'm not going to debate with you guys anymore on this bug as it seems like you guys are not open to admitting fault in your software nor fixing it

@rchl
Copy link
Member

rchl commented Feb 13, 2024

The LSP specification can impose additional restrictions on top of JSON specification and as far I understand this is what is happening here. You can ask at LSP specification repo if you think otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants