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

Incorrect deduplication after 1.18.2 #2506

Closed
derbylock opened this issue Jul 12, 2023 · 4 comments
Closed

Incorrect deduplication after 1.18.2 #2506

derbylock opened this issue Jul 12, 2023 · 4 comments

Comments

@derbylock
Copy link

derbylock commented Jul 12, 2023

Describe the bug
After updating to 1.18.2, spectral begins incorrectly showing path if there are any $ref usages in path.

To Reproduce

  1. Given this OpenAPI/AsyncAPI document spec.yaml
---
info:
  title: Test
  version: 1.0.0
  description: Test spec
  contact:
    name: Team test
    url: http://localhost:8080/contact
    email: foo@bar.baz
  license:
    url: http://localhost:8080/license
    name: Some license
openapi: 3.0.3
servers:
  - url: http://localhost:8080
tags:
  - name: testTag
    description: Test tag
paths:
  "/api/v1/clients/{client_id}/test-runs/{test_run_id}":
    description: Working with Test runs
    summary: Test runs
    parameters:
      - name: client_id
        description: Client's ID
        in: path
        required: true
        schema:
          type: string
      - name: test_run_id
        description: Test run's ID
        in: path
        required: true
        schema:
          type: string
    get:
      operationId: getInfo
      description: get info
      tags:
        - testTag
      parameters:
        - name: hash
          description: Run Hash
          in: query
          required: true
          schema:
            type: string
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                type: object
                properties:
                  error:
                    $ref: '#/components/schemas/Error'
                required:
                  - error
    post:
      operationId: setInfo
      description: set info
      tags:
        - testTag
      requestBody:
        content:
          application/json:
            schema:
              type: object
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                type: object
                properties:
                  error:
                    $ref: '#/components/schemas/Error'
                required:
                  - error
components:
  schemas:
    Error:
      type: string

and rules in rules.yaml:

extends:
  - [spectral:oas, all]
  - spectral:asyncapi
functions:
  - custom_check
rules:
  custom_check:
    severity: warn
    message: "{{error}}"
    recommended: true
    given: "$.paths.*.*.responses.*.content.application/json.schema.properties.error.type"
    then:
      function: pattern
      functionOptions:
        match: "^object$"
  1. Run this CLI command 'spectral lint --ruleset rules.yaml spec.yaml'
  2. We have single error with incorrect path "paths". It is even doesn't match the given property of the rule.
/home/tolokanal/git/api-product-tools-linting/bug/spec.yaml
 19:7  warning  custom_check  Object{} must match the pattern "^object$"  paths

✖ 1 problem (0 errors, 1 warning, 0 infos, 0 hints)

Expected behavior
It should output 2 errors as it relates to different paths:

 57:27  warning  custom_check  "string" must match the pattern "^object$"  paths./api/v1/clients/{client_id}/test-runs/{test_run_id}.get.responses[200].content.application/json.schema.properties.error.type
 79:27  warning  custom_check  "string" must match the pattern "^object$"  paths./api/v1/clients/{client_id}/test-runs/{test_run_id}.post.responses[200].content.application/json.schema.properties.error.type

Environment (remove any that are not applicable):

  • Library version: 1.18.2
  • OS: linux

Additional context
It worked as expected a week ago but now it is broken. I think it is related to #2501

@P0lip
Copy link
Contributor

P0lip commented Jul 12, 2023

What do your transitive dependencies look like?
You might have mixed versions of @stoplight/json and that would cause the issue.
If you use Yarn v1, could you run npx yarn-deduplicate --packages @stoplight/json? For Yarn v2/v3, one can use yarn dedupe to achieve that.

This is what I get locally when I run Spectral
image

@derbylock
Copy link
Author

derbylock commented Jul 12, 2023

Thanks, it seems like I have globally installed spectral which has broken modules. When I run it from locally created ./node_modules/.bin/spectral evrything works like in your screenshot.

But anyway it breaks previous logic. Previously such errors were with full path, e.g.
paths./api/v1/clients/{client_id}/test-runs/{test_run_id}.get.responses[200].content.application/json.schema.properties.error.type
which matches the specified given:
given: "$.paths.*.*.responses.*.content.application/json.schema.properties.error.type"

Now its path is not related to paths.
From my point of view, it should not have error in output for the path components.schemas.Error.type because the rule validates exactly path and without path information it is very difficult to determine the original reason of the error. Previous logic worked much better.

@P0lip
Copy link
Contributor

P0lip commented Jul 12, 2023

The issue is caused by the type set on Error, so the path we output now is expected.

components:
  schemas:
    Error:
      type: string  # this is what causes the error

That logic is always meant to show the actual location that violated the rule (looks like there was some minor bug in the previous logic that didn't quite did that), and in that case it's the type keyword under the Error component, so the outcome it expected.

@P0lip P0lip closed this as completed Jul 12, 2023
@P0lip P0lip reopened this Jul 12, 2023
@derbylock
Copy link
Author

Well, if this is the expected behavior, I think the best thing we can do is just change our tools to match that.

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

No branches or pull requests

2 participants