Add priority classes for add-import #57170
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
the
CyrusNajmabadi
commented
Oct 15, 2021
| var fixesForDiagnostic = await addImportService.GetFixesForDiagnosticsAsync( | ||
| document, span, diagnostics, MaxResults, symbolSearchService, searchReferenceAssemblies, packageSources, cancellationToken).ConfigureAwait(false); | ||
| document, span, diagnostics, MaxResults, this.RequestPriority, | ||
| symbolSearchService, searchReferenceAssemblies, packageSources, cancellationToken).ConfigureAwait(false); |
Member
Author
There was a problem hiding this comment.
this is the start of the change. we are passing RequestPriority along from teh fix provider all the way to the service that computes the fixes.
CyrusNajmabadi
commented
Oct 15, 2021
| protected abstract string GetDescription(IReadOnlyList<string> nameParts); | ||
| protected abstract (string description, bool hasExistingImport) GetDescription(Document document, OptionSet options, INamespaceOrTypeSymbol symbol, SemanticModel semanticModel, SyntaxNode root, CancellationToken cancellationToken); | ||
|
|
||
| public async Task<ImmutableArray<AddImportFixData>> GetFixesAsync( |
Member
Author
There was a problem hiding this comment.
most of the code is just passing the priority along.
CyrusNajmabadi
commented
Oct 15, 2021
| this, document, semanticModel, diagnosticId, node, symbolSearchService, | ||
| searchReferenceAssemblies, packageSources, cancellationToken); | ||
|
|
||
| // Look for exact matches first: |
Member
Author
There was a problem hiding this comment.
this is the meat of the change. here's where we look at waht priority our caller was at and we use that to decide what sorts of results we return.
Member
Author
|
Closing in favor of #57171 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #57157
This is targetting 17.0 to address the async lightbulb regression there. The core issue here is that prior to 'async lightbulbs' 'add using' already had two priority classes. Normal 'exact match' fixes that would show up at the top of the lightbulb, and 'normal pri' 'fuzzy matching' fixes that would show up at the bottom.
When we switched to async lightbulbs this broke as we'd compute all add-using fixes and put them before everything else. The fix here is to make add-using have two entrypoints, one for high and one for normal pri fixes. This way we can have:
High pri add using fixes
Normal pri other fixes
Normal pri add using fixes