Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: create new rule to enforce array parameters format #1338

Merged
merged 5 commits into from
Nov 24, 2023

Conversation

IgorKarpiuk
Copy link
Contributor

@IgorKarpiuk IgorKarpiuk commented Nov 22, 2023

What/Why/How?

Create new rule array-parameter-serialization rule to enforce array parameters format.

This Rule enforces to define fields explode and style for parameter if param's schema type is array or has items or preffixItems

Reference

Testing

#1317

Screenshots (optional)

Check yourself

  • Code is linted
  • Tested with redoc/reference-docs/workflows (internal)
  • All new/updated code is covered with tests

Security

  • Security impact of change has been considered
  • Code follows company security practices and guidelines

@IgorKarpiuk IgorKarpiuk self-assigned this Nov 22, 2023
@IgorKarpiuk IgorKarpiuk requested a review from a team as a code owner November 22, 2023 13:16
Copy link

changeset-bot bot commented Nov 22, 2023

🦋 Changeset detected

Latest commit: 2a4a1a8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@redocly/openapi-core Minor
@redocly/cli Minor

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

Copy link
Contributor

github-actions bot commented Nov 22, 2023

Command Mean [s] Min [s] Max [s] Relative
redocly lint packages/core/src/benchmark/benches/rebilly.yaml 1.038 ± 0.037 0.995 1.108 1.00 ± 0.05
redocly-next lint packages/core/src/benchmark/benches/rebilly.yaml 1.036 ± 0.030 0.994 1.084 1.00

Copy link
Contributor

github-actions bot commented Nov 22, 2023

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 76.17% 4068/5341
🟡 Branches 66.07% 2167/3280
🟡 Functions 68.39% 660/965
🟡 Lines 76.36% 3815/4996
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / array-parameter-serialization.ts
100% 88.89% 100% 100%

Test suite run success

652 tests passing in 94 suites.

Report generated by 🧪jest coverage report action from 2a4a1a8

@lornajane
Copy link
Contributor

Could we get a changeset please?

The rule name doesn't need -rule in it, but the rest is good :)

Copy link
Contributor

@lornajane lornajane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changelog suggestion, I haven't tried the feature itself yet

.changeset/silver-singers-glow.md Outdated Show resolved Hide resolved
docs/rules/array-parameter-serialization.md Outdated Show resolved Hide resolved

## Related rules

- [configurable rules](./configurable-rules.md)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it related? In what way?

Copy link
Contributor Author

@IgorKarpiuk IgorKarpiuk Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, we created this rule from similar configurable rules.

@lornajane
Copy link
Contributor

I think I broke something! The following snippet does return as valid, which is correct:

      parameters:
        - name: f
          in: query
          style: label
          explode: true
          schema:
            type: array
            items:
              type: string

However if the explode field is set to false (it's boolean, so it might well be set to false!) then I get the error Parameter fshould havestyleandexplode fields. Could you take another look?

@IgorKarpiuk
Copy link
Contributor Author

Thanks @lornajane I updated the code and added it to test

Copy link
Contributor

@lornajane lornajane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a test where explode is defined, but false? When I do this on my local version, the rule fails.

Copy link
Contributor

@lornajane lornajane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working as expected now (that failing link check is because we can't publish docs at the moment, so feel free to merge regardless)

@IgorKarpiuk IgorKarpiuk force-pushed the feat/array-parameter-serialization-rule branch from e06e57a to 2a4a1a8 Compare November 24, 2023 14:10
@IgorKarpiuk IgorKarpiuk merged commit efb6a45 into main Nov 24, 2023
28 of 29 checks passed
@IgorKarpiuk IgorKarpiuk deleted the feat/array-parameter-serialization-rule branch November 24, 2023 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants