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.

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

Create new AsyncServiceProvider instead of using AsyncServiceProvider.GlobalProvider #74084

Merged
merged 1 commit into from
Jun 20, 2024
Merged
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
using Roslyn.VisualStudio.NewIntegrationTests.InProcess;
using WindowsInput.Native;
using Xunit;
using COMAsyncServiceProvider = Microsoft.VisualStudio.Shell.Interop.COMAsyncServiceProvider;
using IComponentModel = Microsoft.VisualStudio.ComponentModelHost.IComponentModel;
using IObjectWithSite = Microsoft.VisualStudio.OLE.Interop.IObjectWithSite;
using IOleServiceProvider = Microsoft.VisualStudio.OLE.Interop.IServiceProvider;
Expand Down Expand Up @@ -455,7 +456,7 @@ public async Task<ImmutableArray<string>> GetF1KeywordsAsync(CancellationToken c
ErrorHandler.ThrowOnFailure(vsView.GetBuffer(out var textLines));
ErrorHandler.ThrowOnFailure(textLines.GetLanguageServiceID(out var languageServiceGuid));

var languageService = await ((AsyncServiceProvider)AsyncServiceProvider.GlobalProvider).QueryServiceAsync(languageServiceGuid).WithCancellation(cancellationToken);
var languageService = await new AsyncServiceProvider((COMAsyncServiceProvider.IAsyncServiceProvider)AsyncServiceProvider.GlobalProvider).QueryServiceAsync(languageServiceGuid).WithCancellation(cancellationToken);
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is a fix for a downcast that wasn't documented to succeed, maybe your fix should avoid doing the same thing again. I believe the only supported way to get the COM IAsyncServiceProvider is by querying for it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Querying for it is easy enough. Do you know which service interface matches it?

Copy link
Contributor

Choose a reason for hiding this comment

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

There isn't a different service interface for it. So it would look like this:

AsyncServiceProvider.GlobalProvider.GetService<COMAsyncServiceProvider.IAsyncServiceProvider>()

You'll have to test it though. This is all based on my recollection.

Copy link
Member Author

Choose a reason for hiding this comment

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

➡️ I'll submit it as a separate follow-up PR since the current one already passed integration tests.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that will "work" as the managed version implements the native interface, but the right change is to query it as suggested by Andrew.

Copy link
Member Author

Choose a reason for hiding this comment

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

➡️ Submitted #74100

Assumes.Present(languageService);

var languageContextProvider = (IVsLanguageContextProvider)languageService;
Expand Down
Loading