Merge pull request TypeStrong#62 from DorianGrey/linterOptions-exclude

Correctly handle linterOptions.exclude from tslint config, if present (TypeStrong#61)

Author: piotr-oles

package.json

@@ -49,6 +49,7 @@
     "@types/lodash.isfunction": "^3.0.3",
     "@types/lodash.isstring": "^4.0.3",
     "@types/lodash.startswith": "^4.2.3",
+    "@types/minimatch": "^3.0.1",
     "@types/node": "^8.0.26",
     "@types/webpack": "^3.0.10",
     "chai": "^3.5.0",
@@ -75,6 +76,7 @@
     "lodash.endswith": "^4.2.1",
     "lodash.isfunction": "^3.0.8",
     "lodash.isstring": "^4.0.1",
-    "lodash.startswith": "^4.2.1"
+    "lodash.startswith": "^4.2.1",
+    "minimatch": "^3.0.4"
   }
 }

src/IncrementalChecker.ts

@@ -8,6 +8,16 @@ import FilesWatcher = require('./FilesWatcher');
 import WorkSet = require('./WorkSet');
 import NormalizedMessage = require('./NormalizedMessage');
 import CancellationToken = require('./CancellationToken');
+import minimatch = require('minimatch');
+
+// Need some augmentation here - linterOptions.exclude is not (yet) part of the official
+// types for tslint.
+interface ConfigurationFile extends tslintTypes.Configuration.IConfigurationFile {
+  linterOptions?: {
+    typeCheck?: boolean;
+    exclude?: string[];
+  };
+}
 
 class IncrementalChecker {
   programConfigFile: string;
@@ -19,7 +29,8 @@ class IncrementalChecker {
   files: FilesRegister;
 
   linter: tslintTypes.Linter;
-  linterConfig: tslintTypes.Configuration.IConfigurationFile;
+  linterConfig: ConfigurationFile;
+  linterExclusions: minimatch.IMinimatch[];
 
   program: ts.Program;
   programConfig: ts.ParsedCommandLine;
@@ -39,6 +50,9 @@ class IncrementalChecker {
     this.workNumber = workNumber || 0;
     this.workDivision = workDivision || 1;
     this.checkSyntacticErrors = checkSyntacticErrors || false;
+    // Use empty array of exclusions in general to avoid having
+    // to check of its existence later on.
+    this.linterExclusions = [];
 
     // it's shared between compilations
     this.files = new FilesRegister(() => ({
@@ -58,10 +72,10 @@ class IncrementalChecker {
     );
   }
 
-  static loadLinterConfig(configFile: string) {
+  static loadLinterConfig(configFile: string): ConfigurationFile {
     const tslint: typeof tslintTypes = require('tslint');
 
-    return tslint.Configuration.loadConfigurationFromPath(configFile);
+    return tslint.Configuration.loadConfigurationFromPath(configFile) as ConfigurationFile;
   }
 
   static createProgram(
@@ -110,6 +124,10 @@ class IncrementalChecker {
     return new tslint.Linter({ fix: false }, program);
   }
 
+  static isFileExcluded(filePath: string, linterExclusions: minimatch.IMinimatch[]): boolean {
+    return endsWith(filePath, '.d.ts') || linterExclusions.some(matcher => matcher.match(filePath));
+  }
+
   nextIteration() {
     if (!this.watcher) {
       this.watcher = new FilesWatcher(this.watchPaths, ['.ts', '.tsx']);
@@ -131,6 +149,13 @@ class IncrementalChecker {
 
     if (!this.linterConfig && this.linterConfigFile) {
       this.linterConfig = IncrementalChecker.loadLinterConfig(this.linterConfigFile);
+
+      if (this.linterConfig.linterOptions && this.linterConfig.linterOptions.exclude) {
+        // Pre-build minimatch patterns to avoid additional overhead later on.
+        // Note: Resolving the path is required to properly match against the full file paths,
+        // and also deals with potential cross-platform problems regarding path separators.
+        this.linterExclusions = this.linterConfig.linterOptions.exclude.map(pattern => new minimatch.Minimatch(path.resolve(pattern)));
+      }
     }
 
     this.program = IncrementalChecker.createProgram(this.programConfig, this.files, this.watcher, this.program);
@@ -179,7 +204,7 @@ class IncrementalChecker {
 
     // select files to lint
     const filesToLint = this.files.keys().filter(filePath =>
-      !endsWith(filePath, '.d.ts') && !this.files.getData(filePath).linted
+      !this.files.getData(filePath).linted && !IncrementalChecker.isFileExcluded(filePath, this.linterExclusions)
     );
 
     // calculate subset of work to do

test/unit/IncrementalChecker.spec.js

@@ -0,0 +1,43 @@
+var describe = require('mocha').describe;
+var it = require('mocha').it;
+var expect = require('chai').expect;
+var path = require('path');
+var minimatch = require('minimatch');
+
+var IncrementalChecker = require('../../lib/IncrementalChecker');
+
+describe('[UNIT] IncrementalChecker', function () {
+  describe('isFileExcluded', function() {
+    it('should properly filter definition files and listed exclusions', function() {
+      var linterConfig = {
+        linterOptions: {
+          exclude: [
+            'src/formatter/**/*.ts'
+          ]
+        }
+      };
+
+      var exclusions = linterConfig.linterOptions.exclude.map(function(pattern) {
+        return new minimatch.Minimatch(path.resolve(pattern));
+      });
+
+      var testPaths = [
+        'src/formatter/codeframeFormatter.ts',
+        'src/formatter/defaultFormatter.ts',
+        'src/service.ts',
+        'node_modules/tslint/lib/configuration.d.ts',
+      ].map(function(p) {
+        return path.resolve(p);
+      });
+
+      var pathsAreExcluded = testPaths.map(function(p) {
+        return IncrementalChecker.isFileExcluded(p, exclusions);
+      });
+
+      expect(pathsAreExcluded[0]).to.be.true;
+      expect(pathsAreExcluded[1]).to.be.true;
+      expect(pathsAreExcluded[2]).to.be.false;
+      expect(pathsAreExcluded[3]).to.be.true;
+    });
+  });
+});

yarn.lock

@@ -44,6 +44,10 @@
   version "4.14.74"
   resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.74.tgz#ac3bd8db988e7f7038e5d22bd76a7ba13f876168"
 
+"@types/minimatch@^3.0.1":
+  version "3.0.1"
+  resolved "https://registry.yarnpkg.com/@types/minimatch/-/minimatch-3.0.1.tgz#b683eb60be358304ef146f5775db4c0e3696a550"
+
 "@types/node@*", "@types/node@^8.0.26":
   version "8.0.26"
   resolved "https://registry.yarnpkg.com/@types/node/-/node-8.0.26.tgz#4d58be925306fd22b1141085535a0268b8beb189"