From f7ed68b12de24360f95f5aef667f60908c540ad3 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Thu, 10 Oct 2024 12:36:06 +0300 Subject: [PATCH 1/4] Improve quick info on `using` keyword --- .../QuickInfo/SemanticQuickInfoSourceTests.cs | 549 +++++++++++++++++- .../AbstractSemanticFactsService.cs | 61 +- 2 files changed, 592 insertions(+), 18 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs b/src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs index f76f1c3e52c30..f65586109000d 100644 --- a/src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs +++ b/src/EditorFeatures/CSharpTest/QuickInfo/SemanticQuickInfoSourceTests.cs @@ -83,17 +83,38 @@ private static async Task TestWithOptionsAsync(Document document, QuickInfoServi } } - private static async Task VerifyWithMscorlib45Async(string markup, Action[] expectedResults) + private static async Task VerifyWithMscorlib45Async(string markup, params Action[] expectedResults) { - var xmlString = string.Format(@" - - - -{0} - - -", SecurityElement.Escape(markup)); + var xmlString = string.Format(""" + + + + {0} + + + + """, SecurityElement.Escape(markup)); + + await VerifyWithMarkupAsync(xmlString, expectedResults); + } + + private static async Task VerifyWithNet8Async(string markup, params Action[] expectedResults) + { + var xmlString = string.Format(""" + + + + {0} + + + + """, SecurityElement.Escape(markup)); + await VerifyWithMarkupAsync(xmlString, expectedResults); + } + + private static async Task VerifyWithMarkupAsync(string xmlString, Action[] expectedResults) + { using var workspace = EditorTestWorkspace.Create(xmlString); var sourceDocument = workspace.Documents.Single(d => d.Name == "SourceDocument"); var position = sourceDocument.CursorPosition!.Value; @@ -4950,7 +4971,7 @@ async Task Goo() }"; var description = $"({CSharpFeaturesResources.awaitable}) Task C.Goo()"; - await VerifyWithMscorlib45Async(markup, new[] { MainDescription(description) }); + await VerifyWithMscorlib45Async(markup, MainDescription(description)); } [Fact] @@ -8851,4 +8872,512 @@ await VerifyWithMscorlib45Async(markup, new[] 'a {FeaturesResources.is_} new {{ string @string }}") }); } + + [Theory, CombinatorialData] + public async Task UsingStatement_Class(bool simpleUsing, bool implementsIDisposable) + { + var usingStatement = simpleUsing + ? "$$using var c = new C();" + : "$$using (var c = new C()) { }"; + + // When class doesn't implement 'IDisposable' a compiler error is produced. + // However, we still want to show a specific 'Dispose' method for error recovery + // since that would be the picked method when user fixes the error + await TestAsync($$""" + using System; + + class C {{(implementsIDisposable ? ": IDisposable" : "")}} + { + public void Dispose() + { + } + + void M() + { + {{usingStatement}} + } + } + """, + MainDescription("void C.Dispose()")); + } + + [Theory, CombinatorialData] + public async Task UsingStatement_Class_DisposeMethodInBaseType(bool simpleUsing, bool implementsIDisposable) + { + var usingStatement = simpleUsing + ? "$$using var c = new C();" + : "$$using (var c = new C()) { }"; + + // When class doesn't implement 'IDisposable' a compiler error is produced. + // However, we still want to show a specific 'Dispose' method for error recovery + // since that would be the picked method when user fixes the error + await TestAsync($$""" + using System; + + class CBase {{(implementsIDisposable ? ": IDisposable" : "")}} + { + public void Dispose() + { + } + } + + class C : CBase + { + void M() + { + {{usingStatement}} + } + } + """, + MainDescription("void CBase.Dispose()")); + } + + [Theory, CombinatorialData] + public async Task UsingStatement_Class_ExplicitImplementationAndPattern(bool simpleUsing) + { + var usingStatement = simpleUsing + ? "$$using var c = new C();" + : "$$using (var c = new C()) { }"; + + await TestAsync($$""" + using System; + + class C : IDisposable + { + void IDisposable.Dispose() + { + } + + public void Dispose() + { + } + + void M() + { + {{usingStatement}} + } + } + """, + MainDescription("void C.Dispose()")); + } + + [Theory, CombinatorialData] + public async Task UsingStatement_Class_ImplementsIDisposableButDoesNotHaveDisposeMethod(bool simpleUsing) + { + var usingStatement = simpleUsing + ? "$$using var c = new C();" + : "$$using (var c = new C()) { }"; + + await TestAsync($$""" + using System; + + class C : IDisposable + { + void M() + { + {{usingStatement}} + } + } + """, + MainDescription("void IDisposable.Dispose()")); + } + + [Theory, CombinatorialData] + public async Task UsingStatement_Class_DoesNotImplementIDisposableAndDoesNotHaveDisposeMethod(bool simpleUsing) + { + var usingStatement = simpleUsing + ? "$$using var c = new C();" + : "$$using (var c = new C()) { }"; + + await TestAsync($$""" + using System; + + class C + { + void M() + { + {{usingStatement}} + } + } + """); + } + + [Theory, CombinatorialData] + public async Task UsingStatement_Struct(bool simpleUsing, bool isRefStruct, bool implementsIDisposable) + { + var usingStatement = simpleUsing + ? "$$using var s = new S();" + : "$$using (var s = new S()) { }"; + + // When non-ref struct doesn't implement 'IDisposable' a compiler error is produced. + // However, we still want to show a specific 'Dispose' method for error recovery + // since that would be the picked method when user fixes the error + await TestAsync($$""" + using System; + + {{(isRefStruct ? "ref" : "")}} struct S {{(implementsIDisposable ? ": IDisposable" : "")}} + { + public void Dispose() + { + } + + void M() + { + {{usingStatement}} + } + } + """, + MainDescription("void S.Dispose()")); + } + + [Theory, CombinatorialData] + public async Task UsingStatement_Struct_ExplicitImplementationAndPattern(bool simpleUsing) + { + var usingStatement = simpleUsing + ? "$$using var s = new S();" + : "$$using (var s = new S()) { }"; + + await TestAsync($$""" + using System; + + struct S : IDisposable + { + void IDisposable.Dispose() + { + } + + public void Dispose() + { + } + + void M() + { + {{usingStatement}} + } + } + """, + MainDescription("void S.Dispose()")); + } + + [Theory, CombinatorialData] + public async Task UsingStatement_Struct_ImplementsIDisposableButDoesNotHaveDisposeMethod(bool simpleUsing) + { + var usingStatement = simpleUsing + ? "$$using var s = new S();" + : "$$using (var s = new S()) { }"; + + await TestAsync($$""" + using System; + + struct S : IDisposable + { + void M() + { + {{usingStatement}} + } + } + """, + MainDescription("void IDisposable.Dispose()")); + } + + [Theory, CombinatorialData] + public async Task UsingStatement_Struct_DoesNotImplementIDisposableAndDoesNotHaveDisposeMethod(bool simpleUsing) + { + var usingStatement = simpleUsing + ? "$$using var s = new S();" + : "$$using (var s = new S()) { }"; + + await TestAsync($$""" + using System; + + struct S + { + void M() + { + {{usingStatement}} + } + } + """); + } + + [Fact] + public async Task UsingStatement_Interface() + { + await TestAsync(""" + using System; + + interface IMyInterface : IDisposable + { + } + + class C + { + void M(IMyInterface i) + { + $$using (i) + { + } + } + } + """, + MainDescription("void IDisposable.Dispose()")); + } + + [Theory, CombinatorialData] + public async Task AwaitUsingStatement_Class(bool simpleUsing, bool implementsIAsyncDisposable) + { + var usingStatement = simpleUsing + ? "await $$using var c = new C();" + : "await $$using (var c = new C()) { }"; + + // When class doesn't implement 'IAsyncDisposable' a compiler error is produced. + // However, we still want to show a specific 'DisposeAsync' method for error recovery + // since that would be the picked method when user fixes the error + await VerifyWithNet8Async($$""" + using System; + using System.Threading.Tasks; + + class C {{(implementsIAsyncDisposable ? ": IAsyncDisposable" : "")}} + { + public ValueTask DisposeAsync() + { + } + + void M() + { + {{usingStatement}} + } + } + """, + MainDescription($"({CSharpFeaturesResources.awaitable}) ValueTask C.DisposeAsync()")); + } + + [Theory, CombinatorialData] + public async Task AwaitUsingStatement_Class_DisposeMethodInBaseType(bool simpleUsing, bool implementsIAsyncDisposable) + { + var usingStatement = simpleUsing + ? "await $$using var c = new C();" + : "await $$using (var c = new C()) { }"; + + // When class doesn't implement 'IAsyncDisposable' a compiler error is produced. + // However, we still want to show a specific 'DisposeAsync' method for error recovery + // since that would be the picked method when user fixes the error + await VerifyWithNet8Async($$""" + using System; + using System.Threading.Tasks; + + class CBase {{(implementsIAsyncDisposable ? ": IAsyncDisposable" : "")}} + { + public ValueTask DisposeAsync() + { + } + } + + class C : CBase + { + void M() + { + {{usingStatement}} + } + } + """, + MainDescription($"({CSharpFeaturesResources.awaitable}) ValueTask CBase.DisposeAsync()")); + } + + [Theory, CombinatorialData] + public async Task AwaitUsingStatement_Class_ExplicitImplementationAndPattern(bool simpleUsing) + { + var usingStatement = simpleUsing + ? "await $$using var c = new C();" + : "await $$using (var c = new C()) { }"; + + await VerifyWithNet8Async($$""" + using System; + using System.Threading.Tasks; + + class C : IAsyncDisposable + { + ValueTask IAsyncDisposable.DisposeAsync() + { + } + + public void DisposeAsync() + { + } + + void M() + { + {{usingStatement}} + } + } + """, + MainDescription($"({CSharpFeaturesResources.awaitable}) ValueTask C.DisposeAsync()")); + } + + [Theory, CombinatorialData] + public async Task AwaitUsingStatement_Class_ImplementsIAsyncDisposableButDoesNotHaveDisposeAsyncMethod(bool simpleUsing) + { + var usingStatement = simpleUsing + ? "await $$using var c = new C();" + : "await $$using (var c = new C()) { }"; + + await VerifyWithNet8Async($$""" + using System; + using System.Threading.Tasks; + + class C : IAsyncDisposable + { + void M() + { + {{usingStatement}} + } + } + """, + MainDescription($"({CSharpFeaturesResources.awaitable}) ValueTask IAsyncDisposable.DisposeAsync()")); + } + + [Theory, CombinatorialData] + public async Task AwaitUsingStatement_Class_DoesNotImplementIAsyncDisposableAndDoesNotHaveDisposeAsyncMethod(bool simpleUsing) + { + var usingStatement = simpleUsing + ? "await $$using var c = new C();" + : "await $$using (var c = new C()) { }"; + + await VerifyWithNet8Async($$""" + using System; + + class C + { + void M() + { + {{usingStatement}} + } + } + """); + } + + [Theory, CombinatorialData] + public async Task AwaitUsingStatement_Struct(bool simpleUsing, bool isRefStruct, bool implementsIAsyncDisposable) + { + var usingStatement = simpleUsing + ? "await $$using var s = new S();" + : "await $$using (var s = new S()) { }"; + + // When non-ref struct doesn't implement 'IAsyncDisposable' a compiler error is produced. + // However, we still want to show a specific 'DisposeAsync' method for error recovery + // since that would be the picked method when user fixes the error + await VerifyWithNet8Async($$""" + using System; + using System.Threading.Tasks; + + {{(isRefStruct ? "ref" : "")}} struct S {{(implementsIAsyncDisposable ? ": IAsyncDisposable" : "")}} + { + public ValueTask DisposeAsync() + { + } + + void M() + { + {{usingStatement}} + } + } + """, + MainDescription($"({CSharpFeaturesResources.awaitable}) ValueTask S.DisposeAsync()")); + } + + [Theory, CombinatorialData] + public async Task AwaitUsingStatement_Struct_ExplicitImplementationAndPattern(bool simpleUsing) + { + var usingStatement = simpleUsing + ? "await $$using var s = new S();" + : "await $$using (var s = new S()) { }"; + + await VerifyWithNet8Async($$""" + using System; + using System.Threading.Tasks; + + struct S : IAsyncDisposable + { + ValueTask IAsyncDisposable.DisposeAsync() + { + } + + public ValueTask DisposeAsync() + { + } + + void M() + { + {{usingStatement}} + } + } + """, + MainDescription($"({CSharpFeaturesResources.awaitable}) ValueTask S.DisposeAsync()")); + } + + [Theory, CombinatorialData] + public async Task AwaitUsingStatement_Struct_ImplementsIAsyncDisposableButDoesNotHaveDisposeAsyncMethod(bool simpleUsing) + { + var usingStatement = simpleUsing + ? "await $$using var s = new S();" + : "await $$using (var s = new S()) { }"; + + await VerifyWithNet8Async($$""" + using System; + using System.Threading.Tasks; + + struct S : IAsyncDisposable + { + void M() + { + {{usingStatement}} + } + } + """, + MainDescription($"({CSharpFeaturesResources.awaitable}) ValueTask IAsyncDisposable.DisposeAsync()")); + } + + [Theory, CombinatorialData] + public async Task AwaitUsingStatement_Struct_DoesNotImplementIAsyncDisposableAndDoesNotHaveDisposeAsyncMethod(bool simpleUsing) + { + var usingStatement = simpleUsing + ? "await $$using var s = new S();" + : "await $$using (var s = new S()) { }"; + + await VerifyWithNet8Async($$""" + using System; + + struct S + { + void M() + { + {{usingStatement}} + } + } + """); + } + + [Fact] + public async Task AwaitUsingStatement_Interface() + { + await VerifyWithNet8Async(""" + using System; + using System.Threading.Tasks; + + interface IMyInterface : IAsyncDisposable + { + } + + class C + { + void M(IMyInterface i) + { + await $$using (i) + { + } + } + } + """, + MainDescription($"({CSharpFeaturesResources.awaitable}) ValueTask IAsyncDisposable.DisposeAsync()")); + } } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs index 8485bafe8da52..d8b87b16e2a91 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs @@ -105,19 +105,64 @@ public SyntaxToken GenerateUniqueName(string baseName, IEnumerable usedN if (type is null) return null; - var methodToLookFor = isAsync - ? GetDisposeMethod(typeof(IAsyncDisposable).FullName!, nameof(IAsyncDisposable.DisposeAsync)) - : GetDisposeMethod(typeof(IDisposable).FullName!, nameof(IDisposable.Dispose)); - if (methodToLookFor is null) + var (iDisposableInterfaceType, disposeMethodToLookFor) = isAsync + ? GetIDisposableInterfaceAndDisposeMethod(typeof(IAsyncDisposable).FullName!, nameof(IAsyncDisposable.DisposeAsync)) + : GetIDisposableInterfaceAndDisposeMethod(typeof(IDisposable).FullName!, nameof(IDisposable.Dispose)); + if (disposeMethodToLookFor is null) return null; - var impl = type.FindImplementationForInterfaceMember(methodToLookFor) ?? methodToLookFor; - return impl as IMethodSymbol; + var impl = type.FindImplementationForInterfaceMember(disposeMethodToLookFor); + if (impl is IMethodSymbol implMethod) + { + return implMethod; + } + + // If we didn't find implementation for the method + // look for the method with the right signature. + // This will help with error recovery and produce correct result + // for the case with a pattern-based using statement on `ref struct`s + if (isAsync) + { + var valueTaskType = compilation.ValueTaskType(); + + var currentType = type; + while (currentType is not null) + { + if (currentType + .GetMembers(nameof(IAsyncDisposable.DisposeAsync)) + .FirstOrDefault(m => m is IMethodSymbol { DeclaredAccessibility: Accessibility.Public, ReturnType: var returnType, Parameters.Length: 0 } && + SymbolEqualityComparer.Default.Equals(returnType, valueTaskType)) is { } disposeMethodFromPattern) + { + return (IMethodSymbol)disposeMethodFromPattern; + } + + currentType = currentType.BaseType; + } + } + else + { + var currentType = type; + while (currentType is not null) + { + if (currentType + .GetMembers(nameof(IDisposable.Dispose)) + .FirstOrDefault(m => m is IMethodSymbol { DeclaredAccessibility: Accessibility.Public, ReturnsVoid: true, Parameters.Length: 0 }) is { } disposeMethodFromPattern) + { + return (IMethodSymbol)disposeMethodFromPattern; + } + + currentType = currentType.BaseType; + } + } + + // If type doesn't implement disposable interface and doesn't have dispose method with right shape + // return null rather than completely unrelated dispose method for that type + return type.Implements(iDisposableInterfaceType!) ? disposeMethodToLookFor : null; - IMethodSymbol? GetDisposeMethod(string typeName, string methodName) + (INamedTypeSymbol?, IMethodSymbol?) GetIDisposableInterfaceAndDisposeMethod(string typeName, string methodName) { var disposableType = compilation.GetBestTypeByMetadataName(typeName); - return disposableType?.GetMembers().OfType().FirstOrDefault(m => m.Parameters.Length == 0 && m.Name == methodName); + return (disposableType, disposableType?.GetMembers().OfType().FirstOrDefault(m => m.Parameters.Length == 0 && m.Name == methodName)); } } From 0bac6fb7a43223ac2fbe94fb59490951b1acf58e Mon Sep 17 00:00:00 2001 From: DoctorKrolic <70431552+DoctorKrolic@users.noreply.github.com> Date: Thu, 10 Oct 2024 21:00:12 +0300 Subject: [PATCH 2/4] Simplify Co-authored-by: Cyrus Najmabadi --- .../SemanticsFactsService/AbstractSemanticFactsService.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs index d8b87b16e2a91..43e276157d361 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs @@ -131,9 +131,9 @@ public SyntaxToken GenerateUniqueName(string baseName, IEnumerable usedN if (currentType .GetMembers(nameof(IAsyncDisposable.DisposeAsync)) .FirstOrDefault(m => m is IMethodSymbol { DeclaredAccessibility: Accessibility.Public, ReturnType: var returnType, Parameters.Length: 0 } && - SymbolEqualityComparer.Default.Equals(returnType, valueTaskType)) is { } disposeMethodFromPattern) + SymbolEqualityComparer.Default.Equals(returnType, valueTaskType)) is IMethodSymbol disposeMethodFromPattern) { - return (IMethodSymbol)disposeMethodFromPattern; + return disposeMethodFromPattern; } currentType = currentType.BaseType; From 007bb4895f96176b4eed71d3985f2c7b88238ec3 Mon Sep 17 00:00:00 2001 From: DoctorKrolic <70431552+DoctorKrolic@users.noreply.github.com> Date: Thu, 10 Oct 2024 21:00:34 +0300 Subject: [PATCH 3/4] Simplify Co-authored-by: Cyrus Najmabadi --- .../SemanticsFactsService/AbstractSemanticFactsService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs index 43e276157d361..a16ddb5d77e8d 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs @@ -146,7 +146,7 @@ public SyntaxToken GenerateUniqueName(string baseName, IEnumerable usedN { if (currentType .GetMembers(nameof(IDisposable.Dispose)) - .FirstOrDefault(m => m is IMethodSymbol { DeclaredAccessibility: Accessibility.Public, ReturnsVoid: true, Parameters.Length: 0 }) is { } disposeMethodFromPattern) + .FirstOrDefault(m => m is IMethodSymbol { DeclaredAccessibility: Accessibility.Public, ReturnsVoid: true, Parameters.Length: 0 }) is IMethodSymbol disposeMethodFromPattern) { return (IMethodSymbol)disposeMethodFromPattern; } From d77f2dcabafc32562aca814bada9d07cbfcb2b2d Mon Sep 17 00:00:00 2001 From: DoctorKrolic <70431552+DoctorKrolic@users.noreply.github.com> Date: Thu, 10 Oct 2024 21:00:51 +0300 Subject: [PATCH 4/4] Simplify Co-authored-by: Cyrus Najmabadi --- .../SemanticsFactsService/AbstractSemanticFactsService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs index a16ddb5d77e8d..ee1453718264f 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/LanguageServices/SemanticsFactsService/AbstractSemanticFactsService.cs @@ -148,7 +148,7 @@ public SyntaxToken GenerateUniqueName(string baseName, IEnumerable usedN .GetMembers(nameof(IDisposable.Dispose)) .FirstOrDefault(m => m is IMethodSymbol { DeclaredAccessibility: Accessibility.Public, ReturnsVoid: true, Parameters.Length: 0 }) is IMethodSymbol disposeMethodFromPattern) { - return (IMethodSymbol)disposeMethodFromPattern; + return disposeMethodFromPattern; } currentType = currentType.BaseType;