fix(instrumenter): place mutants in if statements (#2481)

Place mutants in a statement with an `if` statement instead of a `switch` statement.

Fixes #2469

e2e/test/mocha-javascript/src/IsEven.js

@@ -0,0 +1,18 @@
+
+module.exports.isEven = function(number) {
+  // Note: Implemented with a case switch statement, in order to reproduce this issue:
+  // https://github.com/stryker-mutator/stryker/issues/2469#issuecomment-690303849
+  let mod2 = number % 2; 
+  let isEven;
+  switch(mod2) {
+    case 0: {
+      isEven = true;
+      break;
+    }
+    case 1: {
+      isEven = false;
+      break;
+    }
+  }
+  return isEven;
+}

e2e/test/mocha-javascript/test/unit/IsEven.spec.js

@@ -0,0 +1,13 @@
+const { isEven } = require('../../src/IsEven');
+const { expect } = require('chai');
+
+describe('isEven', () => {
+
+  it('should be false for 1', () => {
+    expect(isEven(1)).false;
+  });
+
+  it('should be true for 2', () => {
+    expect(isEven(2)).true
+  });
+});

e2e/test/mocha-javascript/verify/verify.ts

@@ -12,15 +12,15 @@ describe('Verify stryker has ran correctly', () => {
     await expectMetricsResult({
 
       metrics: produceMetrics({
-        killed: 18,
-        mutationScore: 66.67,
-        mutationScoreBasedOnCoveredCode: 66.67,
+        killed: 26,
+        mutationScore: 74.29,
+        mutationScoreBasedOnCoveredCode: 74.29,
         survived: 9,
-        totalCovered: 27,
-        totalDetected: 18,
-        totalMutants: 27,
+        totalCovered: 35,
+        totalDetected: 26,
+        totalMutants: 35,
         totalUndetected: 9,
-        totalValid: 27
+        totalValid: 35
       })
     });
   });

packages/instrumenter/src/mutant-placers/conditional-expression-mutant-placer.ts → packages/instrumenter/src/mutant-placers/expression-mutant-placer.ts

@@ -50,7 +50,7 @@ function isValidParent(child: NodePath<types.Expression>) {
 /**
  * Places the mutants with a conditional expression: `global.activeMutant === 1? mutatedCode : regularCode`;
  */
-const conditionalExpressionMutantPlacer: MutantPlacer = (path: NodePath, mutants: Mutant[]): boolean => {
+const expressionMutantPlacer: MutantPlacer = (path: NodePath, mutants: Mutant[]): boolean => {
   if (path.isExpression() && isValidParent(path)) {
     // First calculated the mutated ast before we start to apply mutants.
     const appliedMutants = mutants.map((mutant) => ({
@@ -75,4 +75,4 @@ const conditionalExpressionMutantPlacer: MutantPlacer = (path: NodePath, mutants
 };
 
 // Export it after initializing so `fn.name` is properly set
-export { conditionalExpressionMutantPlacer };
+export { expressionMutantPlacer };

packages/instrumenter/src/mutant-placers/index.ts

@@ -4,10 +4,10 @@ import { NodePath } from '@babel/core';
 import { Mutant } from '../mutant';
 
 import { MutantPlacer } from './mutant-placer';
-import { switchCaseMutantPlacer } from './switch-case-mutant-placer';
-import { conditionalExpressionMutantPlacer } from './conditional-expression-mutant-placer';
+import { statementMutantPlacer } from './statement-mutant-placer';
+import { expressionMutantPlacer } from './expression-mutant-placer';
 
-export const MUTANT_PLACERS = Object.freeze([conditionalExpressionMutantPlacer, switchCaseMutantPlacer]);
+export const MUTANT_PLACERS = Object.freeze([expressionMutantPlacer, statementMutantPlacer]);
 
 /**
  * Represents a mutant placer, tries to place a mutant in the AST with corresponding mutation switch and mutant covering expression

packages/instrumenter/src/mutant-placers/statement-mutant-placer.ts

@@ -0,0 +1,58 @@
+import { types } from '@babel/core';
+
+import { memberExpressionChain, createMutatedAst, mutationCoverageSequenceExpression, ID } from '../util/syntax-helpers';
+
+import { MutantPlacer } from './mutant-placer';
+
+/**
+ * Mutant placer that places mutants in statements that allow it.
+ * It uses an `if` statement to do so
+ */
+const statementMutantPlacer: MutantPlacer = (path, mutants) => {
+  if (path.isStatement()) {
+    // First transform the mutated ast before we start to apply mutants.
+    const appliedMutants = mutants.map((mutant) => ({
+      mutant,
+      ast: createMutatedAst(path, mutant),
+    }));
+
+    // path.replaceWith(
+    //   types.blockStatement([
+    //     types.switchStatement(memberExpressionChain(ID.GLOBAL, ID.ACTIVE_MUTANT), [
+    //       ...appliedMutants.map(({ ast, mutant }) => types.switchCase(types.numericLiteral(mutant.id), [ast, types.breakStatement()])),
+    //       types.switchCase(null, [
+    //         // Add mutation covering statement
+    //         types.expressionStatement(mutationCoverageSequenceExpression(mutants)),
+    //         path.node,
+    //         types.breakStatement(),
+    //       ]),
+    //     ]),
+    //   ])
+    // );
+
+    const instrumentedAst = appliedMutants.reduce(
+      // Add if statements per mutant
+      (prev: types.Statement, { ast, mutant }) =>
+        types.ifStatement(
+          types.binaryExpression('===', memberExpressionChain(ID.GLOBAL, ID.ACTIVE_MUTANT), types.numericLiteral(mutant.id)),
+          types.blockStatement([ast]),
+          prev
+        ),
+      path.isBlockStatement()
+        ? types.blockStatement([types.expressionStatement(mutationCoverageSequenceExpression(mutants)), ...path.node.body])
+        : types.blockStatement([types.expressionStatement(mutationCoverageSequenceExpression(mutants)), path.node])
+    );
+    if (path.isBlockStatement()) {
+      path.replaceWith(types.blockStatement([instrumentedAst]));
+    } else {
+      path.replaceWith(instrumentedAst);
+    }
+
+    return true;
+  } else {
+    return false;
+  }
+};
+
+// Export it after initializing so `fn.name` is properly set
+export { statementMutantPlacer };

packages/instrumenter/test/unit/mutant-placers/conditional-expression-mutant-placer.spec.ts → packages/instrumenter/test/unit/mutant-placers/expression-mutant-placer.spec.ts

@@ -3,20 +3,20 @@ import { types, NodePath } from '@babel/core';
 import { normalizeWhitespaces } from '@stryker-mutator/util';
 import generate from '@babel/generator';
 
-import { conditionalExpressionMutantPlacer } from '../../../src/mutant-placers/conditional-expression-mutant-placer';
+import { expressionMutantPlacer } from '../../../src/mutant-placers/expression-mutant-placer';
 import { findNodePath, parseJS } from '../../helpers/syntax-test-helpers';
 import { Mutant } from '../../../src/mutant';
 import { createMutant } from '../../helpers/factories';
 
-describe(conditionalExpressionMutantPlacer.name, () => {
+describe(expressionMutantPlacer.name, () => {
   it('should have the correct name', () => {
-    expect(conditionalExpressionMutantPlacer.name).eq('conditionalExpressionMutantPlacer');
+    expect(expressionMutantPlacer.name).eq('expressionMutantPlacer');
   });
 
   it('should not place when the parent is tagged template expression', () => {
     // A templateLiteral is considered an expression, while it is not save to place a mutant there!
     const templateLiteral = findNodePath(parseJS('html`<p></p>`'), (p) => p.isTemplateLiteral());
-    expect(conditionalExpressionMutantPlacer(templateLiteral, [])).false;
+    expect(expressionMutantPlacer(templateLiteral, [])).false;
   });
 
   function arrangeSingleMutant() {
@@ -37,7 +37,7 @@ describe(conditionalExpressionMutantPlacer.name, () => {
     const { binaryExpression, mutant, ast } = arrangeSingleMutant();
 
     // Act
-    const actual = conditionalExpressionMutantPlacer(binaryExpression, [mutant]);
+    const actual = expressionMutantPlacer(binaryExpression, [mutant]);
     const actualCode = normalizeWhitespaces(generate(ast).code);
 
     // Assert
@@ -47,15 +47,15 @@ describe(conditionalExpressionMutantPlacer.name, () => {
 
   it('should place the original code as the alternative', () => {
     const { binaryExpression, mutant, ast } = arrangeSingleMutant();
-    conditionalExpressionMutantPlacer(binaryExpression, [mutant]);
+    expressionMutantPlacer(binaryExpression, [mutant]);
     const actualAlternative = findNodePath<types.ConditionalExpression>(ast, (p) => p.isConditionalExpression()).node.alternate;
     const actualAlternativeCode = generate(actualAlternative).code;
     expect(actualAlternativeCode.endsWith('a + b'), `${actualAlternativeCode} did not end with "a + b"`).true;
   });
 
   it('should add mutant coverage syntax', () => {
     const { binaryExpression, mutant, ast } = arrangeSingleMutant();
-    conditionalExpressionMutantPlacer(binaryExpression, [mutant]);
+    expressionMutantPlacer(binaryExpression, [mutant]);
     const actualAlternative = findNodePath<types.ConditionalExpression>(ast, (p) => p.isConditionalExpression()).node.alternate;
     const actualAlternativeCode = generate(actualAlternative).code;
     const expected = '__global_69fa48.__coverMutant__(1), ';
@@ -80,7 +80,7 @@ describe(conditionalExpressionMutantPlacer.name, () => {
     ];
 
     // Act
-    conditionalExpressionMutantPlacer(binaryExpression, mutants);
+    expressionMutantPlacer(binaryExpression, mutants);
     const actualCode = normalizeWhitespaces(generate(ast).code);
 
     // Assert
@@ -91,13 +91,13 @@ describe(conditionalExpressionMutantPlacer.name, () => {
     it('should not place when the expression is a key', () => {
       // A stringLiteral is considered an expression, while it is not save to place a mutant there!
       const stringLiteral = findNodePath(parseJS("const foo = { 'foo': bar }"), (p) => p.isStringLiteral());
-      expect(conditionalExpressionMutantPlacer(stringLiteral, [])).false;
+      expect(expressionMutantPlacer(stringLiteral, [])).false;
     });
 
     it('should place when the expression is the value', () => {
       // A stringLiteral is considered an expression, while it is not save to place a mutant there!
       const stringLiteral = findNodePath(parseJS("const foo = { 'foo': bar }"), (p) => p.isIdentifier() && p.node.name === 'bar');
-      expect(conditionalExpressionMutantPlacer(stringLiteral, [])).true;
+      expect(expressionMutantPlacer(stringLiteral, [])).true;
     });
   });
 
@@ -116,7 +116,7 @@ describe(conditionalExpressionMutantPlacer.name, () => {
       ];
 
       // Act
-      conditionalExpressionMutantPlacer(expression, mutants);
+      expressionMutantPlacer(expression, mutants);
       const actualCode = normalizeWhitespaces(generate(ast).code);
 
       // Assert

packages/instrumenter/test/unit/mutant-placers/switch-case-mutant-placer.spec.ts → packages/instrumenter/test/unit/mutant-placers/statement-mutant-placer.spec.ts

@@ -3,13 +3,14 @@ import { types } from '@babel/core';
 import generate from '@babel/generator';
 import { normalizeWhitespaces } from '@stryker-mutator/util';
 
-import { switchCaseMutantPlacer } from '../../../src/mutant-placers/switch-case-mutant-placer';
+import { statementMutantPlacer } from '../../../src/mutant-placers/statement-mutant-placer';
 import { findNodePath, parseJS } from '../../helpers/syntax-test-helpers';
 import { Mutant } from '../../../src/mutant';
+import { createMutant } from '../../helpers/factories';
 
-describe(switchCaseMutantPlacer.name, () => {
+describe(statementMutantPlacer.name, () => {
   it('should have the correct name', () => {
-    expect(switchCaseMutantPlacer.name).eq('switchCaseMutantPlacer');
+    expect(statementMutantPlacer.name).eq('statementMutantPlacer');
   });
 
   it("shouldn't place mutants on anything but a statement", () => {
@@ -18,7 +19,7 @@ describe(switchCaseMutantPlacer.name, () => {
       findNodePath(parseJS('foo = bar'), (p) => p.isAssignmentExpression()),
       findNodePath(parseJS('foo.bar()'), (p) => p.isCallExpression()),
     ].forEach((node) => {
-      expect(switchCaseMutantPlacer(node, [])).false;
+      expect(statementMutantPlacer(node, [])).false;
     });
   });
 
@@ -41,33 +42,45 @@ describe(switchCaseMutantPlacer.name, () => {
     const { statement, mutant, ast } = arrangeSingleMutant();
 
     // Act
-    const actual = switchCaseMutantPlacer(statement, [mutant]);
+    const actual = statementMutantPlacer(statement, [mutant]);
     const actualCode = normalizeWhitespaces(generate(ast).code);
 
     // Assert
     expect(actual).true;
-    expect(actualCode).contains(
-      normalizeWhitespaces(`{
-      switch (__global_69fa48.__activeMutant__) {
-        case 1:
-          const foo = bar >>> baz;
-          break;
-          `)
-    );
+    expect(actualCode).contains(normalizeWhitespaces('if (__global_69fa48.__activeMutant__ === 1) { const foo = bar >>> baz; } else '));
+  });
+
+  it('should keep block statements in tact', () => {
+    // Arrange
+    const ast = parseJS('function add(a, b) { return a + b; }');
+    const statement = findNodePath(ast, (p) => p.isBlockStatement());
+    const originalNodePath = findNodePath<types.BinaryExpression>(ast, (p) => p.isBinaryExpression());
+    const mutant = createMutant({
+      original: originalNodePath.node,
+      replacement: types.binaryExpression('>>>', types.identifier('a'), types.identifier('b')),
+    });
+
+    // Act
+    const actual = statementMutantPlacer(statement, [mutant]);
+    const actualCode = normalizeWhitespaces(generate(ast).code);
+
+    // Assert
+    expect(actual).true;
+    expect(actualCode).matches(/function\s*add\s*\(a,\s*b\)\s*{.*}/);
   });
 
-  it('should place the original code as default case', () => {
+  it('should place the original code as alternative (inside `else`)', () => {
     const { ast, mutant, statement } = arrangeSingleMutant();
-    switchCaseMutantPlacer(statement, [mutant]);
+    statementMutantPlacer(statement, [mutant]);
     const actualCode = normalizeWhitespaces(generate(ast).code);
-    expect(actualCode).matches(/default:.*const foo = a \+ b;\s*break;/);
+    expect(actualCode).matches(/else\s*{.*const foo = a \+ b;\s*\}/);
   });
 
   it('should add mutant coverage syntax', () => {
     const { ast, mutant, statement } = arrangeSingleMutant();
-    switchCaseMutantPlacer(statement, [mutant]);
+    statementMutantPlacer(statement, [mutant]);
     const actualCode = normalizeWhitespaces(generate(ast).code);
-    expect(actualCode).matches(/default:\s*__global_69fa48\.__coverMutant__\(1\)/);
+    expect(actualCode).matches(/else\s*{\s*__global_69fa48\.__coverMutant__\(1\)/);
   });
 
   it('should be able to place multiple mutants', () => {
@@ -82,19 +95,17 @@ describe(switchCaseMutantPlacer.name, () => {
     ];
 
     // Act
-    switchCaseMutantPlacer(statement, mutants);
+    statementMutantPlacer(statement, mutants);
     const actualCode = normalizeWhitespaces(generate(ast).code);
 
     // Assert
     expect(actualCode).contains(
-      normalizeWhitespaces(`{
-        switch (__global_69fa48.__activeMutant__) {
-          case 52:
-            const foo = bar >>> baz;
-            break;
-          case 659:
-            const bar = a + b;
-            break;`)
+      normalizeWhitespaces(`if (__global_69fa48.__activeMutant__ === 659) {
+          const bar = a + b;
+        } else if (__global_69fa48.__activeMutant__ === 52) {
+          const foo = bar >>> baz;
+        } else {
+          __global_69fa48.__coverMutant__(52, 659)`)
     );
   });
 });