CA2213:Disposable fields should be disposed and support for `DisposeC…

…oreAsync` (#6976) * WIP * feedback * Remove VB test * Update src/Utilities/Compiler/Extensions/IMethodSymbolExtensions.cs Co-authored-by: Buyaa Namnan <buyankhishig.namnan@microsoft.com> --------- Co-authored-by: Buyaa Namnan <buyankhishig.namnan@microsoft.com>
dotnet · Nov 8, 2023 · b924542 · b924542
1 parent 91fda6d
commit b924542
Show file tree

Hide file tree

Showing 2 changed files with 107 additions and 2 deletions.
diff --git a/...rs/UnitTests/Microsoft.NetCore.Analyzers/Runtime/DisposableFieldsShouldBeDisposedTests.cs b/...rs/UnitTests/Microsoft.NetCore.Analyzers/Runtime/DisposableFieldsShouldBeDisposedTests.cs
@@ -3650,6 +3650,95 @@ End Class"
             }.RunAsync();
         }
 
+        [Fact]
+        public async Task DisposeCoreAsync_NoDiagnosticAsync()
+        {
+            await new VerifyCS.Test
+            {
+                ReferenceAssemblies = AdditionalMetadataReferences.DefaultWithAsyncInterfaces,
+                TestCode = @"
+using System;
+using System.Threading.Tasks;
+
+class A : IAsyncDisposable
+{
+    public ValueTask DisposeAsync()
+    {
+        return default(ValueTask);
+    }
+}
+
+// No diagnostic for DisposeCoreAsync.
+class B : IAsyncDisposable
+{
+    private readonly object disposedValueLock = new object();
+    private bool disposedValue;
+    private readonly A a;
+
+    public B() 
+    {
+        a = new A();
+    }
+
+    protected virtual async ValueTask DisposeCoreAsync()
+    {
+        lock (disposedValueLock)
+        {
+            if (disposedValue)
+            {
+                return;
+            }
+
+            disposedValue = true;
+        }
+
+        await a.DisposeAsync().ConfigureAwait(false);
+    }
+
+    public async ValueTask DisposeAsync()
+    {
+        await DisposeCoreAsync().ConfigureAwait(false);
+        GC.SuppressFinalize(this);
+    }
+}
+
+// No diagnostic for DisposeAsyncCore.
+class C : IAsyncDisposable
+{
+    private readonly object disposedValueLock = new object();
+    private bool disposedValue;
+    private readonly A a;
+
+    public C() 
+    {
+        a = new A();
+    }
+
+    protected virtual async ValueTask DisposeAsyncCore()
+    {
+        lock (disposedValueLock)
+        {
+            if (disposedValue)
+            {
+                return;
+            }
+
+            disposedValue = true;
+        }
+
+        await a.DisposeAsync().ConfigureAwait(false);
+    }
+
+    public async ValueTask DisposeAsync()
+    {
+        await DisposeAsyncCore().ConfigureAwait(false);
+        GC.SuppressFinalize(this);
+    }
+}
+"
+            }.RunAsync();
+        }
+
         [Fact, WorkItem(5099, "https://github.com/dotnet/roslyn-analyzers/issues/5099")]
         public async Task OwnDisposableButDoesNotOverrideDisposableMember_Dispose()
         {

diff --git a/src/Utilities/Compiler/Extensions/IMethodSymbolExtensions.cs b/src/Utilities/Compiler/Extensions/IMethodSymbolExtensions.cs
@@ -288,18 +288,30 @@ private static bool HasDisposeAsyncMethodSignature(this IMethodSymbol method,
         }
 
         /// <summary>
-        /// Checks if the given method has the signature "override Task DisposeCoreAsync(bool)".
+        /// Checks if the given method has the signature "override Task DisposeCoreAsync(bool)" or "override Task DisposeAsyncCore(bool)".
         /// </summary>
         private static bool HasOverriddenDisposeCoreAsyncMethodSignature(this IMethodSymbol method, [NotNullWhen(returnValue: true)] INamedTypeSymbol? task)
         {
-            return method.Name == "DisposeCoreAsync" &&
+            return (method.Name == "DisposeAsyncCore" || method.Name == "DisposeCoreAsync") &&
                 method.MethodKind == MethodKind.Ordinary &&
                 method.IsOverride &&
                 SymbolEqualityComparer.Default.Equals(method.ReturnType, task) &&
                 method.Parameters.Length == 1 &&
                 method.Parameters[0].Type.SpecialType == SpecialType.System_Boolean;
         }
 
+        /// <summary>
+        /// Checks if the given method has the signature "virtual ValueTask DisposeCoreAsync()" or "virtual ValueTask DisposeAsyncCore()".
+        /// </summary>
+        private static bool HasVirtualDisposeCoreAsyncMethodSignature(this IMethodSymbol method, [NotNullWhen(returnValue: true)] INamedTypeSymbol? valueTask)
+        {
+            return (method.Name == "DisposeAsyncCore" || method.Name == "DisposeCoreAsync") &&
+                method.MethodKind == MethodKind.Ordinary &&
+                method.IsVirtual &&
+                SymbolEqualityComparer.Default.Equals(method.ReturnType, valueTask) &&
+                method.Parameters.Length == 0;
+        }
+
         /// <summary>
         /// Gets the <see cref="DisposeMethodKind"/> for the given method.
         /// </summary>
@@ -351,6 +363,10 @@ public static DisposeMethodKind GetDisposeMethodKind(
                 {
                     return DisposeMethodKind.DisposeCoreAsync;
                 }
+                else if (method.HasVirtualDisposeCoreAsyncMethodSignature(valueTask))
+                {
+                    return DisposeMethodKind.DisposeCoreAsync;
+                }
                 else if (method.HasDisposeCloseMethodSignature())
                 {
                     return DisposeMethodKind.Close;