fix(mutator): don't mutate string literal object methods (#2718)

Don't mutate object method names that are disguised as string literals. For example: ```ts const foo = { "bar.baz"(qux) { } } ```
stryker-mutator · Jan 27, 2021 · 964537a · 964537a
1 parent f57c46d
commit 964537a
Show file tree

Hide file tree

Showing 6 changed files with 96 additions and 125 deletions.
diff --git a/packages/instrumenter/src/mutant-placers/index.ts b/packages/instrumenter/src/mutant-placers/index.ts
@@ -30,11 +30,17 @@ export function placeMutants(node: NodePath, mutants: Mutant[], fileName: string
       } catch (error) {
         const location = `${path.relative(process.cwd(), fileName)}:${node.node.loc?.start.line}:${node.node.loc?.start.column}`;
         const message = `${placer.name} could not place mutants with type(s): "${mutants.map((mutant) => mutant.mutatorName).join(', ')}"`;
-        throw node.buildCodeFrameError(
-          `${location} ${message}. Either remove this file from the list of files to be mutated, or ignore the mutators. Please report this issue at https://github.com/stryker-mutator/stryker/issues/new?assignees=&labels=%F0%9F%90%9B+Bug&template=bug_report.md&title=${encodeURIComponent(
-            message
-          )}.`
-        );
+        const errorMessage = `${location} ${message}. Either remove this file from the list of files to be mutated, or ignore the mutators. Please report this issue at https://github.com/stryker-mutator/stryker/issues/new?assignees=&labels=%F0%9F%90%9B+Bug&template=bug_report.md&title=${encodeURIComponent(
+          message
+        )}.`;
+        let builtError = new Error(errorMessage);
+        try {
+          // `buildCodeFrameError` is kind of flaky, see https://github.com/stryker-mutator/stryker/issues/2695
+          builtError = node.buildCodeFrameError(errorMessage);
+        } catch {
+          // Idle, regular error will have to suffice
+        }
+        throw builtError;
       }
     }
   }

diff --git a/packages/instrumenter/src/mutators/string-literal-mutator.ts b/packages/instrumenter/src/mutators/string-literal-mutator.ts
@@ -39,6 +39,7 @@ export class StringLiteralMutator implements NodeMutator {
       types.isJSXAttribute(parent) ||
       types.isExpressionStatement(parent) ||
       types.isTSLiteralType(parent) ||
+      types.isObjectMethod(parent) ||
       (types.isObjectProperty(parent) && parent.key === child.node) ||
       (types.isClassProperty(parent) && parent.key === child.node) ||
       (types.isCallExpression(parent) && types.isIdentifier(parent.callee, { name: 'require' }))

diff --git a/packages/instrumenter/test/unit/mutant-placers/index.spec.ts b/packages/instrumenter/test/unit/mutant-placers/index.spec.ts
@@ -1,5 +1,5 @@
 import sinon from 'sinon';
-import { NodePath } from '@babel/core';
+import { NodePath, parseSync, types } from '@babel/core';
 import { expect } from 'chai';
 
 import { placeMutants, MutantPlacer } from '../../../src/mutant-placers';
@@ -54,4 +54,27 @@ describe(placeMutants.name, () => {
       'foo.js:2:3 fooPlacer could not place mutants with type(s): "fooMutator". Either remove this file from the list of files to be mutated, or ignore the mutators. Please report this issue at https://github.com/stryker-mutator/stryker/issues/new'
     );
   });
+
+  /**
+   * Create a node path _without using the `new File` workaround_ defined here: https://github.com/babel/babel/issues/11889
+   * This will make sure `buildCodeFrameError` fails.
+   * This also happens in normal flows when complex babel transpilation is happening.
+   * @see https://github.com/stryker-mutator/stryker/issues/2695
+   */
+  it('should throw a generic error if `buildCodeFrameError` fails (#2695)', () => {
+    // Arrange
+    const path = findNodePath(parseSync('const a = b') as types.File, (p) => p.isProgram());
+    const expectedError = new Error('expectedError');
+    const fooPlacer: MutantPlacer = () => {
+      throw expectedError;
+    };
+    mutantPlacers[0].throws(expectedError);
+    const mutants = [createMutant()];
+
+    // Arrange & Act
+    expect(() => placeMutants(path, mutants, 'foo.js', [fooPlacer])).throws(
+      Error,
+      'foo.js:1:0 fooPlacer could not place mutants with type(s): "fooMutator". Either remove this file from the list of files to be mutated, or ignore the mutators. Please report this issue at https://github.com/stryker-mutator/stryker/issues/new'
+    );
+  });
 });
diff --git a/packages/instrumenter/test/unit/mutators/string-literal-mutator.spec.ts b/packages/instrumenter/test/unit/mutators/string-literal-mutator.spec.ts
@@ -90,6 +90,9 @@ describe(StringLiteralMutator.name, () => {
     it('should not mutate class property keys', () => {
       expectJSMutation(sut, 'class Foo { "baz-bar" = 4; }');
     });
+    it('should not mutate method names', () => {
+      expectJSMutation(sut, 'const watchers = {"category.type"(categoryType) { }}');
+    });
     it('should mutate class property values', () => {
       expectJSMutation(sut, 'class Foo { bar = "4"; }', 'class Foo { bar = ""; }');
     });

diff --git a/packages/instrumenter/testResources/instrumenter/vue-sample.vue b/packages/instrumenter/testResources/instrumenter/vue-sample.vue
@@ -4,80 +4,35 @@
     <h2>Essential Links</h2>
     <ul>
       <li>
-        <a
-          href="https://vuejs.org"
-          target="_blank"
-        >
-          Core Docs
-        </a>
+        <a href="https://vuejs.org" target="_blank"> Core Docs </a>
       </li>
       <li>
-        <a
-          href="https://forum.vuejs.org"
-          target="_blank"
-        >
-          Forum
-        </a>
+        <a href="https://forum.vuejs.org" target="_blank"> Forum </a>
       </li>
       <li>
-        <a
-          href="https://chat.vuejs.org"
-          target="_blank"
-        >
-          Community Chat
-        </a>
+        <a href="https://chat.vuejs.org" target="_blank"> Community Chat </a>
       </li>
       <li>
-        <a
-          href="https://twitter.com/vuejs"
-          target="_blank"
-        >
-          Twitter
-        </a>
+        <a href="https://twitter.com/vuejs" target="_blank"> Twitter </a>
       </li>
-      <br>
+      <br />
       <li>
-        <a
-          href="http://vuejs-templates.github.io/webpack/"
-          target="_blank"
-        >
-          Docs for This Template
-        </a>
+        <a href="http://vuejs-templates.github.io/webpack/" target="_blank"> Docs for This Template </a>
       </li>
     </ul>
     <h2>Ecosystem</h2>
     <ul>
       <li>
-        <a
-          href="http://router.vuejs.org/"
-          target="_blank"
-        >
-          vue-router
-        </a>
+        <a href="http://router.vuejs.org/" target="_blank"> vue-router </a>
       </li>
       <li>
-        <a
-          href="http://vuex.vuejs.org/"
-          target="_blank"
-        >
-          vuex
-        </a>
+        <a href="http://vuex.vuejs.org/" target="_blank"> vuex </a>
       </li>
       <li>
-        <a
-          href="http://vue-loader.vuejs.org/"
-          target="_blank"
-        >
-          vue-loader
-        </a>
+        <a href="http://vue-loader.vuejs.org/" target="_blank"> vue-loader </a>
       </li>
       <li>
-        <a
-          href="https://github.com/vuejs/awesome-vue"
-          target="_blank"
-        >
-          awesome-vue
-        </a>
+        <a href="https://github.com/vuejs/awesome-vue" target="_blank"> awesome-vue </a>
       </li>
     </ul>
   </div>
@@ -86,20 +41,28 @@
 <script>
 export default {
   name: 'HelloWorld',
-  data () {
+  data() {
     return {
-      msg: 'Welcome to Your Vue.js App'
-    }
-  }
-}
+      msg: 'Welcome to Your Vue.js App',
+    };
+  },
+  watch: {
+    'category.type'(categoryType) {
+      if (this.isNew && this.category) {
+        this.category.permissions = getDefaultDocumentCategoryPermissions(categoryType);
+      }
+    },
+  },
+};
 </script>
 <script>
 const b = a + c;
 </script>
 
 <!-- Add "scoped" attribute to limit CSS to this component only -->
 <style scoped>
-h1, h2 {
+h1,
+h2 {
   font-weight: normal;
 }
 ul {

diff --git a/packages/instrumenter/testResources/instrumenter/vue-sample.vue.out.snap b/packages/instrumenter/testResources/instrumenter/vue-sample.vue.out.snap
@@ -7,80 +7,35 @@ exports[`instrumenter integration should be able to instrument a vue sample 1`]
     <h2>Essential Links</h2>
     <ul>
       <li>
-        <a
-          href=\\"https://vuejs.org\\"
-          target=\\"_blank\\"
-        >
-          Core Docs
-        </a>
+        <a href=\\"https://vuejs.org\\" target=\\"_blank\\"> Core Docs </a>
       </li>
       <li>
-        <a
-          href=\\"https://forum.vuejs.org\\"
-          target=\\"_blank\\"
-        >
-          Forum
-        </a>
+        <a href=\\"https://forum.vuejs.org\\" target=\\"_blank\\"> Forum </a>
       </li>
       <li>
-        <a
-          href=\\"https://chat.vuejs.org\\"
-          target=\\"_blank\\"
-        >
-          Community Chat
-        </a>
+        <a href=\\"https://chat.vuejs.org\\" target=\\"_blank\\"> Community Chat </a>
       </li>
       <li>
-        <a
-          href=\\"https://twitter.com/vuejs\\"
-          target=\\"_blank\\"
-        >
-          Twitter
-        </a>
+        <a href=\\"https://twitter.com/vuejs\\" target=\\"_blank\\"> Twitter </a>
       </li>
-      <br>
+      <br />
       <li>
-        <a
-          href=\\"http://vuejs-templates.github.io/webpack/\\"
-          target=\\"_blank\\"
-        >
-          Docs for This Template
-        </a>
+        <a href=\\"http://vuejs-templates.github.io/webpack/\\" target=\\"_blank\\"> Docs for This Template </a>
       </li>
     </ul>
     <h2>Ecosystem</h2>
     <ul>
       <li>
-        <a
-          href=\\"http://router.vuejs.org/\\"
-          target=\\"_blank\\"
-        >
-          vue-router
-        </a>
+        <a href=\\"http://router.vuejs.org/\\" target=\\"_blank\\"> vue-router </a>
       </li>
       <li>
-        <a
-          href=\\"http://vuex.vuejs.org/\\"
-          target=\\"_blank\\"
-        >
-          vuex
-        </a>
+        <a href=\\"http://vuex.vuejs.org/\\" target=\\"_blank\\"> vuex </a>
       </li>
       <li>
-        <a
-          href=\\"http://vue-loader.vuejs.org/\\"
-          target=\\"_blank\\"
-        >
-          vue-loader
-        </a>
+        <a href=\\"http://vue-loader.vuejs.org/\\" target=\\"_blank\\"> vue-loader </a>
       </li>
       <li>
-        <a
-          href=\\"https://github.com/vuejs/awesome-vue\\"
-          target=\\"_blank\\"
-        >
-          awesome-vue
-        </a>
+        <a href=\\"https://github.com/vuejs/awesome-vue\\" target=\\"_blank\\"> awesome-vue </a>
       </li>
     </ul>
   </div>
@@ -153,8 +108,27 @@ export default stryMutAct_9fa48(0) ? {} : (stryCov_9fa48(0), {
         msg: stryMutAct_9fa48(4) ? \\"\\" : (stryCov_9fa48(4), 'Welcome to Your Vue.js App')
       });
     }
-  }
+  },
+
+  watch: stryMutAct_9fa48(5) ? {} : (stryCov_9fa48(5), {
+    'category.type'(categoryType) {
+      if (stryMutAct_9fa48(6)) {
+        {}
+      } else {
+        stryCov_9fa48(6);
+
+        if (stryMutAct_9fa48(9) ? this.isNew || this.category : stryMutAct_9fa48(8) ? false : stryMutAct_9fa48(7) ? true : (stryCov_9fa48(7, 8, 9), this.isNew && this.category)) {
+          if (stryMutAct_9fa48(10)) {
+            {}
+          } else {
+            stryCov_9fa48(10);
+            this.category.permissions = getDefaultDocumentCategoryPermissions(categoryType);
+          }
+        }
+      }
+    }
 
+  })
 });
 </script>
 <script>
@@ -212,12 +186,13 @@ function stryMutAct_9fa48(id) {
   return isActive(id);
 }
 
-const b = stryMutAct_9fa48(5) ? a - c : (stryCov_9fa48(5), a + c);
+const b = stryMutAct_9fa48(11) ? a - c : (stryCov_9fa48(11), a + c);
 </script>
 
 <!-- Add \\"scoped\\" attribute to limit CSS to this component only -->
 <style scoped>
-h1, h2 {
+h1,
+h2 {
   font-weight: normal;
 }
 ul {