From f9212c28f04a5038497d12a822b0b9e194b7405a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 Nov 2024 13:51:15 -0800 Subject: [PATCH 1/3] improve goto-def on an invalid override --- .../CSharpGoToDefinitionTests.vb | 89 +++++++++++++++++++ .../Extensions/SemanticModelExtensions.cs | 2 +- .../Core/Extensions/ISymbolExtensions.cs | 64 ++++++++++++- .../Core/Extensions/ITypeSymbolExtensions.cs | 13 +-- 4 files changed, 160 insertions(+), 8 deletions(-) diff --git a/src/EditorFeatures/Test2/GoToDefinition/CSharpGoToDefinitionTests.vb b/src/EditorFeatures/Test2/GoToDefinition/CSharpGoToDefinitionTests.vb index 0e2d5bb68980a..5fc5369d1ed53 100644 --- a/src/EditorFeatures/Test2/GoToDefinition/CSharpGoToDefinitionTests.vb +++ b/src/EditorFeatures/Test2/GoToDefinition/CSharpGoToDefinitionTests.vb @@ -855,6 +855,95 @@ class C Await TestAsync(workspace) End Function + + Public Async Function TestCSharpGoToOverriddenDefinition_FromOverride_LooseMatch() As Task + Dim workspace = + + + +class C +{ + public virtual void [|F|](bool x) + { + } +} + +class D : C +{ + public $$override void F(int x) + { + } +} + + + + + Await TestAsync(workspace) + End Function + + + Public Async Function TestCSharpGoToOverriddenDefinition_FromOverride_LooseMatch2() As Task + Dim workspace = + + + +class C +{ + public virtual void F() + { + } + + public virtual void [|F|](bool x) + { + } +} + +class D : C +{ + public $$override void F(int x) + { + } +} + + + + + Await TestAsync(workspace) + End Function + + + Public Async Function TestCSharpGoToOverriddenDefinition_FromOverride_LooseMatch3() As Task + Dim workspace = + + + +class B +{ + public virtual void F(bool x) + { + } +} + +class C +{ + public virtual void [|F|]() + { + } +} + +class D : C +{ + public $$override void F(int x) + { + } +} + + + + + Await TestAsync(workspace) + End Function + Public Async Function TestCSharpGoToUnmanaged_Keyword() As Task Dim workspace = diff --git a/src/Workspaces/Core/Portable/Shared/Extensions/SemanticModelExtensions.cs b/src/Workspaces/Core/Portable/Shared/Extensions/SemanticModelExtensions.cs index 0866fcf4877c4..cfc7ffee17c3a 100644 --- a/src/Workspaces/Core/Portable/Shared/Extensions/SemanticModelExtensions.cs +++ b/src/Workspaces/Core/Portable/Shared/Extensions/SemanticModelExtensions.cs @@ -98,7 +98,7 @@ public static TokenSemanticInfo GetSemanticInfo( { // on an "override" token, we'll find the overridden symbol var overriddingSymbol = semanticFacts.GetDeclaredSymbol(semanticModel, overriddingIdentifier.Value, cancellationToken); - var overriddenSymbol = overriddingSymbol.GetOverriddenMember(); + var overriddenSymbol = overriddingSymbol.GetOverriddenMember(allowLooseMatch: true); allSymbols = overriddenSymbol is null ? [] : [overriddenSymbol]; } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs index 6a023012023e1..bd0f556a5c438 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs @@ -72,8 +72,12 @@ public static SymbolVisibility GetResultantVisibility(this ISymbol symbol) return visibility; } - public static ISymbol? GetOverriddenMember(this ISymbol? symbol) - => symbol switch + public static ISymbol? GetOverriddenMember(this ISymbol? symbol, bool allowLooseMatch = false) + { + if (symbol is null) + return null; + + ISymbol? exactMatch = symbol switch { IMethodSymbol method => method.OverriddenMethod, IPropertySymbol property => property.OverriddenProperty, @@ -81,6 +85,62 @@ public static SymbolVisibility GetResultantVisibility(this ISymbol symbol) _ => null, }; + if (exactMatch != null) + return exactMatch; + + if (allowLooseMatch) + { + foreach (var baseType in symbol.ContainingType.GetBaseTypes()) + { + if (TryFindLooseMatch(symbol, baseType, out var looseMatch)) + return looseMatch; + } + } + + return null; + + bool TryFindLooseMatch(ISymbol symbol, INamedTypeSymbol baseType, [NotNullWhen(true)] out ISymbol? looseMatch) + { + using var _ = ArrayBuilder.GetInstance(out var candidateMethods); + + foreach (var member in baseType.GetMembers(symbol.Name)) + { + if (member.Kind != symbol.Kind) + continue; + + if (member.IsSealed) + continue; + + if (!member.IsVirtual && !member.IsOverride && !member.IsAbstract) + continue; + + if (symbol.Kind is SymbolKind.Event or SymbolKind.Property) + { + // We've found a matching event/property in the base type (perhaps differing by return type). This + // is a good enough match to return as a loose match for the starting symbol. + looseMatch = member; + return true; + } + else if (member is IMethodSymbol method) + { + // We found a method. Keep track of this so we can try to find the best possible loose match. + candidateMethods.Add(method); + } + } + + var parameterCount = symbol.GetParameters().Length; + candidateMethods.Sort((m1, m2) => + { + var parameterDiff1 = Math.Abs(m1.Parameters.Length - parameterCount); + var parameterDiff2 = Math.Abs(m2.Parameters.Length - parameterCount); + return parameterDiff1 - parameterDiff2; + }); + + looseMatch = candidateMethods.FirstOrDefault(); + return looseMatch != null; + } + } + public static ImmutableArray ExplicitInterfaceImplementations(this ISymbol symbol) => symbol switch { diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ITypeSymbolExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ITypeSymbolExtensions.cs index c3db3f7a06779..4480ec0a11ff2 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ITypeSymbolExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ITypeSymbolExtensions.cs @@ -121,13 +121,16 @@ public static IEnumerable GetBaseTypesAndThis(this ITypeSymbol? typ } } - public static IEnumerable GetBaseTypes(this ITypeSymbol type) + public static IEnumerable GetBaseTypes(this ITypeSymbol? type) { - var current = type.BaseType; - while (current != null) + if (type is not null) { - yield return current; - current = current.BaseType; + var current = type.BaseType; + while (current != null) + { + yield return current; + current = current.BaseType; + } } } From ae3061fec0471e4715101f3f6aa4995dfde52259 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 Nov 2024 15:32:59 -0800 Subject: [PATCH 2/3] Simplify --- .../Compiler/Core/Extensions/ITypeSymbolExtensions.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ITypeSymbolExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ITypeSymbolExtensions.cs index 4480ec0a11ff2..1de694e2114dc 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ITypeSymbolExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ITypeSymbolExtensions.cs @@ -123,14 +123,11 @@ public static IEnumerable GetBaseTypesAndThis(this ITypeSymbol? typ public static IEnumerable GetBaseTypes(this ITypeSymbol? type) { - if (type is not null) + var current = type?.BaseType; + while (current != null) { - var current = type.BaseType; - while (current != null) - { - yield return current; - current = current.BaseType; - } + yield return current; + current = current.BaseType; } } From 23edf8dc6fff6ec8020d64a4b4c558bf354704b1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 Nov 2024 15:37:33 -0800 Subject: [PATCH 3/3] Simplify --- .../Core/Extensions/ISymbolExtensions.cs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs index bd0f556a5c438..d4e7dbb5e49a9 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions.cs @@ -101,7 +101,8 @@ public static SymbolVisibility GetResultantVisibility(this ISymbol symbol) bool TryFindLooseMatch(ISymbol symbol, INamedTypeSymbol baseType, [NotNullWhen(true)] out ISymbol? looseMatch) { - using var _ = ArrayBuilder.GetInstance(out var candidateMethods); + IMethodSymbol? bestMethod = null; + var parameterCount = symbol.GetParameters().Length; foreach (var member in baseType.GetMembers(symbol.Name)) { @@ -123,20 +124,13 @@ bool TryFindLooseMatch(ISymbol symbol, INamedTypeSymbol baseType, [NotNullWhen(t } else if (member is IMethodSymbol method) { - // We found a method. Keep track of this so we can try to find the best possible loose match. - candidateMethods.Add(method); + // Prefer methods that are closed in parameter count to the original method we started with. + if (bestMethod is null || Math.Abs(method.Parameters.Length - parameterCount) < Math.Abs(bestMethod.Parameters.Length - parameterCount)) + bestMethod = method; } } - var parameterCount = symbol.GetParameters().Length; - candidateMethods.Sort((m1, m2) => - { - var parameterDiff1 = Math.Abs(m1.Parameters.Length - parameterCount); - var parameterDiff2 = Math.Abs(m2.Parameters.Length - parameterCount); - return parameterDiff1 - parameterDiff2; - }); - - looseMatch = candidateMethods.FirstOrDefault(); + looseMatch = bestMethod; return looseMatch != null; } }