Adding Diagnostic message for missing ']' and ')' in Array literal and conditional statements

[keerthana1212]

Fixes #35597

[ghost]

All CLA requirements met.

[orta]

👋🏻 this is solid work @keerthana1212 - I think it's probably time to reduce the duplication though as this logic is repeated 5 times in the codebase (if merged) (the fifth is in #35586 )

I think you probably want a helper function like:

function parseExpectedWithRecovery(openKind: SyntaxKind, closeKind: SyntaxKind, openPosition: number) {
  if (!parseExpected(openKind)) {
    const lastError = lastOrUndefined(parseDiagnostics);
    if (lastError && lastError.code === Diagnostics._0_expected.code) {
      addRelatedInfo(
        lastError,
        createDetachedDiagnostic(fileName, openBracketPosition, 1, Diagnostics.The_parser_expected_to_find_a_1_to_match_the_0_token_here, tokenToString(openKind), tokenToString(closeKind))
      );
    }
    return false;
  }
  return true;
}

and to call that instead of parseExpected in the parseX functions

[sandersn]

@keerthana1212 do you want to keep working on this? If not, one of us may finish it up.

[keerthana1212]

I wouldn't be able to make progress with this change. Feel free to complete this change.

…

On Wed, Mar 10, 2021 at 4:01 PM Nathan Shively-Sanders < ***@***.***> wrote: @keerthana1212 <https://github.com/keerthana1212> do you want to keep working on this? If not, one of us may finish it up. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#40884 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE5O52ZPPN3DJFHRYP7C6ADTC723LANCNFSM4SA6ED5Q> .

[DanielRosenwasser]

I think this needs a revert - see the explanation here:

#43005 (comment)

[sandersn]

So, undo #35586 as well?

[sandersn]

I looked closer at the code. There are 3 places that look at the last error on the stack.

1. parseErrorAtPosition. This is a longstanding, context-free check to avoid many errors on the same location. It's fine, if ad-hoc. It's the one causing the other problems.
2. parseExpectedMatchingBrackets. I think this has the same problem that you pointed out in Token hints for missing closing braces: classes, enums, jsx, modules, types #37497.
3. parseTypedefTag. This probably has the same problem, but I'm not sure if I could create a repro for it. And it's @typedef, which is a syntactic dumpster fire anyway.

I don't think a revert is the best solution here.
There are couple of other fixes I thought of:

1. Make parseErrorAtPosition and friends return DiagnosticWithDetactedLocation | true | undefined where true means a dupe and undefined means no error. This is the best fix as long as it isn't too slow.
2. Make parseErrorAtPosition overwrite dupes so that it keeps the last instead of the first.

[sandersn]

I tried (2) since it's a simple change and the errors are either the same or worse. They're worse in places where the parser hasn't gone too far off the rails, so it's not a good fix.

[sandersn]

(1) makes a lot of improvements to baselines. I'll put it up and run the perf tests on it.