Skip to content

Commit

Permalink
chore: simplify tracing of known type and required in nested rules
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ChALkeR committed Jan 29, 2024
1 parent 025b477 commit 8057cf2
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 20 deletions.
1 change: 0 additions & 1 deletion doc/samples/draft-next/dynamicRef.md
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,6 @@ const ref0 = function validate(data, dynAnchors = []) {
const dynLocal = [{}]
dynLocal[0]["#meta"] = validate
if (!ref1(data, [...dynAnchors, dynLocal[0] || []])) return false
if (!(typeof data === "object" && data && !Array.isArray(data))) return false
if (data.foo !== undefined && hasOwn(data, "foo")) {
if (!(data.foo === "pass")) return false
}
Expand Down
2 changes: 0 additions & 2 deletions doc/samples/draft2020-12/dynamicRef.md
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,6 @@ const ref0 = function validate(data, dynAnchors = []) {
const dynLocal = [{}]
dynLocal[0]["#meta"] = validate
if (!ref1(data, [...dynAnchors, dynLocal[0] || []])) return false
if (!(typeof data === "object" && data && !Array.isArray(data))) return false
if (data.foo !== undefined && hasOwn(data, "foo")) {
if (!(data.foo === "pass")) return false
}
Expand Down Expand Up @@ -618,7 +617,6 @@ const ref0 = function validate(data, dynAnchors = []) {
const dynLocal = [{}]
dynLocal[0]["#meta"] = validate
if (!ref1(data, [...dynAnchors, dynLocal[0] || []])) return false
if (!(typeof data === "object" && data && !Array.isArray(data))) return false
if (data.foo !== undefined && hasOwn(data, "foo")) {
if (!(data.foo === "pass")) return false
}
Expand Down
24 changes: 10 additions & 14 deletions src/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,9 @@ const compileSchema = (schema, root, opts, scope, basePathRoot = '') => {
const visit = (errors, history, current, node, schemaPath, trace = {}, { constProp } = {}) => {
// e.g. top-level data and property names, OR already checked by present() in history, OR in keys and not undefined
const isSub = history.length > 0 && history[history.length - 1].prop === current
const queryCurrent = () => history.filter((h) => h.prop === current)
const statHistory = history.filter((h) => h.prop === current).map((h) => h.stat) // nested stat objects only for the current node
const definitelyPresent =
!current.parent || current.checked || (current.inKeys && isJSON) || queryCurrent().length > 0
!current.parent || current.checked || (current.inKeys && isJSON) || statHistory.length > 0

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

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

// Trust already applied { type, required } restrictions from parent rules for the current node
// Can't apply items/properties as those affect child unevaluated*, and { fullstring } is just not needed in same-node subrules
for (const { type, required } of statHistory) evaluateDelta({ type, required })

// This is used for typechecks, null means * here
const allIn = (arr, valid) => arr && arr.every((s) => valid.includes(s)) // all arr entries are in valid
const someIn = (arr, possible) => possible.some((x) => arr === null || arr.includes(x)) // all possible are in arrs

const parentCheckedType = (...valid) => queryCurrent().some((h) => allIn(h.stat.type, valid))
const definitelyType = (...valid) => allIn(stat.type, valid) || parentCheckedType(...valid)
const typeApplicable = (...possible) =>
someIn(stat.type, possible) && queryCurrent().every((h) => someIn(h.stat.type, possible))
const definitelyType = (...valid) => allIn(stat.type, valid)
const typeApplicable = (...possible) => someIn(stat.type, possible)

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

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

const checkObjects = () => {
const propertiesCount = format('Object.keys(%s).length', name)
Expand Down Expand Up @@ -1284,10 +1283,7 @@ const compileSchema = (schema, root, opts, scope, basePathRoot = '') => {
evaluateDelta({ type: [current.type] })
return null
}
if (parentCheckedType(...typearr)) {
evaluateDelta({ type: typearr })
return null
}
if (definitelyType(...typearr)) return null
const filteredTypes = typearr.filter((t) => typeApplicable(t))
if (filteredTypes.length === 0) fail('No valid types possible')
evaluateDelta({ type: typearr }) // can be safely done here, filteredTypes already prepared
Expand Down
4 changes: 1 addition & 3 deletions test/regressions/177.js → test/regressions/179.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const tape = require('tape')
const { validator } = require('../../')

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

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

if (l0.type && (!l2a.type || !l2b.type)) continue // Bug, fixed by #179

t.doesNotThrow(() => {
const validate = validator({
required: ['type'],
Expand Down

0 comments on commit 8057cf2

Please sign in to comment.