Running Formatting + Code Actions on save generates duplicate imports

[senpos]

Hello!

I think, there must be some race condition when running formatting and fix all + organize imports code actions on save.

I've tried LSP-ruff before and did not notice such issue, so decided to create an issue.

Consider the following code (app.py):

import typing
import msgspec

class Item(msgspec.Struct):
    name: str

item = Item('hi')
another_item = item
another_item.name = 'banana'
print(item)

Here's my LSP.sublime-settings:

// Settings in here override those in "LSP/LSP.sublime-settings"
{
	"lsp_code_actions_on_save": {
		"source.fixAll.ruff": true,
		"source.organizeImports.ruff": true
	},
	"lsp_format_on_save": true
}

I did not change LSP-ruff.sublime-settings:

// Settings in here override those in "LSP-ruff/LSP-ruff.sublime-settings"
{
	"enabled": true,
}

For the sake of this test, I disabled LSP-pyright globally and restarted my editor.

When I hit Ctrl+S, this is the code I get:

import msgspec

import msgspec


class Item(msgspec.Struct):
    name: str


item = Item("hi")
another_item = item
another_item.name = "banana"
print(item)

Notice the duplicate import of msgspec.

I tried running lint + fix + format from CLI:

(playground) PS C:\Users\Senpos\workspace\playground> ruff check --fix .\app.py
Found 1 error (1 fixed, 0 remaining).
(playground) PS C:\Users\Senpos\workspace\playground> ruff format .\app.py
1 file reformatted

And the result seems to be fine:

import msgspec


class Item(msgspec.Struct):
    name: str


item = Item("hi")
another_item = item
another_item.name = "banana"
print(item)

Here's the LSP log:

:: [12:25:21.134] --> LSP-ruff textDocument/codeAction (65): {'textDocument': {'uri': 'file:///C:/Users/Senpos/workspace/playground/app.py'}, 'range': {'start': {'line': 0, 'character': 0}, 'end': {'line': 9, 'character': 11}}, 'context': {'diagnostics': [{'code': 'F401', 'codeDescription': {'href': 'https://docs.astral.sh/ruff/rules/unused-import'}, 'data': {'code': 'F401', 'edits': [{'newText': '', 'range': {'end': {'character': 0, 'line': 1}, 'start': {'character': 0, 'line': 0}}}], 'kind': {'body': '`typing` imported but unused', 'name': 'UnusedImport', 'suggestion': 'Remove unused import: `typing`'}, 'noqa_edit': {'newText': '  # noqa: F401\n', 'range': {'end': {'character': 0, 'line': 1}, 'start': {'character': 13, 'line': 0}}}}, 'message': '`typing` imported but unused', 'range': {'end': {'character': 13, 'line': 0}, 'start': {'character': 7, 'line': 0}}, 'severity': 2, 'source': 'Ruff', 'tags': [1]}], 'triggerKind': 2, 'only': ['source.fixAll.ruff', 'source.organizeImports.ruff']}}
:: [12:25:21.136] <<< LSP-ruff (65) (duration: 1ms): [{'data': 'file:///C:/Users/Senpos/workspace/playground/app.py', 'kind': 'source.fixAll.ruff', 'title': 'Ruff: Fix all auto-fixable problems'}, {'data': 'file:///C:/Users/Senpos/workspace/playground/app.py', 'kind': 'source.organizeImports.ruff', 'title': 'Ruff: Organize imports'}]
:: [12:25:21.145] --> LSP-ruff codeAction/resolve (66): {'data': 'file:///C:/Users/Senpos/workspace/playground/app.py', 'kind': 'source.fixAll.ruff', 'title': 'Ruff: Fix all auto-fixable problems'}
:: [12:25:21.145] --> LSP-ruff codeAction/resolve (67): {'data': 'file:///C:/Users/Senpos/workspace/playground/app.py', 'kind': 'source.organizeImports.ruff', 'title': 'Ruff: Organize imports'}
:: [12:25:21.150] <<< LSP-ruff (66) (duration: 5ms): {'data': 'file:///C:/Users/Senpos/workspace/playground/app.py', 'edit': {'documentChanges': [{'edits': [{'newText': '', 'range': {'end': {'character': 0, 'line': 1}, 'start': {'character': 0, 'line': 0}}}], 'textDocument': {'uri': 'file:///C:/Users/Senpos/workspace/playground/app.py', 'version': None}}]}, 'kind': 'source.fixAll.ruff', 'title': 'Ruff: Fix all auto-fixable problems'}
:: [12:25:21.162] <<< LSP-ruff (67) (duration: 16ms): {'data': 'file:///C:/Users/Senpos/workspace/playground/app.py', 'edit': {'documentChanges': [{'edits': [{'newText': '\nimport msgspec\n\n', 'range': {'end': {'character': 0, 'line': 2}, 'start': {'character': 0, 'line': 1}}}], 'textDocument': {'uri': 'file:///C:/Users/Senpos/workspace/playground/app.py', 'version': None}}]}, 'kind': 'source.organizeImports.ruff', 'title': 'Ruff: Organize imports'}
:: [12:25:21.207]  -> LSP-ruff textDocument/didChange: {'textDocument': {'uri': 'file:///C:/Users/Senpos/workspace/playground/app.py', 'version': 32}, 'contentChanges': [{'range': {'start': {'line': 0, 'character': 0}, 'end': {'line': 1, 'character': 0}}, 'rangeLength': 14, 'text': ''}, {'range': {'start': {'line': 1, 'character': 0}, 'end': {'line': 2, 'character': 0}}, 'rangeLength': 1, 'text': '\nimport msgspec\n\n'}]}
:: [12:25:21.214] --> LSP-ruff textDocument/formatting (68): {'textDocument': {'uri': 'file:///C:/Users/Senpos/workspace/playground/app.py'}, 'options': {'tabSize': 4, 'insertSpaces': True, 'trimTrailingWhitespace': False, 'insertFinalNewline': False, 'trimFinalNewlines': False}, 'workDoneToken': '$ublime-work-done-progress-68'}
:: [12:25:21.215] <<< LSP-ruff (68) (duration: 1ms): [{'newText': '\nclass Item(msgspec.Struct):\n    name: str\n\n\nitem = Item("hi")\nanother_item = item\nanother_item.name = "banana"\nprint(item)\n', 'range': {'end': {'character': 11, 'line': 10}, 'start': {'character': 0, 'line': 4}}}]
:: [12:25:21.242] --> LSP-ruff textDocument/diagnostic (69): {'textDocument': {'uri': 'file:///C:/Users/Senpos/workspace/playground/app.py'}, 'identifier': 'Ruff'}
:: [12:25:21.243] <<< LSP-ruff (69) (duration: 0ms): {'items': [{'code': 'F811', 'codeDescription': {'href': 'https://docs.astral.sh/ruff/rules/redefined-while-unused'}, 'data': {'code': 'F811', 'edits': [{'newText': '', 'range': {'end': {'character': 0, 'line': 3}, 'start': {'character': 0, 'line': 2}}}], 'kind': {'body': 'Redefinition of unused `msgspec` from line 1', 'name': 'RedefinedWhileUnused', 'suggestion': 'Remove definition: `msgspec`'}, 'noqa_edit': {'newText': '  # noqa: F811\n', 'range': {'end': {'character': 0, 'line': 3}, 'start': {'character': 14, 'line': 2}}}}, 'message': 'Redefinition of unused `msgspec` from line 1', 'range': {'end': {'character': 14, 'line': 2}, 'start': {'character': 7, 'line': 2}}, 'severity': 2, 'source': 'Ruff'}], 'kind': 'full'}
:: [12:25:21.246] --> LSP-ruff textDocument/codeAction (70): {'textDocument': {'uri': 'file:///C:/Users/Senpos/workspace/playground/app.py'}, 'range': {'start': {'line': 13, 'character': 0}, 'end': {'line': 13, 'character': 0}}, 'context': {'diagnostics': [], 'triggerKind': 2}}
:: [12:25:21.254] <<< LSP-ruff (70) (duration: 7ms): [{'data': 'file:///C:/Users/Senpos/workspace/playground/app.py', 'kind': 'source.fixAll.ruff', 'title': 'Ruff: Fix all auto-fixable problems'}, {'data': 'file:///C:/Users/Senpos/workspace/playground/app.py', 'kind': 'source.organizeImports.ruff', 'title': 'Ruff: Organize imports'}]
:: [12:25:21.538]  -> LSP-ruff textDocument/didChange: {'textDocument': {'uri': 'file:///C:/Users/Senpos/workspace/playground/app.py', 'version': 33}, 'contentChanges': [{'range': {'start': {'line': 4, 'character': 0}, 'end': {'line': 10, 'character': 11}}, 'rangeLength': 121, 'text': '\nclass Item(msgspec.Struct):\n    name: str\n\n\nitem = Item("hi")\nanother_item = item\nanother_item.name = "banana"\nprint(item)\n'}]}
:: [12:25:21.554] --> LSP-ruff textDocument/diagnostic (71): {'textDocument': {'uri': 'file:///C:/Users/Senpos/workspace/playground/app.py'}, 'identifier': 'Ruff'}
:: [12:25:21.557] <<< LSP-ruff (71) (duration: 2ms): {'items': [{'code': 'F811', 'codeDescription': {'href': 'https://docs.astral.sh/ruff/rules/redefined-while-unused'}, 'data': {'code': 'F811', 'edits': [{'newText': '', 'range': {'end': {'character': 0, 'line': 3}, 'start': {'character': 0, 'line': 2}}}], 'kind': {'body': 'Redefinition of unused `msgspec` from line 1', 'name': 'RedefinedWhileUnused', 'suggestion': 'Remove definition: `msgspec`'}, 'noqa_edit': {'newText': '  # noqa: F811\n', 'range': {'end': {'character': 0, 'line': 3}, 'start': {'character': 14, 'line': 2}}}}, 'message': 'Redefinition of unused `msgspec` from line 1', 'range': {'end': {'character': 14, 'line': 2}, 'start': {'character': 7, 'line': 2}}, 'severity': 2, 'source': 'Ruff'}], 'kind': 'full'}

Please, advise if I should create an issue in the ruff repository directly.
Thanks!

[LDAP]

@rchl Is this an ST LSP bug perhaps?

[rchl]

Server sends two code actions and we apply both of them which is causing the issue. Server doesn't specify "version" for each code action which would help avoid the issue because then only one would get applied.

But yes, I would call it an LSP issue due to how "code actions on save" work, not being in line with how VSCode behaves. I've mentioned this discrepancy in sublimelsp/LSP#2421 (comment) (second paragraph).

[LDAP]

What do you recommend? "Fixing" it in LSP or creating an issue in ruff?

[rchl]

Fixing in LSP. I believe I've mentioned this issue in at least two places so not sure now if there is some more relevant issue for tracking or whether we should create one.

As for ruff, I would say an enhancement issue could be filed to include version on code action responses. It's a nice to have thing that would prevent outdated code action being applied if the document has changed in the meantime.

[kinoute]

Still the same problem. Imports getting duplicated, imports getting removed while still being used in the code, this is driving us crazy. Any update on this?

[rchl]

Remaining issue will be fixed in the next LSP version.