Skip to content

Commit

Permalink
Fix for issue ajv-validator#1361 - duplicate functions in standalone …
Browse files Browse the repository at this point in the history 
…code with mutually recursive schemas (ajv-validator#1418)

* test: skipped failing test showing duplicate functions in standalone code for mutually recursive schemas, ajv-validator#1361

* fix: duplicate functions in standalone code for mutually recursive schemas, fixes ajv-validator#1361
  • Loading branch information
@epoberezkin @andriyl
epoberezkin authored and andriyl committed Jun 16, 2021
1 parent 7b0acbc commit 25b5dc5
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 55 deletions.
18 changes: 14 additions & 4 deletions lib/compile/codegen/scope.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ export type ScopeValueSets = {
[Prefix in string]?: Set<ValueScopeName>
}

export enum UsedValueState {
Started,
Completed,
}

export type UsedScopeValues = {
[Prefix in string]?: Map<ValueScopeName, UsedValueState | undefined>
}

export const varKinds = {
const: new Name("const"),
let: new Name("let"),
Expand Down Expand Up @@ -161,7 +170,7 @@ export class ValueScope extends Scope {

scopeCode(
values: ScopeValues | ScopeValueSets = this._values,
usedValues?: ScopeValueSets,
usedValues?: UsedScopeValues,
getCode?: (n: ValueScopeName) => Code | undefined
): Code {
return this._reduceValues(
Expand All @@ -178,17 +187,17 @@ export class ValueScope extends Scope {
private _reduceValues(
values: ScopeValues | ScopeValueSets,
valueCode: (n: ValueScopeName) => Code | undefined,
usedValues: ScopeValueSets = {},
usedValues: UsedScopeValues = {},
getCode?: (n: ValueScopeName) => Code | undefined
): Code {
let code: Code = nil
for (const prefix in values) {
const vs = values[prefix]
if (!vs) continue
const nameSet = (usedValues[prefix] = usedValues[prefix] || new Set())
const nameSet = (usedValues[prefix] = usedValues[prefix] || new Map())
vs.forEach((name: ValueScopeName) => {
if (nameSet.has(name)) return
nameSet.add(name)
nameSet.set(name, UsedValueState.Started)
let c = valueCode(name)
if (c) {
const def = this.opts.es5 ? varKinds.var : varKinds.const
Expand All @@ -198,6 +207,7 @@ export class ValueScope extends Scope {
} else {
throw new ValueError(name)
}
nameSet.set(name, UsedValueState.Completed)
})
}
return code
Expand Down
32 changes: 22 additions & 10 deletions lib/standalone/index.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type AjvCore from "../core"
import type {AnyValidateFunction, SourceCode} from "../types"
import type {SchemaEnv} from "../compile"
import {ScopeValueSets, ValueScopeName, varKinds} from "../compile/codegen/scope"
import {_, _Code, Code, getProperty} from "../compile/codegen/code"
import {UsedScopeValues, UsedValueState, ValueScopeName, varKinds} from "../compile/codegen/scope"
import {_, nil, _Code, Code, getProperty} from "../compile/codegen/code"

export default function standaloneCode(
ajv: AjvCore,
Expand All @@ -27,7 +27,7 @@ export default function standaloneCode(
}

function funcExportCode(source?: SourceCode): string {
const usedValues: ScopeValueSets = {}
const usedValues: UsedScopeValues = {}
const n = source?.validateName
const vCode = validateCode(usedValues, source)
return `"use strict";${_n}module.exports = ${n};${_n}module.exports.default = ${n};${_n}${vCode}`
Expand All @@ -37,7 +37,7 @@ export default function standaloneCode(
schemas: {[K in string]?: T},
getValidateFunc: (schOrId: T) => AnyValidateFunction | undefined
): string {
const usedValues: ScopeValueSets = {}
const usedValues: UsedScopeValues = {}
let code = _`"use strict";`
for (const name in schemas) {
const v = getValidateFunc(schemas[name] as T)
Expand All @@ -49,11 +49,10 @@ export default function standaloneCode(
return `${code}`
}

function validateCode(usedValues: ScopeValueSets, s?: SourceCode): Code {
function validateCode(usedValues: UsedScopeValues, s?: SourceCode): Code {
if (!s) throw new Error('moduleCode: function does not have "source" property')
const {prefix} = s.validateName
const nameSet = (usedValues[prefix] = usedValues[prefix] || new Set())
nameSet.add(s.validateName)
if (usedState(s.validateName) === UsedValueState.Completed) return nil
setUsedState(s.validateName, UsedValueState.Started)

const scopeCode = ajv.scope.scopeCode(s.scopeValues, usedValues, refValidateCode)
const code = new _Code(`${scopeCode}${_n}${s.validateCode}`)
Expand All @@ -66,11 +65,24 @@ export default function standaloneCode(
return validateCode(usedValues, v.source)
} else if ((n.prefix === "root" || n.prefix === "wrapper") && typeof vRef == "object") {
const {validate, validateName} = vRef as SchemaEnv
const vCode = validateCode(usedValues, validate?.source)
if (!validateName) throw new Error("ajv internal error")
const def = ajv.opts.code.es5 ? varKinds.var : varKinds.const
return _`${def} ${n} = {validate: ${validateName}};${_n}${vCode}`
const wrapper = _`${def} ${n} = {validate: ${validateName}};`
if (usedState(validateName) === UsedValueState.Started) return wrapper
const vCode = validateCode(usedValues, validate?.source)
return _`${wrapper}${_n}${vCode}`
}
return undefined
}

function usedState(name: ValueScopeName): UsedValueState | undefined {
return usedValues[name.prefix]?.get(name)
}

function setUsedState(name: ValueScopeName, state: UsedValueState): void {
const {prefix} = name
const names = (usedValues[prefix] = usedValues[prefix] || new Map())
names.set(name, state)
}
}
}
143 changes: 102 additions & 41 deletions spec/standalone.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,57 +86,118 @@ describe("standalone code generation", () => {
}
})

describe("two refs to the same schema (issue #1361)", () => {
const userSchema = {
$id: "user.json",
type: "object",
properties: {
name: {type: "string"},
},
required: ["name"],
}
describe("issue #1361", () => {
describe("two refs to the same schema", () => {
const userSchema = {
$id: "user.json",
type: "object",
properties: {
name: {type: "string"},
},
required: ["name"],
}

const infoSchema = {
$id: "info.json",
type: "object",
properties: {
author: {$ref: "user.json"},
contributors: {
type: "array",
items: {$ref: "user.json"},
const infoSchema = {
$id: "info.json",
type: "object",
properties: {
author: {$ref: "user.json"},
contributors: {
type: "array",
items: {$ref: "user.json"},
},
},
},
required: ["author", "contributors"],
}
required: ["author", "contributors"],
}

describe("all exports", () => {
it("should not have duplicate functions", () => {
const ajv = new _Ajv({
allErrors: true,
code: {optimize: false, source: true},
inlineRefs: false, // it is needed to show the issue, schemas with refs won't be inlined anyway
schemas: [userSchema, infoSchema],
describe("all exports", () => {
it("should not have duplicate functions", () => {
const ajv = new _Ajv({
allErrors: true,
code: {optimize: false, source: true},
inlineRefs: false, // it is needed to show the issue, schemas with refs won't be inlined anyway
schemas: [userSchema, infoSchema],
})

const moduleCode = standaloneCode(ajv)
assertNoDuplicateFunctions(moduleCode)
const {"user.json": user, "info.json": info} = requireFromString(moduleCode)
testExports({user, info})
})
})

const moduleCode = standaloneCode(ajv)
assertNoDuplicateFunctions(moduleCode)
const {"user.json": user, "info.json": info} = requireFromString(moduleCode)
testExports({user, info})
describe("named exports", () => {
it("should not have duplicate functions", () => {
const ajv = new _Ajv({
allErrors: true,
code: {optimize: false, source: true},
inlineRefs: false, // it is needed to show the issue, schemas with refs won't be inlined anyway
schemas: [userSchema, infoSchema],
})

const moduleCode = standaloneCode(ajv, {user: "user.json", info: "info.json"})
assertNoDuplicateFunctions(moduleCode)
testExports(requireFromString(moduleCode))
})
})
})

describe("named exports", () => {
it("should not have duplicate functions", () => {
const ajv = new _Ajv({
allErrors: true,
code: {optimize: false, source: true},
inlineRefs: false, // it is needed to show the issue, schemas with refs won't be inlined anyway
schemas: [userSchema, infoSchema],
describe("mutually recursive schemas", () => {
const userSchema = {
$id: "user.json",
type: "object",
properties: {
name: {type: "string"},
infos: {
type: "array",
items: {$ref: "info.json"},
},
},
required: ["name"],
}

const infoSchema = {
$id: "info.json",
type: "object",
properties: {
author: {$ref: "user.json"},
contributors: {
type: "array",
items: {$ref: "user.json"},
},
},
required: ["author", "contributors"],
}

describe("all exports", () => {
it("should not have duplicate functions", () => {
const ajv = new _Ajv({
allErrors: true,
code: {optimize: false, source: true},
inlineRefs: false, // it is needed to show the issue, schemas with refs won't be inlined anyway
schemas: [userSchema, infoSchema],
})

const moduleCode = standaloneCode(ajv)
assertNoDuplicateFunctions(moduleCode)
const {"user.json": user, "info.json": info} = requireFromString(moduleCode)
testExports({user, info})
})
})

const moduleCode = standaloneCode(ajv, {user: "user.json", info: "info.json"})
assertNoDuplicateFunctions(moduleCode)
testExports(requireFromString(moduleCode))
describe("named exports", () => {
it("should not have duplicate functions", () => {
const ajv = new _Ajv({
allErrors: true,
code: {optimize: false, source: true},
inlineRefs: false, // it is needed to show the issue, schemas with refs won't be inlined anyway
schemas: [userSchema, infoSchema],
})

const moduleCode = standaloneCode(ajv, {user: "user.json", info: "info.json"})
assertNoDuplicateFunctions(moduleCode)
testExports(requireFromString(moduleCode))
})
})
})

Expand Down

0 comments on commit 25b5dc5

Please sign in to comment.