Add linting rules for AEP-126: Enumerations #59
Conversation
shakrav2
force-pushed
the
feature/aep-126
branch
from
0a75ed9 to
0ecafa4
Compare
- Replace pattern match with schema function to handle both scalar and array types - Add support for OAS 3.1 type: ['string', 'null'] syntax - Add test cases for OAS 3.1 nullable enums - Fixes critical bug where OAS 3.1 specs would fail with runtime error
mkistler
left a comment
mkistler
left a comment
There was a problem hiding this comment.
This looks good but there are a few problems to fix and some improvements I'd like you to make.
| message: Enum field "{{property}}" should have type "string", not "{{error}}". | ||
| severity: error | ||
| formats: ['oas2', 'oas3'] | ||
| given: $..[?(@property === 'enum')]^ |
There was a problem hiding this comment.
Since this is used in multiple givens, it seems like a good candidate for an alias.
| - type: array | ||
| contains: | ||
| const: string | ||
| maxItems: 2 |
There was a problem hiding this comment.
I guess maxItems is here to allow "null" but it also allows other things that we'd want to flag. E.g. 'type: ["string", "number"]` should fail but I think would not currently.
| description: If enum is nullable, null should be the first value in the enum array. | ||
| message: '{{error}}' | ||
| severity: warn | ||
| formats: ['oas2', 'oas3'] |
There was a problem hiding this comment.
There is no good way to represent "nullable" fields in oas2, so I think any rule that involves nullable types should not declare that it works on oas2.
| formats: ['oas2', 'oas3'] | |
| formats: ['oas3'] |
| description: If enum contains null, field must declare nullable true. | ||
| message: '{{error}}' | ||
| severity: error | ||
| formats: ['oas2', 'oas3'] |
There was a problem hiding this comment.
| formats: ['oas2', 'oas3'] | |
| formats: ['oas3'] |
| // Only check string enums | ||
| if (field.type !== 'string') { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
This will might properties that are declared type: [null, string], which is the way nullable properties are defined in OpenAPI 3.1 and later. Please add some tests for v3.1 with nullable fields and then make sure all the rules work correctly.
| return linter; | ||
| }); | ||
|
|
||
| test('aep-126-enum-case-consistent should find warnings for mixed case', () => { |
There was a problem hiding this comment.
Please add tests for parameters with enum values -- here and in all the other rule test files.
| aep-126-no-standard-value-enums: | ||
| description: Fields should not enumerate standard codes (language, country, currency, media types). | ||
| message: '{{error}}' | ||
| severity: warn | ||
| formats: ['oas2', 'oas3'] | ||
| given: $..[?(@property === 'enum')]^ | ||
| then: | ||
| function: aep-126-no-standard-value-enums |
There was a problem hiding this comment.
I don't think we need a custom function here. I think we can write the rule this way and avoid custom js code.
| aep-126-no-standard-value-enums: | |
| description: Fields should not enumerate standard codes (language, country, currency, media types). | |
| message: '{{error}}' | |
| severity: warn | |
| formats: ['oas2', 'oas3'] | |
| given: $..[?(@property === 'enum')]^ | |
| then: | |
| function: aep-126-no-standard-value-enums | |
| aep-126-no-standard-value-enums: | |
| description: Fields should not enumerate standard codes (language, country, currency, media types). | |
| severity: warn | |
| formats: ['oas2', 'oas3'] | |
| given: | |
| - $..properties[[?(@property == 'language' || @property == 'language_code')]] | |
| - $..properties[[?(@property == 'country' || @property == 'country_code' || @property == 'region_code')]] | |
| - $..properties[[?(@property == 'currency' || @property == 'currency_code')]] | |
| - $..properties[[?(@property == 'media_type' || @property == 'content_type')]] | |
| then: | |
| function: schema | |
| functionOptions: | |
| schema: | |
| type: object | |
| not: | |
| required: ['enum'] |
2 participants
Summary
Implements linting rules for AEP-126 (Enumerations) to validate enum field usage in OpenAPI specifications.
Closes #58
Rules Added
This PR adds 6 new linting rules:
type: string, not integer or numbernullas the first valuenullmust declarenullable: trueImplementation Details
functions/aep-126-*.jstest/0126/docs/0126.md$..[?(@property === 'enum')]^to safely handle enums containing null valuesTest Results
✅ 38/38 tests passing (100%)
✅ Code coverage: 80-85% for all custom functions
✅ Linter: All checks passing
Testing
```bash
npm test -- --testPathPattern=0126
```
Checklist