casraf/DefinitelyTyped-tools
1
0
Fork 0
mirror of https://github.com/chenasraf/DefinitelyTyped-tools.git synced 2026-05-18 01:49:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

WIP dtslint checks

Browse Source 
I'm at the point of checking `version` and I think I need to copy code
from definition-parser maybe. Visions of semver parsing...
This commit is contained in:
Nathan Shively-Sanders
2023-03-30 15:51:03 -07:00
parent b144b978c6
commit 689ccfcb17
5 changed files with 173 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files

85
packages/dtslint/src/checks.ts
View File

@@ -1,38 +1,48 @@
import { makeTypesVersionsForPackageJson } from "@definitelytyped/header-parser";
import { TypeScriptVersion } from "@definitelytyped/typescript-versions";
import assert = require("assert");
import { pathExists } from "fs-extra";
import { pathExistsSync } from "fs-extra";
import { join as joinPaths } from "path";
import { CompilerOptions } from "typescript";
import { readJson } from "./util";
export async function checkPackageJson(dirPath: string, typesVersions: readonly TypeScriptVersion[]): Promise<void> {
import { readJson, packageNameFromPath } from "./util";
export function checkPackageJson(dirPath: string, typesVersions: readonly TypeScriptVersion[]): 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;
if (!pathExistsSync(pkgJsonPath)) {
throw new Error(`${dirPath}: Missing 'package.json'`);
}
return checkPackageJsonContents(dirPath, readJson(pkgJsonPath), typesVersions);
}
const pkgJson = (await readJson(pkgJsonPath)) as Record<string, unknown>;
if ((pkgJson as any).private !== true) {
throw new Error(`${pkgJsonPath} should set \`"private": true\``);
export function checkPackageJsonContents(dirPath: string, pkgJson: Record<string, unknown>, typesVersions: readonly TypeScriptVersion[]): string[] {
const errors = []
const pkgJsonPath = joinPaths(dirPath, "package.json");
const packageName = packageNameFromPath(dirPath);
const needsTypesVersions = typesVersions.length !== 0;
if (pkgJson.private !== true) {
errors.push(`${pkgJsonPath} should have \`"private": true\``)
}
if (pkgJson.name !== "@types/" + packageName) {
errors.push(`${pkgJsonPath} should have \`"name": "@types/${packageName}"\``);
}
if (typeof pkgJson.devDependencies !== "object"
|| pkgJson.devDependencies === null
|| (pkgJson.devDependencies as any)["@types/" + packageName] !== "link:.") {
errors.push(`In ${pkgJsonPath}, devDependencies must include \`"@types/${packageName}": "link:."\``);
}
if (needsTypesVersions) {
assert.strictEqual((pkgJson as any).types, "index", `"types" in '${pkgJsonPath}' should be "index".`);
assert.strictEqual(pkgJson.types, "index", `"types" in '${pkgJsonPath}' should be "index".`);
const expected = makeTypesVersionsForPackageJson(typesVersions) as Record<string, object>;
assert.deepEqual(
(pkgJson as any).typesVersions,
expected,
`"typesVersions" in '${pkgJsonPath}' is not set right. Should be: ${JSON.stringify(expected, undefined, 4)}`
);
if (!deepEquals(pkgJson.typesVersions, expected)) {
errors.push(`"typesVersions" in '${pkgJsonPath}' is not set right. Should be: ${JSON.stringify(expected, undefined, 4)}`)
}
}
// TODO: This needs to be much longer
// TODO: Add checks for name and the other new field(s)
// TODO: Should they be disablable, in lint rules? I mean, if we have the data right now, the answer is NO
// TODO: If these aren't going to be lint rules, they should collect errors instead of throwing. Fixing one thing at a time sucks.
// TODO: Test on a toplevel package, a scoped package, and old-version package, and an old-TS-version package
for (const key in pkgJson) {
// tslint:disable-line forin
switch (key) {
@@ -44,19 +54,21 @@ export async function checkPackageJson(dirPath: string, typesVersions: readonly
case "type":
case "name":
case "version":
case "devDependencies":
// "private"/"typesVersions"/"types" checked above, "dependencies" / "license" checked by types-publisher,
// TODO: "name"/"version" checked by types-publisher/CI.
// TODO: "name"/"version" checked above, plus asserts in 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.`);
errors.push(`${pkgJsonPath} doesn't need to set "${key}" when no 'ts3.x' directories exist.`);
}
break;
default:
throw new Error(`${pkgJsonPath} should not include field ${key}`);
errors.push(`${pkgJsonPath} should not include field ${key}`);
}
}
return errors
}
/**
@@ -72,7 +84,9 @@ export interface DefinitelyTypedInfo {
/** "../" or "../../" or "../../../". This should use '/' even on windows. */
readonly relativeBaseUrl: string;
}
export function checkTsconfig(options: CompilerOptionsRaw, dt: boolean): void {
// TODO: Maybe check ALL of tsconfig, not just compilerOptions
export function checkTsconfig(options: CompilerOptionsRaw, dt: boolean): string[] {
const errors = []
if (dt) {
const mustHave = {
noEmit: true,
@@ -84,7 +98,7 @@ export function checkTsconfig(options: CompilerOptionsRaw, dt: boolean): void {
const expected = mustHave[key];
const actual = options[key];
if (!deepEquals(expected, actual)) {
throw new Error(
errors.push(
`Expected compilerOptions[${JSON.stringify(key)}] === ${JSON.stringify(expected)}, but got ${JSON.stringify(
actual
)}`
@@ -103,7 +117,6 @@ export function checkTsconfig(options: CompilerOptionsRaw, dt: boolean): void {
case "strictFunctionTypes":
case "esModuleInterop":
case "allowSyntheticDefaultImports":
case "paths":
case "target":
case "jsx":
case "jsxFactory":
@@ -115,29 +128,28 @@ export function checkTsconfig(options: CompilerOptionsRaw, dt: boolean): void {
break;
default:
if (!(key in mustHave)) {
throw new Error(`Unexpected compiler option ${key}`);
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"`.');
errors.push('Must specify "module" to `"module": "commonjs"` or `"module": "node16"`.');
}
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"]) {
@@ -148,22 +160,23 @@ export function checkTsconfig(options: CompilerOptionsRaw, dt: boolean): void {
} 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 `/// <reference types="..." />` directives in source files and ensure ' +
'that the "types" field in your tsconfig is an empty array.'
);
}
return errors;
}
function deepEquals(expected: unknown, actual: unknown): boolean {

17
packages/dtslint/src/index.ts
View File

@@ -9,7 +9,7 @@ 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, mapDefinedAsync, packageNameFromPath, withoutPrefix } from "./util";
async function main(): Promise<void> {
const args = process.argv.slice(2);
@@ -132,12 +132,13 @@ async function runTests(
): Promise<void> {
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.
// TODO: We really shouldn't be running off DT anymore.
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);
const packageName = packageNameFromPath(dirPath);
assertPathIsInDefinitelyTyped(dirPath, dtRoot);
assertPathIsNotBanned(packageName);
assertPackageIsNotDeprecated(packageName, await readFile(joinPaths(dtRoot, "notNeededPackages.json"), "utf-8"));
@@ -162,7 +163,10 @@ async function runTests(
});
if (dt) {
await checkPackageJson(dirPath, typesVersions);
const packageJsonErrors = checkPackageJson(dirPath, typesVersions);
if (packageJsonErrors.length > 0) {
throw new Error(packageJsonErrors.join("\n"))
}
}
const minVersion = maxVersion(
@@ -222,8 +226,11 @@ async function testTypesVersion(
tsLocal: string | undefined,
isLatest: boolean
): Promise<void> {
await checkTslintJson(dirPath, dt);
checkTsconfig(await getCompilerOptions(dirPath), dt);
checkTslintJson(dirPath, dt);
const tsconfigErrors = checkTsconfig(getCompilerOptions(dirPath), dt);
if (tsconfigErrors.length > 0) {
throw new Error(tsconfigErrors.join("\n"))
}
const err = await lint(dirPath, lowVersion, hiVersion, isLatest, expectOnly, tsLocal);
if (err) {
throw new Error(err);

18
packages/dtslint/src/lint.ts
View File

@@ -1,7 +1,7 @@
import { TypeScriptVersion } from "@definitelytyped/typescript-versions";
import { typeScriptPath } from "@definitelytyped/utils";
import assert = require("assert");
import { pathExists } from "fs-extra";
import { pathExistsSync } from "fs-extra";
import { dirname, join as joinPaths, normalize } from "path";
import { Configuration, Linter } from "tslint";
import { ESLint } from "eslint";
@@ -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()) {
@@ -190,13 +190,13 @@ function testNoLintDisables(disabler: "tslint:disable" | "eslint-disable", text:
}
}
export async function checkTslintJson(dirPath: string, dt: boolean): Promise<void> {
export function checkTslintJson(dirPath: string, dt: boolean): 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 (!pathExistsSync(configPath)) {
if (dt) {
throw new Error(
`On DefinitelyTyped, must include \`tslint.json\` containing \`{ "extends": "${shouldExtend}" }\`.\n` +
@@ -206,8 +206,8 @@ export async function checkTslintJson(dirPath: string, dt: boolean): Promise<voi
return;
}
const tslintJson = await readJson(configPath);
if (!validateExtends(tslintJson.extends)) {
const tslintJson = readJson(configPath);
if (!validateExtends(tslintJson.extends as any)) {
throw new Error(`If 'tslint.json' is present, it should extend "${shouldExtend}"`);
}
}
@@ -216,14 +216,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<IConfigurationFile> {
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;

16
packages/dtslint/src/util.ts
View File

@@ -1,6 +1,6 @@
import { ESLintUtils } from "@typescript-eslint/utils";
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";
@@ -9,9 +9,11 @@ export const createRule = ESLintUtils.RuleCreator(
(name) => `https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/dtslint/src/rules/${name}.ts`
);
export async function readJson(path: string) {
const text = await readFile(path, "utf-8");
return JSON.parse(stripJsonComments(text));
export function packageNameFromPath(path: string): string {
return /^v\d+(\.\d+)?$/.exec(path) || /^ts\d\.\d/.exec(path) ? basename(dirname(path)) : basename(path)
}
export function readJson(path: string): Record<string, unknown> {
return JSON.parse(stripJsonComments(readFileSync(path, "utf-8")));
}
export function failure(ruleName: string, s: string): string {
@@ -31,12 +33,12 @@ export function getCommonDirectoryName(files: readonly string[]): string {
return basename(minDir);
}
export async function getCompilerOptions(dirPath: string): Promise<ts.CompilerOptions> {
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;
return readJson(tsconfigPath).compilerOptions as ts.CompilerOptions;
}
export function withoutPrefix(s: string, prefix: string): string | undefined {

125
packages/dtslint/test/index.test.ts
View File

@@ -2,7 +2,7 @@
import { join } from "path";
import { consoleTestResultHandler, runTest } from "tslint/lib/test";
import { existsSync, readdirSync, statSync } from "fs";
import { CompilerOptionsRaw, checkTsconfig } from "../src/checks";
import { CompilerOptionsRaw, checkPackageJsonContents, checkTsconfig } from "../src/checks";
import { assertPackageIsNotDeprecated } from "../src/index";
const testDir = __dirname;
@@ -37,43 +37,106 @@ describe("dtslint", () => {
noImplicitThis: true,
strictNullChecks: true,
strictFunctionTypes: true,
baseUrl: "../",
typeRoots: ["../"],
types: [],
noEmit: true,
forceConsistentCasingInFileNames: true,
};
const pkgJson: Record<string, unknown> = {
"private": true,
"name": "@types/hapi",
"version": "18.0.0",
"dependencies": {
"@types/boom": "*",
"@types/catbox": "*",
"@types/iron": "*",
"@types/mimos": "*",
"@types/node": "*",
"@types/podium": "*",
"@types/shot": "*",
"joi": "^17.3.0"
},
"devDependencies": {
"@types/hapi": "link:."
}
}
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({ ...base, completelyInvented: true }, true)).toEqual([
"Unexpected compiler option completelyInvented"
]);
});
it("allows exactOptionalPropertyTypes: true", () => {
expect(checkTsconfig({ ...base, exactOptionalPropertyTypes: true }, true)).toEqual([]);
});
it("allows module: node16", () => {
expect(checkTsconfig({ ...base, module: "node16" }, true)).toEqual([]);
});
it("disallows missing `module`", () => {
const options = { ...base };
delete options.module;
expect(checkTsconfig(options, true)).toEqual([
'Must specify "module" to `"module": "commonjs"` or `"module": "node16"`.'
]);
});
it("disallows exactOptionalPropertyTypes: false", () => {
expect(checkTsconfig({ ...base, exactOptionalPropertyTypes: false }, true)).toEqual([
'When "exactOptionalPropertyTypes" is present, it must be set to `true`.'
]);
});
it("disallows `paths`", () => {
expect(checkTsconfig({ ...base, paths: { "c": ['.'] } }, true)).toEqual([
'Unexpected compiler option paths'
]);
});
});
it("allows exactOptionalPropertyTypes: true", () => {
expect(checkTsconfig({ ...base, exactOptionalPropertyTypes: true }, { relativeBaseUrl: "../" })).toBeFalsy();
describe("checkPackageJson", () => {
it("requires private: true", () => {
const pkg = { ...pkgJson };
delete pkg.private;
expect(checkPackageJsonContents("cort-start/hapi", pkg, [])).toEqual([
"cort-start/hapi/package.json should have `\"private\": true`"
]);
});
it("requires name", () => {
const pkg = { ...pkgJson };
delete pkg.name;
expect(checkPackageJsonContents("cort-start/basis", pkg, [])).toEqual([
"cort-start/basis/package.json should have `\"name\": \"@types/basis\"`"
]);
});
it("requires name to match", () => {
expect(checkPackageJsonContents("cort-start/basis", pkgJson, [])).toEqual([
"cort-start/basis/package.json should have `\"name\": \"@types/basis\"`"
]);
});
it("requires devDependencies", () => {
const pkg = { ...pkgJson };
delete pkg.devDependencies;
expect(checkPackageJsonContents("cort-start/hapi", pkg, [])).toEqual([
`In cort-start/hapi/package.json, devDependencies must include \`"@types/hapi": "link:."\``
]);
});
it("requires devDependencies to contain self-package", () => {
expect(checkPackageJsonContents("cort-start/hapi", { ...pkgJson, devDependencies: { } }, [])).toEqual([
`In cort-start/hapi/package.json, devDependencies must include \`"@types/hapi": "link:."\``
]);
});
it("requires devDependencies to contain self-package version 'link:.'", () => {
expect(checkPackageJsonContents("cort-start/hapi", { ...pkgJson, devDependencies: { "@types/hapi": "*" } }, [])).toEqual([
`In cort-start/hapi/package.json, devDependencies must include \`"@types/hapi": "link:."\``
]);
});
});
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", () => {