Skip to content
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

Go-to-definition on await/yield should probably jump to the containing respective async function/generator #51223

Closed
DanielRosenwasser opened this issue Oct 19, 2022 · 3 comments · Fixed by #51838
Labels
Domain: Symbol Navigation Relates to go-to-definition, find-all-references, highlighting/occurrences. Experience Enhancement Noncontroversial enhancements Good First Issue Well scoped, documented and has the green light Help Wanted You can do this
Milestone

Comments

@DanielRosenwasser
Copy link
Member

Similar to #51222.

async function outerAsyncFun() {
  let af = /*END*/async () => {
    /*START*/await Promise.resolve(0);
  }
}
function* outerGen*() {
  /*END*/function* gen() {
    /*START*/yield 100;
  }
  return gen
}

We should probably do nothing in cases like top-level await, or where await does not have a direct corresponding async function, and yield does not have a direct corresponding generator.

@DanielRosenwasser DanielRosenwasser added Help Wanted You can do this Good First Issue Well scoped, documented and has the green light Experience Enhancement Noncontroversial enhancements Domain: Symbol Navigation Relates to go-to-definition, find-all-references, highlighting/occurrences. labels Oct 19, 2022
@DanielRosenwasser DanielRosenwasser added this to the Backlog milestone Oct 19, 2022
@fatcerberus
Copy link

fatcerberus commented Oct 19, 2022

I would expect GTD on await to jump to the async keyword of the containing function, in line with how e.g. VS C# intellisense highlights the async keyword (and incidentally, all the other awaits in the same function) when you place the cursor on an await.

@DanielRosenwasser
Copy link
Member Author

I guess maybe there's a catch with

class Foo {
  /*END1*/public /*END2*/async yadda() {
    /*START*/await 10;
  }
}

I feel like the distance is not that big that it matters whether we pick END1 or END2. I could go both ways.

Thinking about it some more, I wonder if it makes sense to always make the go-to-definition succeed, even if the containing function isn't async or a generator. If you're in the error case, it can be helpful to jump to whichever function contains the keyword.

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Oct 19, 2022

Also, another fun test case based on the "should not fail" strategy".

function* gen() {
  /*END(???)*/class C { [/*START*/yield 10]() {} }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Symbol Navigation Relates to go-to-definition, find-all-references, highlighting/occurrences. Experience Enhancement Noncontroversial enhancements Good First Issue Well scoped, documented and has the green light Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants