Adding Diagnostic message for missing ']' and ')' in Array literal and conditional statements#40884
Conversation
orta
left a comment
There was a problem hiding this comment.
👋🏻 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
|
@keerthana1212 do you want to keep working on this? If not, one of us may finish it up. |
|
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>
.
|
|
I think this needs a revert - see the explanation here: |
|
So, undo #35586 as well? |
|
I looked closer at the code. There are 3 places that look at the last error on the stack.
I don't think a revert is the best solution here.
|
|
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. |
|
(1) makes a lot of improvements to baselines. I'll put it up and run the perf tests on it. |
…teral and conditional statements (microsoft#40884)" This reverts commit 555ef73.
* Revert "Only issue matching token errors on non-dupe locations (microsoft#43460)" This reverts commit 76a2ae3. * Revert "Adding Diagnostic message for missing ']' and ')' in Array literal and conditional statements (microsoft#40884)" This reverts commit 555ef73. * re-add clobbered merge lines
This reverts commit 5770434.
…brackets (#44158) * Revert "Revert #43460 and #40884 (#44175)" This reverts commit 5770434. * fix missing opening brace match error * refactor parseExpectedMatchingBrackets * use getNodePos * accept baselines * delete mistakenly added files * Revert getNodePos addition Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
Fixes #35597