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

Plugin returns wrong line numbers for some diagnostics #97

Open
freebird-2 opened this issue Dec 8, 2024 · 0 comments · May be fixed by #98
Open

Plugin returns wrong line numbers for some diagnostics #97

freebird-2 opened this issue Dec 8, 2024 · 0 comments · May be fixed by #98

Comments

@freebird-2
Copy link

freebird-2 commented Dec 8, 2024

Summary: If an LSP client sends text to the pylsp server with Windows-style line endings (\r\n), the pylsp-mypy plugin will report the wrong line numbers for some diagnostics.

Description:
The error occurs under the following conditions:

  • The LSP client sends text to the server with Windows-style line endings (\r\n).
  • The plugin has live mode enabled.
  • A change is made to the document, and the document is not saved afterwards.

When the last two conditions are met, the plugin writes the document text to a temp file, from which mypy reads the text and performs its analysis. The problem arises when Python writes the text to the temp file: by default, Python translates all instances of the newline character (\n) to the value of os.linesep when writing to a file. If the LSP server is running on Windows, os.linesep is equal to \r\n, so the line ending \r\n is translated to \r\r\n. Later, when mypy reads the file, it interprets the character sequence \r\r\n as two separate line endings; in effect, mypy is receiving a file with an extra line inserted after every line in the original text. mypy thus reports the wrong line numbers, which are propogated back to the client.

The Langauge Server Protocol specifies that \r, \n, and \r\n are all valid line-ending sequences, so pylsp-mypy should be able to handle any of them.

Reproduction steps:

  1. You will need an LSP client that sends line endings as \r\n instead of \n. Neovim v0.9.5 for Windows does this.
  2. Install python-lsp-server latest (v1.12.0) and pylsp-mypy latest (v0.6.9).
  3. Configure pylsp-mypy to enable live mode.
  4. Run pylsp with the flag -vv for verbose logging.
  5. Open a new Python source file. Save the file. Enter the following into the file but do not save it again.
pass
a
pass
  1. If your editor shows the resulting diagnostic, you will see it appears on line 3 instead of line 2 of the source file. You can also look at the language server's logs to verify that pylsp-mypy is in fact reporting the wrong line numbers to the language server, which then propogates those line numbers to the client. Below is a screenshot from Neovim showing the diagnostic on the wrong line.
    img
    I've attached a log file for this example; see lines 144-151 for the text being passed to the server, and then lines 156, 158, 160, 161 for the response diagnostic with the incorrect line number.
    logs.txt

Proposed fix:
The plugin should write the text to the temporary file in binary mode. This will pass on the text to mypy exactly how it was sent by the client. mypy already reads the content of the file in binary mode, so there will be no line-ending translation at any point in the flow.

Tests should also be added to verify that the plugin can handle any of the valid line endings correctly.

@freebird-2 freebird-2 linked a pull request Dec 8, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

1 participant