diff --git a/.changeset/chatty-jobs-pull.md b/.changeset/chatty-jobs-pull.md new file mode 100644 index 0000000000..c4990d31d2 --- /dev/null +++ b/.changeset/chatty-jobs-pull.md @@ -0,0 +1,5 @@ +--- +"@definitelytyped/dtslint": patch +--- + +Rework error collection diff --git a/.changeset/fifty-points-eat.md b/.changeset/fifty-points-eat.md new file mode 100644 index 0000000000..4f81f74b55 --- /dev/null +++ b/.changeset/fifty-points-eat.md @@ -0,0 +1,5 @@ +--- +"@definitelytyped/dtslint": patch +--- + +Error if tslint.json is present diff --git a/packages/dtslint/src/checks.ts b/packages/dtslint/src/checks.ts index 8334b981dd..534631a7a4 100644 --- a/packages/dtslint/src/checks.ts +++ b/packages/dtslint/src/checks.ts @@ -165,11 +165,11 @@ export async function runAreTheTypesWrong( configPath: string, expectError: boolean, ): Promise<{ - warnings?: string[]; - errors?: string[]; + warnings: string[]; + errors: string[]; }> { - let warnings: string[] | undefined; - let errors: string[] | undefined; + const warnings = []; + const errors = []; let result: { status: "pass" | "fail" | "error"; output: string; @@ -207,12 +207,12 @@ export async function runAreTheTypesWrong( break; case "fail": // Show output without failing the build. - (warnings ??= []).push( + warnings.push( `Ignoring attw failure because "${dirName}" is listed in 'failingPackages'.\n\n@arethetypeswrong/cli\n${output}`, ); break; case "pass": - (errors ??= []).push(`attw passed: remove "${dirName}" from 'failingPackages' in attw.json\n\n${output}`); + errors.push(`attw passed: remove "${dirName}" from 'failingPackages' in attw.json\n\n${output}`); break; default: assertNever(status); @@ -221,7 +221,7 @@ export async function runAreTheTypesWrong( switch (status) { case "error": case "fail": - (errors ??= []).push(`!@arethetypeswrong/cli\n${output}`); + errors.push(`!@arethetypeswrong/cli\n${output}`); break; case "pass": // Don't show anything for passing attw - most lint rules have no output on success. @@ -236,12 +236,12 @@ export async function checkNpmVersionAndGetMatchingImplementationPackage( packageJson: header.Header, packageDirectoryNameWithVersion: string, ): Promise<{ - warnings?: string[]; - errors?: string[]; + warnings: string[]; + errors: string[]; implementationPackage?: attw.Package; }> { - let warnings: string[] | undefined; - let errors: string[] | undefined; + const warnings: string[] = []; + const errors: string[] = []; let hasNpmVersionMismatch = false; let implementationPackage; const attw = await import("@arethetypeswrong/core"); @@ -252,7 +252,7 @@ export async function checkNpmVersionAndGetMatchingImplementationPackage( if (packageId) { const { packageName, packageVersion, tarballUrl } = packageId; if (packageJson.nonNpm === true) { - (errors ??= []).push( + errors.push( `Package ${packageJson.name} is marked as non-npm, but ${packageName} exists on npm. ` + `If these types are being added to DefinitelyTyped for the first time, please choose ` + `a different name that does not conflict with an existing npm package.`, @@ -261,7 +261,7 @@ export async function checkNpmVersionAndGetMatchingImplementationPackage( if (!satisfies(packageVersion, typesPackageVersion)) { hasNpmVersionMismatch = true; const isError = !npmVersionExemptions.has(packageDirectoryNameWithVersion); - const container = isError ? (errors ??= []) : (warnings ??= []); + const container = isError ? errors : warnings; container.push( (isError ? "" @@ -278,7 +278,7 @@ export async function checkNpmVersionAndGetMatchingImplementationPackage( try { implementationPackage = await attw.createPackageFromTarballUrl(tarballUrl); } catch (err: any) { - (warnings ??= []).push( + warnings.push( `Failed to extract implementation package from ${tarballUrl}. This is likely a problem with @arethetypeswrong/core ` + `or the tarball data itself. @arethetypeswrong/cli will not run. Error:\n${err.stack ?? err.message}`, ); @@ -286,12 +286,12 @@ export async function checkNpmVersionAndGetMatchingImplementationPackage( } } } else if (packageJson.nonNpm === "conflict") { - (errors ??= []).push( + errors.push( `Package ${packageJson.name} is marked as \`"nonNpm": "conflict"\`, but no conflicting package name was ` + `found on npm. These non-npm types can be makred as \`"nonNpm": true\` instead.`, ); } else if (!packageJson.nonNpm) { - (errors ??= []).push( + errors.push( `Package ${packageJson.name} is not marked as non-npm, but no implementation package was found on npm. ` + `If these types are not for an npm package, please add \`"nonNpm": true\` to the package.json. ` + `Otherwise, ensure the name of this package matches the name of the npm package.`, @@ -299,7 +299,7 @@ export async function checkNpmVersionAndGetMatchingImplementationPackage( } if (!hasNpmVersionMismatch && npmVersionExemptions.has(packageDirectoryNameWithVersion)) { - (warnings ??= []).push( + warnings.push( `${packageDirectoryNameWithVersion} can be removed from expectedNpmVersionFailures.txt in https://github.com/microsoft/DefinitelyTyped-tools/blob/main/packages/dtslint.`, ); } diff --git a/packages/dtslint/src/index.ts b/packages/dtslint/src/index.ts index 8b567890e8..63e60f696a 100644 --- a/packages/dtslint/src/index.ts +++ b/packages/dtslint/src/index.ts @@ -156,7 +156,7 @@ async function runTests( expectOnly: boolean, npmChecks: boolean | "only", tsLocal: string | undefined, -): Promise { +): Promise { // Assert that we're really on DefinitelyTyped. const dtRoot = findDTRoot(dirPath); const packageName = packageNameFromPath(dirPath); @@ -174,25 +174,34 @@ async function runTests( throw new Error("\n\t* " + packageJson.join("\n\t* ")); } - await assertNpmIgnoreExpected(dirPath); - assertNoOtherFiles(dirPath); + const warnings: string[] = []; + const errors: string[] = []; let implementationPackage; - let warnings: string[] | undefined; - let errors: string[] | undefined; - if (npmChecks) { - ({ implementationPackage, warnings, errors } = await checkNpmVersionAndGetMatchingImplementationPackage( + const result = await checkNpmVersionAndGetMatchingImplementationPackage( packageJson, packageDirectoryNameWithVersion, - )); + ); + + implementationPackage = result.implementationPackage; + warnings.push(...result.warnings); + errors.push(...result.errors); } if (npmChecks !== "only") { const minVersion = maxVersion(packageJson.minimumTypeScriptVersion, TypeScriptVersion.lowest); if (onlyTestTsNext || tsLocal) { const tsVersion = tsLocal ? "local" : TypeScriptVersion.latest; - await testTypesVersion(dirPath, tsVersion, tsVersion, expectOnly, tsLocal, /*isLatest*/ true); + const testTypesResult = await testTypesVersion( + dirPath, + tsVersion, + tsVersion, + expectOnly, + tsLocal, + /*isLatest*/ true, + ); + errors.push(...testTypesResult.errors); } else { // For example, typesVersions of [3.2, 3.5, 3.6] will have // associated ts3.2, ts3.5, ts3.6 directories, for @@ -213,7 +222,8 @@ async function runTests( if (lows.length > 1) { console.log("testing from", low, "to", hi, "in", versionPath); } - await testTypesVersion(versionPath, low, hi, expectOnly, undefined, isLatest); + const testTypesResult = await testTypesVersion(versionPath, low, hi, expectOnly, undefined, isLatest); + errors.push(...testTypesResult.errors); } } } @@ -224,8 +234,8 @@ async function runTests( const dirName = dirPath.slice(dtRoot.length + "/types/".length); const expectError = !!failingPackages?.includes(dirName); const attwResult = await runAreTheTypesWrong(dirName, dirPath, implementationPackage, attwJson, expectError); - (warnings ??= []).push(...(attwResult.warnings ?? [])); - (errors ??= []).push(...(attwResult.errors ?? [])); + warnings.push(...attwResult.warnings); + errors.push(...attwResult.errors); } const result = combineErrorsAndWarnings(errors, warnings); @@ -235,15 +245,9 @@ async function runTests( return result; } -function combineErrorsAndWarnings( - errors: string[] | undefined, - warnings: string[] | undefined, -): Error | string | undefined { - if (!errors && !warnings) { - return undefined; - } - const message = (errors ?? []).concat(warnings ?? []).join("\n\n"); - return errors?.length ? new Error(message) : message; +function combineErrorsAndWarnings(errors: string[], warnings: string[]): Error | string { + const message = errors.concat(warnings).join("\n\n"); + return errors.length ? new Error(message) : message; } function maxVersion(v1: AllTypeScriptVersion, v2: TypeScriptVersion): TypeScriptVersion { @@ -265,16 +269,19 @@ async function testTypesVersion( expectOnly: boolean, tsLocal: string | undefined, isLatest: boolean, -): Promise { - assertIndexdts(dirPath); +): Promise<{ errors: string[] }> { + const errors = []; + const checkExpectedFilesResult = checkExpectedFiles(dirPath, isLatest); + errors.push(...checkExpectedFilesResult.errors); const tsconfigErrors = checkTsconfig(dirPath, getCompilerOptions(dirPath)); if (tsconfigErrors.length > 0) { - throw new Error("\n\t* " + tsconfigErrors.join("\n\t* ")); + errors.push("\n\t* " + tsconfigErrors.join("\n\t* ")); } const err = await lint(dirPath, lowVersion, hiVersion, isLatest, expectOnly, tsLocal); if (err) { - throw new Error(err); + errors.push(err); } + return { errors }; } function findDTRoot(dirPath: string) { @@ -336,42 +343,47 @@ if (require.main === module) { }); } -async function assertNpmIgnoreExpected(dirPath: string) { - const expected = ["*", "!**/*.d.ts", "!**/*.d.cts", "!**/*.d.mts", "!**/*.d.*.ts"]; +function checkExpectedFiles(dirPath: string, isLatest: boolean): { errors: string[] } { + const errors = []; - if (basename(dirname(dirPath)) === "types") { - for (const subdir of fs.readdirSync(dirPath, { withFileTypes: true })) { - if (subdir.isDirectory() && /^v(\d+)(\.(\d+))?$/.test(subdir.name)) { - expected.push(`/${subdir.name}/`); + if (isLatest) { + const expectedNpmIgnore = ["*", "!**/*.d.ts", "!**/*.d.cts", "!**/*.d.mts", "!**/*.d.*.ts"]; + + if (basename(dirname(dirPath)) === "types") { + for (const subdir of fs.readdirSync(dirPath, { withFileTypes: true })) { + if (subdir.isDirectory() && /^v(\d+)(\.(\d+))?$/.test(subdir.name)) { + expectedNpmIgnore.push(`/${subdir.name}/`); + } } } - } - const expectedString = expected.join("\n"); + const expectedNpmIgnoreAsString = expectedNpmIgnore.join("\n"); + const npmIgnorePath = joinPaths(dirPath, ".npmignore"); + if (!fs.existsSync(npmIgnorePath)) { + errors.push(`${dirPath}: Missing '.npmignore'; should contain:\n${expectedNpmIgnoreAsString}`); + } - const npmIgnorePath = joinPaths(dirPath, ".npmignore"); - if (!fs.existsSync(npmIgnorePath)) { - throw new Error(`${dirPath}: Missing '.npmignore'; should contain:\n${expectedString}`); - } + const actualNpmIgnore = fs.readFileSync(npmIgnorePath, "utf-8").trim().split(/\r?\n/); + if (!deepEquals(actualNpmIgnore, expectedNpmIgnore)) { + errors.push(`${dirPath}: Incorrect '.npmignore'; should be:\n${expectedNpmIgnoreAsString}`); + } - const actualRaw = await fs.promises.readFile(npmIgnorePath, "utf-8"); - const actual = actualRaw.trim().split(/\r?\n/); + if (fs.existsSync(joinPaths(dirPath, "OTHER_FILES.txt"))) { + errors.push( + `${dirPath}: Should not contain 'OTHER_FILES.txt'. All files matching "**/*.d.{ts,cts,mts,*.ts}" are automatically included.`, + ); + } + } - if (!deepEquals(actual, expected)) { - throw new Error(`${dirPath}: Incorrect '.npmignore'; should be:\n${expectedString}`); + if (!fs.existsSync(joinPaths(dirPath, "index.d.ts"))) { + errors.push(`${dirPath}: Must contain 'index.d.ts'.`); } -} -function assertNoOtherFiles(dirPath: string) { - if (fs.existsSync(joinPaths(dirPath, "OTHER_FILES.txt"))) { - throw new Error( - `${dirPath}: Should not contain 'OTHER_FILES.txt'. All files matching "**/*.d.{ts,cts,mts,*.ts}" are automatically included.`, + if (fs.existsSync(joinPaths(dirPath, "tslint.json"))) { + errors.push( + `${dirPath}: Should not contain 'tslint.json'. This file is no longer required; place all lint-related options into .eslintrc.json.`, ); } -} -function assertIndexdts(dirPath: string) { - if (!fs.existsSync(joinPaths(dirPath, "index.d.ts"))) { - throw new Error(`${dirPath}: Must contain 'index.d.ts'.`); - } + return { errors }; }