microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.49k stars 29.38k forks source link

LSP Code Action Requests with wrong types are sent #177094

Open torik42 opened 1 year ago

torik42 commented 1 year ago

If VSCode somehow obtains faulty diagnostics, it also sends them – out of spec – to other LSP-servers in Code Action Requests. More specifically, it sends the position:

"start": {
    "line": null,
    "character": 0
},

To my understanding, the line has to be uinteger but not null. This results in failing LSP servers.

This specific issue might be fixed at the source of the faulty diagnostic. I have therefore created an issue in the LaTeX-Workshop extension. It turns out though, that they don’t use an LSP server, but the npm vscode module which requires the line to be of type number which allows for value NaN. Moreover, I think that VSCode should not send faulty LSP messages. For the case of diagnostics, it should either filter such diagnostics, which do not adhere to the LSP, or directly block them, when they are given to VSCode in the first place.

Does this issue occur when all extensions are disabled?: (probably) No

Steps to Reproduce:

What I describe here, is how I noticed this error. I expect, that it can also appear in a lot of other contexts.

  1. Install the LaTeX Workshop extension.
  2. Install any other LSP server which works in LaTeX documents and allows for Code Action Requests (I experience this with the still unpublished YaLafi-LS-VScode extension and will update this issue, as soon as a first version is published. The server itself is available here.)
  3. Open the following LaTeX file and build with xelatex.

    \documentclass{article}
    
    \usepackage{mathtools}
    \usepackage{unicode-math}
    
    \begin{document}
        This is some text.
    \end{document}

    This will issue two warnings with the faulty positions, they are displayed at the beginning of the file.

  4. Move around in the document. VSCode will send faulty Code Action Requests to the LSP server looking like this.
    {
        "jsonrpc": "2.0",
        "id": 10,
        "method": "textDocument/codeAction",
        "params":{
            "textDocument":{
                "uri": "file:///.../test.tex"
            },
            "range":{
                "start":{
                    "line": 7,
                    "character": 14
                },
                "end":{
                    "line": 7,
                    "character": 14
                }
            },
            "context":{
                "diagnostics":[
                    {
                        "range":{
                            "start":{
                                "line": null,
                                "character": 0
                            },
                            "end":{
                                "line": null,
                                "character": 65535
                            }
                        },
                        "message": "Using \\overbracket and \\underbracket from\n(unicode-math)\t`mathtools' package.\n(unicode-math)\t\n(unicode-math)\tUse \\Uoverbracket and \\Uunderbracket for\n(unicode-math)\toriginal `unicode-math' definition.\n",
                        "severity": 2,
                        "source": "LaTeX"
                    },
                    {
                        "range":{
                            "start":{
                                "line": null,
                                "character": 0
                            },
                            "end":{
                                "line": null,
                                "character": 65535
                            }
                        },
                        "message": "I'm going to overwrite the following commands\n(unicode-math)\tfrom the `mathtools' package: \n(unicode-math)\t\n(unicode-math)\t\\dblcolon, \\coloneqq, \\Coloneqq, \\eqqcolon.\n(unicode-math)\t\n(unicode-math)\t\n(unicode-math)\tNote that since I won't overwrite the other\n(unicode-math)\tcolon-like commands, using them will lead to\n(unicode-math)\tinconsistencies.\n",
                        "severity": 2,
                        "source": "LaTeX"
                    }
                ]
            }
        }
    }
  5. In my specific case with the server implemented using pygls I get the attached log test-pygls.log.

Edit: Added link and short comment about issue in LaTeX Workshop extension.

torik42 commented 1 year ago

Further investigation revealed that the LaTeX Workshop extension sets the line nuber to NaN for these problems. And after a quick web search, I understand that it is a valid value for typescript type number, as required by @types/vscode for Position.line. But it should not be cast to null by VSCode when sending the diagnostics to an LSP server. Maybe casting it to 0 is okay, because it is displayed in the first line anyway in VSCode.

torik42 commented 1 year ago

Interestingly, VSCode sends this “faulty” diagnostic to the LSP server for Code Action Requests in all lines, even though it is displayed only in the first line.

dbaeumer commented 1 year ago

@jrieken @mjbvz actually this sounds like a problem in VS Code itself and not LSP.

What happens is that an extension using the VS Code API directly creates a problem with line set to NaN which when the diagnostic comes back in a code action request line is set to null. I can filter them in LSP but I would argue that VS Code should not pass them to the code action request with null. Best would be if VS Code already filters them when provided. The underlying problem might be that typeof NaN === 'number'

jrieken commented 1 year ago

There is multiple issues here:

I don't believe we can change anything for 2 and 3 but anyone is invited to improve range validation