ConfigureAwait diagnostic + code fix for IAsyncEnumerable (and other types)

[jnm2]

Could the existing ConfigureAwait refactoring be loosened to offer this refactoring on any type that has a .ConfigureAwait(bool) instance or extension method in scope?

IAsyncEnumerable and IAsyncDisposable are two that were recently added to the BCL. It seems like all future types and user-defined types could be treated uniformly.

Analyzer package

Microsoft.CodeAnalysis.FxCopAnalyzers

Package Version

v3.0.0 (Latest)

Diagnostic ID

Example: CA2007

[mavasani]

Sounds reasonable. I would want an affirmation from @AArnott and @stephentoub before proceeding.

[stephentoub]

Seems ok. I expect it won't be as easy as treating types "uniformly", as the constructs here vary, e.g. await task.ConfigureAwait(false); vs await using (disposable.ConfigureAwait(false)) vs await foreach (var item in source.ConfigureAwait(false)), but the crux of the suggestion is fine.

[jnm2]

That's a good point. Treating uniformly doesn't make sense. I would specifically ask for code fixes just the two BCL types then. IAsyncDisposable in particular is a pain to manually refactor to start consuming with ConfigureAwait(false).

[sharwell]

IAsyncDisposable support was filed separately as #4888 and fixed in #4896

[MartyIX]

.NET 6.0 (the latest released preview) shows me CA2007 warnings for lines like:

await using DatabaseStorage storage = /* warning starts here */new(testPath, this.storageSettings, this.shutdown, this.storageLock, this.storageSerializer, nameof(this.CheckKeyExistsAsync)); /* warning ends here */

How can I actually fix it? Or is it an analyzer bug?

Hopefully, it's relevant here.

[jnm2]

These are our two options currently, and I've been doing the first one.

var storage = new DatabaseStorage(testPath, this.storageSettings, this.shutdown, this.storageLock, this.storageSerializer, nameof(this.CheckKeyExistsAsync))
await using (storage.ConfigureAwait(false))
{
    // ...

var storage = new DatabaseStorage(testPath, this.storageSettings, this.shutdown, this.storageLock, this.storageSerializer, nameof(this.CheckKeyExistsAsync))
await using var _1 = storage.ConfigureAwait(false);

// ...

See dotnet/csharplang#2235 which requests the ability to do await using _ = expr; or similar.

[MartyIX]

@jnm2 Thanks!

[MartyIX]

Hm, using the first approach, I get CA2000 instead. 😞

[jnm2]

I turn off CA2000 because there are too many false positives for me in solid situations that are just hard for the analyzer to follow.

[MartyIX]

(I like CA2000 even though it would be great to have some attributes specifying owners of objects or something. It helps me find bugs from time to time.)

[manfred-brands]

@genlu Still looking for help to implement this?

[genlu]

@manfred-brands Absolutely, any contribute is welcomed!