OpenAPI 3.1.0 lint and completion rules for openapi object

[tim-lai]

Description

Full lint and completion rules for OpenAPI 3.1.0 openapi Object.

• autocompletion for openapi object fields
• info should be an object, and is required
• webhooks should be a Map of PathItem Objects
• jsonSchemaDialect should be a string
• copy over openapi3_0 rules but with openapi3_1 codes. Per comment This means they don't share any logic.
• openapi contains at least oneOf paths, components, webhooks

also,

• fix error codes for OPENAPI3_1_INFO and OPENAPI3_1_INFO_FIELD_SUMMARY_TYPE to be consistent with equivalent OPENAPI_3_1_INFO_XXX codes
• fix documentation of an unintended partial code snippet formatting
• fix a OpenAPI 3.x documentation comment for contact object instead of context object

Motivation and Context

Ref: #2062
Ref: #2063

How Has This Been Tested?

local testing.

• missing required info object should display an error
• info object should not be a string (should display error)
• info object should be an object
• info object should inherit required title and version fields from OpenAPI 3.0.x rules (should display error if not present)
• webhooks should not have an error if it contains a PathItem Object (see fixture)
• webhooks should display error if not a Map of PathItem Objects, e.g. user write error as string
• jsonSchemaDialect should be a string URI
• jsonSchemaDialect should display error if not a URI, e.g. user write error as object
• existing openapi3_0 rules now work for openapi3_1. components--type, external-docs--type, info--type, paths--type, security-items-type, security--type, servers--items, servers--type, tags--items-type, tags--type
• openapi contains at least oneOf paths, components, webhooks

openapi: 3.1.0
info: 
  title: deref
  version: 1.0.0
webhooks:
  # Each webhook needs a name
  newPet:
    # This is a Path Item Object, the only difference is that the request is initiated by the API provider
    post:
      requestBody:
        description: Information about a new pet in the system
jsonSchemaDialect: onetwoString

Screenshots (if appropriate):

Checklist

My PR contains...

• No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
• Dependency changes (any modification to dependencies in package.json)
• Bug fixes (non-breaking change which fixes an issue)
• Improvements (misc. changes to existing features)
• Features (non-breaking change which adds functionality)

My changes...

• are breaking changes to a public API (config options, System API, major UI change, etc).
• are breaking changes to a private API (Redux, component props, utility functions, etc.).
• are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
• are not breaking changes.

Documentation

• My changes do not require a change to the project documentation.
• My changes require a change to the project documentation.
• If yes to above: I have updated the documentation accordingly.

Automated tests

• My changes can not or do not need to be tested.
• My changes can and should be tested by unit and/or integration tests.
• If yes to above: I have added tests to cover my changes.
• If yes to above: I have taken care to cover edge cases in my tests.
• All new and existing tests passed.

[tim-lai]

@char0n fyi, in keeping with producing smaller PRs, I'm thinking of limiting this PR as-is (subject to review feedback), while moving the following item to a separate smaller PR:

• openapi contains at least oneOf paths, components, webhooks --> requires a new linterFunction

[char0n]

@char0n fyi, in keeping with producing smaller PRs, I'm thinking of limiting this PR as-is (subject to review feedback), while moving the following item to a separate smaller PR:

• openapi contains at least oneOf paths, components, webhooks --> requires a new linterFunction

You should finish coverying openapi3_1 object with rules in this PR, so that we have full picture of rules before meging.

• openapi contains at least oneOf paths, components, webhooks --> requires a new linterFunction

How did you come up with this requirement? I don't see it in the specification document. Can you explain please?

[char0n]

We're missing bunch of field validations. Please have a look at https://github.com/swagger-api/apidom/tree/09248f68c65129425a8224bcda450399c91ec25d/packages/apidom-ls/src/config/openapi/openapi3_0/lint.

openapi3_0 and openapi3_1 are distinct elements. This means they don't share any logic. These two elements are the exception the the rules productions. Other elements only need rules for that divergees between spec versions, but these two needs they own (duplicated) rules.

[tim-lai]

How did you come up with this requirement? I don't see it in the specification document. Can you explain please?

release notes in 3.1.0-rc0

An OpenAPI Document now requires at least one of paths, components or webhooks to exist at the top level. While previous versions required paths, now a valid OpenAPI Document can describe only webhooks, or even only reusable components. Thus, an OpenAPI Document no longer necessarily describes an API.

[tim-lai]

openapi3_0 and openapi3_1 are distinct elements. This means they don't share any logic. These two elements are the exception the the rules productions. Other elements only need rules for that divergees between spec versions, but these two needs they own (duplicated) rules.

Ok thanks for explanation. Will fix

[tim-lai]

We're missing bunch of field validations.

resolved

[char0n]

openapi3_0 and openapi3_1 are distinct elements. This means they don't share any logic. These two elements are the exception the the rules productions. Other elements only need rules for that divergees between spec versions, but these two needs they own (duplicated) rules.

Ok thanks for explanation. Will fix

What we can theoretically do to save some code is to import the lint rules from openapi3_0. But it's a short-cut that will bite us in future - if we need to support OpenAPI 2.0 or OpenAPI 4.0.0, we would have to copy the rules anyway as the rules would have to be distinguish with targetSpecs markers. So it's better to copy now and make it a final solution.

[char0n]

How did you come up with this requirement? I don't see it in the specification document. Can you explain please?

release notes in 3.1.0-rc0

An OpenAPI Document now requires at least one of paths, components or webhooks to exist at the top level. While previous versions required paths, now a valid OpenAPI Document can describe only webhooks, or even only reusable components. Thus, an OpenAPI Document no longer necessarily describes an API.

Right. The single source of truth for us is the specification - its markdown version. I found this rule specified by the JSON Schemas. Whenever there is a conflict between the specification and schemas, the specification always takes precedence. This has been already stipulated in multiple discussions like this one: OAI/OpenAPI-Specification#2666

Anyway I found this information within the specification itself here: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#openapi-document, which means we're fine and should implement this lint rule.

In order to implement it you don't necessarily need a new linting function. You can create 3 linting rules with conditional logic as demonstrated here:

apidom/packages/apidom-ls/src/config/asyncapi/bindings/ibmmq/channel-binding/lint/topic--required.ts

Line 12 in 1591cfe

conditions: [

The logic of the condition for each of these rules will be: if other two fields don't exist, this field must be defined. This makes it possible to create overall linting logic as aggregate of 3 separate linting rules.

[char0n]

@char0n fyi, in keeping with producing smaller PRs, I'm thinking of limiting this PR as-is (subject to review feedback), while moving the following item to a separate smaller PR:

up to you. For me the size of this is still comfortable to review as we only deal with OpenAPI Object rules alone, so the scope is limited.

[tim-lai]

In order to implement it you don't necessarily need a new linting function. You can create 3 linting rules with conditional logic as demonstrated here:

apidom/packages/apidom-ls/src/config/asyncapi/bindings/ibmmq/channel-binding/lint/topic--required.ts

Line 12 in 1591cfe

conditions: [

The logic of the condition for each of these rules will be: if other two fields don't exist, this field must be defined. This makes it possible to create overall linting logic as aggregate of 3 separate linting rules.

I haven't been able to get this condition example to apply to the openapi object. Latest commit in c599710, with commented out condition. At the moment, each of paths, components, and webhooks are required (without the condition), so I know the rules are recognized. Any suggestions?

In the meantime, I think all other requested changes have been made. Thanks.

[char0n]

I haven't been able to get this condition example to apply to the openapi object. Latest commit in c599710, with commented out condition. At the moment, each of paths, components, and webhooks are required (without the condition), so I know the rules are recognized. Any suggestions?

Did some experimenting and the easiest solution I could come up with seems to be introduction new rule called required-fields.ts:

import ApilintCodes from '../../../codes';
import { LinterMeta } from '../../../../apidom-language-types';

const requiredFieldsLint: LinterMeta = {
  code: ApilintCodes.OPENAPI3_1_OPEN_API_REQUIRED_FIELDS,
  source: 'apilint',
  message: 'OpenAPI Object must contain one the following fields: paths, components, webhooks',
  severity: 1,
  linterFunction: 'hasRequiredField',
  linterParams: ['paths'],
  marker: 'key',
  conditions: [
    {
      targets: [{ path: 'root' }],
      function: 'missingFields',
      params: [['paths', 'components', 'webhooks']],
    },
  ],
  data: {
    quickFix: [
      {
        message: "add 'paths' section",
        action: 'addChild',
        snippetYaml: 'paths: \n  \n',
        snippetJson: '"paths": {\n  \n  },\n',
      },
    ],
  },
};

export default requiredFieldsLint;

[tim-lai]

Did some experimenting and the easiest solution I could come up with seems to be introduction new rule called required-fields.ts:

This solution works for me. Implemented in 6d2d7c3

[char0n]

We're almost done here. Added couple of requests for changes.

[tim-lai]

@char0n FYI updated tests in this commit 5e4ad6d, applies to adding the openapi3_1 completion rules, though I'm not entirely surely how the expected result would have been pre-determined. There does not appear to be an equivalent openapi3_0 set of tests.

Separately, more test fixes incoming for openapi3_1 linting rules.

@frantuma FYI, this change alleviates the need to be knowledgeable (deepEqual) about specific completion items. We could build out more fixtures and test scenarios if it's really needed, or if there is more that you had originally intended for CompletionList. (I'll remove the commented original code before final merge)

      // assert.deepEqual(result, input[3] as CompletionList);
      assert(result?.items && result?.items.length > 0);

[char0n]

@char0n FYI updated tests in this commit 5e4ad6d, applies to adding the openapi3_1 completion rules

This change looks strange. Why would you change the the assertion? The assertions must stay the same; in order to fix the tests that stopped passing by introducing new rules, you have to add the expected structures into fixtures. And then the assertion will pass. If you want help, I can issue a PR with the test fixes.

though I'm not entirely surely how the expected result would have been pre-determined. There does not appear to be an equivalent openapi3_0 set of tests.

ApiDOM originally supported only OpenAPI 3.1 in very limited way. Effort of supporting OpenAPI 3.0.x has started during last month and is being finalized now by providing rules in apidom-ls packages. Tests that are failing for you were originally created by @frantuma for OpenAPI 3.1.0. There is also a problem with the tests in apidom-ls: when they fail - they will hang instead of failing with error code of 1. That manifests in CI as never ending test job. When we have more time, I plan to look into those tests and transform them into snapshot based tests with auto generating capability, which will make much easier to fix them in future. OpenAPI 3.0.x support in apidom-ls has very limited test coverage, as that is something I want to address when we have snapshot testing in place and all the old tests reorganized.

[tim-lai]

@char0n Tests updated and are now passing on GA CI. One caveat... the latest commit eb13b1a includes a change that reverts the change from 5e4ad6d. My local observation is that npm run test will now pass as a "bulk" runner when a glob pattern is used, but if one specifies either openapi-json.ts or openapi-yaml.ts in mocharc.json instead of the glob pattern, that specific test suite will fail. The specific test suite case expects an additional two (now commented-out) objects within the list of expected items.

[tim-lai]

We're almost done here. Added couple of requests for changes.

done.

[char0n]

Added comments/questions regarding tests. The rest of the code looks LEGIT.

feedback incorporated