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

@@ -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"],

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",

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<void> {
     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,

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>): 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";
-}

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>): 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.
+}