Optimize helper we use to determine the relevant node of a particular kind the user is on. #73812
Conversation
This file contains 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
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
CyrusNajmabadi
commented
May 31, 2024
...ortable/CodeRefactorings/UseExplicitOrImplicitType/AbstractUseTypeCodeRefactoringProvider.cs
Show resolved
Hide resolved
CyrusNajmabadi
commented
May 31, 2024
| return relevantNodesBuilder.ToImmutableAndClear(); | ||
|
|
||
| using var _2 = ArrayBuilder<TSyntaxNode>.GetInstance(out var nonEmptyNodes); | ||
| foreach (var node in relevantNodesBuilder) |
There was a problem hiding this comment.
this loop went away, since we're only adding the nodes if they respect the allowEmptyNodes flag.
CyrusNajmabadi
commented
May 31, 2024
| private async Task AddRelevantNodesAsync<TSyntaxNode>( | ||
| Document document, TextSpan selectionRaw, ArrayBuilder<TSyntaxNode> relevantNodes, CancellationToken cancellationToken) where TSyntaxNode : SyntaxNode | ||
| public void AddRelevantNodes<TSyntaxNode>( | ||
| ParsedDocument document, TextSpan selectionRaw, bool allowEmptyNodes, bool stopOnFirst, ref TemporaryArray<TSyntaxNode> result, CancellationToken cancellationToken) where TSyntaxNode : SyntaxNode |
There was a problem hiding this comment.
passing in PArsedDocument allows this to all be synchronous. which also allows passing in TempArray to add results into.
CyrusNajmabadi
commented
May 31, 2024
| AddNonHiddenCorrectTypeNodes(ExtractNodesInHeader(root, location, headerFacts), relevantNodes, cancellationToken); | ||
| AddNonHiddenCorrectTypeNodes(ExtractNodesInHeader(root, location, headerFacts), stopOnFirst, ref result, cancellationToken); | ||
| if (stopOnFirst && result.Count > 0) | ||
| return; |
There was a problem hiding this comment.
ugly repetition. dont' have a better solution.
CyrusNajmabadi
commented
May 31, 2024
| { | ||
| var correctTypeNonHiddenNodes = nodes.OfType<TSyntaxNode>().Where(n => !n.OverlapsHiddenPosition(cancellationToken)); |
CyrusNajmabadi
commented
May 31, 2024
| return potentialNodes.FirstOrDefault(); | ||
| using var result = TemporaryArray<TSyntaxNode>.Empty; | ||
| AddRelevantNodes(document, span, allowEmptyNode, stopOnFirst: true, ref result.AsRef(), cancellationToken); | ||
| return result.FirstOrDefault(); |
There was a problem hiding this comment.
this helper (used in a ton of places) can now stop on the first result, and not do more work walking the tree.
CyrusNajmabadi
commented
May 31, 2024
| @@ -40,32 +40,67 @@ internal static class CodeRefactoringContextExtensions | |||
| public static Task<TSyntaxNode?> TryGetRelevantNodeAsync<TSyntaxNode>(this CodeRefactoringContext context) where TSyntaxNode : SyntaxNode | |||
| => TryGetRelevantNodeAsync<TSyntaxNode>(context, allowEmptyNode: false); | |||
|
|
|||
| public static Task<TSyntaxNode?> TryGetRelevantNodeAsync<TSyntaxNode>(this CodeRefactoringContext context, bool allowEmptyNode) where TSyntaxNode : SyntaxNode | |||
| => TryGetRelevantNodeAsync<TSyntaxNode>(context.Document, context.Span, allowEmptyNode, context.CancellationToken); | |||
| public static async Task<TSyntaxNode?> TryGetRelevantNodeAsync<TSyntaxNode>(this CodeRefactoringContext context, bool allowEmptyNode) where TSyntaxNode : SyntaxNode | |||
There was a problem hiding this comment.
lots of bridge methods (still async) to the new sync paths.
ToddGrun
reviewed
Jun 1, 2024
src/Features/Core/Portable/CodeRefactorings/IRefactoringHelpersService.cs
Outdated
Show resolved
Hide resolved
ToddGrun
reviewed
Jun 1, 2024
| private async Task AddRelevantNodesAsync<TSyntaxNode>( | ||
| Document document, TextSpan selectionRaw, ArrayBuilder<TSyntaxNode> relevantNodes, CancellationToken cancellationToken) where TSyntaxNode : SyntaxNode | ||
| public void AddRelevantNodes<TSyntaxNode>( | ||
| ParsedDocument document, TextSpan selectionRaw, bool allowEmptyNodes, bool stopOnFirst, ref TemporaryArray<TSyntaxNode> result, CancellationToken cancellationToken) where TSyntaxNode : SyntaxNode |
There was a problem hiding this comment.
Likely about the same. But I'll change to that as a slightly more flexible pattern!
CyrusNajmabadi
commented
Jun 3, 2024
| var hostServices = document.Project.Solution.Services | ||
| .GetLanguageServices(document.Project.Language).HostLanguageServices; | ||
| #pragma warning restore CS0618 // Type or member is obsolete | ||
| var parsedDocument = new ParsedDocument(document.Id, text, root, hostServices); |
There was a problem hiding this comment.
workaround for a problem i'm having using PArsedDocument in OOP. Talking to Tomas about this now.
This was referenced Jun 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Drops us from:
to