new rule 'no-implicit-dependencies' (palantir#3343)

package.json

@@ -38,6 +38,7 @@
   },
   "dependencies": {
     "babel-code-frame": "^6.22.0",
+    "builtin-modules": "^1.1.1",
     "chalk": "^2.1.0",
     "commander": "^2.9.0",
     "diff": "^3.2.0",

src/configs/all.ts

@@ -109,6 +109,7 @@ export const rules = {
     "no-eval": true,
     "no-floating-promises": true,
     "no-for-in-array": true,
+    "no-implicit-dependencies": true,
     "no-inferred-empty-object-type": true,
     "no-invalid-template-strings": true,
     // "no-invalid-this": Won't this be deprecated?

src/custom-types.d.ts

@@ -0,0 +1,4 @@
+declare module "builtin-modules" {
+    let result: string[];
+    export = result;
+}

src/rules/noImplicitDependenciesRule.ts

@@ -0,0 +1,155 @@
+/**
+ * @license
+ * Copyright 2017 Palantir Technologies, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as builtins from "builtin-modules";
+import * as fs from "fs";
+import * as path from "path";
+import { findImports, ImportKind } from "tsutils";
+import * as ts from "typescript";
+
+import * as Lint from "../index";
+
+interface Options {
+    dev: boolean;
+    optional: boolean;
+}
+
+const OPTION_DEV = "dev";
+const OPTION_OPTIONAL = "optional";
+
+export class Rule extends Lint.Rules.AbstractRule {
+    /* tslint:disable:object-literal-sort-keys */
+    public static metadata: Lint.IRuleMetadata = {
+        ruleName: "no-implicit-dependencies",
+        description: "Disallows importing modules that are not listed as dependency in the project's package.json",
+        descriptionDetails: Lint.Utils.dedent`
+            Disallows importing transient dependencies and modules installed above your package's root directory.
+        `,
+        optionsDescription: Lint.Utils.dedent`
+            By default the rule looks at \`"dependencies"\` and \`"peerDependencies"\`.
+            By adding the \`"${OPTION_DEV}"\` option the rule looks at \`"devDependencies"\` instead of \`"peerDependencies"\`.
+            By adding the \`"${OPTION_OPTIONAL}"\` option the rule also looks at \`"optionalDependencies"\`.
+        `,
+        options: {
+            type: "array",
+            items: {
+                type: "string",
+                enum: [OPTION_DEV, OPTION_OPTIONAL],
+            },
+            minItems: 0,
+            maxItems: 2,
+        },
+        optionExamples: [true, [true, OPTION_DEV], [true, OPTION_OPTIONAL]],
+        type: "functionality",
+        typescriptOnly: false,
+    };
+    /* tslint:enable:object-literal-sort-keys */
+
+    public static FAILURE_STRING_FACTORY(module: string) {
+        return `Module '${module}' is not listed as dependency in package.json`;
+    }
+
+    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+        return this.applyWithFunction(sourceFile, walk, {
+            dev: this.ruleArguments.indexOf(OPTION_DEV) !== - 1,
+            optional: this.ruleArguments.indexOf(OPTION_OPTIONAL) !== -1,
+        });
+    }
+}
+
+function walk(ctx: Lint.WalkContext<Options>) {
+    const {options} = ctx;
+    let dependencies: Set<string> | undefined;
+    for (const name of findImports(ctx.sourceFile, ImportKind.All)) {
+        if (!(ts as {} as {isExternalModuleNameRelative(m: string): boolean}).isExternalModuleNameRelative(name.text)) {
+            const packageName = getPackageName(name.text);
+            if (builtins.indexOf(packageName) === -1 && !hasDependency(packageName)) {
+                ctx.addFailureAtNode(name, Rule.FAILURE_STRING_FACTORY(packageName));
+            }
+        }
+    }
+
+    function hasDependency(module: string): boolean {
+        if (dependencies === undefined) {
+            dependencies = getDependencies(ctx.sourceFile.fileName, options);
+        }
+        return dependencies.has(module);
+    }
+}
+
+function getPackageName(name: string): string {
+    const parts = name.split(/\//g);
+    if (name[0] !== "@") {
+        return parts[0];
+    }
+    return `${parts[0]}/${parts[1]}`;
+}
+
+interface Dependencies extends Object {
+    [name: string]: any;
+}
+
+interface PackageJson {
+    dependencies?: Dependencies;
+    devDependencies?: Dependencies;
+    peerDependencies?: Dependencies;
+    optionalDependencies?: Dependencies;
+}
+
+function getDependencies(fileName: string, options: Options): Set<string> {
+    const result = new Set<string>();
+    const packageJsonPath = findPackageJson(path.resolve(path.dirname(fileName)));
+    if (packageJsonPath !== undefined) {
+        // don't use require here to avoid caching
+        const content = JSON.parse(fs.readFileSync(packageJsonPath, "utf8")) as PackageJson;
+        if (content.dependencies !== undefined) {
+            addDependencies(result, content.dependencies);
+        }
+        if (!options.dev && content.peerDependencies !== undefined) {
+            addDependencies(result, content.peerDependencies);
+        }
+        if (options.dev && content.devDependencies !== undefined) {
+            addDependencies(result, content.devDependencies);
+        }
+        if (options.optional && content.optionalDependencies !== undefined) {
+            addDependencies(result, content.optionalDependencies);
+        }
+    }
+
+    return result;
+}
+
+function addDependencies(result: Set<string>, dependencies: Dependencies) {
+    for (const name in dependencies) {
+        if (dependencies.hasOwnProperty(name)) {
+            result.add(name);
+        }
+    }
+}
+
+function findPackageJson(current: string): string | undefined {
+    let prev: string;
+    do {
+        const fileName = path.join(current, "package.json");
+        if (fs.existsSync(fileName)) {
+            return fileName;
+        }
+        prev = current;
+        current = path.dirname(current);
+    } while (prev !== current);
+    return undefined;
+}

test/ruleLoaderTests.ts

@@ -16,7 +16,6 @@
  */
 
 import { assert } from "chai";
-import * as diff from "diff";
 import * as fs from "fs";
 import * as path from "path";
 import { rules as allRules, RULES_EXCLUDED_FROM_ALL_CONFIG } from "../src/configs/all";
@@ -99,45 +98,20 @@ describe("Rule Loader", () => {
     it("tests every rule", () => {
         const tests = fs.readdirSync(testRulesDir)
             .filter((file) => !file.startsWith("_") && fs.statSync(path.join(testRulesDir, file)).isDirectory())
-            .map(camelize);
-        const diffResults = diffLists(everyRule(), tests);
-        let testFailed = false;
-        for (const { added, removed, value } of diffResults) {
-            if (added) {
-                console.warn(`Test has no matching rule: ${value}`);
-                testFailed = true;
-            } else if (removed) {
-                console.warn(`Missing test: ${value}`);
-                testFailed = true;
-            }
-        }
-
-        assert.isFalse(testFailed, "List of rules doesn't match list of tests");
+            .map(camelize)
+            .sort();
+        assert.deepEqual(everyRule(), tests, "List of rules doesn't match list of tests");
     });
 
     it("includes every rule in 'tslint:all'", () => {
         const expectedAllRules = everyRule().filter((ruleName) =>
             RULES_EXCLUDED_FROM_ALL_CONFIG.indexOf(ruleName) === -1);
         const tslintAllRules = Object.keys(allRules).map(camelize).sort();
-        const diffResults = diffLists(expectedAllRules, tslintAllRules);
-
-        const failures: string[] = [];
-        for (const { added, removed, value } of diffResults) {
-            if (added) {
-                failures.push(`Rule in 'tslint:all' does not exist: ${value}`);
-            } else if (removed) {
-                failures.push(`Rule not in 'tslint:all': ${value}`);
-            }
-        }
-
-        assert(failures.length === 0, failures.join("\n"));
+
+        assert.deepEqual(expectedAllRules, tslintAllRules, "rule is missing in tslint:all");
     });
 });
 
-function diffLists(actual: string[], expected: string[]): diff.IDiffResult[] {
-    return diff.diffLines(actual.join("\n"), expected.join("\n"));
-}
-
 function everyRule(): string[] {
     return fs.readdirSync(srcRulesDir)
         .filter((file) => /Rule.ts$/.test(file))

test/ruleTests.ts

@@ -0,0 +1,52 @@
+/*
+ * Copyright 2017 Palantir Technologies, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { assert } from "chai";
+
+import { parseConfigFile } from "../src/configuration";
+import * as Linter from "../src/linter";
+
+describe("no-implicit-dependencies", () => {
+    it("assumes empty package.json if not found", () => {
+        const linter = new Linter({
+            fix: false,
+            formatter: "prose",
+        });
+        const config = parseConfigFile({
+            rules: {
+                "no-implicit-dependencies": true,
+            },
+        });
+        linter.lint(
+            "/builtin-only.ts",
+            `
+                import * as fs from 'fs';
+                const path = require('path');
+            `,
+            config,
+        );
+        assert.equal(linter.getResult().errorCount, 0, "no error expected");
+
+        linter.lint(
+            "/test.ts",
+            `
+                import {assert} from "chai";
+            `,
+            config,
+        );
+        assert.equal(linter.getResult().errorCount, 1, "expected one error");
+    });
+});

test/rules/no-implicit-dependencies/default/builtin-only.ts.lint

@@ -0,0 +1,4 @@
+import * as fs from 'fs';
+import path = require('path');
+import('child_process').then(cp => cp.exec('foo'));
+const http = require('http');

test/rules/no-implicit-dependencies/default/nested-package/package.json

@@ -0,0 +1,7 @@
+{
+    "name": "nested",
+    "version": "0.0.1",
+    "dependencies": {
+        "baz": "next"
+    }
+}

test/rules/no-implicit-dependencies/default/nested-package/test.ts.lint

@@ -0,0 +1,24 @@
+import {assert} from 'chai';
+                     ~~~~~~ [err % ('chai')]
+import foo from 'foo';
+                ~~~~~ [err % ('foo')]
+
+if (foo) {
+    const common = require('common');
+                           ~~~~~~~~ [err % ('common')]
+}
+
+export * from "@angular/core";
+              ~~~~~~~~~~~~~~~ [err % ('@angular/core')]
+
+export * from "@angular/common/http";
+              ~~~~~~~~~~~~~~~~~~~~~~ [err % ('@angular/common')]
+
+import * as ts from "typescript";
+                    ~~~~~~~~~~~~ [err % ('typescript')]
+
+import "baz";
+
+const http = require('http');
+
+[err]: Module '%s' is not listed as dependency in package.json

test/rules/no-implicit-dependencies/default/subdir/test.ts.lint

@@ -0,0 +1,23 @@
+import {assert} from 'chai';
+                     ~~~~~~ [err % ('chai')]
+import foo from 'foo';
+
+if (foo) {
+    const common = require('common');
+                           ~~~~~~~~ [err % ('common')]
+}
+
+export * from "@angular/core";
+
+export * from "@angular/common/http";
+
+import * as ts from "typescript";
+
+import "baz/foo";
+       ~~~~~~~~~ [err % ('baz')]
+
+const http = require('http');
+
+import test from '../test.ts';
+
+[err]: Module '%s' is not listed as dependency in package.json

test/rules/no-implicit-dependencies/default/test.js.lint

@@ -0,0 +1,10 @@
+const foo = require('foo');
+const bar = require('bar');
+const baz = require('baz');
+                    ~~~~~ [err % ('baz')]
+
+import storageHelper from 'storage-helper';
+                          ~~~~~~~~~~~~~~~~ [err % ('storage-helper')]
+const myModule = require('./myModule');
+
+[err]: Module '%s' is not listed as dependency in package.json

test/rules/no-implicit-dependencies/default/test.ts.lint

@@ -0,0 +1,24 @@
+import {assert} from 'chai';
+                     ~~~~~~ [err % ('chai')]
+import foo from 'foo';
+
+if (foo) {
+    const common = require('common');
+                           ~~~~~~~~ [err % ('common')]
+}
+
+export * from "@angular/core";
+
+export * from "@angular/common/http";
+
+import * as ts from "typescript";
+
+import "baz";
+       ~~~~~ [err % ('baz')]
+
+const http = require('http');
+
+import * as fsevents from "fsevents";
+                          ~~~~~~~~~~ [err % ('fsevents')]
+
+[err]: Module '%s' is not listed as dependency in package.json

test/rules/no-implicit-dependencies/default/tslint.json

@@ -0,0 +1,8 @@
+{
+  "rules": {
+    "no-implicit-dependencies": true
+  },
+  "jsRules": {
+    "no-implicit-dependencies": true
+  }
+}