diff --git a/index.js b/index.js index 8779e5963..dd95a207b 100644 --- a/index.js +++ b/index.js @@ -14,6 +14,15 @@ module.exports = { Openui5Resolver: require("./lib/ui5Framework/Openui5Resolver"), Sapui5Resolver: require("./lib/ui5Framework/Sapui5Resolver") }, + /** + * @public + * @see module:@ui5/project.validation + * @namespace + */ + validation: { + validator: require("./lib/validation/validator"), + ValidationError: require("./lib/validation/ValidationError") + }, /** * @private * @see module:@ui5/project.translators diff --git a/lib/projectPreprocessor.js b/lib/projectPreprocessor.js index 08217886d..2f007a775 100644 --- a/lib/projectPreprocessor.js +++ b/lib/projectPreprocessor.js @@ -201,9 +201,8 @@ class ProjectPreprocessor { return {}; } - await this.validateExistingProject(project); + await this.validateAndNormalizeExistingProject(project); - this.normalizeConfig(project); return {}; } @@ -538,7 +537,7 @@ class ProjectPreprocessor { Object.assign(project, configShim); delete project.shimDependenciesResolved; // Remove shim processing metadata from project - await this.validateExistingProject(project); + await this.validateAndNormalizeExistingProject(project); } // Apply collections @@ -597,7 +596,7 @@ class ProjectPreprocessor { middlewareRepository.addMiddleware(extension.metadata.name, middlewarePath); } - async validateExistingProject(project) { + async validateAndNormalizeExistingProject(project) { // Validate project config, but exclude additional properties const excludedProperties = [ "id", @@ -618,6 +617,8 @@ class ProjectPreprocessor { id: project.id } }); + + this.normalizeConfig(project); } } diff --git a/lib/validation/ValidationError.js b/lib/validation/ValidationError.js index 1091f89d7..5ddcc87fb 100644 --- a/lib/validation/ValidationError.js +++ b/lib/validation/ValidationError.js @@ -2,18 +2,39 @@ const chalk = require("chalk"); const escapeStringRegExp = require("escape-string-regexp"); const matchAll = require("string.prototype.matchall"); +/** + * Error class for validation of project configuration. + * + * @public + * @hideconstructor + * @extends Error + * @memberof module:@ui5/project.validation + */ class ValidationError extends Error { - constructor({errors, schema, data, project, yaml}) { + constructor({errors, project, yaml}) { super(); + /** + * ValidationError + * + * @const + * @readonly + * @public + */ this.name = "ValidationError"; - this.schema = schema; - this.data = data; this.project = project; this.yaml = yaml; + this.errors = ValidationError.filterErrors(errors); + /** + * Error message + * + * @type {string} + * @readonly + * @public + */ this.message = this.formatErrors(); Error.captureStackTrace(this, this.constructor); @@ -22,6 +43,7 @@ class ValidationError extends Error { formatErrors() { let separator = "\n\n"; if (process.stdout.isTTY) { + // Add a horizontal separator line between errors in case a terminal is used separator += chalk.grey.dim("\u2500".repeat(process.stdout.columns || 80)); } separator += "\n\n"; @@ -53,15 +75,19 @@ class ValidationError extends Error { message += chalk.underline(chalk.red(error.dataPath.substr(1))) + " "; } - if (error.keyword === "additionalProperties") { + switch (error.keyword) { + case "additionalProperties": message += `property ${error.params.additionalProperty} is not expected to be here`; - } else if (error.keyword === "required") { - message += error.message; - } else if (error.keyword === "type") { + break; + case "type": message += `should be of type '${error.params.type}'`; - } else if (error.keyword === "enum") { + break; + case "enum": message += error.message + "\n"; message += "Allowed values: " + error.params.allowedValues.join(", "); + break; + default: + message += error.message; } return message; @@ -94,6 +120,7 @@ class ValidationError extends Error { static analyzeYamlError({error, yaml}) { if (error.dataPath === "" && error.keyword === "required") { + // There is no line/column for a missing required property on root level return {line: -1, column: -1}; } @@ -104,6 +131,9 @@ class ValidationError extends Error { objectPath.push(error.params.additionalProperty); } + const matchArrayElement = /(^|\n)([ ]*-[^\n]*)/g; + const matchArrayElementIndentation = /([ ]*)-/; + let currentIndex = 0; let currentSubstring = yaml.source; for (let i = 0; i < objectPath.length; i++) { @@ -111,20 +141,28 @@ class ValidationError extends Error { let newIndex; if (isNaN(property)) { + // Try to find a property + + // Creating a regular expression that matches the property name a line + // except for comments, indicated by a hash sign "#". const propertyRegExp = new RegExp(`^[^#]*?${escapeStringRegExp(property)}`, "m"); + const propertyMatch = propertyRegExp.exec(currentSubstring); if (!propertyMatch) { return {line: -1, column: -1}; } newIndex = propertyMatch.index + propertyMatch[0].length; } else { + // Try to find the right index within an array definition. + // This currently only works for arrays defined with "-" in multiple lines. + // Arrays using square brackets are not supported. + const arrayIndex = parseInt(property); - const matchArrayElement = /(^|\n)([ ]*-[^\n]*)/g; const arrayIndicators = matchAll(currentSubstring, matchArrayElement); let a = 0; let firstIndentation = -1; for (const match of arrayIndicators) { - const indentationMatch = match[2].match(/([ ]*)-/); + const indentationMatch = match[2].match(matchArrayElementIndentation); if (!indentationMatch) { return {line: -1, column: -1}; } diff --git a/lib/validation/schema/specVersion/2.0/kind/extension.json b/lib/validation/schema/specVersion/2.0/kind/extension.json index a44c3cff8..d49c9613d 100644 --- a/lib/validation/schema/specVersion/2.0/kind/extension.json +++ b/lib/validation/schema/specVersion/2.0/kind/extension.json @@ -23,7 +23,8 @@ "if": { "properties": { "type": {"const": null} - } + }, + "$comment": "Using 'if' with null and empty 'then' to ensure no other schemas are applied when the property is not set. Otherwise the first 'if' condition might still be met, causing unexpected errors." }, "then": {}, "else": { @@ -52,16 +53,6 @@ }, "then": { "$ref": "extension/project-shim.json" - }, - "else": { - "if": { - "properties": { - "type": {"const": null} - } - }, - "then": { - - } } } } diff --git a/lib/validation/schema/specVersion/2.0/kind/project.json b/lib/validation/schema/specVersion/2.0/kind/project.json index 208547d5e..53c6fefb5 100644 --- a/lib/validation/schema/specVersion/2.0/kind/project.json +++ b/lib/validation/schema/specVersion/2.0/kind/project.json @@ -25,7 +25,8 @@ "if": { "properties": { "type": {"const": null} - } + }, + "$comment": "Using 'if' with null and empty 'then' to ensure no other schemas are applied when the property is not set. Otherwise the first 'if' condition might still be met, causing unexpected errors." }, "then": {}, "else": { diff --git a/lib/validation/validator.js b/lib/validation/validator.js index ef804f818..f65190500 100644 --- a/lib/validation/validator.js +++ b/lib/validation/validator.js @@ -12,6 +12,10 @@ async function loadSchema(schemaPath) { return JSON.parse(schemaFile); } +/** + * @private + * @memberof module:@ui5/project.validation + */ class Validator { constructor() { this.ajv = new Ajv({ @@ -40,7 +44,6 @@ class Validator { throw new ValidationError({ errors: fnValidate.errors, schema: fnValidate.schema, - data: config, project, yaml }); @@ -52,10 +55,30 @@ class Validator { const validator = new Validator(); +/** + * @public + * @namespace + * @alias module:@ui5/project.validation.validator + */ module.exports = { + /** + * Validates the given configuration. + * + * @param {Object} options + * @param {Object} options.config UI5 Configuration to validate + * @param {Object} options.project Project information + * @param {string} options.project.id ID of the project + * @param {Object} [options.yaml] YAML information + * @param {string} options.yaml.path Path of the YAML file + * @param {string} options.yaml.source Content of the YAML file + * @param {number} [options.yaml.documentIndex=0] Document index in case the YAML file contains multiple documents + * @throws {module:@ui5/project.validation.ValidationError} + * Throws a {@link module:@ui5/project.validation.ValidationError ValidationError} when the validation fails. + * @returns {undefined} Returns when the validation succeeds + * @public + */ validate: (options) => { return validator.validate(options); }, - ValidationError, _Validator: Validator // For testing only }; diff --git a/package-lock.json b/package-lock.json index e7d4d6dc6..246b79c12 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1719,6 +1719,129 @@ } } }, + "chokidar-cli": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/chokidar-cli/-/chokidar-cli-2.1.0.tgz", + "integrity": "sha512-6n21AVpW6ywuEPoxJcLXMA2p4T+SLjWsXKny/9yTWFz0kKxESI3eUylpeV97LylING/27T/RVTY0f2/0QaWq9Q==", + "dev": true, + "requires": { + "chokidar": "^3.2.3", + "lodash.debounce": "^4.0.8", + "lodash.throttle": "^4.1.1", + "yargs": "^13.3.0" + }, + "dependencies": { + "ansi-regex": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-4.1.0.tgz", + "integrity": "sha512-1apePfXM1UOSqw0o9IiFAovVz9M5S1Dg+4TrDwfMewQ6p/rmMueb7tWZjQ1rx4Loy1ArBggoqGpfqqdI4rondg==", + "dev": true + }, + "ansi-styles": { + "version": "3.2.1", + "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-3.2.1.tgz", + "integrity": "sha512-VT0ZI6kZRdTh8YyJw3SMbYm/u+NqfsAxEpWO0Pf9sq8/e94WxxOpPKx9FR1FlyCtOVDNOQ+8ntlqFxiRc+r5qA==", + "dev": true, + "requires": { + "color-convert": "^1.9.0" + } + }, + "camelcase": { + "version": "5.3.1", + "resolved": "https://registry.npmjs.org/camelcase/-/camelcase-5.3.1.tgz", + "integrity": "sha512-L28STB170nwWS63UjtlEOE3dldQApaJXZkOI1uMFfzf3rRuPegHaHesyee+YxQ+W6SvRDQV6UrdOdRiR153wJg==", + "dev": true + }, + "cliui": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/cliui/-/cliui-5.0.0.tgz", + "integrity": "sha512-PYeGSEmmHM6zvoef2w8TPzlrnNpXIjTipYK780YswmIP9vjxmd6Y2a3CB2Ks6/AU8NHjZugXvo8w3oWM2qnwXA==", + "dev": true, + "requires": { + "string-width": "^3.1.0", + "strip-ansi": "^5.2.0", + "wrap-ansi": "^5.1.0" + } + }, + "emoji-regex": { + "version": "7.0.3", + "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-7.0.3.tgz", + "integrity": "sha512-CwBLREIQ7LvYFB0WyRvwhq5N5qPhc6PMjD6bYggFlI5YyDgl+0vxq5VHbMOFqLg7hfWzmu8T5Z1QofhmTIhItA==", + "dev": true + }, + "get-caller-file": { + "version": "2.0.5", + "resolved": "https://registry.npmjs.org/get-caller-file/-/get-caller-file-2.0.5.tgz", + "integrity": "sha512-DyFP3BM/3YHTQOCUL/w0OZHR0lpKeGrxotcHWcqNEdnltqFwXVfhEBQ94eIo34AfQpo0rGki4cyIiftY06h2Fg==", + "dev": true + }, + "is-fullwidth-code-point": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-2.0.0.tgz", + "integrity": "sha1-o7MKXE8ZkYMWeqq5O+764937ZU8=", + "dev": true + }, + "string-width": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/string-width/-/string-width-3.1.0.tgz", + "integrity": "sha512-vafcv6KjVZKSgz06oM/H6GDBrAtz8vdhQakGjFIvNrHA6y3HCF1CInLy+QLq8dTJPQ1b+KDUqDFctkdRW44e1w==", + "dev": true, + "requires": { + "emoji-regex": "^7.0.1", + "is-fullwidth-code-point": "^2.0.0", + "strip-ansi": "^5.1.0" + } + }, + "strip-ansi": { + "version": "5.2.0", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-5.2.0.tgz", + "integrity": "sha512-DuRs1gKbBqsMKIZlrffwlug8MHkcnpjs5VPmL1PAh+mA30U0DTotfDZ0d2UUsXpPmPmMMJ6W773MaA3J+lbiWA==", + "dev": true, + "requires": { + "ansi-regex": "^4.1.0" + } + }, + "wrap-ansi": { + "version": "5.1.0", + "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-5.1.0.tgz", + "integrity": "sha512-QC1/iN/2/RPVJ5jYK8BGttj5z83LmSKmvbvrXPNCLZSEb32KKVDJDl/MOt2N01qU2H/FkzEa9PKto1BqDjtd7Q==", + "dev": true, + "requires": { + "ansi-styles": "^3.2.0", + "string-width": "^3.0.0", + "strip-ansi": "^5.0.0" + } + }, + "yargs": { + "version": "13.3.2", + "resolved": "https://registry.npmjs.org/yargs/-/yargs-13.3.2.tgz", + "integrity": "sha512-AX3Zw5iPruN5ie6xGRIDgqkT+ZhnRlZMLMHAs8tg7nRruy2Nb+i5o9bwghAogtM08q1dpr2LVoS8KSTMYpWXUw==", + "dev": true, + "requires": { + "cliui": "^5.0.0", + "find-up": "^3.0.0", + "get-caller-file": "^2.0.1", + "require-directory": "^2.1.1", + "require-main-filename": "^2.0.0", + "set-blocking": "^2.0.0", + "string-width": "^3.0.0", + "which-module": "^2.0.0", + "y18n": "^4.0.0", + "yargs-parser": "^13.1.2" + } + }, + "yargs-parser": { + "version": "13.1.2", + "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-13.1.2.tgz", + "integrity": "sha512-3lbsNRf/j+A4QuSZfDRA7HRSfWrzO0YjqTJd5kjAq37Zep1CEgaYmrH9Q3GwPiB9cHyd1Y1UwggGhJGoxipbzg==", + "dev": true, + "requires": { + "camelcase": "^5.0.0", + "decamelize": "^1.2.0" + } + } + } + }, "chownr": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/chownr/-/chownr-1.1.4.tgz", diff --git a/test/lib/projectPreprocessor.js b/test/lib/projectPreprocessor.js index ee28524c5..2420aa932 100644 --- a/test/lib/projectPreprocessor.js +++ b/test/lib/projectPreprocessor.js @@ -4,7 +4,7 @@ const mock = require("mock-require"); const path = require("path"); const gracefulFs = require("graceful-fs"); const validator = require("../../lib/validation/validator"); -const {ValidationError} = validator; +const ValidationError = require("../../lib/validation/ValidationError"); const projectPreprocessor = require("../../lib/projectPreprocessor"); const applicationAPath = path.join(__dirname, "..", "fixtures", "application.a"); const applicationBPath = path.join(__dirname, "..", "fixtures", "application.b"); diff --git a/test/lib/validation/ValidationError.js b/test/lib/validation/ValidationError.js index 80f65692a..7f9fedfff 100644 --- a/test/lib/validation/ValidationError.js +++ b/test/lib/validation/ValidationError.js @@ -363,7 +363,6 @@ test.serial("ValidationError.analyzeYamlError: Array", (t) => { "analyzeYamlError should return expected results"); }); - test.serial("ValidationError.analyzeYamlError: Nested array", (t) => { const error = {dataPath: "/items/2/subItems/1"}; const yaml = { @@ -389,6 +388,42 @@ test.serial("ValidationError.analyzeYamlError: Nested array", (t) => { "analyzeYamlError should return expected results"); }); +test.serial("ValidationError.analyzeYamlError: Array with square brackets (not supported)", (t) => { + const error = {dataPath: "/items/2"}; + const yaml = { + path: "/my-project/ui5.yaml", + source: +`items: [1, 2, 3] +`, + documentIndex: 0 + }; + + const info = ValidationError.analyzeYamlError({error, yaml}); + + t.deepEqual(info, {line: -1, column: -1}, + "analyzeYamlError should return expected results"); +}); + +test.serial("ValidationError.analyzeYamlError: Multiline array with square brackets (not supported)", (t) => { + const error = {dataPath: "/items/2"}; + const yaml = { + path: "/my-project/ui5.yaml", + source: +`items: [ + 1, + 2, + 3 +] +`, + documentIndex: 0 + }; + + const info = ValidationError.analyzeYamlError({error, yaml}); + + t.deepEqual(info, {line: -1, column: -1}, + "analyzeYamlError should return expected results"); +}); + test.serial("ValidationError.analyzeYamlError: Nested property with comments", (t) => { const error = {dataPath: "/property1/property2/property3/property4"}; const yaml = { diff --git a/test/lib/validation/schema/ui5.js b/test/lib/validation/schema/ui5.js index ad2019e9b..168b6e97c 100644 --- a/test/lib/validation/schema/ui5.js +++ b/test/lib/validation/schema/ui5.js @@ -18,18 +18,20 @@ async function assertValidation(t, config, expectedErrors = undefined) { test.before((t) => { t.context.validator = new Validator(); - t.context.ajvCoverage = new AjvCoverage(t.context.validator.ajv); + t.context.ajvCoverage = new AjvCoverage(t.context.validator.ajv, { + includes: ["schema/ui5.json"] + }); }); test.after.always((t) => { t.context.ajvCoverage.createReport("html", {dir: "coverage/ajv-ui5"}); - // const thresholds = { - // statements: 58, - // branches: 50, - // functions: 100, - // lines: 58 - // }; - // t.context.ajvCoverage.verify(thresholds); + const thresholds = { + statements: 58, + branches: 50, + functions: 100, + lines: 58 + }; + t.context.ajvCoverage.verify(thresholds); }); test("Undefined", async (t) => {