Switch to reporting errors at end, not throwing one at a time

This commit is contained in:
Nathan Shively-Sanders
2023-04-17 11:06:13 -07:00
parent 2c80d9803c
commit 05fcf435a7
10 changed files with 209 additions and 149 deletions

1
.vscode/launch.json vendored
View File

@@ -14,7 +14,6 @@
"args": [
"--path", "../DefinitelyTyped", "--selection", "all", "--localTypeScriptPath", "../../ts/built/local",
],
"type": "node"
},
{

View File

@@ -1,5 +1,5 @@
import * as ts from "typescript";
import { validatePackageJson } from "@definitelytyped/header-parser";
import { validatePackageJson, Header } from "@definitelytyped/header-parser";
import { allReferencedFiles, createSourceFile, getDeclaredGlobals } from "./module-info";
import {
DependencyVersion,
@@ -29,6 +29,7 @@ import {
import { TypeScriptVersion } from "@definitelytyped/typescript-versions";
import { slicePrefixes } from "./utils";
import path from "path";
import assert from "assert";
function matchesVersion(
typingsDataRaw: TypingsDataRaw,
@@ -47,9 +48,10 @@ function formattedLibraryVersion(typingsDataRaw: TypingsDataRaw): `${number}.${n
return `${typingsDataRaw.libraryMajorVersion}.${typingsDataRaw.libraryMinorVersion}`;
}
export async function getTypingInfo(packageName: string, dt: FS): Promise<TypingsVersionsRaw> {
export async function getTypingInfo(packageName: string, dt: FS): Promise<TypingsVersionsRaw | string[]> {
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;
@@ -67,17 +69,17 @@ export async function getTypingInfo(packageName: string, dt: FS): Promise<Typing
const moduleResolutionHost = createModuleResolutionHost(dt, dt.debugPath());
const considerLibraryMinorVersion = olderVersionDirectories.some(({ version }) => version.minor !== undefined);
const latestData: TypingsDataRaw = {
libraryVersionDirectoryName: undefined,
...(await combineDataForAllTypesVersions(packageName, rootDirectoryLs, fs, moduleResolutionHost)),
};
const latestDataResult = await combineDataForAllTypesVersions(packageName, rootDirectoryLs, fs, moduleResolutionHost)
if (Array.isArray(latestDataResult)) {
return [...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(
errors.push(
`The latest version of the '${packageName}' package is ${latest}, so the subdirectory '${directoryName}' is not allowed` +
(`v${latest}` === directoryName
? "."
@@ -87,35 +89,42 @@ export async function getTypingInfo(packageName: string, dt: FS): Promise<Typing
// 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(
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(
errors.push(
`Directory ${directoryName} indicates major.minor version ${directoryVersion.major}.${directoryVersion.minor ?? "*"}, ` +
`but package.json indicates major.minor version ${data.libraryMajorVersion}.${data.libraryMinorVersion}`);
} else {
errors.push(
`Directory ${directoryName} indicates major version ${directoryVersion.major}, but package.json indicates major version ` +
data.libraryMajorVersion.toString()
);
}
throw new Error(
`Directory ${directoryName} indicates major version ${directoryVersion.major}, but package.json 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;
}
@@ -127,7 +136,8 @@ interface LsMinusTypesVersionsAndPackageJson {
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);
@@ -137,12 +147,15 @@ function getTypesVersionsAndPackageJson(ls: readonly string[]): LsMinusTypesVers
const version = match[1];
if (parseInt(version, 10) < 3) {
throw new Error(
errors.push(
`Directory name starting with 'ts' should be a TypeScript version newer than 3.0. Got: ${version}`
);
}
return version as TypeScriptVersion;
});
if (errors.length) {
return errors
}
return { remainingLs, typesVersions, hasPackageJson: withoutPackageJson.length !== ls.length };
}
@@ -182,8 +195,13 @@ async function combineDataForAllTypesVersions(
ls: readonly string[],
fs: FS,
moduleResolutionHost: ts.ModuleResolutionHost
): Promise<Omit<TypingsDataRaw, "libraryVersionDirectoryName">> {
const { remainingLs, typesVersions, hasPackageJson } = getTypesVersionsAndPackageJson(ls);
): Promise<Omit<TypingsDataRaw, "libraryVersionDirectoryName"> | string[]> {
const errors = []
const typesVersionAndPackageJson = getTypesVersionsAndPackageJson(ls)
if (Array.isArray(typesVersionAndPackageJson)) {
errors.push(...typesVersionAndPackageJson)
}
const { remainingLs, typesVersions, hasPackageJson } = Array.isArray(typesVersionAndPackageJson) ? { remainingLs: [], typesVersions: [], hasPackageJson: false} : typesVersionAndPackageJson;
const packageJson = hasPackageJson
? (fs.readJson(packageJsonName) as {
readonly license?: unknown;
@@ -194,21 +212,6 @@ async function combineDataForAllTypesVersions(
readonly type?: unknown;
})
: {};
const packageJsonType = checkPackageJsonType(packageJson.type, packageJsonName);
const packageJsonResult = validatePackageJson(typingsPackageName, packageJsonName, packageJson, typesVersions);
if (Array.isArray(packageJsonResult)) {
throw Error(packageJsonResult.join("\n"));
}
const {
contributors,
libraryMajorVersion,
libraryMinorVersion,
typeScriptVersion: minTsVersion,
libraryName,
projects,
} = packageJsonResult
const dataForRoot = getTypingDataForSingleTypesVersion(
undefined,
typingsPackageName,
@@ -216,22 +219,57 @@ async function combineDataForAllTypesVersions(
fs,
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,
moduleResolutionHost
);
if (Array.isArray(data)) {
errors.push(...data)
}
return data
});
const allTypesVersions = [dataForRoot, ...dataForOtherTypesVersions];
const license = getLicenseFromPackageJson(packageJson.license);
const allowedDependencies = await getAllowedPackageJsonDependencies()
checkPackageJsonDependencies(packageJson.dependencies, packageJsonName, allowedDependencies);
checkPackageJsonDependencies(packageJson.devDependencies, packageJsonName, allowedDependencies);
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 {
contributors,
libraryMajorVersion,
libraryMinorVersion,
typeScriptVersion: minTsVersion,
libraryName,
projects,
} = Array.isArray(packageJsonResult) ? {} as Header : packageJsonResult
const allowedDependencies = await getAllowedPackageJsonDependencies()
errors.push(...checkPackageJsonDependencies(packageJson.dependencies, packageJsonName, allowedDependencies))
errors.push(...checkPackageJsonDependencies(packageJson.devDependencies, packageJsonName, allowedDependencies))
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);
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}`))
@@ -250,8 +288,8 @@ async function combineDataForAllTypesVersions(
typesVersions,
files,
license,
packageJsonDependencies: packageJson.dependencies,
packageJsonDevDependencies: packageJson.devDependencies,
packageJsonDependencies: packageJson.dependencies as Record<string,string>,
packageJsonDevDependencies: packageJson.devDependencies as Record<string, string>,
// TODO: Add devDependencies here (aka testDependencies)
contentHash: hash(
hasPackageJson ? [...files, packageJsonName] : files,
@@ -259,9 +297,9 @@ async function combineDataForAllTypesVersions(
fs
),
globals: getAllUniqueValues<"globals", string>(allTypesVersions, "globals"),
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,
};
}
@@ -289,7 +327,8 @@ function getTypingDataForSingleTypesVersion(
ls: readonly string[],
fs: FS,
moduleResolutionHost: ts.ModuleResolutionHost
): TypingDataFromIndividualTypeScriptVersion {
): TypingDataFromIndividualTypeScriptVersion | string[] {
const errors = []
const tsconfig = fs.readJson("tsconfig.json") as TsConfig;
const configHost: ts.ParseConfigHost = {
...moduleResolutionHost,
@@ -302,10 +341,10 @@ function getTypingDataForSingleTypesVersion(
configHost,
path.resolve("/", fs.debugPath())
).options;
checkFilesFromTsConfig(packageName, tsconfig, fs.debugPath());
errors.push(...checkFilesFromTsConfig(packageName, tsconfig, fs.debugPath()));
// TODO: tests should be Set<string>, not Map<string, SourceFile>
const { types, tests } = allReferencedFiles(
tsconfig.files!,
tsconfig.files ?? [],
fs,
packageName,
moduleResolutionHost,
@@ -320,18 +359,17 @@ 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}`);
}
}
// Note: findAllUnusedFiles also modifies usedFiles and otherFiles
const unusedFiles = findAllUnusedFiles(ls, usedFiles, otherFiles, packageName, fs);
// TODO: Don't throw here, but instead return a list of errors to be printed by ???
// Note: findAllUnusedFiles also modifies usedFiles and otherFiles and errors
const unusedFiles = findAllUnusedFiles(ls, usedFiles, otherFiles, errors, packageName, fs);
if (unusedFiles.length) {
throw new Error('\n\t* ' + unusedFiles.map(unused => `Unused file ${unused}`).join("\n\t* ") + `\n\t(used files: ${JSON.stringify(Array.from(usedFiles))})`)
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,
@@ -345,6 +383,8 @@ function getTypingDataForSingleTypesVersion(
);
}
if (errors.length)
return errors
return {
typescriptVersion,
globals: getDeclaredGlobals(types),
@@ -354,16 +394,17 @@ function getTypingDataForSingleTypesVersion(
}
// TODO: Expand these checks too, adding name and version just like dtslint
// TODO: Maybe should be in header-parser now?
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.`);
return [`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.`);
return [`Package exports at path ${path} should not be null.`];
}
if (!(exports as Record<string, unknown>)["./package.json"]) {
(exports as Record<string, unknown>)["./package.json"] = "./package.json";
@@ -371,13 +412,13 @@ function checkPackageJsonExportsAndAddPJsonEntry(exports: unknown, path: string)
return exports;
}
function checkPackageJsonImports(imports: unknown, path: string) {
function checkPackageJsonImports(imports: unknown, path: string): object | string[] | undefined {
if (imports === undefined) return imports;
if (typeof imports !== "object") {
throw new Error(`Package imports at path ${path} should be an object or string.`);
return [`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.`);
else if (imports === null) {
return [`Package imports at path ${path} should not be null.`];
}
return imports;
}
@@ -385,7 +426,7 @@ function checkPackageJsonImports(imports: unknown, path: string) {
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 [`Package type at path ${path} can only be 'module'.`];
}
return type;
}
@@ -394,48 +435,51 @@ function checkPackageJsonDependencies(
dependencies: unknown,
path: string,
allowedDependencies: ReadonlySet<string>
): asserts dependencies is Record<string, string> {
): string[] {
if (dependencies === undefined) {
return;
return [];
}
if (dependencies === null || typeof dependencies !== "object") {
throw new Error(`${path} should contain "dependencies" or not exist.`);
return [`${path} should contain "dependencies" or not exist.`];
}
const errors: string[] = [];
for (const dependencyName of Object.keys(dependencies!)) {
// `dependencies` cannot be null because of check above.
if (!dependencyName.startsWith("@types/") && !allowedDependencies.has(dependencyName)) {
const msg = `Dependency ${dependencyName} not in the allowed dependencies list.
Please make a pull request to microsoft/DefinitelyTyped-tools adding it to \`packages/definitions-parser/allowedPackageJsonDependencies.txt\`.`;
throw new Error(`In ${path}: ${msg}`);
errors.push(`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.`);
errors.push(`In ${path}: Dependency version for ${dependencyName} should be a string.`);
}
}
return errors
}
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.`);
}
@@ -447,10 +491,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) {
@@ -484,24 +529,25 @@ export function readFileAndThrowOnBOM(fileName: string, fs: FS): string {
const unusedFilesName = "OTHER_FILES.txt";
/** Modifies usedFiles and otherFiles */
/** Modifies usedFiles and otherFiles and errors */
function findAllUnusedFiles(
ls: readonly string[],
usedFiles: Set<string>,
otherFiles: string[],
errors: string[],
packageName: string,
fs: FS
fs: FS,
): 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}`);
}
}
return findAllUnusedRecur(new Set(ls), usedFiles, new Set(otherFiles), fs);
return findAllUnusedRecur(new Set(ls), usedFiles, new Set(otherFiles), errors, fs);
}
function findAllUnusedRecur(ls: Iterable<string>, usedFiles: Set<string>, otherFiles: Set<string>, fs: FS): string[] {
function findAllUnusedRecur(ls: Iterable<string>, usedFiles: Set<string>, otherFiles: Set<string>, errors: string[], fs: FS): string[] {
const unused = []
for (const lsEntry of ls) {
if (usedFiles.has(lsEntry)) {
@@ -521,8 +567,7 @@ function findAllUnusedRecur(ls: Iterable<string>, usedFiles: Set<string>, otherF
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<string>): Set<string> {
@@ -536,7 +581,7 @@ function findAllUnusedRecur(ls: Iterable<string>, usedFiles: Set<string>, otherF
}
return subdirSet;
}
findAllUnusedRecur(lssubdir, takeSubdirectoryOutOfSet(usedFiles), takeSubdirectoryOutOfSet(otherFiles), subdir);
findAllUnusedRecur(lssubdir, takeSubdirectoryOutOfSet(usedFiles), takeSubdirectoryOutOfSet(otherFiles), errors, subdir);
} else {
if (
lsEntry.toLowerCase() !== "readme.md" &&
@@ -551,11 +596,11 @@ function findAllUnusedRecur(ls: Iterable<string>, usedFiles: Set<string>, otherF
}
for (const otherFile of otherFiles) {
if (usedFiles.has(otherFile)) {
throw new Error(
errors.push(
`File ${fs.debugPath()}${otherFile} listed in ${unusedFilesName} is already reachable from tsconfig.json.`
);
}
throw new Error(`File ${fs.debugPath()}/${otherFile} listed in ${unusedFilesName} does not exist.`);
errors.push(`File ${fs.debugPath()}/${otherFile} listed in ${unusedFilesName} does not exist.`);
}
return unused
}

View File

@@ -46,7 +46,7 @@ export class DTMock {
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), "package.json", packageJson, []);
const latestHeader = validatePackageJson(mangleScopedPackage(packageName), packageJson, []);
if (Array.isArray(latestHeader)) {
throw new Error(latestHeader.join("\n"));
}

View File

@@ -24,6 +24,7 @@ export async function parseDefinitions(
log.info(`Found ${packageNames.length} packages.`);
const typings: { [name: string]: TypingsVersionsRaw } = {};
const errors: string[] = []
const start = Date.now();
if (parallel) {
@@ -34,15 +35,30 @@ export async function parseDefinitions(
workerFile: definitionParserWorkerFilename,
nProcesses: parallel.nProcesses,
handleOutput({ data, packageName }: { data: TypingsVersionsRaw; packageName: string }) {
typings[packageName] = data;
if (Array.isArray(data)) {
errors.push(...data)
}
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 (Array.isArray(info)) {
errors.push(...info)
}
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));

View File

@@ -408,19 +408,19 @@ import route = require('@ember/routing/route');
const dt = createMockDT();
dt.addOldVersionOfPackage("jquery", "3", "3.0.0");
return expect(getTypingInfo("jquery", dt.fs)).rejects.toThrow(
return expect(getTypingInfo("jquery", dt.fs)).resolves.toEqual([
"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", "3.3.0");
return expect(getTypingInfo("jquery", dt.fs)).rejects.toThrow(
return expect(getTypingInfo("jquery", dt.fs)).resolves.toEqual([
"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", () => {

View File

@@ -112,7 +112,7 @@ function getNonNpm(args: { dtPath: string }): void {
for (const item of fs.readdirSync(dtTypesPath)) {
const packageJsonPath = path.join(dtTypesPath, item, "package.json");
const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, "utf8"));
let header= headerParser.validatePackageJson(item, packageJsonPath, packageJson, [])
let header= headerParser.validatePackageJson(item, packageJson, [])
if (!isNpmPackage(item, Array.isArray(header) ? undefined : header, isNpmJson)) {
nonNpm.push(item);
}

View File

@@ -129,7 +129,7 @@ If you want to check the declaration against the JavaScript source code, you mus
}
function parseDtHeader(packageName: string, packageJson: Record<string, unknown>): headerParser.Header | undefined {
const result = headerParser.validatePackageJson(packageName, "package.json", packageJson, []);
const result = headerParser.validatePackageJson(packageName, packageJson, []);
return Array.isArray(result) ? undefined : result;
}

View File

@@ -14,7 +14,7 @@ export function checkPackageJson(dirPath: string, typesVersions: readonly AllTyp
if (!pathExistsSync(pkgJsonPath)) {
throw new Error(`${dirPath}: Missing 'package.json'`);
}
return header.validatePackageJson(packageNameFromPath(dirPath), joinPaths(dirPath, "package.json"), readJson(pkgJsonPath), typesVersions);
return header.validatePackageJson(packageNameFromPath(dirPath), readJson(pkgJsonPath), typesVersions);
}
/**
* numbers in `CompilerOptions` might be enum values mapped from strings

View File

@@ -56,7 +56,7 @@ export function makeTypesVersionsForPackageJson(typesVersions: readonly AllTypeS
return out;
}
export function validatePackageJson(packageName: string, packageJsonPath: string, packageJson: Record<string, unknown>, typesVersions: readonly AllTypeScriptVersion[]): Header | string[] {
export function validatePackageJson(packageName: string, packageJson: Record<string, unknown>, typesVersions: readonly AllTypeScriptVersion[]): Header | string[] {
const errors = []
const needsTypesVersions = typesVersions.length !== 0;
// NOTE: I had to install eslint-plugin-import in DT because DT-tools hasn't shipped a new version
@@ -91,32 +91,32 @@ export function validatePackageJson(packageName: string, packageJsonPath: string
case "typesVersions":
case "types":
if (!needsTypesVersions) {
errors.push(`${packageJsonPath} doesn't need to set "${key}" when no 'ts4.x' directories exist.`);
errors.push(`${packageName}'s package.json doesn't need to set "${key}" when no 'ts4.x' directories exist.`);
}
break;
default:
errors.push(`${packageJsonPath} should not include property ${key}`);
errors.push(`${packageName}'s package.json should not include property ${key}`);
}
}
// private
if (packageJson.private !== true) {
errors.push(`${packageJsonPath} has bad "private": must be \`"private": true\``)
errors.push(`${packageName}'s package.json has bad "private": must be \`"private": true\``)
}
// devDependencies
if (typeof packageJson.devDependencies !== "object"
|| packageJson.devDependencies === null
|| (packageJson.devDependencies as any)["@types/" + packageName] !== "workspace:.") {
errors.push(`${packageJsonPath} has bad "devDependencies": must include \`"@types/${packageName}": "workspace:."\``);
errors.push(`${packageName}'s package.json has bad "devDependencies": must include \`"@types/${packageName}": "workspace:."\``);
}
// TODO: disallow devDeps from containing dependencies (although this is VERY linty)
// TODO: Check that devDeps are NOT used in .d.ts files
// typesVersions
if (needsTypesVersions) {
assert.strictEqual(packageJson.types, "index", `"types" in '${packageJsonPath}' should be "index".`);
assert.strictEqual(packageJson.types, "index", `"types" in '${packageName}'s package.json' should be "index".`);
const expected = makeTypesVersionsForPackageJson(typesVersions) as Record<string, object>;
if (!deepEquals(packageJson.typesVersions, expected)) {
errors.push(`'${packageJsonPath}' has bad "typesVersions". Should be: ${JSON.stringify(expected, undefined, 4)}`)
errors.push(`'${packageName}'s package.json' has bad "typesVersions". Should be: ${JSON.stringify(expected, undefined, 4)}`)
}
}
@@ -189,7 +189,7 @@ export function validatePackageJson(packageName: string, packageJsonPath: string
function validateName(): string | { errors: string[] } {
if (packageJson.name !== "@types/" + packageName) {
return { errors: [ `${packageJsonPath} should have \`"name": "@types/${packageName}"\``] };
return { errors: [ `${packageName}'s package.json should have \`"name": "@types/${packageName}"\``] };
}
else {
return packageName;
@@ -198,13 +198,13 @@ export function validatePackageJson(packageName: string, packageJsonPath: string
function validateVersion(): { major: number, minor: number } | {errors: string[]} {
const errors = []
if (!packageJson.version || typeof packageJson.version !== "string") {
errors.push(`${packageJsonPath} should have \`"version"\` matching the version of the implementation package.`);
errors.push(`${packageName}'s package.json should have \`"version"\` matching the version of the implementation package.`);
}
else if (!/\d+\.\d+\.\d+/.exec(packageJson.version)) {
errors.push(`${packageJsonPath} has bad "version": should look like "NN.NN.0"`);
errors.push(`${packageName}'s package.json has bad "version": should look like "NN.NN.0"`);
}
else if (!packageJson.version.endsWith(".0")) {
errors.push(`${packageJsonPath} has bad "version": must end with ".0"`);
errors.push(`${packageName}'s package.json has bad "version": must end with ".0"`);
}
else {
let version: "*" | { major: number, minor?: number } = "*"
@@ -218,7 +218,7 @@ export function validatePackageJson(packageName: string, packageJsonPath: string
return { major: version.major, minor: version.minor ?? 0 }
}
} catch (e: any) {
errors.push(`'${packageJsonPath}' has bad "version": Semver parsing failed with '${e.message}'`);
errors.push(`'${packageName}'s package.json' has bad "version": Semver parsing failed with '${e.message}'`);
}
}
return { errors }
@@ -227,13 +227,13 @@ export function validatePackageJson(packageName: string, packageJsonPath: string
const errors = []
if (packageJson.nonNpm != undefined) {
if (packageJson.nonNpm !== true) {
errors.push(`${packageJsonPath} has bad "nonNpm": must be true if present.`);
errors.push(`${packageName}'s package.json has bad "nonNpm": must be true if present.`);
}
else if (!packageJson.nonNpmDescription) {
errors.push(`${packageJsonPath} has missing "nonNpmDescription", which is required with "nonNpm": true.`);
errors.push(`${packageName}'s package.json has missing "nonNpmDescription", which is required with "nonNpm": true.`);
}
else if (typeof packageJson.nonNpmDescription !== "string") {
errors.push(`${packageJsonPath} has bad "nonNpmDescription": must be a string if present.`);
errors.push(`${packageName}'s package.json has bad "nonNpmDescription": must be a string if present.`);
}
else {
return true;
@@ -241,7 +241,7 @@ export function validatePackageJson(packageName: string, packageJsonPath: string
return { errors }
}
else if (packageJson.nonNpmDescription !== undefined) {
errors.push(`${packageJsonPath} has "nonNpmDescription" without "nonNpm": true.`);
errors.push(`${packageName}'s package.json has "nonNpmDescription" without "nonNpm": true.`);
}
if (errors.length) {
return { errors }
@@ -252,7 +252,7 @@ export function validatePackageJson(packageName: string, packageJsonPath: string
function validateTypeScriptVersion(): AllTypeScriptVersion | { errors: string[] } {
if (packageJson.typeScriptVersion) {
if ((typeof packageJson.typeScriptVersion !== "string" || !TypeScriptVersion.isTypeScriptVersion(packageJson.typeScriptVersion))) {
return { errors: [`${packageJsonPath} has bad "typeScriptVersion": if present, must be a MAJOR.MINOR semver string up to "${TypeScriptVersion.latest}".
return { errors: [`${packageName}'s package.json has bad "typeScriptVersion": if present, must be a MAJOR.MINOR semver string up to "${TypeScriptVersion.latest}".
(Defaults to "${TypeScriptVersion.lowest}" if not provided.)`] };
}
else {
@@ -264,10 +264,10 @@ export function validatePackageJson(packageName: string, packageJsonPath: string
function validateProjects(): string[] | { errors: string[] } {
const errors = []
if (!packageJson.projects || !Array.isArray(packageJson.projects) || !packageJson.projects.every(p => typeof p === "string")) {
errors.push(`${packageJsonPath} has bad "projects": must be an array of strings that point to the project web site(s).`);
errors.push(`${packageName}'s package.json has bad "projects": must be an array of strings that point to the project web site(s).`);
}
else if (packageJson.projects.length === 0) {
errors.push(`${packageJsonPath} has bad "projects": must have at least one project URL.`);
errors.push(`${packageName}'s package.json has bad "projects": must have at least one project URL.`);
}
else {
return packageJson.projects
@@ -277,13 +277,13 @@ export function validatePackageJson(packageName: string, packageJsonPath: string
function validateContributors(): Author[] | { errors: string[] } {
const errors: string[] = []
if (!packageJson.contributors || !Array.isArray(packageJson.contributors)) {
errors.push(`${packageJsonPath} has bad "contributors": must be an array of type Array<{ name: string, url: string, githubUsername: string}>.`);
errors.push(`${packageName}'s package.json has bad "contributors": must be an array of type Array<{ name: string, url: string, githubUsername: string}>.`);
}
else if (packageJson.contributors.length === 0) {
errors.push(`${packageJsonPath} has bad "contributors": must have at least one contributor.`);
errors.push(`${packageName}'s package.json has bad "contributors": must have at least one contributor.`);
}
else {
const es = checkPackageJsonContributors(packageJsonPath, packageJson.contributors);
const es = checkPackageJsonContributors(packageName, packageJson.contributors);
if (es.length) {
errors.push(...es)
}
@@ -295,32 +295,32 @@ export function validatePackageJson(packageName: string, packageJsonPath: string
}
}
function checkPackageJsonContributors(packageJsonPath: string, packageJsonContributors: readonly unknown[]) {
function checkPackageJsonContributors(packageName: string, packageJsonContributors: readonly unknown[]) {
const errors: string[] = []
for (const c of packageJsonContributors) {
if (typeof c !== "object" || c === null) {
errors.push(`${packageJsonPath} has bad "contributors": must be an array of type Array<{ name: string, url: string } | { name: string, githubUsername: string}>.`)
errors.push(`${packageName}'s package.json has bad "contributors": must be an array of type Array<{ name: string, url: string } | { name: string, githubUsername: string}>.`)
continue
}
if (!("name" in c) || typeof c.name !== "string") {
errors.push(`${packageJsonPath} has bad "name" in contributor ${JSON.stringify(c)}
errors.push(`${packageName}'s package.json has bad "name" in contributor ${JSON.stringify(c)}
Must be an object of type { name: string, url: string } | { name: string, githubUsername: string}.`)
}
else if (c.name === "My Self") {
errors.push(`${packageJsonPath} has bad "name" in contributor ${JSON.stringify(c)}
errors.push(`${packageName}'s package.json has bad "name" in contributor ${JSON.stringify(c)}
Author name should be your name, not the default.`)
}
if ("githubUsername" in c) {
if (typeof c.githubUsername !== "string") {
errors.push(`${packageJsonPath} has bad "githubUsername" in contributor ${JSON.stringify(c)}
errors.push(`${packageName}'s package.json has bad "githubUsername" in contributor ${JSON.stringify(c)}
Must be an object of type { name: string, url: string } | { name: string, githubUsername: string}.`)
}
else if ("url" in c) {
errors.push(`${packageJsonPath} has bad contributor: should not have both "githubUsername" and "url" properties in contributor ${JSON.stringify(c)}`)
errors.push(`${packageName}'s package.json has bad contributor: should not have both "githubUsername" and "url" properties in contributor ${JSON.stringify(c)}`)
}
}
else if ("url" in c && typeof c.url !== "string") {
errors.push(`${packageJsonPath} has bad "url" in contributor ${JSON.stringify(c)}
errors.push(`${packageName}'s package.json has bad "url" in contributor ${JSON.stringify(c)}
Must be an object of type { name: string, url: string } | { name: string, githubUsername: string}.`)
}
for (const key in c) {
@@ -330,7 +330,7 @@ Must be an object of type { name: string, url: string } | { name: string, github
case "githubUsername":
break;
default:
errors.push(`${packageJsonPath} has bad contributor: should not include property ${key} in ${JSON.stringify(c)}`);
errors.push(`${packageName}'s package.json has bad contributor: should not include property ${key} in ${JSON.stringify(c)}`);
}
}
}

View File

@@ -45,58 +45,58 @@ describe("validatePackageJson", () => {
it("requires private: true", () => {
const pkg = { ...pkgJson };
delete pkg.private;
expect(validatePackageJson("hapi", "cort-start/hapi/package.json", pkg, [])).toEqual([
`cort-start/hapi/package.json has bad "private": must be \`"private": true\``
expect(validatePackageJson("hapi", pkg, [])).toEqual([
`hapi's package.json has bad "private": must be \`"private": true\``
]);
});
it("requires name", () => {
const pkg = { ...pkgJson };
delete pkg.name;
expect(validatePackageJson("hapi", "cort-start/hapi/package.json", pkg, [])).toEqual([
"cort-start/hapi/package.json should have `\"name\": \"@types/hapi\"`"
expect(validatePackageJson("hapi", pkg, [])).toEqual([
"hapi's package.json should have `\"name\": \"@types/hapi\"`"
]);
});
it("requires name to match", () => {
expect(validatePackageJson("hapi", "cort-start/hapi/package.json", { ...pkgJson, name: "@types/sad" }, [])).toEqual([
"cort-start/hapi/package.json should have `\"name\": \"@types/hapi\"`"
expect(validatePackageJson("hapi", { ...pkgJson, name: "@types/sad" }, [])).toEqual([
"hapi's package.json should have `\"name\": \"@types/hapi\"`"
]);
});
it("requires devDependencies", () => {
const pkg = { ...pkgJson };
delete pkg.devDependencies;
expect(validatePackageJson("hapi", "cort-start/hapi/package.json", pkg, [])).toEqual([
`cort-start/hapi/package.json has bad "devDependencies": must include \`"@types/hapi": "workspace:."\``
expect(validatePackageJson("hapi", pkg, [])).toEqual([
`hapi's package.json has bad "devDependencies": must include \`"@types/hapi": "workspace:."\``
]);
});
it("requires devDependencies to contain self-package", () => {
expect(validatePackageJson("hapi", "cort-start/hapi/package.json", { ...pkgJson, devDependencies: { } }, [])).toEqual([
`cort-start/hapi/package.json has bad "devDependencies": must include \`"@types/hapi": "workspace:."\``
expect(validatePackageJson("hapi", { ...pkgJson, devDependencies: { } }, [])).toEqual([
`hapi's package.json has bad "devDependencies": must include \`"@types/hapi": "workspace:."\``
]);
});
it("requires devDependencies to contain self-package version 'workspace:.'", () => {
expect(validatePackageJson("hapi", "cort-start/hapi/package.json", { ...pkgJson, devDependencies: { "@types/hapi": "*" } }, [])).toEqual([
`cort-start/hapi/package.json has bad "devDependencies": must include \`"@types/hapi": "workspace:."\``
expect(validatePackageJson("hapi", { ...pkgJson, devDependencies: { "@types/hapi": "*" } }, [])).toEqual([
`hapi's package.json has bad "devDependencies": must include \`"@types/hapi": "workspace:."\``
]);
});
it("requires version", () => {
const pkg = { ...pkgJson };
delete pkg.version;
expect(validatePackageJson("hapi", "cort-start/hapi/package.json", pkg, [])).toEqual([
`cort-start/hapi/package.json should have \`"version"\` matching the version of the implementation package.`
expect(validatePackageJson("hapi", pkg, [])).toEqual([
`hapi's package.json should have \`"version"\` matching the version of the implementation package.`
]);
});
it("requires version to be NN.NN.NN", () => {
expect(validatePackageJson("hapi", "cort-start/hapi/package.json", { ...pkgJson, version: "hi there" }, [])).toEqual([
`cort-start/hapi/package.json has bad "version": should look like "NN.NN.0"`
expect(validatePackageJson("hapi", { ...pkgJson, version: "hi there" }, [])).toEqual([
`hapi's package.json has bad "version": should look like "NN.NN.0"`
]);
});
it("requires version to end with .0", () => {
expect(validatePackageJson("hapi", "cort-start/hapi/package.json", { ...pkgJson, version: "1.2.3" }, [])).toEqual([
`cort-start/hapi/package.json has bad "version": must end with ".0"`
expect(validatePackageJson("hapi", { ...pkgJson, version: "1.2.3" }, [])).toEqual([
`hapi's package.json has bad "version": must end with ".0"`
]);
});
it("works with old-version packages", () => {
expect(Array.isArray(validatePackageJson("hapi", "cort-start/hapi/package.json", { ...pkgJson, version: "16.6.0" }, []))).toBeFalsy();
expect(Array.isArray(validatePackageJson("hapi", { ...pkgJson, version: "16.6.0" }, []))).toBeFalsy();
})
})
describe("makeTypesVersionsForPackageJson", () => {