is-arrow-function icon indicating copy to clipboard operation
is-arrow-function copied to clipboard

False negative for arrow function with closing parenthesis in parameter list

Open tjcrowder opened this issue 6 years ago • 6 comments

For instance, if a default parameter value uses a function:

const isArrow = require("is-arrow-function");
const arrow = (a = Math.random(10)) => {};
console.log(isArrow(arrow)); // false, should be true

There are lots of ways to include a ) in the parameter list. For instance, (a, b = (a)) => {} (but who would write that? ... you know someone would :-) ).

Sadly, I think the only way to deal with this would be to fire up a full parser, since a default can be anything, including a function definition with arbitrary code in it:

const isArrow = require("is-arrow-function");
const arrow = (a = function() {
    if (Math.random() < 0.5) {
        return 42;
    }
    return "something else";
}) => a();
console.log(isArrow(arrow)); // false, should be true

tjcrowder avatar Nov 10 '18 14:11 tjcrowder

You're right; although this is only possible in strict mode.

I don't think a full parser is needed; there's probably other ways to check it reliably.

ljharb avatar Nov 10 '18 22:11 ljharb

You're right; although this is only possible in strict mode.

Sorry, I'm not following. Default parameter values don't require strict mode...? In fact I was running that code in loose mode.

I don't think a full parser is needed; there's probably other ways to check it reliably.

I don't see how, at least via toString. In terms of other approaches, I'm not coming up with anything in the absense of support from the language itself (accessor for the [[ThisMode]] internal slot, or [[HomeObject]] -- something I think would be useful for other reasons -- since I think the only other functions without prototype are methods). But it wouldn't be the first time I didn't see something until it was pointed out to me...

tjcrowder avatar Nov 11 '18 08:11 tjcrowder

By spec, yes, i believe they do - babel might allow it, but the language does not allow non-simple parameter lists in sloppy mode functions due to parsing difficulties.

ljharb avatar Nov 11 '18 17:11 ljharb

This strict mode thing is a sidetrack, since obviously we'd want isArrowFunction to work in strict mode anyway. But:

By spec, yes, i believe they do - babel might allow it, but the language does not allow non-simple parameter lists in sloppy mode functions due to parsing difficulties.

I don't think that's the case, can you point me at the part of the spec that says default parameter values are only allowed in strict mode?

  • I can't see how strict mode would have an effect on the complexity of parsing default parameter values. It's basically just an AssignmentExpression.
  • I don't see anything relevant about strict mode in the arrow parameters stuff (one of the places defaults are defined) or the places it links to, or in Annex C.
  • V8, SpiderMonkey, and Chakra are all happy with default parameter values in loose mode: http://jsfiddle.net/wzy0f1rj/ (I never rely on Babel for these things. Great tool, but constrained.)
  • MDN doesn't mention it (but MDN is community-edited and so not always perfect).
  • Mozilla Hacks doesn't mention it.
  • Rauschmayer doesn't mention it; he's usually fairly reliable.

If it's true strict mode is spec'd for this, I really want to know it. TIA.

tjcrowder avatar Nov 12 '18 07:11 tjcrowder

ahh, you're right - it's that a function with a non-simple parameter list can't change modes - ie, it can't be created in a sloppy mode context and attempt to become strict itself. (Definitely a sidetrack from the overall issue). See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Strict_Non_Simple_Params.

ljharb avatar Nov 12 '18 07:11 ljharb

ahh, you're right - it's that a function with a non-simple parameter list can't change modes

That's fascinating, thanks. (With that hint, found it here.) It's interesting that it's any non-simple param list (including a rest param). If it were a param list containing expressions, I'd figure it was about the execution context between where the function appears and the function body, but a rest param doesn't do that. Huh. Maybe just to keep the rule simple. Thanks!

tjcrowder avatar Nov 12 '18 08:11 tjcrowder