[[FIX]] Tolerate late definition of async function

[pirxpilot]

We used to mark async function as unreachable instead of treating it as any other late function definition. This change makes jshint react to async keyword in the same way as it reacted to function keyword.

[jugglinmike]

Thanks for the patch! This is definitely an oversight, and I'm grateful you've stepped up to fix it.

Since async was never formally reserved, it can be used as an identifier, and that makes it tricky to deal with. For a concrete example, here's a test that ought to pass:

  "async identifier": function (test) {
    var src = [
      "(function() {",
      "  return f(4);",
      "  async();",
      "}());"
    ];

    TestRun(test)
      .addError(3, 3, "Unreachable 'async' after 'return'.")
      .test(src, { esversion: 8, latedef: "nofunc" });

    test.done();
  }

Now, disambiguating async in a lookahead (that is, before formally parsing it) is pretty tough. The good news is that I think we might be able to ignore all that complexity because it mostly comes from accounting for arrow functions, and we're only conscered with function declarations here. As far as I know, a relatively straightforward heuristic that considers the next two tokens ought to be enough (though we probably have to take care about newlines).

Does that sound right to you? Would you be up for addressing it?

[pirxpilot]

How about now?
I have to admit I did not spend much/any time looking at the jshint code, so I may be doing something stupid here. Also - are you sure we need to worry about the newlines? I do skip them but I am thinking newlines between async and function just mean that async is parsed as an identifier. Or do you mean some other newlines?

[jugglinmike]

Much closer! Your expectation about the grammar is correct: a newline between async and function will cause async to be interpreted as an identifier. In that case, we want to preserve this warning, since that identifier (confusing as it may be) represents unreachable code.

However, the latest iteration of suppresses the warning when it encounters the sequence "async [[newline]] function". Here's a test that I believe ought to pass:

  "async asi": function (test) {
    var src = [
      "(function() {",
      "  return f(4);",
      "  async",
      "  function f(p) {",
      "    return p + 1;",
      "  }",
      "}());"
    ];

    TestRun(test)
      .addError(3, 3, "Unreachable 'async' after 'return'.")
      .test(src, { esversion: 8, latedef: "nofunc", asi: true, expr: true });

    test.done();
  },

(I added in the asi and expr linting options to silence the warnings that JSHint would produce by default. I'm hoping that makes the intent of the test more clear.)

Does that make sense to you? If so, I think we may be able to simplify the implementation--we only want to suppress the warning if we see async followed immediately by function.

[pirxpilot]

I added the recommended test and removed the code that used to skip newline. Let me know what you think.

[jugglinmike]

Looks great to me! Thanks so much for sticking with this; I'm happy to have your help :)

I'll publish a patch release including this fix within the week.

[jugglinmike]

We just released this via JSHint version 2.13.5. Thanks again!