From 7ed9e3e206ec7a47f8f7dde7d2a50a75228ae0be Mon Sep 17 00:00:00 2001 From: Rikki Schulte Date: Wed, 29 Nov 2023 11:20:01 +0100 Subject: [PATCH] fix required values fix tests fix required fields validation --- .changeset/heavy-geckos-exist.md | 5 ++ README.md | 2 +- src/__tests__/__fixtures__/schemas.ts | 3 +- src/__tests__/json-completion.spec.ts | 54 +------------------ src/__tests__/json-validation.spec.ts | 22 ++++++-- src/json-validation.ts | 17 ++++-- src/utils/__tests__/jsonPointers.spec.ts | 21 +++++++- src/utils/__tests__/parseJSONDocument.spec.ts | 2 + src/utils/jsonPointers.ts | 3 +- 9 files changed, 64 insertions(+), 65 deletions(-) create mode 100644 .changeset/heavy-geckos-exist.md diff --git a/.changeset/heavy-geckos-exist.md b/.changeset/heavy-geckos-exist.md new file mode 100644 index 0000000..39c2007 --- /dev/null +++ b/.changeset/heavy-geckos-exist.md @@ -0,0 +1,5 @@ +--- +"codemirror-json-schema": patch +--- + +fix required fields validation diff --git a/README.md b/README.md index 5a885a9..fb58295 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ Based on whether you want to support json4, json5 or both, you will need to inst ### Breaking Changes: -- 0.5.0 - this breaking change _does not_ effect users using `jsonSchema()` or `json5Schema()` modes, but those using the "custom path". +- 0.5.0 - this breaking change only impacts those following the "custom usage" approach, it _does not_ effect users using the high level, "bundled" `jsonSchema()` or `json5Schema()` modes. See the custom usages below to learn how to use the new `stateExtensions` and `handleRefresh` exports. ### json4 diff --git a/src/__tests__/__fixtures__/schemas.ts b/src/__tests__/__fixtures__/schemas.ts index 4b5f8ac..4cb2769 100644 --- a/src/__tests__/__fixtures__/schemas.ts +++ b/src/__tests__/__fixtures__/schemas.ts @@ -25,6 +25,7 @@ export const testSchema2 = { description: "an elegant string", }, }, + required: ["foo"], additionalProperties: false, }, oneOfEg: { @@ -71,7 +72,7 @@ export const testSchema2 = { type: "boolean", }, }, - required: ["foo", "object.foo"], + required: ["foo", "object"], additionalProperties: false, definitions: { fancyObject: { diff --git a/src/__tests__/json-completion.spec.ts b/src/__tests__/json-completion.spec.ts index 4f7a0eb..e3f03ba 100644 --- a/src/__tests__/json-completion.spec.ts +++ b/src/__tests__/json-completion.spec.ts @@ -1,7 +1,6 @@ import { describe, it } from "vitest"; -import { expectCompletion } from "./__helpers__/completion"; -import { testSchema3, testSchema4 } from "./__fixtures__/schemas"; +import { expectCompletion } from "./__helpers__/completion.js"; describe("jsonCompletion", () => { it("should return completion data for simple types", async () => { @@ -292,57 +291,6 @@ describe("jsonCompletion", () => { }, ]); }); - it("should autocomplete for array of objects with filter", async () => { - await expectCompletion('{ "arrayOfObjects": [ { "f|" } ] }', [ - { - detail: "string", - info: "", - label: "foo", - template: '"foo": "#{}"', - type: "property", - }, - ]); - }); - it("should autocomplete for a schema with top level $ref", async () => { - await expectCompletion( - '{ "| }', - [ - { - type: "property", - detail: "string", - info: "", - label: "foo", - }, - { - type: "property", - detail: "number", - info: "", - label: "bar", - }, - ], - { schema: testSchema3 } - ); - }); - it("should autocomplete for a schema with top level complex type", async () => { - await expectCompletion( - '{ "| }', - [ - { - type: "property", - detail: "string", - info: "", - label: "foo", - }, - { - type: "property", - detail: "number", - info: "", - label: "bar", - }, - ], - { schema: testSchema4 } - ); - }); }); describe("json5Completion", () => { diff --git a/src/__tests__/json-validation.spec.ts b/src/__tests__/json-validation.spec.ts index 4f0cf4d..2e00a15 100644 --- a/src/__tests__/json-validation.spec.ts +++ b/src/__tests__/json-validation.spec.ts @@ -17,7 +17,7 @@ const getErrors = (jsonString: string, schema?: JSONSchema7) => { }; const expectErrors = ( jsonString: string, - errors: [from: number, to: number, message: string][], + errors: [from: number | undefined, to: number | undefined, message: string][], schema?: JSONSchema7 ) => { expect(getErrors(jsonString, schema)).toEqual( @@ -42,7 +42,9 @@ describe("json-validation", () => { ]); }); it("should not handle invalid json", () => { - expectErrors('{"foo": "example" "bar": 123}', []); + expectErrors('{"foo": "example" "bar": 123}', [ + [undefined, undefined, "Expected `object` but received `null`"], + ]); }); it("should provide range for invalid multline json", () => { expectErrors( @@ -53,13 +55,24 @@ describe("json-validation", () => { [[32, 37, "Additional property `bar` in `#` is not allowed"]] ); }); + it("should provide formatted error message when required fields are missing", () => { + expectErrors( + `{ + "foo": "example", + "object": {} + }`, + [[46, 48, "The required property `foo` is missing at `object`"]], + testSchema2 + ); + }); it("should provide formatted error message for oneOf fields with more than 2 items", () => { expectErrors( `{ "foo": "example", + "object": { "foo": "true" }, "oneOfEg": 123 }`, - [[43, 46, 'Expected one of `"string"`, `"array"`, or `"boolean"`']], + [[80, 83, 'Expected one of `"string"`, `"array"`, or `"boolean"`']], testSchema2 ); }); @@ -67,9 +80,10 @@ describe("json-validation", () => { expectErrors( `{ "foo": "example", + "object": { "foo": "true" }, "oneOfEg2": 123 }`, - [[44, 47, 'Expected one of `"string"` or `"array"`']], + [[81, 84, 'Expected one of `"string"` or `"array"`']], testSchema2 ); }); diff --git a/src/json-validation.ts b/src/json-validation.ts index cf5b746..30b0c08 100644 --- a/src/json-validation.ts +++ b/src/json-validation.ts @@ -106,7 +106,6 @@ export class JSONValidation { try { errors = this.schema.validate(json.data); } catch {} - if (!errors.length) return []; // reduce() because we want to filter out errors that don't have a pointer return errors.reduce((acc, error) => { @@ -114,16 +113,26 @@ export class JSONValidation { const pointer = json.pointers.get(errorPath) as JSONPointerData; if (pointer) { // if the error is a property error, use the key position - const isPropertyError = error.name === "NoAdditionalPropertiesError"; + const isKeyError = + error.name === "NoAdditionalPropertiesError" || + error.name === "RequiredPropertyError"; acc.push({ - from: isPropertyError ? pointer.keyFrom : pointer.valueFrom, - to: isPropertyError ? pointer.keyTo : pointer.valueTo, + from: isKeyError ? pointer.keyFrom : pointer.valueFrom, + to: isKeyError ? pointer.keyTo : pointer.valueTo, // TODO: create a domnode and replace `` with // renderMessage: () => error.message, message: this.rewriteError(error), severity: "error", source: this.schemaTitle, }); + } else { + acc.push({ + from: 0, + to: 0, + message: this.rewriteError(error), + severity: "error", + source: this.schemaTitle, + }); } return acc; }, [] as Diagnostic[]); diff --git a/src/utils/__tests__/jsonPointers.spec.ts b/src/utils/__tests__/jsonPointers.spec.ts index 83ba11d..a0ff1ff 100644 --- a/src/utils/__tests__/jsonPointers.spec.ts +++ b/src/utils/__tests__/jsonPointers.spec.ts @@ -63,7 +63,7 @@ describe("jsonPointerForPosition for json5", () => { describe("getJsonPointers", () => { it("should return a map of all pointers for a document", () => { const state = EditorState.create({ - doc: '{"object": { "foo": true }, "bar": 123}', + doc: '{"object": { "foo": true }, "bar": 123, "baz": [1,2,3], "boop": [{"foo": true}]}', extensions: [json()], }); const pointers = getJsonPointers(state); @@ -79,6 +79,25 @@ describe("getJsonPointers", () => { valueFrom: 35, valueTo: 38, }); + expect(pointers.get("/baz")).toEqual({ + keyFrom: 40, + keyTo: 45, + valueFrom: 47, + valueTo: 54, + }); + expect(pointers.get("/boop/0")).toEqual({ + keyFrom: 65, + keyTo: 78, + valueFrom: 78, + valueTo: 79, + }); + // TODO: return pointers for all array indexes, not just objects + // expect(pointers.get("/baz/0")).toEqual({ + // keyFrom: 40, + // keyTo: 45, + // valueFrom: 47, + // valueTo: 55, + // }); }); }); diff --git a/src/utils/__tests__/parseJSONDocument.spec.ts b/src/utils/__tests__/parseJSONDocument.spec.ts index cc44376..f4f76e4 100644 --- a/src/utils/__tests__/parseJSONDocument.spec.ts +++ b/src/utils/__tests__/parseJSONDocument.spec.ts @@ -8,6 +8,7 @@ describe("parseJSONDocument", () => { const doc = parseJSONDocument(`{"object": { "foo": true }, "bar": 123}`); expect(doc.data).toEqual({ object: { foo: true }, bar: 123 }); expect(Array.from(doc.pointers.keys())).toEqual([ + "", "/object", "/object/foo", "/bar", @@ -20,6 +21,7 @@ describe("parseJSON5Document", () => { const doc = parseJSON5Document(`{'obj"ect': { foo: true }, "bar": 123}`); expect(doc.data).toEqual({ ['obj"ect']: { foo: true }, bar: 123 }); expect(Array.from(doc.pointers.keys())).toEqual([ + "", '/obj"ect', '/obj"ect/foo', "/bar", diff --git a/src/utils/jsonPointers.ts b/src/utils/jsonPointers.ts index f611b98..7ee6187 100644 --- a/src/utils/jsonPointers.ts +++ b/src/utils/jsonPointers.ts @@ -34,6 +34,7 @@ export function getJsonPointerAt(docText: Text, node: SyntaxNode): string { } } path.unshift(""); + return path.join("/"); } @@ -61,7 +62,7 @@ export const getJsonPointers = ( const pointers: JSONPointersMap = new Map(); json.iterate({ enter: (type: SyntaxNodeRef) => { - if (type.name === "PropertyName") { + if (type.name === "PropertyName" || type.name === "Object") { const pointer = getJsonPointerAt(state.doc, type.node); const { from: keyFrom, to: keyTo } = type.node;