diff --git a/.changeset/vast-kids-add.md b/.changeset/vast-kids-add.md new file mode 100644 index 0000000000..2e74873dbd --- /dev/null +++ b/.changeset/vast-kids-add.md @@ -0,0 +1,6 @@ +--- +'@redocly/cli': minor +'@redocly/openapi-core': minor +--- + +Added a `strategy` option to the `component-name-unique` rule, matching the `--component-names-strategy` option of the `bundle` command. diff --git a/docs/@v2/commands/bundle.md b/docs/@v2/commands/bundle.md index cf19cf8246..fb5c68424d 100644 --- a/docs/@v2/commands/bundle.md +++ b/docs/@v2/commands/bundle.md @@ -205,3 +205,6 @@ All other characters, including non-ASCII letters such as `é` or `я`, are repl Schemas without `title` can't be named using the `--component-names-strategy=title` strategy. The bundling process reports an error for such schemas. {% /admonition %} + +To catch name collisions before bundling, set the matching `strategy` option on the +[`component-name-unique`](../rules/oas/component-name-unique.md) rule. diff --git a/docs/@v2/rules/oas/component-name-unique.md b/docs/@v2/rules/oas/component-name-unique.md index b0bdfac425..1d79b9de89 100644 --- a/docs/@v2/rules/oas/component-name-unique.md +++ b/docs/@v2/rules/oas/component-name-unique.md @@ -38,6 +38,7 @@ This clearly is not optimal. Having unique component names prevents these proble | parameters | string | Possible values: `off`, `warn`, `error`. Default: not set. | | responses | string | Possible values: `off`, `warn`, `error`. Default: not set. | | requestBodies | string | Possible values: `off`, `warn`, `error`. Default: not set. | +| strategy | string | Possible values: `basename`, `title`. Default: `basename`. | An example configuration: @@ -48,8 +49,25 @@ rules: parameters: off responses: warn requestBodies: warn + strategy: basename ``` +### Component names strategy + +The rule predicts the component names that `bundle` produces, so `strategy` must match the +[`--component-names-strategy`](../../commands/bundle.md#configure-the-component-names-strategy) option you bundle with. + +With the default `basename`, a schema pulled in from another file is named after the `$ref` fragment or the file name. +Two files both called `Order.yaml` therefore collide, and the rule reports them. + +With `title`, the same schemas are named after their `title` field instead. +Two files called `Order.yaml` with the titles `Order model` and `Order request` become `OrderModel` and `OrderRequest`, so the rule no longer reports them. +Two schemas in differently named files that share a title do collide, and the rule reports those instead. + +The `title` strategy applies only to schemas that are referenced from another file, because those are the only ones `bundle` renames. +Schemas defined directly under the root description's `components/schemas` keep their own key. +A referenced schema without a `title` falls back to the file name — `bundle` reports the missing title itself. + ## Examples Given this configuration: diff --git a/packages/core/src/bundle/bundle-visitor.ts b/packages/core/src/bundle/bundle-visitor.ts index 1753b30c25..3af37945a5 100644 --- a/packages/core/src/bundle/bundle-visitor.ts +++ b/packages/core/src/bundle/bundle-visitor.ts @@ -1,5 +1,5 @@ import { type RuleSeverity } from '../config/types.js'; -import { COMPONENT_NAME_CHARS, type SpecMajorVersion } from '../oas-types.js'; +import { type SpecMajorVersion } from '../oas-types.js'; import { isAbsoluteUrl, replaceRef, @@ -14,11 +14,11 @@ import { import { type ResolvedRefMap, type Document } from '../resolve.js'; import { reportUnresolvedRef } from '../rules/common/no-unresolved-refs.js'; import { type OasRef, type Oas3Discriminator, type Oas3Example } from '../typings/openapi.js'; +import { componentNameFromTitle } from '../utils/component-name-from-title.js'; import { dequal } from '../utils/dequal.js'; import { isPlainObject } from '../utils/is-plain-object.js'; import { isString } from '../utils/is-string.js'; import { makeRefId } from '../utils/make-ref-id.js'; -import { toPascalCase } from '../utils/to-pascal-case.js'; import { type Oas3Visitor, type Oas2Visitor } from '../visitors.js'; import { type UserContext, type ResolveResult, type NonUndefined, type Problem } from '../walk.js'; import { type ComponentNamesStrategy } from './bundle-document.js'; @@ -320,14 +320,14 @@ export function makeBundleVisitor({ return dequal(node, target.node); } - function componentNameFromTitle( + function resolveComponentNameFromTitle( target: ComponentTarget, componentsGroup: ComponentsGroup, ctx: UserContext ): { key: string; problem?: Problem } { const { node } = target; const title = isPlainObject(node) && isString(node.title) ? node.title.trim() : ''; - const key = toPascalCase(title).replace(new RegExp(`[^${COMPONENT_NAME_CHARS}]`, 'g'), '-'); + const key = componentNameFromTitle(title); const titleLocation = target.location.child('title'); if (title === '') { @@ -379,7 +379,7 @@ export function makeBundleVisitor({ const componentsGroup = components[componentType]; if (componentNamesStrategy === 'title' && componentType === schemaComponentType) { - const { key, problem } = componentNameFromTitle(target, componentsGroup, ctx); + const { key, problem } = resolveComponentNameFromTitle(target, componentsGroup, ctx); if (!problem) { firstSchemaLocationByName.set(key, target.location.child('title')); return key; diff --git a/packages/core/src/rules/oas3/__tests__/component-name-unique.test.ts b/packages/core/src/rules/oas3/__tests__/component-name-unique.test.ts index b387202fbf..48964a9678 100644 --- a/packages/core/src/rules/oas3/__tests__/component-name-unique.test.ts +++ b/packages/core/src/rules/oas3/__tests__/component-name-unique.test.ts @@ -986,4 +986,190 @@ describe('Oas3 component-name-unique', () => { `); }); }); + + describe('strategy: title', () => { + it('should not report on same filenames with different titles', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.0.0 + components: + schemas: + Test: + type: object + properties: + model: + $ref: '/a/Order.yaml' + request: + $ref: '/b/Order.yaml' + `, + '/foobar.yaml' + ); + const additionalDocuments = [ + { + absoluteRef: '/a/Order.yaml', + body: outdent` + title: Order model + type: object + `, + }, + { + absoluteRef: '/b/Order.yaml', + body: outdent` + title: Order request + type: object + `, + }, + ]; + + const results = await lintDocumentForTest( + { 'component-name-unique': { severity: 'error', strategy: 'title' } }, + document, + additionalDocuments + ); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + + it('should report on different filenames with the same title', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.0.0 + components: + schemas: + Test: + type: object + properties: + user: + $ref: '/a/User.yaml' + account: + $ref: '/b/Account.yaml' + `, + '/foobar.yaml' + ); + const additionalDocuments = [ + { + absoluteRef: '/a/User.yaml', + body: outdent` + title: User account + type: object + `, + }, + { + absoluteRef: '/b/Account.yaml', + body: outdent` + title: User account + type: object + `, + }, + ]; + + const results = await lintDocumentForTest( + { 'component-name-unique': { severity: 'error', strategy: 'title' } }, + document, + additionalDocuments + ); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/", + "reportOnKey": false, + "source": "/a/User.yaml", + }, + ], + "message": "Component 'schemas/UserAccount' is not unique. It is also defined at: + - /b/Account.yaml", + "reference": "https://redocly.com/docs/cli/rules/oas/component-name-unique", + "ruleId": "component-name-unique", + "severity": "error", + "suggest": [], + }, + { + "location": [ + { + "pointer": "#/", + "reportOnKey": false, + "source": "/b/Account.yaml", + }, + ], + "message": "Component 'schemas/UserAccount' is not unique. It is also defined at: + - /a/User.yaml", + "reference": "https://redocly.com/docs/cli/rules/oas/component-name-unique", + "ruleId": "component-name-unique", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should fall back to the filename when a schema has no title', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.0.0 + components: + schemas: + Order: + type: object + Test: + type: object + properties: + order: + $ref: '/a/Order.yaml' + `, + '/foobar.yaml' + ); + const additionalDocuments = [ + { + absoluteRef: '/a/Order.yaml', + body: outdent` + type: object + `, + }, + ]; + + const results = await lintDocumentForTest( + { 'component-name-unique': { severity: 'error', strategy: 'title' } }, + document, + additionalDocuments + ); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Order", + "reportOnKey": false, + "source": "/foobar.yaml", + }, + ], + "message": "Component 'schemas/Order' is not unique. It is also defined at: + - /a/Order.yaml", + "reference": "https://redocly.com/docs/cli/rules/oas/component-name-unique", + "ruleId": "component-name-unique", + "severity": "error", + "suggest": [], + }, + { + "location": [ + { + "pointer": "#/", + "reportOnKey": false, + "source": "/a/Order.yaml", + }, + ], + "message": "Component 'schemas/Order' is not unique. It is also defined at: + - /foobar.yaml#/components/schemas/Order", + "reference": "https://redocly.com/docs/cli/rules/oas/component-name-unique", + "ruleId": "component-name-unique", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + }); }); diff --git a/packages/core/src/rules/oas3/component-name-unique.ts b/packages/core/src/rules/oas3/component-name-unique.ts index e0be1241c0..e31f3ca1cb 100644 --- a/packages/core/src/rules/oas3/component-name-unique.ts +++ b/packages/core/src/rules/oas3/component-name-unique.ts @@ -10,6 +10,9 @@ import type { Oas3_1Schema, OasRef, } from '../../typings/openapi.js'; +import { componentNameFromTitle } from '../../utils/component-name-from-title.js'; +import { isPlainObject } from '../../utils/is-plain-object.js'; +import { isString } from '../../utils/is-string.js'; import { isSupportedExtension } from '../../utils/is-supported-extension.js'; import type { Oas2Rule, Oas3Rule, Oas3Visitor } from '../../visitors.js'; import type { Problem, UserContext } from '../../walk.js'; @@ -32,6 +35,8 @@ type ComponentsMapValue = { absolutePointers: Set; locations: Location[] export const ComponentNameUnique: Oas3Rule | Oas2Rule = (options) => { const components = new Map(); + const useTitleStrategy = options.strategy === 'title'; + let rootSourceRef: string; const typeNames: string[] = []; if (options.schemas !== 'off') { @@ -55,11 +60,19 @@ export const ComponentNameUnique: Oas3Rule | Oas2Rule = (options) => { const resolvedRef = resolve(ref); if (!resolvedRef.location) return; - addComponentFromAbsoluteLocation(typeName, resolvedRef.location); + const titleName = getTitleComponentName(typeName, resolvedRef); + if (titleName) { + addFoundComponent(typeName, titleName, resolvedRef.location); + } else { + addComponentFromAbsoluteLocation(typeName, resolvedRef.location); + } } }, }, Root: { + enter(_: AnyOas3Definition, { location }: UserContext) { + rootSourceRef = location.source.absoluteRef; + }, leave(root: AnyOas3Definition, ctx: UserContext) { components.forEach((value, key, _) => { if (value.absolutePointers.size > 1) { @@ -147,6 +160,23 @@ export const ComponentNameUnique: Oas3Rule | Oas2Rule = (options) => { const componentName = getComponentNameFromAbsoluteLocation(location.absolutePointer.toString()); addFoundComponent(typeName, componentName, location); } + + function getTitleComponentName( + typeName: string, + resolved: { node: unknown; location: Location } + ): string | null { + if ( + !useTitleStrategy || + typeName !== TYPE_NAME_SCHEMA || + resolved.location.source.absoluteRef === rootSourceRef + ) { + return null; + } + + const { node } = resolved; + const title = isPlainObject(node) && isString(node.title) ? node.title.trim() : ''; + return title === '' ? null : componentNameFromTitle(title); + } }; function getOptionComponentNameForTypeName(typeName: string): string | null { diff --git a/packages/core/src/utils/component-name-from-title.ts b/packages/core/src/utils/component-name-from-title.ts new file mode 100644 index 0000000000..cd5d67cd54 --- /dev/null +++ b/packages/core/src/utils/component-name-from-title.ts @@ -0,0 +1,6 @@ +import { COMPONENT_NAME_CHARS } from '../oas-types.js'; +import { toPascalCase } from './to-pascal-case.js'; + +export function componentNameFromTitle(title: string): string { + return toPascalCase(title).replace(new RegExp(`[^${COMPONENT_NAME_CHARS}]`, 'g'), '-'); +}