Repository: fabien0102/ts-to-zod
Pull request #139 (2023-05-31 16:38:25)

Refactoring of getExtractedTypeNames
# Why

When working on another PR (https://github.com/fabien0102/ts-to-zod/pull/135), I found out that the current `getExtractedTypeNames` actually works, for most cases, "by mistake" because it's always run for all nodes in the file and types are always referenced in the file (so an error in one call would "self-heal" in a subsequent one).

I added some tests (see commit https://github.com/tvillaren/ts-to-zod/commit/9a2294ad0d1066c5722b4bc390f1befcb8870c63 for the tests added to the original file, some not passing) and refactored the method, everything is still green.


codecov-commenter: ## [Codecov](https://app.codecov.io/gh/fabien0102/ts-to-zod/pull/139?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fabien+BERNARD) Report
> Merging [#139](https://app.codecov.io/gh/fabien0102/ts-to-zod/pull/139?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fabien+BERNARD) (96797cc) into [main](https://app.codecov.io/gh/fabien0102/ts-to-zod/commit/8324ee49470b85b13ef5848dd4a694b0811e0687?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fabien+BERNARD) (8324ee4) will **increase** coverage by `0.54%`.
> The diff coverage is `70.58%`.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please [install the Github App Integration](https://github.com/apps/codecov) for your organization. [Read more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fabien+BERNARD).

```diff
@@            Coverage Diff             @@
##             main     #139      +/-   ##
==========================================
+ Coverage   96.04%   96.58%   +0.54%     
==========================================
  Files          13       13              
  Lines         657      645      -12     
  Branches      265      262       -3     
==========================================
- Hits          631      623       -8     
+ Misses         26       22       -4     
```


| [Impacted Files](https://app.codecov.io/gh/fabien0102/ts-to-zod/pull/139?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fabien+BERNARD) | Coverage Δ | |
|---|---|---|
| [src/utils/traverseTypes.ts](https://app.codecov.io/gh/fabien0102/ts-to-zod/pull/139?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fabien+BERNARD#diff-c3JjL3V0aWxzL3RyYXZlcnNlVHlwZXMudHM=) | `76.19% <68.75%> (+3.46%)` | :arrow_up: |
| [src/core/generate.ts](https://app.codecov.io/gh/fabien0102/ts-to-zod/pull/139?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Fabien+BERNARD#diff-c3JjL2NvcmUvZ2VuZXJhdGUudHM=) | `96.11% <100.00%> (ø)` | |


tvillaren: Hello @fabien0102, 

Working on handling import statement, I came upon a potential self-healing issue which I propose to fix (with a bunch of tests to cover our backs 😉). 

WDYT?

Patch:
diff --git a/src/core/generate.ts b/src/core/generate.ts
index 334a6b3..660ae45 100644
--- a/src/core/generate.ts
+++ b/src/core/generate.ts
@@ -95,11 +95,7 @@ export function generate({
       if (!jsDocTagFilter(tags)) return;
       if (!nameFilter(node.name.text)) return;
 
-      const typeNames = getExtractedTypeNames(
-        node,
-        sourceFile,
-        typeNameMapping
-      );
+      const typeNames = getExtractedTypeNames(node, sourceFile);
       typeNames.forEach((typeName) => {
         typesNeedToBeExtracted.add(typeName);
       });
diff --git a/src/utils/traverseTypes.ts b/src/utils/traverseTypes.ts
index 02e0a1d..edff2f5 100644
--- a/src/utils/traverseTypes.ts
+++ b/src/utils/traverseTypes.ts
@@ -1,10 +1,9 @@
 import ts from "typescript";
 
-export type TypeNode = (
+export type TypeNode =
   | ts.InterfaceDeclaration
   | ts.TypeAliasDeclaration
-  | ts.EnumDeclaration
-) & { visited?: boolean };
+  | ts.EnumDeclaration;
 
 export function isTypeNode(node: ts.Node): node is TypeNode {
   return (
@@ -16,56 +15,54 @@ export function isTypeNode(node: ts.Node): node is TypeNode {
 
 export function getExtractedTypeNames(
   node: TypeNode,
-  sourceFile: ts.SourceFile,
-  typeNameMapping: Map<string, TypeNode>
-) {
-  const referenceTypeNames: string[] = [];
+  sourceFile: ts.SourceFile
+): string[] {
+  const referenceTypeNames = new Set<string>();
+  referenceTypeNames.add(node.name.text);
 
-  const recursiveExtract = (node: TypeNode) => {
-    if (node.visited) {
-      return;
-    }
-
-    const heritageClauses = (node as ts.InterfaceDeclaration).heritageClauses;
-
-    if (heritageClauses) {
-      heritageClauses.forEach((clause) => {
-        const extensionTypes = clause.types;
-        extensionTypes.forEach((extensionTypeNode) => {
-          const typeName = extensionTypeNode.expression.getText(sourceFile);
-          const typeNode = typeNameMapping.get(typeName);
+  const heritageClauses = (node as ts.InterfaceDeclaration).heritageClauses;
 
-          referenceTypeNames.push(typeName);
+  if (heritageClauses) {
+    heritageClauses.forEach((clause) => {
+      const extensionTypes = clause.types;
+      extensionTypes.forEach((extensionTypeNode) => {
+        const typeName = extensionTypeNode.expression.getText(sourceFile);
 
-          if (typeNode) {
-            typeNode.visited = true;
-            recursiveExtract(typeNode);
-          }
-        });
+        referenceTypeNames.add(typeName);
       });
-    }
-
-    node.forEachChild((child) => {
-      const childNode = child as ts.PropertySignature;
-      if (childNode.kind !== ts.SyntaxKind.PropertySignature) {
-        return;
-      }
-
-      if (childNode.type?.kind === ts.SyntaxKind.TypeReference) {
-        const typeNode = typeNameMapping.get(
-          childNode.type.getText(sourceFile)
-        );
+    });
+  }
 
-        referenceTypeNames.push(childNode.type.getText(sourceFile));
+  const visitorExtract = (child: ts.Node) => {
+    const childNode = child as ts.PropertySignature;
+    if (!ts.isPropertySignature(childNode)) {
+      return;
+    }
 
-        if (typeNode) {
-          typeNode.visited = true;
-          recursiveExtract(typeNode);
-        }
+    if (childNode.type) {
+      if (ts.isTypeReferenceNode(childNode.type)) {
+        referenceTypeNames.add(childNode.type.getText(sourceFile));
+      } else if (
+        ts.isArrayTypeNode(childNode.type) &&
+        ts.isTypeNode(childNode.type.elementType)
+      ) {
+        referenceTypeNames.add(childNode.type.elementType.getText(sourceFile));
+      } else if (ts.isTypeLiteralNode(childNode.type)) {
+        childNode.type.forEachChild(visitorExtract);
+      } else if (
+        ts.isIntersectionTypeNode(childNode.type) ||
+        ts.isUnionTypeNode(childNode.type)
+      ) {
+        childNode.type.types.forEach((typeNode: ts.TypeNode) => {
+          if (ts.isTypeReferenceNode(typeNode)) {
+            referenceTypeNames.add(typeNode.getText(sourceFile));
+          } else typeNode.forEachChild(visitorExtract);
+        });
       }
-    });
+    }
   };
 
-  recursiveExtract(node);
-  return [node.name.text, ...referenceTypeNames];
+  node.forEachChild(visitorExtract);
+
+  return Array.from(referenceTypeNames);
 }


Test patch:
diff --git a/src/utils/traverseTypes.test.ts b/src/utils/traverseTypes.test.ts
new file mode 100644
index 0000000..0f791af
--- /dev/null
+++ b/src/utils/traverseTypes.test.ts
@@ -0,0 +1,148 @@
+import ts from "typescript";
+import { getExtractedTypeNames } from "./traverseTypes";
+import { findNode } from "./findNode";
+
+describe("traverseTypes", () => {
+  describe("getExtractedTypeNames", () => {
+    it("should find only itself", () => {
+      const testCases = [
+        `
+        export type Superhero = {
+            id: number, 
+            name: string
+          };
+        `,
+        `
+        export interface Superhero = {
+            id: number, 
+            name: string
+          };
+        `,
+        `
+        export enum Superhero = {
+            Superman = "superman",
+            ClarkKent = "clark_kent",
+          };
+        `,
+      ];
+
+      testCases.forEach((source: string) => {
+        const result = extractNames(source);
+        expect(result).toEqual(["Superhero"]);
+      });
+    });
+
+    it("should extract type referenced in property", () => {
+      const source = `
+        export interface Superhero {
+            id: number,
+            person: Person,
+        }`;
+
+      const result = extractNames(source);
+      expect(result).toEqual(["Superhero", "Person"]);
+    });
+
+    it("should extract type referenced in extend clause", () => {
+      const source = `
+          export interface Superhero extends Person {
+              id: number,
+          }`;
+
+      const result = extractNames(source);
+      expect(result).toEqual(["Superhero", "Person"]);
+    });
+
+    it("should extract type referenced in multiple extend clauses", () => {
+      const source = `
+            export interface Superhero extends Person, Person2 {
+                id: number,
+            }`;
+
+      const result = extractNames(source);
+      expect(result).toEqual(["Superhero", "Person", "Person2"]);
+    });
+
+    it("should extract type referenced in property as array", () => {
+      const source = `
+            export interface Superhero {
+                id: number,
+                sidekicks: Person[],
+            }`;
+
+      const result = extractNames(source);
+      expect(result).toEqual(["Superhero", "Person"]);
+    });
+
+    it("should extract nested type reference", () => {
+      const source = `
+          export interface Superhero {
+              id: number,
+              person: {
+                type: Person,
+              }
+          }`;
+
+      const result = extractNames(source);
+      expect(result).toEqual(["Superhero", "Person"]);
+    });
+
+    it("should extract union type reference", () => {
+      const source = `
+        export interface Person {
+            id: number,
+            t: SuperHero | Villain
+        }`;
+
+      const result = extractNames(source);
+      expect(result).toEqual(["Person", "SuperHero", "Villain"]);
+    });
+
+    it("should extract intersection type reference", () => {
+      const source = `
+        export interface Person {
+            id: number,
+            t: SuperHero & Villain
+        }`;
+
+      const result = extractNames(source);
+      expect(result).toEqual(["Person", "SuperHero", "Villain"]);
+    });
+
+    it("should extract intersection type reference with type literal", () => {
+      const source = `
+        export interface Person {
+            id: number,
+            t: SuperHero & { counter: Villain }
+        }`;
+
+      const result = extractNames(source);
+      expect(result).toEqual(["Person", "SuperHero", "Villain"]);
+    });
+  });
+});
+
+function extractNames(sourceText: string) {
+  const sourceFile = ts.createSourceFile(
+    "index.ts",
+    sourceText,
+    ts.ScriptTarget.Latest
+  );
+  const declaration = findNode(
+    sourceFile,
+    (
+      node
+    ): node is
+      | ts.InterfaceDeclaration
+      | ts.TypeAliasDeclaration
+      | ts.EnumDeclaration =>
+      ts.isInterfaceDeclaration(node) ||
+      ts.isTypeAliasDeclaration(node) ||
+      ts.isEnumDeclaration(node)
+  );
+  if (!declaration) {
+    throw new Error("No `type` or `interface` found!");
+  }
+
+  return getExtractedTypeNames(declaration, sourceFile);
+}