From fb4ca4b9586e741a3f70dfaff607a8a5b99f0724 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 8 Dec 2023 00:23:10 -0500 Subject: [PATCH 01/18] Converted npm-naming from TSLint to ESLint --- packages/dts-critic/dt.ts | 28 +- packages/dtslint/dt.json | 5 +- packages/dtslint/dtslint.json | 3 +- packages/dtslint/src/rules/npmNamingRule.ts | 311 ------------------ packages/dtslint/src/updateConfig.ts | 18 +- packages/dtslint/src/util.ts | 21 -- .../dtslint/test/npm-naming/code/tslint.json | 6 - .../code/types/dts-critic/index.d.ts | 1 - .../code/types/dts-critic/index.d.ts.lint | 8 - .../code/types/dts-critic/package.json | 17 - .../dtslint/test/npm-naming/name/tslint.json | 6 - .../name/types/wenceslas/index.d.ts | 1 - .../name/types/wenceslas/index.d.ts.lint | 2 - .../name/types/wenceslas/package.json | 17 - packages/dtslint/test/tsconfig.json | 2 +- .../docs/rules}/npm-naming.md | 0 packages/eslint-plugin/package.json | 2 + packages/eslint-plugin/src/rules/index.ts | 2 + .../eslint-plugin/src/rules/npm-naming.ts | 181 ++++++++++ .../src/rules/npm-naming/types.ts | 14 + packages/eslint-plugin/src/util.ts | 21 ++ .../eslint-plugin/test/npm-naming.test.ts | 55 ++++ pnpm-lock.yaml | 25 ++ 23 files changed, 321 insertions(+), 425 deletions(-) delete mode 100644 packages/dtslint/src/rules/npmNamingRule.ts delete mode 100644 packages/dtslint/test/npm-naming/code/tslint.json delete mode 100644 packages/dtslint/test/npm-naming/code/types/dts-critic/index.d.ts delete mode 100644 packages/dtslint/test/npm-naming/code/types/dts-critic/index.d.ts.lint delete mode 100644 packages/dtslint/test/npm-naming/code/types/dts-critic/package.json delete mode 100644 packages/dtslint/test/npm-naming/name/tslint.json delete mode 100644 packages/dtslint/test/npm-naming/name/types/wenceslas/index.d.ts delete mode 100644 packages/dtslint/test/npm-naming/name/types/wenceslas/index.d.ts.lint delete mode 100644 packages/dtslint/test/npm-naming/name/types/wenceslas/package.json rename packages/{dtslint/docs => eslint-plugin/docs/rules}/npm-naming.md (100%) create mode 100644 packages/eslint-plugin/src/rules/npm-naming.ts create mode 100644 packages/eslint-plugin/src/rules/npm-naming/types.ts create mode 100644 packages/eslint-plugin/test/npm-naming.test.ts diff --git a/packages/dts-critic/dt.ts b/packages/dts-critic/dt.ts index 3348c78656..c532dd2c64 100644 --- a/packages/dts-critic/dt.ts +++ b/packages/dts-critic/dt.ts @@ -2,26 +2,26 @@ import { dtsCritic as critic, ErrorKind } from "./index"; import fs = require("fs"); import stripJsonComments = require("strip-json-comments"); -function hasNpmNamingLintRule(tslintPath: string): boolean { - if (fs.existsSync(tslintPath)) { - const tslint = JSON.parse(stripJsonComments(fs.readFileSync(tslintPath, "utf-8"))); - if (tslint.rules && tslint.rules["npm-naming"] !== undefined) { - return !!tslint.rules["npm-naming"]; +function hasNpmNamingLintRule(eslintPath: string): boolean { + if (fs.existsSync(eslintPath)) { + const eslint = JSON.parse(stripJsonComments(fs.readFileSync(eslintPath, "utf-8"))); + if (eslint.rules?.["@definitelytyped/npm-naming"] !== undefined) { + return !!eslint.rules["@definitelytyped/npm-naming"]; } return true; } return false; } -function addNpmNamingLintRule(tslintPath: string): void { - if (fs.existsSync(tslintPath)) { - const tslint = JSON.parse(stripJsonComments(fs.readFileSync(tslintPath, "utf-8"))); - if (tslint.rules) { - tslint.rules["npm-naming"] = false; +function addNpmNamingLintRule(eslintPath: string): void { + if (fs.existsSync(eslintPath)) { + const eslint = JSON.parse(stripJsonComments(fs.readFileSync(eslintPath, "utf-8"))); + if (eslint.rules) { + eslint.rules["@definitelytyped/npm-naming"] = false; } else { - tslint.rules = { "npm-naming": false }; + eslint.rules = { "@definitelytyped/npm-naming": false }; } - fs.writeFileSync(tslintPath, JSON.stringify(tslint, undefined, 4), "utf-8"); + fs.writeFileSync(eslintPath, JSON.stringify(eslint, undefined, 4), "utf-8"); } } @@ -29,7 +29,7 @@ function main() { for (const item of fs.readdirSync("../DefinitelyTyped/types")) { const entry = "../DefinitelyTyped/types/" + item; try { - if (hasNpmNamingLintRule(entry + "/tslint.json")) { + if (hasNpmNamingLintRule(entry + "/.eslintrc.json")) { const errors = critic(entry + "/index.d.ts"); for (const error of errors) { switch (error.kind) { @@ -57,7 +57,7 @@ function main() { const npmvers = m[2].split(",").map((s: string) => parseFloat(s.trim())); const fixto = npmvers.every((v: number) => headerver > v) ? -1.0 : Math.max(...npmvers); console.log(`npm-version:${item}:${m[1]}:${m[2]}:${fixto}`); - addNpmNamingLintRule(entry + "/tslint.json"); + addNpmNamingLintRule(entry + "/.eslintrc.json"); } else { console.log("could not parse error message: ", error.message); } diff --git a/packages/dtslint/dt.json b/packages/dtslint/dt.json index e0da5d8948..9aa84ed656 100644 --- a/packages/dtslint/dt.json +++ b/packages/dtslint/dt.json @@ -1,6 +1,3 @@ { - "extends": "./dtslint.json", - "rules": { - "npm-naming": [true, { "mode": "code" }] - } + "extends": "./dtslint.json" } diff --git a/packages/dtslint/dtslint.json b/packages/dtslint/dtslint.json index 829f347d4a..13c85cd2f0 100644 --- a/packages/dtslint/dtslint.json +++ b/packages/dtslint/dtslint.json @@ -1,7 +1,6 @@ { "rulesDirectory": "./dist/rules", "rules": { - "expect": true, - "npm-naming": true + "expect": true } } diff --git a/packages/dtslint/src/rules/npmNamingRule.ts b/packages/dtslint/src/rules/npmNamingRule.ts deleted file mode 100644 index 7743745d6b..0000000000 --- a/packages/dtslint/src/rules/npmNamingRule.ts +++ /dev/null @@ -1,311 +0,0 @@ -import { - CheckOptions as CriticOptions, - CriticError, - defaultErrors, - dtsCritic as critic, - ErrorKind, - ExportErrorKind, - Mode, - parseExportErrorKind, - parseMode, -} from "@definitelytyped/dts-critic"; -import * as Lint from "tslint"; -import * as ts from "typescript"; - -import { addSuggestion } from "../suggestions"; -import { failure, isMainFile } from "../util"; - -/** Options as parsed from the rule configuration. */ -type ConfigOptions = - | { - mode: Mode.NameOnly; - singleLine?: boolean; - } - | { - mode: Mode.Code; - errors: [ExportErrorKind, boolean][]; - singleLine?: boolean; - }; - -type Options = CriticOptions & { singleLine?: boolean }; - -const defaultOptions: ConfigOptions = { - mode: Mode.NameOnly, -}; - -export class Rule extends Lint.Rules.AbstractRule { - static metadata: Lint.IRuleMetadata = { - ruleName: "npm-naming", - description: "Ensure that package name and DefinitelyTyped header match npm package info.", - optionsDescription: `An object with a \`mode\` property should be provided. -If \`mode\` is '${Mode.Code}', then option \`errors\` can be provided. -\`errors\` should be an array specifying which code checks should be enabled or disabled.`, - options: { - oneOf: [ - { - type: "object", - properties: { - mode: { - type: "string", - enum: [Mode.NameOnly], - }, - "single-line": { - description: "Whether to print error messages in a single line. Used for testing.", - type: "boolean", - }, - required: ["mode"], - }, - }, - { - type: "object", - properties: { - mode: { - type: "string", - enum: [Mode.Code], - }, - errors: { - type: "array", - items: { - type: "array", - items: [ - { - description: "Name of the check.", - type: "string", - enum: [ErrorKind.NeedsExportEquals, ErrorKind.NoDefaultExport] as ExportErrorKind[], - }, - { - description: "Whether the check is enabled or disabled.", - type: "boolean", - }, - ], - minItems: 2, - maxItems: 2, - }, - default: [], - }, - "single-line": { - description: "Whether to print error messages in a single line. Used for testing.", - type: "boolean", - }, - required: ["mode"], - }, - }, - ], - }, - optionExamples: [ - true, - [true, { mode: Mode.NameOnly }], - [ - true, - { - mode: Mode.Code, - errors: [ - [ErrorKind.NeedsExportEquals, true], - [ErrorKind.NoDefaultExport, false], - ], - }, - ], - ], - type: "functionality", - typescriptOnly: true, - }; - - apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithFunction(sourceFile, walk, toCriticOptions(parseOptions(this.ruleArguments))); - } -} - -function parseOptions(args: unknown[]): ConfigOptions { - if (args.length === 0) { - return defaultOptions; - } - - const arg = args[0] as { [prop: string]: unknown } | null | undefined; - // eslint-disable-next-line eqeqeq - if (arg == null) { - return defaultOptions; - } - - if (!arg.mode || typeof arg.mode !== "string") { - return defaultOptions; - } - - const mode = parseMode(arg.mode); - if (!mode) { - return defaultOptions; - } - - const singleLine = !!arg["single-line"]; - - switch (mode) { - case Mode.NameOnly: - return { mode, singleLine }; - case Mode.Code: - if (!arg.errors || !Array.isArray(arg.errors)) { - return { mode, errors: [], singleLine }; - } - return { mode, errors: parseEnabledErrors(arg.errors), singleLine }; - } -} - -function parseEnabledErrors(errors: unknown[]): [ExportErrorKind, boolean][] { - const enabledChecks: [ExportErrorKind, boolean][] = []; - for (const tuple of errors) { - if (Array.isArray(tuple) && tuple.length === 2 && typeof tuple[0] === "string" && typeof tuple[1] === "boolean") { - const error = parseExportErrorKind(tuple[0]); - if (error) { - enabledChecks.push([error, tuple[1]]); - } - } - } - return enabledChecks; -} - -function toCriticOptions(options: ConfigOptions): Options { - switch (options.mode) { - case Mode.NameOnly: - return options; - case Mode.Code: - return { ...options, errors: new Map(options.errors) }; - } -} - -function walk(ctx: Lint.WalkContext): void { - const { sourceFile } = ctx; - if (isMainFile(sourceFile.fileName, /*allowNested*/ false)) { - try { - const optionsWithSuggestions = toOptionsWithSuggestions(ctx.options); - const diagnostics = critic(sourceFile.fileName, /* sourcePath */ undefined, optionsWithSuggestions); - const errors = filterErrors(diagnostics, ctx); - for (const error of errors) { - switch (error.kind) { - case ErrorKind.NoMatchingNpmPackage: - case ErrorKind.NoMatchingNpmVersion: - case ErrorKind.NonNpmHasMatchingPackage: - case ErrorKind.DtsPropertyNotInJs: - case ErrorKind.DtsSignatureNotInJs: - case ErrorKind.JsPropertyNotInDts: - case ErrorKind.JsSignatureNotInDts: - case ErrorKind.NeedsExportEquals: - case ErrorKind.NoDefaultExport: - if (error.position) { - ctx.addFailureAt( - error.position.start, - error.position.length, - failure(Rule.metadata.ruleName, errorMessage(error, ctx.options)), - ); - } else { - ctx.addFailure(0, 1, failure(Rule.metadata.ruleName, errorMessage(error, ctx.options))); - } - break; - } - } - } catch (e) { - // We're ignoring exceptions. - } - } - // Don't recur, we're done. -} - -const enabledSuggestions: ExportErrorKind[] = [ErrorKind.JsPropertyNotInDts, ErrorKind.JsSignatureNotInDts]; - -function toOptionsWithSuggestions(options: CriticOptions): CriticOptions { - if (options.mode === Mode.NameOnly) { - return options; - } - const optionsWithSuggestions = { mode: options.mode, errors: new Map(options.errors) }; - enabledSuggestions.forEach((err) => optionsWithSuggestions.errors.set(err, true)); - return optionsWithSuggestions; -} - -function filterErrors(diagnostics: CriticError[], ctx: Lint.WalkContext): CriticError[] { - const errors: CriticError[] = []; - diagnostics.forEach((diagnostic) => { - if (isSuggestion(diagnostic, ctx.options)) { - addSuggestion(ctx, diagnostic.message, diagnostic.position?.start, diagnostic.position?.length); - } else { - errors.push(diagnostic); - } - }); - return errors; -} - -function isSuggestion(diagnostic: CriticError, options: Options): boolean { - return ( - options.mode === Mode.Code && - (enabledSuggestions as ErrorKind[]).includes(diagnostic.kind) && - !(options.errors as Map).get(diagnostic.kind) - ); -} - -function tslintDisableOption(error: ErrorKind): string { - switch (error) { - case ErrorKind.NoMatchingNpmPackage: - case ErrorKind.NoMatchingNpmVersion: - case ErrorKind.NonNpmHasMatchingPackage: - return `false`; - case ErrorKind.NoDefaultExport: - case ErrorKind.NeedsExportEquals: - case ErrorKind.JsSignatureNotInDts: - case ErrorKind.JsPropertyNotInDts: - case ErrorKind.DtsSignatureNotInJs: - case ErrorKind.DtsPropertyNotInJs: - return JSON.stringify([true, { mode: Mode.Code, errors: [[error, false]] }]); - } -} - -function errorMessage(error: CriticError, opts: Options): string { - const message = - error.message + - `\nIf 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": ${tslintDisableOption(error.kind)} -`; - if (opts.singleLine) { - return message.replace(/(\r\n|\n|\r|\t)/gm, " "); - } - - return message; -} - -/** - * Given npm-naming lint failures, returns a rule configuration that prevents such failures. - */ -export function disabler(failures: Lint.IRuleFailureJson[]): false | [true, ConfigOptions] { - const disabledErrors = new Set(); - for (const ruleFailure of failures) { - if (ruleFailure.ruleName !== "npm-naming") { - throw new Error(`Expected failures of rule "npm-naming", found failures of rule ${ruleFailure.ruleName}.`); - } - const message = ruleFailure.failure; - // Name errors. - if ( - message.includes("must have a matching npm package") || - message.includes("must match a version that exists on npm") || - message.includes("conflicts with the existing npm package") - ) { - return false; - } - // Code errors. - if (message.includes("declaration should use 'export =' syntax")) { - disabledErrors.add(ErrorKind.NeedsExportEquals); - } else if ( - message.includes( - "declaration specifies 'export default' but the JavaScript source \ - does not mention 'default' anywhere", - ) - ) { - disabledErrors.add(ErrorKind.NoDefaultExport); - } else { - return [true, { mode: Mode.NameOnly }]; - } - } - - if ((defaultErrors as ExportErrorKind[]).every((error) => disabledErrors.has(error))) { - return [true, { mode: Mode.NameOnly }]; - } - const errors: [ExportErrorKind, boolean][] = []; - disabledErrors.forEach((error) => errors.push([error, false])); - return [true, { mode: Mode.Code, errors }]; -} diff --git a/packages/dtslint/src/updateConfig.ts b/packages/dtslint/src/updateConfig.ts index 5d1a3a264e..388b2a0634 100644 --- a/packages/dtslint/src/updateConfig.ts +++ b/packages/dtslint/src/updateConfig.ts @@ -15,7 +15,6 @@ import { Configuration as Config, ILinterOptions, IRuleFailureJson, Linter, Lint import * as ts from "typescript"; import yargs = require("yargs"); import { isExternalDependency } from "./lint"; -import { disabler as npmNamingDisabler } from "./rules/npmNamingRule"; // Rule "expect" needs TypeScript version information, which this script doesn't collect. const ignoredRules: string[] = ["expect"]; @@ -216,12 +215,6 @@ function isVersionDir(dirName: string): boolean { return /^ts\d+\.\d$/.test(dirName) || /^v\d+(\.\d+)?$/.test(dirName); } -type RuleOptions = boolean | unknown[]; -type RuleDisabler = (failures: IRuleFailureJson[]) => RuleOptions; -const defaultDisabler: RuleDisabler = () => { - return false; -}; - function disableRules(allFailures: RuleFailure[]): Config.RawRulesConfig { const ruleToFailures: Map = new Map(); for (const failure of allFailures) { @@ -234,14 +227,11 @@ function disableRules(allFailures: RuleFailure[]): Config.RawRulesConfig { } const newRulesConfig: Config.RawRulesConfig = {}; - ruleToFailures.forEach((failures, rule) => { - if (ignoredRules.includes(rule)) { - return; + for (const rule of ruleToFailures.keys()) { + if (!ignoredRules.includes(rule)) { + newRulesConfig[rule] = false; } - const disabler = rule === "npm-naming" ? npmNamingDisabler : defaultDisabler; - const opts: RuleOptions = disabler(failures); - newRulesConfig[rule] = opts; - }); + } return newRulesConfig; } diff --git a/packages/dtslint/src/util.ts b/packages/dtslint/src/util.ts index 3478b230ca..c4c3c33ac3 100644 --- a/packages/dtslint/src/util.ts +++ b/packages/dtslint/src/util.ts @@ -29,24 +29,3 @@ export function last(a: readonly T[]): T { assert(a.length !== 0); return a[a.length - 1]; } - -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") { - return true; - } - - if (basename(fileName) !== "index.d.ts") { - return false; - } - - let parent = dirname(fileName); - // May be a directory for an older version, e.g. `v0`. - // Note a types redirect `foo/ts3.1` should not have its own header. - if (allowNested && /^v(0\.)?\d+$/.test(basename(parent))) { - parent = dirname(parent); - } - - // Allow "types/foo/index.d.ts", not "types/foo/utils/index.d.ts" - return basename(dirname(parent)) === "types"; -} diff --git a/packages/dtslint/test/npm-naming/code/tslint.json b/packages/dtslint/test/npm-naming/code/tslint.json deleted file mode 100644 index 3c9ea61f7d..0000000000 --- a/packages/dtslint/test/npm-naming/code/tslint.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "rulesDirectory": ["../../../dist/rules"], - "rules": { - "npm-naming": [true, {"mode": "code", "errors": [["NeedsExportEquals", true]], "single-line": true }] - } - } 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 deleted file mode 100644 index 0d3239f05e..0000000000 --- a/packages/dtslint/test/npm-naming/code/types/dts-critic/index.d.ts +++ /dev/null @@ -1 +0,0 @@ -export default dtsCritic(); diff --git a/packages/dtslint/test/npm-naming/code/types/dts-critic/index.d.ts.lint b/packages/dtslint/test/npm-naming/code/types/dts-critic/index.d.ts.lint deleted file mode 100644 index bfbdf7ec78..0000000000 --- a/packages/dtslint/test/npm-naming/code/types/dts-critic/index.d.ts.lint +++ /dev/null @@ -1,8 +0,0 @@ -// Type definitions for package dts-critic 1.0 -~ [0] -// Project: https://https://github.com/DefinitelyTyped/dts-critic -// Definitions by: Jane Doe -// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped - -export default dtsCritic(); -[0]: The declaration doesn't match the JavaScript module 'dts-critic'. Reason: The declaration should use 'export =' syntax because the JavaScript source uses 'module.exports =' syntax and 'module.exports' can be called or constructed. To learn more about 'export =' syntax, see https://www.typescriptlang.org/docs/handbook/modules.html#export--and-import--require. 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": [true,{"mode":"code","errors":[["NeedsExportEquals",false]]}] See: https://github.com/microsoft/DefinitelyTyped-tools/blob/master/packages/dtslint/docs/npm-naming.md 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 deleted file mode 100644 index 844ba94919..0000000000 --- a/packages/dtslint/test/npm-naming/code/types/dts-critic/package.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "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/tslint.json b/packages/dtslint/test/npm-naming/name/tslint.json deleted file mode 100644 index 18337910ac..0000000000 --- a/packages/dtslint/test/npm-naming/name/tslint.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "rulesDirectory": ["../../../dist/rules"], - "rules": { - "npm-naming": [true, {"mode": "name-only", "single-line": true}] - } -} 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 deleted file mode 100644 index 81ab89bd42..0000000000 --- a/packages/dtslint/test/npm-naming/name/types/wenceslas/index.d.ts +++ /dev/null @@ -1 +0,0 @@ -// 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 deleted file mode 100644 index 50888569ef..0000000000 --- a/packages/dtslint/test/npm-naming/name/types/wenceslas/index.d.ts.lint +++ /dev/null @@ -1,2 +0,0 @@ -// 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 deleted file mode 100644 index 0bbc875c71..0000000000 --- a/packages/dtslint/test/npm-naming/name/types/wenceslas/package.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "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/dtslint/test/tsconfig.json b/packages/dtslint/test/tsconfig.json index 01cf16cd34..85ba8b7b57 100644 --- a/packages/dtslint/test/tsconfig.json +++ b/packages/dtslint/test/tsconfig.json @@ -9,6 +9,6 @@ { "path": ".." }, { "path": "../../utils" } ], - "exclude": ["expect", "npm-naming"] + "exclude": ["expect"] } \ No newline at end of file diff --git a/packages/dtslint/docs/npm-naming.md b/packages/eslint-plugin/docs/rules/npm-naming.md similarity index 100% rename from packages/dtslint/docs/npm-naming.md rename to packages/eslint-plugin/docs/rules/npm-naming.md diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index 0d589a9f4e..59e8cabcdd 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -22,6 +22,7 @@ "test": "../../node_modules/.bin/jest --config ../../jest.config.js packages/dtslint" }, "dependencies": { + "@definitelytyped/dts-critic": "workspace:*", "@definitelytyped/utils": "workspace:*", "@typescript-eslint/types": "^6.11.0", "@typescript-eslint/utils": "^6.11.0" @@ -35,6 +36,7 @@ }, "devDependencies": { "@definitelytyped/eslint-plugin": "link:./", + "@typescript-eslint/rule-tester": "^6.11.0", "@types/eslint": "^8.44.7", "glob": "^10.3.10", "jest-file-snapshot": "^0.5.0", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 0fc0b0e9b3..bb81426447 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -16,6 +16,7 @@ import strictExportDeclareModifiers = require("./strict-export-declare-modifiers import noSingleDeclareModule = require("./no-single-declare-module"); import noOldDTHeader = require("./no-old-dt-header"); import noImportOfDevDependencies = require("./no-import-of-dev-dependencies"); +import npmNaming = require("./npm-naming"); export const rules = { "export-just-namespace": exportJustNamespace, @@ -36,4 +37,5 @@ export const rules = { "no-single-declare-module": noSingleDeclareModule, "no-old-dt-header": noOldDTHeader, "no-import-of-dev-dependencies": noImportOfDevDependencies, + "npm-naming": npmNaming, }; diff --git a/packages/eslint-plugin/src/rules/npm-naming.ts b/packages/eslint-plugin/src/rules/npm-naming.ts new file mode 100644 index 0000000000..3828f9dd73 --- /dev/null +++ b/packages/eslint-plugin/src/rules/npm-naming.ts @@ -0,0 +1,181 @@ +import { + CheckOptions as CriticOptions, + dtsCritic as critic, + ErrorKind, + ExportErrorKind, + Mode, + parseExportErrorKind, +} from "@definitelytyped/dts-critic"; + +import { createRule, isMainFile } from "../util"; +import { CodeRawOptionError, NpmNamingOptions } from "./npm-naming/types"; + +function parseEnabledErrors(errors: CodeRawOptionError[]): [ExportErrorKind, boolean][] { + const enabledChecks: [ExportErrorKind, boolean][] = []; + for (const tuple of errors) { + const error = parseExportErrorKind(tuple[0]); + if (error) { + enabledChecks.push([error, tuple[1]]); + } + } + return enabledChecks; +} + +function parseRawOptions(rawOptions: NpmNamingOptions): CriticOptions { + switch (rawOptions.mode) { + case Mode.Code: + return { ...rawOptions, errors: new Map(parseEnabledErrors(rawOptions.errors)) }; + case Mode.NameOnly: + return rawOptions; + } +} + +const enabledSuggestions = [ErrorKind.JsPropertyNotInDts, ErrorKind.JsSignatureNotInDts] as const; + +function toOptionsWithSuggestions(options: CriticOptions): CriticOptions { + if (options.mode === Mode.NameOnly) { + return options; + } + + const optionsWithSuggestions = { mode: options.mode, errors: new Map(options.errors) }; + + for (const err of enabledSuggestions) { + optionsWithSuggestions.errors.set(err, true); + } + + return optionsWithSuggestions; +} + +function eslintDisableOption(error: ErrorKind): string { + switch (error) { + case ErrorKind.NoMatchingNpmPackage: + case ErrorKind.NoMatchingNpmVersion: + case ErrorKind.NonNpmHasMatchingPackage: + return `"off"`; + case ErrorKind.NoDefaultExport: + case ErrorKind.NeedsExportEquals: + case ErrorKind.JsSignatureNotInDts: + case ErrorKind.JsPropertyNotInDts: + case ErrorKind.DtsSignatureNotInJs: + case ErrorKind.DtsPropertyNotInJs: + return JSON.stringify(["error", { mode: Mode.Code, errors: [[error, false]] }], null, 2); + } +} + +const rule = createRule<[NpmNamingOptions], "error">({ + name: "npm-naming", + defaultOptions: [ + { + mode: Mode.NameOnly, + }, + ], + meta: { + type: "problem", + docs: { + description: "Ensure that package name and DefinitelyTyped header match npm package info.", + }, + messages: { + error: `{{ error }} +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": {{ option }}`, + }, + schema: [ + { + oneOf: [ + { + additionalProperties: false, + properties: { + mode: { + type: "string", + enum: [Mode.NameOnly], + }, + }, + type: "object", + }, + { + additionalProperties: false, + type: "object", + properties: { + mode: { + type: "string", + enum: [Mode.Code], + }, + errors: { + type: "array", + items: { + type: "array", + items: [ + { + description: "Name of the check.", + type: "string", + enum: [ErrorKind.NeedsExportEquals, ErrorKind.NoDefaultExport], + }, + { + description: "Whether the check is enabled or disabled.", + type: "boolean", + }, + ], + minItems: 2, + maxItems: 2, + }, + default: [], + }, + }, + }, + ], + }, + ], + }, + create(context, [rawOptions]) { + if (!isMainFile(context.filename, /*allowNested*/ false)) { + return {}; + } + + const options = parseRawOptions(rawOptions); + const optionsWithSuggestions = toOptionsWithSuggestions(options); + const errors = critic(context.filename, /* sourcePath */ undefined, optionsWithSuggestions); + + for (const error of errors) { + switch (error.kind) { + case ErrorKind.NoMatchingNpmPackage: + case ErrorKind.NoMatchingNpmVersion: + case ErrorKind.NonNpmHasMatchingPackage: + case ErrorKind.DtsPropertyNotInJs: + case ErrorKind.DtsSignatureNotInJs: + case ErrorKind.JsPropertyNotInDts: + case ErrorKind.JsSignatureNotInDts: + case ErrorKind.NeedsExportEquals: + case ErrorKind.NoDefaultExport: + context.report({ + data: { + error: error.message, + option: eslintDisableOption(error.kind), + }, + loc: error.position + ? { + start: context.sourceCode.getLocFromIndex(error.position.start), + end: context.sourceCode.getLocFromIndex(error.position.start + error.position.length), + } + : { + end: { + line: 2, + column: 0, + }, + start: { + line: 1, + column: 0, + }, + }, + messageId: "error", + }); + break; + } + } + + return {}; + }, +}); + +export = rule; diff --git a/packages/eslint-plugin/src/rules/npm-naming/types.ts b/packages/eslint-plugin/src/rules/npm-naming/types.ts new file mode 100644 index 0000000000..f2e3cd9a03 --- /dev/null +++ b/packages/eslint-plugin/src/rules/npm-naming/types.ts @@ -0,0 +1,14 @@ +import { ExportErrorKind, Mode } from "@definitelytyped/dts-critic"; + +export type CodeRawOptionError = [ExportErrorKind, boolean]; + +export interface CodeRawOptions { + mode: Mode.Code; + errors: CodeRawOptionError[]; +} + +export interface NameOnlyRawOptions { + mode: Mode.NameOnly; +} + +export type NpmNamingOptions = CodeRawOptions | NameOnlyRawOptions; diff --git a/packages/eslint-plugin/src/util.ts b/packages/eslint-plugin/src/util.ts index 3218c65b55..743f4910e8 100644 --- a/packages/eslint-plugin/src/util.ts +++ b/packages/eslint-plugin/src/util.ts @@ -116,3 +116,24 @@ export function getImportSource( return undefined; } + +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") { + return true; + } + + if (path.basename(fileName) !== "index.d.ts") { + return false; + } + + let parent = path.dirname(fileName); + // May be a directory for an older version, e.g. `v0`. + // Note a types redirect `foo/ts3.1` should not have its own header. + if (allowNested && /^v(0\.)?\d+$/.test(path.basename(parent))) { + parent = path.dirname(parent); + } + + // Allow "types/foo/index.d.ts", not "types/foo/utils/index.d.ts" + return path.basename(path.dirname(parent)) === "types"; +} diff --git a/packages/eslint-plugin/test/npm-naming.test.ts b/packages/eslint-plugin/test/npm-naming.test.ts new file mode 100644 index 0000000000..3e8fa0bc86 --- /dev/null +++ b/packages/eslint-plugin/test/npm-naming.test.ts @@ -0,0 +1,55 @@ +import { RuleTester } from "@typescript-eslint/rule-tester"; + +import * as npmNaming from "../src/rules/npm-naming"; +import { ErrorKind, Mode } from "@definitelytyped/dts-critic"; + +const ruleTester = new RuleTester({ + parser: "@typescript-eslint/parser", +}); + +ruleTester.run("npm-naming", npmNaming, { + invalid: [ + { + code: `export default dtsCritic();`, + errors: [ + { + data: { + explanation: `The declaration doesn't match the JavaScript module 'dts-critic'. Reason: + The declaration should use 'export =' syntax because the JavaScript source uses 'module.exports =' syntax and 'module.exports' can be called or constructed. + + To learn more about 'export =' syntax, see https://www.typescriptlang.org/docs/handbook/modules.html#export--and-import--require.`, + option: `["error", { "mode": "code", "errors": [["NeedsExportEquals", false]] } ]`, + }, + column: 1, + endColumn: 2, + line: 1, + messageId: "error", + }, + ], + filename: "packages/eslint-plugin/test/types/dts-critic/index.d.ts", + options: [{ mode: Mode.Code, errors: [[ErrorKind.NeedsExportEquals, true]] }], + }, + { + code: `// test content`, + errors: [ + { + data: { + error: `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.`, + option: `"off"`, + }, + column: 1, + endColumn: 2, + line: 1, + messageId: "error", + }, + ], + filename: "packages/eslint-plugin/test/types/wenceslas/index.d.ts", + options: [{ mode: Mode.NameOnly }], + }, + ], + valid: [], +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1b889e47ca..f4d105d4fe 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -226,6 +226,9 @@ importers: packages/eslint-plugin: dependencies: + '@definitelytyped/dts-critic': + specifier: workspace:* + version: link:../dts-critic '@definitelytyped/utils': specifier: workspace:* version: link:../utils @@ -254,6 +257,9 @@ importers: '@types/eslint': specifier: ^8.44.7 version: 8.44.7 + '@typescript-eslint/rule-tester': + specifier: ^6.11.0 + version: 6.11.0(@eslint/eslintrc@2.1.3)(eslint@8.53.0)(typescript@5.2.2) glob: specifier: ^10.3.10 version: 10.3.10 @@ -2035,6 +2041,25 @@ packages: transitivePeerDependencies: - supports-color + /@typescript-eslint/rule-tester@6.11.0(@eslint/eslintrc@2.1.3)(eslint@8.53.0)(typescript@5.2.2): + resolution: {integrity: sha512-4OQn7HuOnqtg2yDjBvshUs7NnQ4ySKjylh+dbmGojkau7wX8laUcIRUEu3qS2+LyQAmt1yoM7lKdGzJSG3TXyQ==} + engines: {node: ^16.0.0 || >=18.0.0} + peerDependencies: + '@eslint/eslintrc': '>=2' + eslint: '>=8' + dependencies: + '@eslint/eslintrc': 2.1.3 + '@typescript-eslint/typescript-estree': 6.11.0(typescript@5.2.2) + '@typescript-eslint/utils': 6.11.0(eslint@8.53.0)(typescript@5.2.2) + ajv: 6.12.6 + eslint: 8.53.0 + lodash.merge: 4.6.2 + semver: 7.5.4 + transitivePeerDependencies: + - supports-color + - typescript + dev: true + /@typescript-eslint/scope-manager@6.11.0: resolution: {integrity: sha512-0A8KoVvIURG4uhxAdjSaxy8RdRE//HztaZdG8KiHLP8WOXSk0vlF7Pvogv+vlJA5Rnjj/wDcFENvDaHb+gKd1A==} engines: {node: ^16.0.0 || >=18.0.0} From 0bfae60087cb1248de6eb0f08b49c07f02dabba9 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 8 Dec 2023 00:48:24 -0500 Subject: [PATCH 02/18] Converted npm-naming from TSLint to ESLint --- .../eslint-plugin/src/rules/npm-naming.ts | 38 +++++++++++++++++-- .../src/suggestions.ts | 9 ++--- .../eslint-plugin/test/npm-naming.test.ts | 16 ++++---- .../test/types/dts-critic/index.d.ts | 9 +++++ .../test/types/dts-critic/package.json | 17 +++++++++ .../test/types/wenceslas/index.d.ts | 0 .../test/types/wenceslas/package.json | 17 +++++++++ 7 files changed, 90 insertions(+), 16 deletions(-) rename packages/{dtslint => eslint-plugin}/src/suggestions.ts (87%) create mode 100644 packages/eslint-plugin/test/types/dts-critic/index.d.ts create mode 100644 packages/eslint-plugin/test/types/dts-critic/package.json create mode 100644 packages/eslint-plugin/test/types/wenceslas/index.d.ts create mode 100644 packages/eslint-plugin/test/types/wenceslas/package.json diff --git a/packages/eslint-plugin/src/rules/npm-naming.ts b/packages/eslint-plugin/src/rules/npm-naming.ts index 3828f9dd73..5f12883bbe 100644 --- a/packages/eslint-plugin/src/rules/npm-naming.ts +++ b/packages/eslint-plugin/src/rules/npm-naming.ts @@ -5,8 +5,10 @@ import { ExportErrorKind, Mode, parseExportErrorKind, + CriticError, } from "@definitelytyped/dts-critic"; +import { addSuggestion } from "../suggestions"; import { createRule, isMainFile } from "../util"; import { CodeRawOptionError, NpmNamingOptions } from "./npm-naming/types"; @@ -30,7 +32,7 @@ function parseRawOptions(rawOptions: NpmNamingOptions): CriticOptions { } } -const enabledSuggestions = [ErrorKind.JsPropertyNotInDts, ErrorKind.JsSignatureNotInDts] as const; +const enabledSuggestions: ExportErrorKind[] = [ErrorKind.JsPropertyNotInDts, ErrorKind.JsSignatureNotInDts]; function toOptionsWithSuggestions(options: CriticOptions): CriticOptions { if (options.mode === Mode.NameOnly) { @@ -58,7 +60,7 @@ function eslintDisableOption(error: ErrorKind): string { case ErrorKind.JsPropertyNotInDts: case ErrorKind.DtsSignatureNotInJs: case ErrorKind.DtsPropertyNotInJs: - return JSON.stringify(["error", { mode: Mode.Code, errors: [[error, false]] }], null, 2); + return JSON.stringify(["error", { mode: Mode.Code, errors: [[error, false]] }]); } } @@ -120,7 +122,6 @@ you can disable this check by adding the following options to your project's tsl minItems: 2, maxItems: 2, }, - default: [], }, }, }, @@ -135,7 +136,8 @@ you can disable this check by adding the following options to your project's tsl const options = parseRawOptions(rawOptions); const optionsWithSuggestions = toOptionsWithSuggestions(options); - const errors = critic(context.filename, /* sourcePath */ undefined, optionsWithSuggestions); + const diagnostics = critic(context.filename, /* sourcePath */ undefined, optionsWithSuggestions); + const errors = filterErrors(diagnostics); for (const error of errors) { switch (error.kind) { @@ -175,6 +177,34 @@ you can disable this check by adding the following options to your project's tsl } return {}; + + function filterErrors(diagnostics: CriticError[]): CriticError[] { + const errors: CriticError[] = []; + + diagnostics.forEach((diagnostic) => { + if (isSuggestion(diagnostic)) { + addSuggestion( + context.filename, + "npm-naming", + diagnostic.message, + diagnostic.position?.start, + diagnostic.position?.length, + ); + } else { + errors.push(diagnostic); + } + }); + + return errors; + } + + function isSuggestion(diagnostic: CriticError): boolean { + return ( + options.mode === Mode.Code && + enabledSuggestions.includes(diagnostic.kind as ExportErrorKind) && + !(options.errors as Map).get(diagnostic.kind) + ); + } }, }); diff --git a/packages/dtslint/src/suggestions.ts b/packages/eslint-plugin/src/suggestions.ts similarity index 87% rename from packages/dtslint/src/suggestions.ts rename to packages/eslint-plugin/src/suggestions.ts index 121a92f278..9029283560 100644 --- a/packages/dtslint/src/suggestions.ts +++ b/packages/eslint-plugin/src/suggestions.ts @@ -1,7 +1,6 @@ import fs = require("fs"); import os = require("os"); import path = require("path"); -import { WalkContext } from "tslint"; const suggestionsDir = path.join(os.homedir(), ".dts", "suggestions"); @@ -19,16 +18,16 @@ const existingPackages = new Set(); /** * A rule should call this function to provide a suggestion instead of a lint failure. */ -export function addSuggestion(ctx: WalkContext, message: string, start?: number, width?: number) { +export function addSuggestion(fileName: string, ruleName: string, message: string, start?: number, width?: number) { const suggestion: Suggestion = { - fileName: ctx.sourceFile.fileName, - ruleName: ctx.ruleName, + fileName, + ruleName, message, start, width, }; - const packageName = dtPackageName(ctx.sourceFile.fileName); + const packageName = dtPackageName(fileName); if (!packageName) { return; } diff --git a/packages/eslint-plugin/test/npm-naming.test.ts b/packages/eslint-plugin/test/npm-naming.test.ts index 3e8fa0bc86..c0f6622a11 100644 --- a/packages/eslint-plugin/test/npm-naming.test.ts +++ b/packages/eslint-plugin/test/npm-naming.test.ts @@ -14,15 +14,16 @@ ruleTester.run("npm-naming", npmNaming, { errors: [ { data: { - explanation: `The declaration doesn't match the JavaScript module 'dts-critic'. Reason: - The declaration should use 'export =' syntax because the JavaScript source uses 'module.exports =' syntax and 'module.exports' can be called or constructed. - - To learn more about 'export =' syntax, see https://www.typescriptlang.org/docs/handbook/modules.html#export--and-import--require.`, - option: `["error", { "mode": "code", "errors": [["NeedsExportEquals", false]] } ]`, + error: `The declaration doesn't match the JavaScript module 'dts-critic'. Reason: +The declaration should use 'export =' syntax because the JavaScript source uses 'module.exports =' syntax and 'module.exports' can be called or constructed. + +To learn more about 'export =' syntax, see https://www.typescriptlang.org/docs/handbook/modules.html#export--and-import--require.`, + option: `["error",{"mode":"code","errors":[["NeedsExportEquals",false]]}]`, }, column: 1, - endColumn: 2, + endColumn: 1, line: 1, + endLine: 2, messageId: "error", }, ], @@ -42,8 +43,9 @@ To resolve this error, either: option: `"off"`, }, column: 1, - endColumn: 2, + endColumn: 1, line: 1, + endLine: 2, messageId: "error", }, ], diff --git a/packages/eslint-plugin/test/types/dts-critic/index.d.ts b/packages/eslint-plugin/test/types/dts-critic/index.d.ts new file mode 100644 index 0000000000..56732de851 --- /dev/null +++ b/packages/eslint-plugin/test/types/dts-critic/index.d.ts @@ -0,0 +1,9 @@ +declare function _default(): void; + +declare namespace _default { + export function findDtsName(dtsPath: string): string; + export function checkNames(names: string[]): unknown; + export function checkSource(text: string): unknown; + export function findNames(): string[]; + export function retrieveNpmHomepageOrFail(): string[]; +} diff --git a/packages/eslint-plugin/test/types/dts-critic/package.json b/packages/eslint-plugin/test/types/dts-critic/package.json new file mode 100644 index 0000000000..c75acfe312 --- /dev/null +++ b/packages/eslint-plugin/test/types/dts-critic/package.json @@ -0,0 +1,17 @@ +{ + "devDependencies": { + "@types/dts-critic": "workspace:." + }, + "private": true, + "projects": [ + "https://typescriptlang.org" + ], + "version": "1.0.9999", + "name": "@types/dts-critic", + "owners": [ + { + "githubUsername": "ghost", + "name": "Not Default" + } + ] +} \ No newline at end of file diff --git a/packages/eslint-plugin/test/types/wenceslas/index.d.ts b/packages/eslint-plugin/test/types/wenceslas/index.d.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/eslint-plugin/test/types/wenceslas/package.json b/packages/eslint-plugin/test/types/wenceslas/package.json new file mode 100644 index 0000000000..d5ed8d2e19 --- /dev/null +++ b/packages/eslint-plugin/test/types/wenceslas/package.json @@ -0,0 +1,17 @@ +{ + "devDependencies": { + "@types/wenceslas": "workspace:." + }, + "private": true, + "projects": [ + "https://typescriptlang.org" + ], + "version": "0.9.9999", + "name": "@types/wenceslas", + "owners": [ + { + "githubUsername": "ghost", + "name": "Not Default" + } + ] +} \ No newline at end of file From 338b006f980afe9956ff786f45e9a88b4c86d6e2 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 8 Dec 2023 12:07:29 -0500 Subject: [PATCH 03/18] disable rule in fixtures dir --- packages/eslint-plugin/test/fixtures/.eslintrc.cjs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/test/fixtures/.eslintrc.cjs b/packages/eslint-plugin/test/fixtures/.eslintrc.cjs index 50c87c7fd9..fd4755c695 100644 --- a/packages/eslint-plugin/test/fixtures/.eslintrc.cjs +++ b/packages/eslint-plugin/test/fixtures/.eslintrc.cjs @@ -1,4 +1,7 @@ module.exports = { - root: true, extends: ["plugin:@definitelytyped/all"], + root: true, + rules: { + "@definitelytyped/npm-naming": "off" + } }; From 574cc714264564a239566bac74ddaf6a6ae57734 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 11 Dec 2023 12:19:51 -0500 Subject: [PATCH 04/18] Add I think equivalent logic to expectOnly --- packages/dtslint-runner/src/index.ts | 7 +++++++ packages/dtslint-runner/src/main.ts | 2 ++ packages/dtslint-runner/src/types.ts | 1 + packages/dtslint/src/index.ts | 24 +++++++++++++++++------- packages/dtslint/src/lint.ts | 9 ++++++--- 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/packages/dtslint-runner/src/index.ts b/packages/dtslint-runner/src/index.ts index 468caaf642..d7825a4ad2 100644 --- a/packages/dtslint-runner/src/index.ts +++ b/packages/dtslint-runner/src/index.ts @@ -66,6 +66,12 @@ if (require.main === module) { type: "boolean", default: false, }, + npmNamingOnly: { + group: "dtslint options", + description: "Run only the npm-naming lint rule.", + type: "boolean", + default: false, + }, // Only useful for repeated local runs, so I’m hiding it noInstall: { hidden: true, @@ -102,6 +108,7 @@ if (require.main === module) { localTypeScriptPath: !args.onlyTestTsNext ? args.localTypeScriptPath : undefined, onlyTestTsNext: !!args.onlyTestTsNext, expectOnly: args.expectOnly, + npmNamingOnly: args.npmNamingOnly, noInstall: args.noInstall, childRestartTaskInterval: args.childRestartTaskInterval, writeFailures: args.writeFailures, diff --git a/packages/dtslint-runner/src/main.ts b/packages/dtslint-runner/src/main.ts index f80cffb53d..c6e992a49c 100644 --- a/packages/dtslint-runner/src/main.ts +++ b/packages/dtslint-runner/src/main.ts @@ -22,6 +22,7 @@ export async function runDTSLint({ noInstall, onlyTestTsNext, expectOnly, + npmNamingOnly, localTypeScriptPath, nProcesses, shard, @@ -75,6 +76,7 @@ export async function runDTSLint({ path, onlyTestTsNext: onlyTestTsNext || !packageNames.has(path), expectOnly: expectOnly || !packageNames.has(path), + npmNamingOnly: npmNamingOnly || !packageNames.has(path), })), commandLineArgs: dtslintArgs, workerFile: require.resolve("@definitelytyped/dtslint"), diff --git a/packages/dtslint-runner/src/types.ts b/packages/dtslint-runner/src/types.ts index 381b0d5f34..d37bb7831f 100644 --- a/packages/dtslint-runner/src/types.ts +++ b/packages/dtslint-runner/src/types.ts @@ -14,6 +14,7 @@ export interface RunDTSLintOptions { noInstall: boolean; onlyTestTsNext: boolean; expectOnly: boolean; + npmNamingOnly: boolean; localTypeScriptPath?: string; nProcesses: number; shard?: { id: number; count: number }; diff --git a/packages/dtslint/src/index.ts b/packages/dtslint/src/index.ts index 5e4aa3b302..0b631b4e60 100644 --- a/packages/dtslint/src/index.ts +++ b/packages/dtslint/src/index.ts @@ -21,6 +21,7 @@ async function main(): Promise { let dirPath = process.cwd(); let onlyTestTsNext = false; let expectOnly = false; + let npmNamingOnly = false; let shouldListen = false; let lookingForTsLocal = false; let tsLocal: string | undefined; @@ -52,6 +53,9 @@ async function main(): Promise { case "--expectOnly": expectOnly = true; break; + case "--npmNamingOnly": + npmNamingOnly = true; + break; case "--onlyTestTsNext": onlyTestTsNext = true; break; @@ -85,7 +89,7 @@ async function main(): Promise { listen(dirPath, tsLocal, onlyTestTsNext); } else { await installTypeScriptAsNeeded(tsLocal, onlyTestTsNext); - await runTests(dirPath, onlyTestTsNext, expectOnly, tsLocal); + await runTests(dirPath, onlyTestTsNext, expectOnly, npmNamingOnly, tsLocal); } } @@ -98,11 +102,14 @@ async function installTypeScriptAsNeeded(tsLocal: string | undefined, onlyTestTs } function usage(): void { - console.error("Usage: dtslint [--version] [--installAll] [--onlyTestTsNext] [--expectOnly] [--localTs path]"); + console.error( + "Usage: dtslint [--version] [--installAll] [--onlyTestTsNext] [--expectOnly] [--npmNamingOnly] [--localTs path]", + ); console.error("Args:"); console.error(" --version Print version and exit."); console.error(" --installAll Cleans and installs all TypeScript versions."); console.error(" --expectOnly Run only the ExpectType lint rule."); + console.error(" --npmNamingOnly Run only the npm-naming lint rule."); console.error(" --onlyTestTsNext Only run with `typescript@next`, not with the minimum version."); console.error(" --localTs path Run with *path* as the latest version of TS."); console.error(""); @@ -113,14 +120,15 @@ function listen(dirPath: string, tsLocal: string | undefined, alwaysOnlyTestTsNe // Don't await this here to ensure that messages sent during installation aren't dropped. const installationPromise = installTypeScriptAsNeeded(tsLocal, alwaysOnlyTestTsNext); process.on("message", async (message: unknown) => { - const { path, onlyTestTsNext, expectOnly } = message as { + const { path, onlyTestTsNext, expectOnly, npmNamingOnly } = message as { path: string; onlyTestTsNext: boolean; expectOnly?: boolean; + npmNamingOnly?: boolean; }; await installationPromise; - runTests(joinPaths(dirPath, path), onlyTestTsNext, !!expectOnly, tsLocal) + runTests(joinPaths(dirPath, path), onlyTestTsNext, !!expectOnly, !!npmNamingOnly, tsLocal) .catch((e) => e.stack) .then((maybeError) => { process.send!({ path, status: maybeError === undefined ? "OK" : maybeError }); @@ -133,6 +141,7 @@ async function runTests( dirPath: string, onlyTestTsNext: boolean, expectOnly: boolean, + npmNamingOnly: boolean, tsLocal: string | undefined, ): Promise { // Assert that we're really on DefinitelyTyped. @@ -157,7 +166,7 @@ async function runTests( 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); + await testTypesVersion(dirPath, tsVersion, tsVersion, expectOnly, npmNamingOnly, 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 @@ -178,7 +187,7 @@ async function runTests( if (lows.length > 1) { console.log("testing from", low, "to", hi, "in", versionPath); } - await testTypesVersion(versionPath, low, hi, expectOnly, undefined, isLatest); + await testTypesVersion(versionPath, low, hi, expectOnly, npmNamingOnly, undefined, isLatest); } } } @@ -200,6 +209,7 @@ async function testTypesVersion( lowVersion: TsVersion, hiVersion: TsVersion, expectOnly: boolean, + npmNamingOnly: boolean, tsLocal: string | undefined, isLatest: boolean, ): Promise { @@ -208,7 +218,7 @@ async function testTypesVersion( if (tsconfigErrors.length > 0) { throw new Error("\n\t* " + tsconfigErrors.join("\n\t* ")); } - const err = await lint(dirPath, lowVersion, hiVersion, isLatest, expectOnly, tsLocal); + const err = await lint(dirPath, lowVersion, hiVersion, isLatest, expectOnly, npmNamingOnly, tsLocal); if (err) { throw new Error(err); } diff --git a/packages/dtslint/src/lint.ts b/packages/dtslint/src/lint.ts index 29ec277d9d..b295612ae9 100644 --- a/packages/dtslint/src/lint.ts +++ b/packages/dtslint/src/lint.ts @@ -17,6 +17,7 @@ export async function lint( maxVersion: TsVersion, isLatest: boolean, expectOnly: boolean, + npmNamingOnly: boolean, tsLocal: string | undefined, ): Promise { const tsconfigPath = joinPaths(dirPath, "tsconfig.json"); @@ -27,8 +28,10 @@ export async function lint( // TODO: To remove tslint, replace this with a ts.createProgram (probably) const lintProgram = Linter.createProgram(tsconfigPath); - // tslint no longer checks ExpectType; skip linting entirely if we're only checking ExpectType. - const linter = !expectOnly ? new Linter({ fix: false, formatter: "stylish" }, lintProgram) : undefined; + // tslint no longer checks ExpectType or npm-naming; skip linting entirely if we're only checking either. + // TODO: soon we shall remove tslint altogether! + const linter = + !expectOnly && !npmNamingOnly ? new Linter({ fix: false, formatter: "stylish" }, lintProgram) : undefined; const configPath = 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 @@ -79,7 +82,7 @@ export async function lint( }, }; - if (expectOnly) { + if (expectOnly || npmNamingOnly) { // Disable the regular config, instead load only the plugins and use just the rule above. // TODO(jakebailey): share this with eslint-plugin options.useEslintrc = false; From 6e50f20440b8e6b1fc7594d0d28798dc0370627a Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 11 Dec 2023 12:27:55 -0500 Subject: [PATCH 05/18] fix: .eslintrc.json, not tslint.json --- packages/eslint-plugin/src/rules/npm-naming.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/npm-naming.ts b/packages/eslint-plugin/src/rules/npm-naming.ts index 5f12883bbe..8f61e84f54 100644 --- a/packages/eslint-plugin/src/rules/npm-naming.ts +++ b/packages/eslint-plugin/src/rules/npm-naming.ts @@ -79,7 +79,7 @@ const rule = createRule<[NpmNamingOptions], "error">({ messages: { error: `{{ error }} 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": +you can disable this check by adding the following options to your project's .eslintrc.json file under "rules": "npm-naming": {{ option }}`, }, From d1c80338cbe9dafa337bb7dae99b20251bb2f849 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 11 Dec 2023 12:34:48 -0500 Subject: [PATCH 06/18] docs: mention npmNamingOnly in .md files too --- packages/dtslint-runner/README.md | 2 ++ packages/dtslint/README.md | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/packages/dtslint-runner/README.md b/packages/dtslint-runner/README.md index 5f2f8c4613..1b030362c9 100644 --- a/packages/dtslint-runner/README.md +++ b/packages/dtslint-runner/README.md @@ -20,6 +20,8 @@ $ node node_modules/@definitelytyped/dtslint-runner/dist/index.js --path . --sel $ node node_modules/@definitelytyped/dtslint-runner/dist/index.js --path . --selection all --noInstall --expectOnly # or test only with the $ExpectType rule, which type checks $ node node_modules/@definitelytyped/dtslint-runner/dist/index.js --path . --selection all --noInstall --expectOnly +# or test only with the npm-naming rule +$ node node_modules/@definitelytyped/dtslint-runner/dist/index.js --path . --selection all --noInstall --npmNamingOnly # and you might just want to test on the latest Typescript $ node node_modules/@definitelytyped/dtslint-runner/dist/index.js --path . --selection all --noInstall --expectOnly --onlyTestTsNext # or a local build of Typescript diff --git a/packages/dtslint/README.md b/packages/dtslint/README.md index 8474d54147..fc606b03ee 100644 --- a/packages/dtslint/README.md +++ b/packages/dtslint/README.md @@ -185,6 +185,14 @@ Disable all the lint rules except the one that checks for type correctness. ```sh dtslint --expectOnly types ``` +- `--npmNamingOnly` + +Disable all the lint rules except the one that checks for npm naming correctness. + +```sh +dtslint --npmNamingOnly types +``` + # Contributing From f7d7c301ba7bf8ce0fbcace886216aa8bc856c94 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 11 Dec 2023 11:01:44 -0800 Subject: [PATCH 07/18] Drop new flag --- packages/dtslint-runner/README.md | 2 -- packages/dtslint-runner/src/index.ts | 7 ------- packages/dtslint-runner/src/main.ts | 2 -- packages/dtslint-runner/src/types.ts | 1 - packages/dtslint/README.md | 8 -------- packages/dtslint/src/index.ts | 22 +++++++--------------- packages/dtslint/src/lint.ts | 6 ++---- 7 files changed, 9 insertions(+), 39 deletions(-) diff --git a/packages/dtslint-runner/README.md b/packages/dtslint-runner/README.md index 1b030362c9..5f2f8c4613 100644 --- a/packages/dtslint-runner/README.md +++ b/packages/dtslint-runner/README.md @@ -20,8 +20,6 @@ $ node node_modules/@definitelytyped/dtslint-runner/dist/index.js --path . --sel $ node node_modules/@definitelytyped/dtslint-runner/dist/index.js --path . --selection all --noInstall --expectOnly # or test only with the $ExpectType rule, which type checks $ node node_modules/@definitelytyped/dtslint-runner/dist/index.js --path . --selection all --noInstall --expectOnly -# or test only with the npm-naming rule -$ node node_modules/@definitelytyped/dtslint-runner/dist/index.js --path . --selection all --noInstall --npmNamingOnly # and you might just want to test on the latest Typescript $ node node_modules/@definitelytyped/dtslint-runner/dist/index.js --path . --selection all --noInstall --expectOnly --onlyTestTsNext # or a local build of Typescript diff --git a/packages/dtslint-runner/src/index.ts b/packages/dtslint-runner/src/index.ts index d7825a4ad2..468caaf642 100644 --- a/packages/dtslint-runner/src/index.ts +++ b/packages/dtslint-runner/src/index.ts @@ -66,12 +66,6 @@ if (require.main === module) { type: "boolean", default: false, }, - npmNamingOnly: { - group: "dtslint options", - description: "Run only the npm-naming lint rule.", - type: "boolean", - default: false, - }, // Only useful for repeated local runs, so I’m hiding it noInstall: { hidden: true, @@ -108,7 +102,6 @@ if (require.main === module) { localTypeScriptPath: !args.onlyTestTsNext ? args.localTypeScriptPath : undefined, onlyTestTsNext: !!args.onlyTestTsNext, expectOnly: args.expectOnly, - npmNamingOnly: args.npmNamingOnly, noInstall: args.noInstall, childRestartTaskInterval: args.childRestartTaskInterval, writeFailures: args.writeFailures, diff --git a/packages/dtslint-runner/src/main.ts b/packages/dtslint-runner/src/main.ts index c6e992a49c..f80cffb53d 100644 --- a/packages/dtslint-runner/src/main.ts +++ b/packages/dtslint-runner/src/main.ts @@ -22,7 +22,6 @@ export async function runDTSLint({ noInstall, onlyTestTsNext, expectOnly, - npmNamingOnly, localTypeScriptPath, nProcesses, shard, @@ -76,7 +75,6 @@ export async function runDTSLint({ path, onlyTestTsNext: onlyTestTsNext || !packageNames.has(path), expectOnly: expectOnly || !packageNames.has(path), - npmNamingOnly: npmNamingOnly || !packageNames.has(path), })), commandLineArgs: dtslintArgs, workerFile: require.resolve("@definitelytyped/dtslint"), diff --git a/packages/dtslint-runner/src/types.ts b/packages/dtslint-runner/src/types.ts index d37bb7831f..381b0d5f34 100644 --- a/packages/dtslint-runner/src/types.ts +++ b/packages/dtslint-runner/src/types.ts @@ -14,7 +14,6 @@ export interface RunDTSLintOptions { noInstall: boolean; onlyTestTsNext: boolean; expectOnly: boolean; - npmNamingOnly: boolean; localTypeScriptPath?: string; nProcesses: number; shard?: { id: number; count: number }; diff --git a/packages/dtslint/README.md b/packages/dtslint/README.md index fc606b03ee..8474d54147 100644 --- a/packages/dtslint/README.md +++ b/packages/dtslint/README.md @@ -185,14 +185,6 @@ Disable all the lint rules except the one that checks for type correctness. ```sh dtslint --expectOnly types ``` -- `--npmNamingOnly` - -Disable all the lint rules except the one that checks for npm naming correctness. - -```sh -dtslint --npmNamingOnly types -``` - # Contributing diff --git a/packages/dtslint/src/index.ts b/packages/dtslint/src/index.ts index 0b631b4e60..e4d42c0681 100644 --- a/packages/dtslint/src/index.ts +++ b/packages/dtslint/src/index.ts @@ -21,7 +21,6 @@ async function main(): Promise { let dirPath = process.cwd(); let onlyTestTsNext = false; let expectOnly = false; - let npmNamingOnly = false; let shouldListen = false; let lookingForTsLocal = false; let tsLocal: string | undefined; @@ -53,9 +52,6 @@ async function main(): Promise { case "--expectOnly": expectOnly = true; break; - case "--npmNamingOnly": - npmNamingOnly = true; - break; case "--onlyTestTsNext": onlyTestTsNext = true; break; @@ -89,7 +85,7 @@ async function main(): Promise { listen(dirPath, tsLocal, onlyTestTsNext); } else { await installTypeScriptAsNeeded(tsLocal, onlyTestTsNext); - await runTests(dirPath, onlyTestTsNext, expectOnly, npmNamingOnly, tsLocal); + await runTests(dirPath, onlyTestTsNext, expectOnly, tsLocal); } } @@ -103,13 +99,12 @@ async function installTypeScriptAsNeeded(tsLocal: string | undefined, onlyTestTs function usage(): void { console.error( - "Usage: dtslint [--version] [--installAll] [--onlyTestTsNext] [--expectOnly] [--npmNamingOnly] [--localTs path]", + "Usage: dtslint [--version] [--installAll] [--onlyTestTsNext] [--expectOnly] [--localTs path]", ); console.error("Args:"); console.error(" --version Print version and exit."); console.error(" --installAll Cleans and installs all TypeScript versions."); console.error(" --expectOnly Run only the ExpectType lint rule."); - console.error(" --npmNamingOnly Run only the npm-naming lint rule."); console.error(" --onlyTestTsNext Only run with `typescript@next`, not with the minimum version."); console.error(" --localTs path Run with *path* as the latest version of TS."); console.error(""); @@ -120,15 +115,14 @@ function listen(dirPath: string, tsLocal: string | undefined, alwaysOnlyTestTsNe // Don't await this here to ensure that messages sent during installation aren't dropped. const installationPromise = installTypeScriptAsNeeded(tsLocal, alwaysOnlyTestTsNext); process.on("message", async (message: unknown) => { - const { path, onlyTestTsNext, expectOnly, npmNamingOnly } = message as { + const { path, onlyTestTsNext, expectOnly } = message as { path: string; onlyTestTsNext: boolean; expectOnly?: boolean; - npmNamingOnly?: boolean; }; await installationPromise; - runTests(joinPaths(dirPath, path), onlyTestTsNext, !!expectOnly, !!npmNamingOnly, tsLocal) + runTests(joinPaths(dirPath, path), onlyTestTsNext, !!expectOnly, tsLocal) .catch((e) => e.stack) .then((maybeError) => { process.send!({ path, status: maybeError === undefined ? "OK" : maybeError }); @@ -141,7 +135,6 @@ async function runTests( dirPath: string, onlyTestTsNext: boolean, expectOnly: boolean, - npmNamingOnly: boolean, tsLocal: string | undefined, ): Promise { // Assert that we're really on DefinitelyTyped. @@ -166,7 +159,7 @@ async function runTests( const minVersion = maxVersion(packageJson.minimumTypeScriptVersion, TypeScriptVersion.lowest); if (onlyTestTsNext || tsLocal) { const tsVersion = tsLocal ? "local" : TypeScriptVersion.latest; - await testTypesVersion(dirPath, tsVersion, tsVersion, expectOnly, npmNamingOnly, 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 @@ -187,7 +180,7 @@ async function runTests( if (lows.length > 1) { console.log("testing from", low, "to", hi, "in", versionPath); } - await testTypesVersion(versionPath, low, hi, expectOnly, npmNamingOnly, undefined, isLatest); + await testTypesVersion(versionPath, low, hi, expectOnly, undefined, isLatest); } } } @@ -209,7 +202,6 @@ async function testTypesVersion( lowVersion: TsVersion, hiVersion: TsVersion, expectOnly: boolean, - npmNamingOnly: boolean, tsLocal: string | undefined, isLatest: boolean, ): Promise { @@ -218,7 +210,7 @@ async function testTypesVersion( if (tsconfigErrors.length > 0) { throw new Error("\n\t* " + tsconfigErrors.join("\n\t* ")); } - const err = await lint(dirPath, lowVersion, hiVersion, isLatest, expectOnly, npmNamingOnly, tsLocal); + const err = await lint(dirPath, lowVersion, hiVersion, isLatest, expectOnly, tsLocal); if (err) { throw new Error(err); } diff --git a/packages/dtslint/src/lint.ts b/packages/dtslint/src/lint.ts index b295612ae9..298a6e1c0d 100644 --- a/packages/dtslint/src/lint.ts +++ b/packages/dtslint/src/lint.ts @@ -17,7 +17,6 @@ export async function lint( maxVersion: TsVersion, isLatest: boolean, expectOnly: boolean, - npmNamingOnly: boolean, tsLocal: string | undefined, ): Promise { const tsconfigPath = joinPaths(dirPath, "tsconfig.json"); @@ -30,8 +29,7 @@ export async function lint( // tslint no longer checks ExpectType or npm-naming; skip linting entirely if we're only checking either. // TODO: soon we shall remove tslint altogether! - const linter = - !expectOnly && !npmNamingOnly ? new Linter({ fix: false, formatter: "stylish" }, lintProgram) : undefined; + const linter = !expectOnly ? new Linter({ fix: false, formatter: "stylish" }, lintProgram) : undefined; const configPath = 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 @@ -82,7 +80,7 @@ export async function lint( }, }; - if (expectOnly || npmNamingOnly) { + if (expectOnly) { // Disable the regular config, instead load only the plugins and use just the rule above. // TODO(jakebailey): share this with eslint-plugin options.useEslintrc = false; From 15a91e80fb89dc51495f0205f0e76e3e5e43a8d2 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 11 Dec 2023 11:11:02 -0800 Subject: [PATCH 08/18] Baseline plugin exports --- .../test/__snapshots__/plugin.test.ts.snap | 656 ++++++++++++++++++ packages/eslint-plugin/test/plugin.test.ts | 7 + 2 files changed, 663 insertions(+) create mode 100644 packages/eslint-plugin/test/__snapshots__/plugin.test.ts.snap create mode 100644 packages/eslint-plugin/test/plugin.test.ts diff --git a/packages/eslint-plugin/test/__snapshots__/plugin.test.ts.snap b/packages/eslint-plugin/test/__snapshots__/plugin.test.ts.snap new file mode 100644 index 0000000000..74b3d9ed7e --- /dev/null +++ b/packages/eslint-plugin/test/__snapshots__/plugin.test.ts.snap @@ -0,0 +1,656 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`plugin should have the expected exports 1`] = ` +{ + "configs": { + "all": { + "overrides": [ + { + "files": [ + "*.cts", + "*.mts", + "*.ts", + "*.tsx", + ], + "parser": "@typescript-eslint/parser", + "parserOptions": { + "project": true, + "warnOnUnsupportedTypeScriptVersion": false, + }, + "rules": { + "@definitelytyped/expect": "error", + "@definitelytyped/export-just-namespace": "error", + "@definitelytyped/no-any-union": "error", + "@definitelytyped/no-bad-reference": "error", + "@definitelytyped/no-const-enum": "error", + "@definitelytyped/no-dead-reference": "error", + "@definitelytyped/no-declare-current-package": "error", + "@definitelytyped/no-import-default-of-export-equals": "error", + "@definitelytyped/no-import-of-dev-dependencies": "error", + "@definitelytyped/no-old-dt-header": "error", + "@definitelytyped/no-relative-import-in-test": "error", + "@definitelytyped/no-self-import": "error", + "@definitelytyped/no-single-declare-module": "error", + "@definitelytyped/no-single-element-tuple-type": "error", + "@definitelytyped/no-unnecessary-generics": "error", + "@definitelytyped/no-useless-files": "error", + "@definitelytyped/npm-naming": "error", + "@definitelytyped/prefer-declare-function": "error", + "@definitelytyped/redundant-undefined": "error", + "@definitelytyped/strict-export-declare-modifiers": "error", + "@typescript-eslint/adjacent-overload-signatures": "error", + "@typescript-eslint/array-type": [ + "error", + { + "default": "array-simple", + }, + ], + "@typescript-eslint/ban-ts-comment": [ + "error", + { + "ts-check": false, + "ts-expect-error": false, + "ts-ignore": "allow-with-description", + "ts-nocheck": true, + }, + ], + "@typescript-eslint/ban-types": [ + "error", + { + "extendDefaults": true, + "types": { + "{}": false, + }, + }, + ], + "@typescript-eslint/consistent-type-definitions": "error", + "@typescript-eslint/explicit-member-accessibility": [ + "error", + { + "accessibility": "no-public", + }, + ], + "@typescript-eslint/naming-convention": [ + "error", + { + "custom": { + "match": false, + "regex": "^I[A-Z]", + }, + "format": [], + "selector": "interface", + }, + ], + "@typescript-eslint/no-empty-interface": "error", + "@typescript-eslint/no-invalid-void-type": [ + "error", + { + "allowAsThisParameter": true, + "allowInGenericTypeArguments": true, + }, + ], + "@typescript-eslint/no-misused-new": "error", + "@typescript-eslint/prefer-namespace-keyword": "error", + "@typescript-eslint/triple-slash-reference": [ + "error", + { + "path": "always", + "types": "prefer-import", + }, + ], + "no-duplicate-imports": "error", + "unicode-bom": [ + "error", + "never", + ], + }, + }, + ], + "plugins": [ + "@definitelytyped", + "@typescript-eslint", + "jsdoc", + ], + "rules": { + "jsdoc/check-tag-names": [ + "error", + { + "definedTags": [ + "addVersion", + "also", + "api", + "author", + "beta", + "brief", + "category", + "cfg", + "chainable", + "check", + "checkReturnValue", + "classDescription", + "condparamprivilege", + "constraint", + "credits", + "declaration", + "defApiFeature", + "defaultValue", + "detail", + "end", + "eventproperty", + "experimental", + "export", + "expose", + "extendscript", + "factory", + "field", + "final", + "fixme", + "fluent", + "for", + "governance", + "header", + "hidden-property", + "hidden", + "id", + "jsx", + "jsxImportSource", + "label", + "language", + "legacy", + "link", + "listen", + "locus", + "methodOf", + "minVersion", + "ngdoc", + "nonstandard", + "note", + "npm", + "observable", + "option", + "optionobject", + "options", + "packageDocumentation", + "param", + "parent", + "platform", + "plugin", + "preserve", + "privateRemarks", + "privilegeLevel", + "privilegeName", + "proposed", + "range", + "readOnly", + "related", + "remark", + "remarks", + "required", + "requires", + "restriction", + "returnType", + "section", + "see", + "since", + "const", + "singleton", + "source", + "struct", + "suppress", + "targetfolder", + "enum", + "title", + "record", + "title", + "TODO", + "trigger", + "triggers", + "typeparam", + "typeParam", + "unsupported", + "url", + "usage", + "warn", + "warning", + "version", + ], + "typed": true, + }, + ], + }, + "settings": { + "jsdoc": { + "tagNamePreference": { + "argument": "argument", + "exception": "exception", + "function": "function", + "method": "method", + "param": "param", + "return": "return", + "returns": "returns", + }, + }, + }, + }, + }, + "meta": { + "name": "@definitelytyped/eslint-plugin", + "version": "0.0.197", + }, + "rules": { + "expect": { + "create": [Function], + "defaultOptions": [ + {}, + ], + "meta": { + "docs": { + "description": "Asserts types with $ExpectType.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/expect.md", + }, + "messages": { + "diagnostic": "TypeScript{{versionNameString}} {{message}}", + "failure": "TypeScript{{versionNameString}} expected type to be: + {{expectedType}} +got: + {{actualType}}", + "needInstall": "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: + + pnpm install -w --filter '...{./types/{{dirPath}}}...' + +Then re-run.", + "noMatch": "Cannot match a node to this assertion. If this is a multiline function call, ensure the assertion is on the line above.", + "noTsconfig": "Could not find a tsconfig.json file.", + "programContents": "Program source files differ between TypeScript versions. This may be a dtslint bug. +Expected to find a file '{{fileName}}' present in 5.2, but did not find it in ts@{{versionName}}.", + "twoAssertions": "This line has 2 $ExpectType assertions.", + }, + "schema": [ + { + "additionalProperties": false, + "properties": { + "versionsToTest": { + "items": { + "additionalProperties": false, + "properties": { + "path": { + "type": "string", + }, + "versionName": { + "type": "string", + }, + }, + "required": [ + "versionName", + "path", + ], + "type": "object", + }, + "type": "array", + }, + }, + "type": "object", + }, + ], + "type": "problem", + }, + }, + "export-just-namespace": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbids \`export = foo\` where \`foo\` is a namespace and isn't merged with a function/class/type/interface.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/export-just-namespace.md", + }, + "messages": { + "useTheBody": "Instead of \`export =\`-ing a namespace, use the body of the namespace as the module body.", + }, + "schema": [], + "type": "problem", + }, + }, + "no-any-union": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbid a union to contain \`any\`", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-any-union.md", + }, + "messages": { + "anyUnion": "Including \`any\` in a union will override all other members of the union.", + }, + "schema": [], + "type": "problem", + }, + }, + "no-bad-reference": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbids bad references, including those that resolve outside of the package or path references in non-declaration files.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-bad-reference.md", + }, + "messages": { + "backslashes": "Use forward slashes in paths.", + "importLeaves": "The import "{{text}}" resolves to the current package, but uses relative paths.", + "importOutside": "The import "{{text}}" resolves outside of the package. Use a bare import to reference other packages.", + "referenceLeaves": "The reference "{{text}}" resolves to the current package, but uses relative paths.", + "referenceOutside": "The reference "{{text}}" resolves outside of the package. Use a global reference to reference other packages.", + "testReference": "The path reference "{{text}}" is disallowed outside declaration files. Use "" or include the file in tsconfig instead.", + }, + "schema": [], + "type": "problem", + }, + }, + "no-const-enum": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbid \`const enum\`", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-const-enum.md", + }, + "messages": { + "constEnum": "Use of \`const enum\` is forbidden.", + }, + "schema": [], + "type": "problem", + }, + }, + "no-dead-reference": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Ensures that all \`/// \` comments go at the top of the file.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-dead-reference.md", + }, + "messages": { + "referenceAtTop": "\`/// \` directive must be at top of file to take effect.", + }, + "schema": [], + "type": "problem", + }, + }, + "no-declare-current-package": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Don't use an ambient module declaration of the current package; use a normal module.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-declare-current-package.md", + }, + "messages": { + "noDeclareCurrentPackage": "Instead of declaring a module with \`declare module "{{ text }}"\`, write its contents in directly in {{ preferred }}.", + }, + "schema": [], + "type": "problem", + }, + }, + "no-import-default-of-export-equals": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbid a default import to reference an \`export =\` module.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-import-default-of-export-equals.md", + }, + "messages": { + "noImportDefaultOfExportEquals": "The module {{moduleName}} uses \`export = \`. Import with \`import {{importName}} = require({{moduleName}})\`.", + }, + "schema": [], + "type": "problem", + }, + }, + "no-import-of-dev-dependencies": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbid imports and references to devDependencies inside .d.ts files.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-import-of-dev-dependencies.md", + }, + "messages": { + "noImportOfDevDependencies": ".d.ts files may not import packages in devDependencies.", + "noReferenceOfDevDependencies": ".d.ts files may not triple-slash reference packages in devDependencies.", + }, + "schema": [], + "type": "problem", + }, + }, + "no-old-dt-header": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbids in all files, in declaration files, and all in test files.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-bad-reference.md", + }, + "messages": { + "noOldDTHeader": "Specify package metadata in package.json. Do not use a header like \`// Type definitions for foo 1.2\`", + }, + "schema": [], + "type": "problem", + }, + }, + "no-relative-import-in-test": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbids test (non-declaration) files to use relative imports.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-relative-import-in-test.md", + }, + "messages": { + "useGlobalImport": "Test file should not use a relative import. Use a global import as if this were a user of the package.", + }, + "schema": [], + "type": "problem", + }, + }, + "no-self-import": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbids declaration files to import the current package using a global import or old versions with a relative import.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-self-import.md", + }, + "messages": { + "useOnlyCurrentVersion": "Don't import an old version of the current package.", + "useRelativeImport": "Declaration file should not use a global import of itself. Use a relative import.", + }, + "schema": [], + "type": "problem", + }, + }, + "no-single-declare-module": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Don't use an ambient module declaration if there's just one -- write it as a normal module.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-single-declare-module.md", + }, + "messages": { + "oneModuleDeclaration": "File has only 1 ambient module declaration. Move the contents outside the ambient module block, rename the file to match the ambient module name, and remove the block.", + }, + "schema": [], + "type": "problem", + }, + }, + "no-single-element-tuple-type": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbids \`[T]\`, which should be \`T[]\`.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-single-element-tuple-type.md", + }, + "messages": { + "singleElementTupleType": "Type [T] is a single-element tuple type. You probably meant T[].", + }, + "schema": [], + "type": "problem", + }, + }, + "no-unnecessary-generics": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbids signatures using a generic parameter only once.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-unnecessary-generics.md", + }, + "messages": { + "never": "Type parameter {{name}} is never used.", + "sole": "Type parameter {{name}} is used only once.", + }, + "schema": [], + "type": "problem", + }, + }, + "no-useless-files": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbids files with no content.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/no-useless-files.md", + }, + "messages": { + "noContent": "File has no content.", + }, + "schema": [], + "type": "problem", + }, + }, + "npm-naming": { + "create": [Function], + "defaultOptions": [ + { + "mode": "name-only", + }, + ], + "meta": { + "docs": { + "description": "Ensure that package name and DefinitelyTyped header match npm package info.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/npm-naming.md", + }, + "messages": { + "error": "{{ error }} +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 .eslintrc.json file under "rules": + + "npm-naming": {{ option }}", + }, + "schema": [ + { + "oneOf": [ + { + "additionalProperties": false, + "properties": { + "mode": { + "enum": [ + "name-only", + ], + "type": "string", + }, + }, + "type": "object", + }, + { + "additionalProperties": false, + "properties": { + "errors": { + "items": { + "items": [ + { + "description": "Name of the check.", + "enum": [ + "NeedsExportEquals", + "NoDefaultExport", + ], + "type": "string", + }, + { + "description": "Whether the check is enabled or disabled.", + "type": "boolean", + }, + ], + "maxItems": 2, + "minItems": 2, + "type": "array", + }, + "type": "array", + }, + "mode": { + "enum": [ + "code", + ], + "type": "string", + }, + }, + "type": "object", + }, + ], + }, + ], + "type": "problem", + }, + }, + "prefer-declare-function": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbids \`const x: () => void\`.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/prefer-declare-function.md", + }, + "messages": { + "variableFunction": "Use a function declaration instead of a variable of function type.", + }, + "schema": [], + "type": "problem", + }, + }, + "redundant-undefined": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Forbids optional parameters from including an explicit \`undefined\` in their type; requires it in optional properties.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/redundant-undefined.md", + }, + "messages": { + "redundantUndefined": "Parameter is optional, so no need to include \`undefined\` in the type.", + }, + "schema": [], + "type": "problem", + }, + }, + "strict-export-declare-modifiers": { + "create": [Function], + "defaultOptions": [], + "meta": { + "docs": { + "description": "Enforces strict rules about where the 'export' and 'declare' modifiers may appear.", + "url": "https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/eslint-plugin/docs/rules/strict-export-declare-modifiers.md", + }, + "messages": { + "missingExplicitExport": "All declarations in this module are exported automatically. Prefer to explicitly write 'export' for clarity. If you have a good reason not to export this declaration, add 'export {}' to the module to shut off automatic exporting.", + "redundantDeclare": "'declare' keyword is redundant here.", + "redundantExport": "'export' keyword is redundant here because all declarations in this module are exported automatically. If you have a good reason to export some declarations and not others, add 'export {}' to the module to shut off automatic exporting.", + }, + "schema": [], + "type": "problem", + }, + }, + }, +} +`; diff --git a/packages/eslint-plugin/test/plugin.test.ts b/packages/eslint-plugin/test/plugin.test.ts new file mode 100644 index 0000000000..eb29692182 --- /dev/null +++ b/packages/eslint-plugin/test/plugin.test.ts @@ -0,0 +1,7 @@ +import plugin = require("../src/index"); + +describe("plugin", () => { + it("should have the expected exports", () => { + expect(plugin).toMatchSnapshot(); + }); +}); From e692e205ba13c09f1bfd8795ad42ef72d95abf33 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 11 Dec 2023 11:11:24 -0800 Subject: [PATCH 09/18] Remove npm-naming from preset --- packages/eslint-plugin/src/configs/all.ts | 8 +++++++- .../eslint-plugin/test/__snapshots__/plugin.test.ts.snap | 1 - 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 92d37100e8..683a8bcabb 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -135,7 +135,13 @@ export const all: Linter.BaseConfig = { warnOnUnsupportedTypeScriptVersion: false, }, rules: { - ...Object.fromEntries(Object.keys(rules).map((name) => [`@definitelytyped/${name}`, "error"])), + ...Object.fromEntries( + Object.keys(rules) + // npm-naming is only enabled within dtslint. + // Leave it out of the preset so editors / he tests don't hit the network. + .filter((name) => name !== "npm-naming") + .map((name) => [`@definitelytyped/${name}`, "error"]), + ), "unicode-bom": ["error", "never"], "@typescript-eslint/ban-ts-comment": [ "error", diff --git a/packages/eslint-plugin/test/__snapshots__/plugin.test.ts.snap b/packages/eslint-plugin/test/__snapshots__/plugin.test.ts.snap index 74b3d9ed7e..32ff3bb6d5 100644 --- a/packages/eslint-plugin/test/__snapshots__/plugin.test.ts.snap +++ b/packages/eslint-plugin/test/__snapshots__/plugin.test.ts.snap @@ -34,7 +34,6 @@ exports[`plugin should have the expected exports 1`] = ` "@definitelytyped/no-single-element-tuple-type": "error", "@definitelytyped/no-unnecessary-generics": "error", "@definitelytyped/no-useless-files": "error", - "@definitelytyped/npm-naming": "error", "@definitelytyped/prefer-declare-function": "error", "@definitelytyped/redundant-undefined": "error", "@definitelytyped/strict-export-declare-modifiers": "error", From f2950e04c67f9fef02327b0c42aa21475b005b04 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 11 Dec 2023 11:11:56 -0800 Subject: [PATCH 10/18] Formatting --- packages/dtslint/src/index.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/dtslint/src/index.ts b/packages/dtslint/src/index.ts index e4d42c0681..5e4aa3b302 100644 --- a/packages/dtslint/src/index.ts +++ b/packages/dtslint/src/index.ts @@ -98,9 +98,7 @@ async function installTypeScriptAsNeeded(tsLocal: string | undefined, onlyTestTs } function usage(): void { - console.error( - "Usage: dtslint [--version] [--installAll] [--onlyTestTsNext] [--expectOnly] [--localTs path]", - ); + console.error("Usage: dtslint [--version] [--installAll] [--onlyTestTsNext] [--expectOnly] [--localTs path]"); console.error("Args:"); console.error(" --version Print version and exit."); console.error(" --installAll Cleans and installs all TypeScript versions."); From 351b5e72f32bae8a640866859edc8bb479f9b6f5 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 11 Dec 2023 11:12:45 -0800 Subject: [PATCH 11/18] knip --- packages/dtslint/package.json | 1 - packages/dtslint/src/util.ts | 25 ------------------------- pnpm-lock.yaml | 3 --- 3 files changed, 29 deletions(-) diff --git a/packages/dtslint/package.json b/packages/dtslint/package.json index ca5251681b..be29bce514 100644 --- a/packages/dtslint/package.json +++ b/packages/dtslint/package.json @@ -22,7 +22,6 @@ "test": "../../node_modules/.bin/jest --config ../../jest.config.js packages/dtslint" }, "dependencies": { - "@definitelytyped/dts-critic": "workspace:*", "@definitelytyped/header-parser": "workspace:*", "@definitelytyped/typescript-versions": "workspace:*", "@definitelytyped/utils": "workspace:*", diff --git a/packages/dtslint/src/util.ts b/packages/dtslint/src/util.ts index 3c4d8a195c..5142cf2265 100644 --- a/packages/dtslint/src/util.ts +++ b/packages/dtslint/src/util.ts @@ -12,10 +12,6 @@ export function readJson(path: string) { return JSON.parse(stripJsonComments(text)); } -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 function getCompilerOptions(dirPath: string): ts.CompilerOptions { const tsconfigPath = join(dirPath, "tsconfig.json"); if (!fs.existsSync(tsconfigPath)) { @@ -23,24 +19,3 @@ export function getCompilerOptions(dirPath: string): ts.CompilerOptions { } return readJson(tsconfigPath).compilerOptions as ts.CompilerOptions; } - -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") { - return true; - } - - if (basename(fileName) !== "index.d.ts") { - return false; - } - - let parent = dirname(fileName); - // May be a directory for an older version, e.g. `v0`. - // Note a types redirect `foo/ts3.1` should not have its own header. - if (allowNested && /^v(0\.)?\d+$/.test(basename(parent))) { - parent = dirname(parent); - } - - // Allow "types/foo/index.d.ts", not "types/foo/utils/index.d.ts" - return basename(dirname(parent)) === "types"; -} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f4d105d4fe..5f05a74a98 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -131,9 +131,6 @@ importers: packages/dtslint: dependencies: - '@definitelytyped/dts-critic': - specifier: workspace:* - version: link:../dts-critic '@definitelytyped/header-parser': specifier: workspace:* version: link:../header-parser From 441aeee2770bada407bfd6c6a9fc2aa219917967 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 11 Dec 2023 11:15:06 -0800 Subject: [PATCH 12/18] Make tslint instance undefined --- packages/dtslint/src/lint.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/dtslint/src/lint.ts b/packages/dtslint/src/lint.ts index 298a6e1c0d..bf8bf54153 100644 --- a/packages/dtslint/src/lint.ts +++ b/packages/dtslint/src/lint.ts @@ -27,9 +27,8 @@ export async function lint( // TODO: To remove tslint, replace this with a ts.createProgram (probably) const lintProgram = Linter.createProgram(tsconfigPath); - // tslint no longer checks ExpectType or npm-naming; skip linting entirely if we're only checking either. - // TODO: soon we shall remove tslint altogether! - const linter = !expectOnly ? new Linter({ fix: false, formatter: "stylish" }, lintProgram) : undefined; + // TODO: remove tslint entirely + const linter = undefined as Linter | undefined; const configPath = 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 From 24cc08f1a57e5baa057e17c2f905a679de362842 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 11 Dec 2023 11:18:19 -0800 Subject: [PATCH 13/18] Undo changes in fixture eslintrc --- packages/eslint-plugin/test/fixtures/.eslintrc.cjs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/eslint-plugin/test/fixtures/.eslintrc.cjs b/packages/eslint-plugin/test/fixtures/.eslintrc.cjs index fd4755c695..50c87c7fd9 100644 --- a/packages/eslint-plugin/test/fixtures/.eslintrc.cjs +++ b/packages/eslint-plugin/test/fixtures/.eslintrc.cjs @@ -1,7 +1,4 @@ module.exports = { - extends: ["plugin:@definitelytyped/all"], root: true, - rules: { - "@definitelytyped/npm-naming": "off" - } + extends: ["plugin:@definitelytyped/all"], }; From 159dc92ce872f15ea7493dfa0ab8f3be17e5658f Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 11 Dec 2023 11:20:19 -0800 Subject: [PATCH 14/18] Changeset --- .changeset/eighty-jobs-wave.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/eighty-jobs-wave.md diff --git a/.changeset/eighty-jobs-wave.md b/.changeset/eighty-jobs-wave.md new file mode 100644 index 0000000000..42fdad5811 --- /dev/null +++ b/.changeset/eighty-jobs-wave.md @@ -0,0 +1,7 @@ +--- +"@definitelytyped/eslint-plugin": patch +"@definitelytyped/dts-critic": patch +"@definitelytyped/dtslint": patch +--- + +Move npm-naming lint rule from tslint to eslint From 151f33b2a656b32b01a3b7402f582426e76859ac Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 11 Dec 2023 12:51:20 -0800 Subject: [PATCH 15/18] Forgot to actually enable the rule, oops --- packages/dtslint/src/lint.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/dtslint/src/lint.ts b/packages/dtslint/src/lint.ts index bf8bf54153..0b39748e9e 100644 --- a/packages/dtslint/src/lint.ts +++ b/packages/dtslint/src/lint.ts @@ -73,6 +73,7 @@ export async function lint( files: ["*.ts", "*.cts", "*.mts", "*.tsx"], rules: { "@definitelytyped/expect": ["error", { versionsToTest }], + "@definitelytyped/npm-naming": "error", }, }, ], From 81eae9391eef63182858fa043a0132a20836af5b Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 11 Dec 2023 12:58:07 -0800 Subject: [PATCH 16/18] npm apparently returns a single string when there's one version --- packages/dts-critic/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/dts-critic/index.ts b/packages/dts-critic/index.ts index 8a298634c2..c261d1e169 100644 --- a/packages/dts-critic/index.ts +++ b/packages/dts-critic/index.ts @@ -206,7 +206,7 @@ export function getNpmInfo(name: string): NpmInfo { } return { isNpm: true, - versions: info.versions as string[], + versions: Array.isArray(info.versions) ? info.versions : [info.versions], tags: info["dist-tags"] as { [tag: string]: string | undefined }, }; } From 4c991ae46f1b22fd7202b4938b31209aa8cc11c8 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 11 Dec 2023 14:55:38 -0800 Subject: [PATCH 17/18] Set npm-naming in baseConfig so it can be overridden --- packages/dtslint/src/lint.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/dtslint/src/lint.ts b/packages/dtslint/src/lint.ts index 0b39748e9e..2a9fa1cff4 100644 --- a/packages/dtslint/src/lint.ts +++ b/packages/dtslint/src/lint.ts @@ -67,13 +67,22 @@ export async function lint( const options: ESLint.Options = { cwd: dirPath, + baseConfig: { + overrides: [ + { + files: ["*.ts", "*.cts", "*.mts", "*.tsx"], + rules: { + "@definitelytyped/npm-naming": "error", + }, + }, + ], + }, overrideConfig: { overrides: [ { files: ["*.ts", "*.cts", "*.mts", "*.tsx"], rules: { "@definitelytyped/expect": ["error", { versionsToTest }], - "@definitelytyped/npm-naming": "error", }, }, ], From a0b842d5121bedb47c2ead6af790c730b6d9404a Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 11 Dec 2023 15:53:36 -0800 Subject: [PATCH 18/18] Fix message for disabling lint --- packages/eslint-plugin/src/rules/npm-naming.ts | 2 +- packages/eslint-plugin/test/__snapshots__/plugin.test.ts.snap | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/npm-naming.ts b/packages/eslint-plugin/src/rules/npm-naming.ts index 8f61e84f54..33e5d70703 100644 --- a/packages/eslint-plugin/src/rules/npm-naming.ts +++ b/packages/eslint-plugin/src/rules/npm-naming.ts @@ -81,7 +81,7 @@ const rule = createRule<[NpmNamingOptions], "error">({ 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 .eslintrc.json file under "rules": - "npm-naming": {{ option }}`, + "@definitelytyped/npm-naming": {{ option }}`, }, schema: [ { diff --git a/packages/eslint-plugin/test/__snapshots__/plugin.test.ts.snap b/packages/eslint-plugin/test/__snapshots__/plugin.test.ts.snap index 32ff3bb6d5..9c3bd2f204 100644 --- a/packages/eslint-plugin/test/__snapshots__/plugin.test.ts.snap +++ b/packages/eslint-plugin/test/__snapshots__/plugin.test.ts.snap @@ -546,7 +546,7 @@ Expected to find a file '{{fileName}}' present in 5.2, but did not find it in ts 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 .eslintrc.json file under "rules": - "npm-naming": {{ option }}", + "@definitelytyped/npm-naming": {{ option }}", }, "schema": [ {