From 47c393f2a951d13670d64b36fc80af09f3b9ae3c Mon Sep 17 00:00:00 2001 From: myblindy Date: Fri, 2 Sep 2022 21:02:45 -0400 Subject: [PATCH 1/2] Tracks disposed objects through implicit interface implementations --- .../DisposableFieldsShouldBeDisposedTests.cs | 141 ++++++++++++++++++ .../Extensions/IMethodSymbolExtensions.cs | 19 ++- 2 files changed, 154 insertions(+), 6 deletions(-) diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/DisposableFieldsShouldBeDisposedTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/DisposableFieldsShouldBeDisposedTests.cs index 5d4464c717..b76092366e 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/DisposableFieldsShouldBeDisposedTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/DisposableFieldsShouldBeDisposedTests.cs @@ -651,6 +651,147 @@ End Function }.RunAsync(); } + [Fact, WorkItem(6075, "https://github.com/dotnet/roslyn-analyzers/issues/6075")] + public async Task AsyncDisposableDisposedInExplicitAsyncDisposable_Disposed_NoDiagnosticAsync() + { + await new VerifyCS.Test + { + ReferenceAssemblies = AdditionalMetadataReferences.DefaultWithAsyncInterfaces, + TestCode = @" +using System; +using System.IO; +using System.Net.Http; +using System.Threading.Tasks; + +class FileStream2 : IAsyncDisposable +{ + public ValueTask DisposeAsync() => default; +} + +public sealed class Test : IAsyncDisposable, IDisposable +{ + private readonly HttpClient client; + private readonly FileStream2 stream; + + public Test() + { + client = new HttpClient(); + stream = new FileStream2(); + } + + public void Dispose() + { + client.Dispose(); + } + + async ValueTask IAsyncDisposable.DisposeAsync() + { + await stream.DisposeAsync(); + } +} +" + }.RunAsync(); + + await new VerifyVB.Test + { + ReferenceAssemblies = AdditionalMetadataReferences.DefaultWithAsyncInterfaces, + TestCode = @" +Imports System +Imports System.IO +Imports System.Net.Http +Imports System.Threading.Tasks + +class FileStream2 + implements IAsyncDisposable + public function DisposeAsync() as ValueTask implements IAsyncDisposable.DisposeAsync + return nothing + end function +end class + +public class Test + implements IAsyncDisposable, IDisposable + + private readonly client as HttpClient + private readonly stream as FileStream2 + + public sub new() + client = new HttpClient + stream = new FileStream2 + end sub + + public sub Dispose() implements IDisposable.Dispose + client.Dispose() + end sub + + function DisposeAsync() as ValueTask implements IAsyncDisposable.DisposeAsync + return stream.DisposeAsync() + end function +end class +" + }.RunAsync(); + } + + [Fact, WorkItem(6075, "https://github.com/dotnet/roslyn-analyzers/issues/6075")] + public async Task DisposableDisposedInExplicitDisposable_Disposed_NoDiagnosticAsync() + { + await new VerifyCS.Test + { + ReferenceAssemblies = AdditionalMetadataReferences.DefaultWithAsyncInterfaces, + TestCode = @" +using System; +using System.IO; +using System.Net.Http; +using System.Threading.Tasks; + +public sealed class Test : IDisposable +{ + private readonly HttpClient client; + private readonly FileStream stream; + + public Test() + { + client = new HttpClient(); + stream = new FileStream(""C://some-path"", FileMode.CreateNew); + } + + void IDisposable.Dispose() + { + client.Dispose(); + stream.Dispose(); + } +} +" + }.RunAsync(); + + await new VerifyVB.Test + { + ReferenceAssemblies = AdditionalMetadataReferences.DefaultWithAsyncInterfaces, + TestCode = @" +Imports System +Imports System.IO +Imports System.Net.Http +Imports System.Threading.Tasks + +public class Test + implements IDisposable + + private readonly client as HttpClient + private readonly stream as FileStream + + public sub new() + client = new HttpClient + stream = new FileStream(""C://some-path"", FileMode.CreateNew) + end sub + + public sub Dispose() implements IDisposable.Dispose + client.Dispose() + stream.Dispose() + end sub +end class +" + }.RunAsync(); + } + [Fact] public async Task DisposableAllocation_AssignedThroughLocal_Disposed_NoDiagnosticAsync() { diff --git a/src/Utilities/Compiler/Extensions/IMethodSymbolExtensions.cs b/src/Utilities/Compiler/Extensions/IMethodSymbolExtensions.cs index 03f7a3c106..37c015a102 100644 --- a/src/Utilities/Compiler/Extensions/IMethodSymbolExtensions.cs +++ b/src/Utilities/Compiler/Extensions/IMethodSymbolExtensions.cs @@ -219,12 +219,19 @@ public static bool IsAsyncDisposeImplementation([NotNullWhen(returnValue: true)] method.IsImplementationOfInterfaceMethod(null, iAsyncDisposable, "DisposeAsync"); } + /// + /// Checks the name of the method of an implicit interface implementation (so, as is) or an explicit interface implementation (which also contains the interface as a prefix). + /// + private static bool IsImplicitOrExplicitInterfaceImplementationName(this IMethodSymbol method, string name, string interfacePrefix) + => (method.Name == name && method.MethodKind is MethodKind.Ordinary) || + (method.Name == $"{interfacePrefix}.{name}" && method.MethodKind is MethodKind.ExplicitInterfaceImplementation); + /// /// Checks if the given method has the signature "void Dispose()". /// private static bool HasDisposeMethodSignature(this IMethodSymbol method) { - return method.Name == "Dispose" && method.MethodKind == MethodKind.Ordinary && + return method.IsImplicitOrExplicitInterfaceImplementationName("Dispose", "System.IDisposable") && method.ReturnsVoid && method.Parameters.IsEmpty; } @@ -243,7 +250,7 @@ public static bool HasDisposeSignatureByConvention(this IMethodSymbol method) /// public static bool HasDisposeBoolMethodSignature(this IMethodSymbol method) { - if (method.Name == "Dispose" && method.MethodKind == MethodKind.Ordinary && + if (method.IsImplicitOrExplicitInterfaceImplementationName("Dispose", "System.IDisposable") && method.ReturnsVoid && method.Parameters.Length == 1) { IParameterSymbol parameter = method.Parameters[0]; @@ -260,7 +267,8 @@ public static bool HasDisposeBoolMethodSignature(this IMethodSymbol method) /// private static bool HasDisposeCloseMethodSignature(this IMethodSymbol method) { - return method.Name == "Close" && method.MethodKind == MethodKind.Ordinary && + return method.Name == "Close" && + method.MethodKind is MethodKind.Ordinary && method.ReturnsVoid && method.Parameters.IsEmpty; } @@ -278,8 +286,7 @@ private static bool HasDisposeAsyncMethodSignature(this IMethodSymbol method, INamedTypeSymbol? task, INamedTypeSymbol? valueTask) { - return method.Name == "DisposeAsync" && - method.MethodKind == MethodKind.Ordinary && + return method.IsImplicitOrExplicitInterfaceImplementationName("DisposeAsync", "System.IAsyncDisposable") && method.Parameters.IsEmpty && (SymbolEqualityComparer.Default.Equals(method.ReturnType, task) || SymbolEqualityComparer.Default.Equals(method.ReturnType, valueTask)); @@ -291,7 +298,7 @@ private static bool HasDisposeAsyncMethodSignature(this IMethodSymbol method, private static bool HasOverriddenDisposeCoreAsyncMethodSignature(this IMethodSymbol method, [NotNullWhen(returnValue: true)] INamedTypeSymbol? task) { return method.Name == "DisposeCoreAsync" && - method.MethodKind == MethodKind.Ordinary && + method.MethodKind is MethodKind.Ordinary && method.IsOverride && SymbolEqualityComparer.Default.Equals(method.ReturnType, task) && method.Parameters.Length == 1 && From 790bef2fdd98ae6b189107c84a44f46a85d18fc4 Mon Sep 17 00:00:00 2001 From: myblindy Date: Tue, 8 Nov 2022 14:09:31 -0500 Subject: [PATCH 2/2] alternative implementation, as well as an additional test for VB --- .../DisposableFieldsShouldBeDisposedTests.cs | 39 +++++++++++++++++++ .../Extensions/IMethodSymbolExtensions.cs | 21 ++++------ 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/DisposableFieldsShouldBeDisposedTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/DisposableFieldsShouldBeDisposedTests.cs index b76092366e..3b8aa6afc1 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/DisposableFieldsShouldBeDisposedTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/DisposableFieldsShouldBeDisposedTests.cs @@ -727,6 +727,45 @@ function DisposeAsync() as ValueTask implements IAsyncDisposable.DisposeAsync return stream.DisposeAsync() end function end class +" + }.RunAsync(); + + await new VerifyVB.Test + { + ReferenceAssemblies = AdditionalMetadataReferences.DefaultWithAsyncInterfaces, + TestCode = @" +Imports System +Imports System.IO +Imports System.Net.Http +Imports System.Threading.Tasks + +class FileStream2 + implements IAsyncDisposable + public function DisposeAsync() as ValueTask implements IAsyncDisposable.DisposeAsync + return nothing + end function +end class + +public class Test + implements IAsyncDisposable, IDisposable + + private readonly client as HttpClient + private readonly stream as FileStream2 + + public sub new() + client = new HttpClient + stream = new FileStream2 + end sub + + public sub Dispose() implements IDisposable.Dispose + client.Dispose() + end sub + + rem arbitrary implementation name + function DisposeOtherAsync() as ValueTask implements IAsyncDisposable.DisposeAsync + return stream.DisposeAsync() + end function +end class " }.RunAsync(); } diff --git a/src/Utilities/Compiler/Extensions/IMethodSymbolExtensions.cs b/src/Utilities/Compiler/Extensions/IMethodSymbolExtensions.cs index 37c015a102..3c9cd76252 100644 --- a/src/Utilities/Compiler/Extensions/IMethodSymbolExtensions.cs +++ b/src/Utilities/Compiler/Extensions/IMethodSymbolExtensions.cs @@ -219,19 +219,12 @@ public static bool IsAsyncDisposeImplementation([NotNullWhen(returnValue: true)] method.IsImplementationOfInterfaceMethod(null, iAsyncDisposable, "DisposeAsync"); } - /// - /// Checks the name of the method of an implicit interface implementation (so, as is) or an explicit interface implementation (which also contains the interface as a prefix). - /// - private static bool IsImplicitOrExplicitInterfaceImplementationName(this IMethodSymbol method, string name, string interfacePrefix) - => (method.Name == name && method.MethodKind is MethodKind.Ordinary) || - (method.Name == $"{interfacePrefix}.{name}" && method.MethodKind is MethodKind.ExplicitInterfaceImplementation); - /// /// Checks if the given method has the signature "void Dispose()". /// private static bool HasDisposeMethodSignature(this IMethodSymbol method) { - return method.IsImplicitOrExplicitInterfaceImplementationName("Dispose", "System.IDisposable") && + return method.Name == "Dispose" && method.MethodKind == MethodKind.Ordinary && method.ReturnsVoid && method.Parameters.IsEmpty; } @@ -250,7 +243,7 @@ public static bool HasDisposeSignatureByConvention(this IMethodSymbol method) /// public static bool HasDisposeBoolMethodSignature(this IMethodSymbol method) { - if (method.IsImplicitOrExplicitInterfaceImplementationName("Dispose", "System.IDisposable") && + if (method.Name == "Dispose" && method.MethodKind == MethodKind.Ordinary && method.ReturnsVoid && method.Parameters.Length == 1) { IParameterSymbol parameter = method.Parameters[0]; @@ -267,8 +260,7 @@ public static bool HasDisposeBoolMethodSignature(this IMethodSymbol method) /// private static bool HasDisposeCloseMethodSignature(this IMethodSymbol method) { - return method.Name == "Close" && - method.MethodKind is MethodKind.Ordinary && + return method.Name == "Close" && method.MethodKind == MethodKind.Ordinary && method.ReturnsVoid && method.Parameters.IsEmpty; } @@ -286,7 +278,8 @@ private static bool HasDisposeAsyncMethodSignature(this IMethodSymbol method, INamedTypeSymbol? task, INamedTypeSymbol? valueTask) { - return method.IsImplicitOrExplicitInterfaceImplementationName("DisposeAsync", "System.IAsyncDisposable") && + return method.Name == "DisposeAsync" && + method.MethodKind == MethodKind.Ordinary && method.Parameters.IsEmpty && (SymbolEqualityComparer.Default.Equals(method.ReturnType, task) || SymbolEqualityComparer.Default.Equals(method.ReturnType, valueTask)); @@ -298,7 +291,7 @@ private static bool HasDisposeAsyncMethodSignature(this IMethodSymbol method, private static bool HasOverriddenDisposeCoreAsyncMethodSignature(this IMethodSymbol method, [NotNullWhen(returnValue: true)] INamedTypeSymbol? task) { return method.Name == "DisposeCoreAsync" && - method.MethodKind is MethodKind.Ordinary && + method.MethodKind == MethodKind.Ordinary && method.IsOverride && SymbolEqualityComparer.Default.Equals(method.ReturnType, task) && method.Parameters.Length == 1 && @@ -344,7 +337,7 @@ public static DisposeMethodKind GetDisposeMethodKind( { return DisposeMethodKind.DisposeBool; } - else if (method.HasDisposeAsyncMethodSignature(task, valueTask)) + else if (method.IsAsyncDisposeImplementation(iAsyncDisposable, valueTask) || method.HasDisposeAsyncMethodSignature(task, valueTask)) { return DisposeMethodKind.DisposeAsync; }