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

support ES6 modules (and support custom compilers/parsers e.g. babel/espree) #133

Closed
willheslam opened this issue Jul 27, 2016 · 11 comments
Closed
Labels
🚀 Feature request New feature request

Comments

@willheslam
Copy link

willheslam commented Jul 27, 2016

Firstly - I love this project! This is going to be hugely useful.

The problem:
Esprima will reject 'import' statements if the sourceType is not explicitly set to 'module':

            case 'import':
                if (state.sourceType !== 'module') {
                    tolerateUnexpectedToken(lookahead, Messages.IllegalImportDeclaration);
                }
                return parseImportDeclaration();

from https://github.com/jquery/esprima/blob/master/src/parser.ts#L1643

This requires Stryker to provide it with sourceType: 'module' as seen here:

var esprimaOptions = {
    comment: true,
    loc: true,
    range: true,
    tokens: true,
    sourceType: 'module' //<=== required for esprima
};

from https://github.com/stryker-mutator/stryker/blob/master/src/utils/parserUtils.ts#L9

Prototype pull req to illustrate:
#136

Unfortunately, currently there's no way to tell Stryker this is what you want.

This is the code I am testing:

export default (a, b) => a < b

with this test:

import lessThan from './lessThan.js'
describe('a < b', function(){
  it('it should be true', function(){
    chai.expect(lessThan(1, 2)).to.be.equal(true)
  })
})

and Stryker cleverly realises that

Mutator: ConditionalBoundary
-   export default ((a, b) => a < b);
+   export default ((a, b) => a <= b);

However there's more - because I'm using ES6 module syntax in my actual mocha tests too, I need to use babel-core/register and some transforms to compile them as they're required. This works totally fine with Stryker too - I just make the first file it looks at one with babel-core/register, where I have one or two babel plugins beavering away magically transpiling stuff.

require('babel-core/register')({
  "presets": ["es2015"]
})

This should be the end of it - but I'm also a big fan of the new object rest spread syntax:
https://github.com/sebmarkbage/ecmascript-rest-spread
(https://babeljs.io/docs/plugins/syntax-object-rest-spread/)

I want to use it in the code I'm mutating (not just the tests) in this contrived example:

export default (a, b) => ({ ...{ c: 82, d: 150 } }, a < b)

so I modify my babel-core register hook to use the transform:

require('babel-core/register')({
  "presets": ["es2015"],
  "plugins": [
    "transform-object-rest-spread"
  ]
})

But it doesn't work - because Stryker loads the files it is going to mutate into Esprima, it doesn't require them directly so Babel can't transpile it.

To fix this, I need to go into:
https://github.com/stryker-mutator/stryker/blob/master/src/MutatorOrchestrator.ts#L45
and add babel to transform the file:

                var fileContent = fileUtils.readFile(sourceFile);
                fileContent = babel.transform(fileContent, { plugins: ["transform-object-rest-spread", "transform-runtime"]}).code;
                _this.reportFileRead(sourceFile, fileContent);

(I tried doing this later (just before Esprima parses it) but it caused weird issues where Stryker wouldn't generate any mutations, or claim that all mutations passed - I think probably because it meant reportFileRead and other functions had differing opinions about the state of the source code.)

We need both these transforms because with just the first, it adds a lot of extra code into the file which gets Stryker over excited:

Mutant survived!
Mutator: UnaryOperator
-   var _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; };
+   var _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i--) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; };

Tests ran:
    a < b it should be true

14 mutants tested.
0 mutants untested.

and it generates over 14 mutants, all mostly meaningless variations on Babel's polyfill.
"transform-runtime" extracts the polyfill out into a separate file so Stryker only sees a single function call and isn't tempted to mutate anything.

With Babel:

Mutator: ConditionalBoundary
-   export default ((a, b) => (_extends({ c: 82, d: 150 }), a < b));
+   export default ((a, b) => (_extends({ c: 82, d: 150 }), a <= b));

Tests ran:
    a < b it should be true

2 mutants tested.
0 mutants untested.
0 mutants timed out.
1 mutants killed.
Mutation score based on covered code: 50.00%
Mutation score based on all code: 50.00%

That's more like it!

Prototype pull req to illustrate: #134

There is an alternative to using Babel, which is to use Espree instead of Esprima:
https://github.com/eslint/espree
Espree is based on Esprima but has support for more experimental features (among them object rest spread).

It was a pretty simple swap - in https://github.com/stryker-mutator/stryker/blob/master/src/utils/parserUtils.ts#L30
I just swap out esprima for espree and add

    ecmaFeatures: {
      experimentalObjectRestSpread: true
    }

as an option and it just works.

With Espree:

Mutator: ConditionalBoundary
-   export default (a, b) => ({ ...{ c: 82, d: 150 } }, a < b)
+   export default (a, b) => ({ ...{ c: 82, d: 150 } }, a <= b)

Tests ran:
    a < b it should be true

2 mutants tested.
0 mutants untested.
0 mutants timed out.
1 mutants killed.
Mutation score based on covered code: 50.00%
Mutation score based on all code: 50.00%

Prototype pull req to illustrate: #135

As you can see, Espree is not transforming the code, just correctly parsing it, so the mutation snippet is a bit truer to life.
Babel is a lot more powerful in the sense that you can transform pretty much anything you like into anything else, but you'd need Stryker to understand sourcemaps in order to provide perfect feedback, and you'd need to make sure any Babel transform left only the minimum amount of 'mutative noise' left over (or otherwise wrap any mutative noise in comments that Stryker could ignore - sounds messy).

So there are several options here - hacking it in like in my pull reqs is not ideal, so perhaps there needs to be a way to expose a callback to the config whenever a file is read or parsed so, at the very least, people can provide their own compilers.
Alternatively or alongside which, should it be possible to provide compilers as Stryker plugins?

What do you guys think? Is this kind of configurability on the roadmap, and if so do you mind if I take a crack at it?

Apologies - I didn't use the correct commit style (https://github.com/stryker-mutator/stryker/blob/master/CONTRIBUTING.md) on the pull reqs - they're just prototypes to illustrate the issue, I don't expect they'll be merged!

Thanks,
Will

@simondel
Copy link
Member

Hey Will,

Thank you very much for the extensive description of your issue! As you have noticed, compilers are not supported in Stryker right now but we may have to do something about that in the future because I expect that you won't be the last person who wants support for a language feature we don't support by default.

As of right now I have a few questions:

  1. Won't [WIP]: tell esprima to assume the file is an es6 module #136 break support for script as we force Esprima to use modules?
  2. If we want to support your feature we would have to merge either [WIP]: use babel compiler immediately after reading file #134 or [WIP]: use espree instead of esprima #135 and not both, right?
  3. How would you like to see custom compiler support?
  4. Does escodegen provide support for espree (or vice-versa) because the online escodegen demo is already having trouble with basic es6 features

@nicojs
Copy link
Member

nicojs commented Jul 27, 2016

Thanks a lot for this wealth of information! I've been thinking of supporting transpilers for a while now. As you can see, Stryker uses TypeScript, so it would be awesome to support this (and all transpilers) more natively.

To expand on @simondel 's comments:

  1. Won't [WIP]: tell esprima to assume the file is an es6 module #136 break support for script as we force Esprima to use modules
    We could make it an option: supportES6Modules. Maybe give it the default value true if it doesn't break existing code (to be tested).

  2. Custom compiler support

I think we might have an other option here. We could leave the transpiling/compiling up to your build system of choice. We would instead support sourceMap files. We would interpret the sourceMap and only if a line of code is represented in the source code we would let it be mutated. The problem is that we might miss mutants here or there if we only mutate the transpiled code. Would this be (1) feasible and (2) desirable?

What do you guys think? Is this kind of configurability on the roadmap, and if so do you mind if I take a crack at it? - @willheslam

Yes it is and you can have a crack at it :)

  1. Escodegen support
    I agree that we should of course make sure that whatever we add to the Abstract Syntax Tree should be supported by escodegen or an escodegen like tool. @willheslam I don't know babel enough, do you think we could swap out escode gen for babel (or something else)?

@jeznag
Copy link

jeznag commented Feb 12, 2017

Any progress on this one? The config option for esPrima sounds like a quick win. Shall I do that?

@nicojs
Copy link
Member

nicojs commented Mar 19, 2017

@jeznag give it a shot 👍

@SxMShaDoW
Copy link

SxMShaDoW commented Apr 1, 2017

👍 would love this functionality. using create-react-app with jest built in. would love to use this out of the box by adding a simple .conf.js and running stryker. (no preference on compiled code or not for a v1)

@simondel simondel added the 🚀 Feature request New feature request label Apr 15, 2017
@CrazyBS
Copy link

CrazyBS commented Apr 27, 2017

I am extra excited to see this working in a babel/webpack pipeline. I am using es6 and react plugins in babel and have no luck getting version 0.6.0 working with the mutations turned on. It does run the dry-tests successfully...

I recognize that there is significant challenges to fully parsing and mutating the raw source code no matter what stage-? features the user is adding into their babel plugins. It would be great if this plugin didn't care at all and just registered to the output of babel and mutated the compiled code with coverage to help guide it (is that a thing?).

You have your work cut out for you and I if this feature is successful, it will be the standard include in every client project I do. Keep up the good work!

@FezVrasta
Copy link

Is there any progress on this?

My use case is basically create-react-app

@simondel
Copy link
Member

simondel commented Jul 3, 2017

@FezVrasta do you mean an app created using create-react-app? If I understand correctly it uses babel and webpack, right? Babel support shouldn't be that hard after we support ES6 because we mutate the code using Babel (that way we always support new JavaScript features). However, we won't support all the Babel plugins, configuration and code transpilation. That will be developed in a separate plugin which could use the ES6 plugin, making it easier to develop.

Webpack support will probably be a lot harder as you will have to bundle the files before testing and support a lot of different webpack configurations. This will probably require us to alter the process of Stryker. So this will take some more time unfortunately.

@FezVrasta
Copy link

FezVrasta commented Jul 3, 2017

Yes I mean an app created with create-react-app (and later, ejected actually).

I would need support for es2015, stage2, flow and react presets.

Why would someone need webpack support? The tests aren't usually ran with webpack, but with Jest or Karma or similar.

@CrazyBS
Copy link

CrazyBS commented Jul 3, 2017

@FezVrasta It's pretty common to have webpack handle the dependency loading and in your karma configuration.

@simondel Hard news to swallow. I was really looking forward to adding stryker into my webpack/babel/karma stack. It sounds like there will be some basic lifecycle problems to solve to support the modern javascript stack. Potentially a webpack plugin or module of some type to couple stryker mutation in with the compilation process. However, if Styker doesn't really care what karma does to execute it's tests and if stryker can handle the source code in it's un-babeled form and feed those results back through whatever karma/webpack/babel pipeline is in place and report the results of the tests back to stryker, then we may be in business anyway. Perhaps a hook to supply Stryker's babel configuration additional options?

devonzuegel added a commit to devonzuegel/clarity that referenced this issue Aug 6, 2017
- Typescript is not currently supported
- Open PRs in the project indicate that it may be supported soon:
    stryker-mutator/stryker-js#133
    stryker-mutator/stryker-js#258
@simondel simondel mentioned this issue Dec 5, 2017
5 tasks
@simondel
Copy link
Member

With stryker-javascript-mutator and stryker-babel-transpiler we have support for ES6 modules. React support will be added with #525.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 Feature request New feature request
Projects
None yet
Development

No branches or pull requests

7 participants