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 ruff shows outdated syntax errors #4547

Closed
3 tasks done
vikigenius opened this issue Sep 16, 2024 · 27 comments · Fixed by astral-sh/ruff#13407
Closed
3 tasks done

LSP ruff shows outdated syntax errors #4547

vikigenius opened this issue Sep 16, 2024 · 27 comments · Fixed by astral-sh/ruff#13407
Labels

Comments

@vikigenius
Copy link

Thank you for the bug report

  • I am using the latest version of lsp-mode related packages.
  • I checked FAQ and Troubleshooting sections
  • You may also try reproduce the issue using clean environment using the following command: M-x lsp-start-plain

Bug description

It seems like ruff-lsp has the following variable: astral-sh/ruff-lsp#454 that does not seem to exist in lsp-mode for ruff.

ruff-lsp now shows outdated error messages even on saving
scrot_20240916_154135

None of these errors exist anymore and they go away on revert-buffer which presumably restarts the server.

Steps to reproduce

Use ruff-lsp version >=0.0.54

Enable both pyright and ruff-lsp. Type something and save. Old syntax errors still show up from ruff-lsp. They go away after revert-buffer

Expected behavior

ruff-lsp should only show updated/current errors

Which Language Server did you use?

ruff-lsp

OS

Linux

Error callstack

No response

Anything else?

No response

@jcs090218
Copy link
Member

ruff-lsp is removed and replaced with the built-in ruff. See #4543. Would this be an issue you were referring to? 🤔

@vikigenius
Copy link
Author

@jcs090218 I reverted to a commit before the PR and that fixes the issue. I think the issue is probably with how lsp-mode handles ruff-server or maybe with the server itself.

@tsoernes
Copy link

tsoernes commented Sep 17, 2024

I have this problem also. This was introduced in #4543. I get errors from ruff server that does not occur when running ruff check on the file directly. As you can see in the screenshot below, lsp-mode with ruff server reports errors that are not there.

image

@vikigenius
Copy link
Author

@jcs090218, according to @MichaReiser from ruff, the ruff server uses pull diagnostics when available. Maybe that's the cause of these errors?

In the PR for this change #4543, I don't see anything about pull diagnostics. Does emacs-lsp support them, is it pulling the new diagnostics ?

@kassick
Copy link
Contributor

kassick commented Sep 17, 2024

Does emacs-lsp support them

Apparently, yes, maybe partally? 7e342a5

@jcs090218
Copy link
Member

Can you try to enable lsp-diagnostic-clean-after-change? This is the closest variable I can find; see https://emacs-lsp.github.io/lsp-mode/page/settings/diagnostics/#lsp-diagnostic-clean-after-change. 🤔

@vikigenius
Copy link
Author

@jcs090218 I tried setting the variable it doesn't solve the issue.

Here are the IO logs it seems like after the file was saved no request was made for diagnostics so the last server response still seems to be indicating syntaxerrors.

ruff-io-log.txt

@MichaReiser
Copy link

Thanks for the logs. They're very useful to understand the problem.

I scrolled through the logs and the diagnostics do get updated (they change after a didChange request)

[Trace - 09:12:49 PM] Sending request 'textDocument/diagnostic - (21)'.
Params: {
  "textDocument": {
    "uri": "file:///home/void/Projects/Python/ptspace/src/ptspace/check_ts.py"
  }
}


[Trace - 09:12:49 PM] Received response 'textDocument/diagnostic - (21)' in 4ms.
Result: {
  "items": [
    {
      "code": "F821",
      "codeDescription": {
        "href": "https://docs.astral.sh/ruff/rules/undefined-name"
      },
      "data": {
        "code": "F821",
        "edits": [],
        "kind": {
          "body": "Undefined name `ravym`",
          "name": "UndefinedName",
          "suggestion": null
        },
        "noqa_edit": {
          "newText": "  # noqa: F821\n",
          "range": {
            "end": {
              "character": 9,
              "line": 21
            },
            "start": {
              "character": 5,
              "line": 21
            }
          }
        }
      },
      "message": "Undefined name `ravym`",
      "range": {
        "end": {
          "character": 5,
          "line": 21
        },
        "start": {
          "character": 0,
          "line": 21
        }
      },
      "severity": 1,
      "source": "Ruff"
    }
  ],
  "kind": "full"
}


[Trace - 09:12:49 PM] Sending notification 'textDocument/didChange'.
Params: {
  "textDocument": {
    "uri": "file:///home/void/Projects/Python/ptspace/src/ptspace/check_ts.py",
    "version": 8
  },
  "contentChanges": [
    {
      "range": {
        "start": {
          "line": 20,
          "character": 9
        },
        "end": {
          "line": 20,
          "character": 9
        }
      },
      "rangeLength": 0,
      "text": " "
    }
  ]
}


[Trace - 09:12:49 PM] Sending request 'textDocument/diagnostic - (22)'.
Params: {
  "textDocument": {
    "uri": "file:///home/void/Projects/Python/ptspace/src/ptspace/check_ts.py"
  }
}


[Trace - 09:12:49 PM] Received response 'textDocument/diagnostic - (22)' in 9ms.
Result: {
  "items": [
    {
      "message": "SyntaxError: unindent does not match any outer indentation level",
      "range": {
        "end": {
          "character": 1,
          "line": 21
        },
        "start": {
          "character": 0,
          "line": 21
        }
      },
      "severity": 1,
      "source": "Ruff"
    },
    {
      "message": "SyntaxError: Expected dedent, found end of file",
      "range": {
        "end": {
          "character": 10,
          "line": 21
        },
        "start": {
          "character": 10,
          "line": 21
        }
      },
      "severity": 1,
      "source": "Ruff"
    }
  ],
  "kind": "full"
}


[Trace - 09:12:49 PM] Sending notification 'textDocument/didChange'.
Params: {
  "textDocument": {
    "uri": "file:///home/void/Projects/Python/ptspace/src/ptspace/check_ts.py",
    "version": 9
  },
  "contentChanges": [
    {
      "range": {
        "start": {
          "line": 20,
          "character": 10
        },
        "end": {
          "line": 20,
          "character": 10
        }
      },
      "rangeLength": 0,
      "text": "="
    }
  ]
}

What's difficult for me to say is if those are the expected responses because I don't know changes you made to the document. Can you try a shorter test case. For example:

Type:

a = 

and then change it to

a = 10

ideally in an existing, but otherwise empty file. This should be the most basic case where the editor transitions from code without syntax error to code with sytnax error to code without syntax error.

Could you try setting the logLevel and (depending on where emacs-lsp writes the logs to) [logFile](https://docs.astral.sh/ruff/editors/settings/#logfile) settings. I wonder if there's anything happening on the server that explains the outdated diagnostics.

@kassick
Copy link
Contributor

kassick commented Sep 18, 2024

lsp trace + ruff server --verbose stderr + video of the interaction

ruff-lsp-trace.txt
ruff-stderr.txt

Gravacao.de.tela.de.2024-09-18.11-29-55.mp4

@kassick
Copy link
Contributor

kassick commented Sep 18, 2024

Another workaround: using the latest version of lsp-mode, use the ruff-lsp server instead of ruff server:

pip install -U ruff-lsp ruff

(setq lsp-ruff-server-command '("ruff-lsp"))

@vikigenius
Copy link
Author

@MichaReiser I could not reproduce stale diagnostics with the a = 10 example

But I was able to do it with

mystr = 'hello'

Here are the corresponding traces and logs
ruff-io-log.txt
ruff-server-logs.txt

@MichaReiser
Copy link

MichaReiser commented Sep 18, 2024

I have to do some debugging. Hopefully I find some time tomorrow. I wonder if our UTF32 support is incorrect because the syntax errors aren't really outdated. They make no sense. Like why is there an indent error?

In case anyone knows how to do that in lsp-emacs, could you try changing the encoding to UTF-16 (not the file's encoding but the advertised encoding by the client)

@vikigenius
Copy link
Author

Maybe this issue is related #2080 ?

@vikigenius
Copy link
Author

It seems like utf-16 support is buggy (multibyte) which is why emacs tries to use utf-32 or utf-8 if i understand the issue correctly? @MichaReiser does the server not support those, what's the difference between it and ruff-lsp which seems to handle it just fine?

@kassick
Copy link
Contributor

kassick commented Sep 18, 2024

@MichaReiser I've changed to UTF-16 by forcing it to be the only available option in lsp--client-capabilities. It appears to work much better, indeed!

(defun lsp--client-capabilities (&optional custom-capabilities)
  "Return the client capabilities appending CUSTOM-CAPABILITIES."
  (append
   `((general . ((positionEncodings . ["utf-16"])))
   ;;; rest of function definition here

(though, of course, this is a temporary kludge, not a workaround, as this means I've disabled utf-32 support in for all other servers too ...)

@kassick
Copy link
Contributor

kassick commented Sep 18, 2024

I'll be running rust server with UTF-16 and update here tomorrow if it seems to work OK.
It anyone wants to do the same (force only ruff to use utf-16), here's a init.el kludge

@vikigenius
Copy link
Author

@kassick are you familiar with the multi-byte issue I linked earlier #2080

Maybe the correct solution is to get ruff server to support UTF-32 correctly. But until then the workaround of using UTF-16 is fine I think, unless someone wants to put Emojis in their code 😃

@kassick
Copy link
Contributor

kassick commented Sep 19, 2024

are you familiar with the multi-byte issue I linked earlier #2080

Yes, I am and I can confirm that it happens when using emoji in the code ;)

Maybe the correct solution is to get ruff server to support UTF-32 correctly.

Certainly! The workaround is to make sure that the issue is isolated do utf-32 handling, it's not as a possible solution ;)

unless someone wants to put Emojis in their code 😃

Yeah, that's me ... :P

But restarting the server usually fixes it in the rare occasions I need to change a line with emoji

@MichaReiser
Copy link

@MichaReiser I've changed to UTF-16 by forcing it to be the only available option in lsp--client-capabilities. It appears to work much better, indeed!

(defun lsp--client-capabilities (&optional custom-capabilities)
  "Return the client capabilities appending CUSTOM-CAPABILITIES."
  (append
   `((general . ((positionEncodings . ["utf-16"])))
   ;;; rest of function definition here

That's great news. So we know what the culprit is.

(though, of course, this is a temporary kludge, not a workaround, as this means I've disabled utf-32 support in for all other servers too ...)

Definitely. My suggestion wasn't to use this long term. But it's a great finding for narrowing the root cause. Let me setup lsp-mode and try to get this fixed.

@MichaReiser
Copy link

Any recommendations and setups that I could use? I've never used emacs before...

@MichaReiser
Copy link

Okay, I have it almost working. I'm failing to figure out how to set a custom path for the ruff executable

@MichaReiser
Copy link

I have it working. Now lets fix the bug

@MichaReiser
Copy link

Okay, this is a very silly bug. I have a fix up soon

@MichaReiser
Copy link

Considering that this is a ruff bug, I recommend closing the issue here and following astral-sh/ruff-lsp#495

@kassick
Copy link
Contributor

kassick commented Sep 19, 2024

Thank you @MichaReiser !

@MichaReiser
Copy link

You're welcome. Thanks for helping me reproduce the problem.

We released a new Ruff version yesterday. Could you give it a try?

@vikigenius
Copy link
Author

The new version seems to have fixed the issue. Thanks. Closing.

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

Successfully merging a pull request may close this issue.

5 participants