tunnckoCore/parse-function

Bug with arguments default value consisting of a list of expressions

Closed this issue ยท 11 comments

Consider the following perfectly valid function:

function foo(a = (false, false, true)) {
    return a;
}

When called with no parameter, this function returns true.

Parsing it with parse-function...

const pf = require('parse-function');

let p = pf().parse(foo);

console.warn(p.defaults);

...return the following defaults:

{ a: 'false, false, true' }

The defaults should be:

{ a: 'true' }

Note that this syntax is used by istanbul (among others) to add instrumentation to function default parameters:

function foo(a = (cov_2dieaawgga.b[20][0]++, true)) {
    return a;
}

This bug makes adding code coverage impossible when parse-function is used to retrieve default values.

Not thoroughly tested plugin that work around the issue:

        let parser = parseFunction();
        parser.use((app: any) => {
            return (node: any, result: any) => {
                if (node.type === 'FunctionExpression') {
                    let params = node.params;

                    for (let param of params) {
                        if (param.type === 'AssignmentPattern') {
                            let right = param.right;

                            if (right.type === 'SequenceExpression') {
                                let value: any;
                                let lastExpression = right.expressions.pop();

                                if (lastExpression.type === 'NullLiteral') {
                                    value = null;
                                }
                                else {
                                    value = lastExpression.value;
                                }

                                result.defaults[param.left.name] = value;
                            }
                        }
                    }
                }

                return result;
            }
        });

But I think this should be dealt with by the official plugin.

Please use the syntax highlighting.

Thanks about the solution. And yea, it has basic support, intentionally.
That's the beauty of the smart plugins api ๐Ÿš€

Anyway, I'm not against to include it in core. If you want PR it and write some tests (the example from first comment is enough for start, imho).

Just create new file in src/lib/plugins directory and add it after this:

app.use(initial)

Can't remember what was the reason to not include the other plugins the same way, but try. If not works add it to src/lib/plugins/initial after result = body() plugin:

result = props(app)(node, result)
result = params(app)(node, result)
result = body(app)(node, result)

or try to add it as first plugin.

I'll take care of the PR.

Please use the syntax highlighting.

What do you mean? Is there more than just code formatting?

I'm talking about the comments here. like that

function foo(a = (false, false, true)) {
    return a;
}

Wow, how do you do that?

EDIT: Ok, found it. Thanks.

I'm trying to execute the test suite but I have an error:

$ npm test

> parse-function@0.0.0-semantic-release test /home/ericmorand/Projects/parse-function
> yarn start test

yarn run v1.3.2
$ hela test
ERR! Error: spawn /home/ericmorand/Projects/parse-function/node_modules/hela-preset-tunnckocore/node_modules/.bin/rollup ENOENT
    at exports._errnoException (util.js:1007:11)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:182:32)
    at onErrorNT (internal/child_process.js:348:16)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickCallback (internal/process/next_tick.js:98:9)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Do you have an idea? I switched to node@6.2.2 then node@8.9.4, removing node_modules then running npm i after each change and nothing worked.

Why would rollup be missing from /hela-preset-tunnckocore?

Hm, damn. Did you installed deps with yarn? Btw, the CONTRIBUTING guide is a bit outdated.

Why would rollup be missing from /hela-preset-tunnckocore?

Make sense, a bit. Can't describe it now :D

yarn install
yarn test

Works here.

Good catch! I used npm instead of yarn.

I could haveswore that yarn was just a faster and more secure CLI for npm but it seems it's more than that. Thanks.

Yea, it is amazing. But today i'm considering to switch finally to pnpm - it is amazing now. I used it for awhile when was released but had some problems to my workflow. And btw, still will have another problems, so that's why i'm not sure enough. But is even faster and greater than Yarn and Npm.