Skip to content

Commit

Permalink
feat(typescript): Disable type checking (#2446)
Browse files Browse the repository at this point in the history
Disable type checks by inserting `// @ts-nocheck` and removing all other `// @ts-xxx` directives. 

It can be configured with:

```js
{
  "disableTypeChecks": "{test,src,lib}/**/*.{js,ts,jsx,tsx,html,vue}" // configure a pattern or `false` here
  "warnings": {
    "preprocessorErrors": true // enable/disable error logging here (default: true)
  }
}
```

You can disable it completely by setting `disableTypeChecks` to `false`. The default is `"{test,src,lib}/**/*.{js,ts,jsx,tsx,html,vue}"`.

The disabling of type checks is done in the `DisableTypeChecksPreprocessor` class. It calls the function in the `instrumenter` package to do the actual disabling. The reason for this is that files need to be parsed to distinguish between code and comments (`@ts-xxx` directives are only removed when they appear inside comments). It also works with HTML and vue files, so it also supports use cases where type checking is done inside vue files (with the vue-cli). 

Whenever parsing a file results in an error, a warning is logged and mutation testing will still proceed. The warning can be disabled by setting `warnings.preprocessorErrors` to `false`.

This setting replaces both `sandbox.fileHeaders` and `sandbox.stripComments`, since those resulted in a lot of problems (see #2438 ).
  • Loading branch information
nicojs authored Sep 2, 2020
1 parent d90f08a commit 3ff996b
Show file tree
Hide file tree
Showing 35 changed files with 890 additions and 286 deletions.
53 changes: 18 additions & 35 deletions packages/api/schema/stryker-core.json
Original file line number Diff line number Diff line change
Expand Up @@ -164,37 +164,6 @@
"minimum": 0,
"maximum": 100
},
"sandboxOptions": {
"type": "object",
"additionalProperties": false,
"properties": {
"fileHeaders": {
"type": "object",
"title": "SandboxFileHeaders",
"description": "Configure additional headers to be added to files inside your sandbox. These headers will be added after Stryker has instrumented your code with mutants, but before a test runner or build command is executed. This is used to ignore typescript compile errors and eslint warnings that might have been added in the process of instrumenting your code with mutants. The default setting should work for most use cases.",
"additionalProperties": {
"type": "string"
},
"default": {
"**/*+(.js|.ts|.cjs|.mjs)?(x)": "/* eslint-disable */\n// @ts-nocheck\n"
}
},
"stripComments": {
"description": "Configure files to be stripped of comments (either single line with `//` or multi line with `/**/`. These comments will be stripped after Stryker has instrumented your code with mutants, but before a test runner or build command is executed. This is used to remove any lingering `// @ts-check` or `// @ts-expect-error` comments that interfere with typescript compilation. The default setting allows comments to be stripped from all JavaScript and friend files in your sandbox, you can specify a different glob expression or set it to `false` to completely disable this behavior.",
"anyOf": [
{
"enum": [
false
]
},
{
"type": "string"
}
],
"default": "**/*+(.js|.ts|.cjs|.mjs)?(x)"
}
}
},
"mutatorDescriptor": {
"type": "object",
"additionalProperties": false,
Expand Down Expand Up @@ -235,6 +204,11 @@
"description": "decide whether or not to log warnings when additional stryker options are configured",
"type": "boolean",
"default": true
},
"preprocessorErrors": {
"description": "decide whether or not to log warnings when a preprocessor error occurs. For example, when the disabling of type errors fails.",
"type": "boolean",
"default": true
}
}
}
Expand Down Expand Up @@ -364,10 +338,19 @@
"description": "The options for the html reporter",
"$ref": "#/definitions/htmlReporterOptions"
},
"sandbox": {
"description": "Configure how the files in the sandbox behave. The sandbox is a copy of your source code where Stryker does mutation testing.",
"$ref": "#/definitions/sandboxOptions",
"default": {}
"disableTypeChecks": {
"description": "Configure a pattern that matches the files of which type checking has to be disabled. This is needed because Stryker will create (typescript) type errors when inserting the mutants in your code. Stryker disables type checking by inserting `// @ts-nocheck` atop those files and removing other `// @ts-xxx` directives (so they won't interfere with `@ts-nocheck`). The default setting allows these directives to be stripped from all JavaScript and friend files in `lib`, `src` and `test` directories. You can specify a different glob expression or set it to `false` to completely disable this behavior.",
"anyOf": [
{
"enum": [
false
]
},
{
"type": "string"
}
],
"default": "{test,src,lib}/**/*.{js,ts,jsx,tsx,html,vue}"
},
"symlinkNodeModules": {
"description": "The 'symlinkNodeModules' value indicates whether Stryker should create a symbolic link to your current node_modules directory in the sandbox directories. This makes running your tests by Stryker behave more like your would run the tests yourself in your project directory. Only disable this setting if you really know what you are doing.",
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/config/OptionsValidator.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import os = require('os');

import Ajv = require('ajv');
import { StrykerOptions, strykerCoreSchema, WarningOptions } from '@stryker-mutator/api/core';
import { StrykerOptions, strykerCoreSchema } from '@stryker-mutator/api/core';
import { tokens, commonTokens } from '@stryker-mutator/api/plugin';
import { noopLogger, propertyPath, deepFreeze } from '@stryker-mutator/util';
import { noopLogger, propertyPath, deepFreeze, PropertyPathBuilder } from '@stryker-mutator/util';
import { Logger } from '@stryker-mutator/api/logging';
import type { JSONSchema7 } from 'json-schema';

Expand Down Expand Up @@ -129,7 +129,7 @@ export function markUnknownOptions(options: StrykerOptions, schema: JSONSchema7,
log.warn(`Unknown stryker config option "${unknownPropertyName}".`);
});

const p = `${propertyPath<StrykerOptions>('warnings')}.${propertyPath<WarningOptions>('unknownOptions')}`;
const p = PropertyPathBuilder.create<StrykerOptions>().prop('warnings').prop('unknownOptions').build();

log.warn(`Possible causes:
* Is it a typo on your end?
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/di/coreTokens.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export const checkerPool = 'checkerPool';
export const checkerFactory = 'checkerFactory';
export const checkerConcurrencyTokens = 'checkerConcurrencyTokens';
export const disableTypeChecksHelper = 'disableTypeChecksHelper';
export const execa = 'execa';
export const cliOptions = 'cliOptions';
export const configReader = 'configReader';
Expand Down
10 changes: 6 additions & 4 deletions packages/core/src/sandbox/create-preprocessor.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import { tokens, Injector, commonTokens, PluginContext } from '@stryker-mutator/api/plugin';

import { disableTypeChecks } from '@stryker-mutator/instrumenter';

import { coreTokens } from '../di';

import { TSConfigPreprocessor } from './ts-config-preprocessor';
import { FileHeaderPreprocessor } from './file-header-preprocessor';
import { FilePreprocessor } from './file-preprocessor';
import { MultiPreprocessor } from './multi-preprocessor';
import { StripCommentsPreprocessor } from './strip-comments-preprocessor';
import { DisableTypeChecksPreprocessor } from './disable-type-checks-preprocessor';

createPreprocessor.inject = tokens(commonTokens.injector);
export function createPreprocessor(injector: Injector<PluginContext>): FilePreprocessor {
return new MultiPreprocessor([
injector.injectClass(StripCommentsPreprocessor),
injector.provideValue(coreTokens.disableTypeChecksHelper, disableTypeChecks).injectClass(DisableTypeChecksPreprocessor),
injector.injectClass(TSConfigPreprocessor),
injector.injectClass(FileHeaderPreprocessor),
]);
}
61 changes: 61 additions & 0 deletions packages/core/src/sandbox/disable-type-checks-preprocessor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import path = require('path');

import minimatch = require('minimatch');
import { commonTokens, tokens } from '@stryker-mutator/api/plugin';
import { File, StrykerOptions } from '@stryker-mutator/api/core';
import type { disableTypeChecks } from '@stryker-mutator/instrumenter';
import { Logger } from '@stryker-mutator/api/logging';
import { propertyPath, PropertyPathBuilder } from '@stryker-mutator/util';

import { coreTokens } from '../di';
import { isWarningEnabled } from '../utils/objectUtils';

import { FilePreprocessor } from './file-preprocessor';

/**
* Disabled type checking by inserting `@ts-nocheck` atop TS/JS files and removing other @ts-xxx directives from comments:
* @see https://github.com/stryker-mutator/stryker/issues/2438
*/
export class DisableTypeChecksPreprocessor implements FilePreprocessor {
public static readonly inject = tokens(commonTokens.logger, commonTokens.options, coreTokens.disableTypeChecksHelper);
constructor(private readonly log: Logger, private readonly options: StrykerOptions, private readonly impl: typeof disableTypeChecks) {}

public async preprocess(files: File[]): Promise<File[]> {
if (this.options.disableTypeChecks === false) {
return files;
} else {
const pattern = path.resolve(this.options.disableTypeChecks);
let warningLogged = false;
const outFiles = await Promise.all(
files.map(async (file) => {
if (minimatch(path.resolve(file.name), pattern)) {
try {
return await this.impl(file, { plugins: this.options.mutator.plugins });
} catch (err) {
if (isWarningEnabled('preprocessorErrors', this.options.warnings)) {
warningLogged = true;
this.log.warn(
`Unable to disable type checking for file "${
file.name
}". Shouldn't type checking be disabled for this file? Consider configuring a more restrictive "${propertyPath<StrykerOptions>(
'disableTypeChecks'
)}" settings (or turn it completely off with \`false\`)`,
err
);
}
return file;
}
} else {
return file;
}
})
);
if (warningLogged) {
this.log.warn(
`(disable "${PropertyPathBuilder.create<StrykerOptions>().prop('warnings').prop('preprocessorErrors')}" to ignore this warning`
);
}
return outFiles;
}
}
}
27 changes: 0 additions & 27 deletions packages/core/src/sandbox/file-header-preprocessor.ts

This file was deleted.

33 changes: 0 additions & 33 deletions packages/core/src/sandbox/strip-comments-preprocessor.ts

This file was deleted.

10 changes: 5 additions & 5 deletions packages/core/test/unit/sandbox/create-preprocessor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ describe(createPreprocessor.name, () => {
assertions.expectTextFilesEqual(output, [new File(path.resolve('tsconfig.json'), '{\n "extends": "../../../tsconfig.settings.json"\n}')]);
});

it('should add a header to .ts files', async () => {
const output = await sut.preprocess([new File(path.resolve('app.ts'), 'foo.bar()')]);
assertions.expectTextFilesEqual(output, [new File(path.resolve('app.ts'), '/* eslint-disable */\n// @ts-nocheck\nfoo.bar()')]);
it('should disable type checking for .ts files', async () => {
const output = await sut.preprocess([new File(path.resolve('src/app.ts'), 'foo.bar()')]);
assertions.expectTextFilesEqual(output, [new File(path.resolve('src/app.ts'), '// @ts-nocheck\nfoo.bar()')]);
});

it('should strip // @ts-expect-error (see https://github.com/stryker-mutator/stryker/issues/2364)', async () => {
const output = await sut.preprocess([new File(path.resolve('app.ts'), '// @ts-expect-error\nfoo.bar()')]);
assertions.expectTextFilesEqual(output, [new File(path.resolve('app.ts'), '/* eslint-disable */\n// @ts-nocheck\n\nfoo.bar()')]);
const output = await sut.preprocess([new File(path.resolve('src/app.ts'), '// @ts-expect-error\nfoo.bar()')]);
assertions.expectTextFilesEqual(output, [new File(path.resolve('src/app.ts'), '// @ts-nocheck\n// \nfoo.bar()')]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import path = require('path');

import { File } from '@stryker-mutator/api/core';
import { assertions, testInjector } from '@stryker-mutator/test-helpers';
import sinon = require('sinon');

import { expect } from 'chai';

import { coreTokens } from '../../../src/di';
import { DisableTypeChecksPreprocessor } from '../../../src/sandbox/disable-type-checks-preprocessor';

describe(DisableTypeChecksPreprocessor.name, () => {
let sut: DisableTypeChecksPreprocessor;
let disableTypeCheckingStub: sinon.SinonStub;

beforeEach(() => {
disableTypeCheckingStub = sinon.stub();
sut = testInjector.injector.provideValue(coreTokens.disableTypeChecksHelper, disableTypeCheckingStub).injectClass(DisableTypeChecksPreprocessor);
});

['.ts', '.tsx', '.js', '.jsx', '.html', '.vue'].forEach((extension) => {
it(`should disable type checking a ${extension} file by default`, async () => {
const fileName = `src/app${extension}`;
const expectedFile = new File(path.resolve(fileName), 'output');
const inputFile = new File(path.resolve(fileName), 'input');
const input = [inputFile];
disableTypeCheckingStub.resolves(expectedFile);
const output = await sut.preprocess(input);
expect(disableTypeCheckingStub).calledWith(inputFile);
assertions.expectTextFilesEqual(output, [expectedFile]);
});
});

it('should be able to override "disableTypeChecks" glob pattern', async () => {
testInjector.options.disableTypeChecks = 'src/**/*.ts';
const expectedFile = new File(path.resolve('src/app.ts'), 'output');
const input = [new File(path.resolve('src/app.ts'), 'input')];
disableTypeCheckingStub.resolves(expectedFile);
const output = await sut.preprocess(input);
assertions.expectTextFilesEqual(output, [expectedFile]);
});

it('should not disable type checking when the "disableTypeChecks" glob pattern does not match', async () => {
testInjector.options.disableTypeChecks = 'src/**/*.ts';
const expectedFiles = [new File(path.resolve('test/app.spec.ts'), 'input')];
disableTypeCheckingStub.resolves(new File('', 'not expected'));
const output = await sut.preprocess(expectedFiles);
assertions.expectTextFilesEqual(output, expectedFiles);
});

it('should not disable type checking if "disableTypeChecks" is set to `false`', async () => {
const input = [
new File(path.resolve('src/app.ts'), '// @ts-expect-error\nfoo.bar();'),
new File(path.resolve('test/app.spec.ts'), '/* @ts-expect-error */\nfoo.bar();'),
new File(path.resolve('testResources/project/app.ts'), '/* @ts-expect-error */\nfoo.bar();'),
];
testInjector.options.disableTypeChecks = false;
const output = await sut.preprocess(input);
assertions.expectTextFilesEqual(output, input);
});

it('should not crash on error, instead log a warning', async () => {
const input = [new File('src/app.ts', 'input')];
const expectedError = new Error('Expected error for testing');
disableTypeCheckingStub.rejects(expectedError);
const output = await sut.preprocess(input);
expect(testInjector.logger.warn).calledWithExactly(
'Unable to disable type checking for file "src/app.ts". Shouldn\'t type checking be disabled for this file? Consider configuring a more restrictive "disableTypeChecks" settings (or turn it completely off with `false`)',
expectedError
);
expect(testInjector.logger.warn).calledWithExactly('(disable "warnings.preprocessorErrors" to ignore this warning');
assertions.expectTextFilesEqual(output, input);
});
});
Loading

0 comments on commit 3ff996b

Please sign in to comment.