Merge pull request #268 from phryneas/fix-linter-tests

improve generation of diagnostics, fix linter tests

Author: phryneas

src/patchTypescript.ts

@@ -0,0 +1,64 @@
+// tslint:disable-next-line:no-implicit-dependencies
+import * as ts from 'typescript'; // Imported for types alone
+
+export interface TypeScriptPatchConfig {
+  /**
+   * Ususally, the compilerHost created with typescript.createWatchCompilerHost will bail out of diagnostics collection if there has been any syntactic error.
+   * (see [`emitFilesAndReportErrors`](https://github.com/Microsoft/TypeScript/blob/89386ddda7dafc63cb35560e05412487f47cc267/src/compiler/watch.ts#L141) )
+   * If this plugin is running with `checkSyntacticErrors: false`, this might lead to situations where no syntactic errors are reported within webpack
+   * (because the file causing a syntactic error might not get processed by ts-loader), but there are semantic errors that would be missed due to this behavior.
+   * This ensures that the compilerHost always assumes that there were no syntactic errors to be found and continues to check for semantic errors.
+   */
+  skipGetSyntacticDiagnostics: boolean;
+}
+
+/**
+ * While it is often possible to pass a wrapped or modified copy of `typescript` or `typescript.sys` as a function argument to override/extend some typescript-internal behavior,
+ * sometimes the typescript-internal code ignores these passed objects and directly references the internal `typescript` object reference.
+ * In these situations, the only way of consistently overriding some behavior is to directly replace methods on the `typescript` object.
+ *
+ * So beware, this method directly modifies the passed `typescript` object!
+ * @param typescript TypeScript instance to patch
+ * @param config
+ */
+export function patchTypescript(
+  typescript: typeof ts,
+  config: TypeScriptPatchConfig
+) {
+  if (config.skipGetSyntacticDiagnostics) {
+    patchSkipGetSyntacticDiagnostics(typescript);
+  }
+}
+
+/**
+ * Overrides the [`typescript.createEmitAndSemanticDiagnosticsBuilderProgram`](https://github.com/Microsoft/TypeScript/blob/89386ddda7dafc63cb35560e05412487f47cc267/src/compiler/builder.ts#L1176)
+ * method to return a `ts.Program` instance that does not emit syntactic errors,
+ * to prevent the [`typescript.createWatchCompilerHost`](https://github.com/Microsoft/TypeScript/blob/89386ddda7dafc63cb35560e05412487f47cc267/src/compiler/watch.ts#L333)
+ * method from bailing during diagnostic collection in the [`emitFilesAndReportErrors`](https://github.com/Microsoft/TypeScript/blob/89386ddda7dafc63cb35560e05412487f47cc267/src/compiler/watch.ts#L141) callback.
+ *
+ * See the description of TypeScriptPatchConfig.skipGetSyntacticDiagnostics and
+ * [this github discussion](https://github.com/Realytics/fork-ts-checker-webpack-plugin/issues/257#issuecomment-485414182)
+ * for further information on this problem & solution.
+ */
+function patchSkipGetSyntacticDiagnostics(typescript: typeof ts) {
+  const {
+    createEmitAndSemanticDiagnosticsBuilderProgram: originalCreateEmitAndSemanticDiagnosticsBuilderProgram
+  } = typescript;
+
+  const patchedMethods: Pick<
+    typeof ts,
+    'createEmitAndSemanticDiagnosticsBuilderProgram'
+  > = {
+    createEmitAndSemanticDiagnosticsBuilderProgram(...args: any[]) {
+      const program = originalCreateEmitAndSemanticDiagnosticsBuilderProgram.apply(
+        typescript,
+        args as any
+      );
+      program.getSyntacticDiagnostics = () => [];
+      return program;
+    }
+  };
+
+  // directly patch the typescript object!
+  Object.assign(typescript, patchedMethods);
+}

src/service.ts

@@ -13,6 +13,7 @@ import {
 } from './NormalizedMessageFactories';
 import { RpcProvider } from 'worker-rpc';
 import { RunPayload, RunResult, RUN } from './RpcTypes';
+import { TypeScriptPatchConfig, patchTypescript } from './patchTypescript';
 
 const rpc = new RpcProvider(message => {
   try {
@@ -25,6 +26,13 @@ const rpc = new RpcProvider(message => {
 process.on('message', message => rpc.dispatch(message));
 
 const typescript: typeof ts = require(process.env.TYPESCRIPT_PATH!);
+const patchConfig: TypeScriptPatchConfig = {
+  skipGetSyntacticDiagnostics:
+    process.env.USE_INCREMENTAL_API === 'true' &&
+    process.env.CHECK_SYNTACTIC_ERRORS !== 'true'
+};
+
+patchTypescript(typescript, patchConfig);
 
 // message factories
 export const createNormalizedMessageFromDiagnostic = makeCreateNormalizedMessageFromDiagnostic(

test/integration/index.spec.js

@@ -23,13 +23,15 @@ function makeCommonTests(useTypescriptIncrementalApi) {
     this.timeout(60000);
     var plugin;
 
+    const overrideOptions = { useTypescriptIncrementalApi };
+
     function createCompiler(
       options,
       happyPackMode,
       entryPoint = './src/index.ts'
     ) {
       options = options || {};
-      options.useTypescriptIncrementalApi = useTypescriptIncrementalApi;
+      options = { ...options, ...overrideOptions };
       var compiler = helpers.createCompiler(options, happyPackMode, entryPoint);
       plugin = compiler.plugin;
       return compiler.webpack;
@@ -80,17 +82,42 @@ function makeCommonTests(useTypescriptIncrementalApi) {
       expect(plugin.watch).to.deep.equal(['/test']);
     });
 
+    it('should find lint warnings', function(callback) {
+      const fileName = 'lintingError2';
+      helpers.testLintAutoFixTest(
+        callback,
+        fileName,
+        {
+          tslint: path.resolve(__dirname, './project/tslint.json'),
+          ignoreLintWarnings: false,
+          ...overrideOptions
+        },
+        (err, stats) => {
+          expect(
+            stats.compilation.warnings.filter(warning =>
+              warning.message.includes('missing whitespace')
+            ).length
+          ).to.be.greaterThan(0);
+        }
+      );
+    });
+
     it('should not print warnings when ignoreLintWarnings passed as option', function(callback) {
       const fileName = 'lintingError2';
       helpers.testLintAutoFixTest(
         callback,
         fileName,
         {
-          tslint: true,
-          ignoreLintWarnings: true
+          tslint: path.resolve(__dirname, './project/tslint.json'),
+          ignoreLintWarnings: true,
+          ...overrideOptions
         },
         (err, stats) => {
-          expect(stats.compilation.warnings.length).to.be.eq(0);
+          expect(
+            stats.compilation.warnings.filter(warning =>
+              warning.message.includes('missing whitespace')
+            ).length
+          ).to.be.equal(0);
         }
       );
     });
@@ -101,11 +128,16 @@ function makeCommonTests(useTypescriptIncrementalApi) {
         callback,
         fileName,
         {
-          tslint: true,
-          ignoreLintWarnings: true
+          tslint: path.resolve(__dirname, './project/tslint.json'),
+          ignoreLintWarnings: true,
+          ...overrideOptions
         },
         (err, stats) => {
-          expect(stats.compilation.errors.length).to.be.eq(0);
+          expect(
+            stats.compilation.errors.filter(error =>
+              error.message.includes('missing whitespace')
+            ).length
+          ).to.be.equals(0);
         }
       );
     });
@@ -157,7 +189,8 @@ function makeCommonTests(useTypescriptIncrementalApi) {
         {
           tslintAutoFix: true,
           tslint: path.resolve(__dirname, './project/tslint.autofix.json'),
-          tsconfig: false
+          tsconfig: false,
+          ...overrideOptions
         },
         (err, stats, formattedFileContents) => {
           expect(stats.compilation.warnings.length).to.be.eq(0);
@@ -179,7 +212,8 @@ function makeCommonTests(useTypescriptIncrementalApi) {
         callback,
         fileName,
         {
-          tslint: true
+          tslint: true,
+          ...overrideOptions
         },
         (err, stats) => {
           expect(stats.compilation.warnings.length).to.be.eq(7);
@@ -362,7 +396,8 @@ function makeCommonTests(useTypescriptIncrementalApi) {
       helpers.testLintHierarchicalConfigs(
         callback,
         {
-          tslint: true
+          tslint: true,
+          ...overrideOptions
         },
         (err, stats) => {
           /*