Skip to content

Remove descendent node tracking from SyntaxEditor #67314

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

Merged
CyrusNajmabadi merged 9 commits into from
Mar 16, 2023
Merged

Remove descendent node tracking from SyntaxEditor #67314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0985fc2
Remove descendent node tracking from SyntaxEditor
CyrusNajmabadi Mar 15, 2023
b4e54d6
Use annotation to find node again
CyrusNajmabadi Mar 15, 2023
e89ea38
Simplify
CyrusNajmabadi Mar 15, 2023
78e8b32
Update docs
CyrusNajmabadi Mar 15, 2023
2702ffb
Add docs
CyrusNajmabadi Mar 15, 2023
9d46e37
Remove tests
CyrusNajmabadi Mar 15, 2023
a6e3959
Merge remote-tracking branch 'upstream/main' into syntaxEditor
CyrusNajmabadi Mar 15, 2023
f90ef0b
Merge branch 'main' into syntaxEditor
CyrusNajmabadi Mar 15, 2023
8caccb2
rename
CyrusNajmabadi Mar 15, 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
17 changes: 10 additions & 7 deletions src/Features/Core/Portable/PullMemberUp/MembersPuller.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ internal static class MembersPuller
/// Annotation used to mark imports that we move over, so that we can remove these imports if they are unnecessary
/// (and so we don't remove any other unnecessary imports)
/// </summary>
private static readonly SyntaxAnnotation s_annotation = new("PullMemberRemovableImport");
private static readonly SyntaxAnnotation s_removableImportAnnotation = new("PullMemberRemovableImport");
private static readonly SyntaxAnnotation s_destinationNodeAnnotation = new("DestinationNode");

public static CodeAction TryComputeCodeAction(
Document document,
Expand Down Expand Up @@ -312,7 +313,9 @@ private static async Task<Solution> PullMembersIntoClassAsync(
var options = await destinationEditor.OriginalDocument.GetCleanCodeGenerationOptionsAsync(fallbackOptions, cancellationToken).ConfigureAwait(false);
var info = codeGenerationService.GetInfo(context, options.GenerationOptions, destinationEditor.OriginalDocument.Project.ParseOptions);

var newDestination = codeGenerationService.AddMembers(destinationSyntaxNode, pullUpMembersSymbols, info, cancellationToken);
var newDestination = codeGenerationService
.AddMembers(destinationSyntaxNode, pullUpMembersSymbols, info, cancellationToken)
.WithAdditionalAnnotations(s_destinationNodeAnnotation);
using var _ = PooledHashSet<SyntaxNode>.GetInstance(out var sourceImports);

var syntaxFacts = destinationEditor.OriginalDocument.GetRequiredLanguageService<ISyntaxFactsService>();
Expand All @@ -329,7 +332,7 @@ private static async Task<Solution> PullMembersIntoClassAsync(
sourceImports.Add(
destinationEditor.Generator.NamespaceImportDeclaration(
resultNamespace.ToDisplayString(SymbolDisplayFormats.NameFormat))
.WithAdditionalAnnotations(s_annotation));
.WithAdditionalAnnotations(s_removableImportAnnotation));
}

foreach (var syntax in symbolToDeclarations[analysisResult.Member])
Expand All @@ -342,7 +345,7 @@ private static async Task<Solution> PullMembersIntoClassAsync(
.Select(import => import
.WithoutLeadingTrivia()
.WithTrailingTrivia(originalMemberEditor.Generator.ElasticCarriageReturnLineFeed)
.WithAdditionalAnnotations(s_annotation)));
.WithAdditionalAnnotations(s_removableImportAnnotation)));

if (!analysisResult.MakeMemberDeclarationAbstract || analysisResult.Member.IsAbstract)
{
Expand Down Expand Up @@ -378,7 +381,7 @@ private static async Task<Solution> PullMembersIntoClassAsync(
destinationEditor.ReplaceNode(destinationEditor.OriginalRoot, (node, generator) => addImportsService.AddImports(
destinationEditor.SemanticModel.Compilation,
node,
node.GetCurrentNode(newDestination),
node.GetAnnotatedNodes(s_destinationNodeAnnotation).FirstOrDefault(),
Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi Mar 15, 2023

Choose a reason for hiding this comment

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

this was the bad line. They were basically trying to find this totally fresh node in the current root. but that totally fresh node was already morphed into a different node when teh replacement happened. Trivial fix is to just use our own annotation to track it, instead of the editor tracking all nodes.

sourceImports,
generator,
options.CleanupOptions.AddImportOptions,
Expand All @@ -387,12 +390,12 @@ private static async Task<Solution> PullMembersIntoClassAsync(
var removeImportsService = destinationEditor.OriginalDocument.GetRequiredLanguageService<IRemoveUnnecessaryImportsService>();
var destinationDocument = await removeImportsService.RemoveUnnecessaryImportsAsync(
destinationEditor.GetChangedDocument(),
node => node.HasAnnotation(s_annotation),
node => node.HasAnnotation(s_removableImportAnnotation),
options.CleanupOptions.FormattingOptions,
cancellationToken).ConfigureAwait(false);

// Format whitespace trivia within the import statements we pull up
destinationDocument = await Formatter.FormatAsync(destinationDocument, s_annotation, options.CleanupOptions.FormattingOptions, cancellationToken).ConfigureAwait(false);
destinationDocument = await Formatter.FormatAsync(destinationDocument, s_removableImportAnnotation, options.CleanupOptions.FormattingOptions, cancellationToken).ConfigureAwait(false);

var destinationRoot = AddLeadingTriviaBeforeFirstMember(
await destinationDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false),
Expand Down
Loading