Fix goto-def on partial methods in LSP

src/EditorFeatures/Core.Wpf/Peek/PeekableItemFactory.cs

@@ -61,7 +61,7 @@ public async Task<IEnumerable<IPeekableItem>> GetPeekableItemsAsync(
 
             var solution = project.Solution;
             symbol = await SymbolFinder.FindSourceDefinitionAsync(symbol, solution, cancellationToken).ConfigureAwait(false) ?? symbol;
-            symbol = await GoToDefinitionHelpers.TryGetPreferredSymbolAsync(solution, symbol, cancellationToken).ConfigureAwait(false);
+            symbol = await GoToDefinitionFeatureHelpers.TryGetPreferredSymbolAsync(solution, symbol, cancellationToken).ConfigureAwait(false);
             if (symbol is null)
                 return ImmutableArray<IPeekableItem>.Empty;
 

src/EditorFeatures/Core/GoToDefinition/AbstractGoToDefinitionService.cs

@@ -125,7 +125,7 @@ protected AbstractAsyncGoToDefinitionService(
             using var _ = ArrayBuilder<DefinitionItem>.GetInstance(out var builder);
             foreach (var impl in interfaceImpls)
             {
-                builder.AddRange(await GoToDefinitionHelpers.GetDefinitionsAsync(
+                builder.AddRange(await GoToDefinitionFeatureHelpers.GetDefinitionsAsync(
                     impl, solution, thirdPartyNavigationAllowed: false, cancellationToken).ConfigureAwait(false));
             }
 

src/EditorFeatures/Core/GoToDefinition/GoToDefinitionHelpers.cs

@@ -3,109 +3,18 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Collections.Generic;
-using System.Collections.Immutable;
-using System.Diagnostics.CodeAnalysis;
-using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Editor.Host;
 using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
-using Microsoft.CodeAnalysis.FindSymbols;
 using Microsoft.CodeAnalysis.FindUsages;
 using Microsoft.CodeAnalysis.Navigation;
-using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Extensions;
-using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.GoToDefinition
 {
     internal static class GoToDefinitionHelpers
     {
-        public static async Task<ImmutableArray<DefinitionItem>> GetDefinitionsAsync(
-            ISymbol? symbol,
-            Solution solution,
-            bool thirdPartyNavigationAllowed,
-            CancellationToken cancellationToken)
-        {
-            symbol = await TryGetPreferredSymbolAsync(solution, symbol, cancellationToken).ConfigureAwait(false);
-            if (symbol is null)
-                return ImmutableArray.Create<DefinitionItem>();
-
-            using var _ = ArrayBuilder<DefinitionItem>.GetInstance(out var definitions);
-
-            // Going to a symbol may end up actually showing the symbol in the Find-Usages window.
-            // This happens when there is more than one location for the symbol (i.e. for partial
-            // symbols) and we don't know the best place to take you to.
-            //
-            // The FindUsages window supports showing the classified text for an item.  It does this
-            // in two ways.  Either the item can pass along its classified text (and the window will
-            // defer to that), or the item will have no classified text, and the window will compute
-            // it in the BG.
-            //
-            // Passing along the classified information is valuable for OOP scenarios where we want
-            // all that expensive computation done on the OOP side and not in the VS side.
-            //
-            // However, Go To Definition is all in-process, and is also synchronous.  So we do not
-            // want to fetch the classifications here.  It slows down the command and leads to a
-            // measurable delay in our perf tests.
-            //
-            // So, if we only have a single location to go to, this does no unnecessary work.  And,
-            // if we do have multiple locations to show, it will just be done in the BG, unblocking
-            // this command thread so it can return the user faster.
-            var definitionItem = symbol.ToNonClassifiedDefinitionItem(solution, includeHiddenLocations: true);
-
-            if (thirdPartyNavigationAllowed)
-            {
-                var factory = solution.Services.GetService<IDefinitionsAndReferencesFactory>();
-                if (factory != null)
-                {
-                    var thirdPartyItem = await factory.GetThirdPartyDefinitionItemAsync(solution, definitionItem, cancellationToken).ConfigureAwait(false);
-                    definitions.AddIfNotNull(thirdPartyItem);
-                }
-            }
-
-            definitions.Add(definitionItem);
-            return definitions.ToImmutable();
-        }
-
-        public static async Task<ISymbol?> TryGetPreferredSymbolAsync(
-            Solution solution, ISymbol? symbol, CancellationToken cancellationToken)
-        {
-            // VB global import aliases have a synthesized SyntaxTree.
-            // We can't go to the definition of the alias, so use the target type.
-
-            var alias = symbol as IAliasSymbol;
-            if (alias != null)
-            {
-                if (alias.Target is INamespaceSymbol ns && ns.IsGlobalNamespace)
-                    return null;
-            }
-
-            // VB global import aliases have a synthesized SyntaxTree.
-            // We can't go to the definition of the alias, so use the target type.
-
-            if (alias != null)
-            {
-                var sourceLocations = NavigableItemFactory.GetPreferredSourceLocations(
-                    solution, symbol, cancellationToken);
-
-                if (sourceLocations.All(l => solution.GetDocument(l.SourceTree) == null))
-                    symbol = alias.Target;
-            }
-
-            var definition = await SymbolFinder.FindSourceDefinitionAsync(symbol, solution, cancellationToken).ConfigureAwait(false);
-            cancellationToken.ThrowIfCancellationRequested();
-
-            symbol = definition ?? symbol;
-
-            // If it is a partial method declaration with no body, choose to go to the implementation
-            // that has a method body.
-            if (symbol is IMethodSymbol method)
-                symbol = method.PartialImplementationPart ?? symbol;
-
-            return symbol;
-        }
-
         public static async Task<bool> TryNavigateToLocationAsync(
             ISymbol symbol,
             Solution solution,
@@ -131,7 +40,8 @@ public static async Task<bool> TryNavigateToLocationAsync(
             var title = string.Format(EditorFeaturesResources._0_declarations,
                 FindUsagesHelpers.GetDisplayName(symbol));
 
-            var definitions = await GetDefinitionsAsync(symbol, solution, thirdPartyNavigationAllowed, cancellationToken).ConfigureAwait(false);
+            var definitions = await GoToDefinitionFeatureHelpers.GetDefinitionsAsync(
+                symbol, solution, thirdPartyNavigationAllowed, cancellationToken).ConfigureAwait(false);
 
             return await streamingPresenter.GetStreamingLocationAsync(
                 threadingContext, solution.Workspace, title, definitions, cancellationToken).ConfigureAwait(false);

src/Features/Core/Portable/GoToDefinition/AbstractFindDefinitionService.cs

@@ -20,11 +20,13 @@ public async Task<ImmutableArray<INavigableItem>> FindDefinitionsAsync(
             var symbolService = document.GetRequiredLanguageService<IGoToDefinitionSymbolService>();
             var (symbol, project, _) = await symbolService.GetSymbolProjectAndBoundSpanAsync(document, position, includeType: true, cancellationToken).ConfigureAwait(false);
 
-            symbol = await SymbolFinder.FindSourceDefinitionAsync(symbol, project.Solution, cancellationToken).ConfigureAwait(false) ?? symbol;
+            var solution = project.Solution;
+            symbol = await SymbolFinder.FindSourceDefinitionAsync(symbol, solution, cancellationToken).ConfigureAwait(false) ?? symbol;
+            symbol = await GoToDefinitionFeatureHelpers.TryGetPreferredSymbolAsync(solution, symbol, cancellationToken).ConfigureAwait(false);
 
             // Try to compute source definitions from symbol.
             return symbol != null
-                ? NavigableItemFactory.GetItemsFromPreferredSourceLocations(project.Solution, symbol, displayTaggedParts: FindUsagesHelpers.GetDisplayParts(symbol), cancellationToken: cancellationToken)
+                ? NavigableItemFactory.GetItemsFromPreferredSourceLocations(solution, symbol, displayTaggedParts: FindUsagesHelpers.GetDisplayParts(symbol), cancellationToken: cancellationToken)
                 : ImmutableArray<INavigableItem>.Empty;
         }
     }

src/Features/Core/Portable/GoToDefinition/GoToDefinitionFeatureHelpers.cs

@@ -0,0 +1,106 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Linq;
+using System.Text;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.FindSymbols;
+using Microsoft.CodeAnalysis.FindUsages;
+using Microsoft.CodeAnalysis.Navigation;
+using Microsoft.CodeAnalysis.PooledObjects;
+
+namespace Microsoft.CodeAnalysis.GoToDefinition
+{
+    internal static class GoToDefinitionFeatureHelpers
+    {
+        public static async Task<ISymbol?> TryGetPreferredSymbolAsync(
+            Solution solution, ISymbol? symbol, CancellationToken cancellationToken)
+        {
+            // VB global import aliases have a synthesized SyntaxTree.
+            // We can't go to the definition of the alias, so use the target type.
+
+            var alias = symbol as IAliasSymbol;
+            if (alias != null)
+            {
+                if (alias.Target is INamespaceSymbol ns && ns.IsGlobalNamespace)
+                    return null;
+            }
+
+            // VB global import aliases have a synthesized SyntaxTree.
+            // We can't go to the definition of the alias, so use the target type.
+
+            if (alias != null)
+            {
+                var sourceLocations = NavigableItemFactory.GetPreferredSourceLocations(
+                    solution, symbol, cancellationToken);
+
+                if (sourceLocations.All(l => solution.GetDocument(l.SourceTree) == null))
+                    symbol = alias.Target;
+            }
+
+            var definition = await SymbolFinder.FindSourceDefinitionAsync(symbol, solution, cancellationToken).ConfigureAwait(false);
+            cancellationToken.ThrowIfCancellationRequested();
+
+            symbol = definition ?? symbol;
+
+            // If it is a partial method declaration with no body, choose to go to the implementation
+            // that has a method body.
+            if (symbol is IMethodSymbol method)
+                symbol = method.PartialImplementationPart ?? symbol;
+
+            return symbol;
+        }
+
+        public static async Task<ImmutableArray<DefinitionItem>> GetDefinitionsAsync(
+            ISymbol? symbol,
+            Solution solution,
+            bool thirdPartyNavigationAllowed,
+            CancellationToken cancellationToken)
+        {
+            symbol = await TryGetPreferredSymbolAsync(solution, symbol, cancellationToken).ConfigureAwait(false);
+            if (symbol is null)
+                return ImmutableArray.Create<DefinitionItem>();
+
+            using var _ = ArrayBuilder<DefinitionItem>.GetInstance(out var definitions);
+
+            // Going to a symbol may end up actually showing the symbol in the Find-Usages window.
+            // This happens when there is more than one location for the symbol (i.e. for partial
+            // symbols) and we don't know the best place to take you to.
+            //
+            // The FindUsages window supports showing the classified text for an item.  It does this
+            // in two ways.  Either the item can pass along its classified text (and the window will
+            // defer to that), or the item will have no classified text, and the window will compute
+            // it in the BG.
+            //
+            // Passing along the classified information is valuable for OOP scenarios where we want
+            // all that expensive computation done on the OOP side and not in the VS side.
+            //
+            // However, Go To Definition is all in-process, and is also synchronous.  So we do not
+            // want to fetch the classifications here.  It slows down the command and leads to a
+            // measurable delay in our perf tests.
+            //
+            // So, if we only have a single location to go to, this does no unnecessary work.  And,
+            // if we do have multiple locations to show, it will just be done in the BG, unblocking
+            // this command thread so it can return the user faster.
+            var definitionItem = symbol.ToNonClassifiedDefinitionItem(solution, includeHiddenLocations: true);
+
+            if (thirdPartyNavigationAllowed)
+            {
+                var factory = solution.Services.GetService<IDefinitionsAndReferencesFactory>();
+                if (factory != null)
+                {
+                    var thirdPartyItem = await factory.GetThirdPartyDefinitionItemAsync(solution, definitionItem, cancellationToken).ConfigureAwait(false);
+                    definitions.AddIfNotNull(thirdPartyItem);
+                }
+            }
+
+            definitions.Add(definitionItem);
+            return definitions.ToImmutable();
+        }
+    }
+}

src/Features/LanguageServer/ProtocolUnitTests/Definitions/GoToDefinitionTests.cs

@@ -152,6 +152,33 @@ End Class
             AssertLocationsEqual(testLspServer.GetLocations("definition"), results);
         }
 
+        [Theory, CombinatorialData]
+        [WorkItem("https://github.com/dotnet/vscode-csharp/issues/5740")]
+        public async Task TestGotoDefinitionPartialMethods(bool mutatingLspWorkspace)
+        {
+            var markup =
+                """
+                using System;
+
+                public partial class C
+                {
+                    partial void {|caret:|}P();
+                }
+
+                public partial class C
+                {
+                    partial void {|definition:P|}()
+                    {
+                        Console.WriteLine(");
+                    }
+                }
+                """;
+            await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace);
+
+            var results = await RunGotoDefinitionAsync(testLspServer, testLspServer.GetLocations("caret").Single());
+            AssertLocationsEqual(testLspServer.GetLocations("definition"), results);
+        }
+
         private static async Task<LSP.Location[]> RunGotoDefinitionAsync(TestLspServer testLspServer, LSP.Location caret)
         {
             return await testLspServer.ExecuteRequestAsync<LSP.TextDocumentPositionParams, LSP.Location[]>(LSP.Methods.TextDocumentDefinitionName,