fix: appropriate error message for out of scope label

[Mvmo]

I've changed the error message when a label is not found in scope. It's a pretty easy fix, as it just checks if there's a label behind a break/continue statement and if so, give the message that the just used label doesn't exist - all this is checked as soon as the checkGrammarBreakOrContinueStatement hits a function/class like

Fixes #30408

[Mvmo]

@microsoft-github-policy-service agree

[jakebailey]

Although the old error isn't great, the new one doesn't feel much better because it's now saying "can't find the label" and the user's going to go "but it's right there!".

The best thing to do is going to be to say why the label can't be reached or some other info, which is what @DanielRosenwasser is suggesting in his comments on #30408.

[Mvmo]

@jakebailey should it show in which line the label is defined no matter where it's in the file? Or should there be some boundaries where it makes sense?

[jakebailey]

I'm not really sure what you mean; you could feasibly include the location of the "found" but unreachable label as part of the diagnostic ellaboration, yes.

In terms of search mechanism, #30408 (comment) did define a mechanism, which is to walk up the scopes and see if there's something defined and then mention it.

[Mvmo]

@jakebailey As of my understanding the error should look something like that:
A 'continue' statement can only jump to a label of an enclosing iteration statement.
Label found on ...

Am I right?

If so, which boundaries should there be for the check where the label is placed.
Example given

function justAFunction() {
     for (let i = 0; i < 10; i++) {
         if (i == 5)
             break myLabel;
     }
     
     myLabel:
}

Here i agree we should show where the label is defined

function justAnotherFunction() {
    function justAFunction() {
         for (let i = 0; i < 10; i++) {
             if (i == 5)
                 break myLabel;
         }
    }
    
    myLabel:
}

Should the line with the label definition be included in this case as well? I don't think it should be.

If it should be included, how far should I go? Should I traverse the entire tree of the source file to check if there's a label definition somewhere?

[Mvmo]

@jakebailey what do you think about this? Or should it be one error including a hint?

[jakebailey]

No, you don't want to emit two errors. The latter one is odd because an error on the "declaration" is impacted by whether or not it's used.

What I mean is that you can issue an error on the bad use and then reference the bad one via a new diagnostic with the declaration's location pushed to the main diagnostic's relatedInformation.

[Mvmo]

@jakebailey what do you think about this?

[jakebailey]

That seems better, though the "cannot find" seems like the wrong thing to say if there's an elaboration.

I think I'd say somehing more like "used before declaration" on the use, then in the elaboration say "label defined here". I think we have examples of this already.

That and the error range should only be the label itself, not the entire block like in the screenshot.

[Mvmo]

@jakebailey I've changed the messages.

I could also use The_declaration_of_0_that_you_probably_intended_to_use_is_defined_here for the elaboration message but for now i've created a new one for it.

[jakebailey]

In an effort to try and finish the PR, I've restructured this to avoid the "incorrect" errors.