Skip to content

Commit 04c1db3

Browse files
feat(core): add error line number for yaml and json ruleset validation (#2945)
* feat(core): add error line number for yaml and json ruleset validation * feat(core): arazzo schema removed from stage
1 parent 6666ccc commit 04c1db3

9 files changed

Lines changed: 217 additions & 16 deletions

File tree

packages/cli/src/commands/__tests__/lint.test.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,26 @@ describe('lint', () => {
235235
expect(process.stderr.write).nthCalledWith(5, `Error #3: ${chalk.red('original exception')}\n`);
236236
});
237237

238+
it('prefixes ruleset errors that carry source and range with file:line:col', async () => {
239+
(lint as jest.Mock).mockReset();
240+
const errorWithLocation = Object.assign(new Error('invalid severity'), {
241+
source: '/tmp/ruleset.yaml',
242+
range: { start: { line: 7, character: 14 }, end: { line: 7, character: 22 } },
243+
});
244+
const errorWithSourceOnly = Object.assign(new Error('missing rule'), {
245+
source: '/tmp/ruleset.yaml',
246+
});
247+
(lint as jest.Mock).mockRejectedValueOnce(new AggregateError([errorWithLocation, errorWithSourceOnly]));
248+
249+
await run(`lint ./__fixtures__/empty-oas2-document.json`);
250+
251+
expect(process.stderr.write).nthCalledWith(
252+
3,
253+
`Error #1: /tmp/ruleset.yaml:8:15 — ${chalk.red('invalid severity')}\n`,
254+
);
255+
expect(process.stderr.write).nthCalledWith(4, `Error #2: /tmp/ruleset.yaml — ${chalk.red('missing rule')}\n`);
256+
});
257+
238258
it('given verbose flag, prints each error together with their stacks', async () => {
239259
(lint as jest.Mock).mockReset();
240260
(lint as jest.Mock).mockRejectedValueOnce(
@@ -250,13 +270,13 @@ describe('lint', () => {
250270
expect(process.stderr.write).nthCalledWith(2, `Error #1: ${chalk.red('some unhandled exception')}\n`);
251271
expect(process.stderr.write).nthCalledWith(
252272
3,
253-
expect.stringContaining(`packages/cli/src/commands/__tests__/lint.test.ts:242`),
273+
expect.stringContaining(`packages/cli/src/commands/__tests__/lint.test.ts:262`),
254274
);
255275

256276
expect(process.stderr.write).nthCalledWith(4, `Error #2: ${chalk.red('another one')}\n`);
257277
expect(process.stderr.write).nthCalledWith(
258278
5,
259-
expect.stringContaining(`packages/cli/src/commands/__tests__/lint.test.ts:243`),
279+
expect.stringContaining(`packages/cli/src/commands/__tests__/lint.test.ts:263`),
260280
);
261281

262282
expect(process.stderr.write).nthCalledWith(6, `Error #3: ${chalk.red('original exception')}\n`);

packages/cli/src/commands/lint.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,11 @@ const fail = (error: Error | ErrorWithCause<unknown> | AggregateError, verbose:
252252
for (const [i, error] of errors.entries()) {
253253
const actualError: unknown = isError(error) && 'cause' in error ? (error as ErrorWithCause<unknown>).cause : error;
254254
const message = isError(actualError) ? actualError.message : String(actualError);
255+
const location = formatErrorLocation(actualError);
255256

256257
const info = `Error #${i + 1}: `;
257258

258-
process.stderr.write(`${info}${chalk.red(message)}\n`);
259+
process.stderr.write(`${info}${location}${chalk.red(message)}\n`);
259260

260261
if (verbose && isError(actualError)) {
261262
process.stderr.write(`${chalk.red(printErrorStacks(actualError, info.length))}\n`);
@@ -265,6 +266,19 @@ const fail = (error: Error | ErrorWithCause<unknown> | AggregateError, verbose:
265266
process.exit(2);
266267
};
267268

269+
function formatErrorLocation(error: unknown): string {
270+
if (typeof error !== 'object' || error === null) return '';
271+
const source = (error as { source?: unknown }).source;
272+
if (typeof source !== 'string') return '';
273+
274+
const range = (error as { range?: { start?: { line?: unknown; character?: unknown } } }).range;
275+
if (typeof range?.start?.line === 'number' && typeof range.start.character === 'number') {
276+
return `${source}:${range.start.line + 1}:${range.start.character + 1} — `;
277+
}
278+
279+
return `${source} — `;
280+
}
281+
268282
function getWidth(ratio: number): number {
269283
return Math.min(20, Math.floor(ratio * process.stderr.columns));
270284
}

packages/cli/src/services/linter/utils/getRuleset.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Optional } from '@stoplight/types';
2-
import { Ruleset, RulesetDefinition } from '@stoplight/spectral-core';
2+
import { Ruleset, RulesetDefinition, RulesetSourceContext } from '@stoplight/spectral-core';
3+
import { Json, Yaml } from '@stoplight/spectral-parsers';
34
import * as fs from 'fs';
45
import * as path from '@stoplight/path';
56
import * as process from 'process';
@@ -43,9 +44,13 @@ export async function getRuleset(rulesetFile: Optional<string>): Promise<Ruleset
4344
}
4445

4546
let ruleset: string;
47+
let sourceContext: RulesetSourceContext | undefined;
48+
const originalRulesetFile = rulesetFile;
4649

4750
try {
4851
if (await isBasicRuleset(rulesetFile)) {
52+
sourceContext = await buildSourceContext(originalRulesetFile);
53+
4954
const migratedRuleset = await migrateRuleset(rulesetFile, {
5055
format: 'esm',
5156
fs,
@@ -76,9 +81,36 @@ export async function getRuleset(rulesetFile: Optional<string>): Promise<Ruleset
7681
return new Ruleset(load(ruleset, rulesetFile), {
7782
severity: 'recommended',
7883
source: rulesetFile,
84+
sourceContext,
7985
});
8086
}
8187

88+
async function buildSourceContext(rulesetFile: string): Promise<RulesetSourceContext | undefined> {
89+
try {
90+
const input = await fs.promises.readFile(rulesetFile, 'utf8');
91+
92+
if (path.extname(rulesetFile) === '.json') {
93+
const parserResult = Json.parse(input);
94+
return {
95+
source: rulesetFile,
96+
getLocationForJsonPath(jsonPath) {
97+
return Json.getLocationForJsonPath(parserResult, jsonPath);
98+
},
99+
};
100+
}
101+
102+
const parserResult = Yaml.parse(input);
103+
return {
104+
source: rulesetFile,
105+
getLocationForJsonPath(jsonPath) {
106+
return Yaml.getLocationForJsonPath(parserResult, jsonPath);
107+
},
108+
};
109+
} catch {
110+
return undefined;
111+
}
112+
}
113+
82114
function load(source: string, uri: string): RulesetDefinition {
83115
const actualUri = path.isURL(uri) ? uri.replace(/^https?:\//, '') : uri;
84116
// we could use plain `require`, but this approach has a number of benefits:

packages/core/src/ruleset/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
export { assertValidRuleset, RulesetValidationError } from './validation/index';
1+
export { assertValidRuleset, RulesetSourceContext, RulesetValidationError } from './validation/index';
22
export { getDiagnosticSeverity } from './utils/severity';
33
export { createRulesetFunction, SchemaDefinition as RulesetFunctionSchemaDefinition } from './function';
44
export { Format } from './format';

packages/core/src/ruleset/ruleset.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import type {
1111
RulesetOverridesDefinition,
1212
Stringifable,
1313
} from './types';
14-
import { assertValidRuleset } from './validation/index';
14+
import { assertValidRuleset, RulesetSourceContext } from './validation/index';
1515
import { mergeRule } from './mergers/rules';
1616
import { DEFAULT_PARSER_OPTIONS, getDiagnosticSeverity } from '..';
1717
import { mergeRulesets } from './mergers/rulesets';
@@ -26,6 +26,7 @@ const DEFAULT_RULESET_FILE = /^\.?spectral\.(ya?ml|json|m?js)$/;
2626
type RulesetContext = {
2727
readonly severity?: FileRulesetSeverityDefinition;
2828
readonly source?: string;
29+
readonly sourceContext?: RulesetSourceContext;
2930
readonly [STACK_SYMBOL]?: Map<RulesetDefinition, Ruleset>;
3031
readonly [EXPLICIT_SEVERITY]?: boolean;
3132
};
@@ -63,10 +64,10 @@ export class Ruleset {
6364
if (isPlainObject(maybeDefinition) && 'extends' in maybeDefinition) {
6465
const { extends: _, ...def } = maybeDefinition;
6566
// we don't want to validate extends - this is going to happen later on (line 29)
66-
assertValidRuleset({ extends: [], ...def }, 'js');
67+
assertValidRuleset({ extends: [], ...def }, 'js', context?.sourceContext);
6768
definition = maybeDefinition as RulesetDefinition;
6869
} else {
69-
assertValidRuleset(maybeDefinition, 'js');
70+
assertValidRuleset(maybeDefinition, 'js', context?.sourceContext);
7071
definition = maybeDefinition;
7172
}
7273

packages/core/src/ruleset/validation/__tests__/validation.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,3 +1295,80 @@ describe('JSON Ruleset Validation', () => {
12951295
).toThrowAggregateError(new AggregateError(errors));
12961296
});
12971297
});
1298+
1299+
describe('Ruleset Validation source context', () => {
1300+
it('attaches range and source to validation errors when sourceContext is provided', () => {
1301+
const sourceContext = {
1302+
source: '/tmp/ruleset.yaml',
1303+
getLocationForJsonPath(jsonPath: ReadonlyArray<string | number>) {
1304+
if (jsonPath.join('/') === 'rules/rule-with-invalid-enum/severity') {
1305+
return { range: { start: { line: 7, character: 14 }, end: { line: 7, character: 22 } } };
1306+
}
1307+
return undefined;
1308+
},
1309+
};
1310+
1311+
let caught: AggregateError | undefined;
1312+
try {
1313+
assertValidRuleset(invalidRuleset, 'js', sourceContext);
1314+
} catch (e) {
1315+
caught = e as AggregateError;
1316+
}
1317+
1318+
expect(caught).toBeInstanceOf(AggregateError);
1319+
1320+
const severityError = (caught as AggregateError).errors.find(
1321+
(e: RulesetValidationError) => e.code === 'invalid-severity',
1322+
) as RulesetValidationError;
1323+
expect(severityError.source).toBe('/tmp/ruleset.yaml');
1324+
expect(severityError.range).toEqual({
1325+
start: { line: 7, character: 14 },
1326+
end: { line: 7, character: 22 },
1327+
});
1328+
1329+
const otherError = (caught as AggregateError).errors.find(
1330+
(e: RulesetValidationError) => e.code !== 'invalid-severity',
1331+
) as RulesetValidationError;
1332+
expect(otherError.source).toBe('/tmp/ruleset.yaml');
1333+
expect(otherError.range).toBeUndefined();
1334+
});
1335+
1336+
it('attaches source to early-throw errors when sourceContext is provided', () => {
1337+
const sourceContext = {
1338+
source: '/tmp/ruleset.yaml',
1339+
getLocationForJsonPath() {
1340+
return { range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } } };
1341+
},
1342+
};
1343+
1344+
expect(() => assertValidRuleset(null, 'js', sourceContext)).toThrow(
1345+
expect.objectContaining({
1346+
code: 'invalid-ruleset-definition',
1347+
source: '/tmp/ruleset.yaml',
1348+
}),
1349+
);
1350+
1351+
expect(() => assertValidRuleset({}, 'js', sourceContext)).toThrow(
1352+
expect.objectContaining({
1353+
code: 'invalid-ruleset-definition',
1354+
source: '/tmp/ruleset.yaml',
1355+
range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } },
1356+
}),
1357+
);
1358+
});
1359+
1360+
it('leaves range and source undefined when sourceContext is omitted', () => {
1361+
let caught: AggregateError | undefined;
1362+
try {
1363+
assertValidRuleset(invalidRuleset, 'js');
1364+
} catch (e) {
1365+
caught = e as AggregateError;
1366+
}
1367+
1368+
expect(caught).toBeInstanceOf(AggregateError);
1369+
for (const e of (caught as AggregateError).errors as RulesetValidationError[]) {
1370+
expect(e.source).toBeUndefined();
1371+
expect(e.range).toBeUndefined();
1372+
}
1373+
});
1374+
});

packages/core/src/ruleset/validation/assertions.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,38 @@
11
import { isPlainObject } from '@stoplight/json';
22
import { createValidator } from './ajv';
3-
import { convertAjvErrors, RulesetValidationError } from './errors';
3+
import { convertAjvErrors, RulesetSourceContext, RulesetValidationError } from './errors';
44
import type { FileRuleDefinition, RuleDefinition, RulesetDefinition } from '../types';
55
import AggregateError from 'es-aggregate-error';
66

77
export function assertValidRuleset(
88
ruleset: unknown,
99
format: 'js' | 'json' = 'js',
10+
sourceContext?: RulesetSourceContext,
1011
): asserts ruleset is RulesetDefinition {
1112
if (!isPlainObject(ruleset)) {
12-
throw new RulesetValidationError('invalid-ruleset-definition', 'Provided ruleset is not an object', []);
13+
throw new RulesetValidationError(
14+
'invalid-ruleset-definition',
15+
'Provided ruleset is not an object',
16+
[],
17+
sourceContext === undefined ? undefined : { source: sourceContext.source },
18+
);
1319
}
1420

1521
if (!('rules' in ruleset) && !('extends' in ruleset) && !('overrides' in ruleset)) {
1622
throw new RulesetValidationError(
1723
'invalid-ruleset-definition',
1824
'Ruleset must have rules or extends or overrides defined',
1925
[],
26+
sourceContext === undefined
27+
? undefined
28+
: { source: sourceContext.source, range: sourceContext.getLocationForJsonPath([])?.range },
2029
);
2130
}
2231

2332
const validate = createValidator(format);
2433

2534
if (!validate(ruleset)) {
26-
throw new AggregateError(convertAjvErrors(validate.errors ?? []));
35+
throw new AggregateError(convertAjvErrors(validate.errors ?? [], sourceContext));
2736
}
2837
}
2938

packages/core/src/ruleset/validation/errors.ts

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { ErrorObject } from 'ajv';
2-
import type { IDiagnostic, JsonPath } from '@stoplight/types';
2+
import type { IDiagnostic, IRange, JsonPath } from '@stoplight/types';
33
import { isAggregateError } from '../../guards/isAggregateError';
44

55
export type RulesetValidationErrorCode =
@@ -17,24 +17,40 @@ export type RulesetValidationErrorCode =
1717
| 'undefined-function'
1818
| 'undefined-alias';
1919

20+
export type RulesetSourceContext = {
21+
readonly source: string;
22+
getLocationForJsonPath(path: JsonPath): { range: IRange } | undefined;
23+
};
24+
2025
interface IRulesetValidationSingleError extends Pick<IDiagnostic, 'message' | 'path'> {
2126
readonly code: RulesetValidationErrorCode;
27+
readonly range?: IRange;
28+
readonly source?: string;
2229
}
2330

2431
export class RulesetValidationError extends Error implements IRulesetValidationSingleError {
32+
public readonly range?: IRange;
33+
public readonly source?: string;
34+
2535
constructor(
2636
public readonly code: RulesetValidationErrorCode,
2737
public readonly message: string,
2838
public readonly path: JsonPath,
39+
location?: { range?: IRange; source?: string },
2940
) {
3041
super(message);
42+
this.range = location?.range;
43+
this.source = location?.source;
3144
}
3245
}
3346

3447
const RULE_INSTANCE_PATH = /^\/rules\/[^/]+/;
3548
const GENERIC_INSTANCE_PATH = /^\/(?:aliases|extends|overrides(?:\/\d+\/extends)?)/;
3649

37-
export function convertAjvErrors(errors: ErrorObject[]): RulesetValidationError[] {
50+
export function convertAjvErrors(
51+
errors: ErrorObject[],
52+
sourceContext?: RulesetSourceContext,
53+
): RulesetValidationError[] {
3854
const sortedErrors = [...errors]
3955
.sort((errorA, errorB) => {
4056
const diff = errorA.instancePath.length - errorB.instancePath.length;
@@ -78,11 +94,18 @@ export function convertAjvErrors(errors: ErrorObject[]): RulesetValidationError[
7894

7995
return filteredErrors.flatMap(error => {
8096
if (error.keyword === 'x-spectral-runtime') {
81-
return flatErrors(error.params.errors);
97+
const flat = flatErrors(error.params.errors);
98+
const list = Array.isArray(flat) ? flat : [flat];
99+
return list.map(e => enrichWithLocation(e, sourceContext));
82100
}
83101

84102
const path = error.instancePath.slice(1).split('/');
85-
return new RulesetValidationError(inferErrorCode(path, error.keyword), error.message ?? 'unknown error', path);
103+
return new RulesetValidationError(
104+
inferErrorCode(path, error.keyword),
105+
error.message ?? 'unknown error',
106+
path,
107+
resolveLocation(path, sourceContext),
108+
);
86109
});
87110
}
88111

@@ -94,6 +117,31 @@ function flatErrors(error: RulesetValidationError | AggregateError): RulesetVali
94117
return error;
95118
}
96119

120+
function resolveLocation(
121+
path: JsonPath,
122+
sourceContext: RulesetSourceContext | undefined,
123+
): { range?: IRange; source?: string } | undefined {
124+
if (sourceContext === undefined) {
125+
return undefined;
126+
}
127+
128+
return {
129+
source: sourceContext.source,
130+
range: sourceContext.getLocationForJsonPath(path)?.range,
131+
};
132+
}
133+
134+
function enrichWithLocation(
135+
error: RulesetValidationError,
136+
sourceContext: RulesetSourceContext | undefined,
137+
): RulesetValidationError {
138+
if (sourceContext === undefined || error.source !== undefined || error.range !== undefined) {
139+
return error;
140+
}
141+
142+
return new RulesetValidationError(error.code, error.message, error.path, resolveLocation(error.path, sourceContext));
143+
}
144+
97145
function inferErrorCode(path: string[], keyword: string): RulesetValidationErrorCode {
98146
if (path.length === 0) {
99147
return 'generic-validation-error';
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
export { RulesetValidationError, RulesetValidationErrorCode } from './errors';
1+
export { RulesetSourceContext, RulesetValidationError, RulesetValidationErrorCode } from './errors';
22
export { assertValidRuleset } from './assertions';

0 commit comments

Comments
 (0)