Skip to content

Commit 3b703ab

Browse files
committed
rope_autoimport: continue on name-defined diagnostics provided by mypy
Previously the autoimport code action did only act on diagnostics which message contained "undefined name". In my case this did not trigger at all so that the plugin failed to provide code actions. This change now additionally looks at name-defined error codes provided by mypy.
1 parent 89265cd commit 3b703ab

File tree

2 files changed

+95
-1
lines changed

2 files changed

+95
-1
lines changed

pylsp/plugins/rope_autoimport.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,12 @@ def pylsp_code_actions(
337337
log.debug(f"textDocument/codeAction: {document} {range} {context}")
338338
code_actions = []
339339
for diagnostic in context.get("diagnostics", []):
340-
if "undefined name" not in diagnostic.get("message", "").lower():
340+
# Provided by flake8 and ruff (F821)
341+
matched_message = "undefined name" in diagnostic.get("message", "").lower()
342+
# Provided by mypy
343+
matched_code = diagnostic.get("code", "") == "name-defined"
344+
345+
if not matched_message and not matched_code:
341346
continue
342347

343348
word = get_name_or_module(document, diagnostic)

test/plugins/test_autoimport.py

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,3 +346,92 @@ def test_autoimport_code_actions_and_completions_for_notebook_document(
346346
context = {"diagnostics": [{"message": "A random message"}]}
347347
quick_fixes = server.code_actions("cell_4_uri", {}, context)
348348
assert len(quick_fixes) == 0
349+
350+
351+
def make_mypy_context(module_name, line, character_start, character_end):
352+
return {
353+
"diagnostics": [
354+
{
355+
'range': {
356+
'start': {'line': line, 'character': character_start},
357+
'end': {'line': line, 'character': character_end}
358+
},
359+
'code': 'name-defined'
360+
}
361+
]
362+
}
363+
364+
365+
@pytest.mark.skipif(IS_WIN, reason="Flaky on Windows")
366+
def test_autoimport_code_actions_and_completions_based_on_mypy_error(
367+
client_server_pair,
368+
) -> None:
369+
client, server = client_server_pair
370+
send_initialize_request(
371+
client,
372+
{
373+
"pylsp": {
374+
"plugins": {
375+
"rope_autoimport": {
376+
"memory": True,
377+
"enabled": True,
378+
"completions": {"enabled": True},
379+
},
380+
}
381+
}
382+
},
383+
)
384+
with patch.object(server._endpoint, "notify") as mock_notify:
385+
# Expectations:
386+
# 1. We receive an autoimport suggestion for "os" in the first cell because
387+
# os is imported after that.
388+
# 2. We don't receive an autoimport suggestion for "os" in the second cell because it's
389+
# already imported in the second cell.
390+
# 3. We don't receive an autoimport suggestion for "os" in the third cell because it's
391+
# already imported in the second cell.
392+
# 4. We receive an autoimport suggestion for "sys" because it's not already imported.
393+
# 5. If diagnostics doesn't contain "undefined name ...", we send empty quick fix suggestions.
394+
send_notebook_did_open(client, ["os", "import os\nos", "os", "sys"])
395+
wait_for_condition(lambda: mock_notify.call_count >= 4)
396+
# We received diagnostics messages for every cell
397+
assert all(
398+
"textDocument/publishDiagnostics" in c.args
399+
for c in mock_notify.call_args_list
400+
)
401+
402+
rope_autoimport_settings = server.workspace._config.plugin_settings(
403+
"rope_autoimport"
404+
)
405+
assert rope_autoimport_settings.get("completions", {}).get("enabled", False) is True
406+
assert rope_autoimport_settings.get("memory", False) is True
407+
wait_for_condition(lambda: not cache.is_blocked())
408+
409+
# 1.
410+
quick_fixes = server.code_actions("cell_1_uri", {}, make_mypy_context("os", 0, 0, 2))
411+
assert any(s for s in quick_fixes if contains_autoimport_quickfix(s, "os"))
412+
413+
completions = server.completions("cell_1_uri", position(0, 2)).get("items")
414+
assert any(s for s in completions if contains_autoimport_completion(s, "os"))
415+
416+
# 2.
417+
# We don't test code actions here as in this case, there would be no code actions sent bc
418+
# there wouldn't be a diagnostics message.
419+
completions = server.completions("cell_2_uri", position(1, 2)).get("items")
420+
assert not any(s for s in completions if contains_autoimport_completion(s, "os"))
421+
422+
# 3.
423+
# Same as in 2.
424+
completions = server.completions("cell_3_uri", position(0, 2)).get("items")
425+
assert not any(s for s in completions if contains_autoimport_completion(s, "os"))
426+
427+
# 4.
428+
quick_fixes = server.code_actions("cell_4_uri", {}, make_context("sys", 0, 0, 3))
429+
assert any(s for s in quick_fixes if contains_autoimport_quickfix(s, "sys"))
430+
431+
completions = server.completions("cell_4_uri", position(0, 3)).get("items")
432+
assert any(s for s in completions if contains_autoimport_completion(s, "sys"))
433+
434+
# 5.
435+
context = {"diagnostics": [{"code": "some-code"}]}
436+
quick_fixes = server.code_actions("cell_4_uri", {}, context)
437+
assert len(quick_fixes) == 0

0 commit comments

Comments
 (0)