diff --git a/.changeset/new-items-explode.md b/.changeset/new-items-explode.md new file mode 100644 index 0000000000..1ae1ef91c6 --- /dev/null +++ b/.changeset/new-items-explode.md @@ -0,0 +1,14 @@ +--- +"@definitelytyped/typescript-versions": patch +"@definitelytyped/definitions-parser": patch +"@definitelytyped/dtslint-runner": patch +"@definitelytyped/eslint-plugin": patch +"@definitelytyped/header-parser": patch +"@definitelytyped/dts-critic": patch +"@definitelytyped/publisher": patch +"@definitelytyped/dtslint": patch +"@definitelytyped/retag": patch +"@definitelytyped/utils": patch +--- + +Update @definitelytyped for Definitely Typed's monorepo upgrade diff --git a/.vscode/launch.json b/.vscode/launch.json index b23d58751f..41f7e6e15f 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -5,11 +5,47 @@ "version": "0.2.0", "configurations": [ { - "type": "pwa-chrome", + "name": "dtslint-runner local all", + "program": "${workspaceFolder}/packages/dtslint-runner/dist/index.js", "request": "launch", - "name": "Launch Chrome against localhost", - "url": "http://localhost:8080", - "webRoot": "${workspaceFolder}" - } + "skipFiles": [ + "/**" + ], + "args": [ + "--path", "../DefinitelyTyped", "--selection", "all", "--localTypeScriptPath", "../../ts/built/local", + ], + "type": "node" + }, + { + "name": "jest", + "program": "${workspaceFolder}/node_modules/.bin/jest", + "request": "launch", + "skipFiles": [ + "/**" + ], + "args": [ "definitions-parser"], + "type": "node" + }, + { + "name": "dts-critic test", + "program": "${workspaceFolder}/packages/dts-critic/dist/index.js", + "request": "launch", + "skipFiles": [ + "/**" + ], + "args": [ "--dts", "packages/dts-critic/testsource/tslib/index.d.ts", ], + "type": "node" + }, + { + "name": "dtslint aframe", + "program": "${workspaceFolder}/packages/dtslint/dist/index.js", + "request": "launch", + "skipFiles": [ + "/**" + ], + "args": [ "../DefinitelyTyped/types/aframe", ], + + "type": "node" + }, ] } \ No newline at end of file diff --git a/README.md b/README.md index f8d423804c..607f231ec0 100644 --- a/README.md +++ b/README.md @@ -3,12 +3,13 @@ A monorepo for formerly disparate DefinitelyTyped-related tools: - [definitions-parser](packages/definitions-parser): the part of [microsoft/types-publisher](https://github.com/microsoft/types-publisher) that reads DefinitelyTyped repository data -- [dtslint](packages/dtslint): [microsoft/dtslint](https://github.com/microsoft/dtslint) -- [dtslint-runner](packages/dtslint-runner): [DefinitelyTyped/dtslint-runner](https://github.com/DefinitelyTyped/dtslint-runner) -- [dts-critic](packages/dts-critic): [DefinitelyTyped/dts-critic](https://github.com/DefinitelyTyped/dts-critic) -- [header-parser](packages/header-parser): [microsoft/definitelytyped-header-parser](https://github.com/microsoft/definitelytyped-header-parser) +- [dtslint](packages/dtslint): [microsoft/dtslint](https://github.com/microsoft/dtslint), make sure a package is correct for DT +- [dtslint-runner](packages/dtslint-runner): [DefinitelyTyped/dtslint-runner](https://github.com/DefinitelyTyped/dtslint-runner), test all packages, or all changed packages, on DT with dtslint +- [eslint-plugin](packages/eslint-plugin): provides DT-specific lint rules (except for the few remaining tslint rules, still in dtslint) +- [dts-critic](packages/dts-critic): [DefinitelyTyped/dts-critic](https://github.com/DefinitelyTyped/dts-critic), issue errors when a types packages mismatches its original Javascript package. +- [header-parser](packages/header-parser): [microsoft/definitelytyped-header-parser](https://github.com/microsoft/definitelytyped-header-parser), check and extract DT-related info from package.json - [publisher](packages/publisher): the rest of [microsoft/types-publisher](https://github.com/microsoft/types-publisher) -- [retag](packages/retag): [DefinitelyTyped/dt-retag](https://github.com/DefinitelyTyped/dt-retag) +- [retag](packages/retag): [DefinitelyTyped/dt-retag](https://github.com/DefinitelyTyped/dt-retag), script to add ATA tags to `@types` packages. Run weekly. - [typescript-versions](packages/typescript-versions): the part of [definitelytyped-header-parser](https://github.com/microsoft/definitelytyped-header-parser) that tracked which TypeScript versions are published to npm and supported on DefinitelyTyped - [utils](packages/utils): shared utilities, mostly extracted from [microsoft/types-publisher](https://github.com/microsoft/types-publisher) diff --git a/package.json b/package.json index 4d2591387d..cc2b064cb2 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,7 @@ "@typescript-eslint/eslint-plugin": "^5.55.0", "@typescript-eslint/parser": "^5.55.0", "eslint": "^7.31.0", - "eslint-plugin-import": "^2.22.1", + "eslint-plugin-import": "^2.27.5", "eslint-plugin-jsdoc": "^30.7.8", "jest": "^29.5.0", "prettier": "^2.6.2", diff --git a/packages/definitions-parser/allowedPackageJsonDependencies.txt b/packages/definitions-parser/allowedPackageJsonDependencies.txt index 115463d727..a97486d016 100644 --- a/packages/definitions-parser/allowedPackageJsonDependencies.txt +++ b/packages/definitions-parser/allowedPackageJsonDependencies.txt @@ -605,6 +605,7 @@ bl bookshelf boxen broadcast-channel +broccoli-node-api broccoli-plugin bson buffer @@ -724,6 +725,7 @@ log4js logform loglevel logrocket +long lottie-web magic-string markdownlint diff --git a/packages/definitions-parser/src/get-affected-packages.ts b/packages/definitions-parser/src/get-affected-packages.ts index d000b8a8b4..7f3c5ca325 100644 --- a/packages/definitions-parser/src/get-affected-packages.ts +++ b/packages/definitions-parser/src/get-affected-packages.ts @@ -1,109 +1,82 @@ -import { mapDefined, mapIterable, sort } from "@definitelytyped/utils"; -import { - TypingsData, - AllPackages, - PackageId, - PackageBase, - getMangledNameForScopedPackage, - formatDependencyVersion, -} from "./packages"; - -export interface Affected { - readonly changedPackages: readonly TypingsData[]; - readonly dependentPackages: readonly TypingsData[]; - allPackages: AllPackages; +import { assertDefined, execAndThrowErrors, mapDefined, withoutStart } from "@definitelytyped/utils"; +import { sourceBranch, sourceRemote } from "./lib/settings"; +import { AllPackages, PackageId, formatTypingVersion, getDependencyFromFile } from "./packages"; +import { resolve } from "path"; +import { satisfies } from "semver"; +export interface PreparePackagesResult { + readonly packageNames: Set; + readonly dependents: Set; } /** Gets all packages that have changed on this branch, plus all packages affected by the change. */ -export function getAffectedPackages(allPackages: AllPackages, changedPackageIds: PackageId[]): Affected { - const resolved = changedPackageIds.map((id) => allPackages.tryResolve(id)); - // If a package doesn't exist, that's because it was deleted. - const changed = mapDefined(resolved, (id) => allPackages.tryGetTypingsData(id)); - const dependent = mapIterable(collectDependers(resolved, getReverseDependencies(allPackages, resolved)), (p) => - allPackages.getTypingsData(p) - ); - return { changedPackages: changed, dependentPackages: sortPackages(dependent), allPackages }; -} - -/** Every package name in the original list, plus their dependencies (incl. dependencies' dependencies). */ -export function allDependencies(allPackages: AllPackages, packages: Iterable): TypingsData[] { - return sortPackages(transitiveClosure(packages, (pkg) => allPackages.allDependencyTypings(pkg))); -} - -/** Collect all packages that depend on changed packages, and all that depend on those, etc. */ -function collectDependers( - changedPackages: PackageId[], - reverseDependencies: Map> -): Set { - const dependers = transitiveClosure(changedPackages, (pkg) => reverseDependencies.get(pkg) || []); - // Don't include the original changed packages, just their dependers - for (const original of changedPackages) { - dependers.delete(original); - } - return dependers; -} - -function sortPackages(packages: Iterable): TypingsData[] { - return sort(packages, PackageBase.compare); // tslint:disable-line no-unbound-method -} - -function transitiveClosure(initialItems: Iterable, getRelatedItems: (item: T) => Iterable): Set { - const all = new Set(); - const workList: T[] = []; - - function add(item: T): void { - if (!all.has(item)) { - all.add(item); - workList.push(item); +export async function getAffectedPackages( + allPackages: AllPackages, + deletions: PackageId[], + definitelyTypedPath: string +): Promise { + const allDependents = []; + const filters = [`--filter '...[${sourceRemote}/${sourceBranch}]'`]; + for (const d of deletions) { + for (const dep of allPackages.allTypings()) { + for (const [name, version] of dep.allPackageJsonDependencies()) { + if ( + "@types/" + d.typesDirectoryName === name && + (d.version === "*" || satisfies(formatTypingVersion(d.version), version)) + ) { + filters.push(`--filter '...${dep.name}'`); + break; + } + } } } - - for (const item of initialItems) { - add(item); - } - - while (workList.length) { - const item = workList.pop()!; - for (const newItem of getRelatedItems(item)) { - add(newItem); - } + const changedPackageNames = await execAndThrowErrors( + `pnpm ls -r --depth -1 --parseable --filter '[${sourceRemote}/${sourceBranch}]'`, + definitelyTypedPath + ); + // Chunk into 100-package chunks because of CMD.COM's command-line length limit + for (let i = 0; i < filters.length; i += 100) { + allDependents.push( + await execAndThrowErrors( + `pnpm ls -r --depth -1 --parseable ${filters.slice(i, i + 100).join(" ")}`, + definitelyTypedPath + ) + ); } - - return all; + return getAffectedPackagesWorker(allPackages, changedPackageNames, allDependents, definitelyTypedPath); } - -/** Generate a map from a package to packages that depend on it. */ -function getReverseDependencies( +/** This function is exported for testing, since it's determined entirely by its inputs. */ +export function getAffectedPackagesWorker( allPackages: AllPackages, - changedPackages: PackageId[] -): Map> { - const map = new Map]>(); - for (const changed of changedPackages) { - map.set(packageIdToKey(changed), [changed, new Set()]); - } - for (const typing of allPackages.allTypings()) { - if (!map.has(packageIdToKey(typing.id))) { - map.set(packageIdToKey(typing.id), [typing.id, new Set()]); - } - } - for (const typing of allPackages.allTypings()) { - for (const [name, version] of Object.entries(typing.dependencies)) { - const dependencies = map.get(packageIdToKey(allPackages.tryResolve({ name, version }))); - if (dependencies) { - dependencies[1].add(typing.id); - } - } - for (const dependencyName of typing.testDependencies) { - const version = typing.pathMappings[dependencyName] || "*"; - const dependencies = map.get(packageIdToKey(allPackages.tryResolve({ name: dependencyName, version }))); - if (dependencies) { - dependencies[1].add(typing.id); - } - } - } - return new Map(map.values()); + changedOutput: string, + dependentOutputs: string[], + definitelyTypedPath: string +): PreparePackagesResult { + const dt = resolve(definitelyTypedPath); + const changedDirs = mapDefined(changedOutput.split("\n"), getDirectoryName(dt)); + const dependentDirs = mapDefined(dependentOutputs.join("\n").split("\n"), getDirectoryName(dt)); + const packageNames = new Set( + changedDirs.map( + (c) => + assertDefined( + allPackages.tryGetTypingsData(assertDefined(getDependencyFromFile(c + "/index.d.ts"), "bad path " + c)) + ).subDirectoryPath + ) + ); + const dependents = new Set( + dependentDirs + .map( + (d) => + assertDefined( + allPackages.tryGetTypingsData(assertDefined(getDependencyFromFile(d + "/index.d.ts"), "bad path " + d)), + d + " package not found" + ).subDirectoryPath + ) + .filter((d) => !packageNames.has(d)) + ); + return { packageNames, dependents }; } -function packageIdToKey(pkg: PackageId): string { - return getMangledNameForScopedPackage(pkg.name) + "/v" + formatDependencyVersion(pkg.version); +function getDirectoryName(dt: string): (line: string) => string | undefined { + return (line) => + line && line !== dt ? assertDefined(withoutStart(line, dt + "/"), line + " is missing prefix " + dt) : undefined; } diff --git a/packages/definitions-parser/src/git.ts b/packages/definitions-parser/src/git.ts index 8e9feccfa1..c4bedbbd43 100644 --- a/packages/definitions-parser/src/git.ts +++ b/packages/definitions-parser/src/git.ts @@ -1,27 +1,17 @@ -import assert from "assert"; -import { sourceBranch } from "./lib/settings"; +import { sourceBranch, sourceRemote } from "./lib/settings"; import { PackageId, - DependencyVersion, - formatDependencyVersion, + DirectoryParsedTypingVersion, getDependencyFromFile, + formatTypingVersion, AllPackages, NotNeededPackage, } from "./packages"; -import { - Logger, - execAndThrowErrors, - flatMapIterable, - mapIterable, - mapDefined, - FS, - consoleLogger, - assertDefined, - cacheDir, -} from "@definitelytyped/utils"; +import { Logger, execAndThrowErrors, consoleLogger, assertDefined, cacheDir } from "@definitelytyped/utils"; import * as pacote from "pacote"; import * as semver from "semver"; -import { getAffectedPackages } from "./get-affected-packages"; +import { inspect } from "util"; +import { PreparePackagesResult, getAffectedPackages } from "./get-affected-packages"; export interface GitDiff { status: "A" | "D" | "M"; @@ -29,9 +19,9 @@ export interface GitDiff { } /* -We have to be careful about how we get the diff because travis uses a shallow clone. +We have to be careful about how we get the diff because Actions uses a shallow clone. -Travis runs: +Actions runs: git clone --depth=50 https://github.com/DefinitelyTyped/DefinitelyTyped.git DefinitelyTyped cd DefinitelyTyped git fetch origin +refs/pull/123/merge @@ -45,7 +35,7 @@ export async function gitDiff(log: Logger, definitelyTypedPath: string): Promise // If this succeeds, we got the full clone. } catch (_) { // This is a shallow clone. - await run(`git fetch origin ${sourceBranch}`); + await run(`git fetch ${sourceRemote} ${sourceBranch}`); await run(`git branch ${sourceBranch} FETCH_HEAD`); } @@ -66,62 +56,54 @@ export async function gitDiff(log: Logger, definitelyTypedPath: string): Promise return stdout; } } - -/** Returns all immediate subdirectories of the root directory that have changed. */ -export function gitChanges(diffs: GitDiff[]): PackageId[] { - const changedPackages = new Map>(); - +/** Returns all immediate subdirectories of the root directory that have been deleted. */ +function gitDeletions(diffs: GitDiff[]): { errors: string[] } | { ok: PackageId[] } { + const changedPackages = new Map(); + const errors = []; for (const diff of diffs) { + if (diff.status !== "D") continue; const dep = getDependencyFromFile(diff.file); if (dep) { - const versions = changedPackages.get(dep.name); - if (!versions) { - changedPackages.set(dep.name, new Map([[formatDependencyVersion(dep.version), dep.version]])); - } else { - versions.set(formatDependencyVersion(dep.version), dep.version); - } + const key = `${dep.typesDirectoryName}/v${formatDependencyVersion(dep.version)}`; + changedPackages.set(key, dep); + } else { + errors.push( + `Unexpected file deleted: ${diff.file} +When removing packages, you should only delete files that are a part of removed packages.` + ); } } - - return Array.from( - flatMapIterable(changedPackages, ([name, versions]) => mapIterable(versions, ([_, version]) => ({ name, version }))) - ); + return errors.length ? { errors } : { ok: Array.from(changedPackages.values()) }; } +function formatDependencyVersion(version: DirectoryParsedTypingVersion | "*") { + return version === "*" ? "*" : formatTypingVersion(version); +} export async function getAffectedPackagesFromDiff( - dt: FS, - definitelyTypedPath: string, - selection: "all" | "affected" | RegExp -) { - const allPackages = await AllPackages.read(dt); + allPackages: AllPackages, + definitelyTypedPath: string +): Promise { + const errors = []; const diffs = await gitDiff(consoleLogger.info, definitelyTypedPath); if (diffs.find((d) => d.file === "notNeededPackages.json")) { - for (const deleted of getNotNeededPackages(allPackages, diffs)) { - checkNotNeededPackage(deleted); - } + const deleteds = getNotNeededPackages(allPackages, diffs); + if ("errors" in deleteds) errors.push(...deleteds.errors); + else + for (const deleted of deleteds.ok) { + errors.push(...(await checkNotNeededPackage(deleted))); + } } - - const affected = - selection === "all" - ? { changedPackages: allPackages.allTypings(), dependentPackages: [], allPackages } - : selection === "affected" - ? getAffectedPackages(allPackages, gitChanges(diffs)) - : { - changedPackages: allPackages.allTypings().filter((t) => selection.test(t.name)), - dependentPackages: [], - allPackages, - }; - - console.log( - `Testing ${affected.changedPackages.length} changed packages: ${affected.changedPackages - .map((t) => t.desc) - .toString()}` - ); - console.log( - `Testing ${affected.dependentPackages.length} dependent packages: ${affected.dependentPackages - .map((t) => t.desc) - .toString()}` - ); + const deletions = gitDeletions(diffs); + if ("errors" in deletions) { + errors.push(...deletions.errors); + return errors; + } + if (errors.length) { + return errors; + } + const affected = await getAffectedPackages(allPackages, deletions.ok, definitelyTypedPath); + console.log(`Testing ${affected.packageNames.size} changed packages: ${inspect(affected.packageNames)}`); + console.log(`Testing ${affected.dependents.size} dependent packages: ${inspect(affected.dependents)}`); return affected; } @@ -130,60 +112,55 @@ export async function getAffectedPackagesFromDiff( * 2. asOfVersion must be newer than `@types/name@latest` on npm * 3. `name@asOfVersion` must exist on npm */ -export async function checkNotNeededPackage(unneeded: NotNeededPackage) { - await pacote.manifest(`${unneeded.libraryName}@${unneeded.version}`, { cache: cacheDir }).catch((reason) => { - throw reason.code === "E404" - ? new Error( - `The entry for ${unneeded.fullNpmName} in notNeededPackages.json has +export async function checkNotNeededPackage(unneeded: NotNeededPackage): Promise { + const errors = []; + const replacementPackage = await pacote + .manifest(`${unneeded.libraryName}@${unneeded.version}`, { cache: cacheDir }) + .catch((reason) => { + if (reason.code === "E404") + return `The entry for ${unneeded.name} in notNeededPackages.json has "libraryName": "${unneeded.libraryName}", but there is no npm package with this name. -Unneeded packages have to be replaced with a package on npm.`, - { cause: reason } - ) - : reason.code === "ETARGET" - ? new Error(`The specified version ${unneeded.version} of ${unneeded.libraryName} is not on npm.`, { - cause: reason, - }) - : reason; - }); // eg @babel/parser - const typings = await pacote.manifest(unneeded.fullNpmName, { cache: cacheDir }).catch((reason) => { - throw reason.code === "E404" - ? new Error(`Unexpected error: @types package not found for ${unneeded.fullNpmName}`, { cause: reason }) - : reason; +Unneeded packages have to be replaced with a package on npm.`; + else if (reason.code === "ETARGET") + return `The specified version ${unneeded.version} of ${unneeded.libraryName} is not on npm.`; + else throw reason; + }); // eg @babel/parser + if (typeof replacementPackage === "string") errors.push(replacementPackage); + const typings = await pacote.manifest(unneeded.name, { cache: cacheDir }).catch((reason) => { + if (reason.code === "E404") return `Unexpected error: @types package not found for ${unneeded.name}`; + else throw reason; }); // eg @types/babel__parser - assert( - semver.gt(unneeded.version, typings.version), - `The specified version ${unneeded.version} of ${unneeded.libraryName} must be newer than the version -it is supposed to replace, ${typings.version} of ${unneeded.fullNpmName}.` - ); + if (typeof typings === "string") { + errors.push(typings); + return errors; + } + if (!semver.gt(unneeded.version, typings.version)) + errors.push(`The specified version ${unneeded.version} of ${unneeded.libraryName} must be newer than the version +it is supposed to replace, ${typings.version} of ${unneeded.name}.`); + return errors; } /** * 1. Find all the deleted files and group by package (error on deleted files outside a package). * 2. Make sure that all deleted packages in notNeededPackages have no files left. */ -export function getNotNeededPackages(allPackages: AllPackages, diffs: GitDiff[]) { - const deletedPackages = new Set( - diffs - .filter((d) => d.status === "D") - .map( - (d) => - assertDefined( - getDependencyFromFile(d.file), - `Unexpected file deleted: ${d.file} -When removing packages, you should only delete files that are a part of removed packages.` - ).name - ) - ); - return mapDefined(deletedPackages, (p) => { - const hasTyping = allPackages.hasTypingFor({ name: p, version: "*" }); +export function getNotNeededPackages( + allPackages: AllPackages, + diffs: GitDiff[] +): { errors: string[] } | { ok: NotNeededPackage[] } { + const deletions = gitDeletions(diffs); + if ("errors" in deletions) return deletions; + const deletedPackages = new Set(deletions.ok.map((p) => assertDefined(p.typesDirectoryName))); + const notNeededs = []; + const errors = []; + for (const p of deletedPackages) { + const hasTyping = allPackages.hasTypingFor({ typesDirectoryName: p, version: "*" }); const notNeeded = allPackages.getNotNeededPackage(p); - if (hasTyping) { - if (notNeeded) { - throw new Error(`Please delete all files in ${p} when adding it to notNeededPackages.json.`); - } - return undefined; - } else { - return notNeeded; + if (hasTyping && notNeeded) { + errors.push(`Please delete all files in ${p} when adding it to notNeededPackages.json.`); + } else if (notNeeded) { + notNeededs.push(notNeeded); } - }); + } + return errors.length ? { errors } : { ok: notNeededs }; } diff --git a/packages/definitions-parser/src/lib/definition-parser.ts b/packages/definitions-parser/src/lib/definition-parser.ts index 23ac392ba8..874ce826c8 100644 --- a/packages/definitions-parser/src/lib/definition-parser.ts +++ b/packages/definitions-parser/src/lib/definition-parser.ts @@ -1,37 +1,40 @@ -import * as ts from "typescript"; -import { parseHeaderOrFail } from "@definitelytyped/header-parser"; -import { allReferencedFiles, createSourceFile, getModuleInfo, getTestDependencies } from "./module-info"; import { - DependencyVersion, - formatTypingVersion, + validatePackageJson, + License, getLicenseFromPackageJson, - PackageJsonDependency, - TypingsDataRaw, - TypingsVersionsRaw, - DirectoryParsedTypingVersion, - getMangledNameForScopedPackage, -} from "../packages"; -import { getAllowedPackageJsonDependencies } from "./settings"; + checkPackageJsonType, + checkPackageJsonDependencies, + checkPackageJsonImports, + checkPackageJsonExportsAndAddPJsonEntry, +} from "@definitelytyped/header-parser"; +import { TypeScriptVersion } from "@definitelytyped/typescript-versions"; import { FS, - split, - mapDefined, - filter, - sort, - withoutStart, + assertDefined, computeHash, + createModuleResolutionHost, + filter, + flatMap, hasWindowsSlashes, join, - flatMap, - unique, - unmangleScopedPackage, - removeVersionFromPackageName, - hasVersionNumberInMapping, - mangleScopedPackage, - createModuleResolutionHost, + mapDefined, + sort, + split, + withoutStart, } from "@definitelytyped/utils"; -import { TypeScriptVersion } from "@definitelytyped/typescript-versions"; +import assert from "assert"; import path from "path"; +import * as ts from "typescript"; +import { + DirectoryParsedTypingVersion, + TypingsDataRaw, + TypingsVersionsRaw, + formatTypingVersion, + getMangledNameForScopedPackage, +} from "../packages"; +import { allReferencedFiles, createSourceFile } from "./module-info"; +import { getAllowedPackageJsonDependencies, scopeName } from "./settings"; +import { slicePrefixes } from "./utils"; function matchesVersion( typingsDataRaw: TypingsDataRaw, @@ -39,20 +42,21 @@ function matchesVersion( considerLibraryMinorVersion: boolean ) { return ( - typingsDataRaw.libraryMajorVersion === version.major && + typingsDataRaw.header.libraryMajorVersion === version.major && (considerLibraryMinorVersion - ? version.minor === undefined || typingsDataRaw.libraryMinorVersion === version.minor + ? version.minor === undefined || typingsDataRaw.header.libraryMinorVersion === version.minor : true) ); } function formattedLibraryVersion(typingsDataRaw: TypingsDataRaw): `${number}.${number}` { - return `${typingsDataRaw.libraryMajorVersion}.${typingsDataRaw.libraryMinorVersion}`; + return `${typingsDataRaw.header.libraryMajorVersion}.${typingsDataRaw.header.libraryMinorVersion}`; } -export async function getTypingInfo(packageName: string, dt: FS): Promise { +export async function getTypingInfo(packageName: string, dt: FS): Promise { + const errors = []; if (packageName !== packageName.toLowerCase()) { - throw new Error(`Package name \`${packageName}\` should be strictly lowercase`); + errors.push(`Package name \`${packageName}\` should be strictly lowercase`); } interface OlderVersionDir { readonly directoryName: string; @@ -70,57 +74,65 @@ export async function getTypingInfo(packageName: string, dt: FS): Promise version.minor !== undefined); - - const latestData: TypingsDataRaw = { - libraryVersionDirectoryName: undefined, - ...(await combineDataForAllTypesVersions(packageName, rootDirectoryLs, fs, undefined, moduleResolutionHost)), - }; + const latestDataResult = await combineDataForAllTypesVersions(packageName, rootDirectoryLs, fs, moduleResolutionHost); + if (Array.isArray(latestDataResult)) { + return { errors: [...errors, ...latestDataResult] }; + } + const latestData: TypingsDataRaw = { libraryVersionDirectoryName: undefined, ...latestDataResult }; const older = await Promise.all( olderVersionDirectories.map(async ({ directoryName, version: directoryVersion }) => { if (matchesVersion(latestData, directoryVersion, considerLibraryMinorVersion)) { - const latest = `${latestData.libraryMajorVersion}.${latestData.libraryMinorVersion}`; - throw new Error( + const latest = `${latestData.header.libraryMajorVersion}.${latestData.header.libraryMinorVersion}`; + errors.push( `The latest version of the '${packageName}' package is ${latest}, so the subdirectory '${directoryName}' is not allowed` + (`v${latest}` === directoryName ? "." - : `; since it applies to any ${latestData.libraryMajorVersion}.* version, up to and including ${latest}.`) + : `; since it applies to any ${latestData.header.libraryMajorVersion}.* version, up to and including ${latest}.`) ); } // tslint:disable-next-line:non-literal-fs-path -- Not a reference to the fs package const ls = fs.readdir(directoryName); - const data: TypingsDataRaw = { - libraryVersionDirectoryName: formatTypingVersion(directoryVersion), - ...(await combineDataForAllTypesVersions( - packageName, - ls, - fs.subDir(directoryName), - directoryVersion, - moduleResolutionHost - )), - }; + const result = await combineDataForAllTypesVersions( + packageName, + ls, + fs.subDir(directoryName), + moduleResolutionHost + ); + if (Array.isArray(result)) { + errors.push(...result); + return result; + } + const data: TypingsDataRaw = { libraryVersionDirectoryName: formatTypingVersion(directoryVersion), ...result }; if (!matchesVersion(data, directoryVersion, considerLibraryMinorVersion)) { if (considerLibraryMinorVersion) { - throw new Error( - `Directory ${directoryName} indicates major.minor version ${directoryVersion.major}.${directoryVersion.minor}, ` + - `but header indicates major.minor version ${data.libraryMajorVersion}.${data.libraryMinorVersion}` + errors.push( + `Directory ${directoryName} indicates major.minor version ${directoryVersion.major}.${ + directoryVersion.minor ?? "*" + }, ` + + `but package.json indicates major.minor version ${data.header.libraryMajorVersion}.${data.header.libraryMinorVersion}` + ); + } else { + errors.push( + `Directory ${directoryName} indicates major version ${directoryVersion.major}, but package.json indicates major version ` + + data.header.libraryMajorVersion.toString() ); } - throw new Error( - `Directory ${directoryName} indicates major version ${directoryVersion.major}, but header indicates major version ` + - data.libraryMajorVersion.toString() - ); } return data; }) ); + if (errors.length) { + return { errors }; + } const res: TypingsVersionsRaw = {}; res[formattedLibraryVersion(latestData)] = latestData; for (const o of older) { - res[formattedLibraryVersion(o)] = o; + assert(!Array.isArray(o)); + res[formattedLibraryVersion(o as TypingsDataRaw)] = o; } return res; } @@ -130,9 +142,9 @@ const packageJsonName = "package.json"; interface LsMinusTypesVersionsAndPackageJson { readonly remainingLs: readonly string[]; readonly typesVersions: readonly TypeScriptVersion[]; - readonly hasPackageJson: boolean; } -function getTypesVersionsAndPackageJson(ls: readonly string[]): LsMinusTypesVersionsAndPackageJson { +function getTypesVersionsAndPackageJson(ls: readonly string[]): LsMinusTypesVersionsAndPackageJson | string[] { + const errors: string[] = []; const withoutPackageJson = ls.filter((name) => name !== packageJsonName); const [remainingLs, typesVersions] = split(withoutPackageJson, (fileOrDirectoryName) => { const match = /^ts(\d+\.\d+)$/.exec(fileOrDirectoryName); @@ -142,13 +154,14 @@ function getTypesVersionsAndPackageJson(ls: readonly string[]): LsMinusTypesVers const version = match[1]; if (parseInt(version, 10) < 3) { - throw new Error( - `Directory name starting with 'ts' should be a TypeScript version newer than 3.0. Got: ${version}` - ); + errors.push(`Directory name starting with 'ts' should be a TypeScript version newer than 3.0. Got: ${version}`); } return version as TypeScriptVersion; }); - return { remainingLs, typesVersions, hasPackageJson: withoutPackageJson.length !== ls.length }; + if (errors.length) { + return errors; + } + return { remainingLs, typesVersions }; } /** @@ -174,84 +187,89 @@ export function parseVersionFromDirectoryName( }; } -/** - * Like `parseVersionFromDirectoryName`, but the leading 'v' is optional, - * and falls back to '*' if the input format is not parseable. - */ -export function tryParsePackageVersion(versionString: string | undefined): DependencyVersion { - const match = /^v?(\d+)(\.(\d+))?$/.exec(versionString!); - if (match === null) { - return "*"; - } - return { - major: Number(match[1]), - minor: match[3] !== undefined ? Number(match[3]) : undefined, // tslint:disable-line strict-type-predicates (false positive) - }; -} - -/** - * Like `tryParsePackageVersion`, but throws if the input format is not parseable. - */ -export function parsePackageVersion(versionString: string): DirectoryParsedTypingVersion { - const version = tryParsePackageVersion(versionString); - if (version === "*") { - throw new Error(`Version string '${versionString}' is not a valid format.`); - } - return version; -} - async function combineDataForAllTypesVersions( typingsPackageName: string, ls: readonly string[], fs: FS, - directoryVersion: DirectoryParsedTypingVersion | undefined, moduleResolutionHost: ts.ModuleResolutionHost -): Promise> { - const { remainingLs, typesVersions, hasPackageJson } = getTypesVersionsAndPackageJson(ls); - const packageJson = hasPackageJson - ? (fs.readJson(packageJsonName) as { - readonly license?: unknown; - readonly dependencies?: unknown; - readonly imports?: unknown; - readonly exports?: unknown; - readonly type?: unknown; - }) - : {}; - const packageJsonType = checkPackageJsonType(packageJson.type, packageJsonName); - - // Every typesVersion has an index.d.ts, but only the root index.d.ts should have a header. - const { - contributors, - libraryMajorVersion, - libraryMinorVersion, - typeScriptVersion: minTsVersion, - libraryName, - projects, - } = parseHeaderOrFail(`${typingsPackageName} > index.d.ts`, readFileAndThrowOnBOM("index.d.ts", fs)); - +): Promise | string[]> { + const errors = []; + const typesVersionAndPackageJson = getTypesVersionsAndPackageJson(ls); + if (Array.isArray(typesVersionAndPackageJson)) { + errors.push(...typesVersionAndPackageJson); + } + const { remainingLs, typesVersions } = Array.isArray(typesVersionAndPackageJson) + ? { remainingLs: [], typesVersions: [] } + : typesVersionAndPackageJson; + const packageJson = fs.readJson(packageJsonName) as { + readonly license?: unknown; + readonly dependencies?: unknown; + readonly devDependencies?: unknown; + readonly imports?: unknown; + readonly exports?: unknown; + readonly type?: unknown; + }; const dataForRoot = getTypingDataForSingleTypesVersion( undefined, typingsPackageName, remainingLs, fs, - directoryVersion, moduleResolutionHost ); + if (Array.isArray(dataForRoot)) { + errors.push(...dataForRoot); + } const dataForOtherTypesVersions = typesVersions.map((tsVersion) => { const subFs = fs.subDir(`ts${tsVersion}`); - return getTypingDataForSingleTypesVersion( + const data = getTypingDataForSingleTypesVersion( tsVersion, typingsPackageName, subFs.readdir(), subFs, - directoryVersion, moduleResolutionHost ); + if (Array.isArray(data)) { + errors.push(...data); + } + return data; }); - const allTypesVersions = [dataForRoot, ...dataForOtherTypesVersions]; + const packageJsonType = checkPackageJsonType(packageJson.type, packageJsonName); + if (Array.isArray(packageJsonType)) { + errors.push(...packageJsonType); + } + const packageJsonResult = validatePackageJson(typingsPackageName, packageJson, typesVersions); + if (Array.isArray(packageJsonResult)) { + errors.push(...packageJsonResult); + } + + const header = Array.isArray(packageJsonResult) ? undefined : packageJsonResult; + const allowedDependencies = await getAllowedPackageJsonDependencies(); + errors.push(...checkPackageJsonDependencies(packageJson.dependencies, packageJsonName, allowedDependencies)); + errors.push( + ...checkPackageJsonDependencies( + packageJson.devDependencies, + packageJsonName, + allowedDependencies, + `@${scopeName}/${typingsPackageName}` + ) + ); + const imports = checkPackageJsonImports(packageJson.imports, packageJsonName); + if (Array.isArray(imports)) { + errors.push(...imports); + } + const exports = checkPackageJsonExportsAndAddPJsonEntry(packageJson.exports, packageJsonName); + if (Array.isArray(exports)) { + errors.push(...exports); + } const license = getLicenseFromPackageJson(packageJson.license); - const packageJsonDependencies = await checkPackageJsonDependencies(packageJson.dependencies, packageJsonName); + if (Array.isArray(license)) { + errors.push(...license); + } + if (errors.length) { + return errors; + } + const allTypesVersions = [dataForRoot, ...dataForOtherTypesVersions] as TypingDataFromIndividualTypeScriptVersion[]; const files = Array.from( flatMap(allTypesVersions, ({ typescriptVersion, declFiles }) => declFiles.map((file) => (typescriptVersion === undefined ? file : `ts${typescriptVersion}/${file}`)) @@ -260,47 +278,28 @@ async function combineDataForAllTypesVersions( // Note that only the first project is collected right now return { - libraryName, - typingsPackageName, - projectName: projects[0], - contributors, - libraryMajorVersion, - libraryMinorVersion, - minTsVersion, + header: assertDefined(header), typesVersions, files, - license, - dependencies: Object.assign({}, ...allTypesVersions.map((v) => v.dependencies)), - testDependencies: getAllUniqueValues<"testDependencies", string>(allTypesVersions, "testDependencies"), - pathMappings: Object.assign({}, ...allTypesVersions.map((v) => v.pathMappings)), - packageJsonDependencies, + license: license as License, + dependencies: packageJson.dependencies as Record, + devDependencies: packageJson.devDependencies as Record, contentHash: hash( - hasPackageJson ? [...files, packageJsonName] : files, + [...files, packageJsonName], mapDefined(allTypesVersions, (a) => a.tsconfigPathsForHash), fs ), - globals: getAllUniqueValues<"globals", string>(allTypesVersions, "globals"), - declaredModules: getAllUniqueValues<"declaredModules", string>(allTypesVersions, "declaredModules"), - imports: checkPackageJsonImports(packageJson.imports, packageJsonName), - exports: checkPackageJsonExportsAndAddPJsonEntry(packageJson.exports, packageJsonName), - type: packageJsonType, + imports: imports as object | undefined, + exports: exports as string | object | undefined, + type: packageJsonType as "module" | undefined, }; } -function getAllUniqueValues(records: readonly Record[], key: K): readonly T[] { - return unique(flatMap(records, (x) => x[key])); -} - 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 dependencies: { readonly [name: string]: DependencyVersion }; - readonly testDependencies: readonly string[]; - readonly pathMappings: { readonly [packageName: string]: DirectoryParsedTypingVersion }; - readonly declFiles: readonly string[]; + 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; - readonly globals: readonly string[]; - readonly declaredModules: readonly string[]; } /** @@ -314,9 +313,9 @@ function getTypingDataForSingleTypesVersion( packageName: string, ls: readonly string[], fs: FS, - directoryVersion: DirectoryParsedTypingVersion | undefined, moduleResolutionHost: ts.ModuleResolutionHost -): TypingDataFromIndividualTypeScriptVersion { +): TypingDataFromIndividualTypeScriptVersion | string[] { + const errors = []; const tsconfig = fs.readJson("tsconfig.json") as TsConfig; const configHost: ts.ParseConfigHost = { ...moduleResolutionHost, @@ -329,17 +328,19 @@ function getTypingDataForSingleTypesVersion( configHost, path.resolve("/", fs.debugPath()) ).options; - checkFilesFromTsConfig(packageName, tsconfig, fs.debugPath()); - - const { types, tests, hasNonRelativeImports } = allReferencedFiles( - tsconfig.files!, + errors.push(...checkFilesFromTsConfig(packageName, tsconfig, fs.debugPath())); + const { types, tests } = allReferencedFiles( + tsconfig.files ?? [], fs, packageName, moduleResolutionHost, compilerOptions ); - - const usedFiles = new Set([...types.keys(), ...tests.keys(), "tsconfig.json", "tslint.json"]); + const usedFiles = new Set( + [...types.keys(), ...tests, "tsconfig.json", "tslint.json"].map((f) => + slicePrefixes(f, "node_modules/@types/" + packageName + "/") + ) + ); const otherFiles = ls.includes(unusedFilesName) ? fs // tslint:disable-next-line:non-literal-fs-path -- Not a reference to the fs package @@ -348,14 +349,22 @@ function getTypingDataForSingleTypesVersion( .filter(Boolean) : []; if (ls.includes(unusedFilesName) && !otherFiles.length) { - throw new Error(`In ${packageName}: OTHER_FILES.txt is empty.`); + errors.push(`In ${packageName}: OTHER_FILES.txt is empty.`); } for (const file of otherFiles) { if (!isRelativePath(file)) { - throw new Error(`In ${packageName}: A path segment is empty or all dots ${file}`); + errors.push(`In ${packageName}: A path segment is empty or all dots ${file}`); } } - checkAllFilesUsed(ls, usedFiles, otherFiles, packageName, fs); + // Note: findAllUnusedFiles also modifies usedFiles and otherFiles and errors + const unusedFiles = findAllUnusedFiles(ls, usedFiles, otherFiles, errors, packageName, fs); + if (unusedFiles.length) { + errors.push( + "\n\t* " + + unusedFiles.map((unused) => `Unused file ${unused}`).join("\n\t* ") + + `\n\t(used files: ${JSON.stringify(Array.from(usedFiles))})` + ); + } for (const untestedTypeFile of filter( otherFiles, (name) => name.endsWith(".d.ts") || name.endsWith(".d.mts") || name.endsWith(".d.cts") @@ -368,173 +377,35 @@ function getTypingDataForSingleTypesVersion( ); } - const { dependencies: dependenciesWithDeclaredModules, globals, declaredModules } = getModuleInfo(packageName, types); - const declaredModulesSet = new Set(declaredModules); - // Don't count an import of "x" as a dependency if we saw `declare module "x"` somewhere. - const dependenciesSet = new Set( - [...dependenciesWithDeclaredModules] - .filter((m) => !declaredModulesSet.has(m)) - .map((m) => rootName(m, types, packageName)) - .filter((dependency) => dependency !== packageName) - ); - const testDependencies = [ - ...getTestDependencies(packageName, tests.keys(), dependenciesSet, fs, moduleResolutionHost, compilerOptions), - ] - .filter((m) => !declaredModulesSet.has(m)) - .map((m) => rootName(m, types, packageName)) - .filter((dependency) => dependency !== packageName); - - const { paths } = tsconfig.compilerOptions; - const hydratedPackageName = unmangleScopedPackage(packageName) ?? packageName; - if (directoryVersion && hasNonRelativeImports && !(paths && `${hydratedPackageName}/*` in paths)) { - const mapping = JSON.stringify([`${packageName}/v${formatTypingVersion(directoryVersion)}/*`]); - throw new Error( - `${hydratedPackageName}: Older version ${formatTypingVersion( - directoryVersion - )} must have a "paths" entry of "${hydratedPackageName}/*": ${mapping}` - ); - } - - const { dependencies, pathMappings } = calculateDependencies( - packageName, - tsconfig, - dependenciesSet, - directoryVersion - ); - const tsconfigPathsForHash = JSON.stringify(tsconfig.compilerOptions.paths); + if (errors.length) return errors; return { typescriptVersion, - dependencies, - testDependencies, - pathMappings, - globals, - declaredModules, declFiles: sort(types.keys()), - tsconfigPathsForHash, + tsconfigPathsForHash: JSON.stringify(tsconfig.compilerOptions.paths), }; } -/** - * "foo/bar/baz" -> "foo"; "@foo/bar/baz" -> "@foo/bar" - * Note: Throws an error for references like - * "bar/v3" because referencing old versions of *other* packages is illegal; - * those directories won't exist in the published @types package. - */ -function rootName(importText: string, typeFiles: Map, packageName: string): string { - let slash = importText.indexOf("/"); - // Root of `@foo/bar/baz` is `@foo/bar` - if (importText.startsWith("@")) { - // Use second "/" - slash = importText.indexOf("/", slash + 1); - } - const root = importText.slice(0, slash); - const postImport = importText.slice(slash + 1); - if (slash > -1 && postImport.match(/v\d+$/) && !typeFiles.has(postImport + ".d.ts") && root !== packageName) { - throw new Error(`${importText}: do not directly import specific versions of another types package. -You should work with the latest version of ${root} instead.`); - } - return slash === -1 ? importText : root; -} - -function checkPackageJsonExportsAndAddPJsonEntry(exports: unknown, path: string) { - if (exports === undefined) return exports; - if (typeof exports === "string") { - return exports; - } - if (typeof exports !== "object") { - throw new Error(`Package exports at path ${path} should be an object or string.`); - } - if (exports === null) { - throw new Error(`Package exports at path ${path} should not be null.`); - } - if (!(exports as Record)["./package.json"]) { - (exports as Record)["./package.json"] = "./package.json"; - } - return exports; -} - -function checkPackageJsonImports(imports: unknown, path: string) { - if (imports === undefined) return imports; - if (typeof imports !== "object") { - throw new Error(`Package imports at path ${path} should be an object or string.`); - } - if (imports === null) { - throw new Error(`Package imports at path ${path} should not be null.`); - } - return imports; -} - -function checkPackageJsonType(type: unknown, path: string) { - if (type === undefined) return type; - if (type !== "module") { - throw new Error(`Package type at path ${path} can only be 'module'.`); - } - return type; -} - -async function checkPackageJsonDependencies( - dependencies: unknown, - path: string -): Promise { - if (dependencies === undefined) { - // tslint:disable-line strict-type-predicates (false positive) - return []; - } - if (dependencies === null || typeof dependencies !== "object") { - // tslint:disable-line strict-type-predicates - throw new Error(`${path} should contain "dependencies" or not exist.`); - } - - const deps: PackageJsonDependency[] = []; - - for (const dependencyName of Object.keys(dependencies!)) { - // `dependencies` cannot be null because of check above. - if (!(await getAllowedPackageJsonDependencies()).has(dependencyName)) { - const msg = dependencyName.startsWith("@types/") - ? `Dependency ${dependencyName} not in the allowed dependencies list. -Don't use a 'package.json' for @types dependencies unless this package relies on -an old version of types that have since been moved to the source repo. -For example, if package *P* used to have types on Definitely Typed at @types/P, -but now has its own types, a dependent package *D* will need to use package.json -to refer to @types/P if it relies on old versions of P's types. -In this case, please make a pull request to microsoft/DefinitelyTyped-tools adding @types/P to \`packages/definitions-parser/allowedPackageJsonDependencies.txt\`.` - : `Dependency ${dependencyName} not in the allowed dependencies list. -If you are depending on another \`@types\` package, do *not* add it to a \`package.json\`. Path mapping should make the import work. -For namespaced dependencies you then have to add a \`paths\` mapping from \`@namespace/*\` to \`namespace__*\` in \`tsconfig.json\`. -If this is an external library that provides typings, please make a pull request to microsoft/DefinitelyTyped-tools adding it to \`packages/definitions-parser/allowedPackageJsonDependencies.txt\`.`; - throw new Error(`In ${path}: ${msg}`); - } - - const version = (dependencies as { [key: string]: unknown })[dependencyName]; - if (typeof version !== "string") { - // tslint:disable-line strict-type-predicates - throw new Error(`In ${path}: Dependency version for ${dependencyName} should be a string.`); - } - deps.push({ name: dependencyName, version }); - } - - return deps; -} - -function checkFilesFromTsConfig(packageName: string, tsconfig: TsConfig, directoryPath: string): void { +function checkFilesFromTsConfig(packageName: string, tsconfig: TsConfig, directoryPath: string): string[] { + const errors = []; const tsconfigPath = `${directoryPath}/tsconfig.json`; if (tsconfig.include) { - throw new Error(`In tsconfig, don't use "include", must use "files"`); + errors.push(`In tsconfig, don't use "include", must use "files"`); } const files = tsconfig.files; if (!files) { - throw new Error(`${tsconfigPath} needs to specify "files"`); + errors.push(`${tsconfigPath} needs to specify "files"`); + return errors; } for (const file of files) { if (file.startsWith("./")) { - throw new Error(`In ${tsconfigPath}: Unnecessary "./" at the start of ${file}`); + errors.push(`In ${tsconfigPath}: Unnecessary "./" at the start of ${file}`); } if (!isRelativePath(file)) { - throw new Error(`In ${tsconfigPath}: A path segment is empty or all dots ${file}`); + errors.push(`In ${tsconfigPath}: A path segment is empty or all dots ${file}`); } if (file.endsWith(".d.ts") && file !== "index.d.ts") { - throw new Error(`${packageName}: Only index.d.ts may be listed explicitly in tsconfig's "files" entry. + errors.push(`${packageName}: Only index.d.ts may be listed explicitly in tsconfig's "files" entry. Other d.ts files must either be referenced through index.d.ts, tests, or added to OTHER_FILES.txt.`); } @@ -546,10 +417,11 @@ Other d.ts files must either be referenced through index.d.ts, tests, or added t ? `Expected file '${file}' to be named '${expectedName}' or to be inside a '${directoryPath}/test/' directory` : `Unexpected file extension for '${file}' -- expected '.ts' or '.tsx' (maybe this should not be in "files", but ` + "OTHER_FILES.txt)"; - throw new Error(message); + errors.push(message); } } } + return errors; } function isRelativePath(path: string) { @@ -562,159 +434,6 @@ interface TsConfig { compilerOptions: ts.CompilerOptions; } -/** In addition to dependencies found in source code, also get dependencies from tsconfig. */ -interface DependenciesAndPathMappings { - readonly dependencies: { readonly [name: string]: DependencyVersion }; - readonly pathMappings: { readonly [packageName: string]: DirectoryParsedTypingVersion }; -} -function calculateDependencies( - packageName: string, - tsconfig: TsConfig, - dependencyNames: ReadonlySet, - directoryVersion: DirectoryParsedTypingVersion | undefined -): DependenciesAndPathMappings { - const paths = (tsconfig.compilerOptions && tsconfig.compilerOptions.paths) || {}; - - const dependencies: { [name: string]: DependencyVersion } = {}; - const pathMappings: { [packageName: string]: DirectoryParsedTypingVersion } = {}; - - const scopedPackageName = unmangleScopedPackage(packageName) ?? packageName; - for (const dependencyName of Object.keys(paths)) { - const pathMappingList = paths[dependencyName]; - if (pathMappingList.length !== 1) { - throw new Error(`In ${packageName}: Path mapping for ${dependencyName} may only have 1 entry.`); - } - const pathMapping = pathMappingList[0]; - if (pathMapping === "./node_modules/" + dependencyName) { - // allow passthrough remappings for packages like webpack that have shipped their own types, - // but have some dependents on DT that depend on the new types and some that depend on the old types - continue; - } - - // Path mapping may be for "@foo/*" -> "foo__*". - const unversionedScopedPackageName = removeVersionFromPackageName(unmangleScopedPackage(pathMapping)); - if (unversionedScopedPackageName !== undefined) { - if (dependencyName !== unversionedScopedPackageName) { - throw new Error(`Expected directory ${pathMapping} to be the path mapping for ${dependencyName}`); - } - if (!hasVersionNumberInMapping(pathMapping)) { - continue; - } - } - - // Might have a path mapping for "foo/*" to support subdirectories - const rootDirectory = withoutEnd(dependencyName, "/*"); - if (rootDirectory !== undefined) { - if (!(rootDirectory in paths)) { - throw new Error(`In ${packageName}: found path mapping for ${dependencyName} but not for ${rootDirectory}`); - } - continue; - } - - // buffer -> node/buffer may be required because of the non-node 'buffer' package on npm - // which DT infrastructure depends on, and which resolves before node's ambient module 'buffer' - if (dependencyName === "buffer" && pathMapping === "node/buffer") { - dependencies.node = "*"; - continue; - } - - const pathMappingVersion = parseDependencyVersionFromPath(dependencyName, dependencyName, pathMapping); - if (dependencyName === packageName) { - if (directoryVersion === undefined) { - throw new Error(`In ${packageName}: Latest version of a package should not have a path mapping for itself.`); - } - if (directoryVersion.major !== pathMappingVersion.major || directoryVersion.minor !== pathMappingVersion.minor) { - const correctPathMapping = [`${dependencyName}/v${formatTypingVersion(directoryVersion)}`]; - throw new Error( - `In ${packageName}: Must have a "paths" entry of "${dependencyName}": ${JSON.stringify(correctPathMapping)}` - ); - } - } else { - if (dependencyNames.has(dependencyName)) { - dependencies[dependencyName] = pathMappingVersion; - } - } - // Else, the path mapping may be necessary if it is for a transitive dependency. We will check this in check-parse-results. - pathMappings[dependencyName] = pathMappingVersion; - } - - if (directoryVersion !== undefined && !(paths && scopedPackageName in paths)) { - const mapping = JSON.stringify([`${packageName}/v${formatTypingVersion(directoryVersion)}`]); - throw new Error( - `${scopedPackageName}: Older version ${formatTypingVersion( - directoryVersion - )} must have a "paths" entry of "${scopedPackageName}": ${mapping}` - ); - } - - for (const dependency of dependencyNames) { - if (!dependencies[dependency] && !nodeBuiltins.has(dependency)) { - dependencies[dependency] = "*"; - } - } - - return { dependencies, pathMappings }; -} - -const nodeBuiltins: ReadonlySet = new Set([ - "assert", - "async_hooks", - "buffer", - "child_process", - "cluster", - "console", - "constants", - "crypto", - "dgram", - "dns", - "domain", - "events", - "fs", - "http", - "http2", - "https", - "module", - "net", - "os", - "path", - "perf_hooks", - "process", - "punycode", - "querystring", - "readline", - "repl", - "stream", - "string_decoder", - "timers", - "tls", - "tty", - "url", - "util", - "v8", - "vm", - "zlib", -]); - -function parseDependencyVersionFromPath( - packageName: string, - dependencyName: string, - dependencyPath: string -): DirectoryParsedTypingVersion { - const versionString = withoutStart(dependencyPath, `${mangleScopedPackage(dependencyName)}/`); - const version = versionString === undefined ? undefined : parseVersionFromDirectoryName(versionString); - if (version === undefined) { - throw new Error(`In ${packageName}, unexpected path mapping for ${dependencyName}: '${dependencyPath}'`); - } - return version; -} - -function withoutEnd(s: string, end: string): string | undefined { - if (s.endsWith(end)) { - return s.slice(0, s.length - end.length); - } - return undefined; -} - function hash(files: readonly string[], tsconfigPathsForHash: readonly string[], fs: FS): string { const fileContents = files.map((f) => `${f}**${readFileAndThrowOnBOM(f, fs)}`); let allContent = fileContents.join("||"); @@ -736,29 +455,38 @@ export function readFileAndThrowOnBOM(fileName: string, fs: FS): string { const unusedFilesName = "OTHER_FILES.txt"; -function checkAllFilesUsed( +/** Modifies usedFiles and otherFiles and errors */ +function findAllUnusedFiles( ls: readonly string[], usedFiles: Set, otherFiles: string[], + errors: string[], packageName: string, fs: FS -): void { +): string[] { // Double-check that no windows "\\" broke in. for (const fileName of usedFiles) { if (hasWindowsSlashes(fileName)) { - throw new Error(`In ${packageName}: windows slash detected in ${fileName}`); + errors.push(`In ${packageName}: windows slash detected in ${fileName}`); } } - checkAllUsedRecur(new Set(ls), usedFiles, new Set(otherFiles), fs); + return findAllUnusedRecur(new Set(ls), usedFiles, new Set(otherFiles), errors, fs); } -function checkAllUsedRecur(ls: Iterable, usedFiles: Set, unusedFiles: Set, fs: FS): void { +function findAllUnusedRecur( + ls: Iterable, + usedFiles: Set, + otherFiles: Set, + errors: string[], + fs: FS +): string[] { + const unused = []; for (const lsEntry of ls) { if (usedFiles.has(lsEntry)) { continue; } - if (unusedFiles.has(lsEntry)) { - unusedFiles.delete(lsEntry); + if (otherFiles.has(lsEntry)) { + otherFiles.delete(lsEntry); continue; } @@ -771,8 +499,7 @@ function checkAllUsedRecur(ls: Iterable, usedFiles: Set, unusedF const lssubdir = subdir.readdir(); if (lssubdir.length === 0) { - // tslint:disable-next-line strict-string-expressions - throw new Error(`Empty directory ${subdir.debugPath()} (${join(usedFiles)})`); + errors.push(`Empty directory ${subdir.debugPath()} (${join(usedFiles)})`); } function takeSubdirectoryOutOfSet(originalSet: Set): Set { @@ -786,7 +513,13 @@ function checkAllUsedRecur(ls: Iterable, usedFiles: Set, unusedF } return subdirSet; } - checkAllUsedRecur(lssubdir, takeSubdirectoryOutOfSet(usedFiles), takeSubdirectoryOutOfSet(unusedFiles), subdir); + findAllUnusedRecur( + lssubdir, + takeSubdirectoryOutOfSet(usedFiles), + takeSubdirectoryOutOfSet(otherFiles), + errors, + subdir + ); } else { if ( lsEntry.toLowerCase() !== "readme.md" && @@ -795,19 +528,17 @@ function checkAllUsedRecur(ls: Iterable, usedFiles: Set, unusedF lsEntry !== ".eslintrc.json" && lsEntry !== unusedFilesName ) { - throw new Error( - `Unused file ${fs.debugPath()}/${lsEntry} (used files: ${JSON.stringify(Array.from(usedFiles))})` - ); + unused.push(`${fs.debugPath()}/${lsEntry}`); } } } - - for (const unusedFile of unusedFiles) { - if (usedFiles.has(unusedFile)) { - throw new Error( - `File ${fs.debugPath()}${unusedFile} listed in ${unusedFilesName} is already reachable from tsconfig.json.` + for (const otherFile of otherFiles) { + if (usedFiles.has(otherFile)) { + errors.push( + `File ${fs.debugPath()}${otherFile} listed in ${unusedFilesName} is already reachable from tsconfig.json.` ); } - throw new Error(`File ${fs.debugPath()}/${unusedFile} listed in ${unusedFilesName} does not exist.`); + errors.push(`File ${fs.debugPath()}/${otherFile} listed in ${unusedFilesName} does not exist.`); } + return unused; } diff --git a/packages/definitions-parser/src/lib/module-info.ts b/packages/definitions-parser/src/lib/module-info.ts index 37d59ca078..40283ebf8a 100644 --- a/packages/definitions-parser/src/lib/module-info.ts +++ b/packages/definitions-parser/src/lib/module-info.ts @@ -1,131 +1,10 @@ -import assert = require("assert"); import * as path from "path"; import * as ts from "typescript"; -import { sort, joinPaths, FS, hasWindowsSlashes, assertDefined } from "@definitelytyped/utils"; +import { FS, assertDefined } from "@definitelytyped/utils"; import { readFileAndThrowOnBOM } from "./definition-parser"; import { getMangledNameForScopedPackage } from "../packages"; -export function getModuleInfo(packageName: string, all: Map): ModuleInfo { - const dependencies = new Set(); - const declaredModules: string[] = []; - const globals = new Set(); - - function addDependency(ref: string): void { - if (!ref.startsWith(".")) { - dependencies.add(ref); - } - } - - for (const sourceFile of all.values()) { - for (const ref of imports(sourceFile)) { - addDependency(ref.text); - } - for (const ref of sourceFile.typeReferenceDirectives) { - addDependency(ref.fileName); - } - if (ts.isExternalModule(sourceFile)) { - if (sourceFileExportsSomething(sourceFile)) { - declaredModules.push(properModuleName(packageName, sourceFile.fileName)); - const namespaceExport = sourceFile.statements.find(ts.isNamespaceExportDeclaration); - if (namespaceExport) { - globals.add(namespaceExport.name.text); - } - } - } else { - for (const node of sourceFile.statements) { - switch (node.kind) { - case ts.SyntaxKind.ModuleDeclaration: { - const decl = node as ts.ModuleDeclaration; - const name = decl.name.text; - if (decl.name.kind === ts.SyntaxKind.StringLiteral) { - declaredModules.push(assertNoWindowsSlashes(packageName, name)); - } else if (isValueNamespace(decl)) { - globals.add(name); - } - break; - } - case ts.SyntaxKind.VariableStatement: - for (const decl of (node as ts.VariableStatement).declarationList.declarations) { - if (decl.name.kind === ts.SyntaxKind.Identifier) { - globals.add(decl.name.text); - } - } - break; - case ts.SyntaxKind.EnumDeclaration: - case ts.SyntaxKind.ClassDeclaration: - case ts.SyntaxKind.FunctionDeclaration: { - // Deliberately not doing this for types, because those won't show up in JS code and can't be used for ATA - const nameNode = (node as ts.EnumDeclaration | ts.ClassDeclaration | ts.FunctionDeclaration).name; - if (nameNode) { - globals.add(nameNode.text); - } - break; - } - case ts.SyntaxKind.ImportEqualsDeclaration: - case ts.SyntaxKind.InterfaceDeclaration: - case ts.SyntaxKind.TypeAliasDeclaration: - case ts.SyntaxKind.EmptyStatement: - break; - default: - throw new Error(`Unexpected node kind ${ts.SyntaxKind[node.kind]}`); - } - } - } - } - - return { dependencies, declaredModules, globals: sort(globals) }; -} - -/** - * A file is a proper module if it is an external module *and* it has at least one export. - * A module with only imports is not a proper module; it likely just augments some other module. - */ -function sourceFileExportsSomething({ statements }: ts.SourceFile): boolean { - return statements.some((statement) => { - switch (statement.kind) { - case ts.SyntaxKind.ImportEqualsDeclaration: - case ts.SyntaxKind.ImportDeclaration: - return false; - case ts.SyntaxKind.ModuleDeclaration: - return (statement as ts.ModuleDeclaration).name.kind === ts.SyntaxKind.Identifier; - default: - return true; - } - }); -} - -interface ModuleInfo { - /** Full (possibly deep) module specifiers of dependencies (imports, type references, etc.). */ - dependencies: Set; - /** Anything from a `declare module "foo"` */ - declaredModules: string[]; - /** Every global symbol */ - globals: string[]; -} - -const extensions: Map = new Map(); -extensions.set(".d.ts", ""); // TODO: Inaccurate? -extensions.set(".d.mts", ".mjs"); -extensions.set(".d.cts", ".cjs"); - -/** - * Given a file name, get the name of the module it declares. - * `foo/index.d.ts` declares "foo", `foo/bar.d.ts` declares "foo/bar", "foo/bar/index.d.ts" declares "foo/bar" - */ -function properModuleName(folderName: string, fileName: string): string { - const part = - path.basename(fileName) === "index.d.ts" ? path.dirname(fileName) : withoutExtensions(fileName, extensions); - return part === "." ? folderName : joinPaths(folderName, part); -} - -function withoutExtensions(str: string, exts: typeof extensions): string { - const entries = Array.from(exts.entries()); - const ext = entries.find(([e, _]) => str.endsWith(e)); - assert(ext, `file "${str}" should end with extension ${entries.map(([e, _]) => `"${e}"`).join(", ")}`); - return str.slice(0, str.length - ext[0].length) + ext[1]; -} - /** Returns a map from filename (path relative to `directory`) to the SourceFile we parsed for it. */ export function allReferencedFiles( entryFilenames: readonly string[], @@ -133,10 +12,10 @@ export function allReferencedFiles( packageName: string, moduleResolutionHost: ts.ModuleResolutionHost, compilerOptions: ts.CompilerOptions -): { types: Map; tests: Map; hasNonRelativeImports: boolean } { +): { types: Map; tests: Set } { const seenReferences = new Set(); const types = new Map(); - const tests = new Map(); + const tests = new Set(); // 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/ @@ -145,17 +24,16 @@ export function allReferencedFiles( baseDirectory.lastIndexOf(`types/${getMangledNameForScopedPackage(packageName)}`) + `types/${getMangledNameForScopedPackage(packageName)}`.length ); - let hasNonRelativeImports = false; entryFilenames.forEach((fileName) => recur(undefined, { text: fileName, kind: "path" })); - return { types, tests, hasNonRelativeImports }; + return { types, tests }; function recur(containingFileName: string | undefined, ref: Reference): void { // An absolute file name for use with TS resolution, e.g. '/DefinitelyTyped/types/foo/index.d.ts' - const resolvedFileName = resolveReference(containingFileName, ref); - + let resolvedFileName = resolveReference(containingFileName, ref); if (!resolvedFileName) { return; } + resolvedFileName = fs.realPath(resolvedFileName); if (seenReferences.has(resolvedFileName)) { return; @@ -190,12 +68,11 @@ export function allReferencedFiles( ) { types.set(relativeFileName, src); } else { - tests.set(relativeFileName, src); + tests.add(relativeFileName); } - const { refs, hasNonRelativeImports: result } = findReferencedFiles(src, packageName); + const refs = findReferencedFiles(src, packageName); refs.forEach((ref) => recur(resolvedFileName, ref)); - hasNonRelativeImports = hasNonRelativeImports || result; } } @@ -247,8 +124,6 @@ interface Reference { */ function findReferencedFiles(src: ts.SourceFile, packageName: string) { const refs: Reference[] = []; - let hasNonRelativeImports = false; - for (const ref of src.referencedFiles) { refs.push({ text: ref.fileName, @@ -267,10 +142,9 @@ function findReferencedFiles(src: ts.SourceFile, packageName: string) { const resolutionMode = ts.getModeForUsageLocation(src, ref); if (ref.text.startsWith(".") || getMangledNameForScopedPackage(ref.text).startsWith(packageName + "/")) { refs.push({ kind: "import", text: ref.text, resolutionMode }); - hasNonRelativeImports = !ref.text.startsWith("."); } } - return { refs, hasNonRelativeImports }; + return refs; } /** @@ -355,72 +229,6 @@ function statementDeclaresValue(statement: ts.Statement): boolean { } } -function assertNoWindowsSlashes(packageName: string, fileName: string): string { - if (hasWindowsSlashes(fileName)) { - throw new Error(`In ${packageName}: Use forward slash instead when referencing ${fileName}`); - } - return fileName; -} - -export function getTestDependencies( - packageName: string, - testFiles: Iterable, - dependencies: ReadonlySet, - fs: FS, - moduleResolutionHost: ts.ModuleResolutionHost, - compilerOptions: ts.CompilerOptions -): Iterable { - const testDependencies = new Set(); - for (const filename of testFiles) { - const content = readFileAndThrowOnBOM(filename, fs); - const sourceFile = createSourceFile(filename, content, moduleResolutionHost, compilerOptions); - const { fileName, referencedFiles, typeReferenceDirectives } = sourceFile; - const filePath = () => path.join(packageName, fileName); - let hasImports = false; - let isModule = false; - let referencesSelf = false; - - for (const { fileName: ref } of referencedFiles) { - throw new Error(`Test files should not use ''. '${filePath()}' references '${ref}'.`); - } - for (const { fileName: referencedPackage } of typeReferenceDirectives) { - if (dependencies.has(referencedPackage)) { - throw new Error( - `'${filePath()}' unnecessarily references '${referencedPackage}', which is already referenced in the type definition.` - ); - } - if (referencedPackage === packageName) { - referencesSelf = true; - } - testDependencies.add(referencedPackage); - } - for (const imported of imports(sourceFile)) { - hasImports = true; - if (!imported.text.startsWith(".") && !dependencies.has(imported.text)) { - testDependencies.add(imported.text); - } - } - - isModule = - hasImports || - (() => { - // Note that this results in files without imports to be walked twice, - // once in the `imports(...)` function, and once more here: - for (const node of sourceFile.statements) { - if (node.kind === ts.SyntaxKind.ExportAssignment || node.kind === ts.SyntaxKind.ExportDeclaration) { - return true; - } - } - return false; - })(); - - if (isModule && referencesSelf) { - throw new Error(`'${filePath()}' unnecessarily references the package. This can be removed.`); - } - } - return testDependencies; -} - export function createSourceFile( filename: string, content: string, diff --git a/packages/definitions-parser/src/lib/settings.ts b/packages/definitions-parser/src/lib/settings.ts index 46ce09a019..6a36800f05 100644 --- a/packages/definitions-parser/src/lib/settings.ts +++ b/packages/definitions-parser/src/lib/settings.ts @@ -4,8 +4,8 @@ import { getUrlContentsAsString, withCache } from "./utils"; const root = joinPaths(__dirname, "..", ".."); export const storageDirPath = process.env.STORAGE_DIR || root; export const dataDirPath = joinPaths(storageDirPath, "data"); - export const sourceBranch = "master"; +export const sourceRemote = "origin"; export const typesDirectoryName = "types"; /** URL to download the repository from. */ diff --git a/packages/definitions-parser/src/lib/utils.ts b/packages/definitions-parser/src/lib/utils.ts index 3ab1ce25d9..ab69a055df 100644 --- a/packages/definitions-parser/src/lib/utils.ts +++ b/packages/definitions-parser/src/lib/utils.ts @@ -25,3 +25,10 @@ export function withCache(expiresInMs: number, getValue: () => Promise): ( return value!; }; } + +export function slicePrefixes(s: string, prefix: string): string { + while (s.startsWith(prefix)) { + s = s.slice(prefix.length); + } + return s; +} diff --git a/packages/definitions-parser/src/mocks.ts b/packages/definitions-parser/src/mocks.ts index 8913f64e0a..6f71c6eb48 100644 --- a/packages/definitions-parser/src/mocks.ts +++ b/packages/definitions-parser/src/mocks.ts @@ -1,4 +1,4 @@ -import { parseHeaderOrFail } from "@definitelytyped/header-parser"; +import { validatePackageJson } from "@definitelytyped/header-parser"; import { Dir, FS, InMemoryFS, mangleScopedPackage } from "@definitelytyped/utils"; import * as semver from "semver"; @@ -39,10 +39,17 @@ export class DTMock { * @param packageName The package of which an old version is to be added. * @param olderVersion The older version that's to be added. */ - public addOldVersionOfPackage(packageName: string, olderVersion: `${number}`) { + public addOldVersionOfPackage(packageName: string, olderVersion: `${number}`, fullVersion: string) { const latestDir = this.pkgDir(mangleScopedPackage(packageName)); const index = latestDir.get("index.d.ts") as string; - const latestHeader = parseHeaderOrFail(packageName, index); + if (latestDir.get("package.json") === undefined) { + throw new Error(`Package ${packageName} does not have a package.json`); + } + const packageJson = JSON.parse(latestDir.get("package.json") as string); + const latestHeader = validatePackageJson(mangleScopedPackage(packageName), packageJson, []); + if (Array.isArray(latestHeader)) { + throw new Error(latestHeader.join("\n")); + } const latestVersion = `${latestHeader.libraryMajorVersion}.${latestHeader.libraryMinorVersion}`; const olderVersionParsed = semver.coerce(olderVersion)!; @@ -62,12 +69,14 @@ export class DTMock { }, }) ); + oldDir.set("package.json", JSON.stringify({ ...packageJson, version: fullVersion })); latestDir.forEach((content, entry) => { if ( content !== oldDir && entry !== "index.d.ts" && entry !== "tsconfig.json" && + entry !== "package.json" && !(content instanceof Dir && /^v\d+(\.\d+)?$/.test(entry)) ) { oldDir.set(entry, content); @@ -84,11 +93,7 @@ export function createMockDT() { const boring = dt.pkgDir("boring"); boring.set( "index.d.ts", - `// Type definitions for boring 1.0 -// Project: https://boring.com -// Definitions by: Some Guy From Space -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - + ` import * as React from 'react'; export const drills: number; ` @@ -158,16 +163,22 @@ untested.d.ts ` ); boring.set("tsconfig.json", tsconfig(["boring-tests.ts"])); + boring.set( + "package.json", + packageJson("boring", "1.0", { + "@types/react": "*", + "@types/react-default": "*", + "@types/things": "*", + "@types/vorticon": "*", + "@types/manual": "*", + "@types/super-big-fun-hus": "*", + }) + ); const globby = dt.pkgDir("globby"); globby.set( "index.d.ts", - `// Type definitions for globby 0.2 -// Project: https://globby-gloopy.com -// Definitions by: The Dragon Quest Slime -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - -/// + `/// /// declare var x: number ` @@ -193,31 +204,16 @@ var z = y; ` ); globby.set("tsconfig.json", tsconfig(["globby-tests.ts", "test/other-tests.ts"])); + globby.set("package.json", packageJson("globby", "1.0", { "@types/andere": "*" })); const hasDependency = dt.pkgDir("has-dependency"); - hasDependency.set( - "index.d.ts", - `// Type definitions for has-dependency 3.3 -// Project: https://www.microsoft.com -// Definitions by: Andrew Branch -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// TypeScript Version: 3.1 - -export * from "moment"` - ); + hasDependency.set("index.d.ts", `export * from "moment"`); hasDependency.set("has-dependency-tests.ts", ""); hasDependency.set("tsconfig.json", tsconfig(["has-dependency-tests.ts"])); - hasDependency.set("package.json", `{ "private": true, "dependencies": { "moment": "*" } }`); + hasDependency.set("package.json", packageJson("has-dependency", "1.0", { moment: "*" })); const hasOlderTestDependency = dt.pkgDir("has-older-test-dependency"); - hasOlderTestDependency.set( - "index.d.ts", - `// Type definitions for has-older-test-dependency -// Project: https://github.com/baz/foo -// Definitions by: My Self -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -` - ); + hasOlderTestDependency.set("index.d.ts", ``); hasOlderTestDependency.set( "has-older-test-dependency-tests.ts", `import "jquery"; @@ -226,14 +222,14 @@ export * from "moment"` hasOlderTestDependency.set( "tsconfig.json", JSON.stringify({ - compilerOptions: { - paths: { - jquery: ["jquery/v1"], - }, - }, + compilerOptions: {}, files: ["index.d.ts", "has-older-test-dependency-tests.ts"], }) ); + hasOlderTestDependency.set( + "package.json", + packageJson("has-older-test-dependency", "1.0", { "@types/jquery": "1.0" }) + ); const jquery = dt.pkgDir("jquery"); jquery.set( @@ -244,13 +240,7 @@ declare var jQuery: 1; ); jquery.set( "index.d.ts", - `// Type definitions for jquery 3.3 -// Project: https://jquery.com -// Definitions by: Leonard Thieu -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// TypeScript Version: 2.3 - -/// + `/// export = jQuery; ` @@ -262,16 +252,15 @@ console.log(jQuery); ` ); jquery.set("tsconfig.json", tsconfig(["jquery-tests.ts"])); + jquery.set("package.json", packageJson("jquery", "3.3", {})); const scoped = dt.pkgDir("wordpress__plugins"); scoped.set( "index.d.ts", - `// Type definitions for @wordpress/plguins -// Project: https://github.com/WordPress/gutenberg/tree/master/packages/plugins/README.md -// Definitions by: Derek Sifford -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped + ` ` ); + scoped.set("package.json", packageJson("wordpress__plugins", "1.0", {})); scoped.set( "tsconfig.json", JSON.stringify({ @@ -314,3 +303,24 @@ ${testNames.map((s) => " " + JSON.stringify(s)).join(",\n")} ] }`; } + +function packageJson(packageName: string, version: string, dependencies: Record) { + return `{ + "private": true, + "name": "@types/${packageName}", + "version": "${version}.9999", + "projects": ["https://project"], + "owners": [{ + "name": "The Dragon Quest Slime", + "githubUsername": "slime" + }], + "dependencies": { + ${Object.entries(dependencies) + .map(([name, version]) => ` "${name}": "${version}"`) + .join(",\n")} + }, + "devDependencies": { + "@types/${packageName}": "workspace:." + } +}`; +} diff --git a/packages/definitions-parser/src/packages.ts b/packages/definitions-parser/src/packages.ts index d078a4343b..948c1f3a21 100644 --- a/packages/definitions-parser/src/packages.ts +++ b/packages/definitions-parser/src/packages.ts @@ -1,11 +1,12 @@ import assert = require("assert"); -import { Author } from "@definitelytyped/header-parser"; -import { FS, mapValues, assertSorted, unmangleScopedPackage, assertDefined, unique } from "@definitelytyped/utils"; -import { AllTypeScriptVersion, TypeScriptVersion } from "@definitelytyped/typescript-versions"; +import { Owner, Header, License } from "@definitelytyped/header-parser"; +import { TypeScriptVersion } from "@definitelytyped/typescript-versions"; +import { FS, assertDefined, assertSorted, mapValues, unique, unmangleScopedPackage } from "@definitelytyped/utils"; import * as semver from "semver"; import { readDataFile } from "./data-file"; -import { scopeName, typesDirectoryName } from "./lib/settings"; import { parseVersionFromDirectoryName } from "./lib/definition-parser"; +import { scopeName, typesDirectoryName } from "./lib/settings"; +import { slicePrefixes } from "./lib/utils"; export class AllPackages { static async read(dt: FS): Promise { @@ -40,22 +41,14 @@ export class AllPackages { return new TypingsData(versions[0], /*isLatest*/ true); } - static readSingleNotNeeded(name: string, dt: FS): NotNeededPackage { - const notNeeded = readNotNeededPackages(dt); - const pkg = notNeeded.find((p) => p.name === name); - if (pkg === undefined) { - throw new Error(`Cannot find not-needed package ${name}`); - } - return pkg; - } - private constructor( + /** Keys are `typesDirectoryName` strings */ private readonly data: ReadonlyMap, private readonly notNeeded: readonly NotNeededPackage[] ) {} - getNotNeededPackage(name: string): NotNeededPackage | undefined { - return this.notNeeded.find((p) => p.name === name); + getNotNeededPackage(typesDirectoryName: string): NotNeededPackage | undefined { + return this.notNeeded.find((p) => p.typesDirectoryName === typesDirectoryName); } hasTypingFor(dep: PackageId): boolean { @@ -66,40 +59,44 @@ export class AllPackages { * Whether a package maintains multiple minor versions of typings simultaneously by * using minor-versioned directories like 'react-native/v14.1' */ - hasSeparateMinorVersions(name: string) { - const versions = Array.from(assertDefined(this.data.get(getMangledNameForScopedPackage(name))).getAll()); + hasSeparateMinorVersions(typesDirectoryName: string) { + const versions = Array.from(assertDefined(this.data.get(typesDirectoryName)).getAll()); const minors = versions.map((v) => v.minor); return minors.length !== unique(minors).length; } tryResolve(dep: PackageId): PackageId { - const versions = this.data.get(getMangledNameForScopedPackage(dep.name)); - return (versions && versions.tryGet(dep.version)?.id) || dep; + const typesDirectoryName = dep.typesDirectoryName ?? dep.name.slice(scopeName.length + 2); + const versions = this.data.get(typesDirectoryName); + const depVersion = new semver.Range(dep.version === "*" ? "*" : `^${formatTypingVersion(dep.version)}`); + return (versions && versions.tryGet(depVersion)?.id) || dep; } resolve(dep: PackageId): PackageIdWithDefiniteVersion { - const versions = this.data.get(getMangledNameForScopedPackage(dep.name)); + const typesDirectoryName = dep.typesDirectoryName ?? dep.name.slice(scopeName.length + 2); + const versions = this.data.get(typesDirectoryName); if (!versions) { - throw new Error(`No typings found with name '${dep.name}'.`); + throw new Error(`No typings found with directory name '${dep.typesDirectoryName}'.`); } - return versions.get(dep.version).id; + const depVersion = new semver.Range(dep.version === "*" ? "*" : `^${formatTypingVersion(dep.version)}`); + return versions.get(depVersion).id; } /** Gets the latest version of a package. E.g. getLatest(node v6) was node v10 (before node v11 came out). */ getLatest(pkg: TypingsData): TypingsData { - return pkg.isLatest ? pkg : this.getLatestVersion(pkg.name); + return pkg.isLatest ? pkg : this.getLatestVersion(pkg.typesDirectoryName); } - private getLatestVersion(packageName: string): TypingsData { - const latest = this.tryGetLatestVersion(packageName); + private getLatestVersion(typesDirectoryName: string): TypingsData { + const latest = this.tryGetLatestVersion(typesDirectoryName); if (!latest) { - throw new Error(`No such package ${packageName}.`); + throw new Error(`No such package ${typesDirectoryName}.`); } return latest; } - tryGetLatestVersion(packageName: string): TypingsData | undefined { - const versions = this.data.get(getMangledNameForScopedPackage(packageName)); + tryGetLatestVersion(typesDirectoryName: string): TypingsData | undefined { + const versions = this.data.get(typesDirectoryName); return versions && versions.getLatest(); } @@ -111,9 +108,12 @@ export class AllPackages { return pkg; } - tryGetTypingsData({ name, version }: PackageId): TypingsData | undefined { - const versions = this.data.get(getMangledNameForScopedPackage(name)); - return versions && versions.tryGet(version); + tryGetTypingsData(pkg: PackageId): TypingsData | undefined { + const typesDirectoryName = pkg.typesDirectoryName ?? pkg.name.slice(scopeName.length + 2); + const versions = this.data.get(typesDirectoryName); + return ( + versions && versions.tryGet(new semver.Range(pkg.version === "*" ? "*" : `^${formatTypingVersion(pkg.version)}`)) + ); } allPackages(): readonly AnyPackage[] { @@ -136,30 +136,24 @@ export class AllPackages { return this.notNeeded; } - /** Returns all of the dependences *that have typings*, ignoring others, and including test dependencies. */ + /** Returns all of the dependencies *that are typed on DT*, ignoring others, and including test dependencies. */ *allDependencyTypings(pkg: TypingsData): Iterable { - for (const [name, version] of Object.entries(pkg.dependencies)) { - const versions = this.data.get(getMangledNameForScopedPackage(name)); - if (versions) { - yield versions.get( - version, - pkg.pathMappings[name] - ? `${pkg.name} references this version of ${name} in its path mappings in tsconfig.json. If you are deleting this version, update ${pkg.name}’s path mappings accordingly.\n` - : undefined - ); - } - } - - for (const name of pkg.testDependencies) { - const versions = this.data.get(getMangledNameForScopedPackage(name)); + for (const [name, version] of pkg.allPackageJsonDependencies()) { + if (!name.startsWith(`@${scopeName}/`)) continue; + if (pkg.name === name) continue; + const typesDirectoryName = removeTypesScope(name); + const versions = this.data.get(typesDirectoryName); if (versions) { - const version = pkg.pathMappings[name]; - yield version ? versions.get(version) : versions.getLatest(); + yield versions.get(new semver.Range(version), pkg.name + ":" + JSON.stringify((versions as any).versions)); } } } } +export function removeTypesScope(name: string) { + return slicePrefixes(name, `@${scopeName}/`); +} + // Same as the function in moduleNameResolver.ts in typescript export function getMangledNameForScopedPackage(packageName: string): string { if (packageName.startsWith("@")) { @@ -181,27 +175,31 @@ function* flattenData(data: ReadonlyMap): Iterable; +export interface TypingsDataRaw { /** - * Other definitions, that exist in the same typings repo, that this package depends on. - * - * These will refer to *package names*, not *folder names*. + * Header info contained in package.json */ - readonly dependencies: { readonly [name: string]: DependencyVersion }; + readonly header: Header; /** * Package `imports`, as read in the `package.json` file @@ -366,31 +324,15 @@ export interface TypingsDataRaw extends BaseRaw { readonly type?: string; /** - * Other definitions, that exist in the same typings repo, that the tests, but not the types, of this package depend on. - * - * These are always the latest version and will not include anything already in `dependencies`. - */ - readonly testDependencies: readonly string[]; - - /** - * External packages, from outside the typings repo, that provide definitions that this package depends on. - */ - readonly packageJsonDependencies: readonly PackageJsonDependency[]; - - /** - * Represents that there was a path mapping to a package. - * - * Not all path mappings are direct dependencies, they may be necessary for transitive dependencies. However, where `dependencies` and - * `pathMappings` share a key, they *must* share the same value. + * Packages that provide definitions that this package depends on. + * NOTE: Includes `@types/` packages. */ - readonly pathMappings: { readonly [packageName: string]: DirectoryParsedTypingVersion }; + readonly dependencies: PackageJsonDependencies; /** - * List of people that have contributed to the definitions in this package. - * - * These people will be requested for issue/PR review in the https://github.com/DefinitelyTyped/DefinitelyTyped repo. + * Packages that this package's tests or other development depends on. */ - readonly contributors: readonly Author[]; + readonly devDependencies: PackageJsonDependencies; /** * The [older] version of the library that this definition package refers to, as represented *on-disk*. @@ -399,25 +341,6 @@ export interface TypingsDataRaw extends BaseRaw { */ readonly libraryVersionDirectoryName?: string; - /** - * Major version of the library. - * - * This data is parsed from a header comment in the entry point `.d.ts` and will be `0` if the file did not specify a version. - */ - readonly libraryMajorVersion: number; - - /** - * Minor version of the library. - * - * This data is parsed from a header comment in the entry point `.d.ts` and will be `0` if the file did not specify a version. - */ - readonly libraryMinorVersion: number; - - /** - * Minimum required TypeScript version to consume the definitions from this package. - */ - readonly minTsVersion: AllTypeScriptVersion; - /** * List of TS versions that have their own directories, and corresponding "typesVersions" in package.json. * Usually empty. @@ -427,7 +350,7 @@ export interface TypingsDataRaw extends BaseRaw { /** * Files that should be published with this definition, e.g. ["jquery.d.ts", "jquery-extras.d.ts"] * - * Does *not* include a partial `package.json` because that will not be copied directly. + * Does *not* include `package.json` because that is not copied directly, but generated from TypingsData. */ readonly files: readonly string[]; @@ -442,45 +365,6 @@ export interface TypingsDataRaw extends BaseRaw { * A hash of the names and contents of the `files` list, used for versioning. */ readonly contentHash: string; - - /** - * Name or URL of the project, e.g. "http://cordova.apache.org". - */ - readonly projectName: string; - - /** - * A list of *values* declared in the global namespace. - * - * @note This does not include *types* declared in the global namespace. - */ - readonly globals: readonly string[]; - - /** - * External modules declared by this package. Includes the containing folder name when applicable (e.g. proper module). - */ - readonly declaredModules: readonly string[]; -} - -// Note that BSD is not supported -- for that, we'd have to choose a *particular* BSD license from the list at https://spdx.org/licenses/ -export const enum License { - MIT = "MIT", - Apache20 = "Apache-2.0", -} -const allLicenses = [License.MIT, License.Apache20]; -export function getLicenseFromPackageJson(packageJsonLicense: unknown): License { - if (packageJsonLicense === undefined) { - // tslint:disable-line strict-type-predicates (false positive) - return License.MIT; - } - if (typeof packageJsonLicense === "string" && packageJsonLicense === "MIT") { - throw new Error(`Specifying '"license": "MIT"' is redundant, this is the default.`); - } - if (allLicenses.includes(packageJsonLicense as License)) { - return packageJsonLicense as License; - } - throw new Error( - `'package.json' license is ${JSON.stringify(packageJsonLicense)}.\nExpected one of: ${JSON.stringify(allLicenses)}}` - ); } export class TypingsVersions { @@ -496,7 +380,7 @@ export class TypingsVersions { * Sorted from latest to oldest so that we publish the current version first. * This is important because older versions repeatedly reset the "latest" tag to the current version. */ - this.versions = Object.keys(data).map((key) => new semver.SemVer(`${key}.0`)); + this.versions = Object.keys(data).map((key) => new semver.SemVer(`${key}.9999`)); this.versions.sort(semver.rcompare); this.map = new Map( @@ -507,59 +391,55 @@ export class TypingsVersions { getAll(): Iterable { return this.map.values(); } - - get(version: DependencyVersion, errorMessage?: string): TypingsData { - return version === "*" ? this.getLatest() : this.getLatestMatch(version, errorMessage); - } - - tryGet(version: DependencyVersion): TypingsData | undefined { - return version === "*" ? this.getLatest() : this.tryGetLatestMatch(version); - } - - getLatest(): TypingsData { - return this.map.get(this.versions[0])!; - } - - private getLatestMatch(version: DirectoryParsedTypingVersion, errorMessage?: string): TypingsData { - const data = this.tryGetLatestMatch(version); + get(version: semver.Range, errorMessage?: string): TypingsData { + const data = this.tryGet(version); if (!data) { - throw new Error(`Could not find version ${version.major}.${version.minor ?? "*"}. ${errorMessage || ""}`); + throw new Error(`Could not match version ${version} in ${this.versions}. ${errorMessage || ""}`); } return data; } + tryGet(version: semver.Range): TypingsData | undefined { + try { + const found = this.versions.find((v) => version.test(v)); + return found && this.map.get(found); + } catch (e) { + console.log(version); + throw e; + } + } - private tryGetLatestMatch(version: DirectoryParsedTypingVersion): TypingsData | undefined { - const found = this.versions.find( - (v) => v.major === version.major && (version.minor === undefined || v.minor === version.minor) - ); - return found && this.map.get(found); + getLatest(): TypingsData { + return this.map.get(this.versions[0])!; } } export class TypingsData extends PackageBase { constructor(private readonly data: TypingsDataRaw, readonly isLatest: boolean) { - super(data); + super(); } - get name() { - return this.data.typingsPackageName; + get name(): string { + return this.data.header.name; } - - get testDependencies(): readonly string[] { - return this.data.testDependencies; + get libraryName(): string { + return ( + this.data.header.nonNpmDescription ?? unmangleScopedPackage(this.typesDirectoryName) ?? this.typesDirectoryName + ); } - get contributors(): readonly Author[] { - return this.data.contributors; + get contributors(): readonly Owner[] { + return this.data.header.owners; } get major(): number { - return this.data.libraryMajorVersion; + return this.data.header.libraryMajorVersion; } get minor(): number { - return this.data.libraryMinorVersion; + return this.data.header.libraryMinorVersion; } get minTypeScriptVersion(): TypeScriptVersion { - return TypeScriptVersion.isSupported(this.data.minTsVersion) ? this.data.minTsVersion : TypeScriptVersion.lowest; + return TypeScriptVersion.isSupported(this.data.header.minimumTypeScriptVersion) + ? this.data.header.minimumTypeScriptVersion + : TypeScriptVersion.lowest; } get typesVersions(): readonly TypeScriptVersion[] { return this.data.typesVersions; @@ -574,29 +454,26 @@ export class TypingsData extends PackageBase { get license(): License { return this.data.license; } - get packageJsonDependencies(): readonly PackageJsonDependency[] { - return this.data.packageJsonDependencies; - } - get contentHash(): string { - return this.data.contentHash; - } - get declaredModules(): readonly string[] { - return this.data.declaredModules; + get dependencies(): PackageJsonDependencies { + return this.data.dependencies ?? {}; } - get projectName(): string { - return this.data.projectName; + get devDependencies(): PackageJsonDependencies { + return this.data.devDependencies ?? {}; } - get globals(): readonly string[] { - return this.data.globals; + *allPackageJsonDependencies(): Iterable<[string, string]> { + for (const [name, version] of Object.entries(this.dependencies)) { + yield [name, version]; + } + for (const [name, version] of Object.entries(this.devDependencies)) { + yield [name, version]; + } } - get pathMappings(): { readonly [packageName: string]: DirectoryParsedTypingVersion } { - return this.data.pathMappings; + get contentHash(): string { + return this.data.contentHash; } - - get dependencies(): { readonly [name: string]: DependencyVersion } { - return this.data.dependencies; + get projectName(): string | undefined { + return this.data.header.projects[0]; } - get type() { return this.data.type; } @@ -609,24 +486,34 @@ export class TypingsData extends PackageBase { return this.data.exports; } + get nonNpm() { + return this.data.header.nonNpm; + } + get versionDirectoryName() { return this.data.libraryVersionDirectoryName && `v${this.data.libraryVersionDirectoryName}`; } /** Path to this package, *relative* to the DefinitelyTyped directory. */ get subDirectoryPath(): string { - return this.isLatest ? this.name : `${this.name}/${this.versionDirectoryName}`; + return this.isLatest ? this.typesDirectoryName : `${this.typesDirectoryName}/${this.versionDirectoryName}`; } } /** Uniquely identifies a package. */ -export interface PackageId { - readonly name: string; - readonly version: DependencyVersion; -} +export type PackageId = + | { + readonly typesDirectoryName: string; + readonly version: DirectoryParsedTypingVersion | "*"; + } + | { + readonly name: string; + readonly typesDirectoryName?: undefined; + readonly version: DirectoryParsedTypingVersion | "*"; + }; export interface PackageIdWithDefiniteVersion { - readonly name: string; + readonly typesDirectoryName: string; readonly version: HeaderParsedTypingVersion; } @@ -649,7 +536,9 @@ export function readNotNeededPackages(dt: FS): readonly NotNeededPackage[] { * For "types/a/v3/c", returns { name: "a", version: 3 }. * For "x", returns undefined. */ -export function getDependencyFromFile(file: string): PackageId | undefined { +export function getDependencyFromFile( + file: string +): { typesDirectoryName: string; version: DirectoryParsedTypingVersion | "*" } | undefined { const parts = file.split("/"); if (parts.length <= 2) { // It's not in a typings directory at all. @@ -665,9 +554,9 @@ export function getDependencyFromFile(file: string): PackageId | undefined { if (subDirName) { const version = parseVersionFromDirectoryName(subDirName); if (version !== undefined) { - return { name, version }; + return { typesDirectoryName: name, version }; } } - return { name, version: "*" }; + return { typesDirectoryName: name, version: "*" }; } diff --git a/packages/definitions-parser/src/parse-definitions.ts b/packages/definitions-parser/src/parse-definitions.ts index 162188fe0d..3d2122ed0a 100644 --- a/packages/definitions-parser/src/parse-definitions.ts +++ b/packages/definitions-parser/src/parse-definitions.ts @@ -1,11 +1,9 @@ -import { FS, LoggerWithErrors, filterNAtATimeOrdered, runWithChildProcesses } from "@definitelytyped/utils"; +import { FS, LoggerWithErrors, filterNAtATimeOrdered, joinPaths, runWithChildProcesses } from "@definitelytyped/utils"; import { writeDataFile } from "./data-file"; import { getTypingInfo } from "./lib/definition-parser"; import { definitionParserWorkerFilename } from "./lib/definition-parser-worker"; import { AllPackages, readNotNeededPackages, typesDataFilename, TypingsVersionsRaw } from "./packages"; -export { tryParsePackageVersion, parsePackageVersion } from "./lib/definition-parser"; - export interface ParallelOptions { readonly nProcesses: number; readonly definitelyTypedPath: string; @@ -18,12 +16,15 @@ export async function parseDefinitions( ): Promise { log.info("Parsing definitions..."); const typesFS = dt.subDir("types"); - const packageNames = await filterNAtATimeOrdered(parallel ? parallel.nProcesses : 1, typesFS.readdir(), (name) => - typesFS.isDirectory(name) + const packageNames = await filterNAtATimeOrdered( + parallel ? parallel.nProcesses : 1, + typesFS.readdir(), + (name) => typesFS.isDirectory(name) && typesFS.exists(joinPaths(name, "package.json")) ); log.info(`Found ${packageNames.length} packages.`); const typings: { [name: string]: TypingsVersionsRaw } = {}; + const errors: string[] = []; const start = Date.now(); if (parallel) { @@ -33,19 +34,33 @@ export async function parseDefinitions( commandLineArgs: [`${parallel.definitelyTypedPath}/types`], workerFile: definitionParserWorkerFilename, nProcesses: parallel.nProcesses, - handleOutput({ data, packageName }: { data: TypingsVersionsRaw; packageName: string }) { - typings[packageName] = data; + handleOutput({ data, packageName }: { data: TypingsVersionsRaw | { errors: string[] }; packageName: string }) { + if ("errors" in data) { + errors.push(...data.errors); + } else { + typings[packageName] = data; + } }, }); } else { + const errors = []; log.info("Parsing in main process..."); for (const packageName of packageNames) { - typings[packageName] = await getTypingInfo(packageName, dt); + const info = await getTypingInfo(packageName, dt); + if ("errors" in info) { + errors.push(...info.errors); + } else { + typings[packageName] = info; + } } } + if (errors.length > 0) { + throw new Error(errors.join("\n")); + } log.info("Parsing took " + (Date.now() - start) / 1000 + " s"); - await writeDataFile(typesDataFilename, sorted(typings)); - return AllPackages.from(typings, readNotNeededPackages(dt)); + const sortedTypings = sorted(typings); + await writeDataFile(typesDataFilename, sortedTypings); + return AllPackages.from(sortedTypings, readNotNeededPackages(dt)); } function sorted(obj: { [name: string]: T }): { [name: string]: T } { diff --git a/packages/definitions-parser/test/definition-parser.test.ts b/packages/definitions-parser/test/definition-parser.test.ts index 802460c81b..9c48a58d7f 100644 --- a/packages/definitions-parser/test/definition-parser.test.ts +++ b/packages/definitions-parser/test/definition-parser.test.ts @@ -6,8 +6,8 @@ import { getTypingInfo } from "../src/lib/definition-parser"; describe(getTypingInfo, () => { it("keys data by major.minor version", async () => { const dt = createMockDT(); - dt.addOldVersionOfPackage("jquery", "1.42"); - dt.addOldVersionOfPackage("jquery", "2"); + dt.addOldVersionOfPackage("jquery", "1.42", "1.42.9999"); + dt.addOldVersionOfPackage("jquery", "2", "2.0.9999"); const info = await getTypingInfo("jquery", dt.fs); expect(Object.keys(info).sort()).toEqual(["1.42", "2.0", "3.3"]); @@ -16,21 +16,13 @@ describe(getTypingInfo, () => { it("works for a package with dependencies", async () => { const dt = createMockDT(); const info = await getTypingInfo("has-dependency", dt.fs); - expect(info).toBeDefined(); + expect("errors" in info).toBeFalsy(); }); it("works for non-module files with empty statements", async () => { const dt = createMockDT(); const d = dt.pkgDir("example"); - d.set( - "index.d.ts", - `// Type definitions for example 1.0 -// Project: https://github.com/example/com -// Definitions by: My Self -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - -;;` - ); + d.set("index.d.ts", `;;`); d.set( "tsconfig.json", @@ -39,31 +31,58 @@ describe(getTypingInfo, () => { compilerOptions: {}, }) ); + d.set( + "package.json", + JSON.stringify({ + private: true, + name: "@types/example", + version: "25.0.9999", + projects: ["https://github.com/ckeditor/ckeditor5/tree/master/packages/ckeditor5-engine"], + owners: [ + { + name: "Example", + url: "https://example.com/example", + }, + ], + devDependencies: { + "@types/example": "workspace:.", + }, + }) + ); const info = await getTypingInfo("example", dt.fs); - expect(info).toBeDefined(); + expect("errors" in info).toBeFalsy(); }); it("works for a scoped package with scoped older dependencies", async () => { const dt = createMockDT(); const scopedWithOlderScopedDependency = dt.pkgDir("ckeditor__ckeditor5-engine"); - scopedWithOlderScopedDependency.set( - "index.d.ts", - `// Type definitions for @ckeditor/ckeditor-engine 25.0 -// Project: https://github.com/ckeditor/ckeditor5/tree/master/packages/ckeditor5-engine -// Definitions by: My Self -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - -import * as utils from '@ckeditor/ckeditor5-utils';` - ); + scopedWithOlderScopedDependency.set("index.d.ts", `import * as utils from '@ckeditor/ckeditor5-utils';`); scopedWithOlderScopedDependency.set( "tsconfig.json", JSON.stringify({ files: ["index.d.ts"], - compilerOptions: { - paths: { - "@ckeditor/ckeditor5-utils": ["ckeditor__ckeditor5-utils/v10"], + compilerOptions: {}, + }) + ); + scopedWithOlderScopedDependency.set( + "package.json", + JSON.stringify({ + private: true, + name: "@types/ckeditor__ckeditor5-engine", + version: "25.0.9999", + projects: ["https://github.com/ckeditor/ckeditor5/tree/master/packages/ckeditor5-engine"], + owners: [ + { + name: "Example", + url: "https://zombo.com/ñ", }, + ], + dependencies: { + "@types/ckeditor__ckeditor5-utils": "10.0.9999", + }, + devDependencies: { + "@types/ckeditor__ckeditor5-engine": "workspace:.", }, }) ); @@ -71,10 +90,7 @@ import * as utils from '@ckeditor/ckeditor5-utils';` const olderScopedPackage = dt.pkgDir("ckeditor__ckeditor5-utils"); olderScopedPackage.set( "index.d.ts", - `// Type definitions for @ckeditor/ckeditor5-utils 25.0 -// Project: https://github.com/ckeditor/ckeditor5/tree/master/packages/ckeditor5-utils -// Definitions by: My Self -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped + ` export function myFunction(arg:string): string; ` ); @@ -82,36 +98,41 @@ export function myFunction(arg:string): string; "tsconfig.json", JSON.stringify({ files: ["index.d.ts"], - compilerOptions: { - paths: {}, + compilerOptions: {}, + }) + ); + olderScopedPackage.set( + "package.json", + JSON.stringify({ + private: true, + name: "@types/ckeditor__ckeditor5-utils", + version: "25.0.9999", + projects: ["https://github.com/ckeditor/ckeditor5/tree/master/packages/ckeditor5-utils"], + owners: [ + { + name: "Example", + githubUsername: "ñ", + }, + ], + dependencies: {}, + devDependencies: { + "@types/ckeditor__ckeditor5-utils": "workspace:.", }, }) ); + dt.addOldVersionOfPackage("@ckeditor/ckeditor5-utils", "10", "10.0.9999"); - dt.addOldVersionOfPackage("@ckeditor/ckeditor5-utils", "10"); - - const info = await getTypingInfo("@ckeditor/ckeditor5-engine", dt.fs); - expect(info).toBeDefined(); + const info = await getTypingInfo("ckeditor__ckeditor5-engine", dt.fs); + expect("errors" in info).toBeFalsy(); }); - it("allows path mapping to older versions", () => { - // Actually, the default setup already has 'has-older-test-dependency', so probably doesn't need an explicit test - const dt = createMockDT(); - dt.addOldVersionOfPackage("jquery", "1.42"); - dt.addOldVersionOfPackage("jquery", "2"); - // now add a dependency that maps to jquery/1.42 - }); it("allows path mapping to node/buffer", async () => { // Actually, the default seup already has 'has-older-test-dependency', so probably doesn't need an explicit test const dt = createMockDT(); const safer = dt.pkgDir("safer"); safer.set( "index.d.ts", - `// Type definitions for safer 1.0 -// Project: https://github.com/safer/safer -// Definitions by: Noone -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - + ` /// export * from 'buffer'; ` @@ -148,10 +169,34 @@ export * from 'buffer'; ] } ` ); + safer.set( + "package.json", + JSON.stringify({ + private: true, + name: "@types/safer", + version: "1.0.9999", + projects: ["https://github.com/safer/safer"], + owners: [ + { + name: "Noone", + githubUsername: "noone", + }, + ], + dependencies: { + "@types/node": "*", + }, + devDependencies: { + "@types/safer": "workspace:.", + }, + }) + ); const info = await getTypingInfo("safer", dt.fs); - expect(info).toBeDefined(); - expect(info["1.0"].dependencies).toEqual({ node: "*" }); + if ("errors" in info) { + throw new Error(info.errors.join("\n")); + } + expect(info["1.0"]).toBeDefined(); + expect(info["1.0"].dependencies).toEqual({ "@types/node": "*" }); }); it("errors on arbitrary path mapping", () => {}); it("supports node_modules passthrough path mapping", async () => { @@ -212,18 +257,27 @@ const a = new webpack.AutomaticPrefetchPlugin(); ] }` ); + webpack.set( + "package.json", + JSON.stringify({ + private: true, + name: "@types/webpack", + version: "5.2.9999", + projects: ["https://github.com/webpack/webpack"], + owners: [ + { + name: "Qubo", + githubUsername: "tkqubo", + }, + ], + devDependencies: { + "@types/webpack": "workspace:.", + }, + }) + ); const info = await getTypingInfo("webpack", dt.fs); - expect(info).toBeDefined(); - }); - - it("rejects references to old versions of other @types packages", () => { - return expect( - getTypingInfo( - "typeref-fails", - new DiskFS(path.resolve(__dirname, "fixtures/rejects-references-to-old-versions-of-other-types-packages/")) - ) - ).rejects.toThrow("do not directly import specific versions of another types package"); + expect("errors" in info).toBeFalsy(); }); it("allows references to old versions of self", async () => { @@ -231,7 +285,7 @@ const a = new webpack.AutomaticPrefetchPlugin(); "fail", new DiskFS(path.resolve(__dirname, "fixtures/allows-references-to-old-versions-of-self/")) ); - expect(info).toBeDefined(); + expect("errors" in info).toBeFalsy(); }); it("omits test dependencies on modules declared in index.d.ts", async () => { @@ -239,12 +293,7 @@ const a = new webpack.AutomaticPrefetchPlugin(); const ember = dt.pkgDir("ember"); ember.set( "index.d.ts", - `// Type definitions for ember 2.8 -// Project: https://github.com/ember/ember -// Definitions by: Chris Krycho -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - -/// + `/// declare module '@ember/routing/route' { } declare module '@ember/routing/rotorooter' { @@ -282,9 +331,35 @@ import route = require('@ember/routing/route'); ] }` ); + ember.set( + "package.json", + `{ + "private": true, + "name": "@types/ember", + "version": "2.8.9999", + "dependencies": { + "@types/ember__routing": "*" + }, + "devDependencies": { + "@types/ember": "workspace:." + }, + "projects": [ + "https://github.com/ember" + ], + "owners": [ + { + "name": "Chris Krycho", + "githubUsername": "chriskrycho" + } + ] +}` + ); const info = await getTypingInfo("ember", dt.fs); - expect(info["2.8"].testDependencies).toEqual([]); + if ("errors" in info) { + throw new Error(info.errors.join("\n")); + } + expect(info["2.8"].devDependencies).toEqual({ "@types/ember": "workspace:." }); }); it("doesn't omit dependencies if only some deep modules are declared", async () => { @@ -292,7 +367,10 @@ import route = require('@ember/routing/route'); "styled-components-react-native", new DiskFS(path.resolve(__dirname, "fixtures/doesnt-omit-dependencies-if-only-some-deep-modules-are-declared/")) ); - expect(info["5.1"].dependencies).toEqual({ "styled-components": "*" }); + if ("errors" in info) { + throw new Error(info.errors.join("\n")); + } + expect(info["5.1"].dependencies).toEqual({ "@types/styled-components": "*" }); }); it("rejects relative references to other packages", async () => { @@ -307,8 +385,8 @@ import route = require('@ember/routing/route'); describe("concerning multiple versions", () => { it("records what the version directory looks like on disk", async () => { const dt = createMockDT(); - dt.addOldVersionOfPackage("jquery", "2"); - dt.addOldVersionOfPackage("jquery", "1.5"); + dt.addOldVersionOfPackage("jquery", "2", "2.0.9999"); + dt.addOldVersionOfPackage("jquery", "1.5", "1.5.9999"); const info = await getTypingInfo("jquery", dt.fs); expect(info).toEqual({ @@ -325,136 +403,34 @@ import route = require('@ember/routing/route'); }); }); - it("records a path mapping to the version directory", async () => { - const dt = createMockDT(); - dt.addOldVersionOfPackage("jquery", "2"); - dt.addOldVersionOfPackage("jquery", "1.5"); - const info = await getTypingInfo("jquery", dt.fs); - - expect(info).toEqual({ - "1.5": expect.objectContaining({ - pathMappings: { - jquery: { major: 1, minor: 5 }, - }, - }), - "2.0": expect.objectContaining({ - pathMappings: { - jquery: { major: 2, minor: undefined }, - }, - }), - "3.3": expect.objectContaining({ - // The latest version does not have path mappings of its own - pathMappings: {}, - }), - }); - }); - - it("records a path mapping to the scoped version directory", async () => { - const dt = createMockDT(); - const pkg = dt.pkgDir("ckeditor__ckeditor5-utils"); - pkg.set( - "tsconfig.json", - JSON.stringify({ - files: ["index.d.ts"], - compilerOptions: { - paths: {}, - }, - }) - ); - pkg.set( - "index.d.ts", - `// Type definitions for @ckeditor/ckeditor-utils 25.0 -// Project: https://github.com/ckeditor/ckeditor5/tree/master/packages/ckeditor5-engine -// Definitions by: My Self -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -` - ); - - dt.addOldVersionOfPackage("@ckeditor/ckeditor5-utils", "10"); - - const info = await getTypingInfo("@ckeditor/ckeditor5-utils", dt.fs); - expect(info).toEqual({ - "10.0": expect.objectContaining({ - pathMappings: { - "@ckeditor/ckeditor5-utils": { major: 10 }, - }, - }), - "25.0": expect.objectContaining({ - // The latest version does not have path mappings of its own - pathMappings: {}, - }), - }); - }); - describe("validation thereof", () => { it("throws if a directory exists for the latest major version", () => { const dt = createMockDT(); - dt.addOldVersionOfPackage("jquery", "3"); - - return expect(getTypingInfo("jquery", dt.fs)).rejects.toThrow( - "The latest version of the 'jquery' package is 3.3, so the subdirectory 'v3' is not allowed; " + - "since it applies to any 3.* version, up to and including 3.3." - ); + dt.addOldVersionOfPackage("jquery", "3", "3.0.9999"); + + return expect(getTypingInfo("jquery", dt.fs)).resolves.toEqual({ + errors: [ + "The latest version of the 'jquery' package is 3.3, so the subdirectory 'v3' is not allowed; " + + "since it applies to any 3.* version, up to and including 3.3.", + ], + }); }); it("throws if a directory exists for the latest minor version", () => { const dt = createMockDT(); - dt.addOldVersionOfPackage("jquery", "3.3"); + dt.addOldVersionOfPackage("jquery", "3.3", "3.3.9999"); - return expect(getTypingInfo("jquery", dt.fs)).rejects.toThrow( - "The latest version of the 'jquery' package is 3.3, so the subdirectory 'v3.3' is not allowed." - ); + return expect(getTypingInfo("jquery", dt.fs)).resolves.toEqual({ + errors: ["The latest version of the 'jquery' package is 3.3, so the subdirectory 'v3.3' is not allowed."], + }); }); it("does not throw when a minor version is older than the latest", () => { const dt = createMockDT(); - dt.addOldVersionOfPackage("jquery", "3.0"); + dt.addOldVersionOfPackage("jquery", "3.0", "3.0.9999"); return expect(getTypingInfo("jquery", dt.fs)).resolves.toBeDefined(); }); - - it("checks that older versions with non-relative imports have wildcard path mappings", () => { - const dt = createMockDT(); - const jquery = dt.pkgDir("jquery"); - jquery.set( - "JQuery.d.ts", - `import "jquery/component"; -` - ); - dt.addOldVersionOfPackage("jquery", "1"); - return expect(getTypingInfo("jquery", dt.fs)).rejects.toThrow( - 'jquery: Older version 1 must have a "paths" entry of "jquery/*": ["jquery/v1/*"]' - ); - }); - - it("checks that scoped older versions with non-relative imports have wildcard path mappings", () => { - const dt = createMockDT(); - const pkg = dt.pkgDir("ckeditor__ckeditor5-utils"); - pkg.set( - "index.d.ts", - `// Type definitions for @ckeditor/ckeditor5-utils 25.0 -// Project: https://github.com/ckeditor/ckeditor5/tree/master/packages/ckeditor5-utils -// Definitions by: My Self -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -import first from "@ckeditor/ckeditor5-utils/src/first"; - ` - ); - pkg.set( - "tsconfig.json", - JSON.stringify({ - files: ["index.d.ts"], - compilerOptions: { - paths: {}, - }, - }) - ); - - dt.addOldVersionOfPackage("@ckeditor/ckeditor5-utils", "10"); - - return expect(getTypingInfo("ckeditor__ckeditor5-utils", dt.fs)).rejects.toThrow( - '@ckeditor/ckeditor5-utils: Older version 10 must have a "paths" entry of "@ckeditor/ckeditor5-utils/*": ["ckeditor__ckeditor5-utils/v10/*"]' - ); - }); }); }); diff --git a/packages/definitions-parser/test/fixtures/allows-references-to-old-versions-of-self/types/fail/index.d.ts b/packages/definitions-parser/test/fixtures/allows-references-to-old-versions-of-self/types/fail/index.d.ts index 8b46fb4136..7875eb9be6 100644 --- a/packages/definitions-parser/test/fixtures/allows-references-to-old-versions-of-self/types/fail/index.d.ts +++ b/packages/definitions-parser/test/fixtures/allows-references-to-old-versions-of-self/types/fail/index.d.ts @@ -1,6 +1 @@ -// Type definitions for fail 1.0 -// Project: https://youtube.com/typeref-fails -// Definitions by: Type Ref Fails -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - /// diff --git a/packages/definitions-parser/test/fixtures/allows-references-to-old-versions-of-self/types/fail/package.json b/packages/definitions-parser/test/fixtures/allows-references-to-old-versions-of-self/types/fail/package.json new file mode 100644 index 0000000000..3a0d34b384 --- /dev/null +++ b/packages/definitions-parser/test/fixtures/allows-references-to-old-versions-of-self/types/fail/package.json @@ -0,0 +1,17 @@ +{ + "private": true, + "name": "@types/fail", + "version": "1.0.9999", + "projects": [ + "https://youtube.com/typeref-fails" + ], + "devDependencies": { + "@types/fail": "workspace:." + }, + "owners": [ + { + "name": "Type Ref Fails", + "url": "https://youtube.com/typeref-fails" + } + ] +} diff --git a/packages/definitions-parser/test/fixtures/doesnt-omit-dependencies-if-only-some-deep-modules-are-declared/types/styled-components-react-native/index.d.ts b/packages/definitions-parser/test/fixtures/doesnt-omit-dependencies-if-only-some-deep-modules-are-declared/types/styled-components-react-native/index.d.ts index 702907fc25..f0a84516dd 100644 --- a/packages/definitions-parser/test/fixtures/doesnt-omit-dependencies-if-only-some-deep-modules-are-declared/types/styled-components-react-native/index.d.ts +++ b/packages/definitions-parser/test/fixtures/doesnt-omit-dependencies-if-only-some-deep-modules-are-declared/types/styled-components-react-native/index.d.ts @@ -1,8 +1,3 @@ -// Type definitions for styled-components-react-native 5.1 -// Project: https://github.com/styled-components/styled-components -// Definitions by: Nathan Bierema -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - declare module "styled-components/native" { import {} from "styled-components"; } diff --git a/packages/definitions-parser/test/fixtures/doesnt-omit-dependencies-if-only-some-deep-modules-are-declared/types/styled-components-react-native/package.json b/packages/definitions-parser/test/fixtures/doesnt-omit-dependencies-if-only-some-deep-modules-are-declared/types/styled-components-react-native/package.json new file mode 100644 index 0000000000..35b741ad03 --- /dev/null +++ b/packages/definitions-parser/test/fixtures/doesnt-omit-dependencies-if-only-some-deep-modules-are-declared/types/styled-components-react-native/package.json @@ -0,0 +1,20 @@ +{ + "private": true, + "name": "@types/styled-components-react-native", + "version": "5.1.9999", + "projects": [ + "https://github.com/styled-components/styled-components" + ], + "dependencies": { + "@types/styled-components": "*" + }, + "devDependencies": { + "@types/styled-components-react-native": "workspace:." + }, + "owners": [ + { + "name": "Nathan Bierema", + "githubUsername": "Methuselah96" + } + ] +} diff --git a/packages/definitions-parser/test/fixtures/rejects-references-to-old-versions-of-other-types-packages/types/typeref-fails/index.d.ts b/packages/definitions-parser/test/fixtures/rejects-references-to-old-versions-of-other-types-packages/types/typeref-fails/index.d.ts deleted file mode 100644 index af586356cc..0000000000 --- a/packages/definitions-parser/test/fixtures/rejects-references-to-old-versions-of-other-types-packages/types/typeref-fails/index.d.ts +++ /dev/null @@ -1,6 +0,0 @@ -// Type definitions for fail 1.0 -// Project: https://youtube.com/s-fails -// Definitions by: Type Ref Fails -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - -/// diff --git a/packages/definitions-parser/test/fixtures/rejects-references-to-old-versions-of-other-types-packages/types/typeref-fails/tsconfig.json b/packages/definitions-parser/test/fixtures/rejects-references-to-old-versions-of-other-types-packages/types/typeref-fails/tsconfig.json deleted file mode 100644 index ca03578a07..0000000000 --- a/packages/definitions-parser/test/fixtures/rejects-references-to-old-versions-of-other-types-packages/types/typeref-fails/tsconfig.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "compilerOptions": { - "module": "commonjs", - "lib": ["es6"], - "noImplicitAny": true, - "noImplicitThis": true, - "strictFunctionTypes": true, - "strictNullChecks": true, - "baseUrl": "../", - "typeRoots": ["../"], - "types": [], - "noEmit": true, - "forceConsistentCasingInFileNames": true - }, - "files": ["index.d.ts"] -} diff --git a/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referenced/index.d.ts b/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referenced/index.d.ts index f8660820a8..5ae60499ec 100644 --- a/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referenced/index.d.ts +++ b/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referenced/index.d.ts @@ -1,6 +1 @@ -// Type definitions for referenced 1.0 -// Project: https://youtube.com/s-fails -// Definitions by: Type Ref Fails -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - export interface Referenced {} diff --git a/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referenced/package.json b/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referenced/package.json new file mode 100644 index 0000000000..3ac83d1060 --- /dev/null +++ b/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referenced/package.json @@ -0,0 +1,17 @@ +{ + "private": true, + "name": "@types/referenced", + "version": "1.0.9999", + "projects": [ + "https://youtube.com/s-fails" + ], + "devDependencies": { + "@types/referenced": "workspace:." + }, + "owners": [ + { + "name": "Typescript Bot", + "githubUsername": "typescript-bot" + } + ] +} diff --git a/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referencing/index.d.ts b/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referencing/index.d.ts index f56cad3a74..44a0faca3d 100644 --- a/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referencing/index.d.ts +++ b/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referencing/index.d.ts @@ -1,8 +1,3 @@ -// Type definitions for referencing 1.0 -// Project: https://youtube.com/s-fails -// Definitions by: Type Ref Fails -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - import { Referenced } from "../referenced"; export interface Referencing extends Referenced {} diff --git a/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referencing/package.json b/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referencing/package.json new file mode 100644 index 0000000000..ac25de1ad6 --- /dev/null +++ b/packages/definitions-parser/test/fixtures/rejects-relative-references-to-other-packages/types/referencing/package.json @@ -0,0 +1,17 @@ +{ + "private": true, + "name": "@types/referencing", + "version": "1.0.9999", + "projects": [ + "https://youtube.com/s-fails" + ], + "devDependencies": { + "@types/referencing": "workspace:." + }, + "owners": [ + { + "name": "Typescript Bot", + "githubUsername": "typescript-bot" + } + ] +} diff --git a/packages/definitions-parser/test/get-affected-packages.test.ts b/packages/definitions-parser/test/get-affected-packages.test.ts index d6e7cb2de6..d104b3ae8a 100644 --- a/packages/definitions-parser/test/get-affected-packages.test.ts +++ b/packages/definitions-parser/test/get-affected-packages.test.ts @@ -1,52 +1,75 @@ -import { getAffectedPackages } from "../src/get-affected-packages"; +import { getAffectedPackagesWorker } from "../src/get-affected-packages"; import { NotNeededPackage, TypesDataFile, AllPackages } from "../src/packages"; import { testo, createTypingsVersionRaw } from "./utils"; const typesData: TypesDataFile = { - "has-older-test-dependency": createTypingsVersionRaw("has-older-test-dependency", {}, ["jquery"], { - jquery: { major: 1 }, - }), - jquery: createTypingsVersionRaw("jquery", {}, [], {}), - known: createTypingsVersionRaw("known", { jquery: { major: 1 } }, [], {}), - "known-test": createTypingsVersionRaw("known-test", {}, ["jquery"], {}), - "most-recent": createTypingsVersionRaw("most-recent", { jquery: "*" }, [], {}), - unknown: createTypingsVersionRaw("unknown", { "COMPLETELY-UNKNOWN": { major: 1 } }, [], {}), - "unknown-test": createTypingsVersionRaw("unknown-test", {}, ["WAT"], {}), + "has-older-test-dependency": createTypingsVersionRaw("has-older-test-dependency", {}, { "@types/jquery": "1.0.0" }), + jquery: createTypingsVersionRaw("jquery", {}, {}), + known: createTypingsVersionRaw("known", { "@types/jquery": "1.0.0" }, {}), + "known-test": createTypingsVersionRaw("known-test", {}, { "@types/jquery": "*" }), + "most-recent": createTypingsVersionRaw("most-recent", { "@types/jquery": "*" }, {}), + unknown: createTypingsVersionRaw("unknown", { "@types/COMPLETELY-UNKNOWN": "1.0.0" }, {}), + "unknown-test": createTypingsVersionRaw("unknown-test", {}, { "@types/WAT": "*" }), +}; +typesData.jquery["2.0"] = { + ...typesData.jquery["1.0"], + header: { ...typesData.jquery["1.0"].header, libraryMajorVersion: 2 }, }; -typesData.jquery["2.0"] = { ...typesData.jquery["1.0"], libraryMajorVersion: 2 }; const notNeeded = [new NotNeededPackage("jest", "jest", "100.0.0")]; const allPackages = AllPackages.from(typesData, notNeeded); testo({ updatedPackage() { - const { changedPackages, dependentPackages } = getAffectedPackages(allPackages, [ - { name: "jquery", version: { major: 2 } }, - ]); - expect(changedPackages.map(({ id }) => id)).toEqual([{ name: "jquery", version: { major: 2, minor: 0 } }]); - expect((changedPackages[0] as any).data).toEqual(typesData.jquery["2.0"]); - expect(dependentPackages.map(({ id }) => id)).toEqual([ - { name: "known-test", version: { major: 1, minor: 0 } }, - { name: "most-recent", version: { major: 1, minor: 0 } }, - ]); + const packageOutput = `/dt/types/jquery`; + const dependentOutput = `/dt/types/jquery +/dt/types/known-test +/dt/types/most-recent`; + const { packageNames, dependents } = getAffectedPackagesWorker( + allPackages, + packageOutput, + [dependentOutput], + "/dt" + ); + expect(packageNames).toEqual(new Set(["jquery"])); + expect(dependents).toEqual(new Set(["known-test", "most-recent"])); }, deletedPackage() { - const { changedPackages, dependentPackages } = getAffectedPackages(allPackages, [{ name: "WAT", version: "*" }]); - expect(changedPackages.map(({ id }) => id)).toEqual([]); - expect(dependentPackages.map(({ id }) => id)).toEqual([{ name: "unknown-test", version: { major: 1, minor: 0 } }]); + const packageOutput = ``; + const dependentOutput = `/dt/types/unknown-test`; + const { packageNames, dependents } = getAffectedPackagesWorker( + allPackages, + packageOutput, + [dependentOutput], + "/dt" + ); + expect(packageNames).toEqual(new Set([])); + expect(dependents).toEqual(new Set(["unknown-test"])); }, deletedVersion() { - const { changedPackages } = getAffectedPackages(allPackages, [{ name: "jquery", version: { major: 0 } }]); - expect(changedPackages).toEqual([]); + const packageOutput = `/dt/types/jquery`; + const dependentOutput = [ + `/dt/types/jquery +/dt/types/known-test +/dt/types/most-recent`, + `/dt/types/has-older-test-dependency +/dt/types/known`, + ]; + const { packageNames } = getAffectedPackagesWorker(allPackages, packageOutput, dependentOutput, "/dt"); + expect(packageNames).toEqual(new Set(["jquery"])); }, olderVersion() { - const { changedPackages, dependentPackages } = getAffectedPackages(allPackages, [ - { name: "jquery", version: { major: 1 } }, - ]); - expect(changedPackages.map(({ id }) => id)).toEqual([{ name: "jquery", version: { major: 1, minor: 0 } }]); - expect(dependentPackages.map(({ id }) => id)).toEqual([ - { name: "has-older-test-dependency", version: { major: 1, minor: 0 } }, - { name: "known", version: { major: 1, minor: 0 } }, - ]); + const packageOutput = `/dt/types/jquery`; + const dependentOutput = `/dt/types/jquery +/dt/types/has-older-test-dependency +/dt/types/known`; + const { packageNames, dependents } = getAffectedPackagesWorker( + allPackages, + packageOutput, + [dependentOutput], + "/dt" + ); + expect(packageNames).toEqual(new Set(["jquery"])); + expect(dependents).toEqual(new Set(["has-older-test-dependency", "known"])); }, }); diff --git a/packages/definitions-parser/test/git.test.ts b/packages/definitions-parser/test/git.test.ts index 2153e0cd2c..1584291c4f 100644 --- a/packages/definitions-parser/test/git.test.ts +++ b/packages/definitions-parser/test/git.test.ts @@ -5,12 +5,12 @@ import { GitDiff, getNotNeededPackages, checkNotNeededPackage } from "../src/git import { NotNeededPackage, TypesDataFile, AllPackages } from "../src/packages"; const typesData: TypesDataFile = { - jquery: createTypingsVersionRaw("jquery", {}, [], {}), - known: createTypingsVersionRaw("known", { jquery: { major: 1 } }, [], {}), - "known-test": createTypingsVersionRaw("known-test", {}, ["jquery"], {}), - "most-recent": createTypingsVersionRaw("most-recent", { jquery: "*" }, [], {}), - unknown: createTypingsVersionRaw("unknown", { "COMPLETELY-UNKNOWN": { major: 1 } }, [], {}), - "unknown-test": createTypingsVersionRaw("unknown-test", {}, ["WAT"], {}), + jquery: createTypingsVersionRaw("jquery", {}, {}), + known: createTypingsVersionRaw("known", { "@types/jquery": "1.0.0" }, {}), + "known-test": createTypingsVersionRaw("known-test", {}, { "@types/jquery": "*" }), + "most-recent": createTypingsVersionRaw("most-recent", { "@types/jquery": "*" }, {}), + unknown: createTypingsVersionRaw("unknown", { "@types/COMPLETELY-UNKNOWN": "1.0.0" }, {}), + "unknown-test": createTypingsVersionRaw("unknown-test", {}, { "@types/WAT": "*" }), }; const jestNotNeeded = [new NotNeededPackage("jest", "jest", "100.0.0")]; @@ -24,25 +24,28 @@ const deleteJestDiffs: GitDiff[] = [ testo({ ok() { - expect(getNotNeededPackages(allPackages, deleteJestDiffs)).toEqual(jestNotNeeded); + expect(getNotNeededPackages(allPackages, deleteJestDiffs)).toEqual({ ok: jestNotNeeded }); }, forgotToDeleteFiles() { - expect(() => + expect( getNotNeededPackages( - AllPackages.from({ jest: createTypingsVersionRaw("jest", {}, [], {}) }, jestNotNeeded), + AllPackages.from({ jest: createTypingsVersionRaw("jest", {}, {}) }, jestNotNeeded), deleteJestDiffs ) - ).toThrow("Please delete all files in jest"); + ).toEqual({ errors: ["Please delete all files in jest when adding it to notNeededPackages.json."] }); }, tooManyDeletes() { - expect(() => getNotNeededPackages(allPackages, [{ status: "D", file: "oops.txt" }])).toThrow( - "Unexpected file deleted: oops.txt" - ); + expect(getNotNeededPackages(allPackages, [{ status: "D", file: "oops.txt" }])).toEqual({ + errors: [ + `Unexpected file deleted: oops.txt +When removing packages, you should only delete files that are a part of removed packages.`, + ], + }); }, deleteInOtherPackage() { expect( getNotNeededPackages(allPackages, [...deleteJestDiffs, { status: "D", file: "types/most-recent/extra-tests.ts" }]) - ).toEqual(jestNotNeeded); + ).toEqual({ ok: jestNotNeeded }); }, extraneousFile() { expect( @@ -52,7 +55,7 @@ testo({ { status: "D", file: "types/jest/index.d.ts" }, { status: "D", file: "types/jest/jest-tests.d.ts" }, ]) - ).toEqual(jestNotNeeded); + ).toEqual({ ok: jestNotNeeded }); }, scoped() { expect( @@ -60,7 +63,7 @@ testo({ AllPackages.from(typesData, [new NotNeededPackage("ember__object", "@ember/object", "1.0.0")]), [{ status: "D", file: "types/ember__object/index.d.ts" }] ) - ).toEqual([new NotNeededPackage("ember__object", "@ember/object", "1.0.0")]); + ).toEqual({ ok: [new NotNeededPackage("ember__object", "@ember/object", "1.0.0")] }); }, // TODO: Test npm info (and with scoped names) // TODO: Test with dependents, etc etc @@ -96,30 +99,34 @@ const nonexistentReplacementPackage = new NotNeededPackage("jest", "nonexistent" const nonexistentTypesPackage = new NotNeededPackage("nonexistent", "jest", "100.0.0"); testo({ - missingSource() { - return expect(checkNotNeededPackage(nonexistentReplacementPackage)).rejects.toThrow( - "The entry for @types/jest in notNeededPackages.json" - ); + async missingSource() { + return expect(await checkNotNeededPackage(nonexistentReplacementPackage)).toEqual([ + `The entry for @types/jest in notNeededPackages.json has +"libraryName": "nonexistent", but there is no npm package with this name. +Unneeded packages have to be replaced with a package on npm.`, + ]); }, - missingTypings() { - return expect(checkNotNeededPackage(nonexistentTypesPackage)).rejects.toThrow( - "@types package not found for @types/nonexistent" - ); + async missingTypings() { + return expect(await checkNotNeededPackage(nonexistentTypesPackage)).toEqual([ + "Unexpected error: @types package not found for @types/nonexistent", + ]); }, - deprecatedSameVersion() { - return expect(checkNotNeededPackage(sameVersion)).rejects - .toThrow(`The specified version 50.0.0 of jest must be newer than the version -it is supposed to replace, 50.0.0 of @types/jest.`); + async deprecatedSameVersion() { + return expect(await checkNotNeededPackage(sameVersion)).toEqual([ + `The specified version 50.0.0 of jest must be newer than the version +it is supposed to replace, 50.0.0 of @types/jest.`, + ]); }, - deprecatedOlderVersion() { - return expect(checkNotNeededPackage(olderReplacement)).rejects - .toThrow(`The specified version 4.0.0 of jest must be newer than the version -it is supposed to replace, 50.0.0 of @types/jest.`); + async deprecatedOlderVersion() { + return expect(await checkNotNeededPackage(olderReplacement)).toEqual([ + `The specified version 4.0.0 of jest must be newer than the version +it is supposed to replace, 50.0.0 of @types/jest.`, + ]); }, - missingNpmVersion() { - return expect(checkNotNeededPackage(nonexistentReplacementVersion)).rejects.toThrow( - "The specified version 999.0.0 of jest is not on npm." - ); + async missingNpmVersion() { + return expect(await checkNotNeededPackage(nonexistentReplacementVersion)).toEqual([ + "The specified version 999.0.0 of jest is not on npm.", + ]); }, ok() { return checkNotNeededPackage(newerReplacement); diff --git a/packages/definitions-parser/test/module-info.test.ts b/packages/definitions-parser/test/module-info.test.ts index 1c19beeb00..3e9738e35a 100644 --- a/packages/definitions-parser/test/module-info.test.ts +++ b/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, getModuleInfo, getTestDependencies } from "../src/lib/module-info"; +import { allReferencedFiles } from "../src/lib/module-info"; const fs = createMockDT().fs; const moduleResolutionHost = createModuleResolutionHost(fs); @@ -92,35 +92,6 @@ testo({ expect(Array.from(types.keys())).toEqual(["index.d.ts", "types.d.ts"]); expect(Array.from(tests.keys())).toEqual([]); }, - getModuleInfoWorksWithOtherFiles() { - const { types } = getBoringReferences(); - // written as if it were from OTHER_FILES.txt - types.set( - "untested.d.ts", - ts.createSourceFile( - "untested.d.ts", - fs.subDir("types").subDir("boring").readFile("untested.d.ts"), - ts.ScriptTarget.Latest, - false - ) - ); - const i = getModuleInfo("boring", types); - expect(i.dependencies).toEqual( - new Set(["boring/quaternary", "boring/tertiary", "manual", "react", "react-default", "things", "vorticon"]) - ); - }, - getModuleInfoForNestedTypeReferences() { - const { types } = allReferencedFiles( - ["index.d.ts", "globby-tests.ts", "test/other-tests.ts"], - fs.subDir("types").subDir("globby"), - "globby", - moduleResolutionHost, - compilerOptions - ); - expect(Array.from(types.keys())).toEqual(["index.d.ts", "sneaky.d.ts"]); - const i = getModuleInfo("globby", types); - expect(i.dependencies).toEqual(new Set(["andere/snee"])); - }, selfInScopedPackage() { const dtMock = new DTMock(); const scoped = dtMock.pkgDir("rdfjs__to-ntriples"); @@ -167,17 +138,4 @@ testo({ expect(Array.from(types.keys())).toEqual(["index.d.ts", "../ts1.0/index.d.ts", "component.d.ts"]); expect(Array.from(tests.keys())).toEqual([]); }, - getTestDependenciesWorks() { - const { types, tests } = getBoringReferences(); - const i = getModuleInfo("boring", types); - const d = getTestDependencies( - "boring", - tests.keys(), - i.dependencies, - fs.subDir("types").subDir("boring"), - moduleResolutionHost, - compilerOptions - ); - expect(d).toEqual(new Set(["boring", "boring/commonjs", "boring/secondary", "boring/v1", "super-big-fun-hus"])); - }, }); diff --git a/packages/definitions-parser/test/packages.test.ts b/packages/definitions-parser/test/packages.test.ts index a807829ee6..d8f670758f 100644 --- a/packages/definitions-parser/test/packages.test.ts +++ b/packages/definitions-parser/test/packages.test.ts @@ -4,23 +4,23 @@ import { AllPackages, TypingsVersions, TypingsData, - License, getMangledNameForScopedPackage, NotNeededPackage, - getLicenseFromPackageJson, getDependencyFromFile, } from "../src/packages"; +import { Range } from "semver"; import { parseDefinitions } from "../src/parse-definitions"; import { quietLoggerWithErrors } from "@definitelytyped/utils"; import { createTypingsVersionRaw } from "./utils"; import { TypeScriptVersion } from "@definitelytyped/typescript-versions"; +import { License } from "@definitelytyped/header-parser"; describe(AllPackages, () => { let allPackages: AllPackages; beforeAll(async () => { const dt = createMockDT(); - dt.addOldVersionOfPackage("jquery", "1"); + dt.addOldVersionOfPackage("jquery", "1", "1.0.9999"); const [log] = quietLoggerWithErrors(); allPackages = await parseDefinitions(dt.fs, undefined, log); }); @@ -28,7 +28,7 @@ describe(AllPackages, () => { it("applies path mappings to test dependencies", () => { const pkg = allPackages.tryGetLatestVersion("has-older-test-dependency")!; expect(Array.from(allPackages.allDependencyTypings(pkg), ({ id }) => id)).toEqual([ - { name: "jquery", version: { major: 1, minor: 0 } }, + { typesDirectoryName: "jquery", version: { major: 1, minor: 0 } }, ]); }); @@ -44,13 +44,19 @@ describe(AllPackages, () => { it("returns true if typings exist", () => { expect( allPackages.hasTypingFor({ - name: "jquery", + name: "@types/jquery", version: "*", }) ).toBe(true); expect( allPackages.hasTypingFor({ - name: "nonExistent", + typesDirectoryName: "jquery", + version: "*", + }) + ).toBe(true); + expect( + allPackages.hasTypingFor({ + name: "@types/nonExistent", version: "*", }) ).toBe(false); @@ -63,10 +69,14 @@ describe(TypingsVersions, () => { beforeAll(async () => { const dt = createMockDT(); - dt.addOldVersionOfPackage("jquery", "1"); - dt.addOldVersionOfPackage("jquery", "2"); - dt.addOldVersionOfPackage("jquery", "2.5"); - versions = new TypingsVersions(await getTypingInfo("jquery", dt.fs)); + dt.addOldVersionOfPackage("jquery", "1", "1.0.9999"); + dt.addOldVersionOfPackage("jquery", "2", "2.0.9999"); + dt.addOldVersionOfPackage("jquery", "2.5", "2.5.9999"); + const info = await getTypingInfo("jquery", dt.fs); + if (Array.isArray(info)) { + throw new Error(info.join("\n")); + } + versions = new TypingsVersions(info); }); it("sorts the data from latest to oldest version", () => { @@ -78,27 +88,31 @@ describe(TypingsVersions, () => { }); it("finds the latest version when any version is wanted", () => { - expect(versions.get("*").major).toEqual(3); + expect(versions.get(new Range("*")).major).toEqual(3); }); it("finds the latest minor version for the given major version", () => { - expect(versions.get({ major: 2 }).major).toEqual(2); - expect(versions.get({ major: 2 }).minor).toEqual(5); + expect(versions.get(new Range("2")).major).toEqual(2); + expect(versions.get(new Range("2")).minor).toEqual(5); }); it("finds a specific version", () => { - expect(versions.get({ major: 2, minor: 0 }).major).toEqual(2); - expect(versions.get({ major: 2, minor: 0 }).minor).toEqual(0); + expect(versions.get(new Range("2.0")).major).toEqual(2); + expect(versions.get(new Range("2.0")).minor).toEqual(0); }); it("formats a version directory names", () => { - expect(versions.get({ major: 2, minor: 0 }).versionDirectoryName).toEqual("v2"); - expect(versions.get({ major: 2, minor: 0 }).subDirectoryPath).toEqual("jquery/v2"); + expect(versions.get(new Range("2.0")).versionDirectoryName).toEqual("v2"); + expect(versions.get(new Range("2.0")).subDirectoryPath).toEqual("jquery/v2"); }); it("formats missing version error nicely", () => { - expect(() => versions.get({ major: 111, minor: 1001 })).toThrow("Could not find version 111.1001"); - expect(() => versions.get({ major: 111 })).toThrow("Could not find version 111.*"); + expect(() => versions.get(new Range("111.1001"))).toThrow( + "Could not match version >=111.1001.0 <111.1002.0-0 in 3.3.9999,2.5.9999,2.0.9999,1.0.9999. " + ); + expect(() => versions.get(new Range("111"))).toThrow( + "Could not match version >=111.0.0 <112.0.0-0 in 3.3.9999,2.5.9999,2.0.9999,1.0.9999. " + ); }); }); @@ -111,20 +125,21 @@ describe(TypingsData, () => { { "dependency-1": "*", }, - [], - {} + { + "@types/known": "workspace:.", + } ); data = new TypingsData(versions["1.0"], true); }); it("sets the correct properties", () => { - expect(data.name).toBe("known"); - expect(data.testDependencies).toEqual([]); + expect(data.name).toBe("@types/known"); + expect(data.typesDirectoryName).toBe("known"); + expect(data.libraryName).toBe("known"); expect(data.contributors).toEqual([ { name: "Bender", url: "futurama.com", - githubUsername: "bender", }, ]); expect(data.major).toBe(1); @@ -133,17 +148,16 @@ describe(TypingsData, () => { expect(data.typesVersions).toEqual([]); expect(data.files).toEqual(["index.d.ts"]); expect(data.license).toBe(License.MIT); - expect(data.packageJsonDependencies).toEqual([]); expect(data.contentHash).toBe("11111111111111"); - expect(data.declaredModules).toEqual([]); expect(data.projectName).toBe("zombo.com"); - expect(data.globals).toEqual([]); - expect(data.pathMappings).toEqual({}); expect(data.dependencies).toEqual({ "dependency-1": "*", }); + expect(data.devDependencies).toEqual({ + "@types/known": "workspace:.", + }); expect(data.id).toEqual({ - name: "known", + typesDirectoryName: "known", version: { major: 1, minor: 0, @@ -152,42 +166,29 @@ describe(TypingsData, () => { expect(data.isNotNeeded()).toBe(false); }); - describe("unescapedName", () => { - it("returns the name when unscoped", () => { - expect(data.unescapedName).toBe("known"); - }); - - it("returns scoped names correctly", () => { - const versions = createTypingsVersionRaw("foo__bar", {}, [], {}); - data = new TypingsData(versions["1.0"], true); - - expect(data.unescapedName).toBe("@foo/bar"); - }); - }); - describe("desc", () => { it("returns the name if latest version", () => { - expect(data.desc).toBe("known"); + expect(data.desc).toBe("@types/known"); }); - it("returns the verioned name if not latest", () => { - const versions = createTypingsVersionRaw("known", {}, [], {}); + it("returns the versioned name if not latest", () => { + const versions = createTypingsVersionRaw("known", {}, {}); data = new TypingsData(versions["1.0"], false); - expect(data.desc).toBe("known v1.0"); + expect(data.desc).toBe("@types/known v1.0"); }); }); - describe("fullNpmName", () => { - it("returns scoped name", () => { - expect(data.fullNpmName).toBe("@types/known"); + describe("typesDirectoryName", () => { + it("returns unscoped name", () => { + expect(data.typesDirectoryName).toBe("known"); }); it("returns mangled name if scoped", () => { - const versions = createTypingsVersionRaw("@foo/bar", {}, [], {}); + const versions = createTypingsVersionRaw("@foo/bar", {}, {}); data = new TypingsData(versions["1.0"], false); - expect(data.fullNpmName).toBe("@types/foo__bar"); + expect(data.typesDirectoryName).toBe("foo__bar"); }); }); }); @@ -211,7 +212,7 @@ describe(NotNeededPackage, () => { it("sets the correct properties", () => { expect(data.license).toBe(License.MIT); - expect(data.name).toBe("types-package"); + expect(data.name).toBe("@types/types-package"); expect(data.libraryName).toBe("real-package"); expect(data.version).toMatchObject({ major: 1, @@ -222,7 +223,6 @@ describe(NotNeededPackage, () => { expect(data.minor).toBe(0); expect(data.isLatest).toBe(true); expect(data.isNotNeeded()).toBe(true); - expect(data.declaredModules).toEqual([]); expect(data.minTypeScriptVersion).toBe(TypeScriptVersion.lowest); expect(data.deprecatedMessage()).toBe( "This is a stub types definition. real-package provides its own type definitions, so you do not need this installed." @@ -243,24 +243,6 @@ describe(NotNeededPackage, () => { }); }); -describe(getLicenseFromPackageJson, () => { - it("returns MIT by default", () => { - expect(getLicenseFromPackageJson(undefined)).toBe(License.MIT); - }); - - it("throws if license is MIT", () => { - expect(() => getLicenseFromPackageJson("MIT")).toThrow(); - }); - - it("returns known licenses", () => { - expect(getLicenseFromPackageJson(License.Apache20)).toBe(License.Apache20); - }); - - it("throws if unknown license", () => { - expect(() => getLicenseFromPackageJson("nonsense")).toThrow(); - }); -}); - describe(getDependencyFromFile, () => { it("returns undefined for unversioned paths", () => { expect(getDependencyFromFile("types/a")).toBe(undefined); @@ -272,14 +254,14 @@ describe(getDependencyFromFile, () => { it("returns parsed version for versioned paths", () => { expect(getDependencyFromFile("types/a/v3.5")).toEqual({ - name: "a", + typesDirectoryName: "a", version: { major: 3, minor: 5, }, }); expect(getDependencyFromFile("types/a/v3")).toEqual({ - name: "a", + typesDirectoryName: "a", version: { major: 3, minor: undefined, @@ -289,7 +271,7 @@ describe(getDependencyFromFile, () => { it("returns undefined for unversioned subpaths", () => { expect(getDependencyFromFile("types/a/vnotaversion")).toEqual({ - name: "a", + typesDirectoryName: "a", version: "*", }); }); diff --git a/packages/definitions-parser/test/parse-definitions.test.ts b/packages/definitions-parser/test/parse-definitions.test.ts index 6322430ce6..2792e82a7b 100644 --- a/packages/definitions-parser/test/parse-definitions.test.ts +++ b/packages/definitions-parser/test/parse-definitions.test.ts @@ -23,8 +23,8 @@ testo({ expect(defs.allTypings().length).toBe(6); const j = defs.tryGetLatestVersion("jquery"); expect(j).toBeDefined(); - expect(j!.fullNpmName).toContain("types"); - expect(j!.fullNpmName).toContain("jquery"); + expect(j!.name).toContain("types"); + expect(j!.name).toContain("jquery"); expect(defs.allPackages().length).toEqual(defs.allTypings().length + defs.allNotNeeded().length); }, }); diff --git a/packages/definitions-parser/test/tsconfig.json b/packages/definitions-parser/test/tsconfig.json index ce99c61cfb..beb137e610 100644 --- a/packages/definitions-parser/test/tsconfig.json +++ b/packages/definitions-parser/test/tsconfig.json @@ -8,5 +8,6 @@ "references": [ { "path": ".." }, { "path": "../../utils" } - ] + ], + "exclude": ["fixtures"] } diff --git a/packages/definitions-parser/test/utils.ts b/packages/definitions-parser/test/utils.ts index a980e6546d..59806c785a 100644 --- a/packages/definitions-parser/test/utils.ts +++ b/packages/definitions-parser/test/utils.ts @@ -1,4 +1,6 @@ -import { TypingsVersionsRaw, License, DependencyVersion, DirectoryParsedTypingVersion } from "../src/packages"; +import { License } from "@definitelytyped/header-parser"; +import { scopeName } from "../src/lib/settings"; +import { TypingsVersionsRaw, getMangledNameForScopedPackage } from "../src/packages"; export function testo(o: { [s: string]: () => void }) { for (const k of Object.keys(o)) { @@ -7,30 +9,27 @@ export function testo(o: { [s: string]: () => void }) { } export function createTypingsVersionRaw( - name: string, - dependencies: { readonly [name: string]: DependencyVersion }, - testDependencies: string[], - pathMappings: { readonly [packageName: string]: DirectoryParsedTypingVersion } + libraryName: string, + dependencies: { readonly [name: string]: string }, + devDependencies: { readonly [name: string]: string } ): TypingsVersionsRaw { return { "1.0": { - libraryName: name, - typingsPackageName: name, - dependencies, - testDependencies, + header: { + name: `@${scopeName}/${getMangledNameForScopedPackage(libraryName)}`, + libraryMajorVersion: 1, + libraryMinorVersion: 0, + owners: [{ name: "Bender", url: "futurama.com" }], + minimumTypeScriptVersion: "2.3", + nonNpm: false, + projects: ["zombo.com"], + }, files: ["index.d.ts"], - libraryMajorVersion: 1, - libraryMinorVersion: 0, - pathMappings, - contributors: [{ name: "Bender", url: "futurama.com", githubUsername: "bender" }], - minTsVersion: "2.3", typesVersions: [], license: License.MIT, - packageJsonDependencies: [], + dependencies, + devDependencies, contentHash: "11111111111111", - projectName: "zombo.com", - globals: [], - declaredModules: [], }, }; } diff --git a/packages/dts-critic/README.md b/packages/dts-critic/README.md index a1065bfb98..60d7c98fdc 100644 --- a/packages/dts-critic/README.md +++ b/packages/dts-critic/README.md @@ -6,11 +6,13 @@ problems it has. # Usage Build the program: + ```sh $ npm run build ``` Run the program using node: + ```sh $ node dist/index.js --dts=path-to-d.ts [--js=path-to-source] [--mode=mode] [--debug] ``` @@ -25,43 +27,44 @@ check npm for a package with the same name as the d.ts. ## Mode You can run dts-critic in different modes that affect which checks will be performed: + 1. `name-only`: dts-critic will check your package name and [DefinitelyTyped header](../header-parser) (if present) against npm packages. -For example, if your declaration is for an npm package called 'cool-js-package', it will check if a -package named 'cool-js-package' actually exists in npm. + For example, if your declaration is for an npm package called 'cool-js-package', it will check if a + package named 'cool-js-package' actually exists in npm. 2. `code`: in addition to the checks performed in `name-only` mode, dts-critic will check if your -declaration exports match the source JavaScript module exports. -For example, if your declaration has a default export, it will check if the JavaScript module also -has a default export. + declaration exports match the source JavaScript module exports. + For example, if your declaration has a default export, it will check if the JavaScript module also + has a default export. # Current checks ## Npm declaration + If your declaration is for an npm package: 1. An npm package with the same name of your declaration's package must exist. -2. If your declaration has a [Definitely Typed header](../header-parser) -and the header specifies a target version, the npm package must have -a matching version. +2. If your package has a [Definitely Typed-conforming package.json](../header-parser), the npm package's version must match your version. 3. If you are running under `code` mode, your declaration must also match the source JavaScript module. ## Non-npm declaration + + If your declaration is for a non-npm package (in other words, if your declaration has a -[Definitely Typed header](../header-parser) *and* -the header specifies that the declaration file is for a non-npm package): +[Definitely Typed-conforming package.json](../header-parser) with `"nonNpm": true`): 1. An npm package with the same name of your declaration's package **cannot** exist. -3. If you are running under `code` mode *and* a path to the JavaScript source file was provided, your -declaration must also match the source JavaScript module. +2. If you are running under `code` mode _and_ a path to the JavaScript source file was provided, your + declaration must also match the source JavaScript module. # Planned work 1. Make sure your module structure fits the source. 2. Make sure your exported symbols match the source. 3. Make sure your types match the source types??? -6. Download source based on npm homepage (if it is github). +4. Download source based on npm homepage (if it is github). Note that for real use on Definitely Typed, a lot of these checks need to be pretty loose. diff --git a/packages/dts-critic/develop.ts b/packages/dts-critic/develop.ts index c009fc801d..4749eae510 100644 --- a/packages/dts-critic/develop.ts +++ b/packages/dts-critic/develop.ts @@ -110,16 +110,14 @@ function getNonNpm(args: { dtPath: string }): void { const dtTypesPath = getDtTypesPath(args.dtPath); const isNpmJson = getAllIsNpm(args.dtPath); for (const item of fs.readdirSync(dtTypesPath)) { - const entry = path.join(dtTypesPath, item); - const filePath = entry + "/index.d.ts"; - const dts = fs.readFileSync(filePath, "utf8"); - let header; - try { - header = headerParser.parseHeaderOrFail(filePath, dts); - } catch (e) { - header = undefined; - } - if (!isNpmPackage(item, header, isNpmJson)) { + const packageJsonPath = path.join(dtTypesPath, item, "package.json"); + const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, "utf8")); + const header = headerParser.validatePackageJson( + item, + packageJson, + headerParser.getTypesVersions(path.join(dtTypesPath, item)) + ); + if (!isNpmPackage(item, Array.isArray(header) ? undefined : header, isNpmJson)) { nonNpm.push(item); } } diff --git a/packages/dts-critic/index.test.ts b/packages/dts-critic/index.test.ts index 80b3bd087a..1de0567d79 100644 --- a/packages/dts-critic/index.test.ts +++ b/packages/dts-critic/index.test.ts @@ -252,23 +252,19 @@ suite("dtsCritic", { expect(dtsCritic(testsource("dts-critic.d.ts"), testsource("dts-critic.js"))).toEqual([]); }, noMatchingNpmPackage() { - expect(dtsCritic(testsource("wenceslas.d.ts"))).toEqual([ + expect(dtsCritic(testsource("no-matching-package/wenceslas.d.ts"))).toEqual([ { kind: ErrorKind.NoMatchingNpmPackage, message: `Declaration file must have a matching npm package. To resolve this error, either: 1. Change the name to match an npm package. -2. Add a Definitely Typed header with the first line - - -// Type definitions for non-npm package wenceslas-browser - -Add -browser to the end of your name to make sure it doesn't conflict with existing npm packages.`, +2. Add \`"nonNpm": true\` to the package.json to indicate that this is not an npm package. + Ensure the package name is descriptive enough to avoid conflicts with future npm packages.`, }, ]); }, noMatchingNpmVersion() { - expect(dtsCritic(testsource("typescript.d.ts"))).toEqual([ + expect(dtsCritic(testsource("typescript/index.d.ts"))).toEqual([ { kind: ErrorKind.NoMatchingNpmVersion, message: expect.stringContaining(`The types for 'typescript' must match a version that exists on npm. @@ -277,13 +273,13 @@ You should copy the major and minor version from the package on npm.`), ]); }, nonNpmHasMatchingPackage() { - expect(dtsCritic(testsource("tslib.d.ts"))).toEqual([ + expect(dtsCritic(testsource("example/index.d.ts"))).toEqual([ { kind: ErrorKind.NonNpmHasMatchingPackage, - message: `The non-npm package 'tslib' conflicts with the existing npm package 'tslib'. + message: `The non-npm package 'example' conflicts with the existing npm package 'example'. Try adding -browser to the end of the name to get - tslib-browser + example-browser `, }, ]); diff --git a/packages/dts-critic/index.ts b/packages/dts-critic/index.ts index 222ab1a7f9..50edaafd77 100644 --- a/packages/dts-critic/index.ts +++ b/packages/dts-critic/index.ts @@ -77,13 +77,13 @@ export function dtsCritic( ); } - const dts = fs.readFileSync(dtsPath, "utf-8"); - const header = parseDtHeader(dtsPath, dts); - const name = findDtsName(dtsPath); + const packageJsonPath = path.join(path.dirname(path.resolve(dtsPath)), "package.json"); const npmInfo = getNpmInfo(name); - - if (isNonNpm(header)) { + const header = parsePackageJson(name, JSON.parse(fs.readFileSync(packageJsonPath, "utf-8")), path.dirname(dtsPath)); + if (header === undefined) { + return []; + } else if (header.nonNpm) { const errors: CriticError[] = []; const nonNpmError = checkNonNpm(name, npmInfo); if (nonNpmError) { @@ -128,16 +128,14 @@ If you want to check the declaration against the JavaScript source code, you mus } } -function parseDtHeader(filePath: string, dts: string): headerParser.Header | undefined { - try { - return headerParser.parseHeaderOrFail(filePath, dts); - } catch (e) { - return undefined; - } -} - -function isNonNpm(header: headerParser.Header | undefined): boolean { - return !!header && header.nonNpm; +function parsePackageJson( + packageName: string, + packageJson: Record, + dirPath: string +): headerParser.Header | undefined { + const result = headerParser.validatePackageJson(packageName, packageJson, headerParser.getTypesVersions(dirPath)); + if (Array.isArray(result)) console.log(result.join("\n")); + return Array.isArray(result) ? undefined : result; } export const defaultErrors: ExportErrorKind[] = [ErrorKind.NeedsExportEquals, ErrorKind.NoDefaultExport]; @@ -233,34 +231,32 @@ Try adding -browser to the end of the name to get * Checks DefinitelyTyped npm package. * If all checks are successful, returns the npm version that matches the header. */ -function checkNpm(name: string, npmInfo: NpmInfo, header: headerParser.Header | undefined): NpmError | string { +function checkNpm(name: string, npmInfo: NpmInfo, header: headerParser.Header): NpmError | string { if (!npmInfo.isNpm) { return { kind: ErrorKind.NoMatchingNpmPackage, message: `Declaration file must have a matching npm package. To resolve this error, either: 1. Change the name to match an npm package. -2. Add a Definitely Typed header with the first line - - -// Type definitions for non-npm package ${name}-browser - -Add -browser to the end of your name to make sure it doesn't conflict with existing npm packages.`, +2. Add \`"nonNpm": true\` to the package.json to indicate that this is not an npm package. + Ensure the package name is descriptive enough to avoid conflicts with future npm packages.`, }; } - const target = getHeaderVersion(header); + const target = + header.libraryMajorVersion === 0 && header.libraryMinorVersion === 0 + ? undefined + : `${header.libraryMajorVersion}.${header.libraryMinorVersion}`; const npmVersion = getMatchingVersion(target, npmInfo); if (!npmVersion) { const versions = npmInfo.versions; const verstring = versions.join(", "); const lateststring = versions[versions.length - 1]; - const headerstring = target || "NO HEADER VERSION FOUND"; return { kind: ErrorKind.NoMatchingNpmVersion, message: `The types for '${name}' must match a version that exists on npm. You should copy the major and minor version from the package on npm. -To resolve this error, change the version in the header, ${headerstring}, +To resolve this error, change the version in the package.json, ${target}, to match one on npm: ${verstring}. For example, if you're trying to match the latest version, use ${lateststring}.`, @@ -269,16 +265,6 @@ For example, if you're trying to match the latest version, use ${lateststring}.` return npmVersion; } -function getHeaderVersion(header: headerParser.Header | undefined): string | undefined { - if (!header) { - return undefined; - } - if (header.libraryMajorVersion === 0 && header.libraryMinorVersion === 0) { - return undefined; - } - return `${header.libraryMajorVersion}.${header.libraryMinorVersion}`; -} - /** * Finds an npm version that matches the target version specified, if it exists. * If the target version is undefined, returns the latest version. diff --git a/packages/dts-critic/testsource/wenceslas.d.ts b/packages/dts-critic/testsource/example/index.d.ts similarity index 100% rename from packages/dts-critic/testsource/wenceslas.d.ts rename to packages/dts-critic/testsource/example/index.d.ts diff --git a/packages/dts-critic/testsource/example/package.json b/packages/dts-critic/testsource/example/package.json new file mode 100644 index 0000000000..474806e96e --- /dev/null +++ b/packages/dts-critic/testsource/example/package.json @@ -0,0 +1,19 @@ +{ + "private": true, + "name": "@types/example", + "nonNpm": true, + "nonNpmDescription": "example", + "version": "1.0.9999", + "projects": [ + "https://github.com/microsoft/TypeScript" + ], + "devDependencies": { + "@types/example": "workspace:." + }, + "owners": [ + { + "name": "Typescript Bot", + "githubUsername": "typescript-bot" + } + ] +} diff --git a/packages/dts-critic/testsource/no-matching-package/package.json b/packages/dts-critic/testsource/no-matching-package/package.json new file mode 100644 index 0000000000..cb1cf73cc6 --- /dev/null +++ b/packages/dts-critic/testsource/no-matching-package/package.json @@ -0,0 +1,17 @@ +{ + "private": true, + "name": "@types/wenceslas", + "version": "0.0.9999", + "projects": [ + "https://github.com/microsoft/TypeScript" + ], + "devDependencies": { + "@types/wenceslas": "workspace:." + }, + "owners": [ + { + "name": "Typescript Bot", + "githubUsername": "typescript-bot" + } + ] +} diff --git a/packages/dts-critic/testsource/no-matching-package/wenceslas.d.ts b/packages/dts-critic/testsource/no-matching-package/wenceslas.d.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/dts-critic/testsource/package.json b/packages/dts-critic/testsource/package.json new file mode 100644 index 0000000000..ec89226796 --- /dev/null +++ b/packages/dts-critic/testsource/package.json @@ -0,0 +1,17 @@ +{ + "private": true, + "name": "@types/dts-critic", + "version": "1.0.9999", + "projects": [ + "https://github.com/microsoft/TypeScript" + ], + "devDependencies": { + "@types/dts-critic": "workspace:." + }, + "owners": [ + { + "name": "Typescript Bot", + "githubUsername": "typescript-bot" + } + ] +} diff --git a/packages/dts-critic/testsource/tslib.d.ts b/packages/dts-critic/testsource/tslib.d.ts deleted file mode 100644 index 8cf6116a5b..0000000000 --- a/packages/dts-critic/testsource/tslib.d.ts +++ /dev/null @@ -1,4 +0,0 @@ -// Type definitions for non-npm package tslib -// Project: https://github.com/microsoft/TypeScript -// Definitions by: TypeScript Bot -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped diff --git a/packages/dts-critic/testsource/typescript.d.ts b/packages/dts-critic/testsource/typescript.d.ts deleted file mode 100644 index 2b9fe05584..0000000000 --- a/packages/dts-critic/testsource/typescript.d.ts +++ /dev/null @@ -1,4 +0,0 @@ -// Type definitions for typescript 1200000.5 -// Project: https://github.com/microsoft/TypeScript -// Definitions by: TypeScript Bot -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped diff --git a/packages/dts-critic/testsource/typescript/package.json b/packages/dts-critic/testsource/typescript/package.json new file mode 100644 index 0000000000..a0b75c295d --- /dev/null +++ b/packages/dts-critic/testsource/typescript/package.json @@ -0,0 +1,17 @@ +{ + "private": true, + "name": "@types/typescript", + "version": "1200000.5.9999", + "projects": [ + "https://github.com/microsoft/TypeScript" + ], + "devDependencies": { + "@types/typescript": "workspace:." + }, + "owners": [ + { + "name": "Typescript Bot", + "githubUsername": "typescript-bot" + } + ] +} diff --git a/packages/dts-critic/testsource/typescript/typescript.d.ts b/packages/dts-critic/testsource/typescript/typescript.d.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/dtslint-runner/package.json b/packages/dtslint-runner/package.json index 6d2884b641..e82dd0fba1 100644 --- a/packages/dtslint-runner/package.json +++ b/packages/dtslint-runner/package.json @@ -38,7 +38,7 @@ "@types/fs-extra": "^8.1.0", "@types/glob": "^8.1.0", "@types/pacote": "^11.1.5", - "@types/semver": "^7.3.10", + "@types/semver": "^7.5.2", "@types/stats-lite": "^2.2.0", "source-map-support": "^0.5.21" } diff --git a/packages/dtslint-runner/src/check-parse-results.ts b/packages/dtslint-runner/src/check-parse-results.ts index 0cfc4736ad..fdb45445da 100644 --- a/packages/dtslint-runner/src/check-parse-results.ts +++ b/packages/dtslint-runner/src/check-parse-results.ts @@ -1,217 +1,31 @@ import console from "console"; -import { - ParseDefinitionsOptions, - TypingsData, - AllPackages, - formatTypingVersion, - getDefinitelyTyped, -} from "@definitelytyped/definitions-parser"; -import { mapDefined, FS, logger, writeLog, Logger, ProgressBar, cacheDir, max, min } from "@definitelytyped/utils"; -import * as pacote from "pacote"; +import { AllPackages, getDefinitelyTyped } from "@definitelytyped/definitions-parser"; +import { assertDefined } from "@definitelytyped/utils"; import * as semver from "semver"; - if (require.main === module) { const options = { definitelyTypedPath: undefined, progress: false, parseInParallel: false }; getDefinitelyTyped(options, console).then((dt) => { - checkParseResults(/*includeNpmChecks*/ false, dt); + AllPackages.read(dt).then(checkParseResults); }); } -export async function checkParseResults(includeNpmChecks: false, dt: FS): Promise; -export async function checkParseResults( - includeNpmChecks: true, - dt: FS, - options: ParseDefinitionsOptions -): Promise; -export async function checkParseResults( - includeNpmChecks: boolean, - dt: FS, - options?: ParseDefinitionsOptions -): Promise { - const allPackages = await AllPackages.read(dt); - const [log, logResult] = logger(); - - checkTypeScriptVersions(allPackages); - - checkPathMappings(allPackages); - - const dependedOn = new Set(); - const packages = allPackages.allPackages(); - for (const pkg of packages) { - if (pkg instanceof TypingsData) { - for (const dep of Object.keys(pkg.dependencies)) { - dependedOn.add(dep); - } - for (const dep of pkg.testDependencies) { - dependedOn.add(dep); - } - } - } - - if (includeNpmChecks) { - const allTypings = allPackages.allTypings(); - const progress = options!.progress && new ProgressBar({ name: "Checking for typed packages..." }); - let i = 0; - await Promise.all( - allTypings.map((pkg) => - checkNpm(pkg, log, dependedOn).then(() => { - if (progress) progress.update(++i / allTypings.length, pkg.desc); - }) - ) - ); - if (progress) progress.done(); - } - - await writeLog("conflicts.md", logResult()); -} - -function checkTypeScriptVersions(allPackages: AllPackages): void { +export function checkParseResults(allPackages: AllPackages): string[] { + const errors = []; for (const pkg of allPackages.allTypings()) { for (const dep of allPackages.allDependencyTypings(pkg)) { - if (dep.minTypeScriptVersion > pkg.minTypeScriptVersion) { - throw new Error(`${pkg.desc} depends on ${dep.desc} but has a lower required TypeScript version.`); + // check raw version because parsed version doesn't currently retain range information + const version = assertDefined(new Map(pkg.allPackageJsonDependencies()).get(dep.name)); + if (semver.parse(version)) { + errors.push( + `${pkg.desc}'s dependency on ${dep.desc}@${version} must use "*" or a version range, not a specific version.` + ); } - } - } -} - -export function checkPathMappings(allPackages: AllPackages): void { - for (const pkg of allPackages.allTypings()) { - const unusedPathMappings = new Set( - Object.keys(pkg.pathMappings).filter((m) => m !== pkg.name && m !== pkg.unescapedName) - ); - - // If A depends on B, and B has path mappings, A must have the same mappings. - for (const dependency of allPackages.allDependencyTypings(pkg)) { - for (const [transitiveDependencyName, transitiveDependencyVersion] of Object.entries(dependency.pathMappings)) { - const pathMappingVersion = pkg.pathMappings[transitiveDependencyName]; - if ( - pathMappingVersion && - (pathMappingVersion.major !== transitiveDependencyVersion.major || - pathMappingVersion.minor !== transitiveDependencyVersion.minor) - ) { - const expectedPathMapping = `${transitiveDependencyName}/v${formatTypingVersion( - transitiveDependencyVersion - )}`; - throw new Error( - `${pkg.desc} depends on ${dependency.desc}, which has a path mapping for ${expectedPathMapping}. ` + - `${pkg.desc} must have the same path mappings as its dependencies.` - ); - } - unusedPathMappings.delete(transitiveDependencyName); + if (dep.minTypeScriptVersion > pkg.minTypeScriptVersion) { + errors.push( + `${pkg.desc} depends on ${dep.desc} but has a lower required TypeScript version (${pkg.minTypeScriptVersion} < ${dep.minTypeScriptVersion}).` + ); } - unusedPathMappings.delete(dependency.name); - } - if (unusedPathMappings.size > 0) { - throw new Error(`${pkg.desc} has unused path mappings for [${Array.from(unusedPathMappings).join(", ")}]. -If these mappings are actually used, they could be missing in a dependency's tsconfig.json instead. -Check the path mappings for [${Array.from(allPackages.allDependencyTypings(pkg)) - .map((d) => d.name) - .join(", ")}].`); } } + return errors; } - -async function checkNpm( - { major, minor, name, libraryName, projectName, contributors }: TypingsData, - log: Logger, - dependedOn: ReadonlySet -): Promise { - if (notNeededExceptions.has(name)) { - return; - } - - const info = await pacote.packument(name, { cache: cacheDir, fullMetadata: true }).catch((reason) => { - if (reason.code !== "E404") throw reason; - return undefined; - }); // Gets info for the real package, not the @types package - if (!info) { - return; - } - - const versions = getRegularVersions(info.versions); - const firstTypedVersion = min( - mapDefined(versions, ({ hasTypes, version }) => (hasTypes ? version : undefined)), - semver.compare - ); - // A package might have added types but removed them later, so check the latest version too - if (firstTypedVersion === undefined || !max(versions, (a, b) => semver.compare(a.version, b.version))!.hasTypes) { - return; - } - - const ourVersion = `${major}.${minor}`; - - log(""); - log(`Typings already defined for ${name} (${libraryName}) as of ${firstTypedVersion} (our version: ${ourVersion})`); - const contributorUrls = contributors - .map((c) => { - const gh = "https://github.com/"; - return c.url.startsWith(gh) ? `@${c.url.slice(gh.length)}` : `${c.name} (${c.url})`; - }) - .join(", "); - log(" To fix this:"); - log(` git checkout -b not-needed-${name}`); - const pnpmArgs = [name, firstTypedVersion, projectName]; - if (libraryName !== name) { - pnpmArgs.push(JSON.stringify(libraryName)); - } - log(" pnpm not-needed " + pnpmArgs.join(" ")); - log(` git add --all && git commit -m "${name}: Provides its own types" && git push -u origin not-needed-${name}`); - log(` And comment PR: This will deprecate \`@types/${name}\` in favor of just \`${name}\`. CC ${contributorUrls}`); - if (semver.gt(`${major}.${minor}.0`, firstTypedVersion)) { - log(" WARNING: our version is greater!"); - } - if (dependedOn.has(name)) { - log(" WARNING: other packages depend on this!"); - } -} - -function getRegularVersions( - versions: pacote.Packument["versions"] -): readonly { readonly version: semver.SemVer; readonly hasTypes: boolean }[] { - return Object.entries(versions).map(([versionString, info]) => ({ - version: new semver.SemVer(versionString), - hasTypes: versionHasTypes(info), - })); -} - -function versionHasTypes(info: pacote.Manifest): boolean { - return "types" in info || "typings" in info; -} - -const notNeededExceptions: ReadonlySet = new Set([ - // https://github.com/DefinitelyTyped/DefinitelyTyped/pull/22306 - "angular-ui-router", - "ui-router-extras", - // Declares to bundle types, but they're also in the `.npmignore` (https://github.com/nkovacic/angular-touchspin/issues/21) - "angular-touchspin", - // "typings" points to the wrong file (https://github.com/Microsoft/Bing-Maps-V8-TypeScript-Definitions/issues/31) - "bingmaps", - // Types are bundled, but not officially released (https://github.com/DefinitelyTyped/DefinitelyTyped/pull/22313#issuecomment-353225893) - "dwt", - // Waiting on some typing errors to be fixed (https://github.com/julien-c/epub/issues/30) - "epub", - // Typings file is not in package.json "files" list (https://github.com/silentmatt/expr-eval/issues/127) - "expr-eval", - // NPM package "express-serve-static-core" isn't a real package -- express-serve-static-core exists only for the purpose of types - "express-serve-static-core", - // Has "typings": "index.d.ts" but does not actually bundle typings. https://github.com/kolodny/immutability-helper/issues/79 - "immutability-helper", - // Has `"typings": "compiled/typings/node-mysql-wrapper/node-mysql-wrapper.d.ts",`, but `compiled/typings` doesn't exist. - // Package hasn't updated in 2 years and author seems to have deleted their account, so no chance of being fixed. - "node-mysql-wrapper", - // raspi packages bundle types, but can only be installed on a Raspberry Pi, so they are duplicated to DefinitelyTyped. - // See https://github.com/DefinitelyTyped/DefinitelyTyped/pull/21618 - "raspi", - "raspi-board", - "raspi-gpio", - "raspi-i2c", - "raspi-led", - "raspi-onewire", - "raspi-peripheral", - "raspi-pwm", - "raspi-serial", - "raspi-soft-pwm", - // Declare "typings" but don't actually have them yet (https://github.com/stampit-org/stampit/issues/245) - "stampit", -]); diff --git a/packages/dtslint-runner/src/main.ts b/packages/dtslint-runner/src/main.ts index ab8b65c7c8..9d10d26f24 100644 --- a/packages/dtslint-runner/src/main.ts +++ b/packages/dtslint-runner/src/main.ts @@ -48,8 +48,8 @@ export async function runDTSLint({ const typesPath = joinPaths(definitelyTypedPath, "types"); const { packageNames, dependents } = onlyRunAffectedPackages - ? await prepareAffectedPackages({ definitelyTypedPath, nProcesses, noInstall }) - : await prepareAllPackages({ definitelyTypedPath, nProcesses, noInstall }); + ? await prepareAffectedPackages(definitelyTypedPath, nProcesses) + : await prepareAllPackages(definitelyTypedPath, definitelyTypedAcquisition.kind === "clone", nProcesses); if (!noInstall && !localTypeScriptPath) { if (onlyTestTsNext) { @@ -74,8 +74,8 @@ export async function runDTSLint({ await runWithListeningChildProcesses({ inputs: testedPackages.map((path) => ({ path, - onlyTestTsNext: onlyTestTsNext || !packageNames.includes(path), - expectOnly: expectOnly || !packageNames.includes(path), + onlyTestTsNext: onlyTestTsNext || !packageNames.has(path), + expectOnly: expectOnly || !packageNames.has(path), })), commandLineArgs: dtslintArgs, workerFile: require.resolve("@definitelytyped/dtslint"), @@ -174,12 +174,12 @@ export async function runDTSLint({ return allFailures.length; } -function getExpectedFailures(onlyRunAffectedPackages: boolean, dependents: readonly string[]) { +function getExpectedFailures(onlyRunAffectedPackages: boolean, dependents: Set) { return new Set( (readFileSync(joinPaths(__dirname, "../expectedFailures.txt"), "utf8") as string) .split("\n") .map((s) => s.trim()) - .filter(onlyRunAffectedPackages ? (line) => line && dependents.includes(line) : Boolean) + .filter(onlyRunAffectedPackages ? (line) => line && dependents.has(line) : Boolean) ); } diff --git a/packages/dtslint-runner/src/prepareAffectedPackages.ts b/packages/dtslint-runner/src/prepareAffectedPackages.ts index ac0bfbb94c..66b7b9e588 100644 --- a/packages/dtslint-runner/src/prepareAffectedPackages.ts +++ b/packages/dtslint-runner/src/prepareAffectedPackages.ts @@ -1,21 +1,16 @@ -import { pathExists } from "fs-extra"; import { getDefinitelyTyped, parseDefinitions, getAffectedPackagesFromDiff, - allDependencies, - TypingsData, + PreparePackagesResult, } from "@definitelytyped/definitions-parser"; -import { execAndThrowErrors, joinPaths, loggerWithErrors, npmInstallFlags, sleep } from "@definitelytyped/utils"; +import { loggerWithErrors } from "@definitelytyped/utils"; import { checkParseResults } from "./check-parse-results"; -import { PreparePackagesOptions, PreparePackagesResult } from "./types"; -export async function prepareAffectedPackages({ - definitelyTypedPath, - noInstall, - nProcesses, -}: PreparePackagesOptions): Promise { - const typesPath = joinPaths(definitelyTypedPath, "types"); +export async function prepareAffectedPackages( + definitelyTypedPath: string, + nProcesses: number +): Promise { const log = loggerWithErrors()[0]; const options = { definitelyTypedPath, @@ -23,69 +18,16 @@ export async function prepareAffectedPackages({ parseInParallel: nProcesses > 1, }; const dt = await getDefinitelyTyped(options, log); - await parseDefinitions(dt, nProcesses ? { definitelyTypedPath, nProcesses } : undefined, log); - try { - await checkParseResults(/*includeNpmChecks*/ false, dt); - } catch (err) { - await getAffectedPackagesFromDiff(dt, definitelyTypedPath, "affected"); - throw err; + const allPackages = await parseDefinitions(dt, nProcesses ? { definitelyTypedPath, nProcesses } : undefined, log); + const errors = checkParseResults(allPackages); + const result = await getAffectedPackagesFromDiff(allPackages, definitelyTypedPath); + if (errors.length) { + throw new Error(errors.join("\n")); } - - const { changedPackages, dependentPackages, allPackages } = await getAffectedPackagesFromDiff( - dt, - definitelyTypedPath, - "affected" - ); - - if (!noInstall) { - await installDependencies(allDependencies(allPackages, [...changedPackages, ...dependentPackages]), typesPath); - } - - return { - packageNames: changedPackages.map((p) => p.subDirectoryPath), - dependents: dependentPackages.map((p) => p.subDirectoryPath), - }; -} - -const npmRetryCount = 5; - -export async function installDependencies(packages: Iterable, typesPath: string): Promise { - console.log("Installing NPM dependencies..."); - const start = Date.now(); - - // We need to run `npm install` for all dependencies, too, so that we have dependencies' dependencies installed. - for (const pkg of packages) { - const cwd = joinPaths(typesPath, pkg.subDirectoryPath); - if (!(await pathExists(joinPaths(cwd, "package.json")))) { - continue; - } - - // Scripts may try to compile native code. - // This doesn't work reliably on travis, and we're just installing for the types, so ignore. - const cmd = `npm install ${npmInstallFlags} --tag ts${pkg.minTypeScriptVersion}`; - console.log(` ${cwd}: ${cmd}`); - - let lastError; - for (let i = 0; i < npmRetryCount; i++) { - try { - const stdout = await execAndThrowErrors(cmd, cwd); - if (stdout) { - // Must specify what this is for since these run in parallel. - console.log(` from ${cwd}: ${stdout}`); - } - lastError = undefined; - break; - } catch (e) { - console.error(` from ${cwd} attempt ${i + 1}: ${e}`); - lastError = e; - await sleep(5); - } - } - - if (lastError) { - throw lastError; - } + if (Array.isArray(result)) { + // TODO: This error handling doesn't make sense but matches the old way. Try removing the previous if statement + // and changing this one to if (errors.length || Array.isArray(result)) { ... } + throw new Error([...errors, ...result].join("\n")); } - - console.log(`Took ${(Date.now() - start) / 1000} s`); + return result; } diff --git a/packages/dtslint-runner/src/prepareAllPackages.ts b/packages/dtslint-runner/src/prepareAllPackages.ts index b7dca9e232..a587ff7324 100644 --- a/packages/dtslint-runner/src/prepareAllPackages.ts +++ b/packages/dtslint-runner/src/prepareAllPackages.ts @@ -1,15 +1,12 @@ -import { AllPackages, getDefinitelyTyped, parseDefinitions } from "@definitelytyped/definitions-parser"; -import { joinPaths, loggerWithErrors } from "@definitelytyped/utils"; +import { getDefinitelyTyped, parseDefinitions, PreparePackagesResult } from "@definitelytyped/definitions-parser"; +import { execAndThrowErrors, loggerWithErrors, sleep } from "@definitelytyped/utils"; import { checkParseResults } from "./check-parse-results"; -import { installDependencies } from "./prepareAffectedPackages"; -import { PreparePackagesOptions, PreparePackagesResult } from "./types"; -export async function prepareAllPackages({ - definitelyTypedPath, - noInstall, - nProcesses, -}: PreparePackagesOptions): Promise { - const typesPath = joinPaths(definitelyTypedPath, "types"); +export async function prepareAllPackages( + definitelyTypedPath: string, + clone: boolean, + nProcesses: number +): Promise { const [log] = loggerWithErrors(); const options = { definitelyTypedPath, @@ -17,11 +14,38 @@ export async function prepareAllPackages({ parseInParallel: nProcesses > 1, }; const dt = await getDefinitelyTyped(options, log); - await parseDefinitions(dt, nProcesses ? { definitelyTypedPath, nProcesses } : undefined, log); - await checkParseResults(/*includeNpmChecks*/ false, dt); - const allPackages = await AllPackages.read(dt); - if (!noInstall) { - await installDependencies(allPackages.allTypings(), typesPath); + const allPackages = await parseDefinitions(dt, nProcesses ? { definitelyTypedPath, nProcesses } : undefined, log); + if (clone) { + await installAllDependencies(definitelyTypedPath); + } + const errors = checkParseResults(allPackages); + if (errors.length) { + throw new Error(errors.join("\n")); + } + return { + packageNames: new Set(allPackages.allTypings().map(({ subDirectoryPath }) => subDirectoryPath)), + dependents: new Set(), + }; +} +const npmRetryCount = 5; +export async function installAllDependencies(definitelyTypedPath: string): Promise { + console.log("Installing NPM dependencies..."); + const cmd = `pnpm install --no-save`; + console.log(` ${definitelyTypedPath}: ${cmd}`); + let lastError; + for (let i = 0; i < npmRetryCount; i++) { + try { + await execAndThrowErrors(cmd, definitelyTypedPath); + lastError = undefined; + break; + } catch (e) { + console.error(` from ${definitelyTypedPath} attempt ${i + 1}: ${e}`); + lastError = e; + await sleep(5); + } + } + + if (lastError) { + throw lastError; } - return { packageNames: allPackages.allTypings().map(({ subDirectoryPath }) => subDirectoryPath), dependents: [] }; } diff --git a/packages/dtslint-runner/src/types.ts b/packages/dtslint-runner/src/types.ts index 9b860b66aa..381b0d5f34 100644 --- a/packages/dtslint-runner/src/types.ts +++ b/packages/dtslint-runner/src/types.ts @@ -1,14 +1,3 @@ -export interface PreparePackagesOptions { - definitelyTypedPath: string; - nProcesses: number; - noInstall?: boolean; -} - -export interface PreparePackagesResult { - packageNames: readonly string[]; - dependents: readonly string[]; -} - export interface CloneDefinitelyTyped { kind: "clone"; sha?: string; diff --git a/packages/dtslint-runner/test/check-parse-results.test.ts b/packages/dtslint-runner/test/check-parse-results.test.ts deleted file mode 100644 index 28296d7939..0000000000 --- a/packages/dtslint-runner/test/check-parse-results.test.ts +++ /dev/null @@ -1,137 +0,0 @@ -import { checkPathMappings } from "../src/check-parse-results"; -import { AllPackages } from "@definitelytyped/definitions-parser"; - -// based on ember's structure, but corrected -const transitivePathMappingDependencies = AllPackages.from( - { - application: { - "1.0": { - typingsPackageName: "application", - dependencies: { engine: "*", object: "*", routing: "*" }, - testDependencies: [], - pathMappings: { - engine: { major: 1 }, - object: { major: 1 }, - routing: { major: 1 }, - controller: { major: 1 }, - service: { major: 1 }, - }, - }, - }, - controller: { - "1.0": { typingsPackageName: "controller", dependencies: {}, testDependencies: [], pathMappings: {} }, - }, - engine: { - "1.0": { typingsPackageName: "engine", dependencies: {}, testDependencies: [], pathMappings: {} }, - }, - error: { - "1.0": { typingsPackageName: "error", dependencies: {}, testDependencies: [], pathMappings: {} }, - }, - object: { - "1.0": { typingsPackageName: "object", dependencies: {}, testDependencies: [], pathMappings: {} }, - }, - routing: { - "1.0": { - typingsPackageName: "routing", - dependencies: { controller: "*", service: "*" }, - testDependencies: [], - pathMappings: { - controller: { major: 1 }, - service: { major: 1 }, - }, - }, - }, - service: { - "1.0": { typingsPackageName: "service", dependencies: {}, testDependencies: [], pathMappings: {} }, - }, - "test-helper": { - "1.0": { - typingsPackageName: "test-helper", - dependencies: { application: "*", error: "*" }, - testDependencies: [], - pathMappings: { - application: { major: 1 }, - engine: { major: 1 }, - object: { major: 1 }, - routing: { major: 1 }, - controller: { major: 1 }, - service: { major: 1 }, - error: { major: 1 }, - }, - }, - }, - } as never, - [] -); -// test-helper depends on application, which is missing transitive mappings for controller and service -const missingTransitivePathMappingDependencies = AllPackages.from( - { - application: { - "1.0": { - typingsPackageName: "application", - dependencies: { engine: "*", object: "*", routing: "*" }, - testDependencies: [], - pathMappings: { - engine: { major: 1 }, - object: { major: 1 }, - routing: { major: 1 }, - }, - }, - }, - controller: { - "1.0": { typingsPackageName: "controller", dependencies: {}, testDependencies: [], pathMappings: {} }, - }, - engine: { - "1.0": { typingsPackageName: "engine", dependencies: {}, testDependencies: [], pathMappings: {} }, - }, - error: { - "1.0": { typingsPackageName: "error", dependencies: {}, testDependencies: [], pathMappings: {} }, - }, - object: { - "1.0": { typingsPackageName: "object", dependencies: {}, testDependencies: [], pathMappings: {} }, - }, - routing: { - "1.0": { - typingsPackageName: "routing", - dependencies: { controller: "*", service: "*" }, - testDependencies: [], - pathMappings: { - controller: { major: 1 }, - service: { major: 1 }, - }, - }, - }, - service: { - "1.0": { typingsPackageName: "service", dependencies: {}, testDependencies: [], pathMappings: {} }, - }, - "test-helper": { - "1.0": { - typingsPackageName: "test-helper", - dependencies: { application: "*", error: "*" }, - testDependencies: [], - pathMappings: { - application: { major: 1 }, - engine: { major: 1 }, - object: { major: 1 }, - routing: { major: 1 }, - controller: { major: 1 }, - service: { major: 1 }, - error: { major: 1 }, - }, - }, - }, - } as never, - [] -); - -test("Transitive path mapping dependencies", () => { - expect(checkPathMappings(transitivePathMappingDependencies)).toBeUndefined(); -}); - -test("Missing transitive path mapping dependencies", () => { - expect(() => checkPathMappings(missingTransitivePathMappingDependencies)).toThrow( - `test-helper has unused path mappings for [controller, service]. -If these mappings are actually used, they could be missing in a dependency's tsconfig.json instead. -Check the path mappings for [application, error].` - ); -}); diff --git a/packages/dtslint/README.md b/packages/dtslint/README.md index eabdf511ca..8474d54147 100644 --- a/packages/dtslint/README.md +++ b/packages/dtslint/README.md @@ -5,7 +5,7 @@ Lint rules new to dtslint are documented in the [docs](docs) directory. # Just looking for ExpectType and ExpectError? -[Use tsd instead](https://github.com/SamVerschueren/tsd). +[Use tsd](https://github.com/SamVerschueren/tsd) or [eslint-plugin-expect-type](https://github.com/JoshuaKGoldberg/eslint-plugin-expect-type#readme) instead. # Setup @@ -28,8 +28,8 @@ Read more on bundling types [here](http://www.typescriptlang.org/docs/handbook/d #### `types/index.d.ts` -Only `index.d.ts` needs to be published to NPM. Other files are just for testing. -Write your type definitions here. +Only `index.d.ts` and `package.json` need to be published to NPM. The other files will be required by Definitely Typed for testing. +Write your type definitions in `index.d.ts`. Refer to the [handbook](http://www.typescriptlang.org/docs/handbook/declaration-files/introduction.html) or `dts-gen`'s templates for how to do this. @@ -39,30 +39,76 @@ Refer to the [handbook](http://www.typescriptlang.org/docs/handbook/declaration- { "compilerOptions": { "module": "commonjs", - "lib": ["es6"], + "lib": [ + "es6", + ], "noImplicitAny": true, "noImplicitThis": true, - "strictFunctionTypes": true, "strictNullChecks": true, + "strictFunctionTypes": true, "types": [], "noEmit": true, - "forceConsistentCasingInFileNames": true, - - // If the library is an external module (uses `export`), this allows your test file to import "mylib" instead of "./index". - // If the library is global (cannot be imported via `import` or `require`), leave this out. - "baseUrl": ".", - "paths": { "mylib": ["."] } - } + "forceConsistentCasingInFileNames": true + }, + "files": [ + "index.d.ts", + "PACKAGE-NAME-tests.ts", + ] } ``` You may extend `"lib"` to, for example, `["es6", "dom"]` if you need those typings. You may also have to add `"target": "es6"` if using certain language features. +#### `types/package.json` + +```json5 +{ + "private": true, + "name": "@types/PACKAGE-NAME", + "version": "1.2.9999", + "projects": [ + "https://example.com/" + ], + "dependencies": { + "@types/DEPENDENCY-1": "*", + "@types/DEPENDENCY-2": "*" + }, + "devDependencies": { + "@types/PACKAGE-NAME": "workspace:." + }, + "owners": [ + { + "name": "My Self", + "githubUsername": "ghost" + } + ] +} +``` + +1. If the types are for a scoped package, you must name-mangle `@scope/package` to `@types/scope__package`. +2. The major and minor versions should match some published version of the npm package. The patch version must be 9999; Definitely Typed will increment published patch versions starting at 0, and the patch version of the types will not match the patch version of the npm package. +3. `"projects"` is a link to the source project. +4. There might not be any dependencies if the *types* don't rely on anything but standard types. +5. `"devDependencies"` must include a self-reference like `"@types/PACKAGE-NAME": "workspace:.`. Plus any dependencies used only by tests. +6. Non-`@types` dependencies must be added to `allowedPackageJsonDependencies.txt` in the definitions-parser package. The PR must be approved by a Typescript team member. +7. If you do not have a github user name, you can provide a `"url"` of your own instead. + +Also: + +For types that do not have a matching NPM package, add two properties: -#### `types/tslint.json` +1. `"nonNpm": true` +2. `"nonNpmDescription"`, a human-readable name for the project that is being typed. -If you are using the default rules, this is optional. +#### `types/tslint.json` or `types/.eslintrc.json` + +Definitely Typed is in the process of migrating from tslint to eslint. +If you are using the default rules, .eslintrc.json is optional; tslint.json should be + +```json5 +{ "extends": "@definitelytyped/dtslint/dt.json" } +``` If present, this will override `dtslint`'s [default](https://github.com/microsoft/DefinitelyTyped-tools/blob/master/packages/dtslint/dtslint.json) settings. You can specify new lint [rules](https://palantir.github.io/tslint/rules/), or disable some. An example: @@ -77,13 +123,14 @@ You can specify new lint [rules](https://palantir.github.io/tslint/rules/), or d } ``` +Please don't do this without a good reason. +Disabling lint rules makes a Definitely Typed PR less likely to be merged, and will definitely take longer to review. + #### `types/test.ts` You can have any number of test files you want, with any names. See below on what to put in them. - - ## Write tests A test file should be a piece of sample code that tests using the library. Tests are type-checked, but not run. @@ -108,15 +155,13 @@ f("one"); ## Specify a TypeScript version Normally packages will be tested according to [DefinitelyType's support window](https://github.com/DefinitelyTyped/DefinitelyTyped#support-window). -To use a newer version, specify it by including a comment like so: +To restrict testing to new versions only, specify it in package.json: ```ts -// Minimum TypeScript Version: 2.1 +"minimumTypeScriptVersion: 5.0" ``` -For DefinitelyTyped packages, this should go just under the header (on line 5). -For bundled typings, this can go on any line (but should be near the top). - +This tests only 5.0 and above, although people can still depend on the package with lower versions of Typescript if they want. ## Run tests @@ -141,7 +186,6 @@ Disable all the lint rules except the one that checks for type correctness. dtslint --expectOnly types ``` - # Contributing ## Build @@ -153,13 +197,9 @@ npm run watch ## Test -Use `npm run test` to run all tests. -To run a single test: `node node_modules/tslint/bin/tslint --rules-dir dist/rules --test test/expect`. - -## Publish +Use `pnpm test` to run all tests. -1. Change the version in the `package.json` -2. Push to master +To run a single test: `node node_modules/tslint/bin/tslint --rules-dir dist/rules --test test/expect`. ## Code of Conduct diff --git a/packages/dtslint/package.json b/packages/dtslint/package.json index 8d343194d6..eff9d5b09c 100644 --- a/packages/dtslint/package.json +++ b/packages/dtslint/package.json @@ -32,6 +32,7 @@ "@typescript-eslint/typescript-estree": "^5.55.0", "@typescript-eslint/utils": "^5.55.0", "eslint": "^8.17.0", + "eslint-plugin-import": "^2.27.5", "fs-extra": "^6.0.1", "json-stable-stringify": "^1.0.1", "strip-json-comments": "^2.0.1", @@ -46,7 +47,7 @@ "@types/fs-extra": "^5.0.2", "@types/json-stable-stringify": "^1.0.32", "@types/strip-json-comments": "^0.0.28", - "typescript": "^5.0.2" + "typescript": "next" }, "engines": { "node": ">=10.0.0" diff --git a/packages/dtslint/src/checks.ts b/packages/dtslint/src/checks.ts index bf762a8796..cec25fa11a 100644 --- a/packages/dtslint/src/checks.ts +++ b/packages/dtslint/src/checks.ts @@ -1,61 +1,21 @@ -import { makeTypesVersionsForPackageJson } from "@definitelytyped/header-parser"; -import { TypeScriptVersion } from "@definitelytyped/typescript-versions"; -import assert = require("assert"); -import { pathExists } from "fs-extra"; +import * as header from "@definitelytyped/header-parser"; +import { AllTypeScriptVersion } from "@definitelytyped/typescript-versions"; +import { pathExistsSync } from "fs-extra"; import { join as joinPaths } from "path"; import { CompilerOptions } from "typescript"; +import { deepEquals } from "@definitelytyped/utils"; -import { readJson } from "./util"; - -export async function checkPackageJson(dirPath: string, typesVersions: readonly TypeScriptVersion[]): Promise { +import { readJson, packageNameFromPath } from "./util"; +export function checkPackageJson( + dirPath: string, + typesVersions: readonly AllTypeScriptVersion[] +): header.Header | string[] { const pkgJsonPath = joinPaths(dirPath, "package.json"); - const needsTypesVersions = typesVersions.length !== 0; - if (!(await pathExists(pkgJsonPath))) { - if (needsTypesVersions) { - throw new Error(`${dirPath}: Must have 'package.json' for "typesVersions"`); - } - return; - } - - const pkgJson = (await readJson(pkgJsonPath)) as Record; - - if ((pkgJson as any).private !== true) { - throw new Error(`${pkgJsonPath} should set \`"private": true\``); - } - - if (needsTypesVersions) { - assert.strictEqual((pkgJson as any).types, "index", `"types" in '${pkgJsonPath}' should be "index".`); - const expected = makeTypesVersionsForPackageJson(typesVersions) as Record; - assert.deepEqual( - (pkgJson as any).typesVersions, - expected, - `"typesVersions" in '${pkgJsonPath}' is not set right. Should be: ${JSON.stringify(expected, undefined, 4)}` - ); - } - - for (const key in pkgJson) { - // tslint:disable-line forin - switch (key) { - case "private": - case "dependencies": - case "license": - case "imports": - case "exports": - case "type": - // "private"/"typesVersions"/"types" checked above, "dependencies" / "license" checked by types-publisher, - break; - case "typesVersions": - case "types": - if (!needsTypesVersions) { - throw new Error(`${pkgJsonPath} doesn't need to set "${key}" when no 'ts3.x' directories exist.`); - } - break; - default: - throw new Error(`${pkgJsonPath} should not include field ${key}`); - } + if (!pathExistsSync(pkgJsonPath)) { + throw new Error(`${dirPath}: Missing 'package.json'`); } + return header.validatePackageJson(packageNameFromPath(dirPath), readJson(pkgJsonPath), typesVersions); } - /** * numbers in `CompilerOptions` might be enum values mapped from strings */ @@ -69,76 +29,69 @@ export interface DefinitelyTypedInfo { /** "../" or "../../" or "../../../". This should use '/' even on windows. */ readonly relativeBaseUrl: string; } -export function checkTsconfig(options: CompilerOptionsRaw, dt: DefinitelyTypedInfo | undefined): void { - if (dt) { - const { relativeBaseUrl } = dt; - - const mustHave = { - noEmit: true, - forceConsistentCasingInFileNames: true, - baseUrl: relativeBaseUrl, - typeRoots: [relativeBaseUrl], - types: [], - }; - - for (const key of Object.getOwnPropertyNames(mustHave) as (keyof typeof mustHave)[]) { - const expected = mustHave[key]; - const actual = options[key]; - if (!deepEquals(expected, actual)) { - throw new Error( - `Expected compilerOptions[${JSON.stringify(key)}] === ${JSON.stringify(expected)}, but got ${JSON.stringify( - actual - )}` - ); - } +export function checkTsconfig(dirPath: string, options: CompilerOptionsRaw): string[] { + const errors = []; + const mustHave = { + noEmit: true, + forceConsistentCasingInFileNames: true, + types: [], + }; + + for (const key of Object.getOwnPropertyNames(mustHave) as (keyof typeof mustHave)[]) { + const expected = mustHave[key]; + const actual = options[key]; + if (!deepEquals(expected, actual)) { + errors.push( + `Expected compilerOptions[${JSON.stringify(key)}] === ${JSON.stringify(expected)}, but got ${JSON.stringify( + actual + )}` + ); } + } - for (const key in options) { - switch (key) { - case "lib": - case "noImplicitAny": - case "noImplicitThis": - case "strict": - case "strictNullChecks": - case "noUncheckedIndexedAccess": - case "strictFunctionTypes": - case "esModuleInterop": - case "allowSyntheticDefaultImports": - case "paths": - case "target": - case "jsx": - case "jsxFactory": - case "experimentalDecorators": - case "noUnusedLocals": - case "noUnusedParameters": - case "exactOptionalPropertyTypes": - case "module": - break; - default: - if (!(key in mustHave)) { - throw new Error(`Unexpected compiler option ${key}`); - } - } + for (const key in options) { + switch (key) { + case "lib": + case "noImplicitAny": + case "noImplicitThis": + case "strict": + case "strictNullChecks": + case "noUncheckedIndexedAccess": + case "strictFunctionTypes": + case "esModuleInterop": + case "allowSyntheticDefaultImports": + case "target": + case "jsx": + case "jsxFactory": + case "experimentalDecorators": + case "noUnusedLocals": + case "noUnusedParameters": + case "exactOptionalPropertyTypes": + case "module": + case "paths": + break; + default: + if (!(key in mustHave)) { + errors.push(`Unexpected compiler option ${key}`); + } } } - if (!("lib" in options)) { - throw new Error('Must specify "lib", usually to `"lib": ["es6"]` or `"lib": ["es6", "dom"]`.'); + errors.push('Must specify "lib", usually to `"lib": ["es6"]` or `"lib": ["es6", "dom"]`.'); } if (!("module" in options)) { - throw new Error('Must specify "module" to `"module": "commonjs"` or `"module": "node16"`.'); - } - if ( + errors.push('Must specify "module" to `"module": "commonjs"` or `"module": "node16"`.'); + } else if ( options.module?.toString().toLowerCase() !== "commonjs" && options.module?.toString().toLowerCase() !== "node16" ) { - throw new Error(`When "module" is present, it must be set to "commonjs" or "node16".`); + errors.push(`When "module" is present, it must be set to "commonjs" or "node16".`); } if ("strict" in options) { if (options.strict !== true) { - throw new Error('When "strict" is present, it must be set to `true`.'); + errors.push('When "strict" is present, it must be set to `true`.'); } for (const key of ["noImplicitAny", "noImplicitThis", "strictNullChecks", "strictFunctionTypes"]) { @@ -149,30 +102,35 @@ export function checkTsconfig(options: CompilerOptionsRaw, dt: DefinitelyTypedIn } else { for (const key of ["noImplicitAny", "noImplicitThis", "strictNullChecks", "strictFunctionTypes"]) { if (!(key in options)) { - throw new Error(`Expected \`"${key}": true\` or \`"${key}": false\`.`); + errors.push(`Expected \`"${key}": true\` or \`"${key}": false\`.`); } } } if ("exactOptionalPropertyTypes" in options) { if (options.exactOptionalPropertyTypes !== true) { - throw new Error('When "exactOptionalPropertyTypes" is present, it must be set to `true`.'); + errors.push('When "exactOptionalPropertyTypes" is present, it must be set to `true`.'); } } if (options.types && options.types.length) { - throw new Error( + errors.push( 'Use `/// ` directives in source files and ensure ' + 'that the "types" field in your tsconfig is an empty array.' ); } -} - -function deepEquals(expected: unknown, actual: unknown): boolean { - if (expected instanceof Array) { - return ( - actual instanceof Array && actual.length === expected.length && expected.every((e, i) => deepEquals(e, actual[i])) - ); - } else { - return expected === actual; + if (options.paths) { + for (const key in options.paths) { + if (options.paths[key].length !== 1) { + errors.push(`${dirPath}/tsconfig.json: "paths" must map each module specifier to only one file.`); + } + const [target] = options.paths[key]; + if (target !== "./index.d.ts") { + const m = target.match(/^(?:..\/)+([^\/]+)\/(?:v\d+\.?\d*\/)?index.d.ts$/); + if (!m || m[1] !== key) { + errors.push(`${dirPath}/tsconfig.json: "paths" must map '${key}' to ${key}'s index.d.ts.`); + } + } + } } + return errors; } diff --git a/packages/dtslint/src/index.ts b/packages/dtslint/src/index.ts index 0f51f7d378..479aabf1ec 100644 --- a/packages/dtslint/src/index.ts +++ b/packages/dtslint/src/index.ts @@ -1,15 +1,15 @@ #!/usr/bin/env node -import { parseTypeScriptVersionLine } from "@definitelytyped/header-parser"; import { AllTypeScriptVersion, TypeScriptVersion } from "@definitelytyped/typescript-versions"; import assert = require("assert"); -import { readdir, readFile, stat, existsSync } from "fs-extra"; +import { readFile, existsSync } from "fs-extra"; import { basename, dirname, join as joinPaths, resolve } from "path"; import { cleanTypeScriptInstalls, installAllTypeScriptVersions, installTypeScriptNext } from "@definitelytyped/utils"; import { checkPackageJson, checkTsconfig } from "./checks"; import { checkTslintJson, lint, TsVersion } from "./lint"; -import { getCompilerOptions, mapDefinedAsync, withoutPrefix } from "./util"; +import { getCompilerOptions, packageNameFromPath } from "./util"; +import { getTypesVersions } from "@definitelytyped/header-parser"; async function main(): Promise { const args = process.argv.slice(2); @@ -130,50 +130,23 @@ async function runTests( expectOnly: boolean, tsLocal: string | undefined ): Promise { - const isOlderVersion = /^v(0\.)?\d+$/.test(basename(dirPath)); + // Assert that we're really on DefinitelyTyped. + const dtRoot = findDTRoot(dirPath); + const packageName = packageNameFromPath(dirPath); + assertPathIsInDefinitelyTyped(dirPath, dtRoot); + assertPathIsNotBanned(packageName); + assertPackageIsNotDeprecated(packageName, await readFile(joinPaths(dtRoot, "notNeededPackages.json"), "utf-8")); - const indexText = await readFile(joinPaths(dirPath, "index.d.ts"), "utf-8"); - // If this *is* on DefinitelyTyped, types-publisher will fail if it can't parse the header. - const dt = indexText.includes("// Type definitions for"); - if (dt) { - // Someone may have copied text from DefinitelyTyped to their type definition and included a header, - // so assert that we're really on DefinitelyTyped. - const dtRoot = findDTRoot(dirPath); - const packageName = basename(dirPath); - assertPathIsInDefinitelyTyped(dirPath, dtRoot); - assertPathIsNotBanned(packageName); - assertPackageIsNotDeprecated(packageName, await readFile(joinPaths(dtRoot, "notNeededPackages.json"), "utf-8")); + const typesVersions = getTypesVersions(dirPath); + const packageJson = checkPackageJson(dirPath, typesVersions); + if (Array.isArray(packageJson)) { + throw new Error("\n\t* " + packageJson.join("\n\t* ")); } - const typesVersions = await mapDefinedAsync(await readdir(dirPath), async (name) => { - if (name === "tsconfig.json" || name === "tslint.json" || name === "tsutils") { - return undefined; - } - const version = withoutPrefix(name, "ts"); - if (version === undefined || !(await stat(joinPaths(dirPath, name))).isDirectory()) { - return undefined; - } - - if (!TypeScriptVersion.isTypeScriptVersion(version)) { - throw new Error(`There is an entry named ${name}, but ${version} is not a valid TypeScript version.`); - } - if (!TypeScriptVersion.isRedirectable(version)) { - throw new Error(`At ${dirPath}/${name}: TypeScript version directories only available starting with ts3.1.`); - } - return version; - }); - - if (dt) { - await checkPackageJson(dirPath, typesVersions); - } - - const minVersion = maxVersion( - getMinimumTypeScriptVersionFromComment(indexText), - TypeScriptVersion.lowest - ) as TypeScriptVersion; + const minVersion = maxVersion(packageJson.minimumTypeScriptVersion, TypeScriptVersion.lowest); if (onlyTestTsNext || tsLocal) { const tsVersion = tsLocal ? "local" : TypeScriptVersion.latest; - await testTypesVersion(dirPath, tsVersion, tsVersion, isOlderVersion, dt, expectOnly, tsLocal, /*isLatest*/ true); + await testTypesVersion(dirPath, tsVersion, tsVersion, expectOnly, tsLocal, /*isLatest*/ true); } else { // For example, typesVersions of [3.2, 3.5, 3.6] will have // associated ts3.2, ts3.5, ts3.6 directories, for @@ -194,18 +167,14 @@ async function runTests( if (lows.length > 1) { console.log("testing from", low, "to", hi, "in", versionPath); } - await testTypesVersion(versionPath, low, hi, isOlderVersion, dt, expectOnly, undefined, isLatest); + await testTypesVersion(versionPath, low, hi, expectOnly, undefined, isLatest); } } } -function maxVersion(v1: TypeScriptVersion | undefined, v2: TypeScriptVersion): TypeScriptVersion; -function maxVersion(v1: AllTypeScriptVersion | undefined, v2: AllTypeScriptVersion): AllTypeScriptVersion; -function maxVersion(v1: AllTypeScriptVersion | undefined, v2: AllTypeScriptVersion) { - if (!v1) return v2; - if (!v2) return v1; - if (parseFloat(v1) >= parseFloat(v2)) return v1; - return v2; +function maxVersion(v1: AllTypeScriptVersion, v2: TypeScriptVersion): TypeScriptVersion { + // Note: For v1 to be later than v2, it must be a current Typescript version. So the type assertion is safe. + return parseFloat(v1) >= parseFloat(v2) ? (v1 as TypeScriptVersion) : v2; } function next(v: TypeScriptVersion): TypeScriptVersion { @@ -219,17 +188,15 @@ async function testTypesVersion( dirPath: string, lowVersion: TsVersion, hiVersion: TsVersion, - isOlderVersion: boolean, - dt: boolean, expectOnly: boolean, tsLocal: string | undefined, isLatest: boolean ): Promise { - await checkTslintJson(dirPath, dt); - checkTsconfig( - await getCompilerOptions(dirPath), - dt ? { relativeBaseUrl: ".." + (isOlderVersion ? "/.." : "") + (isLatest ? "" : "/..") + "/" } : undefined - ); + checkTslintJson(dirPath); + const tsconfigErrors = checkTsconfig(dirPath, getCompilerOptions(dirPath)); + if (tsconfigErrors.length > 0) { + throw new Error("\n\t* " + tsconfigErrors.join("\n\t* ")); + } const err = await lint(dirPath, lowVersion, hiVersion, isLatest, expectOnly, tsLocal); if (err) { throw new Error(err); @@ -288,19 +255,6 @@ If you want to re-add @types/${packageName}, please remove its entry from notNee } } -function getMinimumTypeScriptVersionFromComment(text: string): AllTypeScriptVersion | undefined { - const match = text.match(/\/\/ (?:Minimum )?TypeScript Version: /); - if (!match) { - return undefined; - } - - let line = text.slice(match.index, text.indexOf("\n", match.index)); - if (line.endsWith("\r")) { - line = line.slice(0, line.length - 1); - } - return parseTypeScriptVersionLine(line); -} - if (require.main === module) { main().catch((err) => { console.error(err.stack); diff --git a/packages/dtslint/src/lint.ts b/packages/dtslint/src/lint.ts index 2a35d03ff0..c5b1054386 100644 --- a/packages/dtslint/src/lint.ts +++ b/packages/dtslint/src/lint.ts @@ -1,8 +1,8 @@ import { TypeScriptVersion } from "@definitelytyped/typescript-versions"; -import { typeScriptPath } from "@definitelytyped/utils"; +import { typeScriptPath, withoutStart } from "@definitelytyped/utils"; import assert = require("assert"); -import { pathExists } from "fs-extra"; -import { dirname, join as joinPaths, normalize } from "path"; +import { pathExistsSync } from "fs-extra"; +import { join as joinPaths, normalize } from "path"; import { Configuration, Linter } from "tslint"; import { ESLint } from "eslint"; import * as TsType from "typescript"; @@ -10,7 +10,7 @@ type Configuration = typeof Configuration; type IConfigurationFile = Configuration.IConfigurationFile; import { getProgram, Options as ExpectOptions } from "./rules/expectRule"; -import { readJson, withoutPrefix } from "./util"; +import { readJson } from "./util"; export async function lint( dirPath: string, @@ -37,7 +37,7 @@ export async function lint( const configPath = expectOnly ? joinPaths(__dirname, "..", "dtslint-expect-only.json") : getConfigPath(dirPath); // TODO: To port expect-rule, eslint's config will also need to include [minVersion, maxVersion] // Also: expect-rule should be renamed to expect-type or check-type or something - const config = await getLintConfig(configPath, tsconfigPath, minVersion, maxVersion, tsLocal); + const config = getLintConfig(configPath, tsconfigPath, minVersion, maxVersion, tsLocal); const esfiles = []; for (const file of lintProgram.getSourceFiles()) { @@ -108,15 +108,12 @@ function testDependencies( (d) => d.code === 2307 && d.messageText.toString().includes("Cannot find module") ); if (cannotFindDepsDiags && cannotFindDepsDiags.file) { - const path = cannotFindDepsDiags.file.fileName; - const typesFolder = dirname(path); - return ` -A module look-up failed, this often occurs when you need to run \`npm install\` on a dependent module before you can lint. +A module look-up failed, this often occurs when you need to run \`pnpm install\` on a dependent module before you can lint. Before you debug, first try running: - npm install --prefix ${typesFolder} + pnpm install -w --filter '...{./types/${dirPath}}...' Then re-run. Full error logs are below. @@ -140,7 +137,7 @@ function normalizePath(file: string) { function isTypesVersionPath(fileName: string, dirPath: string) { const normalFileName = normalizePath(fileName); const normalDirPath = normalizePath(dirPath); - const subdirPath = withoutPrefix(normalFileName, normalDirPath); + const subdirPath = withoutStart(normalFileName, normalDirPath); return subdirPath && /^\/ts\d+\.\d/.test(subdirPath); } @@ -179,25 +176,14 @@ function testNoLintDisables(disabler: "tslint:disable" | "eslint-disable", text: } } -export async function checkTslintJson(dirPath: string, dt: boolean): Promise { +export function checkTslintJson(dirPath: string): void { const configPath = getConfigPath(dirPath); - const shouldExtend = `@definitelytyped/dtslint/${dt ? "dt" : "dtslint"}.json`; - const validateExtends = (extend: string | string[]) => - extend === shouldExtend || (!dt && Array.isArray(extend) && extend.some((val) => val === shouldExtend)); - - if (!(await pathExists(configPath))) { - if (dt) { - throw new Error( - `On DefinitelyTyped, must include \`tslint.json\` containing \`{ "extends": "${shouldExtend}" }\`.\n` + - "This was inferred as a DefinitelyTyped package because it contains a `// Type definitions for` header." - ); - } - return; + const shouldExtend = "@definitelytyped/dtslint/dt.json"; + if (!pathExistsSync(configPath)) { + throw new Error(`Missing \`tslint.json\` that contains \`{ "extends": "${shouldExtend}" }\`.`); } - - const tslintJson = await readJson(configPath); - if (!validateExtends(tslintJson.extends)) { - throw new Error(`If 'tslint.json' is present, it should extend "${shouldExtend}"`); + if (readJson(configPath).extends !== shouldExtend) { + throw new Error(`'tslint.json' must extend "${shouldExtend}"`); } } @@ -205,14 +191,14 @@ function getConfigPath(dirPath: string): string { return joinPaths(dirPath, "tslint.json"); } -async function getLintConfig( +function getLintConfig( expectedConfigPath: string, tsconfigPath: string, minVersion: TsVersion, maxVersion: TsVersion, tsLocal: string | undefined -): Promise { - const configExists = await pathExists(expectedConfigPath); +): IConfigurationFile { + const configExists = pathExistsSync(expectedConfigPath); const configPath = configExists ? expectedConfigPath : joinPaths(__dirname, "..", "dtslint.json"); // Second param to `findConfiguration` doesn't matter, since config path is provided. const config = Configuration.findConfiguration(configPath, "").results; diff --git a/packages/dtslint/src/rules/npmNamingRule.ts b/packages/dtslint/src/rules/npmNamingRule.ts index 1052939a92..c3cb8d8075 100644 --- a/packages/dtslint/src/rules/npmNamingRule.ts +++ b/packages/dtslint/src/rules/npmNamingRule.ts @@ -172,13 +172,6 @@ function toCriticOptions(options: ConfigOptions): Options { function walk(ctx: Lint.WalkContext): void { const { sourceFile } = ctx; - const { text } = sourceFile; - const lookFor = (search: string, explanation: string) => { - const idx = text.indexOf(search); - if (idx !== -1) { - ctx.addFailureAt(idx, search.length, failure(Rule.metadata.ruleName, explanation)); - } - }; if (isMainFile(sourceFile.fileName, /*allowNested*/ false)) { try { const optionsWithSuggestions = toOptionsWithSuggestions(ctx.options); @@ -189,8 +182,6 @@ function walk(ctx: Lint.WalkContext): void { case ErrorKind.NoMatchingNpmPackage: case ErrorKind.NoMatchingNpmVersion: case ErrorKind.NonNpmHasMatchingPackage: - lookFor("// Type definitions for", errorMessage(error, ctx.options)); - break; case ErrorKind.DtsPropertyNotInJs: case ErrorKind.DtsSignatureNotInJs: case ErrorKind.JsPropertyNotInDts: diff --git a/packages/dtslint/src/util.ts b/packages/dtslint/src/util.ts index a7f4a20a7d..bb3e9b3729 100644 --- a/packages/dtslint/src/util.ts +++ b/packages/dtslint/src/util.ts @@ -1,11 +1,15 @@ import assert = require("assert"); -import { pathExists, readFile } from "fs-extra"; +import { pathExistsSync, readFileSync } from "fs-extra"; import { basename, dirname, join } from "path"; import stripJsonComments = require("strip-json-comments"); import * as ts from "typescript"; -export async function readJson(path: string) { - const text = await readFile(path, "utf-8"); +export function packageNameFromPath(path: string): string { + const base = basename(path); + return /^v\d+(\.\d+)?$/.exec(base) || /^ts\d\.\d/.exec(base) ? basename(dirname(path)) : base; +} +export function readJson(path: string) { + const text = readFileSync(path, "utf-8"); return JSON.parse(stripJsonComments(text)); } @@ -13,16 +17,12 @@ export function failure(ruleName: string, s: string): string { return `${s} See: https://github.com/microsoft/DefinitelyTyped-tools/blob/master/packages/dtslint/docs/${ruleName}.md`; } -export async function getCompilerOptions(dirPath: string): Promise { +export function getCompilerOptions(dirPath: string): ts.CompilerOptions { const tsconfigPath = join(dirPath, "tsconfig.json"); - if (!(await pathExists(tsconfigPath))) { + if (!pathExistsSync(tsconfigPath)) { throw new Error(`Need a 'tsconfig.json' file in ${dirPath}`); } - return (await readJson(tsconfigPath)).compilerOptions; -} - -export function withoutPrefix(s: string, prefix: string): string | undefined { - return s.startsWith(prefix) ? s.slice(prefix.length) : undefined; + return readJson(tsconfigPath).compilerOptions as ts.CompilerOptions; } export function last(a: readonly T[]): T { @@ -30,17 +30,6 @@ export function last(a: readonly T[]): T { return a[a.length - 1]; } -export async function mapDefinedAsync(arr: Iterable, mapper: (t: T) => Promise): Promise { - const out = []; - for (const a of arr) { - const res = await mapper(a); - if (res !== undefined) { - out.push(res); - } - } - return out; -} - export function isMainFile(fileName: string, allowNested: boolean) { // Linter may be run with cwd of the package. We want `index.d.ts` but not `submodule/index.d.ts` to match. if (fileName === "index.d.ts") { diff --git a/packages/dtslint/test/index.test.ts b/packages/dtslint/test/index.test.ts index ff8558702e..c697fc88f9 100644 --- a/packages/dtslint/test/index.test.ts +++ b/packages/dtslint/test/index.test.ts @@ -37,43 +37,112 @@ describe("dtslint", () => { noImplicitThis: true, strictNullChecks: true, strictFunctionTypes: true, - baseUrl: "../", - typeRoots: ["../"], types: [], noEmit: true, forceConsistentCasingInFileNames: true, }; describe("checks", () => { - it("disallows unknown compiler options", () => { - expect(() => checkTsconfig({ ...base, completelyInvented: true }, { relativeBaseUrl: "../" })).toThrow( - "Unexpected compiler option completelyInvented" - ); + describe("checkTsconfig", () => { + it("disallows unknown compiler options", () => { + expect(checkTsconfig("test", { ...base, completelyInvented: true })).toEqual([ + "Unexpected compiler option completelyInvented", + ]); + }); + it("allows exactOptionalPropertyTypes: true", () => { + expect(checkTsconfig("test", { ...base, exactOptionalPropertyTypes: true })).toEqual([]); + }); + it("allows module: node16", () => { + expect(checkTsconfig("test", { ...base, module: "node16" })).toEqual([]); + }); + it("allows `paths`", () => { + expect(checkTsconfig("test", { ...base, paths: { boom: ["../boom/index.d.ts"] } })).toEqual([]); + }); + it("disallows missing `module`", () => { + const options = { ...base }; + delete options.module; + expect(checkTsconfig("test", options)).toEqual([ + 'Must specify "module" to `"module": "commonjs"` or `"module": "node16"`.', + ]); + }); + it("disallows exactOptionalPropertyTypes: false", () => { + expect(checkTsconfig("test", { ...base, 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([]); + }); + it("allows paths: matching ../reference/index.d.ts", () => { + expect( + checkTsconfig("reactive-dep", { ...base, 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"] }, + }) + ).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"] }, + }) + ).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", { ...base, 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"] }, + }) + ).toEqual([]); + }); + it("forbids paths: mapping to self-contained file", () => { + expect(checkTsconfig("rrrr", { ...base, 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([ + `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([ + `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"] } }) + ).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"] } }) + ).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"] } }) + ).toEqual([`rrrr/tsconfig.json: "paths" must map 'react-native' to react-native's index.d.ts.`]); + }); }); - it("allows exactOptionalPropertyTypes: true", () => { - expect(checkTsconfig({ ...base, exactOptionalPropertyTypes: true }, { relativeBaseUrl: "../" })).toBeFalsy(); - }); - it("allows module: node16", () => { - expect(checkTsconfig({ ...base, module: "node16" }, { relativeBaseUrl: "../" })).toBeFalsy(); - }); - it("disallows missing `module`", () => { - const options = { ...base }; - delete options.module; - expect(() => checkTsconfig(options, { relativeBaseUrl: "../" })).toThrow( - 'Must specify "module" to `"module": "commonjs"` or `"module": "node16"`.' - ); - }); - it("disallows exactOptionalPropertyTypes: false", () => { - expect(() => checkTsconfig({ ...base, exactOptionalPropertyTypes: false }, { relativeBaseUrl: "../" })).toThrow( - 'When "exactOptionalPropertyTypes" is present, it must be set to `true`.' - ); - }); - it("disallows packages that are in notNeededPackages.json", () => { - expect(() => assertPackageIsNotDeprecated("foo", '{ "packages": { "foo": { } } }')).toThrow( - "notNeededPackages.json has an entry for foo." - ); - }); - it("allows packages that are not in notNeededPackages.json", () => { - expect(assertPackageIsNotDeprecated("foo", '{ "packages": { "bar": { } } }')).toBeUndefined(); + describe("assertPackageIsNotDeprecated", () => { + it("disallows packages that are in notNeededPackages.json", () => { + expect(() => assertPackageIsNotDeprecated("foo", '{ "packages": { "foo": { } } }')).toThrow( + "notNeededPackages.json has an entry for foo." + ); + }); + it("allows packages that are not in notNeededPackages.json", () => { + expect(assertPackageIsNotDeprecated("foo", '{ "packages": { "bar": { } } }')).toBeUndefined(); + }); }); }); describe("rules", () => { diff --git a/packages/dtslint/test/npm-naming/code/types/dts-critic/index.d.ts b/packages/dtslint/test/npm-naming/code/types/dts-critic/index.d.ts index 0b5ae5e141..0d3239f05e 100644 --- a/packages/dtslint/test/npm-naming/code/types/dts-critic/index.d.ts +++ b/packages/dtslint/test/npm-naming/code/types/dts-critic/index.d.ts @@ -1,6 +1 @@ -// Type definitions for package dts-critic 1.0 -// Project: https://https://github.com/DefinitelyTyped/dts-critic -// Definitions by: Jane Doe -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - export default dtsCritic(); diff --git a/packages/dtslint/test/npm-naming/code/types/dts-critic/package.json b/packages/dtslint/test/npm-naming/code/types/dts-critic/package.json new file mode 100644 index 0000000000..844ba94919 --- /dev/null +++ b/packages/dtslint/test/npm-naming/code/types/dts-critic/package.json @@ -0,0 +1,17 @@ +{ + "private": true, + "name": "@types/dts-critic", + "version": "1.0.9999", + "projects": [ + "https://github.com/microsoft/TypeScript" + ], + "devDependencies": { + "@types/dts-critic": "workspace:." + }, + "owners": [ + { + "name": "Jane Doe", + "githubUsername": "janedoe" + } + ] +} diff --git a/packages/dtslint/test/npm-naming/name/types/wenceslas/index.d.ts b/packages/dtslint/test/npm-naming/name/types/wenceslas/index.d.ts index 477c314203..81ab89bd42 100644 --- a/packages/dtslint/test/npm-naming/name/types/wenceslas/index.d.ts +++ b/packages/dtslint/test/npm-naming/name/types/wenceslas/index.d.ts @@ -1,4 +1 @@ -// Type definitions for package wenceslas 1.0 -// Project: https://github.com/bobby-headers/dt-header -// Definitions by: Jane Doe -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped +// hi diff --git a/packages/dtslint/test/npm-naming/name/types/wenceslas/index.d.ts.lint b/packages/dtslint/test/npm-naming/name/types/wenceslas/index.d.ts.lint index 3e4121e30e..50888569ef 100644 --- a/packages/dtslint/test/npm-naming/name/types/wenceslas/index.d.ts.lint +++ b/packages/dtslint/test/npm-naming/name/types/wenceslas/index.d.ts.lint @@ -1,6 +1,2 @@ -// Type definitions for package wenceslas 1.0 -~~~~~~~~~~~~~~~~~~~~~~~ [0] -// Project: https://github.com/bobby-headers/dt-header -// Definitions by: Jane Doe -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -[0]: Declaration file must have a matching npm package. To resolve this error, either: 1. Change the name to match an npm package. 2. Add a Definitely Typed header with the first line // Type definitions for non-npm package wenceslas-browser Add -browser to the end of your name to make sure it doesn't conflict with existing npm packages. If you won't fix this error now or you think this error is wrong, you can disable this check by adding the following options to your project's tslint.json file under "rules": "npm-naming": false See: https://github.com/microsoft/DefinitelyTyped-tools/blob/master/packages/dtslint/docs/npm-naming.md +// hi +~ [Declaration file must have a matching npm package. To resolve this error, either: 1. Change the name to match an npm package. 2. Add `"nonNpm": true` to the package.json to indicate that this is not an npm package. Ensure the package name is descriptive enough to avoid conflicts with future npm packages. If you won't fix this error now or you think this error is wrong, you can disable this check by adding the following options to your project's tslint.json file under "rules": "npm-naming": false See: https://github.com/microsoft/DefinitelyTyped-tools/blob/master/packages/dtslint/docs/npm-naming.md] diff --git a/packages/dtslint/test/npm-naming/name/types/wenceslas/package.json b/packages/dtslint/test/npm-naming/name/types/wenceslas/package.json new file mode 100644 index 0000000000..0bbc875c71 --- /dev/null +++ b/packages/dtslint/test/npm-naming/name/types/wenceslas/package.json @@ -0,0 +1,17 @@ +{ + "private": true, + "name": "@types/wenceslas", + "version": "0.0.9999", + "projects": [ + "https://github.com/bobby-headers/dt-header" + ], + "devDependencies": { + "@types/wenceslas": "workspace:." + }, + "owners": [ + { + "name": "Jane Doe", + "githubUsername": "janedoe" + } + ] +} diff --git a/packages/eslint-plugin/src/rules/dt-header.ts b/packages/eslint-plugin/src/rules/dt-header.ts deleted file mode 100644 index 35ecdd0c64..0000000000 --- a/packages/eslint-plugin/src/rules/dt-header.ts +++ /dev/null @@ -1,74 +0,0 @@ -import { renderExpected, validate } from "@definitelytyped/header-parser"; -import { createRule, isMainFile } from "../util"; - -type MessageId = - | "definitionsBy" - | "minimumTypeScriptVersion" - | "parseError" - | "typeDefinitionsFor" - | "typescriptVersion"; - -const rule = createRule({ - name: "dt-header", - defaultOptions: [], - meta: { - type: "problem", - docs: { - description: "Ensure consistency of DefinitelyTyped headers.", - recommended: "error", - }, - messages: { - definitionsBy: "Author name should be your name, not the default.", - minimumTypeScriptVersion: "TypeScript version should be specified under header in `index.d.ts`.", - parseError: "Error parsing header. Expected: {{expected}}", - typeDefinitionsFor: "Header should only be in `index.d.ts` of the root.", - typescriptVersion: "Minimum TypeScript version should be specified under header in `index.d.ts`.", - }, - schema: [], - }, - create(context) { - const sourceCode = context.getSourceCode(); - const { lines, text } = sourceCode; - - const lookFor = (search: string, messageId: MessageId) => { - for (let i = 0; i < lines.length; i += 1) { - if (lines[i].startsWith(search)) { - context.report({ - loc: { - end: { line: i + 1, column: search.length }, - start: { line: i + 1, column: 0 }, - }, - messageId, - }); - } - } - }; - - if (!isMainFile(context.getFilename(), /*allowNested*/ true)) { - lookFor("// Type definitions for", "typeDefinitionsFor"); - lookFor("// TypeScript Version", "typescriptVersion"); - lookFor("// Minimum TypeScript Version", "minimumTypeScriptVersion"); - return {}; - } - - lookFor("// Definitions by: My Self", "definitionsBy"); - - const error = validate(text); - if (error) { - context.report({ - data: { - expected: renderExpected(error.expected), - }, - loc: { - column: error.column, - line: error.line, - }, - messageId: "parseError", - }); - } - - return {}; - }, -}); - -export = rule; diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index f931036820..83ad5515c1 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -1,4 +1,3 @@ -import * as dtHeader from "./dt-header"; import * as exportJustNamespace from "./export-just-namespace"; import * as noAnyUnion from "./no-any-union"; import * as noBadReference from "./no-bad-reference"; @@ -6,7 +5,6 @@ import * as noConstEnum from "./no-const-enum"; import * as noDeadReference from "./no-dead-reference"; import * as noDeclareCurrentPackage from "./no-declare-current-package"; import * as noImportDefaultOfExportEquals from "./no-import-default-of-export-equals"; -import * as noOutsideDependencies from "./no-outside-dependencies"; import * as noRelativeImportInTest from "./no-relative-import-in-test"; import * as noSelfImport from "./no-self-import"; import * as noSingleElementTupleType from "./no-single-element-tuple-type"; @@ -16,9 +14,10 @@ import * as preferDeclareFunction from "./prefer-declare-function"; import * as redundantUndefined from "./redundant-undefined"; import * as strictExportDeclareModifiers from "./strict-export-declare-modifiers"; import * as noSingleDeclareModule from "./no-single-declare-module"; +import * as noOldDTHeader from "./no-old-dt-header"; +import * as noImportOfDevDependencies from "./no-import-of-dev-dependencies"; export const rules = { - "dt-header": dtHeader, "export-just-namespace": exportJustNamespace, "no-any-union": noAnyUnion, "no-bad-reference": noBadReference, @@ -26,7 +25,6 @@ export const rules = { "no-dead-reference": noDeadReference, "no-declare-current-package": noDeclareCurrentPackage, "no-import-default-of-export-equals": noImportDefaultOfExportEquals, - "no-outside-dependencies": noOutsideDependencies, "no-relative-import-in-test": noRelativeImportInTest, "no-self-import": noSelfImport, "no-single-element-tuple-type": noSingleElementTupleType, @@ -36,4 +34,6 @@ export const rules = { "redundant-undefined": redundantUndefined, "strict-export-declare-modifiers": strictExportDeclareModifiers, "no-single-declare-module": noSingleDeclareModule, + "no-old-dt-header": noOldDTHeader, + "no-import-of-dev-dependencies": noImportOfDevDependencies, }; diff --git a/packages/eslint-plugin/src/rules/no-bad-reference.ts b/packages/eslint-plugin/src/rules/no-bad-reference.ts index a07f04dcb4..7b5cc18d27 100644 --- a/packages/eslint-plugin/src/rules/no-bad-reference.ts +++ b/packages/eslint-plugin/src/rules/no-bad-reference.ts @@ -1,15 +1,14 @@ import { TSESTree } from "@typescript-eslint/utils"; -import { createRule } from "../util"; - -type MessageId = "referencePathPackage" | "referencePathTest"; +import { commentsMatching, createRule } from "../util"; +type MessageId = "referencePathPackage" | "referencePathTest" | "referencePathOldVersion"; const rule = createRule({ name: "no-bad-reference", defaultOptions: [], meta: { type: "problem", docs: { - description: `Forbids in any file, and forbid in test files.`, + description: `Forbids in all files, in declaration files, and all in test files.`, recommended: "error", }, messages: { @@ -17,27 +16,24 @@ const rule = createRule({ "Don't use to reference another package. Use an import or instead.", referencePathTest: "Don't use in test files. Use or include the file in 'tsconfig.json'.", + referencePathOldVersion: "Don't use to reference an old version of the current package.", }, schema: [], }, create(context) { - const { comments } = context.getSourceCode().ast; const isDeclarationFile = context.getFilename().endsWith(".d.ts"); - - for (const comment of comments) { - const referenceMatch = comment.value.match(//)?.[1]; - if (!referenceMatch) { - continue; + commentsMatching(context.getSourceCode(), //, (ref, comment) => { + if (ref.match(/^\.\/v\d+(?:\.\d+)?(?:\/.*)?$/)) { + report(comment, "referencePathOldVersion"); } - if (isDeclarationFile) { - if (referenceMatch.startsWith("..")) { + if (ref.startsWith("..")) { report(comment, "referencePathPackage"); } } else { report(comment, "referencePathTest"); } - } + }); return {}; diff --git a/packages/eslint-plugin/src/rules/no-import-of-dev-dependencies.ts b/packages/eslint-plugin/src/rules/no-import-of-dev-dependencies.ts new file mode 100644 index 0000000000..274c6ba013 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-import-of-dev-dependencies.ts @@ -0,0 +1,84 @@ +import { TSESTree } from "@typescript-eslint/utils"; +import { createRule, commentsMatching, getTypesPackageForDeclarationFile } from "../util"; +import fs from "fs"; +import path from "path"; + +type MessageId = "noImportOfDevDependencies" | "noReferenceOfDevDependencies"; +const rule = createRule({ + name: "no-import-of-dev-dependencies", + defaultOptions: [], + meta: { + type: "problem", + docs: { + description: "Forbid imports and references to devDependencies inside .d.ts files.", + recommended: "error", + }, + messages: { + noImportOfDevDependencies: `.d.ts files may not import packages in devDependencies.`, + noReferenceOfDevDependencies: `.d.ts files may not triple-slash reference packages in devDependencies.`, + }, + schema: [], + }, + create(context) { + const packageName = getTypesPackageForDeclarationFile(context.getFilename()); + if (context.getFilename().endsWith(".d.ts")) { + const packageJson = getPackageJson(context.getPhysicalFilename?.() ?? context.getFilename()); + const devdeps = packageJson + ? Object.keys(packageJson.devDependencies).map((dep) => dep.replace(/@types\//, "")) + : []; + commentsMatching(context.getSourceCode(), //, (ref, comment) => { + if (devdeps.includes(ref) && ref !== packageName) { + report(comment, "noReferenceOfDevDependencies"); + } + }); + + return { + // eslint-disable-next-line @typescript-eslint/naming-convention + ImportDeclaration(node) { + if (devdeps.includes(node.source.value) && node.source.value !== packageName) { + context.report({ + messageId: "noImportOfDevDependencies", + node, + }); + } + }, + }; + } else { + return {}; + } + function report(comment: TSESTree.Comment, messageId: MessageId) { + context.report({ + loc: { + end: { + column: comment.value.lastIndexOf(`"`), + line: comment.loc.end.line, + }, + start: { + column: comment.value.indexOf(`"`) + 1, + line: comment.loc.start.line, + }, + }, + messageId, + }); + } + }, +}); +function getPackageJson(sourceFile: string): { devDependencies: Record } | undefined { + let dir = path.dirname(sourceFile); + let text: string | undefined; + while (dir !== "/") { + try { + text = fs.readFileSync(path.join(dir, "package.json"), "utf8"); + break; + } catch { + // presumably because file does not exist, so continue + } + dir = path.dirname(dir); + } + if (!text) return undefined; + const json = JSON.parse(text); + if ("devDependencies" in json) return json; + return undefined; +} + +export = rule; diff --git a/packages/eslint-plugin/src/rules/no-old-dt-header.ts b/packages/eslint-plugin/src/rules/no-old-dt-header.ts new file mode 100644 index 0000000000..ba8ea0ba7a --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-old-dt-header.ts @@ -0,0 +1,40 @@ +import { createRule } from "../util"; + +const rule = createRule({ + name: "no-bad-reference", + defaultOptions: [], + meta: { + type: "problem", + docs: { + description: `Forbids in all files, in declaration files, and all in test files.`, + recommended: "error", + }, + messages: { + noOldDTHeader: + "Specify package metadata in package.json. Do not use a header like `// Type definitions for foo 1.2`", + }, + schema: [], + }, + create(context) { + const text = context.getSourceCode().text; + if ( + context.getFilename().endsWith(".d.ts") && + text.indexOf("// Type definitions for ") === 0 && + text.indexOf("// Definitions by: ") > 0 + ) { + context.report({ + messageId: "noOldDTHeader", + loc: { + start: { column: 0, line: 1 }, + end: { + column: "// Type definitions for ".length, + line: 1, + }, + }, + }); + } + return {}; + }, +}); + +export = rule; diff --git a/packages/eslint-plugin/src/rules/no-outside-dependencies.ts b/packages/eslint-plugin/src/rules/no-outside-dependencies.ts deleted file mode 100644 index d080a88d0b..0000000000 --- a/packages/eslint-plugin/src/rules/no-outside-dependencies.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { createRule, isMainFile } from "../util"; -import { ESLintUtils } from "@typescript-eslint/utils"; -const rule = createRule({ - name: "no-outside-dependencies", - defaultOptions: [], - meta: { - type: "problem", - docs: { - description: "Don't import things in `DefinitelyTyped/node_modules`.", - recommended: "error", - }, - messages: { - noOutsideDependencies: `File {{fileName}} comes from a \`node_modules\` but is not declared in this type's \`package.json\`. `, - }, - schema: [], - }, - create(context) { - if (isMainFile(context.getFilename(), /*allowNested*/ true)) { - const parserServices = ESLintUtils.getParserServices(context); - const hasNodeReference = parserServices.program - .getSourceFiles() - .some((f) => f.typeReferenceDirectives.some((dir) => dir.fileName === "node")); - for (const sourceFile of parserServices.program.getSourceFiles()) { - const fileName = sourceFile.fileName; - if ( - fileName.includes("/DefinitelyTyped/node_modules/") && - !parserServices.program.isSourceFileDefaultLibrary(sourceFile) && - !(hasNodeReference && fileName.includes("buffer")) - ) { - context.report({ - messageId: "noOutsideDependencies", - data: { fileName }, - loc: { column: 0, line: 1 }, - }); - } - } - } - return {}; - }, -}); - -export = rule; diff --git a/packages/eslint-plugin/src/rules/no-self-import.ts b/packages/eslint-plugin/src/rules/no-self-import.ts index 4f20109274..f6b98fcfdf 100644 --- a/packages/eslint-plugin/src/rules/no-self-import.ts +++ b/packages/eslint-plugin/src/rules/no-self-import.ts @@ -1,16 +1,17 @@ import { createRule, getTypesPackageForDeclarationFile } from "../util"; - const rule = createRule({ name: "no-self-import", defaultOptions: [], meta: { type: "problem", docs: { - description: "Forbids declaration files to import the current package using a global import.", + description: + "Forbids declaration files to import the current package using a global import or old versions with a relative import.", recommended: "error", }, messages: { useRelativeImport: "Declaration file should not use a global import of itself. Use a relative import.", + useOnlyCurrentVersion: "Don't import an old version of the current package.", }, schema: [], }, @@ -25,6 +26,11 @@ const rule = createRule({ messageId: "useRelativeImport", node, }); + } else if (node.source.value.match(/^\.\/v\d+(?:\.\d+)?(?:\/.*)?$/)) { + context.report({ + messageId: "useOnlyCurrentVersion", + node, + }); } }, }; diff --git a/packages/eslint-plugin/src/rules/no-useless-files.ts b/packages/eslint-plugin/src/rules/no-useless-files.ts index 171a578a87..25dc9cd32e 100644 --- a/packages/eslint-plugin/src/rules/no-useless-files.ts +++ b/packages/eslint-plugin/src/rules/no-useless-files.ts @@ -1,4 +1,4 @@ -import { createRule } from "../util"; +import { commentsMatching, createRule } from "../util"; const rule = createRule({ name: "no-useless-files", @@ -25,15 +25,9 @@ const rule = createRule({ } else { const referenceRegExp = /^\/\s* { noReferenceFound = false; - break; - } + }); if (noReferenceFound) { reportNoContent(); diff --git a/packages/eslint-plugin/src/util.ts b/packages/eslint-plugin/src/util.ts index 2e2c343dd2..38dea30c87 100644 --- a/packages/eslint-plugin/src/util.ts +++ b/packages/eslint-plugin/src/util.ts @@ -1,7 +1,7 @@ import { unmangleScopedPackage } from "@definitelytyped/utils"; -import { ESLintUtils } from "@typescript-eslint/utils"; +import { TSESTree, ESLintUtils } from "@typescript-eslint/utils"; import { RuleWithMetaAndName } from "@typescript-eslint/utils/dist/eslint-utils"; -import { RuleListener, RuleModule } from "@typescript-eslint/utils/dist/ts-eslint"; +import { RuleListener, RuleModule, SourceCode } from "@typescript-eslint/utils/dist/ts-eslint"; import { basename, dirname } from "path"; // Possible TS bug can't figure out how to do declaration emit of created rules @@ -46,3 +46,14 @@ export function isMainFile(fileName: string, allowNested: boolean) { // Allow "types/foo/index.d.ts", not "types/foo/utils/index.d.ts" return basename(dirname(parent)) === "types"; } + +export function commentsMatching( + sourceFile: Readonly, + regex: RegExp, + f: (match: string, c: TSESTree.Comment) => void +): void { + for (const comment of sourceFile.ast.comments) { + const m = comment.value.match(regex); + if (m) f(m[1], comment); + } +} diff --git a/packages/eslint-plugin/test/dt-header.test.ts b/packages/eslint-plugin/test/dt-header.test.ts deleted file mode 100644 index ee5eee1942..0000000000 --- a/packages/eslint-plugin/test/dt-header.test.ts +++ /dev/null @@ -1,189 +0,0 @@ -import { ESLintUtils } from "@typescript-eslint/utils"; - -import * as dtHeader from "../src/rules/dt-header"; - -const ruleTester = new ESLintUtils.RuleTester({ - parser: "@typescript-eslint/parser", -}); - -ruleTester.run("@definitelytyped/dt-header", dtHeader, { - invalid: [ - { - code: ``, - errors: [ - { - column: 2, - data: { - expected: "/\\/\\/ Type definitions for (non-npm package )?/", - }, - line: 1, - messageId: "parseError", - }, - ], - filename: "types/blank/index.d.ts", - }, - { - code: ` - // ... - `, - errors: [ - { - column: 1, - data: { - expected: "/\\/\\/ Type definitions for (non-npm package )?/", - }, - line: 2, - messageId: "parseError", - }, - ], - filename: "types/only-comment/index.d.ts", - }, - { - code: ` -// Type definitions for dt-header 0.75 -// Project: https://github.com/bobby-headers/dt-header -// Definitions by: Jane Doe -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// Minimum TypeScript Version: 3.1 - -`, - errors: [ - { - column: 1, - data: { - expected: "/\\/\\/ Type definitions for (non-npm package )?/", - }, - line: 2, - messageId: "parseError", - }, - ], - filename: "types/start-with-whitespace/index.d.ts", - }, - { - code: `// Type definitions for dt-header 1.0 -// Project: https://github.com/bobby-headers/dt-header -// Definitions by: Jane Doe -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -`, - errors: [ - { - column: 30, - data: { - expected: "/\\/", - }, - line: 3, - messageId: "parseError", - }, - ], - filename: "types/bad-url-github-org/index.d.ts", - }, - { - code: `// Type definitions for dt-header 1.0 -// Project: https://github.com/bobby-headers/dt-header -// Definitions by: Jane Doe -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -`, - errors: [ - { - column: 30, - data: { - expected: "/\\/", - }, - line: 3, - messageId: "parseError", - }, - ], - filename: "types/bad-url-space/index.d.ts", - }, - { - code: `// Type definitions for dt-header 1.0 -// Project: https://github.com/not/important -// Definitions by: My Self -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - -`, - errors: [ - { - column: 1, - endColumn: 27, - line: 3, - messageId: "definitionsBy", - }, - ], - filename: "types/bad-username/index.d.ts", - }, - { - code: `// Type definitions for dt-header v1.0.3 -// Project: https://github.com/bobby-headers/dt-header -// Definitions by: Jane Doe -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -`, - errors: [ - { - column: 26, - data: { - expected: "foo MAJOR.MINOR (patch version not allowed)", - }, - line: 1, - messageId: "parseError", - }, - ], - filename: "types/foo/index.d.ts", - }, - { - code: `// Type definitions for -`, - errors: [ - { - column: 1, - endColumn: 24, - line: 1, - messageId: "typeDefinitionsFor", - }, - ], - filename: "types/foo/notIndex.d.ts", - }, - ], - valid: [ - ``, - { - code: `// This isn't the main index -`, - filename: "types/foo/bar/index.d.ts", - }, - { - code: `// Type definitions for dt-header 0.75 -// Project: https://github.com/bobby-headers/dt-header -// Definitions by: Jane Doe -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// Minimum TypeScript Version: 3.1 - -`, - filename: "types/foo/v0.75/index.d.ts", - }, - { - code: `// Type definitions for dt-header 1.0 -// Project: https://github.com/bobby-headers/dt-header -// Definitions by: Jane Doe -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// Minimum TypeScript Version: 3.1 - -`, - filename: "types/foo/v1/index.d.ts", - }, - { - code: `// Type definitions for dt-header 2.0 -// Project: https://github.com/bobby-headers/dt-header -// Definitions by: Jane Doe -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped -// Minimum TypeScript Version: 3.1 - -`, - filename: "types/foo/index.d.ts", - }, - { - code: ``, - filename: "types/foo/notIndex.d.ts", - }, - ], -}); diff --git a/packages/eslint-plugin/test/no-bad-reference.test.ts b/packages/eslint-plugin/test/no-bad-reference.test.ts index 3fb8993da9..acf3d8861b 100644 --- a/packages/eslint-plugin/test/no-bad-reference.test.ts +++ b/packages/eslint-plugin/test/no-bad-reference.test.ts @@ -44,6 +44,66 @@ ruleTester.run("@definitelytyped/no-bad-reference", noBadReference, { ], filename: "types.d.ts", }, + { + code: `/// `, + errors: [ + { + column: 20, + endColumn: 25, + line: 1, + messageId: "referencePathOldVersion", + }, + ], + filename: "types.d.ts", + }, + { + code: `/// `, + errors: [ + { + column: 20, + endColumn: 31, + line: 1, + messageId: "referencePathOldVersion", + }, + ], + filename: "types.d.ts", + }, + { + code: `/// `, + errors: [ + { + column: 20, + endColumn: 37, + line: 1, + messageId: "referencePathOldVersion", + }, + ], + filename: "types.d.ts", + }, + { + code: `/// `, + errors: [ + { + column: 20, + endColumn: 26, + line: 1, + messageId: "referencePathOldVersion", + }, + ], + filename: "types.d.ts", + }, + { + code: `/// `, + errors: [ + { + column: 20, + endColumn: 32, + line: 1, + messageId: "referencePathOldVersion", + }, + ], + filename: "types.d.ts", + }, ], valid: [ ``, @@ -57,6 +117,14 @@ ruleTester.run("@definitelytyped/no-bad-reference", noBadReference, { code: `/// `, filename: "types.d.ts", }, + { + code: `/// `, + filename: "types.d.ts", + }, + { + code: `/// `, + filename: "types.d.ts", + }, { code: `/// /// `, diff --git a/packages/eslint-plugin/test/no-import-of-dev-dependencies.test.ts b/packages/eslint-plugin/test/no-import-of-dev-dependencies.test.ts new file mode 100644 index 0000000000..436172e490 --- /dev/null +++ b/packages/eslint-plugin/test/no-import-of-dev-dependencies.test.ts @@ -0,0 +1,72 @@ +import { ESLintUtils } from "@typescript-eslint/utils"; + +import * as noImportDefaultOfDevDependencies from "../src/rules/no-import-of-dev-dependencies"; + +const ruleTester = new ESLintUtils.RuleTester({ + parser: "@typescript-eslint/parser", + parserOptions: { + ecmaVersion: 2018, + }, +}); + +ruleTester.run("@definitelytyped/no-import-of-dev-dependencies", noImportDefaultOfDevDependencies, { + invalid: [ + { + filename: "index.d.ts", + code: `import eslint from "eslint"`, + errors: [ + { + line: 1, + messageId: "noImportOfDevDependencies", + }, + ], + }, + // test @types/ removal + { + filename: "index.d.ts", + code: `import yargs from "yargs"`, + errors: [ + { + line: 1, + messageId: "noImportOfDevDependencies", + }, + ], + }, + { + filename: "index.d.ts", + code: `/// `, + errors: [ + { + line: 1, + messageId: "noReferenceOfDevDependencies", + }, + ], + }, + ], + valid: [ + { + filename: "index.d.ts", + code: `import other from 'other'`, + }, + { + filename: "types/yargs/index.d.ts", + code: `import self from "yargs"`, + }, + { + filename: "index.d.ts", + code: `/// `, + }, + { + filename: "test.ts", + code: `import eslint from "eslint"`, + }, + { + filename: "test.ts", + code: `import yargs from "yargs"`, + }, + { + filename: "test.ts", + code: `/// `, + }, + ], +}); diff --git a/packages/eslint-plugin/test/no-old-dt-header.test.ts b/packages/eslint-plugin/test/no-old-dt-header.test.ts new file mode 100644 index 0000000000..687e308fc1 --- /dev/null +++ b/packages/eslint-plugin/test/no-old-dt-header.test.ts @@ -0,0 +1,71 @@ +import { ESLintUtils } from "@typescript-eslint/utils"; + +import * as noOldDtHeader from "../src/rules/no-old-dt-header"; + +const ruleTester = new ESLintUtils.RuleTester({ + parser: "@typescript-eslint/parser", +}); + +ruleTester.run("@definitelytyped/no-old-dt-header", noOldDtHeader, { + invalid: [ + { + code: `// Type definitions for AFRAME 1.2 +// Project: https://aframe.io/ +// Definitions by: Paul Shannon +// Roberto Ritger +// Trygve Wastvedt +// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped +// TypeScript Version: 4.4 + +/** + * Extended tests and examples available at https://github.com/devpaul/aframe-experiments.git + */ + +import * as anime from "animejs"; +import * as three from "three";`, + errors: [ + { + column: 1, + endColumn: 25, + line: 1, + messageId: "noOldDTHeader", + }, + ], + filename: "index.d.ts", + }, + { + code: `// Type definitions for AFRAME 1.2 +// Definitions by: Paul Shannon `, + errors: [ + { + column: 1, + endColumn: 25, + line: 1, + messageId: "noOldDTHeader", + }, + ], + filename: "types.d.ts", + }, + ], + valid: [ + { + code: `// Type definitions for AFRAME 1.2 +// Definitions by: Paul Shannon `, + filename: "test.ts", + }, + { + code: `// Type definitions for AFRAME 1.2`, + filename: "types.d.ts", + }, + { + code: `// Definitions by: Paul Shannon `, + filename: "types.d.ts", + }, + { + code: `// A line before the old header + // Type definitions for AFRAME 1.2 +// Definitions by: Paul Shannon `, + filename: "types.d.ts", + }, + ], +}); diff --git a/packages/eslint-plugin/test/no-self-import.test.ts b/packages/eslint-plugin/test/no-self-import.test.ts index c18016d5b7..1fc8ef51e8 100644 --- a/packages/eslint-plugin/test/no-self-import.test.ts +++ b/packages/eslint-plugin/test/no-self-import.test.ts @@ -28,6 +28,66 @@ ruleTester.run("@definitelytyped/no-self-import", dtHeader, { ], filename: "types/this-package/index.d.ts", }, + { + code: `import old from "./v11"`, + errors: [ + { + column: 1, + endColumn: 24, + line: 1, + messageId: "useOnlyCurrentVersion", + }, + ], + filename: "types.d.ts", + }, + { + code: `import old from "./v11/index"`, + errors: [ + { + column: 1, + endColumn: 30, + line: 1, + messageId: "useOnlyCurrentVersion", + }, + ], + filename: "types.d.ts", + }, + { + code: `import old from "./v11/subdir/file"`, + errors: [ + { + column: 1, + endColumn: 36, + line: 1, + messageId: "useOnlyCurrentVersion", + }, + ], + filename: "types.d.ts", + }, + { + code: `import old from "./v0.1"`, + errors: [ + { + column: 1, + endColumn: 25, + line: 1, + messageId: "useOnlyCurrentVersion", + }, + ], + filename: "types.d.ts", + }, + { + code: `import old from "./v0.1/index"`, + errors: [ + { + column: 1, + endColumn: 31, + line: 1, + messageId: "useOnlyCurrentVersion", + }, + ], + filename: "types.d.ts", + }, ], valid: [ { @@ -42,5 +102,13 @@ ruleTester.run("@definitelytyped/no-self-import", dtHeader, { code: `import myself from "this-package";`, filename: "types/grandparent/this-package/index.d.ts", }, + { + code: `import old from "./v1gardenpath"`, + filename: "types.d.ts", + }, + { + code: `import old from "./v1verb/other"`, + filename: "types.d.ts", + }, ], }); diff --git a/packages/header-parser/README.md b/packages/header-parser/README.md index d48243a55a..476379caff 100644 --- a/packages/header-parser/README.md +++ b/packages/header-parser/README.md @@ -1,10 +1,11 @@ # 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 -To build: `npm run build`. -To test: `npm run test`. (Currently requires a re-build to test changes.) +To build: `pnpm run build`. +To test: `pnpm test`. This project has adopted the [Microsoft Open Source Code of Conduct](https://opensource.microsoft.com/codeofconduct/). For more information see the [Code of Conduct FAQ](https://opensource.microsoft.com/codeofconduct/faq/) or contact [opencode@microsoft.com](mailto:opencode@microsoft.com) with any additional questions or comments. diff --git a/packages/header-parser/package.json b/packages/header-parser/package.json index e60839bade..0333d82df0 100644 --- a/packages/header-parser/package.json +++ b/packages/header-parser/package.json @@ -21,8 +21,11 @@ }, "dependencies": { "@definitelytyped/typescript-versions": "workspace:*", - "@types/parsimmon": "^1.10.1", - "parsimmon": "^1.13.0" + "@definitelytyped/utils": "workspace:*", + "semver": "^7.3.7" + }, + "devDependencies": { + "@types/semver": "^7.5.2" }, "publishConfig": { "access": "public" diff --git a/packages/header-parser/src/index.ts b/packages/header-parser/src/index.ts index 8f08090b25..1ae53c25b7 100644 --- a/packages/header-parser/src/index.ts +++ b/packages/header-parser/src/index.ts @@ -1,42 +1,33 @@ -import pm = require("parsimmon"); import { AllTypeScriptVersion, TypeScriptVersion } from "@definitelytyped/typescript-versions"; +import assert = require("assert"); +import fs = require("fs"); +import * as semver from "semver"; +import { withoutStart, mapDefined, deepEquals, joinPaths } from "@definitelytyped/utils"; -/* - - # Example header format # - - // Type definitions for foo 1.2 - // Project: https://github.com/foo/foo, https://foo.com - // Definitions by: My Self , Some Other Guy - // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - // TypeScript Version: 2.1 - -*/ - +// used in dts-critic export interface Header { readonly nonNpm: boolean; - readonly libraryName: string; + readonly nonNpmDescription?: string; + readonly name: string; readonly libraryMajorVersion: number; readonly libraryMinorVersion: number; - readonly typeScriptVersion: AllTypeScriptVersion; + readonly minimumTypeScriptVersion: AllTypeScriptVersion; readonly projects: readonly string[]; - readonly contributors: readonly Author[]; -} - -export interface Author { - readonly name: string; - readonly url: string; - readonly githubUsername: string | undefined; -} - -export interface ParseError { - readonly index: number; - readonly line: number; - readonly column: number; - readonly expected: readonly string[]; + readonly owners: readonly Owner[]; } +// used in definitions-parser +export type Owner = + | { + readonly name: string; + readonly url: string; + } + | { + readonly name: string; + readonly githubUsername: string; + readonly url?: undefined; + }; -export function makeTypesVersionsForPackageJson(typesVersions: readonly TypeScriptVersion[]): unknown { +export function makeTypesVersionsForPackageJson(typesVersions: readonly AllTypeScriptVersion[]): unknown { if (typesVersions.length === 0) { return undefined; } @@ -50,230 +41,449 @@ export function makeTypesVersionsForPackageJson(typesVersions: readonly TypeScri return out; } -export function parseHeaderOrFail(descriptor: string, mainFileContent: string): Header { - const header = parseHeader(mainFileContent, /*strict*/ false); - if (isParseError(header)) { - throw new Error(renderParseError(descriptor, header)); +export function validatePackageJson( + typesDirectoryName: string, + packageJson: Record, + typesVersions: readonly AllTypeScriptVersion[] +): Header | string[] { + const errors = []; + const needsTypesVersions = typesVersions.length !== 0; + for (const key in packageJson) { + switch (key) { + case "private": + case "dependencies": + case "license": + case "imports": + case "exports": + case "type": + case "name": + case "version": + case "devDependencies": + case "projects": + case "minimumTypeScriptVersion": + case "owners": + case "nonNpm": + case "nonNpmDescription": + case "pnpm": + break; + case "typesVersions": + case "types": + if (!needsTypesVersions) { + errors.push( + `${typesDirectoryName}'s package.json doesn't need to set "${key}" when no 'tsX.X' directories exist.` + ); + } + break; + default: + errors.push(`${typesDirectoryName}'s package.json should not include property ${key}`); + } + } + // private + if (packageJson.private !== true) { + errors.push(`${typesDirectoryName}'s package.json has bad "private": must be \`"private": true\``); + } + // devDependencies + if ( + typeof packageJson.devDependencies !== "object" || + packageJson.devDependencies === null || + (packageJson.devDependencies as any)["@types/" + typesDirectoryName] !== "workspace:." + ) { + errors.push( + `${typesDirectoryName}'s package.json has bad "devDependencies": must include \`"@types/${typesDirectoryName}": "workspace:."\`` + ); + } + // typesVersions + if (needsTypesVersions) { + assert.strictEqual( + packageJson.types, + "index", + `"types" in '${typesDirectoryName}'s package.json' should be "index".` + ); + const expected = makeTypesVersionsForPackageJson(typesVersions) as Record; + if (!deepEquals(packageJson.typesVersions, expected)) { + errors.push( + `'${typesDirectoryName}'s package.json' has bad "typesVersions". Should be: ${JSON.stringify( + expected, + undefined, + 4 + )}` + ); + } } - return header; -} - -export function validate(mainFileContent: string): ParseError | undefined { - const h = parseHeader(mainFileContent, /*strict*/ true); - return isParseError(h) ? h : undefined; -} - -export function renderExpected(expected: readonly string[]): string { - return expected.length === 1 ? expected[0] : `one of\n\t${expected.join("\n\t")}`; -} - -function renderParseError(descriptor: string, { line, column, expected }: ParseError): string { - return `At ${line}:${column} in ${descriptor}: Expected ${renderExpected(expected)}`; -} - -function isParseError(x: {}): x is ParseError { - // tslint:disable-next-line strict-type-predicates - return (x as ParseError).expected !== undefined; -} - -/** @param strict If true, we allow fewer things to be parsed. Turned on by linting. */ -function parseHeader(text: string, strict: boolean): Header | ParseError { - const res = headerParser(strict).parse(text); - return res.status - ? res.value - : { index: res.index.offset, line: res.index.line, column: res.index.column, expected: res.expected }; -} -function headerParser(strict: boolean): pm.Parser
{ - return pm.seqMap( - pm.regex(/\/\/ Type definitions for (non-npm package )?/), - parseLabel(strict), - pm.string("// Project: "), - projectParser, - pm.regexp(/\r?\n\/\/ Definitions by: /), - contributorsParser(strict), - definitionsParser, - typeScriptVersionParser, - pm.all, // Don't care about the rest of the file - // tslint:disable-next-line:variable-name - (str, label, _project, projects, _defsBy, contributors, _definitions, typeScriptVersion) => ({ - libraryName: label.name, - libraryMajorVersion: label.major, - libraryMinorVersion: label.minor, - nonNpm: str.endsWith("non-npm package "), + // building the header object uses a monadic error pattern based on the one in the old header parser + // It's verbose and repetitive, but I didn't feel like writing a monadic `seq` to be used in only one place. + let name = "ERROR"; + let libraryMajorVersion = 0; + let libraryMinorVersion = 0; + let nonNpm = false; + let minimumTypeScriptVersion: AllTypeScriptVersion = TypeScriptVersion.lowest; + let projects: string[] = []; + let owners: Owner[] = []; + const nameResult = validateName(); + const versionResult = validateVersion(); + const nonNpmResult = validateNonNpm(); + const typeScriptVersionResult = validateTypeScriptVersion(); + const projectsResult = validateProjects(); + const ownersResult = validateOwners(); + const pnpmResult = validatePnpm(); + const licenseResult = getLicenseFromPackageJson(packageJson.license); + if (typeof nameResult === "object") { + errors.push(...nameResult.errors); + } else { + name = packageJson.name as string; + } + if ("errors" in versionResult) { + errors.push(...versionResult.errors); + } else { + libraryMajorVersion = versionResult.major; + libraryMinorVersion = versionResult.minor; + } + if (typeof nonNpmResult === "object") { + errors.push(...nonNpmResult.errors); + } else { + nonNpm = nonNpmResult; + } + if (typeof typeScriptVersionResult === "object") { + errors.push(...typeScriptVersionResult.errors); + } else { + minimumTypeScriptVersion = typeScriptVersionResult; + } + if ("errors" in projectsResult) { + errors.push(...projectsResult.errors); + } else { + projects = projectsResult; + } + if ("errors" in ownersResult) { + errors.push(...ownersResult.errors); + } else { + owners = ownersResult; + } + if (typeof pnpmResult === "object") { + errors.push(...pnpmResult.errors); + } + if (Array.isArray(licenseResult)) { + errors.push(...licenseResult); + } + if (errors.length) { + return errors; + } else { + return { + name, + libraryMajorVersion, + libraryMinorVersion, + nonNpm, + minimumTypeScriptVersion, projects, - contributors, - typeScriptVersion, - }) - ); -} - -interface Label { - readonly name: string; - readonly major: number; - readonly minor: number; -} - -/* -Allow any of the following: - -// Project: https://foo.com -// https://bar.com - -// Project: https://foo.com, -// https://bar.com - -// Project: https://foo.com, https://bar.com - -Use `\s\s+` to ensure at least 2 spaces, to disambiguate from the next line being `// Definitions by:`. -*/ -const separator: pm.Parser = pm.regexp(/(, )|(,?\r?\n\/\/\s\s+)/); - -const projectParser: pm.Parser = pm.sepBy1(pm.regexp(/[^,\r\n]+/), separator); - -function contributorsParser(strict: boolean): pm.Parser { - const contributor: pm.Parser = strict - ? pm.seqMap( - pm.regexp(/([^<]+) /, 1), - pm.regexp(/\/, 1), - (name, githubUsername) => ({ name, url: `https://github.com/${githubUsername}`, githubUsername }) - ) - : // In non-strict mode, allows arbitrary URL, and trailing whitespace. - pm.seqMap(pm.regexp(/([^<]+) /, 1), pm.regexp(/<([^>]+)> */, 1), (name, url) => { - const rgx = /^https\:\/\/github.com\/([a-zA-Z\d\-]+)\/?$/; - const match = rgx.exec(url); - const githubUsername = match === null ? undefined : match[1]; - // tslint:disable-next-line no-null-keyword - return { name, url: githubUsername ? `https://github.com/${githubUsername}` : url, githubUsername }; - }); - return pm.sepBy1(contributor, separator); -} - -const definitionsParser = pm.regexp(/\r?\n\/\/ Definitions: [^\r\n]+/); + owners, + }; + } -function parseLabel(strict: boolean): pm.Parser