Skip to content

Confusing error message for labels used before definition #30408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
paulvanbrenk opened this issue Mar 14, 2019 · 13 comments · May be fixed by #54927
Open

Confusing error message for labels used before definition #30408

paulvanbrenk opened this issue Mar 14, 2019 · 13 comments · May be fixed by #54927
Labels
Domain: Error Messages The issue relates to error messaging Domain: Related Error Spans Specifying regions for error messages/diagnostics on multiple locations. Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Experience Enhancement Noncontroversial enhancements Good First Issue Well scoped, documented and has the green light Help Wanted You can do this PursuitFellowship Help wanted from Pursuit fellowship; others please avoid until Dec 19 Suggestion An idea for TypeScript
Milestone

Comments

@paulvanbrenk
Copy link
Contributor

paulvanbrenk commented Mar 14, 2019

The following gives me an unexpected "TS1007: Jump target cannot cross function boundary error" in TS 3.3.33333

function foo() {
    for (let i = 0; i < 10; i++) {
        console.log(`${i}`);
        continue loopend;
    }

    loopend:
    console.log('end of loop');
}

Further investigation... you can jump to the start of the loop, so the error I would argue is that the error message is confusing.

shareable link

@DanielRosenwasser DanielRosenwasser changed the title Invalid "TS1007: Jump target cannot cross function boundary error" error in TS 3.3 Confusing error message for labels used before definition Mar 14, 2019
@DanielRosenwasser
Copy link
Member

Did you try adding 3s to the version to see if the error gets better?

Seems like a reasonable request.

@DanielRosenwasser DanielRosenwasser added Help Wanted You can do this Good First Issue Well scoped, documented and has the green light Domain: Error Messages The issue relates to error messaging Domain: Related Error Spans Specifying regions for error messages/diagnostics on multiple locations. labels Mar 14, 2019
@DanielRosenwasser DanielRosenwasser added this to the Backlog milestone Mar 14, 2019
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Mar 14, 2019

Here's a better message for this specific case, where the containing function of the label reference is the same as that of the labeled statement:

A label cannot be referenced prior to its declared location.

To find if it's applicable, walk up each scope until the first functionlike (isFunctionLike) or classlike (isClassLike) declaration and look for LabelledStatements in that scope.

We can also provide a related span. We already have a diagnostic for

{0} is declared here.

@paulvanbrenk
Copy link
Contributor Author

paulvanbrenk commented Mar 14, 2019 via email

@DanielRosenwasser
Copy link
Member

Actually, we already have a better message for break or continue error messages.

@paulvanbrenk
Copy link
Contributor Author

paulvanbrenk commented Mar 15, 2019 via email

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Experience Enhancement Noncontroversial enhancements labels Mar 15, 2019
@laurafiuza
Copy link

Hey -- is this still something that should be implemented? Happy to make changes now if no one else is already

@paulvanbrenk
Copy link
Contributor Author

Yes, please. This is still an issue in @next

@DanielRosenwasser DanielRosenwasser added the Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". label Mar 28, 2019
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Mar 28, 2019

It may be a bit more work than I originally anticipated (since labels aren't placed in any sort of scope), but we're taking PRs if you're up for it.

I guess if you can walk up the tree and look for labels in each enclosing scope, then:

  • If the label actually contains the source of the break/continue, but it crosses a function boundary, you can continue to give the same error message. The key difference is that you need to keep walking up the tree to search for the label and don't just stop at a function boundary.

  • If the label doesn't contain the origin of the break/continue, then use either

    Diagnostics.A_break_statement_can_only_jump_to_a_label_of_an_enclosing_statement
    

    or

    A_continue_statement_can_only_jump_to_a_label_of_an_enclosing_iteration_statement
    

    (these already exist apparently)

  • Otherwise, give a message like

    Cannot find label '{0}'.
    

In the first two, give the appropriate related span mentioned above.

@karthikkp
Copy link
Contributor

@laurafiuza Still working on this?

@karthikkp
Copy link
Contributor

karthikkp commented May 27, 2019

@DanielRosenwasser For point 1, we will have to go through the entire set of statements in the source file to check if the label exists as, as soon as we travel up the tree and go beyond the function boundry we get the source file in the example in the first example. To simply put we will have to check the siblings of the function statement. Is my understanding right?

@DanielRosenwasser
Copy link
Member

I would avoid trying to walk through every scope. Try going with this initial suggestion (though ensure you don't go past the SourceFile):

To find if it's applicable, walk up each scope until the first functionlike (isFunctionLike) or classlike (isClassLike) declaration and look for LabelledStatements in that scope.

Just scan through the statements at each level until you hit a function-like/class-like - don't try to walk recursively.

@karthikkp
Copy link
Contributor

karthikkp commented May 28, 2019

I would avoid trying to walk through every scope. Try going with this initial suggestion (though ensure you don't go past the SourceFile):

To find if it's applicable, walk up each scope until the first functionlike (isFunctionLike) or classlike (isClassLike) declaration and look for _LabelledStatement_s in that scope.

Just scan through the statements at each level until you hit a function-like/class-like - don't try to walk recursively.

@DanielRosenwasser This is what is already being done. The cross boundary error is returned as soon as the first functionlike statment is found.

@mInzamamMalik
Copy link

@sandersn sandersn added the PursuitFellowship Help wanted from Pursuit fellowship; others please avoid until Dec 19 label Sep 18, 2020
@Mvmo Mvmo linked a pull request Jul 8, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Error Messages The issue relates to error messaging Domain: Related Error Spans Specifying regions for error messages/diagnostics on multiple locations. Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Experience Enhancement Noncontroversial enhancements Good First Issue Well scoped, documented and has the green light Help Wanted You can do this PursuitFellowship Help wanted from Pursuit fellowship; others please avoid until Dec 19 Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants