Cleanup and TODOs

1. Check pnpm overrides
2. Remove done TODOs, or long-term ones that are now recorded in #751
3. Clean up some unused code.
4. Rename some properties to be shorter.

packages/definitions-parser/src/lib/definition-parser.ts

@@ -269,9 +269,8 @@ async function combineDataForAllTypesVersions(
     typesVersions,
     files,
     license,
-    packageJsonDependencies: packageJson.dependencies as Record<string, string>,
-    packageJsonDevDependencies: packageJson.devDependencies as Record<string, string>,
-    // TODO: Add devDependencies here (aka testDependencies)
+    dependencies: packageJson.dependencies as Record<string, string>,
+    devDependencies: packageJson.devDependencies as Record<string, string>,
     contentHash: hash(
       hasPackageJson ? [...files, packageJsonName] : files,
       mapDefined(allTypesVersions, (a) => a.tsconfigPathsForHash),
@@ -289,7 +288,7 @@ function getAllUniqueValues<K extends string, T>(records: readonly Record<K, rea
 }
 
 interface TypingDataFromIndividualTypeScriptVersion {
-  /** Undefined for root (which uses `// TypeScript Version: ` comment instead) */
+  /** Undefined for root (which uses typeScriptVersion in package.json instead) */
   readonly typescriptVersion: TypeScriptVersion | undefined;
   readonly declFiles: readonly string[]; // TODO: Used to map file.d.ts to ts4.1/file.d.ts -- not sure why this is needed
   readonly tsconfigPathsForHash: string | undefined;
@@ -323,7 +322,6 @@ function getTypingDataForSingleTypesVersion(
     path.resolve("/", fs.debugPath())
   ).options;
   errors.push(...checkFilesFromTsConfig(packageName, tsconfig, fs.debugPath()));
-  // TODO: tests should be Set<string>, not Map<string, SourceFile>
   const { types, tests } = allReferencedFiles(
     tsconfig.files ?? [],
     fs,
@@ -332,7 +330,7 @@ function getTypingDataForSingleTypesVersion(
     compilerOptions
   );
   const usedFiles = new Set(
-    [...types.keys(), ...tests.keys(), "tsconfig.json", "tslint.json"].map((f) =>
+    [...types.keys(), ...tests, "tsconfig.json", "tslint.json"].map((f) =>
       slicePrefixes(f, "node_modules/@types/" + packageName + "/")
     )
   );

packages/definitions-parser/src/lib/module-info.ts

@@ -5,11 +5,6 @@ import { FS, assertDefined, sort } from "@definitelytyped/utils";
 import { readFileAndThrowOnBOM } from "./definition-parser";
 import { getMangledNameForScopedPackage } from "../packages";
 
-const extensions: Map<string, string> = new Map();
-extensions.set(".d.ts", ""); // TODO: Inaccurate?
-extensions.set(".d.mts", ".mjs");
-extensions.set(".d.cts", ".cjs");
-
 export function getDeclaredGlobals(all: Map<string, ts.SourceFile>): string[] {
   const globals = new Set<string>();
   for (const sourceFile of all.values()) {
@@ -87,10 +82,10 @@ export function allReferencedFiles(
   packageName: string,
   moduleResolutionHost: ts.ModuleResolutionHost,
   compilerOptions: ts.CompilerOptions
-): { types: Map<string, ts.SourceFile>; tests: Map<string, ts.SourceFile> } {
+): { types: Map<string, ts.SourceFile>; tests: Set<string> } {
   const seenReferences = new Set<string>();
   const types = new Map<string, ts.SourceFile>();
-  const tests = new Map<string, ts.SourceFile>();
+  const tests = new Set<string>();
   // The directory where the tsconfig/index.d.ts is - i.e., may be a version within the package
   const baseDirectory = path.resolve("/", fs.debugPath());
   // The root of the package and all versions, i.e., the direct subdirectory of types/
@@ -143,7 +138,7 @@ export function allReferencedFiles(
       ) {
         types.set(relativeFileName, src);
       } else {
-        tests.set(relativeFileName, src);
+        tests.add(relativeFileName);
       }
 
       const refs = findReferencedFiles(src, packageName);

packages/definitions-parser/src/packages.ts

@@ -136,12 +136,9 @@ export class AllPackages {
     return this.notNeeded;
   }
 
-  /** Returns all of the dependences *that have typings*, ignoring others, and including test dependencies.
-   * I have NO idea why it's an iterator. Surely not for efficiency. */
+  /** Returns all of the dependencies *that are typed on DT*, ignoring others, and including test dependencies. */
   *allDependencyTypings(pkg: TypingsData): Iterable<TypingsData> {
     for (const [name, version] of pkg.allPackageJsonDependencies()) {
-      // TODO: chart.js@3 has types; @types/[email protected] is the last version on DT.
-      // It shouldn't be an error to depend on chart.js@3 but it's currently ambiguous with @types/chart.js.
       if (!name.startsWith(`@${scopeName}/`)) continue;
       if (pkg.name === name) continue;
       const typesDirectoryName = removeTypesScope(name);
@@ -336,12 +333,12 @@ export interface TypingsDataRaw {
    * Packages that provide definitions that this package depends on.
    * NOTE: Includes `@types/` packages.
    */
-  readonly packageJsonDependencies: PackageJsonDependencies;
+  readonly dependencies: PackageJsonDependencies;
 
   /**
    * Packages that this package's tests or other development depends on.
    */
-  readonly packageJsonDevDependencies: PackageJsonDependencies;
+  readonly devDependencies: PackageJsonDependencies;
 
   /**
    * The [older] version of the library that this definition package refers to, as represented *on-disk*.
@@ -493,18 +490,17 @@ export class TypingsData extends PackageBase {
   get license(): License {
     return this.data.license;
   }
-  // TODO: Rename this back to dependencies/devDependencies
-  get packageJsonDependencies(): PackageJsonDependencies {
-    return this.data.packageJsonDependencies ?? {};
+  get dependencies(): PackageJsonDependencies {
+    return this.data.dependencies ?? {};
   }
-  get packageJsonDevDependencies(): PackageJsonDependencies {
-    return this.data.packageJsonDevDependencies ?? {};
+  get devDependencies(): PackageJsonDependencies {
+    return this.data.devDependencies ?? {};
   }
   *allPackageJsonDependencies(): Iterable<[string, string]> {
-    for (const [name, version] of Object.entries(this.packageJsonDependencies)) {
+    for (const [name, version] of Object.entries(this.dependencies)) {
       yield [name, version];
     }
-    for (const [name, version] of Object.entries(this.packageJsonDevDependencies)) {
+    for (const [name, version] of Object.entries(this.devDependencies)) {
       yield [name, version];
     }
   }

packages/definitions-parser/test/definition-parser.test.ts

@@ -196,7 +196,7 @@ export * from 'buffer';
       throw new Error(info.join("\n"));
     }
     expect(info).toBeDefined();
-    expect(info["1.0"].packageJsonDependencies).toEqual({ "@types/node": "*" });
+    expect(info["1.0"].dependencies).toEqual({ "@types/node": "*" });
   });
   it("errors on arbitrary path mapping", () => {});
   it("supports node_modules passthrough path mapping", async () => {
@@ -359,7 +359,7 @@ import route = require('@ember/routing/route');
     if (Array.isArray(info)) {
       throw new Error(info.join("\n"));
     }
-    expect(info["2.8"].packageJsonDevDependencies).toEqual({ "@types/ember": "workspace:." });
+    expect(info["2.8"].devDependencies).toEqual({ "@types/ember": "workspace:." });
   });
 
   it("doesn't omit dependencies if only some deep modules are declared", async () => {
@@ -370,7 +370,7 @@ import route = require('@ember/routing/route');
     if (Array.isArray(info)) {
       throw new Error(info.join("\n"));
     }
-    expect(info["5.1"].packageJsonDependencies).toEqual({ "@types/styled-components": "*" });
+    expect(info["5.1"].dependencies).toEqual({ "@types/styled-components": "*" });
   });
 
   it("rejects relative references to other packages", async () => {

packages/definitions-parser/test/module-info.test.ts

@@ -2,7 +2,7 @@ import * as ts from "typescript";
 import { createModuleResolutionHost } from "@definitelytyped/utils";
 import { DTMock, createMockDT } from "../src/mocks";
 import { testo } from "./utils";
-import { allReferencedFiles } from "../src/lib/module-info";
+import { allReferencedFiles, createSourceFile, getDeclaredGlobals } from "../src/lib/module-info";
 
 const fs = createMockDT().fs;
 const moduleResolutionHost = createModuleResolutionHost(fs);
@@ -140,3 +140,49 @@ testo({
     expect(Array.from(tests.keys())).toEqual([]);
   },
 });
+testo({
+  getDeclaredGlobalsFindsExportAsNamespace() {
+    expect(
+      getDeclaredGlobals(
+        new Map([
+          [
+            "index.d.ts",
+            createSourceFile(
+              "index.d.ts",
+              "export type T = 1; export as namespace jQuery",
+              moduleResolutionHost,
+              compilerOptions
+            ),
+          ],
+        ])
+      )
+    ).toEqual(["jQuery"]);
+  },
+  getDeclaredGlobalsFindsGlobalDeclarations() {
+    // non-modules: namespaces, vars, enum, class, function
+    // NOT for interface, type alias, import=
+    expect(
+      getDeclaredGlobals(
+        new Map([
+          [
+            "index.d.ts",
+            createSourceFile(
+              "index.d.ts",
+              `
+        type T = 1;
+        interface I { i }
+        var i, j: I
+        enum E { e }
+        class C {}
+        function f() {}
+        namespace n { export type T = 1; export var value: "make sure n is a value namespace" }
+        `,
+              moduleResolutionHost,
+              compilerOptions
+            ),
+          ],
+        ])
+      )
+    ).toEqual(["C", "E", "f", "i", "j", "n"]);
+  },
+});

packages/definitions-parser/test/packages.test.ts

@@ -152,10 +152,10 @@ describe(TypingsData, () => {
     expect(data.contentHash).toBe("11111111111111");
     expect(data.projectName).toBe("zombo.com");
     expect(data.globals).toEqual([]);
-    expect(data.packageJsonDependencies).toEqual({
+    expect(data.dependencies).toEqual({
       "dependency-1": "*",
     });
-    expect(data.packageJsonDevDependencies).toEqual({
+    expect(data.devDependencies).toEqual({
       "@types/known": "workspace:.",
     });
     expect(data.id).toEqual({

packages/definitions-parser/test/utils.ts

@@ -26,8 +26,8 @@ export function createTypingsVersionRaw(
       files: ["index.d.ts"],
       typesVersions: [],
       license: License.MIT,
-      packageJsonDependencies: dependencies,
-      packageJsonDevDependencies: devDependencies,
+      dependencies: dependencies,
+      devDependencies: devDependencies,
       contentHash: "11111111111111",
       globals: [],
     },

packages/dtslint-runner/src/check-parse-results.ts

@@ -18,5 +18,5 @@ export function checkParseResults(allPackages: AllPackages): string[] {
       }
     }
   }
-  return errors;
+  return errors
 }

packages/dtslint-runner/src/main.ts

@@ -153,7 +153,7 @@ export async function runDTSLint({
       suggestionLines.push(`"${packageName}": [${suggestions.join(",")}]`);
     }
   }
-  // console.log(`{${suggestionLines.join(",")}}`);
+  console.log(`{${suggestionLines.join(",")}}`);
 
   logPerformance();
 

packages/header-parser/README.md

@@ -1,6 +1,7 @@
 # DefinitelyTyped Header Parser
 
-This library parses headers of [DefinitelyTyped](https://github.com/DefinitelyTyped/DefinitelyTyped) types.
+This library parses package.jsons of [DefinitelyTyped](https://github.com/DefinitelyTyped/DefinitelyTyped) types.
+Its name is left over from when package information was stored in textual headers.
 
 # Contributing
 

packages/header-parser/src/index.ts

@@ -2,22 +2,6 @@ import { AllTypeScriptVersion, TypeScriptVersion } from "@definitelytyped/typesc
 import assert = require("assert");
 import { deepEquals, parsePackageSemver } from "@definitelytyped/utils";
 
-// TODO:
-// 1. Convert this package into a packageJson checker
-// 2. Move checks from dt-header into dtslint/checks.ts and remove the rule.
-// 4. Add test for header in dtslint that forbids it.
-// 5. Update dts-gen and DT README and ??? -- rest of ecosystem.
-/*
-
-  # Example header format #
-
-  // Type definitions for foo 1.2
-  // Project: https://github.com/foo/foo, https://foo.com
-  // Definitions by: My Self <https://github.com/me>, Some Other Guy <https://github.com/otherguy>
-  // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
-  // TypeScript Version: 2.1
-
-*/
 // used in dts-critic
 export interface Header {
   readonly nonNpm: boolean;
@@ -70,14 +54,6 @@ export function validatePackageJson(
 ): Header | string[] {
   const errors = [];
   const needsTypesVersions = typesVersions.length !== 0;
-  // NOTE: I had to install eslint-plugin-import in DT because DT-tools hasn't shipped a new version
-  // DONE: normal package: 3box
-  // DONE: scoped package: adeira__js
-  // DONE: old-version package: gulp/v3
-  // DONE: old-TS-version package: har-format
-  // DONE: react
-  // DONE: node
-  // TODO: Some spelling correction would be nice here, especially for typeScriptVersion's case.
   for (const key in packageJson) {
     switch (key) {
       case "private":
@@ -94,12 +70,10 @@ export function validatePackageJson(
       case "contributors":
       case "nonNpm":
       case "nonNpmDescription":
+      case "pnpm":
         // "dependencies" / "license" checked by types-publisher,
         // TODO: asserts for other fields in types-publisher
         break;
-      case "pnpm":
-        // TODO: write validation rules for pnpm property (should just be overrides, and those should probably be restricted somehow)
-        break;
       case "typesVersions":
       case "types":
         if (!needsTypesVersions) {
@@ -163,6 +137,7 @@ export function validatePackageJson(
   const typeScriptVersionResult = validateTypeScriptVersion();
   const projectsResult = validateProjects();
   const contributorsResult = validateContributors();
+  const pnpmResult = validatePnpm();
   if (typeof nameResult === "object") {
     errors.push(...nameResult.errors);
   } else {
@@ -194,6 +169,9 @@ export function validatePackageJson(
   } else {
     contributors = contributorsResult;
   }
+  if (typeof pnpmResult === "object") {
+    errors.push(...pnpmResult.errors);
+  }
   if (errors.length) {
     return errors;
   } else {
@@ -320,6 +298,38 @@ export function validatePackageJson(
     }
     return { errors };
   }
+  function validatePnpm(): undefined | { errors: string[] } {
+    const errors = [];
+    if (packageJson.pnpm) {
+      if (typeof packageJson.pnpm !== "object" || packageJson.pnpm === null) {
+        errors.push(
+          `${typesDirectoryName}'s package.json has bad "pnpm": must be an object like { "overrides": { "@types/react": "^16" } }`
+        );
+      } else {
+        for (const key in packageJson.pnpm) {
+          if (key !== "overrides") {
+            errors.push(
+              `${typesDirectoryName}'s package.json has bad "pnpm": it should not include property "${key}", only "overrides".`
+            );
+          }
+        }
+        const overrides = (packageJson.pnpm as Record<string, unknown>).overrides;
+        if (overrides && typeof overrides === "object" && overrides !== null) {
+          for (const key in overrides) {
+            if (!key.startsWith("@types/")) {
+              errors.push(`${typesDirectoryName}'s package.json has bad "pnpm": pnpm overrides may only override @types/ packages.`);
+            }
+          }
+        } else {
+          errors.push(`${typesDirectoryName}'s package.json has bad "pnpm": it must contain an "overrides" object.`);
+        }
+      }
+    }
+    if (errors.length) {
+      return { errors };
+    }
+    return undefined;
+  }
 }
 
 function checkPackageJsonContributors(packageName: string, packageJsonContributors: readonly unknown[]) {