Skip to content

Commit

Permalink
Improvements to completion logic (mainly for top level) (#102)
Browse files Browse the repository at this point in the history
Improvements to completion logic (mainly for top level). In true TDD
fashion, I'm fairly confident with the changes given the tests are all
passing.

Added additional tests for
- curly brace in default behavior
#96
- autocomplete for a schema with top level $ref
- autocomplete for a schema with top level complex type

I wanted to also work on the "include value completions for boolean"
test case but there's a bit extra work (when completing the property
value and the node used for the completion is different from the
original node passed in to `getValueCompletions()`, we also need to
figure out the correct `from` and `to` values for the completion
context, so that applying the completion replaces the correct things,
otherwise it will replace the original node which may not be what we
want) to be done to get it working as expected, so decided to tackle
part of it but leave it commented out for now.
  • Loading branch information
imolorhe authored Jun 1, 2024
1 parent c63fc22 commit 296617f
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 28 deletions.
5 changes: 5 additions & 0 deletions .changeset/perfect-balloons-teach.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"codemirror-json-schema": patch
---

Improvements to completion logic (mainly for top level)
10 changes: 10 additions & 0 deletions src/__tests__/__fixtures__/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ export const testSchema2 = {
foo: {
type: "string",
},
stringWithDefault: {
type: "string",
description: "a string with a default value",
default: "defaultString",
},
bracedStringDefault: {
type: "string",
description: "a string with a default value containing braces",
default: "✨ A message from %{whom}: ✨",
},
object: {
type: "object",
properties: {
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/__helpers__/completion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export async function expectCompletion(
} = {}
) {
let cur = doc.indexOf("|"),
currentSchema = conf?.schema || testSchema2;
currentSchema = conf?.schema ?? testSchema2;
doc = doc.slice(0, cur) + doc.slice(cur + 1);

let state = EditorState.create({
Expand Down
100 changes: 93 additions & 7 deletions src/__tests__/json-completion.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, it } from "vitest";

import { expectCompletion } from "./__helpers__/completion.js";
import { MODES } from "../constants.js";
import { testSchema3, testSchema4 } from "./__fixtures__/schemas.js";

describe.each([
{
Expand Down Expand Up @@ -46,6 +47,35 @@ describe.each([
},
],
},
{
name: "include defaults for string when available",
mode: MODES.JSON,
docs: ['{ "stringWithDefault| }'],
expectedResults: [
{
label: "stringWithDefault",
type: "property",
detail: "string",
info: "a string with a default value",
template: '"stringWithDefault": "${defaultString}"',
},
],
},
// TODO: fix the default template with braces: https://discuss.codemirror.net/t/inserting-literal-via-snippets/8136/4
// {
// name: "include defaults for string with braces",
// mode: MODES.JSON,
// docs: ['{ "bracedStringDefault| }'],
// expectedResults: [
// {
// label: "bracedStringDefault",
// type: "property",
// detail: "string",
// info: "a string with a default value containing braces",
// template: '"bracedStringDefault": "${✨ A message from %{whom}: ✨}"',
// },
// ],
// },
{
name: "include defaults for enum when available",
mode: MODES.JSON,
Expand Down Expand Up @@ -110,13 +140,11 @@ describe.each([
},
],
},
// TODO: should provide true/false completions
// TODO: should provide true/false completions. Issue is the detected node is the Property node, which contains the property name and value. The prefix for the autocompletion therefore contains the property name, so it never matches the results
// {
// name: "include value completions for boolean",
// mode: MODES.JSON,
// docs: ['{ "booleanWithDefault": | }',
// },
// ],
// mode: MODES.JSON,
// docs: ['{ "booleanWithDefault": | }'],
// expectedResults: [
// {
// detail: "boolean",
Expand Down Expand Up @@ -221,6 +249,20 @@ describe.each([
],
},
// TODO: completion for array of objects should enhance the template
{
name: "autocomplete for array of objects with filter",
mode: MODES.JSON,
docs: ['{ "arrayOfObjects": [ { "f|" } ] }'],
expectedResults: [
{
detail: "string",
info: "",
label: "foo",
template: '"foo": "#{}"',
type: "property",
},
],
},
{
name: "autocomplete for array of objects with items",
mode: MODES.JSON,
Expand Down Expand Up @@ -298,6 +340,50 @@ describe.each([
},
],
},
{
name: "autocomplete for a schema with top level $ref",
mode: MODES.JSON,
docs: ['{ "| }'],
expectedResults: [
{
type: "property",
detail: "string",
info: "",
label: "foo",
template: '"foo": "#{}"',
},
{
type: "property",
detail: "number",
info: "",
label: "bar",
template: '"bar": #{0}',
},
],
schema: testSchema3,
},
{
name: "autocomplete for a schema with top level complex type",
mode: MODES.JSON,
docs: ['{ "| }'],
expectedResults: [
{
type: "property",
detail: "string",
info: "",
label: "foo",
template: '"foo": "#{}"',
},
{
type: "property",
detail: "number",
info: "",
label: "bar",
template: '"bar": #{0}',
},
],
schema: testSchema4,
},
// JSON5
{
name: "return bare property key when no quotes are used",
Expand Down Expand Up @@ -653,8 +739,8 @@ describe.each([
},
],
},
])("jsonCompletion", ({ name, docs, mode, expectedResults }) => {
])("jsonCompletion", ({ name, docs, mode, expectedResults, schema }) => {
it.each(docs)(`${name} (mode: ${mode})`, async (doc) => {
await expectCompletion(doc, expectedResults, { mode });
await expectCompletion(doc, expectedResults, { mode, schema });
});
});
48 changes: 30 additions & 18 deletions src/json-completion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ export class JSONCompletion {
this.mode = opts.mode ?? MODES.JSON;
}
public doComplete(ctx: CompletionContext) {
this.schema = getJSONSchema(ctx.state)!;
const s = getJSONSchema(ctx.state)!;
this.schema = this.expandSchemaProperty(s, s) ?? s;
if (!this.schema) {
// todo: should we even do anything without schema
// without taking over the existing mode responsibilties?
Expand All @@ -76,7 +77,7 @@ export class JSONCompletion {
let node: SyntaxNode | null = getNodeAtPosition(ctx.state, ctx.pos);

// position node word prefix (without quotes) for matching
const prefix = ctx.state.sliceDoc(node.from, ctx.pos).replace(/^("|')/, "");
let prefix = ctx.state.sliceDoc(node.from, ctx.pos).replace(/^(["'])/, "");

debug.log("xxx", "node", node, "prefix", prefix, "ctx", ctx);

Expand Down Expand Up @@ -212,7 +213,14 @@ export class JSONCompletion {
const types: { [type: string]: boolean } = {};

// value proposals with schema
this.getValueCompletions(this.schema, ctx, types, collector);
const res = this.getValueCompletions(this.schema, ctx, types, collector);
debug.log("xxx", "getValueCompletions res", res);
if (res) {
// TODO: While this works, we also need to handle the completion from and to positions to use it
// // use the value node to calculate the prefix
// prefix = res.valuePrefix;
// debug.log("xxx", "using valueNode prefix", prefix);
}
}

// handle filtering
Expand Down Expand Up @@ -269,6 +277,7 @@ export class JSONCompletion {

// Get matching schemas
const schemas = this.getSchemas(schema, ctx);
debug.log("xxx", "propertyCompletion schemas", schemas);

schemas.forEach((s) => {
if (typeof s !== "object") {
Expand Down Expand Up @@ -458,15 +467,7 @@ export class JSONCompletion {
}
private getInsertTextForPropertyName(key: string, rawWord: string) {
switch (this.mode) {
case MODES.JSON5: {
if (rawWord.startsWith('"')) {
return `"${key}"`;
}
if (rawWord.startsWith("'")) {
return `'${key}'`;
}
return key;
}
case MODES.JSON5:
case MODES.YAML: {
if (rawWord.startsWith('"')) {
return `"${key}"`;
Expand All @@ -484,13 +485,10 @@ export class JSONCompletion {
switch (this.mode) {
case MODES.JSON5:
return `'${prf}{${value}}'`;
break;
case MODES.YAML:
return `${prf}{${value}}`;
break;
default:
return `"${prf}{${value}}"`;
break;
}
}

Expand Down Expand Up @@ -661,6 +659,19 @@ export class JSONCompletion {
}
}
}

// TODO: We need to pass the from and to for the value node as well
// TODO: What should be the from and to when the value node is null?
// TODO: (NOTE: if we pass a prefix but no from and to, it will autocomplete the value but replace
// TODO: the entire property nodewhich isn't what we want). Instead we need to change the from and to
// TODO: based on the corresponding (relevant) value node
const valuePrefix = valueNode
? getWord(ctx.state.doc, valueNode, true, false)
: "";

return {
valuePrefix,
};
}

private addSchemaValueCompletions(
Expand Down Expand Up @@ -816,8 +827,9 @@ export class JSONCompletion {
debug.log("xxx", "pointer..", JSON.stringify(pointer));

// For some reason, it returns undefined schema for the root pointer
// We use the root schema in that case as the relevant (sub)schema
if (!pointer || pointer === "/") {
return [schema];
subSchema = this.expandSchemaProperty(schema, schema) ?? schema;
}
// const subSchema = new Draft07(this.schema).getSchema(pointer);
debug.log("xxx", "subSchema..", subSchema);
Expand Down Expand Up @@ -847,8 +859,8 @@ export class JSONCompletion {
return [subSchema as JSONSchema7];
}

private expandSchemaProperty(
property: JSONSchema7Definition,
private expandSchemaProperty<T extends JSONSchema7Definition>(
property: T,
schema: JSONSchema7
) {
if (typeof property === "object" && property.$ref) {
Expand Down
11 changes: 9 additions & 2 deletions src/utils/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,17 @@ export const surroundingDoubleQuotesToSingle = (str: string) => {
export const getWord = (
doc: Text,
node: SyntaxNode | null,
stripQuotes = true
stripQuotes = true,
onlyEvenQuotes = true
) => {
const word = node ? doc.sliceString(node.from, node.to) : "";
return stripQuotes ? stripSurroundingQuotes(word) : word;
if (!stripQuotes) {
return word;
}
if (onlyEvenQuotes) {
return stripSurroundingQuotes(word);
}
return word.replace(/(^["'])|(["']$)/g, "");
};

export const isInvalidValueNode = (node: SyntaxNode, mode: JSONMode) => {
Expand Down

0 comments on commit 296617f

Please sign in to comment.