From b7b4c033a5a41cd8a6bf989a38272e8ca169dc16 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Wed, 8 Sep 2021 15:05:16 -0700 Subject: [PATCH] check-openapi: Use yaml library for better error messages. Signed-off-by: Anders Kaseorg --- package.json | 2 +- tools/check-openapi | 118 +++++++++++++++++++++++--------------------- version.py | 2 +- yarn.lock | 7 ++- 4 files changed, 70 insertions(+), 59 deletions(-) diff --git a/package.json b/package.json index 77a6f12811..10cffd12ce 100644 --- a/package.json +++ b/package.json @@ -106,7 +106,6 @@ "eslint-import-resolver-webpack": "^0.13.0", "eslint-plugin-import": "^2.22.0", "eslint-plugin-unicorn": "^35.0.0", - "js-yaml": "^4.0.0", "jsdom": "^17.0.0", "mockdate": "^3.0.2", "node-fetch": "^2.6.1", @@ -125,6 +124,7 @@ "vnu-jar": "^21.2.5", "webpack-dev-server": "^3.5.1", "xvfb": "^0.4.0", + "yaml": "^2.0.0-8", "yarn-deduplicate": "^3.0.0", "zulip-js": "^2.0.8" }, diff --git a/tools/check-openapi b/tools/check-openapi index cf4cc9a39b..952d5e467b 100755 --- a/tools/check-openapi +++ b/tools/check-openapi @@ -4,73 +4,79 @@ const fs = require("fs"); -const jsyaml = require("js-yaml"); const ExampleValidator = require("openapi-examples-validator"); const SwaggerParser = require("swagger-parser"); +const {LineCounter, Scalar, YAMLMap, YAMLSeq, parseDocument, visit} = require("yaml"); + +async function checkFile(file) { + const yaml = await fs.promises.readFile(file, "utf8"); + const lineCounter = new LineCounter(); + const doc = parseDocument(yaml, {lineCounter}); + if (doc.errors.length > 0) { + for (const error of doc.errors) { + console.error("%s: %s", file, error.message); + } + process.exitCode = 1; + return; + } + + const root = doc.contents; + if (!(root instanceof YAMLMap && root.has("openapi"))) { + return; + } -function checkRefSiblings(file, path, data) { let ok = true; - if (typeof data === "object" && data !== null) { - if ( - "allOf" in data && - Object.values(data.allOf).filter((subschema) => !("$ref" in subschema)).length > 1 - ) { - console.error( - `${file}: Too many inline allOf subschemas at ${JSON.stringify( - path, - )}: ${JSON.stringify(data, undefined, 2)}`, - ); - ok = false; - } - if ("$ref" in data && Object.entries(data).length !== 1) { - console.error( - `${file}: Siblings of $ref have no effect at ${JSON.stringify( - path, - )}: ${JSON.stringify(data, undefined, 2)}`, - ); - ok = false; - } - for (const [key, child] of Array.isArray(data) ? data.entries() : Object.entries(data)) { - if (!checkRefSiblings(file, [...path, key], child)) { + + visit(doc, { + Map(key, node) { + if (node.has("$ref") && node.items.length !== 1) { + const {line, col} = lineCounter.linePos(node.range[0]); + console.error("%s:%d:%d: Siblings of $ref have no effect", file, line, col); ok = false; } - } + }, + + Pair(key, node) { + if ( + node.key instanceof Scalar && + node.key.value === "allOf" && + node.value instanceof YAMLSeq && + node.value.items.filter( + (subschema) => !(subschema instanceof YAMLMap && subschema.has("$ref")), + ).length > 1 + ) { + const {line, col} = lineCounter.linePos(node.value.range[0]); + console.error("%s:%d:%d: Too many inline allOf subschemas", file, line, col); + ok = false; + } + }, + }); + + if (!ok) { + process.exitCode = 1; + } + + try { + await SwaggerParser.validate(file); + } catch (error) { + if (!(error instanceof SyntaxError)) { + throw error; + } + console.error("%s: %s", file, error.message); + process.exitCode = 1; + } + const res = await ExampleValidator.validateFile(file); + if (!res.valid) { + for (const error of res.errors) { + console.error(error); + } + process.exitCode = 1; } - return ok; } (async () => { - // Iterate through the changed files, passed in the arguments. - // The two first arguments are the call to the Node interpreter and this - // script, hence the starting point at 2. for (const file of process.argv.slice(2)) { - try { - const data = jsyaml.load(await fs.promises.readFile(file, "utf8"), { - filename: file, - }); - if (data.openapi !== undefined) { - if (!checkRefSiblings(file, [], data)) { - process.exitCode = 1; - } - await SwaggerParser.validate(file); - const res = await ExampleValidator.validateFile(file); - if (!res.valid) { - for (const error of res.errors) { - console.error(error); - } - process.exitCode = 1; - } - } - } catch (error) { - if (error instanceof jsyaml.YAMLException) { - console.error(error.message); - } else if (error instanceof SyntaxError) { - console.error(`${file}: ${error.message}`); - } else { - throw error; - } - process.exitCode = 1; - } + await checkFile(file); } })().catch((error) => { console.error(error); diff --git a/version.py b/version.py index 8b27eac55f..e26f90ab55 100644 --- a/version.py +++ b/version.py @@ -48,4 +48,4 @@ API_FEATURE_LEVEL = 96 # historical commits sharing the same major version, in which case a # minor version bump suffices. -PROVISION_VERSION = "157.1" +PROVISION_VERSION = "157.2" diff --git a/yarn.lock b/yarn.lock index a65ad91827..3ef8369da5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8349,7 +8349,7 @@ js-yaml@^3.13.1: argparse "^1.0.7" esprima "^4.0.0" -js-yaml@^4.0.0, js-yaml@^4.1.0: +js-yaml@^4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-4.1.0.tgz#c1fb65f8f5017901cdd2c951864ba18458a10602" integrity sha512-wpxZs9NoxZaJESJGIZTyDEaYpl0FKSA+FB9aJiyemKhMwkxQg63h4T1KJgUGHpTqPDNRcmmYLugrRjJlBtWvRA== @@ -14926,6 +14926,11 @@ yaml@^1.10.0, yaml@^1.10.2, yaml@^1.7.2: resolved "https://registry.yarnpkg.com/yaml/-/yaml-1.10.2.tgz#2301c5ffbf12b467de8da2333a459e29e7920e4b" integrity sha512-r3vXyErRCYJ7wg28yvBY5VSoAF8ZvlcW9/BwUzEtUsjvX/DKs24dIkuwjtuprwJJHsbyUbLApepYTR1BN4uHrg== +yaml@^2.0.0-8: + version "2.0.0-8" + resolved "https://registry.yarnpkg.com/yaml/-/yaml-2.0.0-8.tgz#226365f0d804ba7fb8cc2b527a00a7a4a3d8ea5f" + integrity sha512-QaYgJZMfWD6fKN/EYMk6w1oLWPCr1xj9QaPSZW5qkDb3y8nGCXhy2Ono+AF4F+CSL/vGcqswcAT0BaS//pgD2A== + yargs-parser@^13.1.2: version "13.1.2" resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-13.1.2.tgz#130f09702ebaeef2650d54ce6e3e5706f7a4fb38"