From 8308f61f52a356a6cfa9b614bf5e494fa7350976 Mon Sep 17 00:00:00 2001 From: Adam Coster Date: Fri, 26 Aug 2022 12:33:46 -0500 Subject: [PATCH 1/3] fix: 'null' no longer incorrectly interpreted as string in JSDoc text Fixes #1373 --- .../BasicAnnotationsReader.ts | 9 ++- test/utils.ts | 57 ++++++++++++-- test/valid-data-annotations.test.ts | 29 ++++++++ test/valid-data/annotation-default/main.ts | 31 ++++++++ test/valid-data/annotation-default/samples.ts | 48 ++++++++++++ .../valid-data/annotation-default/schema.json | 74 +++++++++++++++++++ 6 files changed, 240 insertions(+), 8 deletions(-) create mode 100644 test/valid-data/annotation-default/main.ts create mode 100644 test/valid-data/annotation-default/samples.ts create mode 100644 test/valid-data/annotation-default/schema.json diff --git a/src/AnnotationsReader/BasicAnnotationsReader.ts b/src/AnnotationsReader/BasicAnnotationsReader.ts index 568a26189..7768cbe34 100644 --- a/src/AnnotationsReader/BasicAnnotationsReader.ts +++ b/src/AnnotationsReader/BasicAnnotationsReader.ts @@ -93,10 +93,13 @@ export class BasicAnnotationsReader implements AnnotationsReader { if (isTextTag) { return text; - } else if (BasicAnnotationsReader.jsonTags.has(jsDocTag.name)) { - return this.parseJson(text) ?? text; + } + let parsed = this.parseJson(text); + parsed = parsed === undefined ? text : parsed; + if (BasicAnnotationsReader.jsonTags.has(jsDocTag.name)) { + return parsed; } else if (this.extraTags?.has(jsDocTag.name)) { - return this.parseJson(text) ?? text; + return parsed; } else { // Unknown jsDoc tag. return undefined; diff --git a/test/utils.ts b/test/utils.ts index 1ebac2e45..03ef5c963 100644 --- a/test/utils.ts +++ b/test/utils.ts @@ -1,8 +1,8 @@ -import Ajv from "ajv"; +import Ajv, { Options as AjvOptions } from "ajv"; import addFormats from "ajv-formats"; import { readFileSync, writeFileSync } from "fs"; -import stringify from "safe-stable-stringify"; import { resolve } from "path"; +import stringify from "safe-stable-stringify"; import ts from "typescript"; import { createFormatter } from "../factory/formatter"; import { createParser } from "../factory/parser"; @@ -25,7 +25,29 @@ export function assertValidSchema( type?: string, jsDoc: Config["jsDoc"] = "none", extraTags?: Config["extraTags"], - schemaId?: Config["schemaId"] + schemaId?: Config["schemaId"], + options?: { + /** + * Array of sample data + * that should + * successfully validate. + */ + validSamples?: any[]; + /** + * Array of sample data + * that should + * fail to validate. + */ + invalidSamples?: any[]; + /** + * Options to pass to Ajv + * when creating the Ajv + * instance. + * + * @default {strict:false} + */ + ajvOptions?: AjvOptions; + } ) { return (): void => { const config: Config = { @@ -56,12 +78,37 @@ export function assertValidSchema( let localValidator = validator; if (extraTags) { - localValidator = new Ajv({ strict: false }); + localValidator = new Ajv(options?.ajvOptions || { strict: false }); addFormats(localValidator); } localValidator.validateSchema(actual); expect(localValidator.errors).toBeNull(); - localValidator.compile(actual); // Will find MissingRef errors + + // Compile in all cases to detect MissingRef errors + const validate = localValidator.compile(actual); + + // Use the compiled validator if there + // are any samples. + if (options?.invalidSamples) { + for (const sample of options.invalidSamples) { + const isValid = validate(sample); + if (isValid) { + console.log("Unexpectedly Valid:", sample); + } + expect(isValid).toBe(false); + } + } + if (options?.validSamples) { + for (const sample of options.validSamples) { + const isValid = validate(sample); + if (!isValid) { + console.log("Unexpectedly Invalid:", sample); + + console.log("AJV Errors:", validate.errors); + } + expect(isValid).toBe(true); + } + } }; } diff --git a/test/valid-data-annotations.test.ts b/test/valid-data-annotations.test.ts index 182b3da16..78ca7ea16 100644 --- a/test/valid-data-annotations.test.ts +++ b/test/valid-data-annotations.test.ts @@ -1,4 +1,5 @@ import { assertValidSchema } from "./utils"; +import * as annotationDefaultSamples from "./valid-data/annotation-default/samples"; describe("valid-data-annotations", () => { it( @@ -33,6 +34,34 @@ describe("valid-data-annotations", () => { it("annotation-comment", assertValidSchema("annotation-comment", "MyObject", "extended")); + it("annotation-default", function () { + // Without actually using the defaults. + assertValidSchema("annotation-default", "MyObject", "extended", [], undefined, { + validSamples: annotationDefaultSamples.validSamples, + invalidSamples: annotationDefaultSamples.invalidSamplesUnlessDefaults, + })(); + + // Having AJV use the defaults. + + // Since AJV will mutate, make + // shallow copies. + const validWithDefaults = annotationDefaultSamples.invalidSamplesUnlessDefaults.map((sample) => ({ + ...sample, + })); + + assertValidSchema("annotation-default", "MyObject", "extended", [], undefined, { + validSamples: validWithDefaults, + ajvOptions: { useDefaults: true }, + })(); + + // The previously-invalid samples + // should now match the expected + // structure when all defaults are applied. + validWithDefaults.forEach((sample) => { + expect(sample).toEqual(annotationDefaultSamples.expectedAfterDefaults); + }); + }); + it("annotation-example", assertValidSchema("annotation-example", "MyObject", "extended")); it("annotation-id", assertValidSchema("annotation-id", "MyObject", "extended", [], "Test")); diff --git a/test/valid-data/annotation-default/main.ts b/test/valid-data/annotation-default/main.ts new file mode 100644 index 000000000..ba3dc44de --- /dev/null +++ b/test/valid-data/annotation-default/main.ts @@ -0,0 +1,31 @@ +export interface MyObject { + /** + * @default {extra: {field:"value"}} + */ + nestedField: MyNestedObject; + /** + * @default 10 + */ + numberField: number; + /** + * @default "hello" + */ + stringField: string; + /** + * @default true + */ + booleanField?: boolean; + /** + * @default null + */ + nullField: null; + /** + * @default [{ numberField2: 10, stringField2: "yes", anyField: null }] + */ + arrayField: Array<{ numberField2: number; stringField2: string; anyField?: any }>; +} + +/** + * @default {} + */ +export type MyNestedObject = Record; diff --git a/test/valid-data/annotation-default/samples.ts b/test/valid-data/annotation-default/samples.ts new file mode 100644 index 000000000..037dcf0ea --- /dev/null +++ b/test/valid-data/annotation-default/samples.ts @@ -0,0 +1,48 @@ +import type { MyObject } from "./main.js"; + +export const validSamples: MyObject[] = [ + { + nullField: null, + numberField: 100, + stringField: "goodbye", + arrayField: [], + booleanField: false, + nestedField: {}, + }, +]; + +/** + * Samples that should be *invalid* if + * AJV is not using `useDefaults: true`, + * and otherwise valid. The resulting + * mutated object should be the same + * in all cases. + */ +export const invalidSamplesUnlessDefaults: any[] = [ + { + nullField: null, + numberField: 10, + stringField: "hello", + }, + {}, +]; + +/** + * The resulting data structure after + * `useDefaults` is used with the + * {@link invalidSamples} entries. + * + * We aren't testing AJV's behavior here. + * We're assuming that if AJV populates + * defaults, and those defaults are of + * the expected values, then this project + * must be working correctly. + */ +export const expectedAfterDefaults: MyObject = { + nullField: null, + numberField: 10, + stringField: "hello", + arrayField: [{ numberField2: 10, stringField2: "yes", anyField: null }], + booleanField: true, + nestedField: { extra: { field: "value" } }, +}; diff --git a/test/valid-data/annotation-default/schema.json b/test/valid-data/annotation-default/schema.json new file mode 100644 index 000000000..fb15595a7 --- /dev/null +++ b/test/valid-data/annotation-default/schema.json @@ -0,0 +1,74 @@ +{ + "$ref": "#/definitions/MyObject", + "$schema": "http://json-schema.org/draft-07/schema#", + "definitions": { + "MyNestedObject": { + "default": {}, + "type": "object" + }, + "MyObject": { + "additionalProperties": false, + "properties": { + "arrayField": { + "default": [ + { + "anyField": null, + "numberField2": 10, + "stringField2": "yes" + } + ], + "items": { + "additionalProperties": false, + "properties": { + "anyField": {}, + "numberField2": { + "type": "number" + }, + "stringField2": { + "type": "string" + } + }, + "required": [ + "numberField2", + "stringField2" + ], + "type": "object" + }, + "type": "array" + }, + "booleanField": { + "default": true, + "type": "boolean" + }, + "nestedField": { + "$ref": "#/definitions/MyNestedObject", + "default": { + "extra": { + "field": "value" + } + } + }, + "nullField": { + "default": null, + "type": "null" + }, + "numberField": { + "default": 10, + "type": "number" + }, + "stringField": { + "default": "hello", + "type": "string" + } + }, + "required": [ + "nestedField", + "numberField", + "stringField", + "nullField", + "arrayField" + ], + "type": "object" + } + } +} \ No newline at end of file From 01f4d6d0f9946fe74c9fc4c515560a169cbdd13d Mon Sep 17 00:00:00 2001 From: Adam Coster Date: Fri, 26 Aug 2022 14:27:01 -0500 Subject: [PATCH 2/3] docs: added simple readme describing subset of testing --- test/README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 test/README.md diff --git a/test/README.md b/test/README.md new file mode 100644 index 000000000..7154f34f6 --- /dev/null +++ b/test/README.md @@ -0,0 +1,14 @@ +# Testing + +## Schema Generation + +To add/update a test case for generating a valid schema from a typscript file: + +- Look in `test/valid-data` for a sample related to your change. If you don't find one, create your own following the naming convention. For example, when adding the new sample `annotation-default`: + - Create folder `test/valid-data/annotation-default` + - Add `main.ts` to that folder with the type sample +- Update the corresponding `main.ts` file with your changes. +- Compile the JSON Schema via `yarn --silent run run --path 'test/valid-data/{the-target-folder}/*.ts' --type 'MyObject' -o test/valid-data/{the-target-folder}/schema.json` + - This happens automatically when running `yarn test:update`, but that can take a while +- Add a test to `test/valid-data-annotations.test.ts`, matching a similar pattern to the existing tests. +- Run tests via `yarn jest test/valid-data-annotations.test.ts` (this only runs the subset of tests related to schema validation) From 85d763cb29ce10b36e2a3f2164c7105f45445f89 Mon Sep 17 00:00:00 2001 From: Adam Coster Date: Sun, 4 Sep 2022 10:23:28 -0500 Subject: [PATCH 3/3] docs: Simplified testing README --- test/README.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/README.md b/test/README.md index 7154f34f6..02e4264e6 100644 --- a/test/README.md +++ b/test/README.md @@ -2,13 +2,12 @@ ## Schema Generation -To add/update a test case for generating a valid schema from a typscript file: +To add/update a test case for generating a valid schema from a Typescript file: - Look in `test/valid-data` for a sample related to your change. If you don't find one, create your own following the naming convention. For example, when adding the new sample `annotation-default`: - Create folder `test/valid-data/annotation-default` - Add `main.ts` to that folder with the type sample - Update the corresponding `main.ts` file with your changes. -- Compile the JSON Schema via `yarn --silent run run --path 'test/valid-data/{the-target-folder}/*.ts' --type 'MyObject' -o test/valid-data/{the-target-folder}/schema.json` - - This happens automatically when running `yarn test:update`, but that can take a while +- Run `yarn test:update` to compile the JSON schema - Add a test to `test/valid-data-annotations.test.ts`, matching a similar pattern to the existing tests. - Run tests via `yarn jest test/valid-data-annotations.test.ts` (this only runs the subset of tests related to schema validation)