implements fixer for import-barrels and according tests

CHANGELOG.md

@@ -4,6 +4,8 @@ Change Log
 v2.1.0
 ---
 * [new-fixer] `jasmine-no-lambda-expression-callbacks`
+* [new-fixer] `import-barrels`
+* [new-rule-option] `fixWithExplicitBarrelImport` added to `import-barrels`
 * [enhancement] Added preliminary checks for `jasmine-no-lambda-expression-callbacks` rule to skip walking file if there are no jasmine
 top-level statements.
 

README.md

@@ -59,6 +59,8 @@ An argument object may be optionally provided, with the following properties:
 
 * `noExplicitBarrels = false`: disallows usage of explicitly named barrels in import statements (`import foo from './foo/index'`)
 * `fileExtensions = ['ts', 'js']`: uses the provided file extensions for module and barrel file lookup
+* `fixWithExplicitBarrelImport`: uses the provided string to replace non-barrel imports in `--fix` mode
+  (i.e. when set to `'index'`, `import foo from './foo/some-module'` becomes `import foo from './foo/index'`)
 
 
 ## `jasmine-no-lambda-expression-callbacks`

package.json

@@ -45,7 +45,7 @@
     "tslint": "^4.2.0",
     "tslint-microsoft-contrib": "^4.0.0",
     "typescript": "~2.1.4",
-    "yargs": "^6.0.0"
+    "yargs": "^6.6.0"
   },
   "peerDependencies": {
     "tslint": "^4.0.0"

runTests.js

@@ -34,6 +34,7 @@ glob('src/tests/**/*.ts.lint', (error, files) => {
     const pathToBuildDir = path.resolve(dir, buildDir);
     const pathToCoverageDir = path.resolve(dir, coverageDir);
 
+    console.log(`=> running tests from: ${dir}`);
     child_process.execSync(getTestCommand(pathToBuildDir, pathToCoverageDir), {
       cwd: dir,
       stdio: 'inherit'

src/importBarrelsRule.ts

@@ -1,10 +1,11 @@
 import * as fs from 'fs';
 import * as path from 'path';
-import { IRuleMetadata, RuleFailure, Rules, RuleWalker, Utils } from 'tslint/lib';
+import { Fix, IOptions, IRuleMetadata, Replacement, RuleFailure, Rules, RuleWalker, Utils } from 'tslint/lib';
 import { Expression, ImportDeclaration, SourceFile, StringLiteral, SyntaxKind } from 'typescript';
+import { CachedStat } from './utils/cachedStat';
 
 export class Rule extends Rules.AbstractRule {
-  public static metadata:IRuleMetadata = {
+  public static readonly metadata:IRuleMetadata = {
     ruleName: 'import-barrels',
     description: Utils.dedent`
       Enforces usage of barrels (\`index.ts\`) when importing from a directory that has a barrel file.`,
@@ -17,7 +18,9 @@ export class Rule extends Rules.AbstractRule {
       An argument object may be optionally provided, with the following properties:
       
       * \`noExplicitBarrels = false\`: disallows usage of explicitly named barrels in import statements (\`import foo from './foo/index'\`)
-      * \`fileExtensions = ['ts', 'js']\`: uses the provided file extensions for module and barrel file lookup`,
+      * \`fileExtensions = ['ts', 'js']\`: uses the provided file extensions for module and barrel file lookup
+      * \`fixWithExplicitBarrelImport\`: uses the provided string to replace non-barrel imports in \`--fix\` mode
+        (i.e. when set to \`'index'\`, \`import foo from './foo/some-module'\` becomes \`import foo from './foo/index'\`)`,
     optionExamples: ['[true, {"noExplicitBarrels": false, "fileExtensions": ["ts", "js"]}]'],
     options: {
       type: 'array',
@@ -34,6 +37,9 @@ export class Rule extends Rules.AbstractRule {
             },
             minLength: 1,
           },
+          fixWithExplicitBarrelImport: {
+            type: 'string',
+          },
         },
       },
       minLength: 1,
@@ -43,38 +49,66 @@ export class Rule extends Rules.AbstractRule {
     typescriptOnly: false,
   };
 
-  public static USE_BARREL_FAILURE_STRING = 'Use barrel (index) files for imports if they are available for path: ';
-  public static NO_EXPLICIT_BARRELS_FAILURE_STRING = "Don't import barrel files by name, import containing directory instead for path: ";
+  public static readonly USE_BARREL_FAILURE_STRING = 'Use barrel (index) files for imports if they are available for path: ';
+  public static readonly NO_EXPLICIT_BARRELS_FAILURE_STRING =
+    "Don't import barrel files by name, import containing directory instead for path: ";
+
+  public static readonly DEFAULT_OPTIONS = {
+    fileExtensions: ['ts', 'js'],
+    noExplicitBarrels: false,
+    fixWithExplicitBarrelImport: ''
+  };
 
   public apply(sourceFile:SourceFile):RuleFailure[] {
     return this.applyWithWalker(new ImportBarrelsWalker(sourceFile, this.getOptions()));
   }
 }
 
+enum CheckResult {
+  OK,
+  ExplicitBarrelsForbidden,
+  NonBarrelImport
+}
+
 class ImportBarrelsWalker extends RuleWalker {
-  private statCache = new Map<string, fs.Stats>();
+  private static readonly resultErrorMap = {
+    [CheckResult.NonBarrelImport]: Rule.USE_BARREL_FAILURE_STRING,
+    [CheckResult.ExplicitBarrelsForbidden]: Rule.NO_EXPLICIT_BARRELS_FAILURE_STRING,
+  };
+  private readonly cachedStat = new CachedStat();
+  private readonly ruleOptions:typeof Rule.DEFAULT_OPTIONS;
+
+  constructor(sourceFile:SourceFile, options:IOptions) {
+    super(sourceFile, options);
+
+    this.ruleOptions = Object.assign({}, Rule.DEFAULT_OPTIONS, this.getOptions()[0] || {});
+  }
 
   protected visitImportDeclaration(node:ImportDeclaration) {
     const moduleExpression = node.moduleSpecifier;
-    const moduleExpressionError = this.getModuleExpressionErrorMessage(moduleExpression);
-
-    if (moduleExpressionError) {
-      this.addFailureAtNode(moduleExpression, moduleExpressionError);
+    const expressionCheckResult = this.checkModuleExpression(moduleExpression);
+
+    if (expressionCheckResult !== CheckResult.OK) {
+      this.addFailureAtNode(
+        moduleExpression,
+        ImportBarrelsWalker.resultErrorMap[expressionCheckResult] + moduleExpression.getText(),
+        this.getFix(<StringLiteral>moduleExpression, expressionCheckResult)
+      );
     }
 
     super.visitImportDeclaration(node);
   }
 
-  private getModuleExpressionErrorMessage(expression:Expression):string|null {
+  private checkModuleExpression(expression:Expression):CheckResult {
     if (expression.kind !== SyntaxKind.StringLiteral) {
-      return null;
+      return CheckResult.OK;
     }
 
     const modulePathText = (<StringLiteral>expression).text;
 
     // check only relative paths
     if (!modulePathText.startsWith('.')) {
-      return null;
+      return CheckResult.OK;
     }
 
     const sourceFileRelative = this.getSourceFile().path;
@@ -85,80 +119,49 @@ class ImportBarrelsWalker extends RuleWalker {
     // enforce barrel usage only on files that are not in the same directory or in one of the sub-directories
     // of the module
     if (sourceFileDirAbsolute.startsWith(moduleDirAbsolute)) {
-      return null;
+      return CheckResult.OK;
     }
 
     const moduleStats = this.getModuleStats(moduleAbsolute);
 
     // only file imports are of interest
     if (!moduleStats || moduleStats.isDirectory()) {
-      return null;
+      return CheckResult.OK;
     }
 
     // if module's name is 'index', it must be an explicit barrel import, dirs were excluded earlier
     if (path.parse(moduleAbsolute).name === 'index') {
-      return this.getExplicitBarrelsAllowed() ? null : Rule.NO_EXPLICIT_BARRELS_FAILURE_STRING + expression.getText();
+      return this.ruleOptions.noExplicitBarrels ? CheckResult.ExplicitBarrelsForbidden : CheckResult.OK;
     }
 
-    return this.getModuleFileExtensions()
+    const dirHasBarrelFile = this.ruleOptions.fileExtensions
       .map(ext => path.join(moduleDirAbsolute, `index.${ext}`))
-      .some(file => this.isFile(file)) ? Rule.USE_BARREL_FAILURE_STRING + expression.getText() : null;
+      .some(file => this.cachedStat.isFile(file));
+
+    return dirHasBarrelFile ? CheckResult.NonBarrelImport : CheckResult.OK;
   }
 
   private getModuleStats(modulePath:string):fs.Stats|null {
-    let stats = this.cachedStatSync(modulePath);
+    const modulePathCandidates = [modulePath, ...this.ruleOptions.fileExtensions.map(suffix => `${modulePath}.${suffix}`)];
 
-    if (!stats) {
-      this.getModuleFileExtensions().some(suffix => {
-        stats = this.cachedStatSync(`${modulePath}.${suffix}`);
-        return stats !== null;
-      });
-    }
+    let stats:fs.Stats|null = null;
+    modulePathCandidates.some(modulePathCandidate => {
+      stats = this.cachedStat.statSync(modulePathCandidate);
+      return stats !== null;
+    });
 
     return stats;
   }
 
-  private getModuleFileExtensions():string[] {
-    let extensions = this.getOptionsObject().fileExtensions || [];
-
-    if (!extensions.length) {
-      extensions = ['ts', 'js'];
-    }
-    return extensions;
-  }
-
-  private getExplicitBarrelsAllowed():boolean {
-    const {noExplicitBarrels = false} = this.getOptionsObject();
-
-    return !noExplicitBarrels;
-  }
-
-  private getOptionsObject():{fileExtensions?:string[], noExplicitBarrels?:boolean} {
-    return this.getOptions()[0] || {};
-  }
+  private getFix(moduleSpecifier:StringLiteral, checkResult:CheckResult):Fix {
+    let replacement = path.dirname(moduleSpecifier.text);
 
-  private isFile(path:string):boolean {
-    const stats = this.cachedStatSync(path);
-    return stats != null && stats.isFile();
-  }
-
-  private cachedStatSync(path:string):fs.Stats|null {
-    if (this.statCache.has(path)) {
-      return <fs.Stats>this.statCache.get(path);
-    }
-
-    const stat = this.statSync(path);
-    if (stat !== null) {
-      this.statCache.set(path, stat);
+    if (checkResult === CheckResult.NonBarrelImport && this.ruleOptions.fixWithExplicitBarrelImport) {
+      replacement += `/${this.ruleOptions.fixWithExplicitBarrelImport}`;
     }
-    return stat;
-  }
 
-  private statSync(path:string):fs.Stats|null {
-    try {
-      return fs.statSync(path);
-    } catch (e) {
-      return null;
-    }
+    return new Fix(Rule.metadata.ruleName, [
+      // account for quotes
+      new Replacement(moduleSpecifier.getStart() + 1, moduleSpecifier.getWidth() - `''`.length, replacement)]);
   }
 }

src/jasmineNoLambdaExpressionCallbacksRule.ts

@@ -4,7 +4,7 @@ import { isJasmineDescribe, isJasmineIt, isJasmineSetupTeardown, isJasmineTest }
 import find = require('lodash/find');
 
 export class Rule extends Rules.AbstractRule {
-  public static metadata:IRuleMetadata = {
+  public static readonly metadata:IRuleMetadata = {
     ruleName: 'jasmine-no-lambda-expression-callbacks',
     description: Utils.dedent`
       Disallows usage of ES6-style lambda expressions as callbacks to Jasmine BDD functions.`,
@@ -32,7 +32,7 @@ export class Rule extends Rules.AbstractRule {
     typescriptOnly: false,
   };
 
-  public static FAILURE_STRING = "Don't use lambda expressions as callbacks to jasmine functions";
+  public static readonly FAILURE_STRING = "Don't use lambda expressions as callbacks to jasmine functions";
 
   public apply(sourceFile:SourceFile):RuleFailure[] {
     return this.applyWithWalker(new JasmineNoLambdaExpressionCallbacksWalker(sourceFile, this.getOptions()));

src/tests/importBarrels/default/default.ts.fix

@@ -0,0 +1,10 @@
+import relativeFromNonBarrel from './dir-with-barrel';
+import relativeFromNonBarrelWithIndexLikeName from './dir-with-barrel';
+
+// following should be OK
+import relativeFromBarrel from './dir-with-barrel';
+import relativeFromBarrelExplicit from './dir-with-barrel/index';
+import relativeFromDirWithoutBarrel from './dir-without-barrel/some-module';
+import relativeFromSameDirWithBarrel from './some-module';
+import relativeFromSameDirWithBarrelExplicit from './index';
+import absoluteFromLib from 'lib';

src/tests/importBarrels/default/importBarrelsRuleDefault.ts.lint → src/tests/importBarrels/default/default.ts.lint

@@ -2,6 +2,8 @@ import relativeFromNonBarrel from './dir-with-barrel/some-module';
                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  [Use barrel (index) files for imports if they are available for path: './dir-with-barrel/some-module']
 import relativeFromNonBarrelWithIndexLikeName from './dir-with-barrel/some-module-index';
                                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  [Use barrel (index) files for imports if they are available for path: './dir-with-barrel/some-module-index']
+
+// following should be OK
 import relativeFromBarrel from './dir-with-barrel';
 import relativeFromBarrelExplicit from './dir-with-barrel/index';
 import relativeFromDirWithoutBarrel from './dir-without-barrel/some-module';

src/tests/importBarrels/fixWithExplicitBarrelImport/fixWithExplicitBarrelImport.ts.fix

@@ -0,0 +1,2 @@
+import relativeFromNonBarrel from './dir-with-barrel/index.ts';
+import relativeFromNonBarrelWithIndexLikeName from './dir-with-barrel/index.ts';

src/tests/importBarrels/fixWithExplicitBarrelImport/fixWithExplicitBarrelImport.ts.lint

@@ -0,0 +1,4 @@
+import relativeFromNonBarrel from './dir-with-barrel/some-module';
+                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  [Use barrel (index) files for imports if they are available for path: './dir-with-barrel/some-module']
+import relativeFromNonBarrelWithIndexLikeName from './dir-with-barrel/some-module-index';
+                                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  [Use barrel (index) files for imports if they are available for path: './dir-with-barrel/some-module-index']

src/tests/importBarrels/fixWithExplicitBarrelImport/tslint.json

@@ -0,0 +1,13 @@
+{
+  "rulesDirectory": [
+    "../../../../dist"
+  ],
+  "rules": {
+    "import-barrels": [
+      true,
+      {
+        "fixWithExplicitBarrelImport": "index.ts"
+      }
+    ]
+  }
+}

src/tests/importBarrels/no-explicit-barrels/noExplicitBarrels.ts.fix

@@ -0,0 +1,5 @@
+import relativeFromBarrelExplicit from './dir-with-barrel';
+
+// following should be OK
+import relativeFromBarrel from './dir-with-barrel';
+import relativeFromNonBarrelWithIndexLikeName from './dir-without-barrel/some-module-index';

src/tests/importBarrels/no-explicit-barrels/importBarrelRulesNoExplicitBarrels.ts.lint → src/tests/importBarrels/no-explicit-barrels/noExplicitBarrels.ts.lint

@@ -1,5 +1,6 @@
 import relativeFromBarrelExplicit from './dir-with-barrel/index';
                                        ~~~~~~~~~~~~~~~~~~~~~~~~~  [Don't import barrel files by name, import containing directory instead for path: './dir-with-barrel/index']
 
+// following should be OK
 import relativeFromBarrel from './dir-with-barrel';
 import relativeFromNonBarrelWithIndexLikeName from './dir-without-barrel/some-module-index';

src/tests/importBarrels/with-extension-list/withExtensionList.ts.fix

@@ -0,0 +1,4 @@
+import relativeFromNonBarrel from './dir-with-barrel';
+
+// following should be OK
+import relativeFromNonBarrelWithIndexName from './dir-with-barrel/some-module-index';

src/tests/importBarrels/with-extension-list/importBarrelRulesWithExtensionList.ts.lint → src/tests/importBarrels/with-extension-list/withExtensionList.ts.lint

@@ -1,3 +1,5 @@
 import relativeFromNonBarrel from './dir-with-barrel/some-module';
                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  [Use barrel (index) files for imports if they are available for path: './dir-with-barrel/some-module']
+
+// following should be OK
 import relativeFromNonBarrelWithIndexName from './dir-with-barrel/some-module-index';

src/utils/cachedStat.ts

@@ -0,0 +1,30 @@
+import * as fs from 'fs';
+
+export class CachedStat {
+  private readonly statCache = new Map<string, fs.Stats>();
+
+  public statSync(path:string):fs.Stats|null {
+    if (this.statCache.has(path)) {
+      return <fs.Stats>this.statCache.get(path);
+    }
+
+    const stat = this.statSyncUncached(path);
+    if (stat !== null) {
+      this.statCache.set(path, stat);
+    }
+    return stat;
+  }
+
+  public isFile(path:string):boolean {
+    const stats = this.statSync(path);
+    return stats != null && stats.isFile();
+  }
+
+  private statSyncUncached(path:string):fs.Stats|null {
+    try {
+      return fs.statSync(path);
+    } catch (e) {
+      return null;
+    }
+  }
+}

src/utils/languageUtils.ts

@@ -1,11 +1,5 @@
 import { unwrapParentheses } from 'tslint';
-import {
-  Block,
-  CallExpression,
-  Expression,
-  FunctionExpression,
-  SyntaxKind
-} from 'typescript';
+import { Block, CallExpression, Expression, FunctionExpression, SyntaxKind } from 'typescript';
 
 export function getIIFEExpressionBody(expression:Expression):Block|null {
   if (expression.kind === SyntaxKind.CallExpression) {