New `CA2007` behavior does not interact well with `CA2000`

[MartyIX]

Analyzer

Diagnostic ID: CA2007: Do not directly await a Task

Analyzer source

SDK: Built-in CA analyzers in .NET 6 RC-1 SDK or later

Version: SDK 6.0.0-rc.1.21451.13

Describe the bug

CA2007 reports new valid warnings in .NET 6 RC-1 but it's far from clear what the user should actually do to resolve the issue with reasonable time effort to have warning free code when CA2000 and CA2007 are turned on.

Steps To Reproduce

I have CA2000 and CA2007 enabled in my project and in .NET 5 my code was warning free. Now in .NET 6 RC-1, I have a CA2007 warning:

await using AsyncLock resourceLock = /*<CA2007-warning>*/new("Shared") /*</CA2007-warning>*/;

Now I wonder how to actually resolve the warning. Here are my attempts:

1. Apply "Append .Configure(false)" in Visual Studio

Click CTRL + . on the warning location and apply "Append .Configure(false)" action. This leads to await using AsyncLock resourceLock = (new("Shared")).ConfigureAwait(false); and this code cannot be actually compiled. So this approach does not work.

2. Attempt to go with Cyrus' pattern

Applying the pattern from here https://github.com/dotnet/roslyn-analyzers/issues/4888#issue-813977100 leads me to:

AsyncLock resourceLock = /*<CA2000-warning>*/new("Shared") /*</CA2000-warning>*/;
await using var _1 = resourceLock.ConfigureAwait(false); // `_1` reports SA1312 and `var` reports IDE0008.

So now I have removed one CA2007 warning to get a new CA2000 warning.

Expected behavior

The new CA2007 warning is correct but to allow easy migration to .NET 6, one of the following should probably hold:

• CA2000 should be smarter
• CA2007 should be configurable to disable the new behavior temporarily
• Visual Studiou should fix the issue automatically so that one can fix it in a non-trivial project easily.

Currently, I don't really know an acceptable workaround. I would not really like to disable CA2007.

[MartyIX]

@jmarolf Any workaround or recommendation please? Or is it out-of-scope for .NET 6?

[jmarolf]

@MartyIX where is the type AsyncLock defined? That isn't a BCL type correct?

[MartyIX]

@MartyIX where is the type AsyncLock defined? That isn't a BCL type correct?

That's a type from this library https://github.com/StephenCleary/AsyncEx/blob/master/doc/AsyncLock.md#example-usage. However, it's not really important as any C# type implementing IAsyncDisposable can be put in the example.

[jmarolf]

I do not understand AsyncLock from that library does not implement IAsyncDisposable, does it? Can you provide a complete code example?

[jmarolf]

This is what I can reproduce using the public API surface area of Nito.AsyncEx

Project File:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Nito.AsyncEx" Version="5.1.2" />
  </ItemGroup>

</Project>

editorconfig

[*.cs]
dotnet_diagnostic.CA2007.severity = error
dotnet_diagnostic.CA2000.severity = error

code

using Nito.AsyncEx;

namespace ClassLibrary3
{
    public class C
    {
        public async Task SomeMethod(CancellationToken cancellationToken)
        {
            AsyncLock resourceLock = new();
            using IDisposable? disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false);
        }
    }
}

[jmarolf]

I am seeing a diagnostic for this case:

public class C
{
    public static async Task SomeMethod(CancellationToken cancellationToken)
    {
        OtherAsyncLock resourceLock = new();
        await using IAsyncDisposable disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false);
    }
}

public class OtherAsyncLock
{
    public  Task<IAsyncDisposable> LockAsync(CancellationToken cancellationToken)
    {
        throw new NotImplementedException();
    }
}

But I do not get a warning if I change the code to be this:

public class C
{
    public static async Task SomeMethod(CancellationToken cancellationToken)
    {
        OtherAsyncLock resourceLock = new();
        IAsyncDisposable disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false);
        await using var _ = disposable.ConfigureAwait(false);
    }
}

public class OtherAsyncLock
{
    public  Task<IAsyncDisposable> LockAsync(CancellationToken cancellationToken)
    {
        throw new NotImplementedException();
    }
}

which appears correct. if you are doing await using on a disposable without first calling ConfigureAwait you are violating CA2007.

I would expect this to work:

await using ConfiguredAsyncDisposable resourceLock = (new AsyncLock("Shared")).ConfigureAwait(false);

calling ConfigureAwait changes the return type to ConfiguredAsyncDisposable (see here).

@sharwell I believe the bug here is that the codefix does not change the return type when adding ConfigureAwait for and IAsyncDisposable. Do you agree?

[MartyIX]

@jmarolf Thank you. I see I failed to describe the issue well. Your elaboration is very useful. So let me show you my actual issue by modifying your code.

Project file (the same as yours):

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Nito.AsyncEx" Version="5.1.2" />
  </ItemGroup>

</Project>

.editorconfig (essentially same as yours; I find it better to report warnings rather than errors by analyzers to avoid potential confusion with compilation errors)

# To learn more about .editorconfig see https://aka.ms/editorconfigdocs
root = true

[*.cs]
dotnet_diagnostic.CA2007.severity = warning
dotnet_diagnostic.CA2000.severity = warning

The code is long but it just demonstrates different implementations of MyAsyncLock (first not disposable in C1, then IDisposable in C2 and then IAsyncDisposable in C3):

public class C1
{
    public static async Task A(CancellationToken cancellationToken)
    {
        MyAsyncLock resourceLock = new();
        await using IAsyncDisposable disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false); // CA2007.
    }

    /// <summary>No warning in the method.</summary>
    public static async Task B(CancellationToken cancellationToken)
    {
        MyAsyncLock resourceLock = new();
        IAsyncDisposable disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false);
        await using var _ = disposable.ConfigureAwait(false);
    }

    /// <summary>Async lock implements neither <see cref="IDisposable"/> nor <see cref="IAsyncDisposable"/>.</summary>
    private class MyAsyncLock
    {
        public Task<IAsyncDisposable> LockAsync(CancellationToken cancellationToken)
        {
            throw new NotImplementedException();
        }
    }
}

public class C2
{
    public static async Task A(CancellationToken cancellationToken)
    {
        MyAsyncLock resourceLock = new(); // CA2000.
        await using IAsyncDisposable disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false); // CA2007.
    }

    public static async Task B(CancellationToken cancellationToken)
    {
        MyAsyncLock resourceLock = new(); // CA2000.
        IAsyncDisposable disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false);
        await using var _ = disposable.ConfigureAwait(false);
    }

    /// <summary>No warning in the method.</summary>
    public static async Task C(CancellationToken cancellationToken)
    {
        using MyAsyncLock resourceLock = new();
        IAsyncDisposable disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false);
        await using var _ = disposable.ConfigureAwait(false);
    }

    /// <summary>Async lock implements <see cref="IDisposable"/>.</summary>
    private class MyAsyncLock : IDisposable
    {
        public Task<IAsyncDisposable> LockAsync(CancellationToken cancellationToken)
        {
            throw new NotImplementedException();
        }

        public void Dispose()
        {
            throw new NotImplementedException();
        }
    }
}

public class C3
{
    /// <summary>Analyzer behaves as expected.</summary>
    public static async Task A(CancellationToken cancellationToken)
    {
        MyAsyncLock resourceLock = new(); // CA2000.
        await using IAsyncDisposable disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false); // CA2007.
    }

    /// <summary>Analyzer behaves as expected.</summary>
    public static async Task B(CancellationToken cancellationToken)
    {
        MyAsyncLock resourceLock = new(); // CA2000.
        IAsyncDisposable disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false);
        await using var _ = disposable.ConfigureAwait(false);
    }

    /// <summary>This is actually my reported issue.</summary>
    public static async Task C(CancellationToken cancellationToken)
    {
        await using MyAsyncLock resourceLock = new(); // CA2007: How to fix this?
        IAsyncDisposable disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false);
        await using var _ = disposable.ConfigureAwait(false);
    }

    /// <summary>An attempt to fix warning in <see cref="C(CancellationToken)"/>. But a warning is reported.</summary>
    public static async Task D(CancellationToken cancellationToken)
    {
        await using (MyAsyncLock resourceLock = new()) // CA2007: How to fix this?
        {
            IAsyncDisposable disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false);
            await using var _ = disposable.ConfigureAwait(false);
        }
    }

    /// <summary>No warning.</summary>
    /// <remarks>This seems like a lot of code to fix the warning.</remarks>
    public static async Task E(CancellationToken cancellationToken)
    {
        MyAsyncLock? resourceLock = new();

        try
        {
            IAsyncDisposable disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false);
            await using var _ = disposable.ConfigureAwait(false);
        }
        finally
        {
            if (resourceLock != null)
                await resourceLock.DisposeAsync().ConfigureAwait(false);
        }
    }

    /// <summary>Async lock implements <see cref="IAsyncDisposable"/>.</summary>
    private class MyAsyncLock : IAsyncDisposable
    {
        public Task<IAsyncDisposable> LockAsync(CancellationToken cancellationToken)
        {
            throw new NotImplementedException();
        }     

        public ValueTask DisposeAsync()
        {
            throw new NotImplementedException();
        }
    }
}

So my original issue is actually demonstrated in C3.C method.

PS: Sorry for the confusion with AsyncLock from NitoEx library. I thought that it is implemented as the lock we use in our project. It was meant to simplify the issue description.

[sharwell]

I struggle with both CA2000 and CA2007.

For CA2000, there is no known pattern which can be followed in new code to avoid both false positives (object correctly disposed but warning still reported) and false negatives (object not correctly disposed, but no warning reported). To me, the inability to write code without false positives renders the rule unusable and I would only ever recommend disabling it.

While CA2007 is still useful in limited shared library scenarios, Microsoft.VisualStudio.Threading renders the practice of calling ConfigureAwait(false) obsolete in many real-world projects. See this topic for more information on when ConfigureAwait is appropriate or can be avoided.

[jmarolf]

As @sharwell says CA2000 is not set to be a warning in code for a reason. I has a high false positive rate. In this case the correct thing to do here is:

#pragma warning disable CA2000 // CA2000 does not track transformations of IAsyncDisposable to ConfiguredAsyncDisposable
MyAsyncLock resourceLock = new();
#pragma warning restore CA2000 
IAsyncDisposable disposable = await resourceLock.LockAsync(cancellationToken).ConfigureAwait(false);
await using var _ = disposable.ConfigureAwait(false);

So there are two bugs here

• CA2000 does not track transformations of IAsyncDisposable to ConfiguredAsyncDisposable and thinks the original IAsyncDisposable is never disposed
• The codefix for CA2007 should change the types from IAsyncDisposable to ConfiguredAsyncDisposable when adding a ConfigureAwait call

[daviddunson]

I just upgraded to VS 2022 and opened an existing project and got CA2007 warning wanting me to add ConfigureAwait to SQLiteConnection.CreateCommand() which obviously is impossible.

await using var command = this.Connection.CreateCommand();

[sharwell]

@daviddunson Assuming CreateCommand() returns a type implementing IAsyncDisposable, the analyzer is expecting a call to TaskAsyncEnumerableExtensions.ConfigureAwait.

[MartyIX]

Personally, it's a bit sad that there is no way to fix the warnings gradually. Either one disables the analyzers or fixes everything. Most of the times, the former will be true, I guess.

edit: But then one can change the diagnostic level to INFO and that is probably what I will do. So taking that back.

[KenBrannigan]

Can someone confirm that when using say the MemoryStream this is the recommended approach to remove the CA2007 and suppress CA2000:

#pragma warning disable CA2000 // Dispose objects before losing scope
            var memoryStream = new MemoryStream(fileByteArray);
#pragma warning restore CA2000 // Dispose objects before losing scope
            await using var _ = memoryStream.ConfigureAwait(false);

Is there any better approach that doesn't require CA2000 to be suppressed? Is there any chance CA2000 will be adjusted so that it recognizes the fact that the objects are being disposed of in the next line?

Not that I like it but if the following approach is used would the DisposeAsync be called first making the second Dispose call be no-op:

using var memoryStream = new MemoryStream(fileByteArray);
await using var _ = memoryStream.ConfigureAwait(false);

[TimLovellSmith]

@KenBrannigan Howzat. https://github.com/dotnet/roslyn-analyzers/pull/5716

[daviddunson]

I have the same problem here when trying to download a file:

  using var client = new HttpClient();
  await using var sourceStream = await client.GetStreamAsync(requestUri).ConfigureAwait(true); // CA2007
  await using var targetStream = File.Create(imageFileName);; // CA2007
  await sourceStream.CopyToAsync(targetStream).ConfigureAwait(true);

Trying to determine the proper syntax is confusing and difficult to keep the code simplified. If I fall back to replacing the using with a try/finally, I can eliminate the CA2007; however, now I get a CA1508 false positive on target stream inside the finally block.

Stream sourceStream = null;
Stream targetStream = null;

try
{
    using var client = new HttpClient();
    sourceStream = await client.GetStreamAsync(requestUri).ConfigureAwait(true);
    targetStream =  File.Create(imageFileName);
    await sourceStream.CopyToAsync(targetStream).ConfigureAwait(true);
}
finally
{
    if (sourceStream != null)
    {
        await sourceStream.DisposeAsync().ConfigureAwait(true);
    }

    if (targetStream != null) // CA1508 false positive (client.GetStreamAsync may throw exception)
    {
        await targetStream.DisposeAsync().ConfigureAwait(true);
    }
}

edit: I'm aware I should move this to a helper method and use ConfigureAwait(false). The code is in my view model at the moment and I'm just fixing the warnings.

It's nice having these language helpers to simplify code; it sucks that we can't use them and constantly get caught in these catch 22 situations. I realize this is the wrong thread for reporting CA1508 bug, but the process for submitting a bug report is complicated and I don't have the time.

At least this is a work around for the CA2000/CA2007 conflict.