Require one .d.ts, one .ts in tsconfig (#920)

* Require one .d.ts, one .ts in tsconfig

Previously this was checked implicitly by the unused-files check. Now
it's handled only by .npmignore, which works for publishing but not for
CI -- in particular, it's easy to forget to put test files into
tsconfig, thereby skipping testing.

This fix is NOT foolproof, but it will prevent common mistakes while
allowing uncommon layouts. `.npmignore` will continue to make sure that stray files
don't get published.

* Add changeset

* Address PR feedback, add tests

Also fix which errors are issued: don't report both excludes/includes AND
missing-files errors. Do report both missing-index and missing-test
errors, if both apply.

.changeset/wicked-days-joke.md

@@ -0,0 +1,5 @@
+---
+"@definitelytyped/dtslint": patch
+---
+
+Require 1 .d.ts, 1 .ts in tsconfigs

packages/dtslint/src/checks.ts

@@ -3,7 +3,7 @@ import { AllTypeScriptVersion } from "@definitelytyped/typescript-versions";
 import fs from "fs";
 import { join as joinPaths } from "path";
 import { CompilerOptions } from "typescript";
-import { deepEquals } from "@definitelytyped/utils";
+import { deepEquals, isDeclarationPath } from "@definitelytyped/utils";
 
 import { readJson, packageNameFromPath } from "./util";
 export function checkPackageJson(
@@ -24,14 +24,35 @@ export type CompilerOptionsRaw = {
     ? string | number | undefined
     : CompilerOptions[K];
 };
+interface Tsconfig {
+  compilerOptions: CompilerOptionsRaw;
+  files?: string[];
+  include?: string[];
+  exclude?: string[];
+}
 
-export function checkTsconfig(dirPath: string, options: CompilerOptionsRaw): string[] {
+export function checkTsconfig(dirPath: string, config: Tsconfig): string[] {
   const errors = [];
   const mustHave = {
     noEmit: true,
     forceConsistentCasingInFileNames: true,
     types: [],
   };
+  const options = config.compilerOptions;
+  if ("include" in config) {
+    errors.push('Use "files" instead of "include".');
+  } else if ("exclude" in config) {
+    errors.push('Use "files" instead of "exclude".');
+  } else if (!config.files) {
+    errors.push('Must specify "files".');
+  } else {
+    if (!config.files.includes("index.d.ts")) {
+      errors.push('"files" list must include "index.d.ts".');
+    }
+    if (!config.files.some((f) => /\.[cm]?ts$/.test(f) && !isDeclarationPath(f))) {
+      errors.push('"files" list must include at least one ".ts", ".mts" or ".cts" file for testing.');
+    }
+  }
 
   for (const key of Object.getOwnPropertyNames(mustHave) as (keyof typeof mustHave)[]) {
     const expected = mustHave[key];

packages/dtslint/src/util.ts

@@ -12,10 +12,20 @@ export function readJson(path: string) {
   return JSON.parse(stripJsonComments(text));
 }
 
-export function getCompilerOptions(dirPath: string): ts.CompilerOptions {
+export function getCompilerOptions(dirPath: string): {
+  compilerOptions: ts.CompilerOptions;
+  files?: string[];
+  includes?: string[];
+  excludes?: string[];
+} {
   const tsconfigPath = join(dirPath, "tsconfig.json");
   if (!fs.existsSync(tsconfigPath)) {
     throw new Error(`Need a 'tsconfig.json' file in ${dirPath}`);
   }
-  return readJson(tsconfigPath).compilerOptions as ts.CompilerOptions;
+  return readJson(tsconfigPath) as {
+    compilerOptions: ts.CompilerOptions;
+    files?: string[];
+    includes?: string[];
+    excludes?: string[];
+  };
 }

packages/dtslint/test/index.test.ts

@@ -14,98 +14,136 @@ describe("dtslint", () => {
     noEmit: true,
     forceConsistentCasingInFileNames: true,
   };
+  function based(extra: object) {
+    return { compilerOptions: { ...base, ...extra }, files: ["index.d.ts", "base.test.ts"] };
+  }
   describe("checks", () => {
     describe("checkTsconfig", () => {
       it("disallows unknown compiler options", () => {
-        expect(checkTsconfig("test", { ...base, completelyInvented: true })).toEqual([
+        expect(checkTsconfig("test", based({ completelyInvented: true }))).toEqual([
           "Unexpected compiler option completelyInvented",
         ]);
       });
       it("allows exactOptionalPropertyTypes: true", () => {
-        expect(checkTsconfig("test", { ...base, exactOptionalPropertyTypes: true })).toEqual([]);
+        expect(checkTsconfig("test", based({ exactOptionalPropertyTypes: true }))).toEqual([]);
       });
       it("allows module: node16", () => {
-        expect(checkTsconfig("test", { ...base, module: "node16" })).toEqual([]);
+        expect(checkTsconfig("test", based({ module: "node16" }))).toEqual([]);
       });
       it("allows `paths`", () => {
-        expect(checkTsconfig("test", { ...base, paths: { boom: ["../boom/index.d.ts"] } })).toEqual([]);
+        expect(checkTsconfig("test", based({ paths: { boom: ["../boom/index.d.ts"] } }))).toEqual([]);
       });
       it("disallows missing `module`", () => {
-        const options = { ...base };
-        delete options.module;
-        expect(checkTsconfig("test", options)).toEqual([
+        const compilerOptions = { ...base };
+        delete compilerOptions.module;
+        expect(checkTsconfig("test", { compilerOptions, files: ["index.d.ts", "base.test.ts"] })).toEqual([
           'Must specify "module" to `"module": "commonjs"` or `"module": "node16"`.',
         ]);
       });
       it("disallows exactOptionalPropertyTypes: false", () => {
-        expect(checkTsconfig("test", { ...base, exactOptionalPropertyTypes: false })).toEqual([
+        expect(checkTsconfig("test", based({ exactOptionalPropertyTypes: false }))).toEqual([
           'When "exactOptionalPropertyTypes" is present, it must be set to `true`.',
         ]);
       });
       it("allows paths: self-reference", () => {
-        expect(checkTsconfig("react-native", { ...base, paths: { "react-native": ["./index.d.ts"] } })).toEqual([]);
+        expect(checkTsconfig("react-native", based({ paths: { "react-native": ["./index.d.ts"] } }))).toEqual([]);
       });
       it("allows paths: matching ../reference/index.d.ts", () => {
         expect(
-          checkTsconfig("reactive-dep", { ...base, paths: { "react-native": ["../react-native/index.d.ts"] } }),
+          checkTsconfig("reactive-dep", based({ paths: { "react-native": ["../react-native/index.d.ts"] } })),
         ).toEqual([]);
         expect(
-          checkTsconfig("reactive-dep", {
-            ...base,
-            paths: { "react-native": ["../react-native/index.d.ts"], react: ["../react/v16/index.d.ts"] },
-          }),
+          checkTsconfig(
+            "reactive-dep",
+            based({ paths: { "react-native": ["../react-native/index.d.ts"], react: ["../react/v16/index.d.ts"] } }),
+          ),
         ).toEqual([]);
       });
       it("forbids paths: mapping to multiple things", () => {
         expect(
-          checkTsconfig("reactive-dep", {
-            ...base,
-            paths: { "react-native": ["./index.d.ts", "../react-native/v0.68/index.d.ts"] },
-          }),
+          checkTsconfig(
+            "reactive-dep",
+            based({ paths: { "react-native": ["./index.d.ts", "../react-native/v0.68/index.d.ts"] } }),
+          ),
         ).toEqual([`reactive-dep/tsconfig.json: "paths" must map each module specifier to only one file.`]);
       });
       it("allows paths: matching ../reference/version/index.d.ts", () => {
-        expect(checkTsconfig("reactive-dep", { ...base, paths: { react: ["../react/v16/index.d.ts"] } })).toEqual([]);
+        expect(checkTsconfig("reactive-dep", based({ paths: { react: ["../react/v16/index.d.ts"] } }))).toEqual([]);
         expect(
-          checkTsconfig("reactive-dep", { ...base, paths: { "react-native": ["../react-native/v0.69/index.d.ts"] } }),
+          checkTsconfig("reactive-dep", based({ paths: { "react-native": ["../react-native/v0.69/index.d.ts"] } })),
         ).toEqual([]);
         expect(
-          checkTsconfig("reactive-dep/v1", {
-            ...base,
-            paths: { "react-native": ["../../react-native/v0.69/index.d.ts"] },
-          }),
+          checkTsconfig(
+            "reactive-dep/v1",
+            based({ paths: { "react-native": ["../../react-native/v0.69/index.d.ts"] } }),
+          ),
         ).toEqual([]);
       });
       it("forbids paths: mapping to self-contained file", () => {
-        expect(checkTsconfig("rrrr", { ...base, paths: { "react-native": ["./other.d.ts"] } })).toEqual([
+        expect(checkTsconfig("rrrr", based({ paths: { "react-native": ["./other.d.ts"] } }))).toEqual([
           `rrrr/tsconfig.json: "paths" must map 'react-native' to react-native's index.d.ts.`,
         ]);
       });
       it("forbids paths: mismatching ../NOT/index.d.ts", () => {
-        expect(checkTsconfig("rrrr", { ...base, paths: { "react-native": ["../cocoa/index.d.ts"] } })).toEqual([
+        expect(checkTsconfig("rrrr", based({ paths: { "react-native": ["../cocoa/index.d.ts"] } }))).toEqual([
           `rrrr/tsconfig.json: "paths" must map 'react-native' to react-native's index.d.ts.`,
         ]);
       });
       it("forbids paths: mismatching ../react-native/NOT.d.ts", () => {
-        expect(checkTsconfig("rrrr", { ...base, paths: { "react-native": ["../react-native/other.d.ts"] } })).toEqual([
+        expect(checkTsconfig("rrrr", based({ paths: { "react-native": ["../react-native/other.d.ts"] } }))).toEqual([
           `rrrr/tsconfig.json: "paths" must map 'react-native' to react-native's index.d.ts.`,
         ]);
       });
       it("forbids paths: mismatching ../react-native/NOT/index.d.ts", () => {
         expect(
-          checkTsconfig("rrrr", { ...base, paths: { "react-native": ["../react-native/deep/index.d.ts"] } }),
+          checkTsconfig("rrrr", based({ paths: { "react-native": ["../react-native/deep/index.d.ts"] } })),
         ).toEqual([`rrrr/tsconfig.json: "paths" must map 'react-native' to react-native's index.d.ts.`]);
       });
       it("forbids paths: mismatching ../react-native/version/NOT/index.d.ts", () => {
         expect(
-          checkTsconfig("rrrr", { ...base, paths: { "react-native": ["../react-native/v0.68/deep/index.d.ts"] } }),
+          checkTsconfig("rrrr", based({ paths: { "react-native": ["../react-native/v0.68/deep/index.d.ts"] } })),
         ).toEqual([`rrrr/tsconfig.json: "paths" must map 'react-native' to react-native's index.d.ts.`]);
       });
       it("forbids paths: mismatching ../react-native/version/NOT.d.ts", () => {
         expect(
-          checkTsconfig("rrrr", { ...base, paths: { "react-native": ["../react-native/v0.70/other.d.ts"] } }),
+          checkTsconfig("rrrr", based({ paths: { "react-native": ["../react-native/v0.70/other.d.ts"] } })),
         ).toEqual([`rrrr/tsconfig.json: "paths" must map 'react-native' to react-native's index.d.ts.`]);
       });
+      it("Forbids exclude", () => {
+        expect(checkTsconfig("exclude", { compilerOptions: base, exclude: ["**/node_modules"] })).toEqual([
+          `Use "files" instead of "exclude".`,
+        ]);
+      });
+      it("Forbids include", () => {
+        expect(checkTsconfig("include", { compilerOptions: base, include: ["**/node_modules"] })).toEqual([
+          `Use "files" instead of "include".`,
+        ]);
+      });
+      it("Requires files", () => {
+        expect(checkTsconfig("include", { compilerOptions: base })).toEqual([`Must specify "files".`]);
+      });
+      it("Requires files to contain index.d.ts", () => {
+        expect(
+          checkTsconfig("include", { compilerOptions: base, files: ["package-name.d.ts", "package-name.test.ts"] }),
+        ).toEqual([`"files" list must include "index.d.ts".`]);
+      });
+      it("Requires files to contain .[mc]ts file", () => {
+        expect(checkTsconfig("include", { compilerOptions: base, files: ["index.d.ts"] })).toEqual([
+          `"files" list must include at least one ".ts", ".mts" or ".cts" file for testing.`,
+        ]);
+      });
+      it("Allows files to contain index.d.ts plus a .mts", () => {
+        expect(checkTsconfig("include", { compilerOptions: base, files: ["index.d.ts", "tests.mts"] })).toEqual([]);
+      });
+      it("Allows files to contain index.d.ts plus a .cts", () => {
+        expect(checkTsconfig("include", { compilerOptions: base, files: ["index.d.ts", "tests.cts"] })).toEqual([]);
+      });
+      it("Issues both errors on empty files list", () => {
+        expect(checkTsconfig("include", { compilerOptions: base, files: [] })).toEqual([
+          `"files" list must include "index.d.ts".`,
+          `"files" list must include at least one ".ts", ".mts" or ".cts" file for testing.`,
+        ]);
+      });
     });
     describe("assertPackageIsNotDeprecated", () => {
       it("disallows packages that are in notNeededPackages.json", () => {