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

ARROW_FUNCTION position set error #303

Closed
nabice opened this issue Apr 24, 2017 · 13 comments · Fixed by #363
Closed

ARROW_FUNCTION position set error #303

nabice opened this issue Apr 24, 2017 · 13 comments · Fixed by #363

Comments

@nabice
Copy link

nabice commented Apr 24, 2017

JS Code:
setTimeout(() => {}, 0);

In org.mozilla.javascript.Parser.java, line 887,
params.position is -1, this will make a wrong FunctionNode position.

@clearw5
Copy link

clearw5 commented May 15, 2017

Rhino supports arrow function? I try your code but only get a syntax error.

@nabice
Copy link
Author

nabice commented May 15, 2017

支持的。可能你没有用到最新的版本?

@clearw5
Copy link

clearw5 commented May 15, 2017

I am using the latest release 1.7.7.1 but cannot run any code with arrow function. How to enable this feature?
怎么支持呀 我设置语言版本为1.8和ES6也不行耶

@nabice
Copy link
Author

nabice commented May 15, 2017

不太清楚你怎么弄的,直接编译最新的是可以的。没有进行什么设置。

@clearw5
Copy link

clearw5 commented May 15, 2017

I finally found it was supported in the latest SNAPSHOT.

@Yidadaa
Copy link

Yidadaa commented Jul 2, 2017

@hyb1996 活捉autoJS作者2333

@gbrail
Copy link
Collaborator

gbrail commented Aug 15, 2017

Since this seems to be fixed in master I'm closing this.

@gbrail gbrail closed this as completed Aug 15, 2017
@nabice
Copy link
Author

nabice commented Nov 15, 2017

@gbrail, I found it still not fixed. FunctionNode has wrong position.

@gbrail
Copy link
Collaborator

gbrail commented Nov 15, 2017 via email

@nabice
Copy link
Author

nabice commented Nov 16, 2017

file: a.js
setTimeout(() => {}, 0);

When a.js is parsed, setTimeout's first argument is a FucntionNode instance. Property position of the instance should be 12, but 0 we get.
It can be fixed like this, probably not correct:

diff --git a/src/org/mozilla/javascript/Parser.java b/src/org/mozilla/javascript/Parser.java
index da03f0c7..787e8bc3 100644
--- a/src/org/mozilla/javascript/Parser.java
+++ b/src/org/mozilla/javascript/Parser.java
@@ -3106,7 +3106,7 @@ public class Parser
             Comment jsdocNode = getAndResetJsDoc();
             int lineno = ts.lineno;
             int begin = ts.tokenBeg;
-            AstNode e = (peekToken() == Token.RP ? new EmptyExpression() : expr());
+            AstNode e = (peekToken() == Token.RP ? new EmptyExpression(ts.tokenBeg) : expr());
             if (peekToken() == Token.FOR) {
                 return generatorExpression(e, begin);
             }

@sainaen
Copy link
Contributor

sainaen commented Nov 16, 2017

@nabice
For what it's worth, I don't see how to fix this differently. I even went ahead and create a pull request. 🙂

Well, there's one minor thing: I noticed that this way position will point at the opening parenthesis, while in case of parameters present (e.g. (a, b) => a+b) it will be one character further (ParenthesisedExpression copies its child position) — is it ok? (See the test in my PR for example of this.)

@gbrail gbrail reopened this Nov 17, 2017
@gbrail
Copy link
Collaborator

gbrail commented Nov 18, 2017

Can you please take a look at @sainaen 's PR and see if it fixes this for you?

#363

Thanks!

@nabice
Copy link
Author

nabice commented Nov 18, 2017

Yes, it does.

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

Successfully merging a pull request may close this issue.

5 participants