Spruced up convertComments

JoshuaKGoldberg · JoshuaKGoldberg · commit 824b7951d61a · 2020-09-19T14:59:42.000-04:00
diff --git a/docs/Development.md b/docs/Development.md
@@ -21,6 +21,6 @@ Compile with `npm run tsc` and run tests with `npm run test`.
 
 ## Further Reading
 
--   [Architecture](./Architecture.md): How the general app structure operates
+-   [Architecture](./Architecture/README.md): How the general app structure operates
 -   [Dependencies](./Dependencies.md): How functions pass and receive static dependencies
 -   [Testing](./Testing.md): Unit tests
diff --git a/src/converters/comments/convertComments.test.ts b/src/converters/comments/convertComments.test.ts
@@ -9,7 +9,10 @@ const createStubDependencies = (
         include: ["a.ts"],
     }),
     convertFileComments: jest.fn(),
-    globAsync: jest.fn().mockResolvedValue(["a.ts", "b.ts"]),
+    extractGlobPaths: jest.fn().mockResolvedValue({
+        data: ["a.ts", "b.ts"],
+        status: ResultStatus.Succeeded,
+    }),
     reportCommentResults: jest.fn(),
     ...overrides,
 });
@@ -56,77 +59,27 @@ describe("convertComments", () => {
         });
     });
 
-    it("returns the failure result when a file path glob fails", async () => {
+    it("returns the failure result when a globbing file paths fails", async () => {
         // Arrange
         const globAsyncError = new Error();
         const dependencies = createStubDependencies({
-            globAsync: jest.fn().mockResolvedValueOnce(globAsyncError),
-        });
-
-        // Act
-        const result = await convertComments(
-            dependencies,
-            { comments: true },
-            createStubOriginalConfigurationsData({
-                typescript: {
-                    include: ["src/*.ts"],
-                },
+            extractGlobPaths: jest.fn().mockResolvedValueOnce({
+                errors: [globAsyncError],
+                status: ResultStatus.Failed,
             }),
-            new Map(),
-        );
-
-        // Assert
-        expect(result).toEqual({
-            errors: [globAsyncError],
-            status: ResultStatus.Failed,
-        });
-    });
-
-    it("returns an error when there are no resultant file paths", async () => {
-        // Arrange
-        const dependencies = createStubDependencies({
-            collectCommentFileNames: async () => ({
-                include: [],
-            }),
-            globAsync: jest.fn().mockResolvedValueOnce([]),
         });
 
         // Act
         const result = await convertComments(
             dependencies,
-            { comments: [] },
-            createStubOriginalConfigurationsData(),
-            new Map(),
-        );
-
-        // Assert
-        expect(result).toEqual({
-            errors: expect.arrayContaining([expect.any(Error)]),
-            status: ResultStatus.Failed,
-        });
-    });
-
-    it("returns an error when all globbed file paths are excluded", async () => {
-        // Arrange
-        const dependencies = createStubDependencies({
-            collectCommentFileNames: async () => ({
-                exclude: ["*.ts"],
-                include: ["a.ts"],
-            }),
-            globAsync: jest.fn().mockResolvedValueOnce(["a.ts"]),
-        });
-
-        // Act
-        const result = await convertComments(
-            dependencies,
-            { comments: ["*.ts"] },
+            { comments: true },
             createStubOriginalConfigurationsData(),
             new Map(),
         );
 
         // Assert
         expect(result).toEqual({
-            errors: expect.arrayContaining([expect.any(Error)]),
+            errors: [globAsyncError],
             status: ResultStatus.Failed,
         });
     });
diff --git a/src/converters/comments/convertComments.ts b/src/converters/comments/convertComments.ts
@@ -1,27 +1,30 @@
-import minimatch from "minimatch";
-
-import { GlobAsync } from "../../adapters/globAsync";
 import { SansDependencies } from "../../binding";
 import { collectCommentFileNames } from "../../comments/collectCommentFileNames";
 import { AllOriginalConfigurations } from "../../input/findOriginalConfigurations";
 import { ResultWithDataStatus, ResultStatus, TSLintToESLintSettings } from "../../types";
-import { uniqueFromSources, separateErrors, isError } from "../../utils";
+import { isError } from "../../utils";
 import { convertFileComments } from "./convertFileComments";
+import { extractGlobPaths } from "./extractGlobPaths";
 import { reportCommentResults } from "./reporting/reportCommentResults";
 
 export type ConvertCommentsDependencies = {
     collectCommentFileNames: SansDependencies<typeof collectCommentFileNames>;
     convertFileComments: SansDependencies<typeof convertFileComments>;
-    globAsync: GlobAsync;
+    extractGlobPaths: SansDependencies<typeof extractGlobPaths>;
     reportCommentResults: SansDependencies<typeof reportCommentResults>;
 };
 
+/**
+ * Root-level driver to convert a tslint:disable comments to eslint-disables.
+ * @see `/docs/Architecture/Comments.md` for documentation.
+ */
 export const convertComments = async (
     dependencies: ConvertCommentsDependencies,
     { comments }: Pick<TSLintToESLintSettings, "comments">,
     { typescript }: Pick<AllOriginalConfigurations, "typescript">,
     ruleEquivalents: Map<string, string[]>,
 ): Promise<ResultWithDataStatus<string[] | undefined>> => {
+    // 1. If no comments are requested to be converted, immediately report it out and mark this as passed.
     if (comments === undefined) {
         dependencies.reportCommentResults();
         return {
@@ -30,57 +33,27 @@ export const convertComments = async (
         };
     }
 
+    // 2. Create the list of include and possibly exclude globs to search on.
     const commentFileNames = await dependencies.collectCommentFileNames(comments, typescript);
-
     if (commentFileNames instanceof Error) {
         return {
             errors: [commentFileNames],
             status: ResultStatus.Failed,
         };
     }
 
-    const { exclude, include } = commentFileNames;
-    const [fileGlobErrors, globbedFilePaths] = separateErrors(
-        await Promise.all(include.map(dependencies.globAsync)),
-    );
-
-    if (fileGlobErrors.length !== 0) {
-        return {
-            errors: fileGlobErrors,
-            status: ResultStatus.Failed,
-        };
-    }
-
-    if (globbedFilePaths.join("") === "") {
-        return {
-            errors: [
-                new Error(
-                    "--comments found no files. Consider passing no globs to it, to default to all TypeScript files.",
-                ),
-            ],
-            status: ResultStatus.Failed,
-        };
-    }
-
-    const uniqueGlobbedFilePaths = uniqueFromSources(...globbedFilePaths).filter(
-        (filePathGlob) => !exclude?.some((exclusion) => minimatch(filePathGlob, exclusion)),
-    );
-
-    if (uniqueGlobbedFilePaths.join("") === "") {
-        return {
-            errors: [
-                new Error(
-                    `All files passed to --comments were excluded. Consider removing 'exclude' from your TypeScript configuration.`,
-                ),
-            ],
-            status: ResultStatus.Failed,
-        };
+    // 3. Search for files matching those globs to have their comments converted.
+    const globbedFilePaths = await dependencies.extractGlobPaths(commentFileNames);
+    if (globbedFilePaths.status !== ResultStatus.Succeeded) {
+        return globbedFilePaths;
     }
 
+    // 4. Convert comments in the contents of each file, storing equivalents in a cache.
+    const uniqueGlobbedFilePaths = globbedFilePaths.data;
     const ruleCommentsCache = new Map<string, string[]>();
     const fileFailures = (
         await Promise.all(
-            uniqueGlobbedFilePaths.map(async (filePath) =>
+            globbedFilePaths.data.map(async (filePath) =>
                 dependencies.convertFileComments(filePath, ruleCommentsCache, ruleEquivalents),
             ),
         )
diff --git a/src/converters/comments/extractGlobPaths.test.ts b/src/converters/comments/extractGlobPaths.test.ts
@@ -0,0 +1,83 @@
+import { ResultStatus } from "../../types";
+import { extractGlobPaths, ExtractGlobPathsDependencies } from "./extractGlobPaths";
+
+const createStubDependencies = (
+    overrides: Partial<ExtractGlobPathsDependencies> = {},
+): ExtractGlobPathsDependencies => ({
+    globAsync: jest.fn().mockResolvedValue(["a.ts", "b.ts"]),
+    ...overrides,
+});
+
+describe("extractGlobPaths", () => {
+    it("returns the failure result when a file path glob fails", async () => {
+        // Arrange
+        const globAsyncError = new Error();
+        const dependencies = createStubDependencies({
+            globAsync: jest.fn().mockResolvedValueOnce(globAsyncError),
+        });
+
+        // Act
+        const result = await extractGlobPaths(dependencies, {
+            include: ["src/*.ts"],
+        });
+
+        // Assert
+        expect(result).toEqual({
+            errors: [globAsyncError],
+            status: ResultStatus.Failed,
+        });
+    });
+
+    it("returns an error when there are no resultant file paths", async () => {
+        // Arrange
+        const dependencies = createStubDependencies({
+            globAsync: jest.fn().mockResolvedValueOnce([]),
+        });
+
+        // Act
+        const result = await extractGlobPaths(dependencies, {
+            include: ["src/*.ts"],
+        });
+
+        // Assert
+        expect(result).toEqual({
+            errors: expect.arrayContaining([expect.any(Error)]),
+            status: ResultStatus.Failed,
+        });
+    });
+
+    it("returns an error when all globbed file paths are excluded", async () => {
+        // Arrange
+        const dependencies = createStubDependencies({
+            globAsync: jest.fn().mockResolvedValueOnce(["a.ts"]),
+        });
+
+        // Act
+        const result = await extractGlobPaths(dependencies, {
+            exclude: ["a.ts"],
+            include: ["*.ts"],
+        });
+
+        // Assert
+        expect(result).toEqual({
+            errors: expect.arrayContaining([expect.any(Error)]),
+            status: ResultStatus.Failed,
+        });
+    });
+
+    it("returns the paths when unique file paths are not excluded", async () => {
+        // Arrange
+        const dependencies = createStubDependencies({
+            globAsync: jest.fn().mockResolvedValueOnce(["a.ts"]),
+        });
+
+        // Act
+        const result = await extractGlobPaths(dependencies, { include: ["*.ts"] });
+
+        // Assert
+        expect(result).toEqual({
+            data: ["a.ts"],
+            status: ResultStatus.Succeeded,
+        });
+    });
+});
diff --git a/src/converters/comments/extractGlobPaths.ts b/src/converters/comments/extractGlobPaths.ts
@@ -0,0 +1,57 @@
+import minimatch from "minimatch";
+
+import { GlobAsync } from "../../adapters/globAsync";
+import { CommentFileNames } from "../../comments/collectCommentFileNames";
+import { ResultStatus, ResultWithDataStatus } from "../../types";
+import { separateErrors, uniqueFromSources } from "../../utils";
+
+export type ExtractGlobPathsDependencies = {
+    globAsync: GlobAsync;
+};
+
+export const extractGlobPaths = async (
+    dependencies: ExtractGlobPathsDependencies,
+    { exclude, include }: CommentFileNames,
+): Promise<ResultWithDataStatus<string[]>> => {
+    const [fileGlobErrors, globbedFilePaths] = separateErrors(
+        await Promise.all(include.map(dependencies.globAsync)),
+    );
+
+    if (fileGlobErrors.length !== 0) {
+        return {
+            errors: fileGlobErrors,
+            status: ResultStatus.Failed,
+        };
+    }
+
+    if (globbedFilePaths.join("") === "") {
+        return {
+            errors: [
+                new Error(
+                    "--comments found no files. Consider passing no globs to it, to default to all TypeScript files.",
+                ),
+            ],
+            status: ResultStatus.Failed,
+        };
+    }
+
+    const uniqueGlobbedFilePaths = uniqueFromSources(...globbedFilePaths).filter(
+        (filePathGlob) => !exclude?.some((exclusion) => minimatch(filePathGlob, exclusion)),
+    );
+
+    if (uniqueGlobbedFilePaths.join("") === "") {
+        return {
+            errors: [
+                new Error(
+                    `All files passed to --comments were excluded. Consider removing 'exclude' from your TypeScript configuration.`,
+                ),
+            ],
+            status: ResultStatus.Failed,
+        };
+    }
+
+    return {
+        data: uniqueGlobbedFilePaths,
+        status: ResultStatus.Succeeded,
+    };
+};
diff --git a/src/converters/editorConfigs/convertEditorConfig.ts b/src/converters/editorConfigs/convertEditorConfig.ts
@@ -16,6 +16,7 @@ export type ConvertEditorConfigDependencies = {
 
 /**
  * Root-level driver to convert an editor configuration.
+ * @see `/docs/Architecture/Editors.md` for documentation.
  */
 export const convertEditorConfig = async (
     dependencies: ConvertEditorConfigDependencies,
diff --git a/src/converters/lintConfigs/convertLintConfig.ts b/src/converters/lintConfigs/convertLintConfig.ts
@@ -17,7 +17,7 @@ export type ConvertLintConfigDependencies = {
 
 /**
  * Root-level driver to convert a TSLint configuration to ESLint.
- * @see `Architecture.md` for documentation.
+ * @see `/docs/Architecture/Linting.md` for documentation.
  */
 export const convertLintConfig = async (
     dependencies: ConvertLintConfigDependencies,
