Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: $ref cannot have siblings #473

Merged
merged 16 commits into from
Aug 27, 2019
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,21 @@ import { terser } from 'rollup-plugin-terser';
const BASE_PATH = process.cwd();

module.exports = [
"oasOp2xxResponse",
"oasOpFormDataConsumeCheck",
"oasOpIdUnique",
"oasOpParams",
"oasOpSecurityDefined",
"oasPathParam"
'oasOp2xxResponse',
'oasOpFormDataConsumeCheck',
'oasOpIdUnique',
'oasOpParams',
'oasOpSecurityDefined',
'oasPathParam',
'refSiblings'
].map(fn => ({
input: path.resolve(BASE_PATH, 'dist/rulesets/oas/functions', `${fn}.js`),
plugins: [
typescript({
tsconfig: path.join(BASE_PATH, './tsconfig.rollup.json'),
include: ['dist/**/*.{ts,tsx}'],
}),
resolve({
only: ['json-schema-merge-allof', /lodash\/?.*/],
philsturgeon marked this conversation as resolved.
Show resolved Hide resolved
}),
resolve(),
commonjs(),
terser(),
],
Expand Down
16 changes: 8 additions & 8 deletions src/__tests__/functions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const applyRuleToObject = async (r: Rule, o: object): Promise<IRuleResult[]> =>

describe('functions', () => {
describe('xor', () => {
test('returns result if no properties are present', async () => {
test('returns resolved if no properties are present', async () => {
expect(
applyRuleToObject(
{
Expand All @@ -35,7 +35,7 @@ describe('functions', () => {
).resolves.toHaveLength(1);
});

test('returns result if both properties are present', async () => {
test('returns resolved if both properties are present', async () => {
expect(
applyRuleToObject(
{
Expand Down Expand Up @@ -106,7 +106,7 @@ describe('functions', () => {
).resolves.toHaveLength(1);
});

test('returns result if pattern is not matched (on object keys)', async () => {
test('returns resolved if pattern is not matched (on object keys)', async () => {
expect(
applyRuleToObject(
{
Expand Down Expand Up @@ -135,7 +135,7 @@ describe('functions', () => {
).resolves.toHaveLength(1);
});

test('dont return result if pattern is matched (on string)', async () => {
test('dont return resolved if pattern is matched (on string)', async () => {
expect(
applyRuleToObject(
{
Expand All @@ -157,7 +157,7 @@ describe('functions', () => {
).resolves.toHaveLength(0);
});

test('dont return result if pattern is matched (on object keys)', async () => {
test('dont return resolved if pattern is matched (on object keys)', async () => {
expect(
applyRuleToObject(
{
Expand Down Expand Up @@ -207,7 +207,7 @@ describe('functions', () => {
},
];

test('return result if string, number, array, or object is greater than max', async () => {
test('return resolved if string, number, array, or object is greater than max', async () => {
expect(
applyRuleToObject(
{
Expand All @@ -231,7 +231,7 @@ describe('functions', () => {
).resolves.toHaveLength(4);
});

test('return result if string, number, array, or object is less than min', async () => {
test('return resolved if string, number, array, or object is less than min', async () => {
expect(
applyRuleToObject(
{
Expand All @@ -255,7 +255,7 @@ describe('functions', () => {
).resolves.toHaveLength(4);
});

test('dont return a result if string, number, array, or object is between min and max', async () => {
test('dont return a resolved if string, number, array, or object is between min and max', async () => {
expect(
applyRuleToObject(
{
Expand Down
75 changes: 75 additions & 0 deletions src/__tests__/linter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,81 @@ responses:: !!foo
});
});

test('should report ref siblings', async () => {
await spectral.loadRuleset('spectral:oas');

const results = await spectral.run({
$ref: '#/',
responses: {
200: {
description: 'a',
},
201: {
description: 'b',
},
300: {
description: 'c',
abc: 'd',
$ref: '#/d',
},
},
openapi: '3.0.0',
});

expect(results).toEqual(
expect.arrayContaining([
{
code: '$ref-siblings',
message: 'Property cannot be placed among $ref',
path: ['responses'],
range: {
end: {
character: 19,
line: 12,
},
start: {
character: 14,
line: 2,
},
},
severity: DiagnosticSeverity.Error,
},
{
code: '$ref-siblings',
message: 'Property cannot be placed among $ref',
path: ['responses', '300', 'description'],
range: {
end: {
character: 24,
line: 10,
},
start: {
character: 21,
line: 10,
},
},
severity: DiagnosticSeverity.Error,
},
{
code: '$ref-siblings',
message: 'Property cannot be placed among $ref',
path: ['responses', '300', 'abc'],
range: {
end: {
character: 16,
line: 11,
},
start: {
character: 13,
line: 11,
},
},
severity: DiagnosticSeverity.Error,
},
]),
);
});

describe('runWithResolved', () => {
test('should include both resolved and validation results', async () => {
spectral.setRules({
Expand Down
7 changes: 5 additions & 2 deletions src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const lintNode = (
}

if (!targets.length) {
// must call then at least once, with no result
// must call then at least once, with no resolved
targets.push({
path: [],
value: undefined,
Expand Down Expand Up @@ -96,7 +96,10 @@ export const lintNode = (
results = results.concat(
targetResults.map<IRuleResult>(result => {
const escapedJsonPath = (result.path || targetPath).map(segment => decodePointerFragment(String(segment)));
const path = getRealJsonPath(resolved.result, escapedJsonPath);
const path = getRealJsonPath(
rule.isResolved === false ? resolved.unresolved : resolved.resolved,
escapedJsonPath,
);
const location = resolved.getLocationForJsonPath(path, true);

return {
Expand Down
113 changes: 58 additions & 55 deletions src/meta/rule.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,69 +59,72 @@
"oneOf": [
{
"properties": {
"description": {
"type": "string"
},
"recommended": {
"type": "boolean"
},
"given": {
"type": "string"
},
"severity": {
"$ref": "#/definitions/Severity"
},
"message": {
"type": "string"
},
"tags": {
"items": {
"type": "string"
},
"type": "array"
},
"formats": {
"type": "array",
"items": {
"type": "string",
"minItems": 1
}
},
"then": {
"anyOf": [
{
"$ref": "#/definitions/IThen<T,O>"
"description": {
"type": "string"
},
{
"recommended": {
"type": "boolean"
},
"given": {
"type": "string"
},
"isResolved": {
"type": "boolean"
},
"severity": {
"$ref": "#/definitions/Severity"
},
"message": {
"type": "string"
},
"tags": {
"items": {
"$ref": "#/definitions/IThen<T,O>"
"type": "string"
},
"type": "array"
}
]
},
"type": {
"enum": [
"style",
"validation"
],
"type": "string"
},
"when": {
"properties": {
"field": {
"type": "string"
},
"pattern": {
"formats": {
"type": "array",
"items": {
"type": "string",
"minItems": 1
}
},
"then": {
"anyOf": [
{
"$ref": "#/definitions/IThen<T,O>"
},
{
"items": {
"$ref": "#/definitions/IThen<T,O>"
},
"type": "array"
}
]
},
"type": {
"enum": [
"style",
"validation"
],
"type": "string"
},
"when": {
"properties": {
"field": {
"type": "string"
},
"pattern": {
"type": "string"
}
},
"required": [
"field"
],
"type": "object"
}
},
"required": [
"field"
],
"type": "object"
}
},
"required": [
"given",
"then"
Expand Down
18 changes: 9 additions & 9 deletions src/resolved.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
import { IResolveError, IResolveResult, IResolveRunner } from '@stoplight/json-ref-resolver/types';
import { IResolveError, IResolveResult } from '@stoplight/json-ref-resolver/types';
import { Dictionary, ILocation, JsonPath } from '@stoplight/types';
import { Segment } from '@stoplight/types/dist';
import { get } from 'lodash';
import { IParseMap, REF_METADATA } from './spectral';
import { IParsedResult } from './types';

export class Resolved implements IResolveResult {
export class Resolved {
public refMap: Dictionary<string>;
public result: IResolveResult;
public resolved: unknown;
public unresolved: unknown;
public errors: IResolveError[];
public runner: IResolveRunner;
public format?: string | null;

constructor(public spec: IParsedResult, result: IResolveResult, public parsedMap: IParseMap) {
this.refMap = result.refMap;
this.result = result.result;
this.errors = result.errors;
this.runner = result.runner;
constructor(public spec: IParsedResult, resolveResult: IResolveResult, public parsedMap: IParseMap) {
this.refMap = resolveResult.refMap;
this.resolved = resolveResult.result;
this.unresolved = spec.parsed.data;
this.errors = resolveResult.errors;
this.format = spec.format;
}

Expand Down
Loading