Repository: Redocly/openapi-cli (ts)

Issue:
The remove-x-internal decorator doesn't remove references to x-internal components
Let's say we're referencing two parameter components, one internal:
```
      parameters:
        - $ref: '#/components/parameters/ProductID'
        - $ref: '#/components/parameters/Vendor'
```
```
  parameters:
    ProductID:
      in: path
      name: product_id
      description: The ID of the product.
      required: true
      schema:
        type: string
        format: uuid
    Vendor:
      in: header
      name: X-Vendor
      description: The vendor.
      schema:
        type: string
      x-internal: true
```

Once we run the remove-x-internal decorator, the Vendor component will be removed, but the reference to it will remain. Perhaps because the order of the removals is wrong (we remove the component then check the reference?)

Full repro: https://github.com/bojanz/openapi-cli-bug/tree/builtin-decorator

Pull request:
fix: remove-x-internal decorator doesn't remove references to x-internal components
## What/Why/How?
Closes #501


## Check yourself
- [x] Code is linted
- [ ] Tested with redoc/reference-docs/workflows
- [x] All new/updated code is covered with tests

## Security
- [x] Security impact of change has been considered
- [x] Code follows company security practices and guidelines


Patch:
diff --git a/packages/core/src/rules/common/remove-x-internal.ts b/packages/core/src/rules/common/remove-x-internal.ts
index 768e4190..61e86aba 100644
--- a/packages/core/src/rules/common/remove-x-internal.ts
+++ b/packages/core/src/rules/common/remove-x-internal.ts
@@ -1,6 +1,7 @@
 import { Oas3Decorator, Oas2Decorator } from '../../visitors';
 import { isEmptyArray, isEmptyObject, isPlainObject } from '../../utils';
 import { UserContext } from '../../walk';
+import { isRef } from '../../ref-utils';
 
 const DEFAULT_INTERNAL_PROPERTY_NAME = 'x-internal';
 
@@ -12,7 +13,15 @@ export const RemoveXInternal: Oas3Decorator | Oas2Decorator = ({ internalFlagPro
     let didDelete = false;
     if (Array.isArray(node)) {
       for (let i = 0; i < node.length; i++) {
-        if (node[i] && node[i][hiddenTag]) {
+        if (isRef(node[i])) {
+          const resolved = ctx.resolve(node[i]);
+          if (resolved.node?.[hiddenTag]) {
+            node.splice(i, 1);
+            didDelete = true;
+            i--;
+          }
+        }
+        if (node[i]?.[hiddenTag]) {
           node.splice(i, 1);
           didDelete = true;
           i--;
@@ -20,8 +29,16 @@ export const RemoveXInternal: Oas3Decorator | Oas2Decorator = ({ internalFlagPro
       }
     } else if (isPlainObject(node)) {
       for (const key of Object.keys(node)) {
-        if ((node as any)[key][hiddenTag]) {
-          delete (node as any)[key];
+        node = node as any;
+        if (isRef(node[key])) {
+          const resolved = ctx.resolve(node[key]);
+          if (resolved.node?.[hiddenTag]) {
+            delete node[key];
+            didDelete = true;
+          }
+        }
+        if (node[key]?.[hiddenTag]) {
+          delete node[key];
           didDelete = true;
         }
       }


Test patch:
diff --git a/packages/core/src/rules/__tests__/hide-internals.test.ts b/packages/core/src/rules/__tests__/hide-internals.test.ts
index 28d738e3..30f4c13c 100644
--- a/packages/core/src/rules/__tests__/hide-internals.test.ts
+++ b/packages/core/src/rules/__tests__/hide-internals.test.ts
@@ -1,5 +1,5 @@
 import { outdent } from 'outdent';
-import { bundleDocument } from '../../bundle'
+import { bundleDocument } from '../../bundle';
 import { BaseResolver } from '../../resolve';
 import { parseYamlToDocument, yamlSerializer } from '../../../__tests__/utils';
 import { makeConfig } from './config';
@@ -19,23 +19,23 @@ describe('oas3 remove-x-internal', () => {
         parameters:
           x:
             name: x
-    `);
+    `,
+  );
 
   it('should use `internalFlagProperty` option to remove internal paths', async () => {
     const { bundle: res } = await bundleDocument({
       document: testDocument,
       externalRefResolver: new BaseResolver(),
-      config: makeConfig({}, { 'remove-x-internal': { 'internalFlagProperty': 'removeit' } })
+      config: makeConfig({}, { 'remove-x-internal': { internalFlagProperty: 'removeit' } }),
     });
-    expect(res.parsed).toMatchInlineSnapshot(
-    `
-    openapi: 3.0.0
-    components:
-      parameters:
-        x:
-          name: x
+    expect(res.parsed).toMatchInlineSnapshot(`
+          openapi: 3.0.0
+          components:
+            parameters:
+              x:
+                name: x
 
-    `);
+        `);
   });
 
   it('should clean types: Server, Operation, Parameter, PathItem, Example', async () => {
@@ -88,14 +88,14 @@ describe('oas3 remove-x-internal', () => {
               name: x
             y:
               name: y
-      `);
-      const { bundle: res } = await bundleDocument({
-        document: testDoc,
-        externalRefResolver: new BaseResolver(),
-        config: makeConfig({}, { 'remove-x-internal': 'on' })
-      });
-      expect(res.parsed).toMatchInlineSnapshot(
-      `
+      `,
+    );
+    const { bundle: res } = await bundleDocument({
+      document: testDoc,
+      externalRefResolver: new BaseResolver(),
+      config: makeConfig({}, { 'remove-x-internal': 'on' }),
+    });
+    expect(res.parsed).toMatchInlineSnapshot(`
       openapi: 3.1.0
       paths:
         /pet:
@@ -115,8 +115,7 @@ describe('oas3 remove-x-internal', () => {
           'y':
             name: 'y'
 
-      `
-      );
+      `);
   });
 
   it('should clean types: Schema, Response, RequestBody, MediaType, Callback', async () => {
@@ -162,14 +161,15 @@ describe('oas3 remove-x-internal', () => {
                     servers:
                       - url: //callback-url.path-level/v1
                         description: Path level server
-      `);
-      const { bundle: res } = await bundleDocument({
-        document: testDoc,
-        externalRefResolver: new BaseResolver(),
-        config: makeConfig({}, { 'remove-x-internal': 'on' })
-      });
-      expect(res.parsed).toMatchInlineSnapshot(
-      `
+      `,
+    );
+    const { bundle: res } = await bundleDocument({
+      document: testDoc,
+      externalRefResolver: new BaseResolver(),
+      config: makeConfig({}, { 'remove-x-internal': 'on' }),
+    });
+
+    expect(res.parsed).toMatchInlineSnapshot(`
       openapi: 3.1.0
       paths:
         /pet:
@@ -185,6 +185,90 @@ describe('oas3 remove-x-internal', () => {
 
       `);
   });
+
+  it('should remove refs', async () => {
+    const testDoc = parseYamlToDocument(
+      outdent`
+        openapi: 3.0.1
+        info:
+          version: 1.0.0
+          title: Test API
+        paths:
+          /test1:
+            get:
+              parameters:
+                - $ref: '#/components/parameters/Internal'
+                - in: query
+                  name: inline
+                  schema:
+                    type: string
+                - in: query
+                  name: inline-internal
+                  schema:
+                    type: string
+                  x-internal: true
+          /test2:
+            get:
+              parameters:
+                - $ref: '#/components/parameters/Public'
+                - $ref: '#/components/parameters/Internal'
+              requestBody:
+                $ref: '#/components/requestBodies/Public'
+        components:
+          requestBodies:
+            Public:
+              content:
+                application/json:
+                  schema:
+                    type: string
+              required: true
+              x-internal: true
+          parameters:
+            Public:
+              in: path
+              name: product_id
+              schema:
+                type: string
+            Internal:
+              in: header
+              name: X-Vendor
+              schema:
+                type: string
+              x-internal: true
+      `,
+    );
+    const { bundle: res } = await bundleDocument({
+      document: testDoc,
+      externalRefResolver: new BaseResolver(),
+      config: makeConfig({}, { 'remove-x-internal': 'on' }),
+    });
+    expect(res.parsed).toMatchInlineSnapshot(`
+      openapi: 3.0.1
+      info:
+        version: 1.0.0
+        title: Test API
+      paths:
+        /test1:
+          get:
+            parameters:
+              - in: query
+                name: inline
+                schema:
+                  type: string
+        /test2:
+          get:
+            parameters:
+              - $ref: '#/components/parameters/Public'
+      components:
+        parameters:
+          Public:
+            in: path
+            name: product_id
+            schema:
+              type: string
+
+    `);
+  });
 });
 
 describe('oas2 remove-x-internal', () => {
@@ -214,21 +298,20 @@ describe('oas2 remove-x-internal', () => {
                 '200':
                   x-internal: true
                   description: List of recent media entries.
-      `);
+      `,
+    );
     const { bundle: res } = await bundleDocument({
       document: testDoc,
       externalRefResolver: new BaseResolver(),
-      config: makeConfig({}, { 'remove-x-internal': 'on' })
+      config: makeConfig({}, { 'remove-x-internal': 'on' }),
     });
-    expect(res.parsed).toMatchInlineSnapshot(
-    `
-    swagger: '2.0'
-    host: api.instagram.com
-    paths:
-      /geographies/{geo-id}/media/recent:
-        get: {}
+    expect(res.parsed).toMatchInlineSnapshot(`
+          swagger: '2.0'
+          host: api.instagram.com
+          paths:
+            /geographies/{geo-id}/media/recent:
+              get: {}
 
-    `
-    );
+        `);
   });
 });