stoplightio / spectral

A flexible JSON/YAML linter for creating automated style guides, with baked in support for OpenAPI v3.1, v3.0, and v2.0 as well as AsyncAPI v2.x.
https://stoplight.io/spectral
Apache License 2.0
2.43k stars 235 forks source link

Incorrect deduplication after 1.18.2 #2506

Closed derbylock closed 1 year ago

derbylock commented 1 year ago

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](https://github.com/stoplightio/spectral/pull/2501)

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

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 commented 1 year ago

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 commented 1 year ago

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.

derbylock commented 1 year ago

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