fix(instrumenter): don't mutate string literals in object properties (#…

…2354) Don't mutate string literals in object property keys. An example: ```js // don't mutate these! const { "aria-label": label } = props; const foo = { ["aria-label"]: label }; // However, still mutate strings in property values const bar = { baz: 'qux' } // mutated to { baz: '' } ```
stryker-mutator · Aug 5, 2020 · cd43952 · cd43952
1 parent 9e6e6e0
commit cd43952
Show file tree

Hide file tree

Showing 9 changed files with 105 additions and 74 deletions.
diff --git a/e2e/tasks/run-e2e-tests.ts b/e2e/tasks/run-e2e-tests.ts
@@ -24,7 +24,8 @@ const mutationSwitchingTempWhiteList = [
   'vue-cli-typescript-mocha',
   'jest-react-ts',
   'jest-node',
-  'jest-with-ts'
+  'jest-with-ts',
+  'jest-react',
 ]
 
 function runE2eTests() {

diff --git a/e2e/test/jest-react-ts/verify/verify.ts b/e2e/test/jest-react-ts/verify/verify.ts
@@ -1,6 +1,6 @@
 import { expectMetrics } from '../../../helpers';
 
-describe('After running stryker on jest-react project', () => {
+describe('After running stryker on jest-react-ts project', () => {
   it('should report expected scores', async () => {
     await expectMetrics({
       survived: 53,

diff --git a/e2e/test/jest-react/stryker.conf.js b/e2e/test/jest-react/stryker.conf.js
@@ -1,7 +1,7 @@
 module.exports = function (config) {
   config.set({
     testRunner: "jest",
-    reporters: ["event-recorder", "html", "progress"],
+    reporters: ["event-recorder", "html", "progress", "clear-text"],
     tempDirName: "stryker-tmp",
     coverageAnalysis: "off",
     timeoutMS: 60000, // High timeout to survive high build server load. Mutants created here should never timeout

diff --git a/e2e/test/jest-react/verify/verify.ts b/e2e/test/jest-react/verify/verify.ts
@@ -4,22 +4,22 @@ describe('After running stryker on jest-react project', () => {
   it('should report expected scores', async () => {
     await expectMetricsResult({
       metrics: produceMetrics({
-        killed: 34,
-        mutationScore: 64.15,
-        mutationScoreBasedOnCoveredCode: 64.15,
-        survived: 19,
-        totalCovered: 53,
-        totalDetected: 34,
-        totalMutants: 53,
-        totalUndetected: 19,
-        totalValid: 53
+        killed: 33,
+        timeout: 0,
+        mutationScore: 67.35,
+        mutationScoreBasedOnCoveredCode: 67.35,
+        survived: 16,
+        totalCovered: 49,
+        totalDetected: 33,
+        totalMutants: 49,
+        totalUndetected: 16,
+        totalValid: 49
       }),
     });
     /*
-    ---------------|---------|----------|-----------|------------|----------|---------|
-    File           | % score | # killed | # timeout | # survived | # no cov | # error |
-    ---------------|---------|----------|-----------|------------|----------|---------|
-    All files      |   64.15 |       34 |         0 |         19 |        0 |       0 |
-    ---------------|---------|----------|-----------|------------|----------|---------|*/
+      ---------------|---------|----------|-----------|------------|----------|---------|
+      File           | % score | # killed | # timeout | # survived | # no cov | # error |
+      ---------------|---------|----------|-----------|------------|----------|---------|
+      All files      |   67.35 |       33 |         0 |         16 |        0 |       0 |*/
   });
 });
diff --git a/e2e/test/mocha-ts-node/package.json b/e2e/test/mocha-ts-node/package.json
@@ -3,6 +3,6 @@
   "description": "A test for mocha using ts-node (no transpiler) using mocha package config",
   "scripts": {
     "test": "stryker run",
-    "posttest": "mocha --no-package --require \"ts-node/register\" verify/verify.ts"
+    "posttest": "mocha --no-package --no-config --require \"ts-node/register\" verify/verify.ts"
   }
 }
diff --git a/e2e/test/vue-cli-typescript-mocha/verify/verify.ts b/e2e/test/vue-cli-typescript-mocha/verify/verify.ts
@@ -6,11 +6,11 @@ describe('Verify stryker has ran correctly', () => {
     await expectMetrics({
       killed: 2,
       survived: 1,
-      noCoverage: 11,
+      noCoverage: 10,
       compileErrors: 3,
       runtimeErrors: 0,
       timeout: 0,
-      mutationScore: 14.29
+      mutationScore: 15.38
     });
   });
 });
diff --git a/packages/instrumenter/src/mutators/string-literal-mutator.ts b/packages/instrumenter/src/mutators/string-literal-mutator.ts
@@ -24,7 +24,7 @@ export class StringLiteralMutator implements NodeMutator {
           },
         ];
       }
-    } else if (path.isStringLiteral() && this.isValidParent(path.parent)) {
+    } else if (path.isStringLiteral() && this.isValidParent(path)) {
       return [
         {
           original: path.node,
@@ -36,7 +36,8 @@ export class StringLiteralMutator implements NodeMutator {
     }
   }
 
-  private isValidParent(parent?: types.Node): boolean {
+  private isValidParent(child: NodePath<types.StringLiteral>): boolean {
+    const parent = child.parent;
     return !(
       types.isImportDeclaration(parent) ||
       types.isExportDeclaration(parent) ||
@@ -45,6 +46,7 @@ export class StringLiteralMutator implements NodeMutator {
       types.isJSXAttribute(parent) ||
       types.isExpressionStatement(parent) ||
       types.isTSLiteralType(parent) ||
+      (types.isObjectProperty(parent) && parent.key === child.node) ||
       (types.isCallExpression(parent) && types.isIdentifier(parent.callee, { name: 'require' }))
     );
   }

diff --git a/packages/instrumenter/src/transformers/babel-transformer.ts b/packages/instrumenter/src/transformers/babel-transformer.ts
@@ -1,4 +1,7 @@
-import traverse from '@babel/traverse';
+import { traverse } from '@babel/core';
+
+// @ts-expect-error The babel types don't define "File" yet
+import { File } from '@babel/core';
 
 import { placeMutant } from '../mutant-placers';
 import { mutate } from '../mutators';
@@ -7,8 +10,11 @@ import { AstFormat } from '../syntax';
 
 import { AstTransformer } from '.';
 
-export const transformBabel: AstTransformer<AstFormat.JS | AstFormat.TS> = ({ root, originFileName }, mutantCollector) => {
-  traverse(root, {
+export const transformBabel: AstTransformer<AstFormat.JS | AstFormat.TS> = ({ root, originFileName, rawContent }, mutantCollector) => {
+  // Wrap the AST in a file, so `nodePath.buildError` works
+  // https://github.com/babel/babel/issues/11889
+  const file = new File({ filename: originFileName }, { code: rawContent, ast: root });
+  traverse(file.ast, {
     enter(path) {
       if (isTypeAnnotation(path) || isImportDeclaration(path) || path.isDecorator()) {
         // Don't mutate type declarations or import statements

diff --git a/packages/instrumenter/test/unit/mutators/string-literal-mutator.spec.ts b/packages/instrumenter/test/unit/mutators/string-literal-mutator.spec.ts
@@ -13,63 +13,85 @@ describe(StringLiteralMutator.name, () => {
     expect(sut.name).eq('StringLiteral');
   });
 
-  it('should mutate a string literal with double quotes', () => {
-    expectJSMutation(sut, 'const b = "Hello world!";', 'const b = "";');
+  describe('string literals', () => {
+    it('should mutate a string literal with double quotes', () => {
+      expectJSMutation(sut, 'const b = "Hello world!";', 'const b = "";');
+    });
+
+    it('should mutate a string literal with single quotes', () => {
+      expectJSMutation(sut, "const b = 'Hello world!';", 'const b = "";');
+    });
+
+    it('should mutate a template string', () => {
+      expectJSMutation(sut, 'const b = `Hello world!`;', 'const b = ``;');
+    });
+
+    it('should mutate a template string referencing another variable', () => {
+      expectJSMutation(sut, 'const a = 10; const b = `${a} mutations`;', 'const a = 10; const b = ``;');
+      expectJSMutation(sut, 'const a = 10; const b = `mutations: ${a}`;', 'const a = 10; const b = ``;');
+      expectJSMutation(sut, 'const a = 10; const b = `mutations: ${a} out of 10`;', 'const a = 10; const b = ``;');
+    });
+
+    it('should mutate empty strings', () => {
+      expectJSMutation(sut, 'const b = "";', 'const b = "Stryker was here!";');
+    });
+
+    it('should mutate empty template strings', () => {
+      expectJSMutation(sut, 'const b = ``;', 'const b = `Stryker was here!`;');
+    });
+
+    it('should not mutate directive prologues', () => {
+      expectJSMutation(sut, '"use strict";"use asm";');
+      expectJSMutation(sut, 'function a() {"use strict";"use asm";}');
+    });
   });
 
-  it('should mutate a string literal with single quotes', () => {
-    expectJSMutation(sut, "const b = 'Hello world!';", 'const b = "";');
+  describe('imports/exports', () => {
+    it('should not mutate import statements', () => {
+      expectJSMutation(sut, 'import * as foo from "foo";');
+      expectJSMutation(sut, 'import { foo } from "foo";');
+      expectJSMutation(sut, 'import foo from "foo";');
+      expectJSMutation(sut, 'import "foo";');
+    });
+
+    it('should not mutate require call statements', () => {
+      expectJSMutation(sut, 'require("./lib/square");');
+    });
+
+    it('should mutate other call statements', () => {
+      expectJSMutation(sut, 'require2("./lib/square");', 'require2("");');
+    });
+
+    it('should not mutate export statements', () => {
+      expectJSMutation(sut, 'export * from "./foo";');
+      expectJSMutation(sut, 'export { foo as boo } from "./foo";');
+    });
   });
 
-  it('should mutate a template string', () => {
-    expectJSMutation(sut, 'const b = `Hello world!`;', 'const b = ``;');
-  });
-
-  it('should mutate a template string referencing another variable', () => {
-    expectJSMutation(sut, 'const a = 10; const b = `${a} mutations`;', 'const a = 10; const b = ``;');
-    expectJSMutation(sut, 'const a = 10; const b = `mutations: ${a}`;', 'const a = 10; const b = ``;');
-    expectJSMutation(sut, 'const a = 10; const b = `mutations: ${a} out of 10`;', 'const a = 10; const b = ``;');
-  });
-
-  it('should mutate empty strings', () => {
-    expectJSMutation(sut, 'const b = "";', 'const b = "Stryker was here!";');
-  });
-
-  it('should mutate empty template strings', () => {
-    expectJSMutation(sut, 'const b = ``;', 'const b = `Stryker was here!`;');
-  });
-
-  it('should not mutate import statements', () => {
-    expectJSMutation(sut, 'import * as foo from "foo";');
-    expectJSMutation(sut, 'import { foo } from "foo";');
-    expectJSMutation(sut, 'import foo from "foo";');
-    expectJSMutation(sut, 'import "foo";');
-  });
-
-  it('should not mutate require call statements', () => {
-    expectJSMutation(sut, 'require("./lib/square");');
-  });
-
-  it('should mutate other call statements', () => {
-    expectJSMutation(sut, 'require2("./lib/square");', 'require2("");');
-  });
-
-  it('should not mutate export statements', () => {
-    expectJSMutation(sut, 'export * from "./foo";');
-    expectJSMutation(sut, 'export { foo as boo } from "./foo";');
-  });
+  describe('type declarations', () => {
+    it('should not mutate type declarations', () => {
+      expectJSMutation(sut, 'const a: "hello" = "hello";', 'const a: "hello" = "";');
+      expectJSMutation(sut, 'const a: Record<"id", number> = { id: 10 }');
+    });
 
-  it('should not mutate type declarations', () => {
-    expectJSMutation(sut, 'const a: "hello" = "hello";', 'const a: "hello" = "";');
-    expectJSMutation(sut, 'const a: Record<"id", number> = { id: 10 }');
+    // interfaces itself are skipped entirely by the babel-transformer
   });
 
-  it('should not mutate string JSX attributes', () => {
-    expectJSMutation(sut, '<Record class="row" />');
+  describe('object properties', () => {
+    it('should not mutate inside object property keys', () => {
+      expectJSMutation(sut, 'const { className, "aria-label": label } = props;');
+    });
+    it('should not mutate inside object property keys', () => {
+      expectJSMutation(sut, 'const foo = { className, ["aria-label"]: label };');
+    });
+    it('should still mutate inside object property values', () => {
+      expectJSMutation(sut, 'const foo = { bar: "baz" };', 'const foo = { bar: "" };');
+    });
   });
 
-  it('should not mutate directive prologues', () => {
-    expectJSMutation(sut, '"use strict";"use asm";');
-    expectJSMutation(sut, 'function a() {"use strict";"use asm";}');
+  describe('jsx', () => {
+    it('should not mutate string JSX attributes', () => {
+      expectJSMutation(sut, '<Record class="row" />');
+    });
   });
 });