Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dead goto-def/peek/ctrl-click EA impls for TS. TS is now entirely on LSP for these experiences. #71098

Merged
merged 7 commits into from
Dec 6, 2023
Merged

Remove dead goto-def/peek/ctrl-click EA impls for TS. TS is now entirely on LSP for these experiences. #71098

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
681446f
Remove uncalled TS api
CyrusNajmabadi Dec 5, 2023
18009b9
Remove impls
CyrusNajmabadi Dec 5, 2023
aa62e88
REvert
CyrusNajmabadi Dec 5, 2023
e123b26
obsoletE
CyrusNajmabadi Dec 5, 2023
ca358ca
sort
CyrusNajmabadi Dec 5, 2023
f50ed04
text
CyrusNajmabadi Dec 5, 2023
8ffa18a
Be resilient to missing service
CyrusNajmabadi Dec 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ...EditorFeatures/Core/ExternalAccess/VSTypeScript/Api/IVSTypeScriptGoToDefinitionService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
// 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.Threading;
using System.Threading.Tasks;
using System.Collections.Generic;

namespace Microsoft.CodeAnalysis.ExternalAccess.VSTypeScript.Api
{
[Obsolete("TS, remove your implementation of this type now that you're entirely on LSP for go-to-def. Then let us know.", error: false)]
internal interface IVSTypeScriptGoToDefinitionService
{
Task<IEnumerable<IVSTypeScriptNavigableItem>?> FindDefinitionsAsync(Document document, int position, CancellationToken cancellationToken);
Expand Down
2 changes: 2 additions & 0 deletions ...xternalAccess/VSTypeScript/Api/IVSTypeScriptGoToDefinitionServiceFactoryImplementation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
// 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 Microsoft.CodeAnalysis.Host;

namespace Microsoft.CodeAnalysis.ExternalAccess.VSTypeScript.Api
{
[Obsolete("TS, remove your implementation of this type now that you're entirely on LSP for go-to-def. Then let us know.", error: false)]
internal interface IVSTypeScriptGoToDefinitionServiceFactoryImplementation
{
IVSTypeScriptGoToDefinitionService? CreateLanguageService(HostLanguageServices languageServices);
Expand Down
41 changes: 0 additions & 41 deletions ...Features/Core/ExternalAccess/VSTypeScript/VSTypeScriptDefinitionLocationServiceFactory.cs
View file
Open in desktop

This file was deleted.

41 changes: 0 additions & 41 deletions ...itorFeatures/Core/ExternalAccess/VSTypeScript/VSTypeScriptNavigableItemsServiceFactory.cs
View file
Open in desktop

This file was deleted.

4 changes: 3 additions & 1 deletion src/Features/LanguageServer/Protocol/Handler/Definitions/AbstractGoToDefinitionHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ public AbstractGoToDefinitionHandler(IMetadataAsSourceFileService metadataAsSour
var locations = ArrayBuilder<LSP.Location>.GetInstance();
var position = await document.GetPositionFromLinePositionAsync(ProtocolConversions.PositionToLinePosition(request.Position), cancellationToken).ConfigureAwait(false);

var service = document.GetRequiredLanguageService<INavigableItemsService>();
var service = document.GetLanguageService<INavigableItemsService>();
if (service is null)
return null;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is paranoid as our LSP server is only for languages that support this. But this just means we're not dependong that. And, for example, if xaml didn't support this, we'd be ok.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of prefer throwing actually. It isn't expected that we would generally hit this, and if we do we'll get errors that tell us what is going wrong. It won't take down the LSP server or anything.


var definitions = await service.GetNavigableItemsAsync(document, position, cancellationToken).ConfigureAwait(false);
if (definitions.Length > 0)
Expand Down
5 changes: 4 additions & 1 deletion src/Tools/ExternalAccess/OmniSharp/GoToDefinition/OmniSharpFindDefinitionService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ internal static class OmniSharpFindDefinitionService
{
internal static async Task<ImmutableArray<OmniSharpNavigableItem>> FindDefinitionsAsync(Document document, int position, CancellationToken cancellationToken)
{
var service = document.GetRequiredLanguageService<INavigableItemsService>();
var service = document.GetLanguageService<INavigableItemsService>();
if (service is null)
return ImmutableArray<OmniSharpNavigableItem>.Empty;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just being paranoid. this is omnisharp. so presumably only for C#/VB. But i'm not taking chances.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note even VB, just C#. But I'm ok with paranoia.


var result = await service.GetNavigableItemsAsync(document, position, cancellationToken).ConfigureAwait(false);
return await result.NullToEmpty().SelectAsArrayAsync(
async (original, solution, cancellationToken) => new OmniSharpNavigableItem(original.DisplayTaggedParts, await original.Document.GetRequiredDocumentAsync(solution, cancellationToken).ConfigureAwait(false), original.SourceSpan),
Expand Down