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

certain bug: Illegal async arrow function parameter list #25

Closed
nchanged opened this issue Jul 30, 2019 · 7 comments
Closed

certain bug: Illegal async arrow function parameter list #25

nchanged opened this issue Jul 30, 2019 · 7 comments

Comments

@nchanged
Copy link
Contributor

nchanged commented Jul 30, 2019

version: 1.4.8
Snippet:

async function test(){
  const someVar = null;
  const done = async foo => {}
}

Throws Illegal async arrow function parameter list (also reproducible in the REPL)

Observations: commenting out const someVar = null; fixes the problem.

Can be found in the npm package electron-updater right here node_modules/electron-updater/out/AppUpdater.js

Related issue: fuse-box/fuse-box#1570

Being parsed without any issues with typescript or babel or esprima
https://astexplorer.net/#/gist/3e1e685684c77a841dd4452b677654f9/6e25a2a530fc046e1dc55f5f3285fa7d5acd20d0

@nchanged nchanged changed the title possible bug: Illegal async arrow function parameter list certain bug: Illegal async arrow function parameter list Jul 30, 2019
@KFlash KFlash closed this as completed in c2b96cb Jul 30, 2019
@leeoniya
Copy link

@KFlash have you considered doing something like https://github.com/rust-lang/crater#crater- against large/popular NPM or github repos? i think it would quickly uncover more "edge" cases.

@nchanged
Copy link
Contributor Author

@leeoniya the best way to test it is to run against as many libraries as possible. That I can provide, FuseBox' users started adapting the new version based on meriyah.

@KFlash
Copy link
Contributor

KFlash commented Jul 31, 2019

@leeoniya I will try to validate against as some libraries soon as I get time. ATM. What @nchanged mentioned is the best solution.

Beside that I'm stress testing the code entire time, and run 3x different fuzzers and so on.

But to be honest. It's a known issue for me that async and async arrows are troublesome because I'm parsing this in a different way than what the specs says to avoid tons of known issues from other parsers and V8 with async arrow + call expr. This so I can avoid performance bottlenecks.

And sometimes I suddenly find more "edge cases" because one of my previous students did some code changes without knowing what he was doing as what happened with v. 1.4.5.

@nchanged
Copy link
Contributor Author

nchanged commented Jul 31, 2019

const a = {
  foo: () => {
  },
  bar: async event => {
  }
);

This bug hasn't been fixed. Which makes me think there are more similar cases. @KFlash please take a look at all possible variations.

@nchanged nchanged reopened this Jul 31, 2019
@KFlash
Copy link
Contributor

KFlash commented Jul 31, 2019

@nchanged Are you sure about this? Btw. Your example is invalid. Missing a right brace.

If I modify your example into this, it pass for me

const a = {
        foo: () => {
        },
        bar: async event => {
        }
      }

@KFlash KFlash closed this as completed in 776199d Jul 31, 2019
@KFlash
Copy link
Contributor

KFlash commented Jul 31, 2019

@nchanged FYI. This bug was caused because of a mutual parser flag and validation against it. This flag have been replaced with a func arg assignable and validations against that arg instead. You can find it here. This means that this bug will never happen again, and everything related to it can't happen either. So permanently solved :)

@KFlash
Copy link
Contributor

KFlash commented Jul 31, 2019

@leeoniya @nchanged Parsed a lot of libraries now but couldn't find any issues or "edge cases".
I added a bunch of new tests just to be sure. Even the Chinese language is very well supported :)

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

No branches or pull requests

3 participants