Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn when Mutator in directive does not exist #3842

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