Spectral and OAS 3.x requirement mismatches

[BigBlueHat]

Describe the bug
Spectral requires servers and description, but OpenAPI 3.x does not.

To Reproduce

1. Given this OpenAPI document

openapi: 3.0.1
info:
  title: Testing Overly Requirements
  version: 1.0.0
  contact:
    email: byoung@bigbluehat.com

paths:
  /example:
    get:
      responses:
        200:
          content:
            application/json:
              schema:
                type: object
                properties:
                  name: OK
                  type: boolean
                  default: true

2. Run this CLI command spectral lint file.yaml
3. See the error output below...

  1:1   warning  api-servers            OpenAPI `servers` must be present and non-empty array.
  2:6   warning  info-description       OpenAPI object info `description` must be present and non-empty string.
  10:9  warning  operation-description  Operation `description` must be present and non-empty string.
  10:9  warning  operation-operationId  Operation should have an `operationId`.
  10:9  warning  operation-tags         Operation should have non-empty `tags` array.
 12:13    error  oas3-schema            /paths//example/get/responses/200 should have required property '$ref'

Expected behavior

• 1.1 is incorrect as servers is not required (see the OpenAPI Object fields):

  An array of Server Objects, which provide connectivity information to a target server. If the servers property is not provided, or is an empty array, the default value would be a Server Object with a url value of /.

• 2:6 is incorrect as description is optional (and not even a SHOULD...per the Info object fields info):

  A short description of the application. CommonMark syntax MAY be used for rich text representation.

• 10:9 is incorrect as description on a Path Item Object is also not required:

  An optional, string description, intended to apply to all operations in this path. CommonMark syntax MAY be used for rich text representation.

• 12:13 is incorrect as $ref is not required--but this probably relates to Invalid schema causes "should have required property '$ref'" error #403.

• Additionally, description is required for the Response Object and should be appearing in this list, but doesn't...though that may be due to Invalid schema causes "should have required property '$ref'" error #403 also...

So...there seem to be some mismatches here and there. 😕 Hopefully they're easy tweaks!

[philsturgeon]

Spectral is not trying to only tell you if it's valid OpenAPI. It's helping you make a useful document.

We welcome everyone to turn off rules they don't like in our default OpenAPI ruleset, or create their own rulesets which do match their expectations.

Enjoy Spectral! :D

[pixelshaded]

Making description required (which deviates from the spec) could do the inverse of making a useful doc. I.e. an operation like getObject doesn't need both a summary Get an object by id. and then a description Uh there isn't much more to say here but description is required so here we go. Get an object by id.. Hell you could even make the argument that this doesn't need a summary either because it's not providing any more value than what we already expect from a GET REST endpoint. All this does is make the document verbose/filled with fluff. It's like forcing all loggers to the debug level. You don't need nor want this every time, and that is the same with description. The spec makes a differentiation between summary and description because description supports CommonMark, aka it's meant for a verbose text block. Not every operation is going to need a verbose explanation and adding it when it is not needed hurts the document. I could understand making summary required because a lot of spec doc renderers use this as the title for an operation and not having it does hurt the browsing experience. Why not follow Speccy's example and require summary OR description? http://speccy.io/rules/1-rulesets#operation-summary-or-description

I don't know for sure, but from your response and closing this, I'm guessing that this issue will not be addressed?

[pixelshaded]

I think I may see now what the intent is. If you use the form editor of stoplight studio, they don't even expose summary as a property at all. I think Spectral/Stoplight is intending for description to replace summary entirely. Summary is sort of the elephant in the room for the spec anyway since description is used just about everywhere and summary is only used in a few places.

[BigBlueHat]

Summary is in use. It's the header on every endpoint in the form editor.

There was a good bit of back and form at API Specification Conference 2019 about this (@philsturgeon and I were on "opposing" sides 😉). The core of the argument is as you described it: requiring can result in worse docs, not requiring it can also result in worse docs. 😜

It is a tough call to make, and thankfully Spectral is configurable and the description check is only a warning anyway (and not an error). So there's that. 😺

In the end, if your API can't be understood from GET /user/{id} and a meaningful payload, then you probably do need to mandate a description...and should probably use Stoplight. 😁

[philsturgeon]

People have been saying that REST API, HTTP APIs, and more recently GraphQL APIs are "self documenting" for a while, but hey look, here we are working on documentation tooling and it's selling like hot cakes.

If you are producing an API which is so incredibly simple that every single operation can be explained entirely by the URL alone, then you can disable operation-descriptions in your ruleset and you don't need to worry about it.

Nobody likes 100% of the default eslint rules either. They tell you to do all sorts of things which JavaScript does not require, or they disallow it and JavaScript allows it, and everyone has opinions about it. Look, here is our tslint file for Spectral: https://github.com/stoplightio/tslint-config-stoplight/blob/master/tslint.json

  "rules": {
    "prettier": [
      true,
      { "printWidth": 120, "trailingComma": "all", "singleQuote": true }
    ],
    "object-literal-sort-keys": false,
    "radix": false,
    "variable-name": false,
    "curly": false,
    "no-console": false,
    "no-empty-interface": false,
    "ban-types": false,
    "max-classes-per-file": [true, 3],
    "no-var-requires": false,
    "member-ordering": false,
    "no-floating-promises": true,
    "interface-over-type-literal": false
  },

Everyone tweaks default rulesets.

So the only question is: should this be on by default. We think so.

We are not replacing summary with description (you can edit both in the GUI, we use summary as the title). The description is optional and for more information, and yes for GET /users/id it is rather clear you're getting the Users by their ID, but over time you will start to add more information like "Users can be fetch by their UUID or their username, but UUID has been deprecated in favor of username."

Even then, GETs are pretty common and POST and PATCH require a lot more information as they grow, especially when state becomes involved. So turn it off for now, and if your API grows to a point where you find a description would be useful, add it back, and everyone is happy.