From ac5c3aa7b89748f31cd24022fb2747da39eb805a Mon Sep 17 00:00:00 2001 From: Martin Aeschlimann Date: Thu, 28 Jan 2016 15:39:25 +0100 Subject: [PATCH] Fixes #2342: [json] JSON suggestion completed incorrectly --- extensions/json/server/src/jsonCompletion.ts | 9 +- .../json/server/src/test/completion.test.ts | 86 +++++++++++-------- .../json/server/src/test/formatter.test.ts | 14 +-- .../json/server/src/test/textEditSupport.ts | 23 +++++ 4 files changed, 79 insertions(+), 53 deletions(-) create mode 100644 extensions/json/server/src/test/textEditSupport.ts diff --git a/extensions/json/server/src/jsonCompletion.ts b/extensions/json/server/src/jsonCompletion.ts index 7eecb4c65c9..27b6a296f55 100644 --- a/extensions/json/server/src/jsonCompletion.ts +++ b/extensions/json/server/src/jsonCompletion.ts @@ -35,6 +35,7 @@ export class JSONCompletion { let offset = document.offsetAt(textDocumentPosition.position); let node = doc.getNodeFromOffsetEndInclusive(offset); + let currentWord = this.getCurrentWord(document, offset); let overwriteRange = null; let result: CompletionList = { items: [], @@ -43,6 +44,8 @@ export class JSONCompletion { if (node && (node.type === 'string' || node.type === 'number' || node.type === 'boolean' || node.type === 'null')) { overwriteRange = Range.create(document.positionAt(node.start), document.positionAt(node.end)); + } else { + overwriteRange = Range.create(document.positionAt(offset - currentWord.length), textDocumentPosition.position); } let proposed: { [key: string]: boolean } = {}; @@ -70,7 +73,7 @@ export class JSONCompletion { let addValue = true; let currentKey = ''; - let currentWord = ''; + let currentProperty: Parser.PropertyASTNode = null; if (node) { @@ -80,12 +83,10 @@ export class JSONCompletion { addValue = !(node.parent && ((node.parent).value)); currentProperty = node.parent ? node.parent : null; currentKey = document.getText().substring(node.start + 1, node.end - 1); - currentWord = document.getText().substring(node.start + 1, offset); if (node.parent) { node = node.parent.parent; } } - } } @@ -114,7 +115,7 @@ export class JSONCompletion { let location = node.getNodeLocation(); this.contributions.forEach((contribution) => { - let collectPromise = contribution.collectPropertySuggestions(textDocumentPosition.uri, location, this.getCurrentWord(document, offset), addValue, isLast, collector); + let collectPromise = contribution.collectPropertySuggestions(textDocumentPosition.uri, location, currentWord, addValue, isLast, collector); if (collectPromise) { collectionPromises.push(collectPromise); } diff --git a/extensions/json/server/src/test/completion.test.ts b/extensions/json/server/src/test/completion.test.ts index 0953543a530..1ead7464fa8 100644 --- a/extensions/json/server/src/test/completion.test.ts +++ b/extensions/json/server/src/test/completion.test.ts @@ -12,6 +12,7 @@ import {JSONCompletion} from '../jsonCompletion'; import {IXHROptions, IXHRResponse} from '../utils/httpRequest'; import {CompletionItem, CompletionItemKind, CompletionOptions, ITextDocument, TextDocumentIdentifier, TextDocumentPosition, Range, Position, TextEdit} from 'vscode-languageserver'; +import {applyEdits} from './textEditSupport'; suite('JSON Completion', () => { @@ -19,14 +20,18 @@ suite('JSON Completion', () => { return Promise.reject({ responseText: '', status: 404 }); } - var assertSuggestion = function(completions: CompletionItem[], label: string, documentation?: string) { + var assertSuggestion = function(completions: CompletionItem[], label: string, documentation?: string, document?: ITextDocument, resultText?: string) { var matches = completions.filter(function(completion: CompletionItem) { return completion.label === label && (!documentation || completion.documentation === documentation); - }).length; - assert.equal(matches, 1, label + " should only existing once"); + }); + assert.equal(matches.length, 1, label + " should only existing once"); + if (document && resultText) { + assert.equal(applyEdits(document, [ matches[0].textEdit ]), resultText); + } }; + - var testSuggestionsFor = function(value: string, stringAfter: string, schema?: JsonSchema.IJSONSchema): Thenable { + var testSuggestionsFor = function(value: string, stringAfter: string, schema: JsonSchema.IJSONSchema, test: (items: CompletionItem[], document: ITextDocument) => void) : Thenable { var uri = 'test://test.json'; var idx = stringAfter ? value.indexOf(stringAfter) : 0; @@ -40,32 +45,35 @@ suite('JSON Completion', () => { var document = ITextDocument.create(uri, value); var textDocumentLocation = TextDocumentPosition.create(uri, Position.create(0, idx)); var jsonDoc = Parser.parse(value); - return completionProvider.doSuggest(document, textDocumentLocation, jsonDoc).then(list => list.items); + return completionProvider.doSuggest(document, textDocumentLocation, jsonDoc).then(list => list.items).then(completions => { + test(completions, document); + return null; + }) }; test('Complete keys no schema', function(testDone) { Promise.all([ - testSuggestionsFor('[ { "name": "John", "age": 44 }, { /**/ }', '/**/').then((result) => { + testSuggestionsFor('[ { "name": "John", "age": 44 }, { /**/ }', '/**/', null, result => { assert.strictEqual(result.length, 2); assertSuggestion(result, 'name'); assertSuggestion(result, 'age'); }), - testSuggestionsFor('[ { "name": "John", "age": 44 }, { "/**/ }', '/**/').then((result) => { + testSuggestionsFor('[ { "name": "John", "age": 44 }, { "/**/ }', '/**/', null, result => { assert.strictEqual(result.length, 2); assertSuggestion(result, 'name'); assertSuggestion(result, 'age'); }), - testSuggestionsFor('[ { "name": "John", "age": 44 }, { "n/**/ }', '/**/').then((result) => { + testSuggestionsFor('[ { "name": "John", "age": 44 }, { "n/**/ }', '/**/', null, result => { assert.strictEqual(result.length, 2); assertSuggestion(result, 'name'); }), - testSuggestionsFor('[ { "name": "John", "age": 44 }, { "name/**/" }', '/**/').then((result) => { + testSuggestionsFor('[ { "name": "John", "age": 44 }, { "name/**/" }', '/**/', null, result => { assert.strictEqual(result.length, 2); assertSuggestion(result, 'name'); }), - testSuggestionsFor('[ { "name": "John", "address": { "street" : "MH Road", "number" : 5 } }, { "name": "Jack", "address": { "street" : "100 Feet Road", /**/ }', '/**/').then((result) => { + testSuggestionsFor('[ { "name": "John", "address": { "street" : "MH Road", "number" : 5 } }, { "name": "Jack", "address": { "street" : "100 Feet Road", /**/ }', '/**/', null, result => { assert.strictEqual(result.length, 1); assertSuggestion(result, 'number'); }) @@ -74,33 +82,33 @@ suite('JSON Completion', () => { test('Complete values no schema', function(testDone) { Promise.all([ - testSuggestionsFor('[ { "name": "John", "age": 44 }, { "name": /**/', '/**/').then((result) => { + testSuggestionsFor('[ { "name": "John", "age": 44 }, { "name": /**/', '/**/', null, result => { assert.strictEqual(result.length, 1); assertSuggestion(result, '"John"'); }), - testSuggestionsFor('[ { "data": { "key": 1, "data": true } }, { "data": /**/', '/**/').then((result) => { + testSuggestionsFor('[ { "data": { "key": 1, "data": true } }, { "data": /**/', '/**/', null, result => { assert.strictEqual(result.length, 3); assertSuggestion(result, '{}'); assertSuggestion(result, 'true'); assertSuggestion(result, 'false'); }), - testSuggestionsFor('[ { "data": "foo" }, { "data": "bar" }, { "data": "/**/" } ]', '/**/').then((result) => { + testSuggestionsFor('[ { "data": "foo" }, { "data": "bar" }, { "data": "/**/" } ]', '/**/', null, result => { assert.strictEqual(result.length, 3); assertSuggestion(result, '"foo"'); assertSuggestion(result, '"bar"'); assertSuggestion(result, '"/**/"'); }), - testSuggestionsFor('[ { "data": "foo" }, { "data": "bar" }, { "data": "f/**/" } ]', '/**/').then((result) => { + testSuggestionsFor('[ { "data": "foo" }, { "data": "bar" }, { "data": "f/**/" } ]', '/**/', null, result => { assert.strictEqual(result.length, 3); assertSuggestion(result, '"foo"'); assertSuggestion(result, '"bar"'); assertSuggestion(result, '"f/**/"'); }), - testSuggestionsFor('[ { "data": "foo" }, { "data": "bar" }, { "data": "xoo"/**/ } ]', '/**/').then((result) => { + testSuggestionsFor('[ { "data": "foo" }, { "data": "bar" }, { "data": "xoo"/**/ } ]', '/**/', null, result => { assert.strictEqual(result.length, 3); assertSuggestion(result, '"xoo"'); }), - testSuggestionsFor('[ { "data": "foo" }, { "data": "bar" }, { "data": "xoo" /**/ } ]', '/**/').then((result) => { + testSuggestionsFor('[ { "data": "foo" }, { "data": "bar" }, { "data": "xoo" /**/ } ]', '/**/', null, result => { assert.strictEqual(result.length, 0); }) ]).then(() => testDone(), (error) => testDone(error)); @@ -125,23 +133,27 @@ suite('JSON Completion', () => { } }; Promise.all([ - testSuggestionsFor('{/**/}', '/**/', schema).then((result) => { + testSuggestionsFor('{/**/}', '/**/', schema, result => { assert.strictEqual(result.length, 3); assertSuggestion(result, 'a', 'A'); assertSuggestion(result, 'b', 'B'); assertSuggestion(result, 'c', 'C'); }), - testSuggestionsFor('{ "/**/}', '/**/', schema).then((result) => { + testSuggestionsFor('{ "/**/}', '/**/', schema, result => { assert.strictEqual(result.length, 3); assertSuggestion(result, 'a', 'A'); assertSuggestion(result, 'b', 'B'); assertSuggestion(result, 'c', 'C'); }), - testSuggestionsFor('{ "a/**/}', '/**/', schema).then((result) => { + testSuggestionsFor('{ "a/**/}', '/**/', schema, (result, document) => { assert.strictEqual(result.length, 3); - assertSuggestion(result, 'a', 'A'); + assertSuggestion(result, 'a', 'A', document, '{ "a": {{0}}'); }), - testSuggestionsFor('{ "a" = 1;/**/}', '/**/', schema).then((result) => { + testSuggestionsFor('{ a/**/}', '/**/', schema, (result, document) => { + assert.strictEqual(result.length, 3); + assertSuggestion(result, 'a', 'A', document, '{ "a": {{0}}/**/}'); + }), + testSuggestionsFor('{ "a" = 1;/**/}', '/**/', schema, result => { assert.strictEqual(result.length, 2); assertSuggestion(result, 'b', 'B'); assertSuggestion(result, 'c', 'C'); @@ -161,20 +173,20 @@ suite('JSON Completion', () => { } }; Promise.all([ - testSuggestionsFor('{ "a": /**/ }', '/**/', schema).then((result) => { + testSuggestionsFor('{ "a": /**/ }', '/**/', schema, result => { assert.strictEqual(result.length, 3); assertSuggestion(result, '"John"'); assertSuggestion(result, '"Jeff"'); assertSuggestion(result, '"George"'); }), - testSuggestionsFor('{ "a": "J/**/ }', '/**/', schema).then((result) => { + testSuggestionsFor('{ "a": "J/**/ }', '/**/', schema, result => { assert.strictEqual(result.length, 3); assertSuggestion(result, '"John"'); assertSuggestion(result, '"Jeff"'); }), - testSuggestionsFor('{ "a": "John"/**/ }', '/**/', schema).then((result) => { + testSuggestionsFor('{ "a": "John"/**/ }', '/**/', schema, result => { assert.strictEqual(result.length, 3); assertSuggestion(result, '"John"'); }) @@ -202,7 +214,7 @@ suite('JSON Completion', () => { }] }; Promise.all([ - testSuggestionsFor(content, '/**/', schema).then((result) => { + testSuggestionsFor(content, '/**/', schema, result => { assert.strictEqual(result.length, 2); assertSuggestion(result, 'a', 'A'); assertSuggestion(result, 'b', 'B'); @@ -242,14 +254,14 @@ suite('JSON Completion', () => { }] }; Promise.all([ - testSuggestionsFor('{/**/}', '/**/', schema).then((result) => { + testSuggestionsFor('{/**/}', '/**/', schema, result => { assert.strictEqual(result.length, 4); assertSuggestion(result, 'a', 'A'); assertSuggestion(result, 'b', 'B'); assertSuggestion(result, 'c', 'C'); assertSuggestion(result, 'd', 'D'); }), - testSuggestionsFor('{ "a": "", /**/}', '/**/', schema).then((result) => { + testSuggestionsFor('{ "a": "", /**/}', '/**/', schema, result => { assert.strictEqual(result.length, 1); assertSuggestion(result, 'b', 'B'); }) @@ -282,13 +294,13 @@ suite('JSON Completion', () => { }] }; Promise.all([ - testSuggestionsFor('{/**/}', '/**/', schema).then((result) => { + testSuggestionsFor('{/**/}', '/**/', schema, result => { assert.strictEqual(result.length, 3); assertSuggestion(result, 'type'); assertSuggestion(result, 'b'); assertSuggestion(result, 'c'); }), - testSuggestionsFor('{ "type": "appartment", /**/}', '/**/', schema).then((result) => { + testSuggestionsFor('{ "type": "appartment", /**/}', '/**/', schema, result => { assert.strictEqual(result.length, 1); assertSuggestion(result, 'c'); }) @@ -340,7 +352,7 @@ suite('JSON Completion', () => { }] }; Promise.all([ - testSuggestionsFor('{/**/}', '/**/', schema).then((result) => { + testSuggestionsFor('{/**/}', '/**/', schema, result => { assert.strictEqual(result.length, 5); assertSuggestion(result, 'a', 'A'); assertSuggestion(result, 'b1', 'B1'); @@ -348,7 +360,7 @@ suite('JSON Completion', () => { assertSuggestion(result, 'c', 'C'); assertSuggestion(result, 'd', 'D'); }), - testSuggestionsFor('{ "b1": "", /**/}', '/**/', schema).then((result) => { + testSuggestionsFor('{ "b1": "", /**/}', '/**/', schema, result => { assert.strictEqual(result.length, 2); assertSuggestion(result, 'a', 'A'); assertSuggestion(result, 'b2', 'B2'); @@ -404,36 +416,36 @@ suite('JSON Completion', () => { }] }; Promise.all([ - testSuggestionsFor('{/**/}', '/**/', schema).then((result) => { + testSuggestionsFor('{/**/}', '/**/', schema, result => { assert.strictEqual(result.length, 4); assertSuggestion(result, 'type'); assertSuggestion(result, 'a'); assertSuggestion(result, 'b'); assertSuggestion(result, 'c'); }), - testSuggestionsFor('{ "type": /**/}', '/**/', schema).then((result) => { + testSuggestionsFor('{ "type": /**/}', '/**/', schema, result => { assert.strictEqual(result.length, 3); assertSuggestion(result, '"1"'); assertSuggestion(result, '"2"'); assertSuggestion(result, '"3"'); }), - testSuggestionsFor('{ "a": { "x": "", "y": "" }, "type": /**/}', '/**/', schema).then((result) => { + testSuggestionsFor('{ "a": { "x": "", "y": "" }, "type": /**/}', '/**/', schema, result => { assert.strictEqual(result.length, 2); assertSuggestion(result, '"1"'); assertSuggestion(result, '"2"'); }), - testSuggestionsFor('{ "type": "1", "a" : { /**/ }', '/**/', schema).then((result) => { + testSuggestionsFor('{ "type": "1", "a" : { /**/ }', '/**/', schema, result => { assert.strictEqual(result.length, 2); assertSuggestion(result, 'x'); assertSuggestion(result, 'y'); }), - testSuggestionsFor('{ "type": "1", "a" : { "x": "", "z":"" }, /**/', '/**/', schema).then((result) => { + testSuggestionsFor('{ "type": "1", "a" : { "x": "", "z":"" }, /**/', '/**/', schema, result => { // both alternatives have errors: intellisense proposes all options assert.strictEqual(result.length, 2); assertSuggestion(result, 'b'); assertSuggestion(result, 'c'); }), - testSuggestionsFor('{ "a" : { "x": "", "z":"" }, /**/', '/**/', schema).then((result) => { + testSuggestionsFor('{ "a" : { "x": "", "z":"" }, /**/', '/**/', schema, result => { assert.strictEqual(result.length, 2); assertSuggestion(result, 'type'); assertSuggestion(result, 'c'); diff --git a/extensions/json/server/src/test/formatter.test.ts b/extensions/json/server/src/test/formatter.test.ts index ccd94ad55bc..2dc01cf60bb 100644 --- a/extensions/json/server/src/test/formatter.test.ts +++ b/extensions/json/server/src/test/formatter.test.ts @@ -8,6 +8,7 @@ import Json = require('../json-toolbox/json'); import {ITextDocument, DocumentFormattingParams, Range, Position, FormattingOptions, TextEdit} from 'vscode-languageserver'; import Formatter = require('../jsonFormatter'); import assert = require('assert'); +import {applyEdits} from './textEditSupport'; suite('JSON Formatter', () => { @@ -28,18 +29,7 @@ suite('JSON Formatter', () => { var document = ITextDocument.create(uri, unformatted); let edits = Formatter.format(document, range, { tabSize: 2, insertSpaces: insertSpaces }); - - let formatted = unformatted; - let sortedEdits = edits.sort((a, b) => document.offsetAt(b.range.start) - document.offsetAt(a.range.start)); - let lastOffset = formatted.length; - sortedEdits.forEach(e => { - let startOffset = document.offsetAt(e.range.start); - let endOffset = document.offsetAt(e.range.end); - assert.ok(startOffset <= endOffset); - assert.ok(endOffset <= lastOffset); - formatted = formatted.substring(0, startOffset) + e.newText + formatted.substring(endOffset, formatted.length); - lastOffset = startOffset; - }) + let formatted = applyEdits(document, edits); assert.equal(formatted, expected); } diff --git a/extensions/json/server/src/test/textEditSupport.ts b/extensions/json/server/src/test/textEditSupport.ts new file mode 100644 index 00000000000..b3b78552c5b --- /dev/null +++ b/extensions/json/server/src/test/textEditSupport.ts @@ -0,0 +1,23 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ +'use strict'; + +import {ITextDocument, DocumentFormattingParams, Range, Position, FormattingOptions, TextEdit} from 'vscode-languageserver'; +import assert = require('assert'); + +export function applyEdits(document: ITextDocument, edits: TextEdit[]) : string { + let formatted = document.getText(); + let sortedEdits = edits.sort((a, b) => document.offsetAt(b.range.start) - document.offsetAt(a.range.start)); + let lastOffset = formatted.length; + sortedEdits.forEach(e => { + let startOffset = document.offsetAt(e.range.start); + let endOffset = document.offsetAt(e.range.end); + assert.ok(startOffset <= endOffset); + assert.ok(endOffset <= lastOffset); + formatted = formatted.substring(0, startOffset) + e.newText + formatted.substring(endOffset, formatted.length); + lastOffset = startOffset; + }); + return formatted; +} \ No newline at end of file