Illegal return statements

[Itay Neeman]

I wanted to put this in the mailing list first because I'm not sure this is a bug. In Node, the following file works fine:

if (1) {

    console.log("a");

    return;

}

However, if I give it to esprima, with the latest checkout from today, I get:

Illegal return statement

    at throwError (/Users/ineeman/work/esprima/esprima.js:1170:21)

    at parseReturnStatement (/Users/ineeman/work/esprima/esprima.js:2484:13)

    at parseStatement (/Users/ineeman/work/esprima/esprima.js:2752:24)

    at parseSourceElement (/Users/ineeman/work/esprima/esprima.js:3025:24)

    at parseStatementList (/Users/ineeman/work/esprima/esprima.js:2050:25)

    at parseBlock (/Users/ineeman/work/esprima/esprima.js:2065:17)

    at parseStatement (/Users/ineeman/work/esprima/esprima.js:2727:24)

    at parseIfStatement (/Users/ineeman/work/esprima/esprima.js:2200:22)

    at parseStatement (/Users/ineeman/work/esprima/esprima.js:2750:24)

    at parseSourceElement (/Users/ineeman/work/esprima/esprima.js:3025:24)

This worked with an older version of Esprima (0.9.8), but seems to not work anymore. There are node packages that use this - could we make this into an option or something, so that we can be lax?

Itay

[David Rajchenbach-Teller]

It counts as a syntax error in SpiderMonkey, fwiw.

Cheers,
David

[Ariya Hidayat]

> I wanted to put this in the mailing list first because I'm not sure this is
> a bug. In Node, the following file works fine:
>
> if (1) {
>     console.log("a");
>     return;
> }
>
> However, if I give it to esprima, with the latest checkout from today, I
> get:
> Illegal return statement

That's the correct behavior per ECMAScript specification.

It works on Node.js because Node.js wraps everything under a function
(see the wrapper version with node inspector).

--
Ariya Hidayat, http://ariya.ofilabs.com

[Kris Kowal]

On Fri, Mar 16, 2012 at 10:30 AM, Ariya Hidayat <ariya....@gmail.com> wrote:
> It works on Node.js because Node.js wraps everything under a function
> (see the wrapper version with node inspector).

Right. For this reason, it would be very useful to expose a parse
function that begins with a function body production.

Kris Kowal

[Itay Neeman]

That is unfortunate - it puts Cover in a pickle. If I wrap it, then I throw off the line/offset information. If I don't wrap it, esprima will fail the parse step.

Any suggestions welcome.

[Kris Kowal]

It is possible to wrap without altering line numbers.

"(function (require, exports, module){" + text + "\n}"

Kris Kowal

[Itay Neeman]

This will still alter intra-line offset numbers. One of the good features of Cover is that it does intra-line coverage. For example, if you have 'if (true || a()) ...`) it will tell you that 'a()' isn't covered.

I could offset each line by the "wrapper" amount, but then it just becomes a pain. It would be good to be able to put esprima in "node.js" mode where it assumes we are executing within a function context.

[Itay Neeman]

For those curious, here is the change in Cover to handle this:

https://github.com/itay/node-cover/commit/888850a7a9ed832cffc63b45de864ea11ed8165d

Also bumped it to 0.2.4 on npm to handle this case.

[Ariya Hidayat]

> I could offset each line by the "wrapper" amount, but then it just becomes a
> pain. It would be good to be able to put esprima in "node.js" mode where it
> assumes we are executing within a function context.

http://code.google.com/p/esprima/issues/detail?id=226