Skip to content

Commit

Permalink
fix(instrumenter): don't mutate constructor blocks with initialized c…
Browse files Browse the repository at this point in the history
…lass properties (#2484)

Don't mutate constructor block statements when the class contains initialized class properties. For example:

```ts
class Foo extends Baz {
  bar = 'bar';
  constructor() {  // Don't mutate this block!
     super();
  }
}
```

If you mutate the constructor block above, and TypeScript transpiles the code, it will call `this.bar` before the `super()` is initiated. Resulting in a runtime error.

Fixes #2474
  • Loading branch information
nicojs authored Sep 17, 2020
1 parent 4df4102 commit ca464a3
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 10 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/mutation-testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ jobs:
npm install
npm run build
- name: Run Stryker
run: npx lerna run --scope "@stryker-mutator/{api,instrumenter,util,typescript-checker,jest-runner}" --concurrency 1 --stream stryker -- -- --concurrency 3
run: npx lerna run --scope "@stryker-mutator/{api,instrumenter,util,typescript-checker,jest-runner,karma-runner}" --concurrency 1 --stream stryker -- -- --concurrency 3
env:
STRYKER_DASHBOARD_API_KEY: ${{ secrets.STRYKER_DASHBOARD_API_KEY }}
45 changes: 36 additions & 9 deletions packages/instrumenter/src/mutators/block-statement-mutator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,58 @@ export class BlockStatementMutator implements NodeMutator {
}

private isValid(path: NodePath<types.BlockStatement>) {
return !this.isEmpty(path) && !this.isConstructorBodyWithTSParameterPropertiesAndSuperCall(path);
return !this.isEmpty(path) && !this.isInvalidConstructorBody(path);
}

private isEmpty(path: NodePath<types.BlockStatement>) {
return !path.node.body.length;
}

/**
* Checks to see if a statement is the body of a constructor with TS parameter properties and a super call as it's first expression.
* Checks to see if a statement is an invalid constructor body
* @example
* // Invalid!
* class Foo extends Bar {
* constructor(public baz: string) {
* super(42);
* }
* }
* @example
* // Invalid!
* class Foo extends Bar {
* public baz = 'string';
* constructor() {
* super(42);
* }
* }
* @see https://github.com/stryker-mutator/stryker/issues/2314
* @see https://github.com/stryker-mutator/stryker/issues/2474
*/
private isConstructorBodyWithTSParameterPropertiesAndSuperCall(path: NodePath<types.BlockStatement>): boolean {
private isInvalidConstructorBody(blockStatement: NodePath<types.BlockStatement>): boolean {
return !!(
path.parentPath.isClassMethod() &&
path.parentPath.node.kind === 'constructor' &&
path.parentPath.node.params.some((param) => types.isTSParameterProperty(param)) &&
types.isExpressionStatement(path.node.body[0]) &&
types.isCallExpression(path.node.body[0].expression) &&
types.isSuper(path.node.body[0].expression.callee)
blockStatement.parentPath.isClassMethod() &&
blockStatement.parentPath.node.kind === 'constructor' &&
(this.containsTSParameterProperties(blockStatement.parentPath) || this.containsInitializedClassProperties(blockStatement.parentPath)) &&
this.hasSuperExpressionOnFirstLine(blockStatement)
);
}

private containsTSParameterProperties(constructor: NodePath<types.ClassMethod>): boolean {
return constructor.node.params.some((param) => types.isTSParameterProperty(param));
}

private containsInitializedClassProperties(constructor: NodePath<types.ClassMethod>): boolean {
return (
constructor.parentPath.isClassBody() &&
constructor.parentPath.node.body.some((classMember) => types.isClassProperty(classMember) && classMember.value)
);
}

private hasSuperExpressionOnFirstLine(constructor: NodePath<types.BlockStatement>): boolean {
return (
types.isExpressionStatement(constructor.node.body[0]) &&
types.isCallExpression(constructor.node.body[0].expression) &&
types.isSuper(constructor.node.body[0].expression.callee)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,12 @@ describe(BlockStatementMutator.name, () => {
it('should not mutate a constructor containing a super call and has (typescript) parameter properties', () => {
expectJSMutation(sut, 'class Foo extends Bar { constructor(private baz: string) { super(); } }');
});

/**
* @see https://github.com/stryker-mutator/stryker/issues/2474
*/
it('should not mutate a constructor containing a super call and contains initialized properties', () => {
expectJSMutation(sut, 'class Foo extends Bar { private baz = "qux"; constructor() { super(); } }');
});
});
});

0 comments on commit ca464a3

Please sign in to comment.