Skip to content

Commit

Permalink
chore(ruleset): improve then validation (#1144)
Browse files Browse the repository at this point in the history
* feat(ruleset): improve then validation

* chore: accept null for better backwards compatbility

* chore: cover all functions

* chore(karma): support describe.each

* fix(alphabetical): options are optional

* chore: minor cleanup

* fix: cover xor

* test: remove irrelevant fn args

* docs(functions): include types
  • Loading branch information
P0lip authored May 14, 2020
1 parent e5fe1dc commit a02842a
Show file tree
Hide file tree
Showing 21 changed files with 1,299 additions and 273 deletions.
2 changes: 1 addition & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ root = true
[*]
charset = utf-8

[*.{ts,js,yaml,scenario,md}]
[*.{ts,js,yaml,yml,json,scenario,md}]
indent_style = space
indent_size = 2

Expand Down
123 changes: 62 additions & 61 deletions docs/reference/functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ Enforce alphabetical content, for simple arrays, or for objects by passing a key

<!-- title: functionOptions -->

name | description | required?
---------|----------|---------
keyedBy | key to sort an object by | no
| name | description | type | required? |
| ------- | ------------------------ | -------- | --------- |
| keyedBy | key to sort an object by | `string` | no |

<!-- title: example -->

Expand All @@ -33,9 +33,9 @@ Does the field value exist in this set of possible values?
<!-- title: functionOptions -->
name | description | required?
---------|----------|---------
values | an array of possible values | yes
| name | description | type | required? |
| ------ | --------------------------- | ---------------------- | --------- |
| values | an array of possible values | `Array<number,string>` | yes |

<!-- title: example -->

Expand Down Expand Up @@ -83,22 +83,22 @@ Regular expressions!

<!-- title: functionOptions -->

name | description | required?
---------|----------|---------
match | if provided, value must match this regex | no
notMatch | if provided, value must _not_ match this regex | no
| name | description | type | required? |
| -------- | ---------------------------------------------- | -------- | --------- |
| match | if provided, value must match this regex | `string` | no |
| notMatch | if provided, value must _not_ match this regex | `string` | no |

<!-- title: example -->

```yaml
path-no-trailing-slash:
description: Paths should not end with `#/`.
type: style
given: "$.paths[*]~"
then:
function: pattern
functionOptions:
notMatch: ".+\/$"
path-no-trailing-slash:
description: Paths should not end with `#/`.
type: style
given: "$.paths[*]~"
then:
function: pattern
functionOptions:
notMatch: ".+\/$"
```
## casing
Expand All @@ -107,22 +107,23 @@ Text must match a certain case, like `camelCase` or `snake_case`.

<!-- title: functionOptions -->

name | description | required?
-----------------------|---------------------------------------------------------|----------
type | the casing type to match against | yes
disallowDigits | if not truthy, digits are allowed | no
separator.char | additional char to separate groups of words | no
separator.allowLeading | can the group separator char be used at the first char? | no
| name | description | type | required? |
| ---------------------- | ------------------------------------------------------- | ------------ | --------- |
| type | the casing type to match against | `CasingType` | yes |
| disallowDigits | if not truthy, digits are allowed | `boolean` | no |
| separator.char | additional char to separate groups of words | `string` | no |
| separator.allowLeading | can the group separator char be used at the first char? | `boolean` | no |

**Note:** In advanced scenarios, `separator.char` and `separator.allowLeading` can be leveraged to validate certain naming conventions.
For instance, the following naming style could be enforced:
- Headers _(eg. `X-YourMighty-Header`)_: type: `pascal`, separator.char: `-`
- Camel cased paths _(eg. `/path/toThe/amazingResource`)_: type: `camel`, separator.char: `/`, separator.allowLeading: `true`

- Headers _(eg. `X-YourMighty-Header`)_: type: `pascal`, separator.char: `-`
- Camel cased paths _(eg. `/path/toThe/amazingResource`)_: type: `camel`, separator.char: `/`, separator.allowLeading: `true`

Available types are:

| name | sample |
|--------|----------------|
| ------ | -------------- |
| flat | verylongname |
| camel | veryLongName |
| pascal | VeryLongName |
Expand All @@ -146,14 +147,14 @@ camel-case-name:

## schema

Use JSON Schema (draft 4, 6 or 7) to treat the contents of the $given JSON Path as a JSON instance.
Use JSON Schema (draft 4, 6 or 7) to treat the contents of the \$given JSON Path as a JSON instance.

<!-- title: functionOptions -->

name | description | required?
---------|----------|---------
schema | a valid JSON Schema document | yes
allErrors | returns all errors when `true`; otherwise only returns the first error | no
| name | description | type | required? |
| --------- | ---------------------------------------------------------------------- | ------------ | --------- |
| schema | a valid JSON Schema document | `JSONSchema` | yes |
| allErrors | returns all errors when `true`; otherwise only returns the first error | `boolean` | no |

<!-- title: example -->

Expand All @@ -180,27 +181,27 @@ The schema-path rule is very meta. It is an extension of the schema rule, but it
<!-- title: functionOptions -->
name | description | required?
---------|----------|---------
field | the field to check | yes
schemaPath | a json path pointing to the json schema to use | yes
allErrors | returns all errors when `true`; otherwise only returns the first error | no
| name | description | type | required? |
| ---------- | ---------------------------------------------------------------------- | --------- | --------- |
| field | the field to check | `string` | yes |
| schemaPath | a json path pointing to the json schema to use | `string` | yes |
| allErrors | returns all errors when `true`; otherwise only returns the first error | `boolean` | no |

<!-- title: example -->

``` yaml
valid-oas-example-in-parameters:
description: Examples must be valid against their defined schema.
message: "{{error}}"
recommended: true
severity: 0
type: validation
given: "$..parameters..[?(@.example && @.schema)]"
then:
function: schemaPath
functionOptions:
field: example
schemaPath: "$.schema"
```yaml
valid-oas-example-in-parameters:
description: Examples must be valid against their defined schema.
message: "{{error}}"
recommended: true
severity: 0
type: validation
given: "$..parameters..[?(@.example && @.schema)]"
then:
function: schemaPath
functionOptions:
field: example
schemaPath: "$.schema"
```

## truthy
Expand All @@ -209,7 +210,7 @@ The value should not be `false`, `""`, `0`, `null` or `undefined`. Basically any

<!-- title: example -->

``` yaml
```yaml
important-fields:
description: Absolutely must have a title and a description
message: "Missing the {{property}}"
Expand All @@ -226,7 +227,7 @@ important-fields:

The value must be `undefined`. When combined with `field: foo` on an object the `foo` property must be undefined.

_**Note:** Due to the way YAML works, just having `foo: ` with no value set is not the same as being `undefined`. This would be `falsy`._
_**Note:** Due to the way YAML works, just having `foo:` with no value set is not the same as being `undefined`. This would be `falsy`._

## unreferencedReusableObject

Expand All @@ -238,13 +239,13 @@ _Warning:_ This function may identify false positives when used against a specif

<!-- title: functionOptions -->

name | description | required?
---------|----------|---------
reusableObjectsLocation | a local json pointer to the document member holding the reusable objects (eg. `#/definitions` for an OAS2 document, `#/components/schemas` for an OAS3 document). | yes
| name | description | type | required? |
| ----------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | --------- |
| reusableObjectsLocation | a local json pointer to the document member holding the reusable objects (eg. `#/definitions` for an OAS2 document, `#/components/schemas` for an OAS3 document). | `string` | yes |

<!-- title: example -->

``` yaml
```yaml
unused-definition:
description: Potentially unused definition has been detected.
recommended: true
Expand All @@ -263,9 +264,9 @@ Communicate that one of these properties is required, and no more than one is al

<!-- title: functionOptions -->

name | description | required?
---------|----------|---------
properties | the properties to check | yes
| name | description | type | required? |
| ---------- | ----------------------- | ---------- | --------- |
| properties | the properties to check | `string[]` | yes |

<!-- title: example -->

Expand All @@ -278,8 +279,8 @@ components-examples-value-or-externalValue:
function: xor
functionOptions:
properties:
- externalValue
- value
- externalValue
- value
```
## typedEnum
Expand Down
3 changes: 3 additions & 0 deletions karma-jest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,6 @@ test.each = input => (name: string, fn: Function) => {
}
}
};

// @ts-ignore
describe.each = test.each;
6 changes: 3 additions & 3 deletions src/__tests__/linter.jest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { httpAndFileResolver } from '../resolvers/http-and-file';
import { readRuleset } from '../rulesets';
import { setFunctionContext } from '../rulesets/evaluators';
import oasDocumentSchema from '../rulesets/oas/functions/oasDocumentSchema';
import { Spectral } from '../spectral';
import { IFunctionResult, Spectral } from '../spectral';
import { IRuleset, RulesetExceptionCollection } from '../types/ruleset';

const customFunctionOASRuleset = path.join(__dirname, './__fixtures__/custom-functions-oas-ruleset.json');
Expand Down Expand Up @@ -257,7 +257,7 @@ console.log(this.cache.get('test') || this.cache.set('test', []).get('test'));
it('should handle basic example', async () => {
spectral.setFunctions({
[fnName]() {
return new Promise(resolve => {
return new Promise<IFunctionResult[]>(resolve => {
setTimeout(resolve, 200, [
{
message: 'Error reported by async fn',
Expand Down Expand Up @@ -289,7 +289,7 @@ console.log(this.cache.get('test') || this.cache.set('test', []).get('test'));
it('should handle rejections', async () => {
spectral.setFunctions({
[fnName]() {
return new Promise((resolve, reject) => {
return new Promise<void>((resolve, reject) => {
setTimeout(reject, 1000, new Error('Some unknown error'));
});
},
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/linter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ describe('linter', () => {
const message = '4xx responses require a description';

spectral.setFunctions({
func1: val => {
func1: (val: unknown) => {
if (!val) {
return [
{
Expand Down
38 changes: 0 additions & 38 deletions src/functions/__tests__/casing.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { AssertionError } from 'assert';
import { casing, CasingType } from '../casing';

function runCasing(target: unknown, type: CasingType, disallowDigits?: boolean, separator?: any) {
Expand All @@ -20,10 +19,6 @@ describe('casing', () => {
expect(runCasing('', CasingType.camel)).toBeUndefined();
});

test('given unknown case type should return nothing', () => {
expect(() => runCasing('2', 'foo' as any)).toThrowError(AssertionError);
});

describe('casing type', () => {
describe('flat', () => {
const invalid = ['foo_test', 'Foo', '123', '1d', 'foo-bar'];
Expand Down Expand Up @@ -220,39 +215,6 @@ describe('casing', () => {
});

describe('casing with custom separator configuration', () => {
describe('parameters validation', () => {
const casingTypes = Object.keys(CasingType);

test.each([...casingTypes])(
'with "%s" type - throws when allowLeadingSeparator option is used without specifying separator',
type => {
expect(() =>
runCasing('irrelevant value', CasingType[type], undefined, { char: undefined, allowLeading: true }),
).toThrow(AssertionError);
},
);

const invalidSepLengthCombinations = Object.values(casingTypes).reduce<Array<[string, number]>>(
(combinations, type) => {
for (const len of [0, 2, 3, 17]) {
combinations.push([type, len]);
}

return combinations;
},
[],
);

test.each(invalidSepLengthCombinations)(
'with type "%s" - throws when separator value is "%i" (not exactly one char long)',
(type, len) => {
expect(() => runCasing('irrelevant value', CasingType[type], undefined, { char: '-'.repeat(len) })).toThrow(
AssertionError,
);
},
);
});

const baseData: Array<[string, string, string, string]> = [[CasingType.flat, 'flat', 'flat01', 'Nope']];

const testCases: Array<[string, boolean, string, boolean, string, string, string]> = [];
Expand Down
2 changes: 1 addition & 1 deletion src/functions/__tests__/schema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Optional } from '@stoplight/types';
import { JSONSchema4, JSONSchema6 } from 'json-schema';
import { schema } from '../schema';

function runSchema(target: any, schemaObj: object, oasVersion?: Optional<number>) {
function runSchema(target: any, schemaObj: object, oasVersion?: Optional<2 | 3 | 3.1>) {
return schema(target, { schema: schemaObj, oasVersion }, { given: [] }, { given: null, original: null } as any);
}

Expand Down
4 changes: 2 additions & 2 deletions src/functions/alphabetical.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const getUnsortedItems = <T>(arr: T[], compareFn: (a: T, B: T) => number): null
return null;
};

export const alphabetical: IFunction<IAlphaRuleOptions> = (targetVal, opts, paths, { documentInventory }) => {
export const alphabetical: IFunction<IAlphaRuleOptions | null> = (targetVal, opts, paths, { documentInventory }) => {
const results: IFunctionResult[] = [];

if (!isObject(targetVal)) {
Expand All @@ -49,7 +49,7 @@ export const alphabetical: IFunction<IAlphaRuleOptions> = (targetVal, opts, path
return results;
}

const { keyedBy } = opts;
const keyedBy = opts?.keyedBy;

const unsortedItems = getUnsortedItems<unknown>(
targetArray,
Expand Down
23 changes: 0 additions & 23 deletions src/functions/casing.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Dictionary } from '@stoplight/types';
import { AssertionError } from 'assert';
import { escapeRegExp } from 'lodash';
import { IFunction } from '../types';

Expand Down Expand Up @@ -37,8 +36,6 @@ export const casing: IFunction<ICasingOptions> = (targetVal, opts) => {
return;
}

assertValidOptions(opts);

const casingValidator = buildFrom(CASES[opts.type], opts);

if (casingValidator.test(targetVal)) {
Expand All @@ -52,26 +49,6 @@ export const casing: IFunction<ICasingOptions> = (targetVal, opts) => {
];
};

function assertValidOptions(opts: ICasingOptions): asserts opts is ICasingOptions {
if (!(opts.type in CASES)) {
throw new AssertionError({ message: `Invalid '${opts.type}' type value.` });
}

if (opts.separator === undefined) {
return;
}

if (opts.separator.allowLeading !== undefined && opts.separator.char === undefined) {
throw new AssertionError({
message: "'separator.allowLeading' can only be valued when 'separator.char' is defined.",
});
}

if (opts.separator.char.length !== 1) {
throw new AssertionError({ message: "When valued, 'separator.char' should only be one character long." });
}
}

const DIGITS_PATTERN = '0-9';

const buildFrom = (basePattern: string, overrides: ICasingOptions): RegExp => {
Expand Down
Loading

0 comments on commit a02842a

Please sign in to comment.