Skip to content

Commit

Permalink
fix(disable-comment): log a warning when a specified mutator doesn't …
Browse files Browse the repository at this point in the history
…exist(#3842)

Log a warning when the name of the mutator inside a Stryker disable comment does not exist:

For example:

> WARN Unused 'Stryker disable next-line' directive. Mutator with name 'RandomName' not found. Directive found at: example.ts:1
  • Loading branch information
Giovds authored Nov 22, 2022
1 parent 79a13d4 commit fe79d49
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 17 deletions.
2 changes: 1 addition & 1 deletion packages/instrumenter/src/instrumenter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class Instrumenter {
const parse = this._createParser(options);
for await (const { name, mutate, content } of files) {
const ast = await parse(content, name);
this._transform(ast, mutantCollector, { options, mutateDescription: toBabelLineNumber(mutate) });
this._transform(ast, mutantCollector, { options, mutateDescription: toBabelLineNumber(mutate), logger: this.logger });
const mutatedContent = this._print(ast);
outFiles.push({
name,
Expand Down
4 changes: 2 additions & 2 deletions packages/instrumenter/src/transformers/babel-transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type PlacementMap = Map<types.Node, MutantsPlacement<types.Node>>;
export const transformBabel: AstTransformer<ScriptFormat> = (
{ root, originFileName, rawContent, offset },
mutantCollector,
{ options, mutateDescription },
{ options, mutateDescription, logger },
mutators = allMutators,
mutantPlacers = allMutantPlacers
) => {
Expand All @@ -39,7 +39,7 @@ export const transformBabel: AstTransformer<ScriptFormat> = (
const placementMap: PlacementMap = new Map();

// Create the bookkeeper responsible for the // Stryker ... directives
const directiveBookkeeper = new DirectiveBookkeeper();
const directiveBookkeeper = new DirectiveBookkeeper(logger, mutators, originFileName);

// Now start the actual traversing of the AST
//
Expand Down
46 changes: 37 additions & 9 deletions packages/instrumenter/src/transformers/directive-bookkeeper.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import type { types } from '@babel/core';
import { notEmpty } from '@stryker-mutator/util';

import { Logger } from '@stryker-mutator/api/logging';

import { NodeMutator } from '../mutators/node-mutator.js';

const WILDCARD = 'all';
const DEFAULT_REASON = 'Ignored using a comment';

Expand Down Expand Up @@ -47,18 +51,26 @@ export class DirectiveBookkeeper {
private readonly strykerCommentDirectiveRegex = /^\s?Stryker (disable|restore)(?: (next-line))? ([a-zA-Z, ]+)(?::(.+)?)?/;

private currentIgnoreRule = rootRule;
private readonly allMutatorNames: string[];

constructor(private readonly logger: Logger, private readonly allMutators: NodeMutator[], private readonly originFileName: string) {
this.allMutatorNames = this.allMutators.map((x) => x.name.toLowerCase());
}

public processStrykerDirectives({ loc, leadingComments }: types.Node): void {
leadingComments
?.map(
(comment) =>
this.strykerCommentDirectiveRegex.exec(comment.value) as
| [fullMatch: string, directiveType: string, scope: string | undefined, mutators: string, reason: string | undefined]
| null
)
.filter(notEmpty)
.forEach(([, directiveType, scope, mutators, optionalReason]) => {
const mutatorNames = mutators.split(',').map((mutator) => mutator.trim().toLowerCase());
?.map((comment) => ({
comment,
matchResult: this.strykerCommentDirectiveRegex.exec(comment.value) as
| [fullMatch: string, directiveType: string, scope: string | undefined, mutators: string, reason: string | undefined]
| null,
}))
.filter(({ matchResult }) => notEmpty(matchResult))
.forEach(({ comment, matchResult }) => {
const [, directiveType, scope, mutators, optionalReason] = matchResult!;
let mutatorNames = mutators.split(',').map((mutator) => mutator.trim());
this.warnAboutUnusedDirective(mutatorNames, directiveType, scope, comment);
mutatorNames = mutatorNames.map((mutator) => mutator.toLowerCase());
const reason = (optionalReason ?? DEFAULT_REASON).trim();
switch (directiveType) {
case 'disable':
Expand Down Expand Up @@ -89,4 +101,20 @@ export class DirectiveBookkeeper {
mutatorName = mutatorName.toLowerCase();
return this.currentIgnoreRule.findIgnoreReason(mutatorName, line);
}

private warnAboutUnusedDirective(mutators: string[], directiveType: string, scope: string | undefined, comment: types.Comment) {
for (const mutator of mutators) {
if (mutator === WILDCARD) continue;
if (!this.allMutatorNames.includes(mutator.toLowerCase())) {
this.logger.warn(
// Scope can be global and therefore undefined
`Unused 'Stryker ${
scope ? directiveType + ' ' + scope : directiveType
}' directive. Mutator with name '${mutator}' not found. Directive found at: ${this.originFileName}:${comment.loc!.start.line}:${
comment.loc!.start.column
}.`
);
}
}
}
}
5 changes: 4 additions & 1 deletion packages/instrumenter/src/transformers/transformer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { MutateDescription } from '@stryker-mutator/api/core';
import { I } from '@stryker-mutator/util';

import { Logger } from '@stryker-mutator/api/src/logging/logger';

import { Ast, AstByFormat, AstFormat } from '../syntax/index.js';

import { TransformerOptions } from './transformer-options.js';
Expand All @@ -18,7 +20,7 @@ import { MutantCollector } from './mutant-collector.js';
export function transform(
ast: Ast,
mutantCollector: I<MutantCollector>,
transformerContext: Pick<TransformerContext, 'mutateDescription' | 'options'>
transformerContext: Pick<TransformerContext, 'logger' | 'mutateDescription' | 'options'>
): void {
const context: TransformerContext = {
...transformerContext,
Expand All @@ -42,4 +44,5 @@ export interface TransformerContext {
transform: AstTransformer<AstFormat>;
options: TransformerOptions;
mutateDescription: MutateDescription;
logger: Logger;
}
3 changes: 3 additions & 0 deletions packages/instrumenter/test/helpers/stubs.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import sinon from 'sinon';

import { testInjector } from '@stryker-mutator/test-helpers';

import { ParserContext } from '../../src/parsers/parser-context.js';
import { PrinterContext } from '../../src/printers/index.js';
import { TransformerContext } from '../../src/transformers/index.js';
Expand All @@ -23,5 +25,6 @@ export function transformerContextStub(): sinon.SinonStubbedInstance<Transformer
transform: sinon.stub(),
mutateDescription: true,
options: createTransformerOptions(),
logger: testInjector.logger,
};
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { expect } from 'chai';
import { testInjector } from '@stryker-mutator/test-helpers';

import { createHtmlAst, createJSAst, createTransformerOptions, createTSAst } from '../helpers/factories.js';
import { transform } from '../../src/transformers/index.js';
Expand All @@ -9,21 +10,21 @@ describe('transformers integration', () => {
const htmlAst = createHtmlAst();
htmlAst.root.scripts.push(createJSAst({ rawContent: 'const foo = 40 + 2' }));
const mutantCollector = new MutantCollector();
transform(htmlAst, mutantCollector, { options: createTransformerOptions(), mutateDescription: true });
transform(htmlAst, mutantCollector, { options: createTransformerOptions(), mutateDescription: true, logger: testInjector.logger });
expect(mutantCollector.mutants).lengthOf(1);
expect(htmlAst).matchSnapshot();
});
it('should transform a js file', () => {
const jsAst = createJSAst({ rawContent: 'const foo = 40 + 2' });
const mutantCollector = new MutantCollector();
transform(jsAst, mutantCollector, { options: createTransformerOptions(), mutateDescription: true });
transform(jsAst, mutantCollector, { options: createTransformerOptions(), mutateDescription: true, logger: testInjector.logger });
expect(mutantCollector.mutants).lengthOf(1);
expect(jsAst).matchSnapshot();
});
it('should transform a ts file', () => {
const tsAst = createTSAst({ rawContent: 'const foo: number = 40 + 2' });
const mutantCollector = new MutantCollector();
transform(tsAst, mutantCollector, { options: createTransformerOptions(), mutateDescription: true });
transform(tsAst, mutantCollector, { options: createTransformerOptions(), mutateDescription: true, logger: testInjector.logger });
expect(mutantCollector.mutants).lengthOf(1);
expect(tsAst).matchSnapshot();
});
Expand Down
6 changes: 5 additions & 1 deletion packages/instrumenter/test/unit/instrumenter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ describe(Instrumenter.name, () => {
const expected: transformers.TransformerOptions = createInstrumenterOptions({
excludedMutations: [],
});
expect(actual).deep.eq({ options: expected, mutateDescription: [{ start: { line: 1, column: 0 }, end: { line: 7, column: 42 } }] });
expect(actual).deep.eq({
options: expected,
mutateDescription: [{ start: { line: 1, column: 0 }, end: { line: 7, column: 42 } }],
logger: testInjector.logger,
});
});

it('should log about instrumenting', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,68 @@ describe('babel-transformer', () => {
expect(notIgnoredMutants()).lengthOf(1);
});

it('should warn when a mutator name without scope does not match any of the enabled mutators.', () => {
const ast = createTSAst({
rawContent: `// Stryker disable RandomName: This Mutator does not exist
function test(a, b) {
return a - b >= 0 ? 1 : -1;
}
`,
});
act(ast);

expect(context.logger.warn).calledWithMatch(
sinon.match("Unused 'Stryker disable' directive. Mutator with name 'RandomName' not found. Directive found at: example.ts:1")
);
});

it('should warn when a mutator name with scope does not match any of the enabled mutators.', () => {
const ast = createTSAst({
rawContent: `// Stryker disable next-line RandomName: This Mutator does not exist
function test(a, b) {
return a - b >= 0 ? 1 : -1;
}
`,
});
act(ast);

expect(context.logger.warn).calledWithMatch(
sinon.match("Unused 'Stryker disable next-line' directive. Mutator with name 'RandomName' not found. Directive found at: example.ts:1")
);
});

it('should warn when a mutator name does not match any of the enabled mutators.', () => {
const ast = createTSAst({
rawContent: `// Stryker disable Foo
// Stryker disable RandomName: This Mutator does not exist
// Stryker disable Plus
function test(a, b) {
return a - b >= 0 ? 1 : -1;
}
`,
});
act(ast);

expect(context.logger.warn).calledWithMatch(
sinon.match("Unused 'Stryker disable' directive. Mutator with name 'RandomName' not found. Directive found at: example.ts:2")
);
});

it('should not warn when a disabled Mutator exists.', () => {
const ast = createTSAst({
rawContent: `
// Stryker disable Foo
// Stryker disable all
function test(a, b) {
return a - b >= 0 ? 1 : -1;
}
`,
});
act(ast);

expect(context.logger.warn).not.called;
});

it('should allow to restore for next-line using a specific "Stryker restore next-line mutator" comment', () => {
const ast = createTSAst({
rawContent: `
Expand Down

0 comments on commit fe79d49

Please sign in to comment.