refactor: extract functions into util folder with minor fix

Author: hkleungai

packages/dtslint/src/index.ts

@@ -12,7 +12,7 @@ import {
   runAreTheTypesWrong,
 } from "./checks";
 import { TsVersion, lint } from "./lint";
-import { getCompilerOptions, packageDirectoryNameWithVersionFromPath, packageNameFromPath } from "./util";
+import { findDTRootAndPackageNameFrom, getCompilerOptions, packageDirectoryNameWithVersionFromPath } from "./util";
 import assert = require("assert");
 
 async function main(): Promise<void> {
@@ -151,16 +151,8 @@ async function runTests(
   npmChecks: boolean | "only",
   tsLocal: string | undefined,
 ): Promise<string> {
-  // Assert that we're really on DefinitelyTyped.
-  const dtRoot = findDTRoot(dirPath);
-  const packageName = packageNameFromPath(dirPath);
+  const { dtRoot } = await findDTRootAndPackageNameFrom(dirPath);
   const packageDirectoryNameWithVersion = packageDirectoryNameWithVersionFromPath(dirPath);
-  assertPathIsInDefinitelyTyped(dirPath, dtRoot);
-  assertPathIsNotBanned(packageName);
-  assertPackageIsNotDeprecated(
-    packageName,
-    await fs.promises.readFile(joinPaths(dtRoot, "notNeededPackages.json"), "utf-8"),
-  );
 
   const typesVersions = getTypesVersions(dirPath);
   const packageJson = checkPackageJson(dirPath, typesVersions);
@@ -293,58 +285,6 @@ async function testTypesVersion(
   return { errors };
 }
 
-function findDTRoot(dirPath: string) {
-  let path = dirPath;
-  while (basename(path) !== "types" && dirname(path) !== "." && dirname(path) !== "/") {
-    path = dirname(path);
-  }
-  return dirname(path);
-}
-
-function assertPathIsInDefinitelyTyped(dirPath: string, dtRoot: string): void {
-  // TODO: It's not clear whether this assertion makes sense, and it's broken on Azure Pipelines (perhaps because DT isn't cloned into DefinitelyTyped)
-  // Re-enable it later if it makes sense.
-  // if (basename(dtRoot) !== "DefinitelyTyped")) {
-  if (!fs.existsSync(joinPaths(dtRoot, "types"))) {
-    throw new Error(
-      "Since this type definition includes a header (a comment starting with `// Type definitions for`), " +
-        "assumed this was a DefinitelyTyped package.\n" +
-        "But it is not in a `DefinitelyTyped/types/xxx` directory: " +
-        dirPath,
-    );
-  }
-}
-
-/**
- * Starting at some point in time, npm has banned all new packages whose names
- * contain the word `download`. However, some older packages exist that still
- * contain this name.
- * @NOTE for contributors: The list of literal exceptions below should ONLY be
- * extended with packages for which there already exists a corresponding type
- * definition package in the `@types` scope. More information:
- * https://github.com/microsoft/DefinitelyTyped-tools/pull/381.
- */
-function assertPathIsNotBanned(packageName: string) {
-  if (
-    /(^|\W)download($|\W)/.test(packageName) &&
-    packageName !== "download" &&
-    packageName !== "downloadjs" &&
-    packageName !== "s3-download-stream"
-  ) {
-    // Since npm won't release their banned-words list, we'll have to manually add to this list.
-    throw new Error(`${packageName}: Contains the word 'download', which is banned by npm.`);
-  }
-}
-
-export function assertPackageIsNotDeprecated(packageName: string, notNeededPackages: string) {
-  const unneeded = JSON.parse(notNeededPackages).packages;
-  if (Object.keys(unneeded).includes(packageName)) {
-    throw new Error(`${packageName}: notNeededPackages.json has an entry for ${packageName}.
-That means ${packageName} ships its own types, and @types/${packageName} was deprecated and removed from Definitely Typed.
-If you want to re-add @types/${packageName}, please remove its entry from notNeededPackages.json.`);
-  }
-}
-
 if (require.main === module) {
   main().catch((err) => {
     console.error(err.stack);

packages/dtslint/src/util.ts

@@ -1,4 +1,4 @@
-import { createGitHubStringSetGetter, joinPaths } from "@definitelytyped/utils";
+import { createGitHubStringSetGetter, joinPaths, readFile } from "@definitelytyped/utils";
 import fs from "fs";
 import { basename, dirname } from "path";
 import stripJsonComments = require("strip-json-comments");
@@ -42,3 +42,82 @@ export const getExpectedNpmVersionFailures = createGitHubStringSetGetter(
   "packages/dtslint/expectedNpmVersionFailures.txt",
   joinPaths(root, "expectedNpmVersionFailures.txt"),
 );
+
+export async function findDTRootAndPackageNameFrom(dirPath: string): Promise<{ dtRoot: string; packageName: string }> {
+  const dtRoot = findDTRoot(dirPath);
+  assertPathIsInDefinitelyTyped(dirPath, dtRoot);
+
+  const packageName = packageNameFromPath(dirPath);
+  assertPathIsNotBanned(packageName);
+
+  const notNeededPackagesJsonPath = joinPaths(dtRoot, "notNeededPackages.json");
+  const notNeededPackagesJson = await readFile(notNeededPackagesJsonPath);
+  assertPackageIsNotDeprecated(packageName, notNeededPackagesJson);
+
+  return { dtRoot, packageName };
+}
+
+export function findDTRoot(dirPath: string) {
+  let path = dirPath;
+  while (basename(path) !== "types" && dirname(path) !== "." && dirname(path) !== "/") {
+    path = dirname(path);
+  }
+  return dirname(path);
+}
+
+export function assertPathIsInDefinitelyTyped(dirPath: string, dtRoot: string): void {
+  // TODO: It's not clear whether this assertion makes sense, and it's broken on Azure Pipelines (perhaps because DT isn't cloned into DefinitelyTyped)
+  // Re-enable it later if it makes sense.
+  // if (basename(dtRoot) !== "DefinitelyTyped")) {
+  if (!fs.existsSync(joinPaths(dtRoot, dirPath))) {
+    throw new Error(
+      [
+        `The type definition for "${dirPath}" is expected to be a DefinitelyTyped package`,
+        `located in \`DefinitelyTyped/${dirPath}\` directory`,
+      ]
+        .map((sentence) => sentence.trim())
+        .join(" "),
+    );
+  }
+}
+
+/**
+ * Starting at some point in time, npm has banned all new packages whose names
+ * contain the word `download`. However, some older packages exist that still
+ * contain this name.
+ * @NOTE for contributors: The list of literal exceptions below should ONLY be
+ * extended with packages for which there already exists a corresponding type
+ * definition package in the `@types` scope. More information:
+ * https://github.com/microsoft/DefinitelyTyped-tools/pull/381.
+ */
+export function assertPathIsNotBanned(packageName: string) {
+  if (
+    /(^|\W)download($|\W)/.test(packageName) &&
+    packageName !== "download" &&
+    packageName !== "downloadjs" &&
+    packageName !== "s3-download-stream"
+  ) {
+    // Since npm won't release their banned-words list, we'll have to manually add to this list.
+    throw new Error(`${packageName}: Contains the word 'download', which is banned by npm.`);
+  }
+}
+
+export function assertPackageIsNotDeprecated(packageName: string, notNeededPackages: string) {
+  const unneeded = JSON.parse(notNeededPackages).packages;
+  if (Object.keys(unneeded).includes(packageName)) {
+    throw new Error(
+      [
+        `${packageName}:`,
+        `notNeededPackages.json has an entry for ${packageName}.`,
+
+        `That means ${packageName} ships its own types,`,
+        `and @types/${packageName} was deprecated and removed from Definitely Typed.`,
+
+        `If you want to re-add @types/${packageName}, `,
+        `please remove its entry from notNeededPackages.json.`,
+      ]
+        .map((sentence) => sentence.trim())
+        .join(" "),
+    );
+  }
+}

packages/dtslint/test/index.test.ts

@@ -1,6 +1,6 @@
 /// <reference types="jest" />
 import { CompilerOptionsRaw, checkTsconfig } from "../src/checks";
-import { assertPackageIsNotDeprecated } from "../src/index";
+import { assertPackageIsNotDeprecated } from "../src/util";
 
 describe("dtslint", () => {
   const base: CompilerOptionsRaw = {