From 25b5dc5ecd2f187fe3fed8ed5f45553084f0f317 Mon Sep 17 00:00:00 2001 From: Evgeny Poberezkin <2769109+epoberezkin@users.noreply.github.com> Date: Mon, 1 Feb 2021 18:36:15 +0000 Subject: [PATCH] Fix for issue #1361 - duplicate functions in standalone code with mutually recursive schemas (#1418) * test: skipped failing test showing duplicate functions in standalone code for mutually recursive schemas, #1361 * fix: duplicate functions in standalone code for mutually recursive schemas, fixes #1361 --- lib/compile/codegen/scope.ts | 18 ++++- lib/standalone/index.ts | 32 +++++--- spec/standalone.spec.ts | 143 +++++++++++++++++++++++++---------- 3 files changed, 138 insertions(+), 55 deletions(-) diff --git a/lib/compile/codegen/scope.ts b/lib/compile/codegen/scope.ts index f6a6a0548..511992297 100644 --- a/lib/compile/codegen/scope.ts +++ b/lib/compile/codegen/scope.ts @@ -42,6 +42,15 @@ export type ScopeValueSets = { [Prefix in string]?: Set } +export enum UsedValueState { + Started, + Completed, +} + +export type UsedScopeValues = { + [Prefix in string]?: Map +} + export const varKinds = { const: new Name("const"), let: new Name("let"), @@ -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( @@ -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 @@ -198,6 +207,7 @@ export class ValueScope extends Scope { } else { throw new ValueError(name) } + nameSet.set(name, UsedValueState.Completed) }) } return code diff --git a/lib/standalone/index.ts b/lib/standalone/index.ts index 27a4bca1b..8cfca1da0 100644 --- a/lib/standalone/index.ts +++ b/lib/standalone/index.ts @@ -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, @@ -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}` @@ -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) @@ -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}`) @@ -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) + } } } diff --git a/spec/standalone.spec.ts b/spec/standalone.spec.ts index fff7b2000..a0f6fd22e 100644 --- a/spec/standalone.spec.ts +++ b/spec/standalone.spec.ts @@ -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)) + }) }) })