Skip to content

Go-to-definition on continue and break should jump around(?) corresponding statement #51224

Open
@DanielRosenwasser

Description

@DanielRosenwasser

Similar to #51222 and #51223.

/*END*/while (true) {
  if (Math.random()) /*START*/break;
}
/*END1*/label: while (true) {
  while (true) {
    if (Math.random()) /*START*/break label;
  }
}
/*END2*/
/*END1*/while (true) {
  if (Math.random()) /*START*/continue;
}
/*END2*/
/*END1*/switch (Math.random() < 0.5) {
  case true: /*START*/break;
}
/*END2*/

Given how go-to-definition on return, await, and yield might work, it's tempting to jump up to the top of the corresponding statement.

However, I could also see us jumping to wherever the break or continue itself would jump. That's a bit at odds with the original intent of go-to-definition on return since the point of that was to figure out "who owns the current return statement?"

If someone wants to send a PR and discuss more in that PR, they're welcome to.

Activity

added
Good First IssueWell scoped, documented and has the green light
Effort: CasualGood issue if you're already used to contributing to the codebase. Harder than "good first issue".
and removed
Good First IssueWell scoped, documented and has the green light
on Oct 19, 2022
added this to the Backlog milestone on Oct 19, 2022
sviat9440

sviat9440 commented on Oct 21, 2022

@sviat9440
Contributor

Both options have a reason to be. Maybe take it out in the config? But then this option should also be applicable for the return for uniformity.

If we still go to the definition on return, then it is more logical to do the same in these cases.

sviat9440

sviat9440 commented on Oct 21, 2022

@sviat9440
Contributor

Additional test cases:

Should works

/*END*/label: switch (null) {
  case null: {
    test: switch (null) {
      case null: /*START*/break label;
    }
  }
}
/*END*/label: while (true) {
  while (true) {
    if (Math.random()) /*START*/continue label;
  }
}

Should not fail:

label: switch (null) {
  case null: [|/*START*/break|] test;
}
sviat9440

sviat9440 commented on Oct 21, 2022

@sviat9440
Contributor

okay, my pr is ready. Waiting for closure #51236

SamB

SamB commented on Mar 3, 2023

@SamB

However, I could also see us jumping to wherever the break or continue itself would jump. That's a bit at odds with the original intent of go-to-definition on return since the point of that was to figure out "who owns the current return statement?"

Well, personally I would prefer if go-to-definition on return would go to the call. 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Effort: CasualGood issue if you're already used to contributing to the codebase. Harder than "good first issue".Experience EnhancementNoncontroversial enhancementsHelp WantedYou can do thisIn DiscussionNot yet reached consensus

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @SamB@DanielRosenwasser@sviat9440

        Issue actions

          Go-to-definition on `continue` and `break` should jump around(?) corresponding statement · Issue #51224 · microsoft/TypeScript