Skip to content

Commit

Permalink
Merge pull request #68804 from sharwell/non-responsive-load
Browse files Browse the repository at this point in the history
Force public APIs to return complete results
  • Loading branch information
sharwell authored Dec 27, 2023
2 parents 182e829 + 6909093 commit ffea8b3
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 20 deletions.
5 changes: 4 additions & 1 deletion src/Features/Core/Portable/Completion/CompletionOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ internal sealed record class CompletionOptions
public bool PerformSort { get; init; } = true;

/// <summary>
/// Test-only option.
/// Force completion APIs to produce complete results, even in cases where caches have not been pre-populated.
/// This is typically used for testing scenarios, and by public APIs where consumers do not have access to
/// other internal APIs used to control cache creation and/or wait for caches to be populated before examining
/// completion results.
/// </summary>
public bool ForceExpandedCompletionIndexCreation { get; init; } = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,17 +93,22 @@ private ValueTask ProcessBatchAsync(ImmutableSegmentedList<IReadOnlyList<Analyze
return ValueTaskFactory.CompletedTask;
}

public ImmutableArray<CompletionProvider> GetCachedProjectCompletionProvidersOrQueueLoadInBackground(Project? project)
public ImmutableArray<CompletionProvider> GetCachedProjectCompletionProvidersOrQueueLoadInBackground(Project? project, CompletionOptions options)
{
if (project is null || project.Solution.WorkspaceKind == WorkspaceKind.Interactive)
{
// TODO (https://github.com/dotnet/roslyn/issues/4932): Don't restrict completions in Interactive
return ImmutableArray<CompletionProvider>.Empty;
}

// Don't load providers if they are not already cached,
// return immediately and load them in background instead.
// On primary completion paths, don't load providers if they are not already cached,
// return immediately and load them in background instead. If the test hook
// 'ForceExpandedCompletionIndexCreation' is set, calculate the values immediately.
// https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1620947
if (options.ForceExpandedCompletionIndexCreation)
{
return ProjectCompletionProvider.GetExtensions(_service.Language, project.AnalyzerReferences);
}

if (ProjectCompletionProvider.TryGetCachedExtensions(project.AnalyzerReferences, out var providers))
return providers;
Expand Down Expand Up @@ -153,14 +158,17 @@ ImmutableArray<CompletionProvider> GetImportedAndBuiltInProvidersWorker(Immutabl
return provider;

using var _ = PooledDelegates.GetPooledFunction(static (p, n) => p.Name == n, item.ProviderName, out Func<CompletionProvider, bool> isNameMatchingProviderPredicate);
return GetCachedProjectCompletionProvidersOrQueueLoadInBackground(project).FirstOrDefault(isNameMatchingProviderPredicate);

// Publicly available options will not impact this call, since the completion item must have already
// existed if it produced the input completion item.
return GetCachedProjectCompletionProvidersOrQueueLoadInBackground(project, CompletionOptions.Default).FirstOrDefault(isNameMatchingProviderPredicate);
}

public ConcatImmutableArray<CompletionProvider> GetFilteredProviders(
Project? project, ImmutableHashSet<string>? roles, CompletionTrigger trigger, in CompletionOptions options)
{
var allCompletionProviders = FilterProviders(GetImportedAndBuiltInProviders(roles), trigger, options);
var projectCompletionProviders = FilterProviders(GetCachedProjectCompletionProvidersOrQueueLoadInBackground(project), trigger, options);
var projectCompletionProviders = FilterProviders(GetCachedProjectCompletionProvidersOrQueueLoadInBackground(project, options), trigger, options);
return allCompletionProviders.ConcatFast(projectCompletionProviders);
}

Expand Down Expand Up @@ -278,12 +286,12 @@ public ImmutableArray<CompletionProvider> GetImportedAndBuiltInProviders(Immutab
return _providerManager.GetImportedAndBuiltInProviders(roles);
}

public async Task<ImmutableArray<CompletionProvider>> GetProjectProvidersAsync(Project project)
public ImmutableArray<CompletionProvider> GetProjectProviders(Project project)
{
_providerManager._projectProvidersWorkQueue.AddWork(project.AnalyzerReferences);
await _providerManager._projectProvidersWorkQueue.WaitUntilCurrentBatchCompletesAsync().ConfigureAwait(false);
// Now the extension cache is guaranteed to be populated.
return _providerManager.GetCachedProjectCompletionProvidersOrQueueLoadInBackground(project);
// Force-load the extension completion providers
return _providerManager.GetCachedProjectCompletionProvidersOrQueueLoadInBackground(
project,
CompletionOptions.Default with { ForceExpandedCompletionIndexCreation = true });
}
}
}
Expand Down
13 changes: 7 additions & 6 deletions src/Features/Core/Portable/Completion/CompletionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,9 @@ public bool ShouldTriggerCompletion(
var document = text.GetOpenDocumentInCurrentContextWithChanges();
var languageServices = document?.Project.Services ?? _services.GetLanguageServices(Language);

// Publicly available options do not affect this API.
var completionOptions = CompletionOptions.Default;
// Publicly available options do not affect this API. Force complete results from this public API since
// external consumers do not have access to Roslyn's waiters.
var completionOptions = CompletionOptions.Default with { ForceExpandedCompletionIndexCreation = true };
var passThroughOptions = options ?? document?.Project.Solution.Options ?? OptionSet.Empty;

return ShouldTriggerCompletion(document?.Project, languageServices, text, caretPosition, trigger, completionOptions, passThroughOptions, roles);
Expand Down Expand Up @@ -369,8 +370,8 @@ internal void LoadImportedProviders()
/// <summary>
/// Don't call. Used for pre-load project providers only.
/// </summary>
internal void TriggerLoadProjectProviders(Project project)
=> _providerManager.GetCachedProjectCompletionProvidersOrQueueLoadInBackground(project);
internal void TriggerLoadProjectProviders(Project project, CompletionOptions options)
=> _providerManager.GetCachedProjectCompletionProvidersOrQueueLoadInBackground(project, options);

internal CompletionProvider? GetProvider(CompletionItem item, Project? project)
=> _providerManager.GetProvider(item, project);
Expand All @@ -385,8 +386,8 @@ internal readonly struct TestAccessor(CompletionService completionServiceWithPro
public ImmutableArray<CompletionProvider> GetImportedAndBuiltInProviders(ImmutableHashSet<string> roles)
=> _completionServiceWithProviders._providerManager.GetTestAccessor().GetImportedAndBuiltInProviders(roles);

public Task<ImmutableArray<CompletionProvider>> GetProjectProvidersAsync(Project project)
=> _completionServiceWithProviders._providerManager.GetTestAccessor().GetProjectProvidersAsync(project);
public ImmutableArray<CompletionProvider> GetProjectProviders(Project project)
=> _completionServiceWithProviders._providerManager.GetTestAccessor().GetProjectProviders(project);

public async Task<CompletionContext> GetContextAsync(
CompletionProvider provider,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ public Task<CompletionList> GetCompletionsAsync(
OptionSet? options = null,
CancellationToken cancellationToken = default)
{
// Publicly available options do not affect this API.
var completionOptions = CompletionOptions.Default;
// Publicly available options do not affect this API. Force complete results from this public API since
// external consumers do not have access to Roslyn's waiters.
var completionOptions = CompletionOptions.Default with { ForceExpandedCompletionIndexCreation = true };
var passThroughOptions = options ?? document.Project.Solution.Options;

return GetCompletionsAsync(document, caretPosition, completionOptions, passThroughOptions, trigger, roles, cancellationToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ private async ValueTask BatchProcessProjectsWithOpenedDocumentAsync(ImmutableSeg
// from a race caused by multiple features (codefix, refactoring, etc.) attempting to get extensions
// from analyzer references at the same time when they are not cached.
if (project.GetLanguageService<CompletionService>() is CompletionService completionService)
completionService.TriggerLoadProjectProviders(project);
completionService.TriggerLoadProjectProviders(project, GlobalOptions.GetCompletionOptions(project.Language));

await InitializeServiceForProjectWithOpenedDocumentAsync(project).ConfigureAwait(false);
}
Expand Down

0 comments on commit ffea8b3

Please sign in to comment.