diff --git a/src/cli-validator/runValidator.js b/src/cli-validator/runValidator.js index e10dd6ea7..2ba838674 100644 --- a/src/cli-validator/runValidator.js +++ b/src/cli-validator/runValidator.js @@ -18,6 +18,7 @@ const printError = require('./utils/printError'); const preprocessFile = require('./utils/preprocessFile'); const spectralValidator = require('../spectral/utils/spectral-validator'); const dedupFunction = require('../cli-validator/utils/noDeduplication'); +const addPathsToComponents = require('./utils/addPathsToComponents'); const { Spectral } = require('@stoplight/spectral'); // import the init module for creating a .validaterc file const init = require('./utils/init.js'); @@ -48,7 +49,7 @@ const processInput = async function(program) { const limitsFileOverride = program.limits; - const printRuleNames = program.verbose > 0; + const verbose = program.verbose > 0; // turn off coloring if explicitly requested if (turnOffColoring) { @@ -314,6 +315,10 @@ const processInput = async function(program) { } } + if (verbose) { + addPathsToComponents(results, swagger.jsSpec); + } + if (jsonOutput) { printJson(results, originalFile, errorsOnly); } else { @@ -322,7 +327,7 @@ const processInput = async function(program) { results, chalk, printValidators, - printRuleNames, + verbose, reportingStats, originalFile, errorsOnly diff --git a/src/cli-validator/utils/addPathsToComponents.js b/src/cli-validator/utils/addPathsToComponents.js new file mode 100644 index 000000000..84e125c7e --- /dev/null +++ b/src/cli-validator/utils/addPathsToComponents.js @@ -0,0 +1,100 @@ +const { each } = require('lodash'); +const getPathAsArray = require('./getPathAsArray'); + +// takes validator results dictionary in the format: +// { +// errors: { +// validator-name: [ +// { +// path: [path, to, error], +// message: 'error message', +// rule: rule_name +// }, +// ], +// }, +// warnings: ... +// } +// +// adds path to the originating component if it exists +// modifies the given results object +module.exports = function(originalResults, unresolvedSpec) { + each(originalResults, function(validatorsDict) { + each(validatorsDict, function(errors) { + pointToComponents(errors, unresolvedSpec); + }); + }); +}; + +// takes an array of validator errors: +// [ +// { +// path: [path, to, error], +// message: 'error message', +// rule: rule_name +// }, +// ] +// +// adds componentPath field to existing errors +// modifies existing errors in place +function pointToComponents(errors, unresolvedSpec) { + each(errors, function(err) { + const pathArray = getPathAsArray(err.path); + let componentPath = null; + let refObj = findRef(pathArray, unresolvedSpec); + while (refObj.refString !== null) { + componentPath = [ + ...parseRefString(refObj.refString), + ...refObj.remainingPath + ]; + refObj = findRef(componentPath, unresolvedSpec); + } + // only add the componentPath if the componentPath exists + // and it is the result of a valid path. + // protects against invalid $refs from the user and against + // bugs in the path to the error. + if (componentPath && refObj.validPath) { + err.componentPath = componentPath; + } + }); +} + +function findRef(pathArray, unresolvedSpec) { + let ref = null; + let indexOfRef = 0; + let currentObj = unresolvedSpec; + let validPath = true; + for (let i = 0; i < pathArray.length && currentObj; ++i) { + let nextKey = pathArray[i]; + if (Array.isArray(currentObj)) { + // convert the key to an index + nextKey = Number(nextKey); + if (!(nextKey >= 0 && nextKey < currentObj.length)) { + validPath = false; + break; + } + } else if (!(nextKey in currentObj)) { + if ('$ref' in currentObj) { + ref = currentObj['$ref']; + indexOfRef = i; + } else if (i != pathArray.length - 1) { + // nextKey is not in the object + // no $ref in the object + // nextKey is not the last element of the path + // hence, the given path is invalid + validPath = false; + } + break; + } + currentObj = currentObj[nextKey]; + } + return { + refString: ref, + remainingPath: pathArray.slice(indexOfRef, pathArray.length), + validPath: validPath + }; +} + +function parseRefString(refString) { + const refArray = refString.split('/'); + return refArray.slice(refArray.indexOf('#') + 1, refArray.length); +} diff --git a/src/cli-validator/utils/getPathAsArray.js b/src/cli-validator/utils/getPathAsArray.js new file mode 100644 index 000000000..628c2509c --- /dev/null +++ b/src/cli-validator/utils/getPathAsArray.js @@ -0,0 +1,3 @@ +module.exports = function(path) { + return Array.isArray(path) ? path : path.split('.'); +}; diff --git a/src/cli-validator/utils/printResults.js b/src/cli-validator/utils/printResults.js index dae4f896b..89d5baee9 100644 --- a/src/cli-validator/utils/printResults.js +++ b/src/cli-validator/utils/printResults.js @@ -1,5 +1,6 @@ const each = require('lodash/each'); const pad = require('pad'); +const getPathAsArray = require('./getPathAsArray'); // get line-number-producing, 'magic' code from Swagger Editor const getLineNumberForPath = require(__dirname + '/../../plugins/ast/ast') @@ -10,7 +11,7 @@ module.exports = function print( results, chalk, printValidators, - printRuleNames, + verbose, reportingStats, originalFile, errorsOnly @@ -88,11 +89,22 @@ module.exports = function print( // print the path array as a dot-separated string console.log(chalk[color](` Message : ${problem.message}`)); - if (printRuleNames && problem.rule) { + if (verbose && problem.rule) { console.log(chalk[color](` Rule : ${problem.rule}`)); } console.log(chalk[color](` Path : ${path.join('.')}`)); console.log(chalk[color](` Line : ${lineNumber}`)); + if (verbose && problem.componentPath) { + const componentPath = getPathAsArray(problem.componentPath); + const componentLine = getLineNumberForPath( + originalFile, + componentPath + ); + console.log( + chalk[color](` Component Path : ${componentPath.join('.')}`) + ); + console.log(chalk[color](` Component Line : ${componentLine}`)); + } console.log(); }); }); diff --git a/test/cli-validator/tests/expected-output.test.js b/test/cli-validator/tests/expected-output.test.js index 417caba45..646a52694 100644 --- a/test/cli-validator/tests/expected-output.test.js +++ b/test/cli-validator/tests/expected-output.test.js @@ -332,8 +332,8 @@ describe('test expected output - OpenAPI 3', function() { program.default_mode = true; program.json = true; - await commandLineValidator(program); - // exit code is not set for JSON output + const exitCode = await commandLineValidator(program); + expect(exitCode).toEqual(1); const capturedText = getCapturedText(consoleSpy.mock.calls); const jsonOutput = JSON.parse(capturedText); @@ -372,4 +372,19 @@ describe('test expected output - OpenAPI 3', function() { expect(msg).toHaveProperty('rule') ); }); + + it('should include the path to the component (if it exists) when in verbose mode', async function() { + const program = {}; + program.args = ['./test/cli-validator/mockFiles/oas3/testoneof.yaml']; + program.default_mode = true; + program.verbose = 1; + + const exitCode = await commandLineValidator(program); + expect(exitCode).toEqual(1); + + const capturedText = getCapturedText(consoleSpy.mock.calls); + const allText = capturedText.join(); + expect(allText).toContain('Component Path'); + expect(allText).toContain('Component Line'); + }); }); diff --git a/test/cli-validator/tests/utils/addPathsToComponents.test.js b/test/cli-validator/tests/utils/addPathsToComponents.test.js new file mode 100644 index 000000000..47977c30a --- /dev/null +++ b/test/cli-validator/tests/utils/addPathsToComponents.test.js @@ -0,0 +1,369 @@ +const addPathsToComponents = require('../../../../src/cli-validator/utils/addPathsToComponents'); + +describe('test postprocessing util - test component path finding', function() { + it('should correctly add component paths', function() { + const unresolvedSpec = { + paths: { + '/path1': { + get: { + responses: { + '200': { + $ref: '#/components/responses/GenericResponse' + }, + '201': { + $ref: '#/components/responses/GenericResponse' + } + } + } + } + }, + components: { + responses: { + GenericResponse: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/GenericSchema' + } + } + } + } + }, + schemas: { + GenericSchema: { + type: 'object', + properties: { + stringProp: { + type: 'string' + } + } + } + } + } + }; + // mimicing errors from a resolved spec + const originalResults = { + errors: { + 'validator-name': [ + { + message: 'simple error message', + path: + 'paths./path1.get.responses.200.content.application/json.schema.properties.stringProp', + rule: 'made_up_rule' + }, + { + message: 'simple error message', + path: + 'paths./path1.get.responses.201.content.application/json.schema.properties.stringProp', + rule: 'made_up_rule' + } + ] + } + }; + addPathsToComponents(originalResults, unresolvedSpec); + const errors = originalResults.errors['validator-name']; + expect(errors.length).toBe(2); + expect(errors[0].componentPath).toEqual([ + 'components', + 'schemas', + 'GenericSchema', + 'properties', + 'stringProp' + ]); + expect(errors[1].componentPath).toEqual([ + 'components', + 'schemas', + 'GenericSchema', + 'properties', + 'stringProp' + ]); + }); + + it('should correctly add component paths when an array index is in the path', function() { + const unresolvedSpec = { + paths: { + '/path1': { + post: { + requestBody: { + $ref: '#/components/requestBodies/OneOfRequest' + } + } + } + }, + components: { + requestBodies: { + OneOfRequest: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/OneOfSchema' + } + } + } + } + }, + schemas: { + OneOfSchema: { + oneOf: [ + { + $ref: '#/components/schemas/Cat' + }, + { + $ref: '#/components/schemas/Dog' + } + ] + }, + Cat: { + type: 'object', + properties: { + meows: { + type: 'boolean' + } + } + }, + Dog: { + type: 'object', + properties: { + barks: { + type: 'boolean' + } + } + } + } + } + }; + // mimicing errors from a resolved spec + const originalResults = { + errors: { + 'validator-name': [ + { + message: 'simple error message', + path: + 'paths./path1.post.requestBody.content.application/json.schema.oneOf.0.properties.meows', + rule: 'made_up_rule' + }, + { + message: 'simple error message with array path', + path: [ + 'paths', + '/path1', + 'post', + 'requestBody', + 'content', + 'application/json', + 'schema', + 'oneOf', + '0', + 'properties', + 'meows' + ], + rule: 'made_up_rule' + }, + { + message: 'simple error message with array path with a number index', + path: [ + 'paths', + '/path1', + 'post', + 'requestBody', + 'content', + 'application/json', + 'schema', + 'oneOf', + 0, + 'properties', + 'meows' + ], + rule: 'made_up_rule' + } + ] + } + }; + addPathsToComponents(originalResults, unresolvedSpec); + const errors = originalResults.errors['validator-name']; + expect(errors.length).toBe(3); + expect(errors[0].componentPath).toEqual([ + 'components', + 'schemas', + 'Cat', + 'properties', + 'meows' + ]); + expect(errors[1].componentPath).toEqual([ + 'components', + 'schemas', + 'Cat', + 'properties', + 'meows' + ]); + expect(errors[2].componentPath).toEqual([ + 'components', + 'schemas', + 'Cat', + 'properties', + 'meows' + ]); + }); + + it('should not add a component path when the error path is invalid', function() { + const unresolvedSpec = { + paths: { + '/path1': { + get: { + responses: { + '200': { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/GenericSchema' + } + } + } + } + } + } + } + }, + components: { + schemas: { + GenericSchema: { + type: 'string' + } + } + } + }; + // invalid error path + const originalResults = { + errors: { + 'validator-name': [ + { + message: 'simple error message', + path: + 'paths./path1.get.requestBody.content.application/json.schema', + rule: 'made_up_rule' + } + ] + } + }; + addPathsToComponents(originalResults, unresolvedSpec); + const errors = originalResults.errors['validator-name']; + expect(errors.length).toBe(1); + expect(errors[0].componentPath).toBeUndefined(); + }); + + it('should not add a component path when the ref is invalid', function() { + const unresolvedSpec = { + paths: { + '/path1': { + get: { + responses: { + '200': { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/SchemaDoesntExist' + } + } + } + } + } + } + } + }, + components: { + schemas: { + GenericSchema: { + type: 'object', + properties: { + stringProp: { + type: 'string' + } + } + } + } + } + }; + + // invalid error path + const originalResults = { + errors: { + 'validator-name': [ + { + message: 'simple error message', + path: + 'paths./path1.get.responses.200.content.application/json.schema.stringProp', + rule: 'made_up_rule' + } + ] + } + }; + + addPathsToComponents(originalResults, unresolvedSpec); + const errors = originalResults.errors['validator-name']; + expect(errors.length).toBe(1); + expect(errors[0].componentPath).toBeUndefined(); + }); + + it('should not add a component path when the ref is invalid', function() { + const unresolvedSpec = { + swagger: '2.0', + paths: { + '/pet': { + post: { + description: 'post a pet to store', + operationId: 'opId', + produces: ['application/json'], + parameters: [ + { + in: 'body', + name: 'body', + description: 'Pet object that needs to be added to the store', + required: true, + schema: { + $ref: '#/definitions/Pet' + } + } + ] + } + } + }, + definitions: { + Pet: { + type: 'object', + properties: { + id: { + type: 'integer', + format: 'int64' + }, + name: { + type: 'string', + description: 'string' + } + } + } + } + }; + + const originalResults = { + errors: { + 'validator-name': [ + { + message: 'simple error message', + path: 'paths./pet.post.parameters.0.schema.properties.id', + rule: 'made_up_rule' + } + ] + } + }; + + addPathsToComponents(originalResults, unresolvedSpec); + const errors = originalResults.errors['validator-name']; + expect(errors.length).toBe(1); + expect(errors[0].componentPath).toEqual([ + 'definitions', + 'Pet', + 'properties', + 'id' + ]); + }); +});