tslint-no-circular-imports - npm Package Compare versions

Comparing version 0.4.0 to 0.5.0

noCircularImportsRule.js

@@ -12,2 +12,32 @@ "use strict";
 })();
+var __values = (this && this.__values) || function (o) {
+    var m = typeof Symbol === "function" && o[Symbol.iterator], i = 0;
+    if (m) return m.call(o);
+    return {
+        next: function () {
+            if (o && i >= o.length) o = void 0;
+            return { value: o && o[i++], done: !o };
+        }
+    };
+};
+var __read = (this && this.__read) || function (o, n) {
+    var m = typeof Symbol === "function" && o[Symbol.iterator];
+    if (!m) return o;
+    var i = m.call(o), r, ar = [], e;
+    try {
+        while ((n === void 0 || n-- > 0) && !(r = i.next()).done) ar.push(r.value);
+    }
+    catch (error) { e = { error: error }; }
+    finally {
+        try {
+            if (r && !r.done && (m = i["return"])) m.call(i);
+        }
+        finally { if (e) throw e.error; }
+    }
+    return ar;
+};
+var __spread = (this && this.__spread) || function () {
+    for (var ar = [], i = 0; i < arguments.length; i++) ar = ar.concat(__read(arguments[i]));
+    return ar;
+};
 Object.defineProperty(exports, "__esModule", { value: true });
@@ -54,19 +84,52 @@ var path_1 = require("path");
     }
-    NoCircularImportsWalker.prototype.visitNode = function (node) {
-        // export declarations seem to be missing from the current SyntaxWalker
-        if (ts.isExportDeclaration(node)) {
-            this.visitExportDeclaration(node);
-            this.walkChildren(node);
+    NoCircularImportsWalker.prototype.visitSourceFile = function (sourceFile) {
+        var _this = this;
+        // Instead of visiting all children, this is faster. We know imports are statements anyway.
+        sourceFile.statements.forEach(function (statement) {
+            // export declarations seem to be missing from the current SyntaxWalker
+            if (ts.isExportDeclaration(statement)) {
+                _this.visitImportOrExportDeclaration(statement);
+            }
+            else if (ts.isImportDeclaration(statement)) {
+                _this.visitImportOrExportDeclaration(statement);
+            }
+        });
+        var fileName = sourceFile.fileName;
+        // Check for cycles, remove any cycles that have been found already (otherwise we'll report
+        // false positive on every files that import from the real cycles, and users will be driven
+        // mad).
+        // The checkCycle is many order of magnitude faster than getCycle, but does not keep a history
+        // of the cycle itself. Only get the full cycle if we found one.
+        if (this.checkCycle(fileName)) {
+            var allCycles = this.getAllCycles(fileName);
+            var _loop_1 = function (maybeCycle) {
+                // Slice the array so we don't match this file twice.
+                if (maybeCycle.slice(1, -1).some(function (fileName) { return found.has(fileName); })) {
+                    return "continue";
+                }
+                maybeCycle.forEach(function (x) { return found.add(x); });
+                var node = imports.get(fileName).get(maybeCycle[1]);
+                var compilerOptions = this_1.program.getCompilerOptions();
+                this_1.addFailureAt(node.getStart(), node.getWidth(), Rule.FAILURE_STRING + ": " + maybeCycle
+                    .concat(fileName)
+                    .map(function (x) { return path_1.relative(compilerOptions.rootDir || process.cwd(), x); })
+                    .join(' -> '));
+            };
+            var this_1 = this;
+            try {
+                for (var allCycles_1 = __values(allCycles), allCycles_1_1 = allCycles_1.next(); !allCycles_1_1.done; allCycles_1_1 = allCycles_1.next()) {
+                    var maybeCycle = allCycles_1_1.value;
+                    _loop_1(maybeCycle);
+                }
+            }
+            catch (e_1_1) { e_1 = { error: e_1_1 }; }
+            finally {
+                try {
+                    if (allCycles_1_1 && !allCycles_1_1.done && (_a = allCycles_1.return)) _a.call(allCycles_1);
+                }
+                finally { if (e_1) throw e_1.error; }
+            }
         }
-        else {
-            _super.prototype.visitNode.call(this, node);
-        }
+        var e_1, _a;
     };
-    NoCircularImportsWalker.prototype.visitExportDeclaration = function (node) {
-        this.visitImportOrExportDeclaration(node);
-    };
-    NoCircularImportsWalker.prototype.visitImportDeclaration = function (node) {
-        this.visitImportOrExportDeclaration(node);
-        _super.prototype.visitImportDeclaration.call(this, node);
-    };
     NoCircularImportsWalker.prototype.visitImportOrExportDeclaration = function (node) {
@@ -95,27 +158,29 @@ if (!node.parent || !ts.isSourceFile(node.parent)) {
         }
-        this.addToGraph(fileName, resolvedImportFileName);
-        // Check for cycles, remove any cycles that have been found already (otherwise we'll report
-        // false positive on every files that import from the real cycles, and users will be driven
-        // mad).
-        var maybeCycle = this.getCycle(fileName, resolvedImportFileName);
-        if (maybeCycle.length > 0) {
-            // Slice the array so we don't match this file twice.
-            if (maybeCycle.slice(1).some(function (fn) { return found.has(fn); })) {
-                return;
-            }
-            maybeCycle.forEach(function (x) { return found.add(x); });
-            this.addFailureAt(node.getStart(), node.getWidth(), Rule.FAILURE_STRING + ": " + maybeCycle
-                .concat(fileName)
-                .map(function (x) { return path_1.relative(compilerOptions.rootDir || process.cwd(), x); })
-                .join(' -> '));
-        }
+        this.addToGraph(fileName, resolvedImportFileName, node);
     };
-    NoCircularImportsWalker.prototype.addToGraph = function (thisFileName, importCanonicalName) {
+    NoCircularImportsWalker.prototype.addToGraph = function (thisFileName, importCanonicalName, node) {
         var i = imports.get(thisFileName);
         if (!i) {
-            imports.set(thisFileName, i = new Set);
+            imports.set(thisFileName, i = new Map);
         }
-        i.add(importCanonicalName);
+        i.set(importCanonicalName, node);
     };
-    NoCircularImportsWalker.prototype.getCycle = function (moduleName, startFromImportName, accumulator) {
+    NoCircularImportsWalker.prototype.checkCycle = function (moduleName) {
+        var accumulator = new Set();
+        var moduleImport = imports.get(moduleName);
+        if (!moduleImport)
+            return false;
+        var toCheck = Array.from(moduleImport.keys());
+        for (var i = 0; i < toCheck.length; i++) {
+            var current = toCheck[i];
+            if (current == moduleName) {
+                return true;
+            }
+            accumulator.add(current);
+            toCheck.push.apply(toCheck, __spread(Array.from((imports.get(current) || new Map).keys())
+                .filter(function (i) { return !accumulator.has(i); })));
+        }
+        return false;
+    };
+    NoCircularImportsWalker.prototype.getAllCycles = function (moduleName, accumulator) {
         if (accumulator === void 0) { accumulator = []; }
@@ -126,17 +191,21 @@ var moduleImport = imports.get(moduleName);
         if (accumulator.indexOf(moduleName) !== -1)
-            return accumulator;
-        if (startFromImportName !== undefined && imports.has(startFromImportName)) {
-            var c = this.getCycle(startFromImportName, undefined, accumulator.concat(moduleName));
-            if (c.length)
-                return c;
-        }
-        else {
-            for (var _i = 0, _a = Array.from(moduleImport.values()); _i < _a.length; _i++) {
-                var imp = _a[_i];
-                var c = this.getCycle(imp, undefined, accumulator.concat(moduleName));
+            return [accumulator];
+        var all = [];
+        try {
+            for (var _a = __values(Array.from(moduleImport.keys())), _b = _a.next(); !_b.done; _b = _a.next()) {
+                var imp = _b.value;
+                var c = this.getAllCycles(imp, accumulator.concat(moduleName));
                 if (c.length)
-                    return c;
+                    all.push.apply(all, __spread(c));
             }
         }
-        return [];
+        catch (e_2_1) { e_2 = { error: e_2_1 }; }
+        finally {
+            try {
+                if (_b && !_b.done && (_c = _a.return)) _c.call(_a);
+            }
+            finally { if (e_2) throw e_2.error; }
+        }
+        return all;
+        var e_2, _c;
     };
@@ -143,0 +212,0 @@ return NoCircularImportsWalker;

noCircularImportsRule.ts

@@ -32,3 +32,3 @@ import { relative, sep } from 'path';
 // Graph of imports.
-const imports = new Map<string, Set<string>>()
+const imports = new Map<string, Map<string, ts.Node>>()
 // Keep a list of found circular dependencies to avoid showing them twice.
@@ -43,21 +43,39 @@ const found = new Set<string>()
 
-  visitNode(node: ts.Node) {
-    // export declarations seem to be missing from the current SyntaxWalker
-    if (ts.isExportDeclaration(node)) {
-      this.visitExportDeclaration(node)
-      this.walkChildren(node)
-    }
-    else {
-      super.visitNode(node)
-    }
+  visitSourceFile(sourceFile: ts.SourceFile) {
+    // Instead of visiting all children, this is faster. We know imports are statements anyway.
+    sourceFile.statements.forEach(statement => {
+      // export declarations seem to be missing from the current SyntaxWalker
+      if (ts.isExportDeclaration(statement)) {
+          this.visitImportOrExportDeclaration(statement)
+      }
+      else if (ts.isImportDeclaration(statement)) {
+          this.visitImportOrExportDeclaration(statement)
+      }
+    })
 
-  }
+    const fileName = sourceFile.fileName
 
-  visitExportDeclaration(node: ts.ExportDeclaration) {
-    this.visitImportOrExportDeclaration(node)
-  }
+    // Check for cycles, remove any cycles that have been found already (otherwise we'll report
+    // false positive on every files that import from the real cycles, and users will be driven
+    // mad).
+    // The checkCycle is many order of magnitude faster than getCycle, but does not keep a history
+    // of the cycle itself. Only get the full cycle if we found one.
+    if (this.checkCycle(fileName)) {
+      const allCycles = this.getAllCycles(fileName)
 
-  visitImportDeclaration(node: ts.ImportDeclaration) {
-    this.visitImportOrExportDeclaration(node)
-    super.visitImportDeclaration(node)
+      for (const maybeCycle of allCycles) {
+        // Slice the array so we don't match this file twice.
+        if (maybeCycle.slice(1, -1).some(fileName => found.has(fileName))) {
+            continue
+        }
+        maybeCycle.forEach(x => found.add(x))
+        const node = imports.get(fileName) !.get(maybeCycle[1]) !
+
+        const compilerOptions = this.program.getCompilerOptions()
+        this.addFailureAt(node.getStart(), node.getWidth(), Rule.FAILURE_STRING + ": " + maybeCycle
+            .concat(fileName)
+            .map(x => relative(compilerOptions.rootDir || process.cwd(), x))
+            .join(' -> '))
+      }
+    }
   }
@@ -92,55 +110,53 @@
 
-    this.addToGraph(fileName, resolvedImportFileName)
-
-    // Check for cycles, remove any cycles that have been found already (otherwise we'll report
-    // false positive on every files that import from the real cycles, and users will be driven
-    // mad).
-    const maybeCycle = this.getCycle(fileName, resolvedImportFileName)
-    if (maybeCycle.length > 0) {
-      // Slice the array so we don't match this file twice.
-      if (maybeCycle.slice(1).some(fn => found.has(fn))) {
-        return
-      }
-
-      maybeCycle.forEach(x => found.add(x))
-
-      this.addFailureAt(
-        node.getStart(),
-        node.getWidth(),
-        `${Rule.FAILURE_STRING}: ${
-          maybeCycle
-            .concat(fileName)
-            // Show relative to baseUrl (or the tsconfig path itself).
-            .map(x => relative(compilerOptions.rootDir || process.cwd(), x))
-            .join(' -> ')
-        }`)
-    }
+    this.addToGraph(fileName, resolvedImportFileName, node)
   }
 
-  private addToGraph(thisFileName: string, importCanonicalName: string) {
+  private addToGraph(thisFileName: string, importCanonicalName: string, node: ts.Node) {
     let i = imports.get(thisFileName)
     if (!i) {
-      imports.set(thisFileName, i = new Set)
+      imports.set(thisFileName, i = new Map)
     }
-    i.add(importCanonicalName)
+    i.set(importCanonicalName, node)
   }
 
-  private getCycle(moduleName: string, startFromImportName?: string | undefined, accumulator: string[] = []): string[] {
+  private checkCycle(moduleName: string): boolean {
+    const accumulator = new Set<string>()
+
     const moduleImport = imports.get(moduleName)
-    if (!moduleImport) return []
-    if (accumulator.indexOf(moduleName) !== -1) return accumulator
+    if (!moduleImport)
+      return false
 
-    if(startFromImportName !== undefined && imports.has(startFromImportName)) {
-      const c = this.getCycle(startFromImportName, undefined, accumulator.concat(moduleName))
-      if(c.length) return c
-    }
-    else {
-      for (const imp of Array.from(moduleImport.values())) {
-        const c = this.getCycle(imp, undefined, accumulator.concat(moduleName))
-        if(c.length) return c
+    const toCheck = Array.from(moduleImport.keys())
+    for (let i = 0; i < toCheck.length; i++) {
+      const current = toCheck[i]
+      if (current == moduleName) {
+        return true
       }
+      accumulator.add(current)
+
+      toCheck.push(
+        ...Array.from((imports.get(current) || new Map).keys())
+                .filter(i => !accumulator.has(i))
+      )
     }
 
-    return []
+    return false
   }
+
+  private getAllCycles(moduleName: string, accumulator: string[] = []): string[][] {
+    const moduleImport = imports.get(moduleName)
+    if (!moduleImport) return []
+    if (accumulator.indexOf(moduleName) !== -1)
+      return [accumulator]
+
+    const all: string[][] = []
+    for (const imp of Array.from(moduleImport.keys())) {
+      const c = this.getAllCycles(imp, accumulator.concat(moduleName))
+
+      if (c.length)
+        all.push(...c)
+    }
+
+    return all
+  }
 }

package.json

 {
   "name": "tslint-no-circular-imports",
-  "version": "0.4.0",
+  "version": "0.5.0",
   "description": "TSLint plugin to detect and warn about circular imports",
@@ -5,0 +5,0 @@ "main": "tslint-no-circular-imports.json",

test/test.js

 "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
+const child_process_1 = require("child_process");
 const assert = require("assert");
-const child_process_1 = require("child_process");
-const path_1 = require("path");
-const tslintBin = path_1.join('..', 'node_modules', '.bin', 'tslint');
-const tslintConfig = path_1.join('.', 'tslint.json');
-const tslintFormat = 'json';
-const tsFiles = `${path_1.join('.', '*.ts')} ${path_1.join('.', '*/*.ts')}`;
-const tsconfig = path_1.join('.', 'tsconfig.json');
-child_process_1.exec(`${tslintBin} -p ${tsconfig} -c ${tslintConfig} -r .. -t ${tslintFormat} ${tsFiles}`, { cwd: __dirname }, (error, stdout, stderr) => {
-    // Only validate failures and names.
-    const actual = JSON.parse(stdout)
-        .map(x => ({ failure: x.failure, name: x.name }));
-    assert.deepEqual(actual, [
-        // case1
-        {
-            failure: 'circular import detected: case1.ts -> case1.1.ts -> case1.ts',
-            name: path_1.join(__dirname, 'case1.ts')
-        },
-        {
-            failure: 'circular import detected: case1.ts -> case1.2.ts -> case1.ts',
-            name: path_1.join(__dirname, 'case1.ts')
-        },
-        // case2
-        {
-            failure: 'circular import detected: case2/b.ts -> case2/a.ts -> case2/b.ts',
-            name: path_1.join(__dirname, 'case2/b.ts')
-        },
-        // case3
-        {
-            failure: 'circular import detected: case3/b.ts -> case3/a.ts -> case3/b.ts',
-            name: path_1.join(__dirname, 'case3/b.ts')
-        },
-        // case4
-        {
-            failure: 'circular import detected: case4/index.ts -> case4/a.ts -> case4/index.ts',
-            name: path_1.join(__dirname, 'case4/index.ts')
-        }
-    ]);
+console.log(__dirname);
+child_process_1.exec('../node_modules/.bin/tslint -c ./tslint.json -r ../ ./*.ts', { cwd: __dirname }, (error, stdout, stderr) => {
+    assert.equal(stdout, `
+ERROR: case1.ts[1, 1]: circular import detected: case1.ts -> case1.1.ts -> case1.ts
+ERROR: case1.ts[2, 1]: circular import detected: case1.ts -> case1.2.ts -> case1.ts
+
+`);
 });
 //# sourceMappingURL=test.js.map

test/test.ts

@@ -20,8 +20,8 @@ import * as assert from 'assert'
     {
-      failure: 'circular import detected: case1.ts -> case1.1.ts -> case1.ts',
+      failure: 'circular import detected: case1.ts -> case1.2.ts -> case1.ts',
       name: join(__dirname, 'case1.ts')
     },
     {
-      failure: 'circular import detected: case1.ts -> case1.2.ts -> case1.ts',
-      name: join(__dirname, 'case1.ts')
+      failure: 'circular import detected: case1.1.ts -> case1.ts -> case1.1.ts',
+      name: join(__dirname, 'case1.1.ts')
     },
@@ -31,4 +31,4 @@
     {
-      failure: 'circular import detected: case2/b.ts -> case2/a.ts -> case2/b.ts',
-      name: join(__dirname, 'case2/b.ts')
+      failure: 'circular import detected: case2/a.ts -> case2/b.ts -> case2/a.ts',
+      name: join(__dirname, 'case2/a.ts')
     },
@@ -38,4 +38,4 @@
     {
-      failure: 'circular import detected: case3/b.ts -> case3/a.ts -> case3/b.ts',
-      name: join(__dirname, 'case3/b.ts')
+      failure: 'circular import detected: case3/a.ts -> case3/b.ts -> case3/a.ts',
+      name: join(__dirname, 'case3/a.ts')
     },
@@ -45,6 +45,6 @@
     {
-      failure: 'circular import detected: case4/index.ts -> case4/a.ts -> case4/index.ts',
-      name: join(__dirname, 'case4/index.ts')
+      failure: 'circular import detected: case4/a.ts -> case4/index.ts -> case4/a.ts',
+      name: join(__dirname, 'case4/a.ts')
     }
   ])
 })

tsconfig.json

@@ -18,2 +18,3 @@ {
     "preserveConstEnums": true,
+    "downlevelIteration": true,
     "pretty": true,
@@ -20,0 +21,0 @@ "sourceMap": true,

No alert changes

Improved metrics

Total package byte prevSize

28767

increased by8.4%

Lines of code

516

increased by11.21%

No dependency changes