mockdeep / typewiz

Automatically discover and add missing types in your TypeScript code
https://medium.com/@urish/manual-typing-is-no-fun-introducing-typewiz-58e3e8813f4c
1.1k stars 46 forks source link

Fails typescript parsing when having nested arrow functions #75

Closed Nysosis closed 5 years ago

Nysosis commented 6 years ago

Versions:

This is a stripped down version of some code I have that causes the error

// ~/src/app.ts
async function Main () {
    await Promise.resolve([
        1, 2, 3
    ]).then((results) => {
        if (results.length > 0) {
            results.forEach((result) => console.log(result));
            process.exit(1);
        }
    });
}

Main();

However I've managed to strip it back even further (to eliminate it being something related to async) to the below. If I remove the results.forEach… and instead just do console.log(results) then typewiz works fine. So I think it due to having the nested arrow statements somehow?

// ~/src/app.ts
function doTheThing(cb) {
    cb([1,2,3]);
}

doTheThing((results) => {
    results.forEach((result) => console.log(result));
});

On running: typewiz-node src/app.ts

Error:

[PROJECT_ROOT]\node_modules\typescript\lib\typescript.js:8984
        return ts.skipTrivia((sourceFile || getSourceFileOfNode(node)).text, node.pos);
                                                                       ^
TypeError: Cannot read property 'text' of undefined
    at Object.getTokenPosOfNode ([PROJECT_ROOT]\node_modules\typescript\lib\typescript.js:8984:72)
    at NodeObject.getStart ([PROJECT_ROOT]\node_modules\typescript\lib\typescript.js:106669:23)
    at hasParensAroundArguments ([PROJECT_ROOT]\node_modules\typewiz-core\src\transformer.ts:67:13)
    at visitor ([PROJECT_ROOT]\node_modules\typewiz-core\src\transformer.ts:169:21)
    at visitNodes ([PROJECT_ROOT]\node_modules\typescript\lib\typescript.js:56887:48)
    at Object.visitEachChild ([PROJECT_ROOT]\node_modules\typescript\lib\typescript.js:57052:156)
    at visitor ([PROJECT_ROOT]\node_modules\typewiz-core\src\transformer.ts:143:10)
    at visitNode ([PROJECT_ROOT]\node_modules\typescript\lib\typescript.js:56836:23)
    at Object.visitEachChild ([PROJECT_ROOT]\node_modules\typescript\lib\typescript.js:57106:49)
    at visitor ([PROJECT_ROOT]\node_modules\typewiz-core\src\transformer.ts:143:10)
urish commented 6 years ago

Thanks!

It reproduces with the following integration test case:

    it('issue #75', () => {
        const input = `
            function doTheThing(cb) {
                cb([1,2,3]);
            }

            doTheThing((results) => {
                results.forEach((result) => console.log(result));
            });
        `;

        expect(typeWiz(input)).toBe(`
            function doTheThing(cb) {
                cb([1,2,3]);
            }

            doTheThing((results: number[]) => {
                results.forEach((result: number) => console.log(result));
            });
        `);
    });

I will try to come up with a fix

devdoomari3 commented 5 years ago

@urish I've stumbled on this error while using typewiz-webpack.

Are you working on this? if not, can I have a go?

alecf commented 5 years ago

Any thoughts here? I dug around in the source and it seems like somehow the parameters themselves are not getting annotated. The simplest test case I could come up with: (in integration.spec.ts)

    it('should correctly handle nested arrow functions', () => {
        const input = `(x=>y=>x+y+5)(10)(7)`;
        expect(typeWiz(input)).toBe(`(x: number)=>(y: number) => x+y+5)(10)(7)`);
    });

(the annotated result may be incorrect, it was just a guess)

Here's an odd bit - if I just make hasParensAroundArguments return false (just as a hack to see if I can get this to work) when node.getSourceFile() isn't present:

    if (ts.isArrowFunction(node)) {
        if (!node.getSourceFile()) {
            return false;
        }

then I get this interesting test failure:

    evalmachine.<anonymous>:135
    (function (x) { $_$twiz("x", x, 2, "c:/test.ts", "{\"arrow\":true,\"parens\":[1,2]}"); (function (y) { $_$twiz("y", y, 5, "c:/test.ts", "{\"arrow\":true,\"parens\":[4,5]}"); x + y + 5; }); })(10)(7);
                                                                                                                                                                                                       ^

    TypeError: (intermediate value)(intermediate value)(...) is not a function

pretty-printed, this evals to:

(function(x) {
  $_$twiz("x", x, 2, "c:/test.ts", '{"arrow":true,"parens":[1,2]}');
  (function(y) {
    $_$twiz("y", y, 5, "c:/test.ts", '{"arrow":true,"parens":[4,5]}');
    x + y + 5;
  });
})(10)(7);

which seems wrong - it isn't applying the result of the first function to the second..

urish commented 5 years ago

Thanks for looking into this @alecf . It seems like your test case uncovered another issue, where arrow functions with a concise body (without curly braces, e.g. x => 5) were not instrumented correctly. I have opened another issue for this, #82 and will look into it shortly.

urish commented 5 years ago

Released this fix as part of typewiz 1.2.1