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

Support "Make Async" on methods #7769

Closed
kuhlenh opened this issue Jan 4, 2016 · 10 comments
Closed

Support "Make Async" on methods #7769

kuhlenh opened this issue Jan 4, 2016 · 10 comments

Comments

@kuhlenh
Copy link

kuhlenh commented Jan 4, 2016

It's always a huge bummer to convert a bunch of my methods to async. Right now, I have to manually add "async" to my signature, change my return type to Task, and await my method call, etc. etc.

I would love if I could just Ctrl+. on a method and click "Make Async" and it would do all these tedious changes for me for each method. It would also be cool if you could pipe async through all affected code...not sure how plausible that is though.

@kuhlenh
Copy link
Author

kuhlenh commented Jan 4, 2016

@CyrusNajmabadi, @dpoeschl

@MgSam
Copy link

MgSam commented Jan 4, 2016

Adding `async' and changing the return type to Task in a single step sounds like a useful feature.

It would also be cool if you could pipe async through all affected code...not sure how plausible that is though.

This sounds really dangerous, being that properly using async requires propagating it up the call stack. It seems pretty likely a tool doing that recursively would have unintended side effects on the program execution.

@i3arnon
Copy link

i3arnon commented Jan 4, 2016

However, without changing the methods all the way up the transformation may end up with non-compiling code.

Adding await to the method call will not compile in a non-async method and not adding an await may not compile if there's an assignment to something which is not a Task.

@daveaglick
Copy link
Contributor

In addition to what @i3arnon said, if the originally synchronous method calls any async methods it's likely doing some combination of .Result, custom contexts, etc. Unwinding that automatically so that your newly-async method makes purley async calls to other async methods sounds very tricky (if it's even possible).

@MgSam
Copy link

MgSam commented Jan 4, 2016

However, without changing the methods all the way up the transformation may end up with non-compiling code.

I think this is preferable. You want to force the user (via errors) to consider the implications of making each and every method async, not just do work that may compile but is likely to have runtime bugs.

@jmarolf
Copy link
Contributor

jmarolf commented Jan 4, 2016

Note that if you await a method that is not async we offer to convert the method for you (see CSharpAddAsyncCodeFixProvider)

Few things we could do to improve:

  1. We don't convert void to Task, we should probably have some heuristics to detect the few cases where async void is appropriate and in all others change the return type.
  2. We don't check for entry points, we should have special logic if the async method is an entry point, possibly creating an encapsulating method that we call GetAwaiter().GetResult() on (unless we implement proposal Async Main [Speclet] #7476)
  3. We don't change MethodName to MethodNameAsync
  4. We should also offer this as a refactoring on any non-async method

@roji
Copy link
Member

roji commented Aug 9, 2016

I wrote something that does almost exactly what this issue describes: http://github.com/roji/AsyncRewriter. It's used in Npgsql (the .NET driver for PostgreSQL) to selectively generated async methods from sync counterparts.

FWIW I really don't think this kind of thing belongs in the compiler.

@jcouv
Copy link
Member

jcouv commented Apr 7, 2018

As @jmarolf described, the MakeAsync fix is offered as soon as you use await in a method.
But it looks like multiple improvements have been made since his comment:

  • the method gets renamed with Async suffix
  • the return type void can either be left as void or upgraded to Task (see screenshots below)

I think we could close this issue. Alternatively, clarify what functionality is left missing.

image

image

@scottsauber
Copy link

scottsauber commented May 8, 2018

+1 to this feature. It's a minor annoyance that when you create a new Razor Page or Controller from the default scaffolder in VS and have to convert the methods to async. Especially in Core where it's encouraged to async all the things.

I get I could just wait until I await the method, but my brain struggles not to fix something immediately when I see it's wrong. 😄

@CyrusNajmabadi
Copy link
Member

I think we could close this issue.

Agreed. I think a core case here has been addressed. If there are more, we should call them out specifically with new issues that we can evaluate.

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

No branches or pull requests

10 participants