From 7449c8c7af2b5bbf688390ecdf0682623a32195d Mon Sep 17 00:00:00 2001 From: Shen Chen Date: Thu, 30 May 2024 13:55:06 -0700 Subject: [PATCH 1/4] Add a symbol verification test --- .../SymbolKey/SymbolKeyCompilationsTests.cs | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/EditorFeatures/CSharpTest/SymbolKey/SymbolKeyCompilationsTests.cs b/src/EditorFeatures/CSharpTest/SymbolKey/SymbolKeyCompilationsTests.cs index 2864f54807b68..51430c5d8c820 100644 --- a/src/EditorFeatures/CSharpTest/SymbolKey/SymbolKeyCompilationsTests.cs +++ b/src/EditorFeatures/CSharpTest/SymbolKey/SymbolKeyCompilationsTests.cs @@ -164,6 +164,34 @@ public partial void M() { } Assert.Equal(implementation, ResolveSymbol(implementation, comp, SymbolKeyComparison.None)); } + [Fact] + public void ExtendedPartialPropertyDefinitionAndImplementationResolveCorrectly() + { + var src = """ + using System; + namespace NS + { + public partial class C1 + { + private int x; + public partial int Prop { get; set; } + public partial int Prop { get => x; set => x = value; } + } + } + """; + + var comp = (Compilation)CreateCompilation(src, assemblyName: "Test"); + + var ns = comp.SourceModule.GlobalNamespace.GetMembers("NS").Single() as INamespaceSymbol; + var type = ns.GetTypeMembers("C1").FirstOrDefault(); + var definition = type.GetMembers("Prop").First() as IPropertySymbol; + var implementation = definition.PartialImplementationPart; + + // Assert that both the definition and implementation resolve back to themselves + Assert.Equal(definition, ResolveSymbol(definition, comp, SymbolKeyComparison.None)); + Assert.Equal(implementation, ResolveSymbol(implementation, comp, SymbolKeyComparison.None)); + } + [Fact, WorkItem("http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/916341")] public void ExplicitIndexerImplementationResolvesCorrectly() { From fd83234d0864894a2071ac7e5d7c40ded5d8294b Mon Sep 17 00:00:00 2001 From: Shen Chen Date: Thu, 30 May 2024 13:59:35 -0700 Subject: [PATCH 2/4] Also check the property partial impl --- .../Portable/Debugging/AbstractBreakpointResolver.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Features/Core/Portable/Debugging/AbstractBreakpointResolver.cs b/src/Features/Core/Portable/Debugging/AbstractBreakpointResolver.cs index 3d03ff39d94a1..75229c8f43e08 100644 --- a/src/Features/Core/Portable/Debugging/AbstractBreakpointResolver.cs +++ b/src/Features/Core/Portable/Debugging/AbstractBreakpointResolver.cs @@ -211,7 +211,7 @@ private IEnumerable FindMembers(IEnumerable con private IEnumerable FindMembers(IEnumerable types, NameAndArity nameAndArity) { // Get the matching members from all types (including constructors and explicit interface - // implementations). If there is a partial method, prefer returning the implementation over + // implementations). If there is a partial method/property, prefer returning the implementation over // the definition (since the definition will not be a candidate for setting a breakpoint). var members = types.SelectMany(t => GetMembers(t, nameAndArity.Name)) .Select(s => GetPartialImplementationPartOrNull(s) ?? s); @@ -227,8 +227,12 @@ private async Task> GetAllTypesAsync(CancellationT return namespaces.GetAllTypes(cancellationToken); } - private static IMethodSymbol GetPartialImplementationPartOrNull(ISymbol symbol) - => (symbol.Kind == SymbolKind.Method) ? ((IMethodSymbol)symbol).PartialImplementationPart : null; + private static ISymbol GetPartialImplementationPartOrNull(ISymbol symbol) => symbol.Kind switch + { + SymbolKind.Method => ((IMethodSymbol)symbol).PartialImplementationPart, + SymbolKind.Property => ((IPropertySymbol)symbol).PartialImplementationPart, + _ => null + }; /// /// Is this method or property a valid place to set a breakpoint and does it match the expected parameter count? From 7256773d8693ca8fd60fc66e0cbeb0af71281687 Mon Sep 17 00:00:00 2001 From: Shen Chen Date: Thu, 30 May 2024 14:49:12 -0700 Subject: [PATCH 3/4] Check partial property in nav bar servivce --- .../CSharpNavigationBarItemService.cs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Features/CSharp/Portable/NavigationBar/CSharpNavigationBarItemService.cs b/src/Features/CSharp/Portable/NavigationBar/CSharpNavigationBarItemService.cs index df3ba6343c311..128f43f91cd0b 100644 --- a/src/Features/CSharp/Portable/NavigationBar/CSharpNavigationBarItemService.cs +++ b/src/Features/CSharp/Portable/NavigationBar/CSharpNavigationBarItemService.cs @@ -73,15 +73,22 @@ private static ImmutableArray GetMembersInTypes( continue; } - var method = member as IMethodSymbol; - if (method != null && method.PartialImplementationPart != null) + if (member is IMethodSymbol { PartialImplementationPart: { } } methodSymbol) { - memberItems.AddIfNotNull(CreateItemForMember(solution, method, tree, cancellationToken)); - memberItems.AddIfNotNull(CreateItemForMember(solution, method.PartialImplementationPart, tree, cancellationToken)); + memberItems.AddIfNotNull(CreateItemForMember(solution, methodSymbol, tree, cancellationToken)); + memberItems.AddIfNotNull(CreateItemForMember(solution, methodSymbol.PartialImplementationPart, tree, cancellationToken)); + } + else if (member is IPropertySymbol { PartialImplementationPart: { } } propertySymbol) + { + memberItems.AddIfNotNull(CreateItemForMember(solution, propertySymbol, tree, cancellationToken)); + memberItems.AddIfNotNull(CreateItemForMember(solution, propertySymbol.PartialImplementationPart, tree, cancellationToken)); } else { - Debug.Assert(method == null || method.PartialDefinitionPart == null, "NavBar expected GetMembers to return partial method definition parts but the implementation part was returned."); + if (member is IMethodSymbol { PartialDefinitionPart: null } or IPropertySymbol { PartialDefinitionPart: null }) + { + Debug.Fail($"NavBar expected GetMembers to return partial method/property definition parts but the implementation part was returned."); + } memberItems.AddIfNotNull(CreateItemForMember(solution, member, tree, cancellationToken)); } From dc70302f46512c1db61027310484b54da3f63e1b Mon Sep 17 00:00:00 2001 From: Shen Chen Date: Thu, 30 May 2024 15:31:50 -0700 Subject: [PATCH 4/4] Refactor the code --- .../NavigationBar/CSharpNavigationBarItemService.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Features/CSharp/Portable/NavigationBar/CSharpNavigationBarItemService.cs b/src/Features/CSharp/Portable/NavigationBar/CSharpNavigationBarItemService.cs index 128f43f91cd0b..bd8d73f27781c 100644 --- a/src/Features/CSharp/Portable/NavigationBar/CSharpNavigationBarItemService.cs +++ b/src/Features/CSharp/Portable/NavigationBar/CSharpNavigationBarItemService.cs @@ -83,15 +83,17 @@ private static ImmutableArray GetMembersInTypes( memberItems.AddIfNotNull(CreateItemForMember(solution, propertySymbol, tree, cancellationToken)); memberItems.AddIfNotNull(CreateItemForMember(solution, propertySymbol.PartialImplementationPart, tree, cancellationToken)); } - else + else if (member is IMethodSymbol or IPropertySymbol) { - if (member is IMethodSymbol { PartialDefinitionPart: null } or IPropertySymbol { PartialDefinitionPart: null }) - { - Debug.Fail($"NavBar expected GetMembers to return partial method/property definition parts but the implementation part was returned."); - } + Debug.Assert(member is IMethodSymbol { PartialDefinitionPart: null } or IPropertySymbol { PartialDefinitionPart: null }, + $"NavBar expected GetMembers to return partial method/property definition parts but the implementation part was returned."); memberItems.AddIfNotNull(CreateItemForMember(solution, member, tree, cancellationToken)); } + else + { + memberItems.AddIfNotNull(CreateItemForMember(solution, member, tree, cancellationToken)); + } } memberItems.Sort((x, y) =>