Skip to content

Commit f28dd6f

Browse files
authored
chore: simplify tracing of known type and required in nested rules (#179)
E.g. in allOf/oneOf, we just rely on the checks in parent rule, but only for type/required checks. In the following schema, the nested allOf will just check for `{ required: ['yyy'] }`: ``` { type: 'object', required: ['xxx','zzz'], allOf: [{ type: 'object', required: ['xxx', 'yyy'] }] } ``` Properties/items can't be inherited as they affect evaluation checks, which should be isolated from the parent.
1 parent 8adc130 commit f28dd6f

File tree

4 files changed

+11
-20
lines changed

4 files changed

+11
-20
lines changed

doc/samples/draft-next/dynamicRef.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,6 @@ const ref0 = function validate(data, dynAnchors = []) {
296296
const dynLocal = [{}]
297297
dynLocal[0]["#meta"] = validate
298298
if (!ref1(data, [...dynAnchors, dynLocal[0] || []])) return false
299-
if (!(typeof data === "object" && data && !Array.isArray(data))) return false
300299
if (data.foo !== undefined && hasOwn(data, "foo")) {
301300
if (!(data.foo === "pass")) return false
302301
}

doc/samples/draft2020-12/dynamicRef.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,6 @@ const ref0 = function validate(data, dynAnchors = []) {
553553
const dynLocal = [{}]
554554
dynLocal[0]["#meta"] = validate
555555
if (!ref1(data, [...dynAnchors, dynLocal[0] || []])) return false
556-
if (!(typeof data === "object" && data && !Array.isArray(data))) return false
557556
if (data.foo !== undefined && hasOwn(data, "foo")) {
558557
if (!(data.foo === "pass")) return false
559558
}
@@ -618,7 +617,6 @@ const ref0 = function validate(data, dynAnchors = []) {
618617
const dynLocal = [{}]
619618
dynLocal[0]["#meta"] = validate
620619
if (!ref1(data, [...dynAnchors, dynLocal[0] || []])) return false
621-
if (!(typeof data === "object" && data && !Array.isArray(data))) return false
622620
if (data.foo !== undefined && hasOwn(data, "foo")) {
623621
if (!(data.foo === "pass")) return false
624622
}

src/compile.js

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,9 @@ const compileSchema = (schema, root, opts, scope, basePathRoot = '') => {
204204
const visit = (errors, history, current, node, schemaPath, trace = {}, { constProp } = {}) => {
205205
// e.g. top-level data and property names, OR already checked by present() in history, OR in keys and not undefined
206206
const isSub = history.length > 0 && history[history.length - 1].prop === current
207-
const queryCurrent = () => history.filter((h) => h.prop === current)
207+
const statHistory = history.filter((h) => h.prop === current).map((h) => h.stat) // nested stat objects only for the current node
208208
const definitelyPresent =
209-
!current.parent || current.checked || (current.inKeys && isJSON) || queryCurrent().length > 0
209+
!current.parent || current.checked || (current.inKeys && isJSON) || statHistory.length > 0
210210

211211
const name = buildName(current)
212212
const currPropImm = (...args) => propimm(current, ...args)
@@ -452,14 +452,15 @@ const compileSchema = (schema, root, opts, scope, basePathRoot = '') => {
452452

453453
/* Preparation and methods, post-$ref validation will begin at the end of the function */
454454

455+
// Trust already applied { type, required } restrictions from parent rules for the current node
456+
// Can't apply items/properties as those affect child unevaluated*, and { fullstring } is just not needed in same-node subrules
457+
for (const { type, required } of statHistory) evaluateDelta({ type, required })
458+
455459
// This is used for typechecks, null means * here
456460
const allIn = (arr, valid) => arr && arr.every((s) => valid.includes(s)) // all arr entries are in valid
457461
const someIn = (arr, possible) => possible.some((x) => arr === null || arr.includes(x)) // all possible are in arrs
458-
459-
const parentCheckedType = (...valid) => queryCurrent().some((h) => allIn(h.stat.type, valid))
460-
const definitelyType = (...valid) => allIn(stat.type, valid) || parentCheckedType(...valid)
461-
const typeApplicable = (...possible) =>
462-
someIn(stat.type, possible) && queryCurrent().every((h) => someIn(h.stat.type, possible))
462+
const definitelyType = (...valid) => allIn(stat.type, valid)
463+
const typeApplicable = (...possible) => someIn(stat.type, possible)
463464

464465
const enforceRegex = (source, target = node) => {
465466
enforce(typeof source === 'string', 'Invalid pattern:', source)
@@ -756,9 +757,7 @@ const compileSchema = (schema, root, opts, scope, basePathRoot = '') => {
756757
}
757758

758759
// if allErrors is false, we can skip present check for required properties validated before
759-
const checked = (p) =>
760-
!allErrors &&
761-
(stat.required.includes(p) || queryCurrent().some((h) => h.stat.required.includes(p)))
760+
const checked = (p) => !allErrors && stat.required.includes(p)
762761

763762
const checkObjects = () => {
764763
const propertiesCount = format('Object.keys(%s).length', name)
@@ -1284,10 +1283,7 @@ const compileSchema = (schema, root, opts, scope, basePathRoot = '') => {
12841283
evaluateDelta({ type: [current.type] })
12851284
return null
12861285
}
1287-
if (parentCheckedType(...typearr)) {
1288-
evaluateDelta({ type: typearr })
1289-
return null
1290-
}
1286+
if (definitelyType(...typearr)) return null
12911287
const filteredTypes = typearr.filter((t) => typeApplicable(t))
12921288
if (filteredTypes.length === 0) fail('No valid types possible')
12931289
evaluateDelta({ type: typearr }) // can be safely done here, filteredTypes already prepared

test/regressions/177.js renamed to test/regressions/179.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
const tape = require('tape')
44
const { validator } = require('../../')
55

6-
tape('regression #177', (t) => {
6+
tape('regression #177 + #179', (t) => {
77
const variants = [{ type: 'object' }, {}]
88

99
for (const l0 of variants) {
@@ -15,8 +15,6 @@ tape('regression #177', (t) => {
1515
if (!l0.type && !l1b.type && !l2a.type) continue
1616
if (!l0.type && !l1b.type && !l2b.type) continue
1717

18-
if (l0.type && (!l2a.type || !l2b.type)) continue // Bug, fixed by #179
19-
2018
t.doesNotThrow(() => {
2119
const validate = validator({
2220
required: ['type'],

0 commit comments

Comments
 (0)