Skip to content

Commit

Permalink
Merge pull request #473 from stoplightio/feat/no-ref-siblings
Browse files Browse the repository at this point in the history
feat: $ref cannot have siblings
  • Loading branch information
Phil Sturgeon authored Aug 27, 2019
2 parents b810a5d + d7ff2eb commit 2a85491
Show file tree
Hide file tree
Showing 16 changed files with 584 additions and 143 deletions.
18 changes: 18 additions & 0 deletions docs/getting-started/rulesets.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,24 @@ While running it with this object, it will succeed:
}
```

By default, Spectral processes each rule on resolved document with all $refs resolved.
If you would like to have an original input supplied to your rule, you can place `resolved` property as follows:

```yaml
rules:
my-rule-name:
description: Tags must have a description.
given: $.tags[*]
severity: error
resolved: false # note - if not specified or true, a resolved document will be given
then:
field: description
function: truthy
```

In most cases, you will want to operate on resolved document and therefore won't specify that property.
You might find `resolved` useful if your rule requires access to $refs.

## Formats

Formats are an optional way to specify which API description formats a rule, or ruleset, is applicable to. Currently Spectral supports these two formats:
Expand Down
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\/?.*/],
}),
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: 'no-$ref-siblings',
message: '$ref cannot be placed next to any other properties',
path: ['responses'],
range: {
end: {
character: 19,
line: 12,
},
start: {
character: 14,
line: 2,
},
},
severity: DiagnosticSeverity.Error,
},
{
code: 'no-$ref-siblings',
message: '$ref cannot be placed next to any other properties',
path: ['responses', '300', 'description'],
range: {
end: {
character: 24,
line: 10,
},
start: {
character: 21,
line: 10,
},
},
severity: DiagnosticSeverity.Error,
},
{
code: 'no-$ref-siblings',
message: '$ref cannot be placed next to any other properties',
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.resolved === 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"
},
"resolved": {
"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

0 comments on commit 2a85491

Please sign in to comment.