Skip to content

Commit d0fba7b

Browse files
committed
fix(lsp): consume text after cursor when completion textEdit range covers it
JSON key completion inside autoclosed quotes (`"|"`) inserted `"author": {}"` - the server's textEdit range covers both quotes, but insertHint always clamped the replacement end to the cursor, leaving the closing quote dangling. Now a range.end beyond the cursor is honored; an end at/before the cursor is still clamped so cached completion lists served while typing forward keep working as before. Adds a JSON LSP regression spec driving the hint provider directly (the popup UI needs OS window focus the embedded test window may not have), plus a test-only _getClient hook in JsonLsp.
1 parent 4ddac10 commit d0fba7b

4 files changed

Lines changed: 85 additions & 5 deletions

File tree

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/extensionsIntegrated/JSONSupport/JsonLsp.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,4 +235,12 @@ define(function (require, exports, module) {
235235
exports.canRun = canRun;
236236
exports.SERVER_ID = SERVER_ID;
237237
exports._setTestSchemaAssociations = _setTestSchemaAssociations;
238+
if (Phoenix.isTestWindow) {
239+
// Test hook - the registered LanguageClient (null until the server has started). Lets
240+
// integration tests drive the LSP providers (e.g. codeHints) directly, since the hint UI
241+
// needs OS window focus that the embedded test window may not have.
242+
exports._getClient = function () {
243+
return client;
244+
};
245+
}
238246
});

src/languageTools/DefaultProviders.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -642,12 +642,19 @@ define(function (require, exports, module) {
642642
// That start is stable as the user types forward (word/member starts don't move) and, crucially,
643643
// for member completions it points AT the trigger "." while newText itself includes the dot
644644
// (e.g. "console." + item ".log" -> replace from the "." -> "console.log", not "console..log").
645-
// We deliberately end at the CURRENT cursor rather than token.textEdit.range.end: that end is
646-
// stale when completions are served from cache while typing continues, which would otherwise
647-
// replace only part of the word (e.g. "conso"+enter -> "consolenso").
648645
if (textEditRange && textEditRange.start.line === cursor.line &&
649646
textEditRange.start.character <= cursor.ch) {
650647
startCh = textEditRange.start.character;
648+
// Honor a range.end BEYOND the cursor: the server wants existing text after the caret
649+
// overwritten because newText includes it (e.g. JSON key completion inside autoclosed
650+
// quotes `"|"` - newText `"author": {$1}` covers both quotes; keeping the closing quote
651+
// would leave a dangling `"` behind the insert). A range.end at/before the cursor is
652+
// clamped TO the cursor instead: that end is stale when completions are served from
653+
// cache while typing continues, which would otherwise replace only part of the word
654+
// (e.g. "conso"+enter -> "consolenso").
655+
if (textEditRange.end.line === cursor.line && textEditRange.end.character > cursor.ch) {
656+
endCh = textEditRange.end.character;
657+
}
651658
} else {
652659
startCh = cursor.ch;
653660
while (startCh > 0 && /[\w$]/.test(lineText.charAt(startCh - 1))) {

test/spec/Extn-JSONSupport-integ-test.js

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,71 @@ define(function (require, exports, module) {
8888
JsonLsp._setTestSchemaAssociations(null);
8989
}, 45000);
9090

91+
it("should consume the autoclosed closing quote when inserting a key completion", async function () {
92+
// Regression: typing `"` autocloses to `"|"`; the server's completion textEdit range
93+
// covers BOTH quotes (its end is past the cursor) and newText is a full `"key": {$1}`
94+
// snippet. insertHint used to clamp the replacement end to the cursor, leaving the
95+
// closing quote dangling -> `"key": {}"` (invalid JSON).
96+
const JsonLsp = testWindow.require("extensionsIntegrated/JSONSupport/JsonLsp");
97+
const EditorManager = testWindow.brackets.test.EditorManager;
98+
JsonLsp._setTestSchemaAssociations([{
99+
fileMatch: ["appsettings.json"],
100+
schema: {
101+
type: "object",
102+
properties: {
103+
serverConfig: { type: "object" }
104+
}
105+
}
106+
}]);
107+
await _openFile("appsettings.json");
108+
const editor = EditorManager.getActiveEditor();
109+
editor.document.setText('{\n ""\n}');
110+
editor.setCursorPos(1, 3); // between the quotes: ` "|"`
111+
112+
// Drive the LSP hint provider directly (the popup UI needs OS window focus that the
113+
// embedded test window may not have). Poll fresh requests until the schema completion
114+
// shows up - the pushed schema association may take a moment to apply server-side.
115+
await awaitsFor(function () {
116+
return !!JsonLsp._getClient();
117+
}, "JSON language client registered", 30000);
118+
const client = JsonLsp._getClient();
119+
let $targetHint = null,
120+
requestInFlight = false;
121+
await awaitsFor(function () {
122+
if ($targetHint) {
123+
return true;
124+
}
125+
if (!requestInFlight) {
126+
requestInFlight = true;
127+
client._completionCache = null; // context key is stable here - don't reuse stale lists
128+
client.codeHints.getHints(null).done(function (result) {
129+
requestInFlight = false;
130+
const hints = (result && result.hints) || [];
131+
$targetHint = hints.find(function ($hint) {
132+
const token = $hint.data("token");
133+
return token && token.label === "serverConfig";
134+
}) || null;
135+
}).fail(function () {
136+
requestInFlight = false;
137+
});
138+
}
139+
return false;
140+
}, "serverConfig schema completion from the JSON server", 30000);
141+
142+
client.codeHints.insertHint($targetHint); // what selecting the hint with Enter does
143+
await awaitsFor(function () {
144+
return editor.document.getLine(1).indexOf("serverConfig") !== -1;
145+
}, "completion inserted into the document", 30000);
146+
147+
expect(editor.document.getLine(1)).toBe(' "serverConfig": {}');
148+
// the whole document must stay valid JSON - no dangling quote after the insert
149+
expect(function () { JSON.parse(editor.document.getText()); }).not.toThrow();
150+
151+
JsonLsp._setTestSchemaAssociations(null);
152+
await awaitsForDone(CommandManager.execute(Commands.FILE_CLOSE,
153+
{ fullPath: testFolder + "/appsettings.json", _forceClose: true }));
154+
}, 45000);
155+
91156
it("should squiggle vulnerable dependencies using advisory data", async function () {
92157
const NpmRegistry = testWindow.require("extensionsIntegrated/JSONSupport/NpmRegistry");
93158
NpmRegistry._setFetcherForTests(function (url, options) {

0 commit comments

Comments
 (0)