From 12d5edf6679a1b7a43d07852ee2525886fe7c802 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Fri, 22 Mar 2019 09:13:09 -0700 Subject: [PATCH] Move dtscritic to tslint rule (#218) * update to newest version of dts-critic * First attempt. Works but tests fail. Because dts-critic inspects the filesystem, and the test filesystem has extra .lint extensions, and I don't care enough to make it understand the tslint test environment. * No new tests, but tests work now I had to hack dts-critic though, with a new entry point: ```js /** @param {string} dts - the text of the d.ts */ dtsCritic.alternate = function (dts, dtsPath) { let header; try { header = headerParser.parseHeaderOrFail(dts); } catch(e) { header = undefined; } const names = findNames(dtsPath, undefined, header) const src = download("https://unpkg.com/" + mangleScoped(names.src)); checkNames(names, header); if (header && !header.nonNpm) { checkSource(names.dts, dts, src); } } ``` and modified findDtsName to know about .lint extension in tests. This second thing is not good and should probably be inverted as well: ``` /** * If dtsName is 'index' (as with DT) then look to the parent directory for the name. * @param {string} dtsPath */ function findDtsName(dtsPath) { const resolved = path.resolve(dtsPath); const baseName = path.basename(resolved, '.d.ts'); if (baseName && baseName !== "index") { const baseName2 = path.basename(resolved, '.d.ts.lint'); if (baseName2 && baseName2 !== "index") { return baseName; } } return path.basename(path.dirname(resolved)); } ``` * Add one test * move dts-critic into new rule * Turn on rule and give more error spans * Revert test changes -- this was a manual test * Missed test revert --- dtslint.json | 1 + package-lock.json | 108 +++++++++++-------------------------- package.json | 2 +- src/index.ts | 28 +--------- src/rules/dtHeaderRule.ts | 25 +-------- src/rules/npmNamingRule.ts | 57 ++++++++++++++++++++ src/util.ts | 21 ++++++++ 7 files changed, 113 insertions(+), 129 deletions(-) create mode 100644 src/rules/npmNamingRule.ts diff --git a/dtslint.json b/dtslint.json index 2a704edd..ac6f7c4a 100644 --- a/dtslint.json +++ b/dtslint.json @@ -21,6 +21,7 @@ "trim-file": true, "unified-signatures": true, "void-return": true, + "npm-naming": true, "comment-format": [true, "check-space"], // But not check-uppercase or check-lowercase "interface-name": [true, "never-prefix"], diff --git a/package-lock.json b/package-lock.json index ab979b42..4ca5075a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "dtslint", - "version": "0.5.4", + "version": "0.5.5", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -108,7 +108,7 @@ "dependencies": { "chalk": { "version": "1.1.3", - "resolved": "https://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", + "resolved": "http://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", "integrity": "sha1-qBFcVeSnAv5NFQq9OHKCKn4J/Jg=", "requires": { "ansi-styles": "^2.2.1", @@ -148,9 +148,9 @@ "integrity": "sha1-Jw8HbFpywC9bZaR9+Uxf46J4iS8=" }, "camelcase": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/camelcase/-/camelcase-5.0.0.tgz", - "integrity": "sha512-faqwZqnWxbxn+F1d399ygeamQNy3lPp/H9H6rNrqYh4FSVCtcY+3cub1MxA8o9mDd55mM8Aghuu/kuyYA6VTsA==" + "version": "5.2.0", + "resolved": "https://registry.npmjs.org/camelcase/-/camelcase-5.2.0.tgz", + "integrity": "sha512-IXFsBS2pC+X0j0N/GE7Dm7j3bsEBp+oTpb7F50dwEVX7rf3IgwO9XatnegTsDtniKCUtEJH4fSU6Asw7uoVLfQ==" }, "caseless": { "version": "0.12.0", @@ -295,24 +295,19 @@ "resolved": "https://registry.npmjs.org/diff/-/diff-3.5.0.tgz", "integrity": "sha512-A46qtFgd+g7pDZinpnwiRJtxbC1hpgf0uzP3iG89scHk0AUC7A1TGxf5OiiOUv/JMZR8GOt8hL900hV0bOy5xA==" }, + "download-file-sync": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/download-file-sync/-/download-file-sync-1.0.4.tgz", + "integrity": "sha1-0+PFQ/g29BA5RVuQNMcuNVsDYBk=" + }, "dts-critic": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/dts-critic/-/dts-critic-1.0.1.tgz", - "integrity": "sha512-zWiELfgF4lTg7PvJxLsk3isw/A6TaxdMlFN0Jk3DV1Rv/6pDXaGpHigjW5b9g7ROnFUWSfPtvwwFBm/OvdHJjA==", + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/dts-critic/-/dts-critic-1.0.4.tgz", + "integrity": "sha512-pBIkn1L8fFtYnoftv6e6KYO9zieDQU0AbbdKZWEp/M3VFvDyythNfcAUv+QRncboQQxmo3bDGBoVkZbSXdzBBA==", "requires": { - "definitelytyped-header-parser": "github:Microsoft/definitelytyped-header-parser#production", - "request-promise-native": "^1.0.5", + "definitelytyped-header-parser": "^1.0.1", + "download-file-sync": "^1.0.4", "yargs": "^12.0.5" - }, - "dependencies": { - "definitelytyped-header-parser": { - "version": "github:Microsoft/definitelytyped-header-parser#d957ad0bb2f4ecb60ac04f734e0b38fbc8e70b8a", - "from": "github:Microsoft/definitelytyped-header-parser#production", - "requires": { - "@types/parsimmon": "^1.3.0", - "parsimmon": "^1.2.0" - } - } } }, "ecc-jsbn": { @@ -514,11 +509,6 @@ "resolved": "https://registry.npmjs.org/invert-kv/-/invert-kv-2.0.0.tgz", "integrity": "sha512-wPVv/y/QQ/Uiirj/vh3oP+1Ww+AWehmi1g5fFWGPF6IpCBCDVrhgHRMvrLfdYcwDh3QJbGXDW4JAuzxElLSqKA==" }, - "ip-regex": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/ip-regex/-/ip-regex-2.1.0.tgz", - "integrity": "sha1-+ni/XS5pE8kRzp+BnuUUa7bYROk=" - }, "is-fullwidth-code-point": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-2.0.0.tgz", @@ -614,11 +604,6 @@ "path-exists": "^3.0.0" } }, - "lodash": { - "version": "4.17.11", - "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.11.tgz", - "integrity": "sha512-cQKh8igo5QUhZ7lg38DYWAxMvjSAKG0A8wGSVimP07SIUEK2UO+arSRKbRZWtelMtN5V0Hkwh5ryOto/SshYIg==" - }, "map-age-cleaner": { "version": "0.1.3", "resolved": "https://registry.npmjs.org/map-age-cleaner/-/map-age-cleaner-0.1.3.tgz", @@ -628,12 +613,12 @@ } }, "mem": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/mem/-/mem-4.1.0.tgz", - "integrity": "sha512-I5u6Q1x7wxO0kdOpYBB28xueHADYps5uty/zg936CiG8NTe5sJL8EjrCuLneuDW3PlMdZBGDIn8BirEVdovZvg==", + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/mem/-/mem-4.2.0.tgz", + "integrity": "sha512-5fJxa68urlY0Ir8ijatKa3eRz5lwXnRCTvo9+TbTGAuTFJOwpGcY0X05moBd0nW45965Njt4CDI2GFQoG8DvqA==", "requires": { "map-age-cleaner": "^0.1.1", - "mimic-fn": "^1.0.0", + "mimic-fn": "^2.0.0", "p-is-promise": "^2.0.0" } }, @@ -651,9 +636,9 @@ } }, "mimic-fn": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/mimic-fn/-/mimic-fn-1.2.0.tgz", - "integrity": "sha512-jf84uxzwiuiIVKiOLpfYk7N46TSy8ubTonmneY9vrpHNAnp0QBt2BxWV9dO3/j+BoVAb+a5G6YDPW3M5HOdMWQ==" + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/mimic-fn/-/mimic-fn-2.0.0.tgz", + "integrity": "sha512-jbex9Yd/3lmICXwYT6gA/j2mNQGU48wCh/VzRd+/Y/PjYQtlg1gLMdZqvu9s/xH7qKvngxRObl56XZR609IMbA==" }, "minimatch": { "version": "3.0.4", @@ -733,9 +718,9 @@ "integrity": "sha512-pzQPhYMCAgLAKPWD2jC3Se9fEfrD9npNos0y150EeqZll7akhEgGhTW/slB6lHku8AvYGiJ+YJ5hfHKePPgFWg==" }, "p-limit": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-2.1.0.tgz", - "integrity": "sha512-NhURkNcrVB+8hNfLuysU8enY5xn2KXphsHBaC2YmRNTZRc7RWusw6apSpdEj3jo4CMb6W9nrF6tTnsJsJeyu6g==", + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-2.2.0.tgz", + "integrity": "sha512-pZbTJpoUsCzV48Mc9Nh51VbwO0X9cuPFE8gYwx9BTCt9SF8/b7Zljd2fVgOxhIF/HDTKgpVzs+GPhyKfjLLFRQ==", "requires": { "p-try": "^2.0.0" } @@ -749,9 +734,9 @@ } }, "p-try": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/p-try/-/p-try-2.0.0.tgz", - "integrity": "sha512-hMp0onDKIajHfIkdRk3P4CdCmErkYAxxDtP3Wx/4nZ3aGlau2VKh3mZpcuFkH27WQkL/3WBCPOktzA9ZOAnMQQ==" + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/p-try/-/p-try-2.1.0.tgz", + "integrity": "sha512-H2RyIJ7+A3rjkwKC2l5GGtU4H1vkxKCAGsWasNVd0Set+6i4znxbWy6/j16YDPJDWxhsgZiKAstMEP8wCdSpjA==" }, "parsimmon": { "version": "1.12.0", @@ -765,7 +750,7 @@ }, "path-is-absolute": { "version": "1.0.1", - "resolved": "https://registry.npmjs.org/path-is-absolute/-/path-is-absolute-1.0.1.tgz", + "resolved": "http://registry.npmjs.org/path-is-absolute/-/path-is-absolute-1.0.1.tgz", "integrity": "sha1-F0uSaHNVNP+8es5r9TpanhtcX18=" }, "path-key": { @@ -850,24 +835,6 @@ } } }, - "request-promise-core": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/request-promise-core/-/request-promise-core-1.1.1.tgz", - "integrity": "sha1-Pu4AssWqgyOc+wTFcA2jb4HNCLY=", - "requires": { - "lodash": "^4.13.1" - } - }, - "request-promise-native": { - "version": "1.0.5", - "resolved": "https://registry.npmjs.org/request-promise-native/-/request-promise-native-1.0.5.tgz", - "integrity": "sha1-UoF3D2jgyXGeUWP9P6tIIhX0/aU=", - "requires": { - "request-promise-core": "1.1.1", - "stealthy-require": "^1.1.0", - "tough-cookie": ">=2.3.3" - } - }, "require-directory": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/require-directory/-/require-directory-2.1.1.tgz", @@ -926,7 +893,7 @@ }, "sprintf-js": { "version": "1.0.3", - "resolved": "https://registry.npmjs.org/sprintf-js/-/sprintf-js-1.0.3.tgz", + "resolved": "http://registry.npmjs.org/sprintf-js/-/sprintf-js-1.0.3.tgz", "integrity": "sha1-BOaSb2YolTVPPdAVIDYzuFcpfiw=" }, "sshpk": { @@ -945,11 +912,6 @@ "tweetnacl": "~0.14.0" } }, - "stealthy-require": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/stealthy-require/-/stealthy-require-1.1.1.tgz", - "integrity": "sha1-NbCYdbT/SfJqd35QmzCQoyJr8ks=" - }, "string-width": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/string-width/-/string-width-2.1.1.tgz", @@ -997,16 +959,6 @@ "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-2.0.0.tgz", "integrity": "sha1-U10EXOa2Nj+kARcIRimZXp3zJMc=" }, - "tough-cookie": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/tough-cookie/-/tough-cookie-3.0.1.tgz", - "integrity": "sha512-yQyJ0u4pZsv9D4clxO69OEjLWYw+jbgspjTue4lTQZLfV0c5l1VmK2y1JK8E9ahdpltPOaAThPcp5nKPUgSnsg==", - "requires": { - "ip-regex": "^2.1.0", - "psl": "^1.1.28", - "punycode": "^2.1.1" - } - }, "tslib": { "version": "1.9.3", "resolved": "https://registry.npmjs.org/tslib/-/tslib-1.9.3.tgz", diff --git a/package.json b/package.json index b69b99c8..bac067e3 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ }, "dependencies": { "definitelytyped-header-parser": "^1.0.1", - "dts-critic": "^1.0.1", + "dts-critic": "^1.0.4", "fs-extra": "^6.0.1", "request": "^2.88.0", "strip-json-comments": "^2.0.1", diff --git a/src/index.ts b/src/index.ts index 5b9eda6b..c0e4bb71 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,13 +1,11 @@ -#!/usr/bin/env node import { isTypeScriptVersion, parseTypeScriptVersionLine, TypeScriptVersion } from "definitelytyped-header-parser"; -import { pathExists, readdir, readFile, stat } from "fs-extra"; +import { readdir, readFile, stat } from "fs-extra"; import { basename, dirname, join as joinPaths } from "path"; -import critic = require("dts-critic"); import { checkPackageJson, checkTsconfig } from "./checks"; import { cleanInstalls, installAll, installNext } from "./installer"; import { checkTslintJson, lint, TsVersion } from "./lint"; -import { assertDefined, last, mapDefinedAsync, readJson, withoutPrefix } from "./util"; +import { assertDefined, last, mapDefinedAsync, withoutPrefix } from "./util"; async function main(): Promise { const args = process.argv.slice(2); @@ -144,9 +142,6 @@ async function runTests( }); if (dt) { - if (!expectOnly && await hasDtHeaderLintRule(joinPaths(dirPath, "tslint.json")) && isToplevelDtPath(dirPath)) { - await critic(joinPaths(dirPath, "index.d.ts")); - } await checkPackageJson(dirPath, typesVersions); } @@ -179,25 +174,6 @@ async function runTests( } } -function isToplevelDtPath(dirPath: string) { - return basename(dirname(dirPath)) === "types" && - basename(dirname(dirname(dirPath))) === "DefinitelyTyped"; - -} - -async function hasDtHeaderLintRule(tslintPath: string) { - if (await pathExists(tslintPath)) { - const tslint = await readJson(tslintPath); - if (tslint.rules && tslint.rules["dt-header"] !== undefined) { - return !!tslint.rules["dt-header"]; - } - - // if dt-header is not present, assume that tslint.json extends dtslint.json - return true; - } - return false; -} - async function testTypesVersion( dirPath: string, lowVersion: TsVersion | undefined, diff --git a/src/rules/dtHeaderRule.ts b/src/rules/dtHeaderRule.ts index a5eb62d7..a8eededa 100644 --- a/src/rules/dtHeaderRule.ts +++ b/src/rules/dtHeaderRule.ts @@ -1,9 +1,7 @@ import { renderExpected, validate } from "definitelytyped-header-parser"; -import { basename, dirname } from "path"; import * as Lint from "tslint"; import * as ts from "typescript"; - -import { failure } from "../util"; +import { failure, isMainFile } from "../util"; export class Rule extends Lint.Rules.AbstractRule { static metadata: Lint.IRuleMetadata = { @@ -44,24 +42,3 @@ function walk(ctx: Lint.WalkContext): void { } // Don't recurse, we're done. } - -function isMainFile(fileName: string) { - // 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 (/^v\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/src/rules/npmNamingRule.ts b/src/rules/npmNamingRule.ts new file mode 100644 index 00000000..596562b9 --- /dev/null +++ b/src/rules/npmNamingRule.ts @@ -0,0 +1,57 @@ +import * as Lint from "tslint"; +import * as ts from "typescript"; +import critic = require("dts-critic"); + +import { failure, isMainFile } from "../util"; + +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: "Not configurable.", + options: null, + type: "functionality", + typescriptOnly: true, + }; + + apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + return this.applyWithFunction(sourceFile, walk); + } +} + +function walk(ctx: Lint.WalkContext): void { + const { sourceFile } = ctx; + const { text } = sourceFile; + const lookFor = (search: string, explanation: string) => { + const idx = text.indexOf(search); + if (idx !== -1) { + ctx.addFailureAt(idx, search.length, failure(Rule.metadata.ruleName, explanation)); + } + }; + if (isMainFile(sourceFile.fileName)) { + try { + critic(sourceFile.fileName); + } + catch (e) { + // TODO: dts-critic should + // 1. not really be using exceptions, but lists + // 2. export an error code enum + // 3. add an errorCode property to the exception (or return value) + if (e.message.indexOf('d.ts file must have a matching npm package') > -1 || + e.message.indexOf('The non-npm package') > -1) { + lookFor("// Type definitions for", e.message); + } + else if (e.message.indexOf('At least one of the project urls listed') > -1) { + lookFor("// Project:", e.message); + } + else if (e.message.indexOf('export default') > -1) { + lookFor("export default", e.message); + } + else { + // should be unused! + ctx.addFailureAt(0, 1, e.message); + } + } + } + // Don't recur, we're done. +} diff --git a/src/util.ts b/src/util.ts index 0b0d7efe..4a64b849 100644 --- a/src/util.ts +++ b/src/util.ts @@ -85,3 +85,24 @@ export async function mapDefinedAsync(arr: Iterable, mapper: (t: T) => } return out; } + +export function isMainFile(fileName: string) { + // 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 (/^v\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"; +}