espruino / Espruino

The Espruino JavaScript interpreter - Official Repo
http://www.espruino.com/
Other
2.73k stars 741 forks source link

Fix parsing of multiple expressions without newlines/semis #2489

Closed bobrippling closed 2 months ago

bobrippling commented 2 months ago

This fixes parsing such as:

>a=[1,2]
=[ 1, 2 ]
>b = a[1]0          // <----------------
=0
>b = a[1]5          // <----------------
=5

>f=()=>{}
=function () {}
>b = f()5          // <----------------
=5
>b = f()5*2          // <----------------
=10

>f=()=>console.log("hi")
=function () { ... }
>b = f()5*2          // <----------------
hi
=10
>b = f()5*2 3          // <----------------
hi
=3
>b = f()5*2 3 6          // <----------------
hi
=6
>b = f()5*2 f() f()          // <----------------
hi
hi
hi
=undefined
>b = f()5*2 f() f() "abc"          // <----------------
hi
hi
hi
="abc"
bobrippling commented 2 months ago

Currently a draft as I'm interested in if you think this is worth pursuing, would like to confirm that this is the correct approach and sort out / assert the tests raise a parse error

gfwilliams commented 2 months ago

Thanks - I'm in two minds about this... Generally I tried to avoid implementing JS's newline behaviour and went for this 'execute anything' approach as being dependent on newlines to make the code work just felt wrong. I'd expect that if this went in, pretokenising or functions with "ram" keyword (which pretokenise themselves) would potentially fail on some code as the pretokeniser just removes whitespace without any regard for this (and making it take syntax into account could be problematic/slow).

Apart from the issues with the return statement handling, is there anything where this actually stops Espruino from executing valid code correctly?

Or is it more that Espruino will execute JS code that isn't valid, rather than throwing an error?

Because if it's the first then yes, we need to get a fix in for it - but if it's the latter I'm less concerned (as the IDE's linter would error anyway) and if it did go in I'd want to ensure it doesn't break anything (like the pretokenised code) and definitely that it didn't hurt execution speed.

bobrippling commented 2 months ago

I agree, I suppose we could fix this but it'd necessitate handling newlines and possibly open a can of worms with ASI, and I like your point about the linter catching these issues anyway. I found the problem via a typo while using the IDE, so I don't think it's a large issue. Perhaps we park this and come back to it if anyone sees it in a more expected development pattern

gfwilliams commented 2 months ago

Ok, sounds good - thanks!