diff --git a/packages/validator/src/cli-validator/utils/print-results.js b/packages/validator/src/cli-validator/utils/print-results.js index daf5ec3e9..1703075c8 100644 --- a/packages/validator/src/cli-validator/utils/print-results.js +++ b/packages/validator/src/cli-validator/utils/print-results.js @@ -38,6 +38,7 @@ module.exports = function print(context, results) { console.log(chalk[color](` Message : ${result.message}`)); console.log(chalk[color](` Rule : ${result.rule}`)); console.log(chalk[color](` Path : ${result.path.join('.')}`)); + console.log(chalk[color](` DocLink : ${result.docLink}`)); console.log(chalk[color](` Line : ${result.line}`)); console.log(''); }); diff --git a/packages/validator/src/markdown-report/report.js b/packages/validator/src/markdown-report/report.js index 71071ab2a..3dcc0999b 100644 --- a/packages/validator/src/markdown-report/report.js +++ b/packages/validator/src/markdown-report/report.js @@ -34,8 +34,14 @@ inherently weighted by the scoring algorithm, so that security violations are 5 as usability violations, evolution 3 times, and robustness 2 times. ## Scoring information +
+ +Information about how the scoring gets calculated + ${scoringData(results)} +
+ ## Error summary ${errorSummary(results)} diff --git a/packages/validator/src/markdown-report/tables/rule-violation-details.js b/packages/validator/src/markdown-report/tables/rule-violation-details.js index f73051dfa..ecb13f80c 100644 --- a/packages/validator/src/markdown-report/tables/rule-violation-details.js +++ b/packages/validator/src/markdown-report/tables/rule-violation-details.js @@ -5,24 +5,102 @@ const MarkdownTable = require('../markdown-table'); -function getTable({ error, warning }) { - const table = new MarkdownTable( - 'Rule', - 'Message', - 'Path', - 'Line', - 'Severity' - ); - - error.results.forEach(({ message, path, rule, line }) => { - table.addRow(rule, message, path.join('.'), line, 'error'); +function splitMessage(message) { + const colonIdx = message.indexOf(':'); + if (colonIdx === -1) { + return { base: message, detail: null }; + } + return { + base: message.slice(0, colonIdx).trim(), + detail: message.slice(colonIdx + 1).trim() || null, + }; +} + +function groupResultsByRule(results) { + const groupedByRule = {}; + + results.forEach(result => { + const { base, detail } = splitMessage(result.message); + + if (!groupedByRule[result.rule]) { + groupedByRule[result.rule] = { + base, + docLink: result.docLink, + violations: [], + }; + } + + const pathStr = result.path.join('.'); + + if ( + !groupedByRule[result.rule].violations.some( + v => v.line === result.line && v.path === pathStr + ) + ) { + groupedByRule[result.rule].violations.push({ + line: result.line, + path: pathStr, + detail, + }); + } }); - warning.results.forEach(({ message, path, rule, line }) => { - table.addRow(rule, message, path.join('.'), line, 'warning'); + return groupedByRule; +} + +function renderRuleGroups(groupedByRule) { + const sections = []; + + Object.entries(groupedByRule).forEach(([rule, data]) => { + // Create heading with link + sections.push(`### [${rule}](${data.docLink})`); + sections.push(''); + + // Add the base message as italic description + sections.push(`_${data.base}_`); + sections.push(''); + + // Use a 3-col table when any violation carries extra detail + const hasDetail = data.violations.some(v => v.detail); + const table = hasDetail + ? new MarkdownTable('Line', 'Path', 'Detail') + : new MarkdownTable('Line', 'Path'); + + data.violations.forEach(({ line, path, detail }) => { + if (hasDetail) { + table.addRow(line, path, detail); + } else { + table.addRow(line, path); + } + }); + + sections.push(table.render()); + sections.push(''); }); - return table.render(); + return sections.join('\n'); +} + +function getTable({ error, warning }) { + const sections = []; + + // Process errors + if (error.results.length > 0) { + sections.push('### Errors'); + sections.push(''); + const errorGroups = groupResultsByRule(error.results); + sections.push(renderRuleGroups(errorGroups)); + } + + // Process warnings + if (warning.results.length > 0) { + sections.push('### Warnings'); + sections.push(''); + const warningGroups = groupResultsByRule(warning.results); + sections.push(renderRuleGroups(warningGroups)); + } + + return sections.join('\n').trim(); } module.exports = getTable; diff --git a/packages/validator/src/schemas/results-object.yaml b/packages/validator/src/schemas/results-object.yaml index 168e1a41b..5c4ff9b5b 100644 --- a/packages/validator/src/schemas/results-object.yaml +++ b/packages/validator/src/schemas/results-object.yaml @@ -125,6 +125,7 @@ $defs: - message - path - rule + - docLink - line properties: message: @@ -139,6 +140,9 @@ $defs: rule: type: string description: The rule identifier in the Spectral ruleset + docLink: + type: string + description: The documentation link to the problem discovered in the API line: type: integer description: The line number in the original file that the problem @@ -169,3 +173,4 @@ $defs: type: number description: A number describing the demerit impact a rule has on an API minimum: 0.01 + diff --git a/packages/validator/src/spectral/index.js b/packages/validator/src/spectral/index.js index e10bae1a4..9868085dd 100644 --- a/packages/validator/src/spectral/index.js +++ b/packages/validator/src/spectral/index.js @@ -87,6 +87,7 @@ function convertResults(spectralResults, { config, logger }) { message: r.message, path: r.path, rule: r.code, + docLink: getDocumentationURL(r.code), line: r.range.start.line + 1, }); @@ -224,3 +225,15 @@ function convertSpectralSeverity(s) { const mapping = { 0: 'error', 1: 'warning', 2: 'info', 3: 'hint' }; return mapping[s]; } + +function getDocumentationURL(ruleCode) { + if (ruleCode.includes('ibm')) { + const baseUrl = + 'https://github.com/IBM/openapi-validator/blob/main/docs/ibm-cloud-rules.md'; + return `${baseUrl}#${ruleCode}`; + } else { + const baseUrl = + 'https://meta.stoplight.io/docs/spectral/4dec24461f3af-open-api-rules'; + return `${baseUrl}#${ruleCode}`; + } +} diff --git a/packages/validator/test/cli-validator/tests/expected-output.test.js b/packages/validator/test/cli-validator/tests/expected-output.test.js index 9d27a4e65..14c201488 100644 --- a/packages/validator/test/cli-validator/tests/expected-output.test.js +++ b/packages/validator/test/cli-validator/tests/expected-output.test.js @@ -87,39 +87,39 @@ describe('Expected output tests', function () { // errors const errorStart = 3; - expect(capturedText[errorStart + 5].match(/\S+/g)[2]).toEqual('52'); - expect(capturedText[errorStart + 10].match(/\S+/g)[2]).toEqual('96'); - expect(capturedText[errorStart + 15].match(/\S+/g)[2]).toEqual('103'); + expect(capturedText[errorStart + 6].match(/\S+/g)[2]).toEqual('52'); + expect(capturedText[errorStart + 12].match(/\S+/g)[2]).toEqual('96'); + expect(capturedText[errorStart + 18].match(/\S+/g)[2]).toEqual('103'); // Specifically verify that the no-$ref-siblings error occurred. // We do this because this rule is inherited from Spectral's oas ruleset, // but we modify the rule definition in ibmoas.js so that it is run // against both OpenAPI 3.0 and 3.1 documents. - expect(capturedText[errorStart + 17].split(':')[1].trim()).toEqual( + expect(capturedText[errorStart + 20].split(':')[1].trim()).toEqual( '$ref must not be placed next to any other properties' ); - expect(capturedText[errorStart + 18].split(':')[1].trim()).toEqual( + expect(capturedText[errorStart + 21].split(':')[1].trim()).toEqual( 'no-$ref-siblings' ); - expect(capturedText[errorStart + 19].split(':')[1].trim()).toEqual( + expect(capturedText[errorStart + 22].split(':')[1].trim()).toEqual( 'components.schemas.Pet.properties.category.description' ); - expect(capturedText[errorStart + 20].match(/\S+/g)[2]).toEqual('184'); + expect(capturedText[errorStart + 24].match(/\S+/g)[2]).toEqual('184'); - // warnings - const warningStart = 25; + // warnings - each warning block is now 6 lines (5 content + 1 blank) + const warningStart = 30; expect(capturedText[warningStart + 5].match(/\S+/g)[2]).toEqual('22'); - expect(capturedText[warningStart + 10].match(/\S+/g)[2]).toEqual('24'); - expect(capturedText[warningStart + 15].match(/\S+/g)[2]).toEqual('40'); - expect(capturedText[warningStart + 20].match(/\S+/g)[2]).toEqual('41'); - expect(capturedText[warningStart + 25].match(/\S+/g)[2]).toEqual('52'); - expect(capturedText[warningStart + 30].match(/\S+/g)[2]).toEqual('56'); - expect(capturedText[warningStart + 35].match(/\S+/g)[2]).toEqual('57'); - expect(capturedText[warningStart + 40].match(/\S+/g)[2]).toEqual('59'); - expect(capturedText[warningStart + 45].match(/\S+/g)[2]).toEqual('61'); - expect(capturedText[warningStart + 50].match(/\S+/g)[2]).toEqual('96'); - // Skip a few, then verify the last one. - expect(capturedText[warningStart + 145].match(/\S+/g)[2]).toEqual( + expect(capturedText[warningStart + 11].match(/\S+/g)[2]).toEqual('24'); + expect(capturedText[warningStart + 17].match(/\S+/g)[2]).toEqual('40'); + expect(capturedText[warningStart + 23].match(/\S+/g)[2]).toEqual('41'); + expect(capturedText[warningStart + 29].match(/\S+/g)[2]).toEqual('52'); + expect(capturedText[warningStart + 35].match(/\S+/g)[2]).toEqual('56'); + expect(capturedText[warningStart + 41].match(/\S+/g)[2]).toEqual('57'); + expect(capturedText[warningStart + 47].match(/\S+/g)[2]).toEqual('59'); + expect(capturedText[warningStart + 53].match(/\S+/g)[2]).toEqual('61'); + expect(capturedText[warningStart + 59].match(/\S+/g)[2]).toEqual('96'); + // Skip a few, then verify the last one (29 warnings × 6 lines = 174 lines) + expect(capturedText[warningStart + 173].match(/\S+/g)[2]).toEqual( '210' ); } @@ -214,7 +214,10 @@ describe('Expected output tests', function () { [] ); - allMessages.forEach(msg => expect(msg).toHaveProperty('rule')); + allMessages.forEach(msg => { + expect(msg).toHaveProperty('rule'); + expect(msg).toHaveProperty('docLink'); + }); } ); @@ -244,6 +247,7 @@ describe('Expected output tests', function () { const warningToCheck = jsonOutput.warning.results[0]; expect(warningToCheck.rule).toEqual('ibm-prefer-token-pagination'); + expect(warningToCheck.docLink).toContain('ibm-prefer-token-pagination'); expect(warningToCheck.path.join('.')).toBe('paths./letters.get'); expect(warningToCheck.line).toEqual(20); } diff --git a/packages/validator/test/markdown-report/report.test.js b/packages/validator/test/markdown-report/report.test.js index 5877f713d..debfe4a8e 100644 --- a/packages/validator/test/markdown-report/report.test.js +++ b/packages/validator/test/markdown-report/report.test.js @@ -18,7 +18,7 @@ describe('getReport tests', function () { // Check all subtitle-level headers. const headers = report .split('\n') - .filter(l => l.startsWith('##')) + .filter(l => /^## /.test(l)) .map(l => l.slice(3)); expect(headers).toEqual([ 'Quick view', diff --git a/packages/validator/test/markdown-report/tables/rule-violation-details.test.js b/packages/validator/test/markdown-report/tables/rule-violation-details.test.js index f140dc1ca..f61e15e6b 100644 --- a/packages/validator/test/markdown-report/tables/rule-violation-details.test.js +++ b/packages/validator/test/markdown-report/tables/rule-violation-details.test.js @@ -7,20 +7,138 @@ const { ruleViolationDetails } = require('../../../src/markdown-report/tables'); const validatorResults = require('../../test-utils/mock-json-output.json'); describe('ruleViolationDetails table tests', function () { - it('should produce a table with all rule violations from the results', function () { - const tableRows = ruleViolationDetails(validatorResults).split('\n'); + it('should produce grouped sections for errors and warnings', function () { + const output = ruleViolationDetails(validatorResults); + const lines = output.split('\n'); - expect(tableRows).toHaveLength(5); - expect(tableRows[0]).toBe('| Rule | Message | Path | Line | Severity |'); - expect(tableRows[1]).toBe('| --- | --- | --- | --- | --- |'); - expect(tableRows[2]).toBe( - '| ibm-no-consecutive-path-parameter-segments | Path contains two or more consecutive path parameter references: /pets/{pet_id}/{id} | paths./pets/{pet_id}/{id} | 84 | error |' + // Errors heading + expect(lines[0]).toBe('### Errors'); + expect(lines[1]).toBe(''); + + // 3-col table + expect(lines[2]).toBe( + '### [ibm-no-consecutive-path-parameter-segments](https://github.com/IBM/openapi-validator/blob/main/docs/ibm-cloud-rules.md#ibm-no-consecutive-path-parameter-segments)' + ); + expect(lines[3]).toBe(''); + expect(lines[4]).toBe( + '_Path contains two or more consecutive path parameter references_' + ); + expect(lines[5]).toBe(''); + expect(lines[6]).toBe('| Line | Path | Detail |'); + expect(lines[7]).toBe('| --- | --- | --- |'); + expect(lines[8]).toBe( + '| 84 | paths./pets/{pet_id}/{id} | /pets/{pet_id}/{id} |' + ); + expect(lines[9]).toBe(''); + + // 2-col table + expect(lines[10]).toBe( + '### [ibm-integer-attributes](https://github.com/IBM/openapi-validator/blob/main/docs/ibm-cloud-rules.md#ibm-integer-attributes)' ); - expect(tableRows[3]).toBe( - "| ibm-integer-attributes | Integer schemas should define property 'minimum' | components.schemas.Pet.properties.id | 133 | error |" + expect(lines[11]).toBe(''); + expect(lines[12]).toBe( + "_Integer schemas should define property 'minimum'_" ); - expect(tableRows[4]).toBe( - "| ibm-anchored-patterns | A regular expression used in a 'pattern' attribute should be anchored with ^ and $ | components.schemas.Error.properties.message.pattern | 233 | warning |" + expect(lines[13]).toBe(''); + expect(lines[14]).toBe('| Line | Path |'); + expect(lines[15]).toBe('| --- | --- |'); + expect(lines[16]).toBe('| 133 | components.schemas.Pet.properties.id |'); + expect(lines[17]).toBe(''); + + // Warnings heading + expect(lines[18]).toBe('### Warnings'); + expect(lines[19]).toBe(''); + + // 2-col table + expect(lines[20]).toBe( + '### [ibm-anchored-patterns](https://github.com/IBM/openapi-validator/blob/main/docs/ibm-cloud-rules.md#ibm-anchored-patterns)' + ); + expect(lines[21]).toBe(''); + expect(lines[22]).toBe( + "_A regular expression used in a 'pattern' attribute should be anchored with ^ and $_" ); + expect(lines[23]).toBe(''); + expect(lines[24]).toBe('| Line | Path |'); + expect(lines[25]).toBe('| --- | --- |'); + expect(lines[26]).toBe( + '| 233 | components.schemas.Error.properties.message.pattern |' + ); + + expect(lines).toHaveLength(27); + }); + + it('should return an empty string when there are no results', function () { + const empty = { + error: { results: [] }, + warning: { results: [] }, + }; + expect(ruleViolationDetails(empty)).toBe(''); + }); + + it('should only render the Errors section when there are no warnings', function () { + const errorsOnly = { + error: { results: validatorResults.error.results }, + warning: { results: [] }, + }; + const output = ruleViolationDetails(errorsOnly); + expect(output).toContain('### Errors'); + expect(output).not.toContain('### Warnings'); + }); + + it('should only render the Warnings section when there are no errors', function () { + const warningsOnly = { + error: { results: [] }, + warning: { results: validatorResults.warning.results }, + }; + const output = ruleViolationDetails(warningsOnly); + expect(output).not.toContain('### Errors'); + expect(output).toContain('### Warnings'); + }); + + it('should deduplicate violations with the same line and path', function () { + const duplicate = { + error: { + results: [ + { + message: 'Some error message', + path: ['paths', '/foo'], + rule: 'some-rule', + docLink: 'https://example.com', + line: 10, + }, + { + message: 'Some error message', + path: ['paths', '/foo'], + rule: 'some-rule', + docLink: 'https://example.com', + line: 10, + }, + ], + }, + warning: { results: [] }, + }; + const output = ruleViolationDetails(duplicate); + const dataRows = output.split('\n').filter(l => l.startsWith('| 10 |')); + expect(dataRows).toHaveLength(1); + }); + + it('should use a 3-column table when at least one violation has a detail', function () { + const withDetail = { + error: { + results: [ + { + message: 'Base message: detail value', + path: ['paths', '/foo'], + rule: 'rule-with-detail', + docLink: 'https://example.com', + line: 5, + }, + ], + }, + warning: { results: [] }, + }; + const output = ruleViolationDetails(withDetail); + expect(output).toContain('| Line | Path | Detail |'); + expect(output).toContain('| 5 | paths./foo | detail value |'); }); }); diff --git a/packages/validator/test/test-utils/mock-json-output.json b/packages/validator/test/test-utils/mock-json-output.json index e99aa8fcb..b65e7341a 100644 --- a/packages/validator/test/test-utils/mock-json-output.json +++ b/packages/validator/test/test-utils/mock-json-output.json @@ -8,6 +8,7 @@ "/pets/{pet_id}/{id}" ], "rule": "ibm-no-consecutive-path-parameter-segments", + "docLink": "https://github.com/IBM/openapi-validator/blob/main/docs/ibm-cloud-rules.md#ibm-no-consecutive-path-parameter-segments", "line": 84 }, { @@ -20,6 +21,7 @@ "id" ], "rule": "ibm-integer-attributes", + "docLink": "https://github.com/IBM/openapi-validator/blob/main/docs/ibm-cloud-rules.md#ibm-integer-attributes", "line": 133 } ], @@ -52,6 +54,7 @@ "pattern" ], "rule": "ibm-anchored-patterns", + "docLink": "https://github.com/IBM/openapi-validator/blob/main/docs/ibm-cloud-rules.md#ibm-anchored-patterns", "line": 233 } ],