Peast cannot parse comma expression in assignment expression prior to ES8 (ES2017)

[lucaswerkmeister]

Minimal example:

(a=1,2)

As far as I understand, this should be valid in all syntax versions – comma expressions and assignment expressions are both carried over from C. However, Peast refuses to parse this prior to ES8:

> Peast\Peast::ES7('(a=1,2)')->parse()  # aka ES2016

   Peast\Syntax\Exception  Unexpected: 2.

> Peast\Peast::ES8('(a=1,2)')->parse()  # aka ES2017
= Peast\Syntax\Node\Program {#6389
    +location: Peast\Syntax\SourceLocation {#6411
      +start: Peast\Syntax\Position {#6388},
      +end: Peast\Syntax\Position {#6326},
    },
  }

I believe this is a bug in Peast, though I’m not sure. I wasn’t able to find a trustworthy-looking “canonical” validator for earlier ES versions, but at least Node.js has been able to parse this syntax since the earliest versions on Docker Hub (published June 2015):

$ sudo docker run -it node:0.10.38
> (a=1,2)
2

Slightly less minimal version (you can try that it prints 3 in Node, but it’s also rejected by Peast pre-ES8):

function f(){return 1,(a=2,3);}console.log(f())

See also T354807 at Wikimedia’s bugtracker if you’re interested.

[lucaswerkmeister]

I wasn’t able to find a trustworthy-looking “canonical” validator for earlier ES versions

Here’s acorn, for what it’s worth, reporting this as parseable in ES3 and ES5:

> acorn.version
'8.11.3'
> acorn.parse('(a=1,2)', { ecmaVersion: 3 })
Node {
  type: 'Program',
  start: 0,
  end: 7,
  body: [
    Node {
      type: 'ExpressionStatement',
      start: 0,
      end: 7,
      expression: [Node]
    }
  ],
  sourceType: 'script'
}
> acorn.parse('(a=1,2)', { ecmaVersion: 5 })
Node {
  type: 'Program',
  start: 0,
  end: 7,
  body: [
    Node {
      type: 'ExpressionStatement',
      start: 0,
      end: 7,
      expression: [Node]
    }
  ],
  sourceType: 'script'
}

[lucaswerkmeister]

The difference is apparently caused by the trailingCommaFunctionCallDeclaration syntax feature, specifically its check in parseFormalParameterList() (as opposed to the one in parseArgumentList(), which doesn’t affect this issue AFAICT). If I add &&false to that condition in parseFormalParameterList(), Peast can parse the code again. So I’m guessing Peast is trying to parse (a=1,2) as the beginning of an arrow function (like (a=1,b=2)=>a+b) and complaining that 2 is not a parameter name, and then… somehow not backtracking properly, whereas in ES8+ it does backtrack? (But I’m way out of my depth here, I don’t know how Peast’s parser works.)

[mck89]

Your analysis is correct, when the parser reads the 2 it stops parsing the parseFormalParameterList because it's not valid for that context, for this reason it detects the comma before 2 as a trailing comma so it throws an exception if the trailingCommaFunctionCallDeclaration feature is not active.

I will look into this, it seems that it is caused by the wrong detection of what is a trailing comma in a parameters list.

[mck89]

I've just released the new version 1.16 containing the fix for this bug. Thank you for reporting!

[lucaswerkmeister]

Amazing, thank you!

[Krinkle]

@mck89 In case you'd like to keep track, your library is now used in the MediaWiki software that powers Wikipedia!

I selected your library as part of T75714, for our ResourceLoader component, where we validate user-submitted JavaScript code (what?).

I found your library the most well-maintained and up-to-date implementation I could find, valuing in particular:

• easy to use (one function as entry point),
• dependency-free (thus easy to audit and review for our checked-into-git vendor, read more),
• under an OSI-approved and GPL-compatible free software license,
• fast enough to validate new submissions in real-time during a web response.

You're listed and credited at https://en.wikipedia.org/wiki/Special:Version. The code using it is also mirrored at https://github.com/wikimedia/mediawiki.

[mck89]

@Krinkle that's amazing, thank you for letting me know!