From 5e8d791615bcf334cf256c9d7bceb12b50cd5cdd Mon Sep 17 00:00:00 2001 From: shellyln Date: Wed, 19 Feb 2020 22:30:19 +0900 Subject: [PATCH 1/4] [WIP][FIX] Fix validation reporting. (incorrect parent name) --- src/lib/reporter.ts | 118 +++++++++++++++++++++++++++++--------------- 1 file changed, 79 insertions(+), 39 deletions(-) diff --git a/src/lib/reporter.ts b/src/lib/reporter.ts index 9ef33e4..caaee3c 100644 --- a/src/lib/reporter.ts +++ b/src/lib/reporter.ts @@ -119,26 +119,6 @@ function nvl(v: any, alt: any) { } -function findTopNamedAssertion(ctx: ValidationContext): TypeAssertion | null { - const ret = ctx.typeStack - .slice() - .reverse() - .map(x => Array.isArray(x) ? x[0] : x) - .find(x => x.name && x.name !== x.typeName) || null; - return ret; -} - - -function findTopObjectAssertion(ctx: ValidationContext): ObjectAssertion | null { - const ret = ctx.typeStack - .slice() - .reverse() - .map(x => Array.isArray(x) ? x[0] : x) - .find(x => x.kind === 'object') as ObjectAssertion || null; - return ret; -} - - function findTopRepeatableAssertion(ctx: ValidationContext): TopRepeatable { const ret = ctx.typeStack .slice() @@ -168,7 +148,7 @@ function getExpectedType(ty: TypeAssertion): string { case 'optional': return getExpectedType(ty.optional); case 'one-of': - return `(one of ${ty.oneOf.map(x => getExpectedType(x)).join(', ')}`; + return `(one of ${ty.oneOf.map(x => getExpectedType(x)).join(', ')})`; case 'never': case 'any': case 'unknown': return ty.kind; case 'symlink': @@ -182,7 +162,7 @@ function getExpectedType(ty: TypeAssertion): string { export function formatErrorMessage( msg: string, data: any, ty: TypeAssertion, args: ReportErrorArguments, - values: {dataPath: string, topRepeatable: TopRepeatable, parentType: string}) { + values: {dataPath: string, topRepeatable: TopRepeatable, parentType: string, entryName: string}) { let ret = msg; // TODO: complex type object members' custom error messages are not displayed? @@ -242,11 +222,10 @@ export function formatErrorMessage( 'repeated item of ' : ty.kind !== 'sequence' && values.dataPath.endsWith('sequence)') ? 'sequence item of ' : ''}${ - (ty.name && ty.name !== ty.typeName ? ty.name : null) || - findTopNamedAssertion(args.ctx)?.name || '?'}`)], + values.entryName || '?'}`)], ['parentType', escapeString( - findTopObjectAssertion(args.ctx)?.typeName || ty.typeName || values.parentType || '?')], + values.parentType || '?')], ['dataPath', values.dataPath], @@ -261,6 +240,12 @@ export function formatErrorMessage( } +interface DataPathEntry { + name: string; + kind: 'type' | 'key' | 'index'; +} + + export function reportError( errType: ErrorTypes, data: any, ty: TypeAssertion, args: ReportErrorArguments) { @@ -274,8 +259,8 @@ export function reportError( } messages.push(defaultMessages); - let parentType = ''; - const dataPathArray: string[] = []; + const dataPathEntryArray: DataPathEntry[] = []; + for (let i = 0; i < args.ctx.typeStack.length; i++) { const p = args.ctx.typeStack[i]; const next = args.ctx.typeStack[i + 1]; @@ -286,43 +271,97 @@ export function reportError( if (pt.kind === 'repeated') { if (i !== args.ctx.typeStack.length - 1) { if (pt.name) { - dataPathArray.push(`${pt.name}.(${pi !== void 0 ? `${pi}:` : ''}repeated)`); - } else { - dataPathArray.push(`(repeated)`); + dataPathEntryArray.push({kind: 'key', name: pt.name}); } + dataPathEntryArray.push({kind: 'index', name: `(${pi !== void 0 ? `${pi}:` : ''}repeated)`}); isSet = true; } } else if (pt.kind === 'sequence') { if (i !== args.ctx.typeStack.length - 1) { if (pt.name) { - dataPathArray.push(`${pt.name}.(${pi !== void 0 ? `${pi}:` : ''}sequence)`); - } else { - dataPathArray.push(`(sequence)`); + dataPathEntryArray.push({kind: 'key', name: pt.name}); } + dataPathEntryArray.push({kind: 'index', name: `(${pi !== void 0 ? `${pi}:` : ''}sequence)`}); isSet = true; } } if (! isSet) { if (pt.name) { - dataPathArray.push(`${pt.name}`); + if (i === 0) { + if (pt.typeName) { + dataPathEntryArray.push({kind: 'type', name: pt.typeName}); + } else { + dataPathEntryArray.push({kind: 'key', name: pt.name}); + } + } else { + const len = dataPathEntryArray.length; + if (len && dataPathEntryArray[len - 1].kind === 'type') { + dataPathEntryArray.push({kind: 'key', name: pt.name}); + } else { + if (pt.typeName) { + dataPathEntryArray.push({kind: 'type', name: pt.typeName}); + } else { + dataPathEntryArray.push({kind: 'key', name: pt.name}); + } + } + } } else if (pt.typeName) { - dataPathArray.push(`${pt.typeName}`); + dataPathEntryArray.push({kind: 'type', name: pt.typeName}); } } - if (!parentType && pt.typeName) { - parentType = pt.typeName; + } + + let dataPath = ''; + for (let i = 0; i < dataPathEntryArray.length; i++) { + const p = dataPathEntryArray[i]; + dataPath += p.name; + if (i + 1 === dataPathEntryArray.length) { + break; + } + dataPath += p.kind === 'type' ? '.' : '.'; + } + + let parentType = ''; + let entryName = ''; + for (let i = dataPathEntryArray.length - 1; 0 <= i; i--) { + const p = dataPathEntryArray[i]; + if (p.kind === 'type') { + if (i !== 0) { + const q = dataPathEntryArray[i - 1]; + if (q.kind === 'index') { + continue; + } + } + parentType = p.name; + for (let j = i + 1; j < dataPathEntryArray.length; j++) { + const q = dataPathEntryArray[j]; + if (q.kind === 'key') { + entryName = q.name; + } + } + break; + } + } + if (! parentType) { + for (let i = args.ctx.typeStack.length - 1; 0 <= i; i--) { + const p = args.ctx.typeStack[i]; + const pt = Array.isArray(p) ? p[0] : p; + if (pt.typeName) { + parentType = pt.typeName; + } } } - const dataPath = dataPathArray.join('.'); const topRepeatable: TopRepeatable = findTopRepeatableAssertion(args.ctx); - const values = {dataPath, topRepeatable, parentType}; + const values = {dataPath, topRepeatable, parentType, entryName}; const constraints: TypeAssertionErrorMessageConstraints = {}; const cSrces: TypeAssertionErrorMessageConstraints[] = [ty as any]; + if (errType === ErrorTypes.RepeatQtyUnmatched && topRepeatable) { cSrces.unshift(topRepeatable as any); } + for (const cSrc of cSrces) { if (nvl(cSrc.minValue, false)) { constraints.minValue = cSrc.minValue; @@ -355,6 +394,7 @@ export function reportError( } const val: {value?: any} = {}; + switch (typeof data) { case 'number': case 'bigint': case 'string': case 'boolean': case 'undefined': val.value = data; From c98470445afbdde254b54e8c27046ab25483c37a Mon Sep 17 00:00:00 2001 From: shellyln Date: Wed, 19 Feb 2020 23:29:09 +0900 Subject: [PATCH 2/4] [WIP][FIX] Fix validation reporting. (incorrect parent name) --- src/_spec/fixes-1.spec.ts | 60 +++++++++++++++++++++++++++++++++++++++ src/lib/reporter.ts | 7 +++-- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/_spec/fixes-1.spec.ts b/src/_spec/fixes-1.spec.ts index 8193016..dc23296 100644 --- a/src/_spec/fixes-1.spec.ts +++ b/src/_spec/fixes-1.spec.ts @@ -745,4 +745,64 @@ describe("fix-1", function() { err.message.includes( 'interfaceKey: Unexpected token has appeared. Expect "]".\n')); }); + it("fix-improve-validation-message-1", function() { + const schema = compile(` + interface ACL { + target: string; + value: string; + } + + /** Entry base */ + interface EntryBase { + /** Entry name */ + name: string; + /** ACL infos */ + acl: ACL[]; + } + + /** File entry */ + interface File extends EntryBase { + /** Entry type */ + type: 'file'; + } + + /** Folder entry */ + interface Folder extends EntryBase { + /** Entry type */ + type: 'folder'; + /** Child entries */ + entries: Entry[]; + } + + /** Entry (union type) */ + type Entry = File | Folder; + type Foo = string[]|number; + `); + { + const ctx: Partial = {checkAll: true}; + const z = validate({type: 'file', name: '', acl: [{}]}, getType(schema, 'File'), ctx); + expect(ctx.errors).toEqual([{ + code: 'Required', + message: '"target" of "ACL" is required.', + dataPath: 'File.acl.(0:repeated).ACL.target', + constraints: {}, + }, { + code: 'Required', + message: '"value" of "ACL" is required.', + dataPath: 'File.acl.(0:repeated).ACL.value', + constraints: {}, + }] as any); + } + { + const ctx: Partial = {checkAll: true}; + const z = validate({type: 'file', name: '', acl: [1]}, getType(schema, 'File'), ctx); + expect(ctx.errors).toEqual([{ + code: 'TypeUnmatched', + message: '"acl" of "File" should be type "ACL".', + dataPath: 'File.acl.(0:repeated).ACL', + constraints: {}, + value: 1, + }] as any); + } + }); }); diff --git a/src/lib/reporter.ts b/src/lib/reporter.ts index caaee3c..c3702d1 100644 --- a/src/lib/reporter.ts +++ b/src/lib/reporter.ts @@ -326,17 +326,18 @@ export function reportError( for (let i = dataPathEntryArray.length - 1; 0 <= i; i--) { const p = dataPathEntryArray[i]; if (p.kind === 'type') { - if (i !== 0) { + if (i !== 0 && i === dataPathEntryArray.length - 1) { const q = dataPathEntryArray[i - 1]; if (q.kind === 'index') { - continue; + continue; // e.g.: "File:acl.(0:repeated).ACL" } - } + } // else: "File:acl.(0:repeated).ACL:target" parentType = p.name; for (let j = i + 1; j < dataPathEntryArray.length; j++) { const q = dataPathEntryArray[j]; if (q.kind === 'key') { entryName = q.name; + break; } } break; From 9b146eed67b2a56a38552b292d0d3bd54e8ee799 Mon Sep 17 00:00:00 2001 From: shellyln Date: Thu, 20 Feb 2020 07:39:09 +0900 Subject: [PATCH 3/4] Improve validation reporting. (dataPath separator after 'type') --- CHANGELOG.md | 10 ++++++++++ src/_spec/compiler-2.spec.ts | 4 ++-- src/_spec/compiler-3.spec.ts | 2 +- src/_spec/compiler-7.spec.ts | 32 ++++++++++++++++---------------- src/_spec/fixes-1.spec.ts | 6 +++--- src/lib/reporter.ts | 2 +- 6 files changed, 33 insertions(+), 23 deletions(-) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..8dff32c --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,10 @@ +# Changelog + +## v0.1.7 +### Breaking changes +* Change the `dataPath` format for validation errors. + * Path separator after `type` is changed from `.` to `:`. + * before changed: `File.acl.(0:repeated).ACL.target` + * after changed: `File:acl.(0:repeated).ACL:target` + +--- diff --git a/src/_spec/compiler-2.spec.ts b/src/_spec/compiler-2.spec.ts index b78935f..fcb8b07 100644 --- a/src/_spec/compiler-2.spec.ts +++ b/src/_spec/compiler-2.spec.ts @@ -749,7 +749,7 @@ describe("compiler-2", function() { expect(ctx1.errors).toEqual([{ code: 'InvalidDefinition', message: '"entries" of "Folder" type definition is invalid.', - dataPath: 'Folder.entries.(0:repeated).Entry', + dataPath: 'Folder:entries.(0:repeated).Entry', constraints: {} }]); expect(validate(v, ty, {schema})).toEqual({value: v}); @@ -880,7 +880,7 @@ describe("compiler-2", function() { expect(ctx1.errors).toEqual([{ code: 'InvalidDefinition', message: '"entries" of "Folder" type definition is invalid.', - dataPath: 'Folder.entries.(0:sequence).Entry', + dataPath: 'Folder:entries.(0:sequence).Entry', constraints: {} }]); expect(validate(v, ty, {schema})).toEqual({value: v}); diff --git a/src/_spec/compiler-3.spec.ts b/src/_spec/compiler-3.spec.ts index b4b5090..2c6f74d 100644 --- a/src/_spec/compiler-3.spec.ts +++ b/src/_spec/compiler-3.spec.ts @@ -642,7 +642,7 @@ describe("compiler-3", function() { expect(ctx.errors).toEqual([{ code: 'TypeUnmatched', message: '"D" of "C" should be type "number".', - dataPath: 'C.D', + dataPath: 'C:D', constraints: {}, value: '', }]); diff --git a/src/_spec/compiler-7.spec.ts b/src/_spec/compiler-7.spec.ts index 6f974e7..e92e796 100644 --- a/src/_spec/compiler-7.spec.ts +++ b/src/_spec/compiler-7.spec.ts @@ -551,7 +551,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'Required', message: 'A.a1:required: a1 A', - dataPath: 'A.a1', + dataPath: 'A:a1', constraints: {}, }]); }); @@ -578,7 +578,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'TypeUnmatched', message: 'A.a1:typeUnmatched: a1 A string', - dataPath: 'A.a1', + dataPath: 'A:a1', constraints: {}, value: 1, }]); @@ -607,7 +607,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'TypeUnmatched', message: 'A.a1:typeUnmatched: a1 A string', - dataPath: 'A.a1', + dataPath: 'A:a1', constraints: {}, }]); } @@ -635,7 +635,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'TypeUnmatched', message: 'A.a1:typeUnmatched: a1 A (repeated string)', - dataPath: 'A.a1', + dataPath: 'A:a1', constraints: {}, value: '1', }]); @@ -664,7 +664,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'TypeUnmatched', message: '"repeated item of a1" of "A" should be type "string".', // TODO: - dataPath: 'A.a1.(0:repeated)', + dataPath: 'A:a1.(0:repeated)', constraints: {}, value: 1, }]); @@ -693,7 +693,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'TypeUnmatched', message: 'A.a1:typeUnmatched: a1 A (sequence)', - dataPath: 'A.a1', + dataPath: 'A:a1', constraints: {}, value: '1', }]); @@ -722,7 +722,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'TypeUnmatched', message: '"sequence item of a1" of "A" should be type "string".', // TODO: - dataPath: 'A.a1.(0:sequence)', + dataPath: 'A:a1.(0:sequence)', constraints: {}, value: 1, }]); @@ -753,7 +753,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'ValueRangeUnmatched', message: 'A.a1:valueRangeUnmatched: a1 A 3 5', - dataPath: 'A.a1', + dataPath: 'A:a1', constraints: {minValue: 3, maxValue: 5}, value: 1, }]); @@ -784,7 +784,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'ValueLengthUnmatched', message: 'A.a1:valueLengthUnmatched: a1 A 3 5', - dataPath: 'A.a1', + dataPath: 'A:a1', constraints: {minLength: 3, maxLength: 5}, value: '1', }]); @@ -816,7 +816,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'ValuePatternUnmatched', message: 'A.a1:valuePatternUnmatched: a1 A /^[0-9]+$/', - dataPath: 'A.a1', + dataPath: 'A:a1', constraints: {pattern: '/^[0-9]+$/'}, value: 'A', }]); @@ -849,7 +849,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'ValuePatternUnmatched', message: 'A.a1:valuePatternUnmatched: a1 A /^[0-9]+$/gi', - dataPath: 'A.a1', + dataPath: 'A:a1', constraints: {pattern: '/^[0-9]+$/gi'}, value: 'A', }]); @@ -879,7 +879,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'ValueUnmatched', message: 'A.a1:valueUnmatched: a1 A 5', - dataPath: 'A.a1', + dataPath: 'A:a1', constraints: {}, value: 4, }]); @@ -908,7 +908,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'TypeUnmatched', // TODO: expect SequenceUnmatched? message: '"sequence item of a1" of "A" should be type "string".', // TODO: custom error message is ignored - dataPath: 'A.a1.(0:sequence)', + dataPath: 'A:a1.(0:sequence)', constraints: {min: 3, max: 5}, }]); } @@ -936,7 +936,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'RepeatQtyUnmatched', message: '"sequence item of a1" of "A" should repeat 3..5 times.', // TODO: custom error message is ignored - dataPath: 'A.a1.(2:sequence)', + dataPath: 'A:a1.(2:sequence)', constraints: {min: 3, max: 5}, }]); } @@ -964,7 +964,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'SequenceUnmatched', message: 'A.a1:sequenceUnmatched: a1 A', - dataPath: 'A.a1', + dataPath: 'A:a1', constraints: {}, }]); } @@ -992,7 +992,7 @@ describe("compiler-7", function() { expect(ctx.errors).toEqual([{ code: 'RepeatQtyUnmatched', message: '"sequence item of a1" of "A" should repeat 0..1 times.', // TODO: custom error message is ignored - dataPath: 'A.a1.(2:sequence)', + dataPath: 'A:a1.(2:sequence)', constraints: {}, }]); } diff --git a/src/_spec/fixes-1.spec.ts b/src/_spec/fixes-1.spec.ts index dc23296..021f434 100644 --- a/src/_spec/fixes-1.spec.ts +++ b/src/_spec/fixes-1.spec.ts @@ -784,12 +784,12 @@ describe("fix-1", function() { expect(ctx.errors).toEqual([{ code: 'Required', message: '"target" of "ACL" is required.', - dataPath: 'File.acl.(0:repeated).ACL.target', + dataPath: 'File:acl.(0:repeated).ACL:target', constraints: {}, }, { code: 'Required', message: '"value" of "ACL" is required.', - dataPath: 'File.acl.(0:repeated).ACL.value', + dataPath: 'File:acl.(0:repeated).ACL:value', constraints: {}, }] as any); } @@ -799,7 +799,7 @@ describe("fix-1", function() { expect(ctx.errors).toEqual([{ code: 'TypeUnmatched', message: '"acl" of "File" should be type "ACL".', - dataPath: 'File.acl.(0:repeated).ACL', + dataPath: 'File:acl.(0:repeated).ACL', constraints: {}, value: 1, }] as any); diff --git a/src/lib/reporter.ts b/src/lib/reporter.ts index c3702d1..b215aaa 100644 --- a/src/lib/reporter.ts +++ b/src/lib/reporter.ts @@ -318,7 +318,7 @@ export function reportError( if (i + 1 === dataPathEntryArray.length) { break; } - dataPath += p.kind === 'type' ? '.' : '.'; + dataPath += p.kind === 'type' ? ':' : '.'; } let parentType = ''; From ae00f59c54e5ea133357209b9182094d5b0d6aba Mon Sep 17 00:00:00 2001 From: shellyln Date: Thu, 20 Feb 2020 07:41:09 +0900 Subject: [PATCH 4/4] Edit changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8dff32c..4f48e2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,4 +7,7 @@ * before changed: `File.acl.(0:repeated).ACL.target` * after changed: `File:acl.(0:repeated).ACL:target` +### Changes +* `[FIX]` Fix validation reporting. (incorrect parent name) + ---