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

Avoid awaiting inside a method where you don't need to run any continuation #24

Open
tugberkugurlu opened this issue Nov 17, 2014 · 9 comments

Comments

@tugberkugurlu
Copy link

This sounds actually an easy one to implement but it can easily turn out to be a challenging one. Let me put it here and we can discuss the corner cases.

We should make sure to directly return the Task or Task<T> object without awaiting inside a method where you don't need to run any continuation. For instance:

public class Foo 
{
    private const string FooBar = "foobar";
    private readonly IBar _bar;

    public Foo(IBar bar)
    {
        _bar = bar;
    }

    public async Task<Bar> GetBarsAsync()
    {
        Bar bar = await _bar.GetBarAsync(FooBar);
        return bar;
    }
}

The GetBarsAsync method here doesn't need to await on the _bar.GetBarAsync method call. It can just directly return the result as below:

public Task<Bar> GetBarsAsync()
{
    return _bar.GetBarAsync(FooBar);
}

Cases to Cover

  • IMO, This should be an information level diagnostic.
  • Cases where awaiting on ConfigureAwait(false) needs to be covered and it should be omitted on the modified code.
@sharwell sharwell changed the title "Avoid awaiting inside a method where you don't need to run any continuation" diagnostic Avoid awaiting inside a method where you don't need to run any continuation Nov 17, 2014
@jaredpar
Copy link

I'd been thinking of adding this, and a few other await style analyzers.

Where should this be added to? Should there be a separate async analyzer or should it go in the core DotNetAnalyzers?

@tugberkugurlu
Copy link
Author

@jaredpar I think the idea is to bundle all async analyzers/diagnostic/code-fixes into one repository as stated under #12. @sharwell can confirm this.

There are also other async related proposals: #10, #26.

@jaredpar
Copy link

I don't have a ton of experience with organizations. Is creating the repo something I can do or something you all have to do? This is actually something I'm rather eager to get started on (well analyzers and async in general).

@sharwell
Copy link
Member

I'll create it for you and make you an admin on it so you can set it up 👍

@jaredpar
Copy link

@sharwell awesome, thanks!

@BillWagner
Copy link
Contributor

@jaredpar @sharwell I'd be interested in helping on this as well. I've seen a bunch of anti-patterns in the field on async and await. It's a great way to help people find and fix them. (And learn better patterns).

@sharwell
Copy link
Member

Best thing I can suggest is start creating issues in the new AsyncUsageAnalyzers repository. If you make a copy of any existing ones which are currently proposals we can close those.

@tugberkugurlu
Copy link
Author

There are also really useful async analyzers inside AsyncPackage sample from Roslyn code base. I guess they are just there for the team to play around and they don't intend to ship these (my assumption only). Would that be OK to get those in here as well?

@sharwell
Copy link
Member

@jaredpar If you get your initial project structure checked in, it will be easier for @BillWagner to start working in parallel with you. 👍

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

4 participants