jsParensError for parenthesized arrow function

[davidchambers]

I just noticed that the closing parens in the following line is marked as jsParensError:

f(() => {});
          ^

The same applies in this case:

f(() => {
});
 ^

This also exhibits the problem:

f((x) => {});
           ^

Interestingly, this does not:

f(x => {});

When I have a bit more time I will run git bisect to determine which change is responsible.

[amadeus]

Good find, this is part of the challenge with arrow functions in general. Finding the specific commit is probably not necessary, I'll take a look.

[davidchambers]

It was something in #363. I've played around a bit, but so far haven't determined exactly which change is responsible.

[TamsynUlthara]

I just ran into this; it's 36f34b2 according to git-bisect.

[TamsynUlthara]

Reverting this change fixes the issue:

-syntax match   jsArrowFuncArgs  /(\(\k\|,\|\s\|\n\|\.\)*)\s\+\(=>\)\@=/ skipwhite contains=jsFuncArgs extend
+syntax match   jsArrowFuncArgs  /(\%(.\)*)\s*\(=>\)\@=/ skipempty skipwhite contains=jsFuncArgs nextgroup=jsArrowFunction

[TamsynUlthara]

It's the greedy \%(.\)* inside the arrow parameters' parentheses match. It's consuming too much of the line, including the closing ) of the parameters.

A quick fix is changing that to the non-greedy equivalent .{-} — the Vim equivalent of PCRE's .*? — but then you're still matching unbalanced parentheses and whatnot. (We don't need the surrounding \%( and \) either way.)

[davidchambers]

Thanks for investigating, @tomxtobin!

A quick fix is changing that to the non-greedy equivalent .{-} — the Vim equivalent of PCRE's .*? — but then you're still matching unbalanced parentheses and whatnot.

It's not valid to have ( or ) characters in the first elision of (…) => {…}, though, so we needn't worry about balancing parens. Perhaps all we need to do is replace the .* with [^()]* (or whatever the Vim equivalent of this PCRE is).

[amadeus]

@davidchambers I think it is valid to have something like:

(x = (2 + 3)) => {}

Ideally the requirements for this regex are:

• Cannot be contained
• Must allow pretty much every type of expression
• Needs to be matched differently than jsParen
• Handle destructuring

Some crazier examples that make this regex really hard to do right:

((x = (2 + 3)) => { return x; })();
((x = (true ? 1 : 3)) => { return x; })();
((x = (true ? event => event.response : () => { return error.response; })) => { return x; })();

var multiline = ({
  x = 2,
  y = false
}) => {
    console.log()
}

[davidchambers]

I think it is valid to have something like:

(x = (2 + 3)) => {}

Oh! I wasn't aware of this, @amadeus. The problem is trickier than I imagined.

[amadeus]

I mean, to be fair, just because people could write such shitty and hard to read code, doesn't mean they should.

To paraphrase a brilliant friend of mine, we may be reaching the limits of what a regex can interpret in a complex language.

[amadeus]

Like, ideally, jsArrowFuncArgs should probably be region, not a match. But in all my trial and error of trying out different regex combinations, and priorities, I have yet to find something that works in all cases.

[amadeus]

Ok, so this issues is pretty bad, and I think a good workaround in the meantime, until we can get it properly fixed is to comment out the jsArrowFuncArgs match that incorporates parenthesis. At the very least people won't see those major red errors.

[amadeus]

So, I've been playing with things a bit, and I have something that works pretty well, with a few caveats: 9b58fcc

Some notes:

1. It fixes all examples posted above - EXCEPT multiline
2. You cannot use parenthesis inside a destructuring statement (this includes creating another lambda statement
3. It also has the added benefit of just disabling itself if there are parenthesis inside, so you still get something that falls back 'safely' (at least so far from what I've been able to test).

I've done some research into other editors, like Sublime and Atom, and all seem to break down if you incorporate new lines into the arguments section or if you use lambdas or parens inside the destructuring statement, so that's not a huge issue imo.

[amadeus]

A basic version of this is fixed in develop