fix: remove unused components was skipping schemas with a top-level a…

…llOf. fixes #1209

.changeset/light-games-clap.md

@@ -0,0 +1,6 @@
+---
+'@redocly/openapi-core': minor
+'@redocly/cli': minor
+---
+
+Fixed an issue where remove-unused-components would not remove schemas which had a top-level allOf and improved detection of unused schemas in no-unused-components rule.

packages/core/src/rules/oas2/__tests__/remove-unused-components.test.ts

@@ -1,5 +1,6 @@
 import { outdent } from 'outdent';
-import { parseYamlToDocument, replaceSourceWithRef, makeConfig } from '../../../../__tests__/utils';
+import { cloneDeep } from 'lodash';
+import { parseYamlToDocument, makeConfig } from '../../../../__tests__/utils';
 import { bundleDocument } from '../../../bundle';
 import { BaseResolver } from '../../../resolve';
 
@@ -152,4 +153,68 @@ describe('oas2 remove-unused-components', () => {
  },
  });
  });
+
+ it('should remove unused components even if they have a root allOf', async () => {
+ const document = parseYamlToDocument(
+ outdent`
+ swagger: '2.0'
+ paths:
+ /pets:
+ get:
+ produces:
+ - application/json
+ parameters: []
+ responses:
+ '200':
+ schema:
+ items:
+ $ref: '#/definitions/Pet'
+ type: array
+ operationId: listPets
+ summary: List all pets
+ definitions:
+ Cat:
+ allOf:
+ - $ref: '#/definitions/Pet'
+ - properties:
+ name:
+ type: string
+ type: object
+ Dog:
+ allOf:
+ - $ref: '#/definitions/Pet'
+ - properties:
+ bark:
+ type: string
+ type: object
+ Lizard:
+ allOf:
+ - $ref: '#/definitions/Pet'
+ - properties:
+ lovesRocks:
+ type: boolean
+ type: object
+ Pet:
+ discriminator: petType
+ properties:
+ petType:
+ type: string
+ required:
+ - petType
+ type: object
+ `,
+ 'foobar.yaml'
+ );
+
+ const orig = cloneDeep(document.parsed);
+
+ const results = await bundleDocument({
+ externalRefResolver: new BaseResolver(),
+ document,
+ config: await makeConfig({}),
+ removeUnusedComponents: true,
+ });
+
+ expect(results.bundle.parsed).toEqual(orig);
+ });
 });

packages/core/src/rules/oas2/remove-unused-components.ts

@@ -12,10 +12,11 @@ export const RemoveUnusedComponents: Oas2Rule = () => {
  function registerComponent(
  location: Location,
  componentType: keyof Oas2Components,
- name: string
+ name: string,
+ used = false
  ): void {
  components.set(location.absolutePointer, {
- used: components.get(location.absolutePointer)?.used || false,
+ used: components.get(location.absolutePointer)?.used || used,
  componentType,
  name,
  });
@@ -61,10 +62,13 @@ export const RemoveUnusedComponents: Oas2Rule = () => {
  },
  },
  NamedSchemas: {
- Schema(schema, { location, key }) {
- if (!schema.allOf) {
- registerComponent(location, 'definitions', key.toString());
- }
+ Schema(schema, { location, key, resolve }) {
+ registerComponent(
+ location,
+ 'definitions',
+ key.toString(),
+ schema.allOf && schema.allOf.some((v) => v.$ref && resolve(v).node?.discriminator)
+ );
  },
  },
  NamedParameters: {

packages/core/src/rules/oas3/__tests__/no-unused-components.test.ts

@@ -128,4 +128,152 @@ describe('Oas3 no-unused-components', () => {
  ]
  `);
  });
+
+ it('should properly handle allOf when reporting unused components', async () => {
+ const document = parseYamlToDocument(
+ outdent`
+ openapi: "3.0.0"
+ paths:
+ /pets:
+ get:
+ summary: List all pets
+ operationId: listPets
+ parameters:
+ - $ref: '#/components/parameters/used'
+ components:
+ parameters:
+ used:
+ name: used
+ unused:
+ name: unused
+ responses:
+ unused: {}
+ examples:
+ unused: {}
+ requestBodies:
+ unused: {}
+ headers:
+ unused: {}
+ schemas:
+ Unused:
+ allOf:
+ - type: object
+ properties:
+ one:
+ type: string
+ - type: object
+ properties:
+ two:
+ type: string
+ # this is referenced so is considered used
+ Pet:
+ type: object
+ properties:
+ petType:
+ type: string
+ discriminator:
+ propertyName: petType
+ required:
+ - petType
+ # this is potentially used
+ Cat:
+ allOf:
+ - $ref: '#/components/schemas/Pet'
+ - type: object
+ properties:
+ name:
+ type: string
+ `,
+ 'foobar.yaml'
+ );
+
+ const results = await lintDocument({
+ externalRefResolver: new BaseResolver(),
+ document,
+ config: await makeConfig({ 'no-unused-components': 'error' }),
+ });
+
+ expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`
+ Array [
+ Object {
+ "location": Array [
+ Object {
+ "pointer": "#/components/parameters/unused",
+ "reportOnKey": true,
+ "source": "foobar.yaml",
+ },
+ ],
+ "message": "Component: \\"unused\\" is never used.",
+ "ruleId": "no-unused-components",
+ "severity": "error",
+ "suggest": Array [],
+ },
+ Object {
+ "location": Array [
+ Object {
+ "pointer": "#/components/schemas/Unused",
+ "reportOnKey": true,
+ "source": "foobar.yaml",
+ },
+ ],
+ "message": "Component: \\"Unused\\" is never used.",
+ "ruleId": "no-unused-components",
+ "severity": "error",
+ "suggest": Array [],
+ },
+ Object {
+ "location": Array [
+ Object {
+ "pointer": "#/components/responses/unused",
+ "reportOnKey": true,
+ "source": "foobar.yaml",
+ },
+ ],
+ "message": "Component: \\"unused\\" is never used.",
+ "ruleId": "no-unused-components",
+ "severity": "error",
+ "suggest": Array [],
+ },
+ Object {
+ "location": Array [
+ Object {
+ "pointer": "#/components/examples/unused",
+ "reportOnKey": true,
+ "source": "foobar.yaml",
+ },
+ ],
+ "message": "Component: \\"unused\\" is never used.",
+ "ruleId": "no-unused-components",
+ "severity": "error",
+ "suggest": Array [],
+ },
+ Object {
+ "location": Array [
+ Object {
+ "pointer": "#/components/requestBodies/unused",
+ "reportOnKey": true,
+ "source": "foobar.yaml",
+ },
+ ],
+ "message": "Component: \\"unused\\" is never used.",
+ "ruleId": "no-unused-components",
+ "severity": "error",
+ "suggest": Array [],
+ },
+ Object {
+ "location": Array [
+ Object {
+ "pointer": "#/components/headers/unused",
+ "reportOnKey": true,
+ "source": "foobar.yaml",
+ },
+ ],
+ "message": "Component: \\"unused\\" is never used.",
+ "ruleId": "no-unused-components",
+ "severity": "error",
+ "suggest": Array [],
+ },
+ ]
+ `);
+ });
 });