Skip to content

#1125 Fix parsing of unusual trailing expressions separated by whitespace #2252

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

Conversation

technohippy
Copy link
Member

Concerning:

  • Is "unexpected token error" appropriate for this error?

  • Serialized source text still has ";". Is it OK?

  • I've read the contributing guidelines

  • I've added my name and email to the NOTICE file

@dcodeIO
Copy link
Member

dcodeIO commented Apr 9, 2022

I wonder whether this is only applicable to variables and returns. Another one that comes to mind is throw. Do you know what the spec says exactly for this case? If it runs deeper, perhaps a fix could be more general.

@technohippy
Copy link
Member Author

I wonder whether this is only applicable to variables and returns. Another one that comes to mind is throw. Do you know what the spec says exactly for this case? If it runs deeper, perhaps a fix could be more general.

I agree. I don't know the spec well but I should consider more deeply.

Anyway, this change counts some valid codes like below as an error.

function foo(): i32 {
  return 123 }

I'll close this pullreq for now and reopen it after making more proper solution.

@technohippy technohippy closed this Apr 9, 2022
@technohippy
Copy link
Member Author

I read the spec but it's too complecated for me to handle all cases.

Anyway, this pull req can fix issue #1125. If other cases are found, please replace tn.skip(Token.SEMICOLON) to this.checkRuleCompleted(tn).

@technohippy technohippy reopened this Apr 11, 2022
@dcodeIO
Copy link
Member

dcodeIO commented Jan 8, 2023

Thanks :) There are likely more ASI-related issues, but seems good to have this merged for the (probably most prominent) cases covered herein.

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 this pull request may close these issues.

2 participants