From 44d0f40b13baf840b2ea61fc0d9e390b9f72bd80 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:40:41 +0800 Subject: [PATCH] fix(completion): replace prefixes typed after cached requests (#324) Accepting an LSP completion after typing more characters than the request saw (`System.out.` requested, then `Print` typed while the cached list stayed open) inserted the item after the typed text: `System.out.Printprint()`. The cached edit range started exactly at the request caret and was mapped forward past the newly typed characters. The range start now stays at the request caret, so the typed prefix is replaced, whatever its casing. Enter (insert) and Tab (replace) keep their behaviour for the text after the caret; this applies to every language server. Fixes #316 --- crates/ide-lsp/src/convert.rs | 34 +++++++++ src/editor/lsp/completion.test.ts | 110 +++++++++++++++++++++++++++++- src/editor/lsp/completion.ts | 10 ++- 3 files changed, 149 insertions(+), 5 deletions(-) diff --git a/crates/ide-lsp/src/convert.rs b/crates/ide-lsp/src/convert.rs index 74bbb609..08032f6a 100644 --- a/crates/ide-lsp/src/convert.rs +++ b/crates/ide-lsp/src/convert.rs @@ -1176,6 +1176,40 @@ mod tests { assert!(matches!(back.text_edit, Some(lsp::CompletionTextEdit::InsertAndReplace(_)))); } + #[test] + fn jdtls_completion_defaults_preserve_prefix_range_and_edit_text() { + // JDT.LS 1.63: request after `System.out.` or `System.out.Print`. + for end in [19, 24] { + let (_, items) = parse_completion_response(json!({ + "isIncomplete": false, + "itemDefaults": { + "editRange": { + "insert": {"start": {"line":2,"character":19}, "end": {"line":2,"character":end}}, + "replace": {"start": {"line":2,"character":19}, "end": {"line":2,"character":end}} + }, + "insertTextFormat": 2, + "data": {"completionKinds": [2, 6]} + }, + "items": [{ + "label": "print", + "labelDetails": {"detail": "(Object obj)", "description": "void"}, + "kind": 2, + "detail": "PrintStream.print(Object obj) : void", + "filterText": "print(Object obj) : void", + "insertText": "print", + "textEditText": "print(${0});", + "data": {"pid": "0", "rid": "0"} + }] + })); + let item = completion_item(&Conv::utf16(), Path::new("/Main.java"), items[0].clone(), ServerId(1)); + assert_eq!(item.replace_range, Some(Range::new(p(2, 19), p(2, end)))); + assert_eq!(item.replace_range_tab, None); + assert_eq!(item.filter_text.as_deref(), Some("print(Object obj) : void")); + assert_eq!(item.insert_text, "print(${0});"); + assert!(item.is_snippet); + } + } + #[test] fn hover_normalisation() { let conv = Conv::utf16(); diff --git a/src/editor/lsp/completion.test.ts b/src/editor/lsp/completion.test.ts index 3c53bb29..83f537c3 100644 --- a/src/editor/lsp/completion.test.ts +++ b/src/editor/lsp/completion.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { EditorState, StateEffect } from "@codemirror/state"; -import { EditorView } from "@codemirror/view"; +import { EditorView, runScopeHandlers } from "@codemirror/view"; import type { CompletionItem } from "@/bindings"; import type { EditorDocContext } from "@/editor/api"; import { lspDoc } from "./lsp"; @@ -22,7 +22,13 @@ import { matchRanges, sortAlphabetically, } from "./completion"; -import type { CompletionSource } from "@codemirror/autocomplete"; +import { + acceptCompletion, + currentCompletions, + snippet, + startCompletion, + type CompletionSource, +} from "@codemirror/autocomplete"; import { useProject } from "@/platform/project"; import type { Settings } from "@/platform/ipc"; @@ -198,6 +204,106 @@ describe("applying completions", () => { expect(view.state.doc.toString()).toBe("x.create()"); }); + for (const mode of ["insert", "replace"] as const) { + for (const shape of ["textEdit", "insertReplace", "noEdit"] as const) { + for (const isSnippet of [false, true]) { + it(`${mode} replaces a mixed-case prefix typed after a ${shape} request (snippet: ${isSnippet})`, () => { + const { view, req, pos } = setup("/* 😀 */ System.out.|Suffix", "Print"); + const range = { start: { line: 0, col: pos }, end: { line: 0, col: pos } }; + const c = makeCompletion( + item({ + label: "print", + filterText: "print(Object obj) : void", + insertText: isSnippet ? "print(${0})" : "print()", + isSnippet, + replaceRange: shape === "noEdit" ? null : range, + replaceRangeTab: shape === "insertReplace" ? { ...range, end: { line: 0, col: pos + 6 } } : null, + }), + req, + ); + applyLspCompletion(view, c, pos, pos + 5, mode); + expect(view.state.doc.toString()).toBe(`/* 😀 */ System.out.print()${mode === "insert" ? "Suffix" : ""}`); + }); + } + } + + it(`${mode} accepts the fresh JDT.LS Print response range`, () => { + const { view, req } = setup(" System.out.Print|"); + const c = makeCompletion( + item({ + label: "print", + filterText: "print(Object obj) : void", + insertText: "print(${0});", + isSnippet: true, + replaceRange: { start: { line: 0, col: 19 }, end: { line: 0, col: 24 } }, + }), + req, + ); + applyLspCompletion(view, c, 19, 24, mode); + expect(view.state.doc.toString()).toBe(" System.out.print();"); + }); + } + + for (const mode of ["insert", "replace"] as const) { + it(`${mode} accepts a cached lookup requested before typing Print`, async () => { + const { view, req, pos } = setup("System.out.|Suffix"); + const c = makeCompletion( + item({ + label: "print", + filterText: "print(Object obj) : void", + insertText: "print(${0})", + isSnippet: true, + replaceRange: { start: { line: 0, col: pos }, end: { line: 0, col: pos } }, + }), + req, + ); + const source = vi.fn(() => ({ from: pos, options: [c], validFor: /^\w*$/ })); + view.dispatch({ + effects: StateEffect.appendConfig.of([ + completionExtension(), + EditorState.languageData.of(() => [{ openideaCompletion: source }]), + ]), + }); + startCompletion(view); + await vi.waitFor(() => expect(currentCompletions(view.state)).toHaveLength(1)); + view.dispatch({ + changes: { from: pos, insert: "Print" }, + selection: { anchor: pos + 5 }, + userEvent: "input.type", + }); + await vi.waitFor(() => expect(currentCompletions(view.state)[0]).toBe(c)); + await new Promise((resolve) => setTimeout(resolve, 100)); + expect( + mode === "insert" + ? acceptCompletion(view) + : runScopeHandlers(view, new KeyboardEvent("keydown", { key: "Tab" }), "editor"), + ).toBe(true); + expect(source).toHaveBeenCalledTimes(1); + expect(view.state.doc.toString()).toBe(`System.out.print()${mode === "insert" ? "Suffix" : ""}`); + }); + } + + for (const template of [false, true]) { + it(`replaces mixed-case prefixes in contributed ${template ? "snippet" : "word"} completions`, async () => { + const { view } = setup("Print|"); + const source: CompletionSource = () => ({ + from: 0, + options: [{ label: "print", apply: template ? snippet("print(#{})") : "print" }], + }); + view.dispatch({ + effects: StateEffect.appendConfig.of([ + completionExtension(), + EditorState.languageData.of(() => [{ openideaCompletion: source }]), + ]), + }); + startCompletion(view); + await vi.waitFor(() => expect(currentCompletions(view.state)).toHaveLength(1)); + await new Promise((resolve) => setTimeout(resolve, 100)); + expect(acceptCompletion(view)).toBe(true); + expect(view.state.doc.toString()).toBe(template ? "print()" : "print"); + }); + } + it("applies additional edits (imports) in the same change", () => { const { view, req } = setup("\nfoo.ba|"); const c = makeCompletion( diff --git a/src/editor/lsp/completion.ts b/src/editor/lsp/completion.ts index 083fdbc7..7a9bf7ca 100644 --- a/src/editor/lsp/completion.ts +++ b/src/editor/lsp/completion.ts @@ -201,13 +201,17 @@ export function applyLspCompletion( const item = completion.resolved ?? resolvedItems.get(completion.lsp) ?? completion.lsp; const state = view.state; const delta = to - req.pos; - const mapOld = (offset: number) => - Math.max(0, Math.min(state.doc.length, offset < req.pos ? offset : offset + delta)); + const mapOld = (offset: number, assoc = 1) => + Math.max( + 0, + Math.min(state.doc.length, offset < req.pos || (offset === req.pos && assoc < 0) ? offset : offset + delta), + ); let start = from; let end = to; if (item.replaceRange) { - start = Math.min(mapOld(posToOffset(req.text, item.replaceRange.start)), to); + // An edit requested just after a trigger must include the prefix typed since then. + start = Math.min(mapOld(posToOffset(req.text, item.replaceRange.start), -1), to); end = Math.max(mapOld(posToOffset(req.text, item.replaceRange.end)), to); } if (mode === "replace") {