fix: remove unused components transitively (#1472)

.changeset/ten-fireants-reflect.md

@@ -0,0 +1,5 @@
+---
+"@redocly/openapi-core": patch
+---
+
+Process remove-unused-components rule transitively; components are now removed if they were previously referenced by a removed component.

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

@@ -1,5 +1,5 @@
 import { outdent } from 'outdent';
-import { parseYamlToDocument, replaceSourceWithRef, makeConfig } from '../../../../__tests__/utils';
+import { parseYamlToDocument, makeConfig } from '../../../../__tests__/utils';
 import { bundleDocument } from '../../../bundle';
 import { BaseResolver } from '../../../resolve';
 
@@ -152,4 +152,77 @@ describe('oas2 remove-unused-components', () => {
       },
     });
   });
+
+  it('should remove transitively unused components', async () => {
+    const document = parseYamlToDocument(
+      outdent`
+        swagger: '2.0'
+        paths:
+          /pets:
+            get:
+              produces:
+                - application/json
+              parameters: []
+              responses:
+                '200':
+                  schema:
+                    $ref: '#/definitions/Used'
+              operationId: listPets
+              summary: List all pets
+        definitions:
+          Unused:
+            enum:
+              - 1
+              - 2
+            type: integer
+          UnusedTransitive:
+            type: object
+            properties:
+              link:
+                $ref: '#/definitions/Unused'
+          Used:
+            properties:
+              link:
+                type: string
+            type: object
+        `,
+      'foobar.yaml'
+    );
+
+    const results = await bundleDocument({
+      externalRefResolver: new BaseResolver(),
+      document,
+      config: await makeConfig({}),
+      removeUnusedComponents: true,
+    });
+
+    expect(results.bundle.parsed).toEqual({
+      swagger: '2.0',
+      definitions: {
+        Used: {
+          properties: {
+            link: { type: 'string' },
+          },
+          type: 'object',
+        },
+      },
+      paths: {
+        '/pets': {
+          get: {
+            produces: ['application/json'],
+            parameters: [],
+            summary: 'List all pets',
+            operationId: 'listPets',
+            responses: {
+              '200': {
+                schema: {
+                  $ref: '#/definitions/Used',
+                },
+              },
+            },
+          },
+        },
+      },
+    });
+  });
 });

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

@@ -2,12 +2,12 @@ import { Location } from '../../ref-utils';
 import { isEmptyObject } from '../../utils';
 
 import type { Oas2Decorator } from '../../visitors';
-import type { Oas2Components } from '../../typings/swagger';
+import type { Oas2Components, Oas2Definition } from '../../typings/swagger';
 
 export const RemoveUnusedComponents: Oas2Decorator = () => {
   const components = new Map<
     string,
-    { used: boolean; componentType?: keyof Oas2Components; name: string }
+    { usedIn: Location[]; componentType?: keyof Oas2Components; name: string }
   >();
 
   function registerComponent(
@@ -16,15 +16,46 @@ export const RemoveUnusedComponents: Oas2Decorator = () => {
     name: string
   ): void {
     components.set(location.absolutePointer, {
-      used: components.get(location.absolutePointer)?.used || false,
+      usedIn: components.get(location.absolutePointer)?.usedIn ?? [],
       componentType,
       name,
     });
   }
 
+  function removeUnusedComponents(root: Oas2Definition, removedPaths: string[]): number {
+    const removedLengthStart = removedPaths.length;
+
+    for (const [path, { usedIn, name, componentType }] of components) {
+      const used = usedIn.some(
+        (location) =>
+          !removedPaths.some(
+            (removed) =>
+              // Check if the current location's absolute pointer starts with the 'removed' path
+              // and either its length matches exactly with 'removed' or the character after the 'removed' path is a '/'
+              location.absolutePointer.startsWith(removed) &&
+              (location.absolutePointer.length === removed.length ||
+                location.absolutePointer[removed.length] === '/')
+          )
+      );
+      if (!used && componentType) {
+        removedPaths.push(path);
+        delete root[componentType]![name];
+        components.delete(path);
+
+        if (isEmptyObject(root[componentType])) {
+          delete root[componentType];
+        }
+      }
+    }
+
+    return removedPaths.length > removedLengthStart
+      ? removeUnusedComponents(root, removedPaths)
+      : removedPaths.length;
+  }
+
   return {
     ref: {
-      leave(ref, { type, resolve, key }) {
+      leave(ref, { location, type, resolve, key }) {
         if (['Schema', 'Parameter', 'Response', 'SecurityScheme'].includes(type.name)) {
           const resolvedRef = resolve(ref);
           if (!resolvedRef.location) return;
@@ -33,32 +64,23 @@ export const RemoveUnusedComponents: Oas2Decorator = () => {
           const componentLevelLocalPointer = localPointer.split('/').slice(0, 3).join('/');
           const pointer = `${fileLocation}#${componentLevelLocalPointer}`;
 
-          components.set(pointer, {
+          const registered = components.get(pointer);
-            used: true,
+
-            name: key.toString(),
+          if (registered) {
-          });
+            registered.usedIn.push(location);
+          } else {
+            components.set(pointer, {
+              usedIn: [location],
+              name: key.toString(),
+            });
+          }
         }
       },
     },
     Root: {
       leave(root, ctx) {
         const data = ctx.getVisitorData() as { removedCount: number };
-        data.removedCount = 0;
+        data.removedCount = removeUnusedComponents(root, []);
-
-        const rootComponents = new Set<keyof Oas2Components>();
-        components.forEach((usageInfo) => {
-          const { used, name, componentType } = usageInfo;
-          if (!used && componentType) {
-            rootComponents.add(componentType);
-            delete root[componentType]![name];
-            data.removedCount++;
-          }
-        });
-        for (const component of rootComponents) {
-          if (isEmptyObject(root[component])) {
-            delete root[component];
-          }
-        }
       },
     },
     NamedSchemas: {

packages/core/src/decorators/oas3/__tests__/remove-unused-components.test.ts

@@ -168,4 +168,146 @@ describe('oas3 remove-unused-components', () => {
       },
     });
   });
+
+  it('should remove transitively 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
+          schemas:
+            Unused:
+              type: integer
+              enum:
+                - 1
+                - 2
+            Transitive:
+              type: object
+              properties:
+                link:
+                  $ref: '#/components/schemas/Unused'
+        `,
+      'foobar.yaml'
+    );
+
+    const results = await bundleDocument({
+      externalRefResolver: new BaseResolver(),
+      document,
+      config: await makeConfig({}),
+      removeUnusedComponents: true,
+    });
+
+    expect(results.bundle.parsed).toEqual({
+      openapi: '3.0.0',
+      paths: {
+        '/pets': {
+          get: {
+            summary: 'List all pets',
+            operationId: 'listPets',
+            parameters: [
+              {
+                $ref: '#/components/parameters/used',
+              },
+            ],
+          },
+        },
+      },
+      components: {
+        parameters: {
+          used: {
+            name: 'used',
+          },
+        },
+      },
+    });
+  });
+
+  it('should remove transitively unused components with colliding paths', async () => {
+    const document = parseYamlToDocument(
+      outdent`
+        openapi: "3.0.0"
+        paths:
+          /pets:
+            get:
+              responses: 
+                200:
+                  content: 
+                    application/json: 
+                      schema: 
+                        $ref: "#/components/schemas/Transitive2"
+        components:
+          schemas:
+            Unused: # <-- this will be removed correctly
+              type: integer
+            Transitive: # <-- this will be removed correctly
+              type: object
+              properties:
+                link:
+                  $ref: '#/components/schemas/Unused'
+            Used:
+              type: integer
+            Transitive2:
+              type: object
+              properties:
+                link:
+                  $ref: '#/components/schemas/Used'
+        `,
+
+      'foobar.yaml'
+    );
+
+    const results = await bundleDocument({
+      externalRefResolver: new BaseResolver(),
+      document,
+      config: await makeConfig({}),
+      removeUnusedComponents: true,
+    });
+
+    expect(results.bundle.parsed).toEqual({
+      openapi: '3.0.0',
+      paths: {
+        '/pets': {
+          get: {
+            responses: {
+              200: {
+                content: {
+                  'application/json': {
+                    schema: {
+                      $ref: '#/components/schemas/Transitive2',
+                    },
+                  },
+                },
+              },
+            },
+          },
+        },
+      },
+      components: {
+        schemas: {
+          Transitive2: {
+            type: 'object',
+            properties: {
+              link: {
+                $ref: '#/components/schemas/Used',
+              },
+            },
+          },
+          Used: {
+            type: 'integer',
+          },
+        },
+      },
+    });
+  });
 });

packages/core/src/decorators/oas3/remove-unused-components.ts

@@ -2,12 +2,12 @@ import { Location } from '../../ref-utils';
 import { isEmptyObject } from '../../utils';
 
 import type { Oas3Decorator } from '../../visitors';
-import type { Oas3Components } from '../../typings/openapi';
+import type { Oas3Components, Oas3Definition } from '../../typings/openapi';
 
 export const RemoveUnusedComponents: Oas3Decorator = () => {
   const components = new Map<
     string,
-    { used: boolean; componentType?: keyof Oas3Components; name: string }
+    { usedIn: Location[]; componentType?: keyof Oas3Components; name: string }
   >();
 
   function registerComponent(
@@ -16,15 +16,45 @@ export const RemoveUnusedComponents: Oas3Decorator = () => {
     name: string
   ): void {
     components.set(location.absolutePointer, {
-      used: components.get(location.absolutePointer)?.used || false,
+      usedIn: components.get(location.absolutePointer)?.usedIn ?? [],
       componentType,
       name,
     });
   }
 
+  function removeUnusedComponents(root: Oas3Definition, removedPaths: string[]): number {
+    const removedLengthStart = removedPaths.length;
+
+    for (const [path, { usedIn, name, componentType }] of components) {
+      const used = usedIn.some(
+        (location) =>
+          !removedPaths.some(
+            (removed) =>
+              location.absolutePointer.startsWith(removed) &&
+              (location.absolutePointer.length === removed.length ||
+                location.absolutePointer[removed.length] === '/')
+          )
+      );
+
+      if (!used && componentType && root.components) {
+        removedPaths.push(path);
+        const componentChild = root.components[componentType];
+        delete componentChild![name];
+        components.delete(path);
+        if (isEmptyObject(componentChild)) {
+          delete root.components[componentType];
+        }
+      }
+    }
+
+    return removedPaths.length > removedLengthStart
+      ? removeUnusedComponents(root, removedPaths)
+      : removedPaths.length;
+  }
+
   return {
     ref: {
-      leave(ref, { type, resolve, key }) {
+      leave(ref, { location, type, resolve, key }) {
         if (
           ['Schema', 'Header', 'Parameter', 'Response', 'Example', 'RequestBody'].includes(
             type.name
@@ -37,29 +67,24 @@ export const RemoveUnusedComponents: Oas3Decorator = () => {
           const componentLevelLocalPointer = localPointer.split('/').slice(0, 4).join('/');
           const pointer = `${fileLocation}#${componentLevelLocalPointer}`;
 
-          components.set(pointer, {
+          const registered = components.get(pointer);
-            used: true,
+
-            name: key.toString(),
+          if (registered) {
-          });
+            registered.usedIn.push(location);
+          } else {
+            components.set(pointer, {
+              usedIn: [location],
+              name: key.toString(),
+            });
+          }
         }
       },
     },
     Root: {
       leave(root, ctx) {
         const data = ctx.getVisitorData() as { removedCount: number };
-        data.removedCount = 0;
+        data.removedCount = removeUnusedComponents(root, []);
 
-        components.forEach((usageInfo) => {
-          const { used, componentType, name } = usageInfo;
-          if (!used && componentType && root.components) {
-            const componentChild = root.components[componentType];
-            delete componentChild![name];
-            data.removedCount++;
-            if (isEmptyObject(componentChild)) {
-              delete root.components[componentType];
-            }
-          }
-        });
         if (isEmptyObject(root.components)) {
           delete root.components;
         }