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.45k stars 235 forks source link

Identify subpar error messages #1072

Open P0lip opened 4 years ago

P0lip commented 4 years ago

Chore summary

Although the overall usefulness of errors reported by Spectral has increased over the course of last months, there is still plenty of room for improvement.

Certain errors are still inconsistent, some of them are not particularly human-friendly, to say at least. Error messages that are not constant are primarily prone to be troublesome since there is a variety of cases to cover, therefore let's focus on them - looking at you, schema function.

To help us identify flaws, we can leverage some existing repositories containing plenty of public APIs, such as APIs guru, and similar.

Then, ideally, in the context of Studio, let's play around with specs using form or code view and watch errors produced in the diagnostics panel - see whether they make sense, they are easy to locate, and ideally, easy to resolve by a potentially non-technical person.

While testing the openapi documents and playing around making edits in studio forms/code view, document each subpar spectral error that you come across, along with as basic a spec document example as possible to reproduce the bad error.

Chore Scope

Let's accumulate these bad error messages and an example reproduction spec for each one in this issue (we'll use these example specs as test cases when we improve the errors). Fixing the issues is out of scope, we'll create a separate issue once this is done. I suggest time boxing this exercise to a couple of hours.

nulltoken commented 4 years ago

@P0lip Would #1037 be considered as a relevant contestant? I could work out a repro case if needed.

P0lip commented 4 years ago

@nulltoken yeah, totally! BTW this might be related to lack of allErrors.

philsturgeon commented 4 years ago

I would like to add a note to this: can we evaluate the backtick convention?

It looks super weird in Studio, looks... ok in CLI, but I would like to find out if there is something else we can do. Even single quotes would look nicer.

P0lip commented 4 years ago

@philsturgeon yeah, agree. In general, you might have seen my converting quotes to backticks, but the only reason I've been going this was the fact we already had a quite a few messages containing backticks back then, so I wanted to unify all of them. Our life should be slightly easier now that we have backticks pretty much everywhere.

philsturgeon commented 4 years ago

Here's a not entirely helpful issue:

image

We need to override this to try and talk about the specific extra item that should not be there.

openapi: 3.0.0
info:
  title: Should Not Have What API
  version: '1.0'
  contact:
    email: phil@stoplight.io
  description: >
    We get a "should NOT have addition properties" which points to `paths`, with no further information about the actual problem.

servers:
  - url: 'http://localhost/api/v1'

paths:
  regions/{id}':
    get:
      summary: Get region
      responses:
        '200':
          description: OK

Back in the day we used to have message and details, one of which was a short one-liner and the other was "more information". We were inconsistent with which one was going to be the useful "show this in the listing" and which was going to contain specific information about the problem so we threw it away, but I would love to see a "More Information" come back.

It would be pretty useful to explain background information for rules, like we do in the ruleset docs over here https://stoplight.io/p/docs/gh/stoplightio/spectral/docs/reference/openapi-rules.md

But it would also be good to see the full error if people wanted to try and figure out what was going on. I'd rather see JSON-blobs of raw AJV output than be given nothing to work with (in Studio or anywhere else).

philsturgeon commented 4 years ago

Working on a RemoteOK API description and noticed this:

Screen Shot 2020-04-29 at 14 26 01

Turns out an empty required: [] will confuse it.

openapi: 3.0.3

info:
  version: v1.0
  title: RemoteOk Jobs API
  description: ''
servers:
  - url: 'https://remoteok.io/api'
    description: Production

paths:
  /:
    get:
      summary: All Listings
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                required: []

Other validators say:

Structural error at components.schemas.Listing.required
should NOT have less than 1 items
limit: 1
philsturgeon commented 4 years ago

When an example has an array of objects where one of the examples is missing a required property:

image
openapi: 3.0.3

info:
  version: v1.0
  title: RemoteOk Jobs API
  contact:
    email: support@remoteok.io
  description: ''

servers:
  - url: 'https://remoteok.io/api'
    description: Production

paths:
  /:
    get:
      summary: All Listings
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/LegalOrListings'
              example:
                - legal: By using Remote OK's API feed you legally agree to mention Remote OK as a
                    source and link back to the job listing URL on Remote OK with a DIRECT link, no
                    redirects please. Please don't use our Remote OK and r|OK logo without written
                    permission as they're registered trademarks. And thanks for using Remote OK! ^__^
                - slug: 84864-remote-full-stack-software-engineer-deep-consulting-solutions
                  id: '84864'
                  epoch: '1588135875'
                  date: '2020-04-28T21:51:15-07:00'
                  company: Deep Consulting Solutions
                  company_logo: https://remoteok.io/assets/jobs/b5ccf9a54729263861b85b962a420457.png
                  position: Full Stack Software Engineer
                  tags:
                  - full stack
                  - dev
                  - engineer
                  - digital nomad
                  description: "(Remote, Full-Time, Anywhere in the World)We are looking for a skilled
                    and passionate Full Stack Software Engineer to join our Tech Team to build and
                    scale effective software solutions. We are in need of an enthusiastic self-starter
                    with expertise in Node.js, React, and..."
                  url: https://remoteok.io/remote-jobs/84864
                - slug: 84862-remote-paid-research-study-for-mobile-developers-using-azure-cli-user-research-international
                  id: '84862'
                  epoch: '1588120876'
                  date: '2020-04-28T17:41:16-07:00'
                  company: User Research International
                  company_logo: https://remoteok.io/assets/jobs/4301d029e86b6006eb6ea26980d7a504.png
                  position: Paid Research Study For Mobile Developers Using Azure CLI
                  tags:
                  - mobile
                  description: User Research International is a research company based out of Redmond,
                    Washington. Working with some of the biggest companies in the industry, we aim
                    to improve your experience via paid research studies. Whether it be
                    the latest video game or productivity tools, we val...
                  url: https://remoteok.io/remote-jobs/84862

components:
  schemas:
    LegalOrListings:
      type: array
      items:
        oneOf:
          - $ref: '#/components/schemas/Legal'
          - $ref: '#/components/schemas/Listing'
    Listing:
      type: object
      additionalProperties: false
      properties:
        slug:
          type: string
        id:
          type: string
          format: integer
        epoch:
          type: string
          format: integer
        date:
          type: string
          format: date-time
        company:
          type: string
        company_logo:
          type: string
        position:
          type: string
        tags:
          type: array
          items:
            type: string
        description:
          type: string
        url:
          type: string
          format: uri
        logo:
          type: string
          format: uri
        original:
          type: boolean
        verified:
          type: boolean
      required: 
      - slug
      - id
      - epoch
      - date
      - company
      - company_logo
      - position
      - tags
      - description
      - url
      - logo
      - original
      - verified
    Legal:
      type: object
      additionalProperties: false
      properties:
        legal:
          type: string
jonsgold commented 4 years ago

I don't know if this has been identified yet. Using "required": true gives error oas3-schema 'requestBody' property should have required property '$ref':

{
  "components": {
    "schemas": {
      "association_by_id": {
        "oneOf": [{ "type": "string" }, { "type": "integer" }]
      },
      "association_by_name": {
        "type": "object",
        "oneOf": [{
          "type": "object",
          "required": ["name"],
          "properties": {
            "name": { "type": "string", "required": true }
          }
        },
        { "enum": ["nil", "", null] }]
      }
    }
  }
}