feat: removed the `type:none` sourceDescriptions Arazzo extension

[DmitryAnansky]

What/Why/How?

• Removed the type:none sourceDescriptions Arazzo extension.
• Add additional lint rule for sourceDescriptions:

sourceDescriptions	[Source Description Object]	REQUIRED. A list of source descriptions (such as an OpenAPI description) this Arazzo Description SHALL apply to. The list MUST have at least one entry.

Reference

https://github.com/Redocly/redocly/issues/11343

Testing

Screenshots (optional)

Check yourself

• [ ] Code changed? - Tested with redoc/reference-docs/workflows (internal)
• [ ] All new/updated code is covered with tests
• [ ] New package installed? - Tested in different environments (browser/node)

Security

• [x] Security impact of change has been considered
• [x] Code follows company security practices and guidelines

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 9affd5ad8c796b9397d05746685610fbe4bec47c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@redocly/cli	Patch
@redocly/openapi-core	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions[bot]]

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
redocly lint packages/core/src/benchmark/benches/rebilly.yaml	980.2 ± 52.1	941.2	1118.9	1.01 ± 0.06
redocly-next lint packages/core/src/benchmark/benches/rebilly.yaml	972.0 ± 17.5	938.4	993.4	1.00

[github-actions[bot]]

Coverage report

St.:grey_question:	Category	Percentage	Covered / Total
🟡	Statements	78.57%	4995/6357
🟡	Branches	67.19%	2060/3066
🟡	Functions	73.03%	826/1131
🟡	Lines	78.88%	4712/5974

Show new covered files 🐣

St.:grey_question:	File	Statements	Branches	Functions	Lines
🟢	... / sourceDescription-type.ts	87.5%	50%	100%	100%
🟢	... / sourceDescriptions-not-empty.ts	100%	100%	100%	100%

Test suite run success

811 tests passing in 122 suites.

Report generated by 🧪jest coverage report action from 9affd5ad8c796b9397d05746685610fbe4bec47c

[lornajane]

I can fix the docs header but I thought we were holding off on adding docs until we were happy with how they were all shaping up in #1697 . Maybe we can make the code changes here, and put the docs over there? (where I will also revisit the section headers on the Arazzo rules docs)

[DmitryAnansky]

I can fix the docs header but I thought we were holding off on adding docs until we were happy with how they were all shaping up in #1697 . Maybe we can make the code changes here, and put the docs over there? (where I will also revisit the section headers on the Arazzo rules docs)

Also I can merge changes just to have rule described - so we can update docs later.

[DmitryAnansky]

Rule works as expected and I agree it should be in our recommended set. I think we should remove the docs though since we haven't added other docs yet - could you transfer the new rule page into #1697?

Moved docs update to this PR

[DmitryAnansky]

I don't like this. It seems like it could be part of the spec rule. What am I missing?

It is by the Spec rule:

• We have sourceDescriptions required in type, so we require it to be defined but can be empty array(so we need an additional rule).
• And checking that it has at least one sourceDescription with the rule to follow Arazzo Spec.

[DmitryAnansky]

I don't like this. It seems like it could be part of the spec rule. What am I missing?

This can be also done as part of type function check

const Root: NodeType = {
  properties: {
    arazzo: { type: 'string' },
    info: 'Info',
    sourceDescriptions: (items) => {
      if (items.length === 0) {
        throw new Error('SourceDescriptions must not be empty');
      }
      return 'SourceDescriptions'
    },
    workflows: 'Workflows',
    components: 'Components',
  },
  required: ['arazzo', 'info', 'sourceDescriptions', 'workflows'],
  extensionsPrefix: 'x-',
};

But error report is worse than having a rule, we don't have a way for proper line highlight and won't work well in the Editor:

With the rule:

@adamaltman , please clarify what variant would you prefer.

[adamaltman]

@DmitryAnansky I think you're missing my point. It must be part of the spec rule. When the spec rule covers less and less of the spec we shouldn't even call it a spec rule then. Get my point?

[DmitryAnansky]

@DmitryAnansky I think you're missing my point. It must be part of the spec rule. When the spec rule covers less and less of the spec we shouldn't even call it a spec rule then. Get my point?

In current cli implementation of the rules - spec rule is an abstraction on checking defined custom type. It is not extendable to apply similar logic as in the rule I introduced. And our custom types does not have minItems.

Please correct me if I am wrong @tatomyr , @RomanHotsiy

[DmitryAnansky]

@DmitryAnansky I think you're missing my point. It must be part of the spec rule. When the spec rule covers less and less of the spec we shouldn't even call it a spec rule then. Get my point?

@adamaltman moreover some of existing unique rules can't be defined in our spec rule.

[tatomyr]

And our custom types does not have minItems.

@DmitryAnansky, yes, this is a limitation of the existing spec rule. It doesn't include this type of check out of the box. It can be extended if needed, but I noticed we have a few exceptions in the OAS spec rule as well. For instance, the no-example-value-and-externalValue rule which covers the requirement that the fields are mutually exclusive by the specification, or operation-description which must be unique.

[adamaltman]

@tatomyr those were honest omissions. Mostly added as separate rules in the future for backwards compatibility reasons.

We are not in that situation with Arazzo. @DmitryAnansky we should not start the day with technical debt.

[tatomyr]

@adamaltman thanks for the clarification!

[adamaltman]

Given this information, do we have enough information to move forward?

[RomanHotsiy]

@adamaltman I would like to discuss it with you quickly before we proceed. If we want to make changes in this area we should consider bigger changes or we may end up with not the most optimal design.

The problem is that the spec rule is named incorrectly in my opinion. It should be named struct which means "Structural validation". It was never designed to validate the "spec" conformance.

Sure, we can wrap up all the various spec-related checks under the spec rule but it has some disadvantages:

• all the spec related errors will end up with spec ruleId in the output which may limit tooling in the future (or tooling will have to rely on error message)
• we loose some extra flexibility to be able to disable some aspects of spec validation (it doesn't make sense in most of cases but openapi-core can be used for various use-cases so you never know).
• (minor) spec rule code will get much harder to maintain if we want to keep same performance

I think the better solution would be to:

• rename the spec rule into struct rule (or something like that)
• prefix all of the other spec-related rules with spec- prefix (we already have a few like that)
• create a spec ruleset which includes all of the spec-related rules for each type of spec (openapi, asyncapi, arazzo)

This solution is a breaking change so I'm not suggesting doing it right now (or maybe?). I want to bring it up though so whatever we do right now is not blocking us in the future.

Let's discuss it offline and decide?

[adamaltman]

Yes, this is exactly what I was thinking (I like struct as I had a similar name in mind). I think when you say spec-{specname} you mean it is a config that someone would use like:

extends:
  - spec

And other configs like recommended could extend that itself too. Right?

[RomanHotsiy]

And other configs like recommended could extend that itself too. Right?

yes!

[adamaltman]

@DmitryAnansky we discussed not to rename the spec rule but rather to start a similar struct rule (for Arazzo). We would need to make any breaking changes for OpenAPI and AsyncAPI in version 2 with the struct rule.

The idea is that the struct rule is part of a ruleset that includes the struct rule and other rules such as sourceDescriptions has at least one value, IDs are unique, etc... It is misleading as a spec rule because it is only the basic structure of the spec, not actual things like there must be at least one sourceDescription and workflow IDs are unique.

[DmitryAnansky]

Moving code changes to https://github.com/Redocly/redocly-cli/pull/1800