feat: add new linting rules for compositions keys oneOf, anyOf, allOf - #2369
feat: add new linting rules for compositions keys oneOf, anyOf, allOf#2369AlbinaBlazhko17 wants to merge 6 commits into
Conversation
🦋 Changeset detectedLatest commit: 3938123 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
4ee9d5e to
b6216c1
Compare
tatomyr
left a comment
tatomyr
left a comment
There was a problem hiding this comment.
Had a quick look. I suggest to reorganise the checks, so they cover fist for oneOfs, then anyOfs, and then allOfs. Just duplicate the checks when needed.
Will have another look after you implement those changes.
| // Helper function to get the composition keyword and schemas | ||
| const compositionData = (() => { |
There was a problem hiding this comment.
Please use correct naming so you don't need the comments, e.g.:
| // Helper function to get the composition keyword and schemas | |
| const compositionData = (() => { | |
| const getSchemaCompositionData = (() => { |
or something similar.
There was a problem hiding this comment.
Oh, wait, it's an IIFE? Then it's a bit of overengineering. You can do simply this:
const oneOfs = schema.oneOfif you prefer to have a separate variable, and then just do every needed check. Don't be afraid to repeat yourself.
There was a problem hiding this comment.
I was thinking about using a separate variable for every composition keyword, but in that case, i need to duplicate checks (duplicated schemas, length of schemas) for every composition keyword and the only difference will be in the warning message. So, the purpose of the IIFE is to return the schema and string, and after that to easily retrieve them in the report message and checks.
| // Helper function to get the composition keyword and schemas | ||
| const compositionData = (() => { | ||
| if (schema.oneOf && Array.isArray(schema.oneOf)) { | ||
| return { keyword: 'oneOf' as const, schemas: schema.oneOf }; |
There was a problem hiding this comment.
Technically, your schema could contain oneOf and allOf simultaneously. The code doesn't handle this case.
There was a problem hiding this comment.
This functionality at all doesn't resolve all nested composition keywords, because we consider, that every composition keyword is a new scope of validation.
|
|
||
| const { keyword, schemas } = compositionData; | ||
|
|
||
| // Check for minimum schema count (oneOf and anyOf require at least 2) |
There was a problem hiding this comment.
You don't need comments like this as the intention should be clear enough from the condition and the report message.
There was a problem hiding this comment.
Yeah, you are right. I just left all comments to simplify the review of PR, i will remove it after finalizing the functionality.
| return null; | ||
| })(); | ||
|
|
||
| if (!compositionData) return; |
There was a problem hiding this comment.
This duplicates the skip() functianality.
There was a problem hiding this comment.
Yeah, it is duplication, because it doesn't see proper type, even we check in skip method.
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Performance Benchmark (Lower is Faster)
|
65712c8 to
556e9a3
Compare
AlbinaBlazhko17
changed the title
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b0c7308. Configure here.
…ical-composition-keywords
2 participants
What/Why/How?
Added new linting rules to handle illogical use of composition keys:
oneOf,anyOf,allOf.Reference
Resolves #2326
Testing
Locally with different specs.
Screenshots (optional)
Check yourself
Security
Note
Medium Risk
Turns on a new recommended lint rule that can emit warnings/errors on existing OpenAPI descriptions. Overlap detection is heuristic and may false-positive or miss cases, but it does not change auth or data handling.
Overview
Adds the OAS 3.x lint rule
no-illogical-composition-keywordssooneOf,anyOf, andallOfmust combine schemas a value can actually match.It flags too-few members, duplicates, empty
{}members, overlappingoneOfbranches (using a narrow comparison oftype/enum/const/properties/required/additionalProperties: false), and discriminator misuse (property not required, or inline members that cannot be selected). Single-schemaallOfwrappers used for sibling keywords or discriminated subtypes are allowed.Enabled as warn in
recommendedand error inrecommended-strict/all(off inminimal/spec). Registers namedOneOf/AnyOf/AllOfnode types so built-in and configurable rules can target one composition keyword. Existing descriptions may report new problems.Reviewed by Cursor Bugbot for commit b0c7308. Bugbot is set up for automated code reviews on this repo. Configure here.