Skip to content

Commit fcfa9c6

Browse files
feat(ipa): error on unneeded exceptions IPA 117-126
1 parent fe9db99 commit fcfa9c6

21 files changed

+141
-369
lines changed

tools/spectral/ipa/CONTRIBUTING.md

Lines changed: 41 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,16 @@ When your PR is approved and merged to main, the package will be automatically p
9393
Spectral custom rule functions follow this format:
9494

9595
```js
96-
export default (input, _, { path, documentInventory })
96+
export default (input, options, context)
9797
```
98+
9899
- `input`: The current component from the OpenAPI spec. Derived from the given and field values in the rule definition.
99-
- `path`: JSONPath array to the current component.
100-
- `documentInventory`: The entire OpenAPI specification (use `resolved` or `unresolved` depending on rule context).
100+
- `options`: Optional input options passed from the rule definition.
101+
- `context`: Additional input context such as the OAS being evaluated, the following properties are commonly used:
102+
- `context.path`: JSONPath array to the current component.
103+
- `context.documentInventory`: The entire OpenAPI specification (use `resolved` or `unresolved` depending on rule context).
104+
105+
To learn more about Spectral custom functions, see the [Spectral Documentation](https://docs.stoplight.io/docs/spectral/a781e290eb9f9-custom-functions).
101106

102107
---
103108

@@ -144,11 +149,14 @@ As a rule developer, you need to define:
144149
---
145150
#### Helper Functions
146151

147-
Use the following helper functions from the `collectionUtils` module:
152+
Use the following helper function from the `collectionUtils` module:
153+
154+
[`evaluateAndCollectAdoptionStatus(errors, ruleName, object, jsonPath)`](https://github.com/mongodb/openapi/blob/cd4e085a68cb3bb6078e85dba85ad8ce1674f7da/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js#L14)
155+
156+
Passing the validation errors, the name of the rule, the object (which may contain an exception), and the path to the object, the helper function will collect adoptions, violations and exceptions accordingly. If the object adopts the rule (i.e. there are no errors passed) but the object has an exception. An error message will be returned informing the developer to remove the unnecessary exception.
157+
158+
The helper [`evaluateAndCollectAdoptionStatusWithoutExceptions(errors, ruleName, jsonPath)`](https://github.com/mongodb/openapi/blob/cd4e085a68cb3bb6078e85dba85ad8ce1674f7da/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js#L32) is used for reporting rule adoptions and violations (exceptions will be ignored - used for rules that do not allow exceptions).
148159

149-
- [`collectAndReturnViolation(jsonPath, ruleName, errorData)`](https://github.com/mongodb/openapi/blob/cd4e085a68cb3bb6078e85dba85ad8ce1674f7da/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js#L14) — for reporting rule violations.
150-
- [`collectAdoption(jsonPath, ruleName)`](https://github.com/mongodb/openapi/blob/cd4e085a68cb3bb6078e85dba85ad8ce1674f7da/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js#L32) — for marking rule adoption.
151-
- [`collectException(object, ruleName, jsonPath)`](https://github.com/mongodb/openapi/blob/cd4e085a68cb3bb6078e85dba85ad8ce1674f7da/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js#L32) — for recording rule exceptions.
152160
---
153161

154162
#### How to Decide the component level at which the rule will be processed
@@ -183,7 +191,7 @@ When designing a rule, it is important to decide at which component level the ru
183191
}
184192
}
185193
```
186-
3. In the rule implementation, use the `collectException(object, ruleName, jsonPath)` helper function to collect exceptions. The object here is what you get when you traverse the path defined by the `jsonPath`.
194+
3. In the rule implementation, use the `evaluateAndCollectAdoptionStatus(errors, ruleName, object, jsonPath)` helper function to collect adoptions, violations and exceptions. The `object` here is what you get when you traverse the path defined by the `jsonPath`. In this example, `jsonPath` would be `['components', 'schemas', 'SchemaName]`.
187195

188196
Exceptions can be defined at different levels, such as:
189197
- Resource level
@@ -274,14 +282,13 @@ A rule must collect **only one** of the following for each evaluation:
274282

275283
You can include **multiple error messages** for a violation. To do so:
276284
- Gather the messages into an array
277-
- Pass them to `collectAndReturnViolation`
285+
- Pass them to `evaluateAndCollectAdoptionStatus`
278286

279287
###### Considerations
280288

281289
- Use the **same `jsonPath`** for:
282-
- `collectAndReturnViolation`
283-
- `collectAdoption`
284-
- `collectException`
290+
- The `path` property in the errors/violations collected
291+
- The `path` property passed to `evaluateAndCollectAdoptionStatus`
285292

286293
> 💡 This path should either be the `path` parameter from the rule function or a derived value from it.
287294

@@ -294,39 +301,39 @@ You can include **multiple error messages** for a violation. To do so:
294301
const RULE_NAME = 'xgen-IPA-xxx-rule-name'
295302
296303
export default (input, opts, { path, documentInventory }) => {
297-
//Optional filter cases that we do not want to handle
304+
// Optional filter cases that we do not want to handle
298305
// Return no response for those cases.
299-
300-
//Decide the jsonPath (component level) at which you want to collect exceptions, adoption, and violation
301-
//It can be "path" parameter of custom rule function
302-
//Or, a derived path from "path" parameter
303-
if (hasException(input, RULE_NAME)) {
304-
collectException(input, RULE_NAME, jsonPath);
305-
return;
306+
if (!Array.isArray(input)) {
307+
return;
306308
}
309+
310+
// Decide the jsonPath (component level) at which you want to collect exceptions, adoption, and violation
311+
// It can be "path" parameter of custom rule function
312+
// Or, a derived path from "path" parameter
307313
308-
errors = checkViolationsAndReturnErrors(...);
309-
if (errors.length != 0) {
310-
return collectAndReturnViolation(jsonPath, RULE_NAME, errors);
311-
}
312-
return collectAdoption(jsonPath, RULE_NAME);
314+
const errors = checkViolationsAndReturnErrors(input);
315+
316+
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, input, path);
313317
};
314318
315319
316-
//This function can accept "input", "path", "documentInventory", or other derived parameters
317-
function checkViolationsAndReturnErrors(...){
320+
// This function can accept "input", "path", "documentInventory", or other derived parameters
321+
function checkViolationsAndReturnErrors(enums){
318322
try {
319323
const errors = [];
320-
for (const value of enumValues) {
321-
if (!isUpperSnakeCase(value)) {
322-
errors.push({
323-
path: [...path, 'enum'],
324-
message: `${value} is not in UPPER_SNAKE_CASE`,
325-
});
326-
}
324+
enums.forEach((index, enumValue) => {
325+
if (!isUpperSnakeCase(enumValue)) {
326+
// Append the enum index to the path to collect multiple errors
327+
errors.push({
328+
path: [...path, index],
329+
message: `${enumValue} is not in UPPER_SNAKE_CASE`,
330+
});
331+
}
332+
});
327333
}
328334
return errors;
329335
} catch(e) {
336+
// Use common error handler to return useful error messages in case of unexpected failures
330337
handleInternalError(RULE_NAME, jsonPathArray, e);
331338
}
332339
}

tools/spectral/ipa/__tests__/IPA117DescriptionMustNotUseHtml.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ testRule('xgen-IPA-117-description-must-not-use-html', [
9797
},
9898
},
9999
selfClosingHtml: {
100-
description: 'This is something.</br>With a line break.',
100+
description: 'This is something.<br/>With a line break.',
101101
'x-xgen-IPA-exception': {
102102
'xgen-IPA-117-description-must-not-use-html': 'reason',
103103
},

tools/spectral/ipa/rulesets/functions/IPA117DescriptionEndsWithPeriod.js

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
1-
import { hasException } from './utils/exceptions.js';
2-
import {
3-
collectAdoption,
4-
collectAndReturnViolation,
5-
collectException,
6-
handleInternalError,
7-
} from './utils/collectionUtils.js';
1+
import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js';
82

93
const RULE_NAME = 'xgen-IPA-117-description-ends-with-period';
104
const ERROR_MESSAGE_PERIOD = 'Descriptions must end with a full stop(.).';
@@ -15,16 +9,8 @@ export default (input, opts, { path }) => {
159
return;
1610
}
1711

18-
if (hasException(input, RULE_NAME)) {
19-
collectException(input, RULE_NAME, path);
20-
return;
21-
}
22-
2312
const errors = checkViolationsAndReturnErrors(input['description'], path);
24-
if (errors.length !== 0) {
25-
return collectAndReturnViolation(path, RULE_NAME, errors);
26-
}
27-
collectAdoption(path, RULE_NAME);
13+
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, input, path);
2814
};
2915

3016
function checkViolationsAndReturnErrors(description, path) {

tools/spectral/ipa/rulesets/functions/IPA117DescriptionMustNotUseHtml.js

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
1-
import { hasException } from './utils/exceptions.js';
2-
import {
3-
collectAdoption,
4-
collectAndReturnViolation,
5-
collectException,
6-
handleInternalError,
7-
} from './utils/collectionUtils.js';
1+
import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js';
82

93
const RULE_NAME = 'xgen-IPA-117-description-must-not-use-html';
104
const ERROR_MESSAGE = 'Descriptions must not use raw HTML.';
@@ -15,16 +9,8 @@ export default (input, opts, { path }) => {
159
return;
1610
}
1711

18-
if (hasException(input, RULE_NAME)) {
19-
collectException(input, RULE_NAME, path);
20-
return;
21-
}
22-
2312
const errors = checkViolationsAndReturnErrors(input['description'], path);
24-
if (errors.length !== 0) {
25-
return collectAndReturnViolation(path, RULE_NAME, errors);
26-
}
27-
collectAdoption(path, RULE_NAME);
13+
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, input, path);
2814
};
2915

3016
function checkViolationsAndReturnErrors(description, path) {

tools/spectral/ipa/rulesets/functions/IPA117DescriptionShouldNotUseLinks.js

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
1-
import { hasException } from './utils/exceptions.js';
2-
import {
3-
collectAdoption,
4-
collectAndReturnViolation,
5-
collectException,
6-
handleInternalError,
7-
} from './utils/collectionUtils.js';
1+
import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js';
82

93
const RULE_NAME = 'xgen-IPA-117-description-should-not-use-inline-links';
104
const ERROR_MESSAGE =
@@ -16,16 +10,8 @@ export default (input, opts, { path }) => {
1610
return;
1711
}
1812

19-
if (hasException(input, RULE_NAME)) {
20-
collectException(input, RULE_NAME, path);
21-
return;
22-
}
23-
2413
const errors = checkViolationsAndReturnErrors(input['description'], path);
25-
if (errors.length !== 0) {
26-
return collectAndReturnViolation(path, RULE_NAME, errors);
27-
}
28-
collectAdoption(path, RULE_NAME);
14+
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, input, path);
2915
};
3016

3117
function checkViolationsAndReturnErrors(description, path) {

tools/spectral/ipa/rulesets/functions/IPA117DescriptionShouldNotUseTables.js

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
1-
import { hasException } from './utils/exceptions.js';
2-
import {
3-
collectAdoption,
4-
collectAndReturnViolation,
5-
collectException,
6-
handleInternalError,
7-
} from './utils/collectionUtils.js';
1+
import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js';
82

93
const RULE_NAME = 'xgen-IPA-117-description-should-not-use-inline-tables';
104
const ERROR_MESSAGE =
@@ -16,16 +10,8 @@ export default (input, opts, { path }) => {
1610
return;
1711
}
1812

19-
if (hasException(input, RULE_NAME)) {
20-
collectException(input, RULE_NAME, path);
21-
return;
22-
}
23-
2413
const errors = checkViolationsAndReturnErrors(input['description'], path);
25-
if (errors.length !== 0) {
26-
return collectAndReturnViolation(path, RULE_NAME, errors);
27-
}
28-
collectAdoption(path, RULE_NAME);
14+
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, input, path);
2915
};
3016

3117
function checkViolationsAndReturnErrors(description, path) {

tools/spectral/ipa/rulesets/functions/IPA117DescriptionStartsWithUpperCase.js

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
1-
import { hasException } from './utils/exceptions.js';
2-
import {
3-
collectAdoption,
4-
collectAndReturnViolation,
5-
collectException,
6-
handleInternalError,
7-
} from './utils/collectionUtils.js';
1+
import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js';
82

93
const RULE_NAME = 'xgen-IPA-117-description-starts-with-uppercase';
104
const ERROR_MESSAGE_UPPER_CASE = 'Descriptions must start with Uppercase.';
@@ -14,16 +8,8 @@ export default (input, opts, { path }) => {
148
return;
159
}
1610

17-
if (hasException(input, RULE_NAME)) {
18-
collectException(input, RULE_NAME, path);
19-
return;
20-
}
21-
2211
const errors = checkViolationsAndReturnErrors(input['description'], path);
23-
if (errors.length !== 0) {
24-
return collectAndReturnViolation(path, RULE_NAME, errors);
25-
}
26-
collectAdoption(path, RULE_NAME);
12+
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, input, path);
2713
};
2814

2915
function checkViolationsAndReturnErrors(description, path) {

tools/spectral/ipa/rulesets/functions/IPA117HasDescription.js

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,12 @@
1-
import { hasException } from './utils/exceptions.js';
2-
import {
3-
collectAdoption,
4-
collectAndReturnViolation,
5-
collectException,
6-
handleInternalError,
7-
} from './utils/collectionUtils.js';
1+
import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js';
82

93
const RULE_NAME = 'xgen-IPA-117-description';
104
const ERROR_MESSAGE =
115
'Description not found. API producers must provide descriptions for Properties, Operations and Parameters.';
126

137
export default (input, opts, { path }) => {
14-
if (hasException(input, RULE_NAME)) {
15-
collectException(input, RULE_NAME, path);
16-
return;
17-
}
18-
198
const errors = checkViolationsAndReturnErrors(input, path);
20-
if (errors.length !== 0) {
21-
return collectAndReturnViolation(path, RULE_NAME, errors);
22-
}
23-
collectAdoption(path, RULE_NAME);
9+
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, input, path);
2410
};
2511

2612
function checkViolationsAndReturnErrors(input, path) {

tools/spectral/ipa/rulesets/functions/IPA117ObjectsMustBeWellDefined.js

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
1-
import { hasException } from './utils/exceptions.js';
2-
import {
3-
collectAdoption,
4-
collectAndReturnViolation,
5-
collectException,
6-
handleInternalError,
7-
} from './utils/collectionUtils.js';
1+
import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js';
82
import { pathIsForRequestVersion, pathIsForResponseVersion } from './utils/componentUtils.js';
93
import { schemaIsObject } from './utils/schemaUtils.js';
104

@@ -47,16 +41,8 @@ export default (input, { ignoredPaths }, { path }) => {
4741
return;
4842
}
4943

50-
if (hasException(input, RULE_NAME)) {
51-
collectException(input, RULE_NAME, path);
52-
return;
53-
}
54-
5544
const errors = checkViolationsAndReturnErrors(input, path);
56-
if (errors.length !== 0) {
57-
return collectAndReturnViolation(path, RULE_NAME, errors);
58-
}
59-
collectAdoption(path, RULE_NAME);
45+
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, input, path);
6046
};
6147

6248
function checkViolationsAndReturnErrors(object, path) {

tools/spectral/ipa/rulesets/functions/IPA117ParameterHasExamplesOrSchema.js

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,11 @@
1-
import { hasException } from './utils/exceptions.js';
2-
import {
3-
collectAdoption,
4-
collectAndReturnViolation,
5-
collectException,
6-
handleInternalError,
7-
} from './utils/collectionUtils.js';
1+
import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js';
82

93
const RULE_NAME = 'xgen-IPA-117-parameter-has-examples-or-schema';
104
const ERROR_MESSAGE = 'API producers must provide a well-defined schema or example(s) for parameters.';
115

126
export default (input, _, { path }) => {
13-
if (hasException(input, RULE_NAME)) {
14-
collectException(input, RULE_NAME, path);
15-
return;
16-
}
17-
187
const errors = checkViolationsAndReturnErrors(input, path);
19-
if (errors.length !== 0) {
20-
return collectAndReturnViolation(path, RULE_NAME, errors);
21-
}
22-
collectAdoption(path, RULE_NAME);
8+
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, input, path);
239
};
2410

2511
function checkViolationsAndReturnErrors(object, path) {

0 commit comments

Comments
 (0)