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

Alphabetical and '@key' #730

Closed
dillonredding opened this issue Nov 4, 2019 · 9 comments · Fixed by #835
Closed

Alphabetical and '@key' #730

dillonredding opened this issue Nov 4, 2019 · 9 comments · Fixed by #835
Assignees
Labels
t/bug Something isn't working

Comments

@dillonredding
Copy link

Describe the bug

I may be doing it wrong, but the alphabetical function doesn't appear to work with '@key'. Here's the rule I'm trying to create:

  response-order:
    message: Responses should be in alphabetical order
    recommended: true
    given: $.paths.*.*
    then:
      field: responses
      function: alphabetical
      functionOptions:
        keyedBy: '@key'

I've also tried this form:

  response-order:
    message: Responses should be in alphabetical order
    recommended: true
    given: $.paths.*.*.responses
    then:
      field: '@key'
      function: alphabetical

To Reproduce

Given this OpenAPI document

openapi: 3.0.2
info:
  title: Test Spec
  version: 0.0.0
paths:
  /foo:
    get:
      operationId: get-foo
      responses:
        '400':
          description: ''
        '200':
          description: ''

Run this CLI command

spectral lint test-spec.yaml -r my-rules.yaml

Expected behavior

The lint output should contain the following:

 9:17  warning  response-order    Responses should be in alphabetical order

Environment

  • Library version: 4.2.0
  • OS: Windows 10
@dillonredding dillonredding added the t/bug Something isn't working label Nov 4, 2019
@P0lip
Copy link
Contributor

P0lip commented Nov 4, 2019

@dillonredding
Hey! The root cause is exactly the same as the one I described in #726.
JS object literals must comply with a certain order of keys as stated in the spec.

@dillonredding
Copy link
Author

@P0lip
I was wondering if that might be the case lol. Thanks again for looking into this. Let me know if you come up with a solution!

@P0lip
Copy link
Contributor

P0lip commented Dec 4, 2019

@dillonredding
Just wanted to give you heads up that this particular issue is going to be resolved in the next release.

@dillonredding
Copy link
Author

@P0lip
Excellent! Do you happen to have a release date yet?

@P0lip
Copy link
Contributor

P0lip commented Dec 5, 2019

@dillonredding soon! hopefully next week or in 2 weeks :) Def before the end of year.

@raworre
Copy link

raworre commented Feb 18, 2020

I noticed an issue with getting the alphabetical function to work. It seems when a $ref exists in the items to be checked for alphabetization the rule is ignored.

To Reproduce

Using the rules taken directly from this test scenario and a modified version of the OpenAPI document in the same test we are not seeing the failed rule as we should.

my-rules.yaml:

rules:
  response-order:
    message: Responses should be in alphabetical order
    recommended: true
    given: $.paths.*.*.responses
    then:
      function: alphabetical

test-spec.yaml:

openapi: 3.0.2
info:
  title: Test Spec
  version: 0.0.0
paths:
  /foo:
    get:
      operationId: get-foo
      responses:
        '404':
          description: ''
        '200':
          description: ''
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/foo'
components:
  schemas:
    foo:
      title: More incorrect casing
      type: object
      properties:
        id:
          type: integer
        bar:
          type: string

CLI command:

spectral lint test-spec.yaml -r my-rules.yaml

We should see the following in our results:

 19:15  warning  response-order  Responses should be in alphabetical order

Environment

  • Library version: 5.0.0
  • OS: Windows 10
  • Node version: 12.14.1

@raworre
Copy link

raworre commented Feb 19, 2020

@P0lip does this look like it maybe should be a new issue, or am I missing something with the alphabetical implementation?

@P0lip
Copy link
Contributor

P0lip commented Feb 20, 2020

@raworre it's somewhat related, but a bit different. Could you please file a new issue?
Thank you!

@raworre
Copy link

raworre commented Feb 20, 2020

@P0lip thanks! Filed Issue 979.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants