Skip to content

Commit

Permalink
Merge pull request #51284 from CyrusNajmabadi/moveRecord
Browse files Browse the repository at this point in the history 
Fix issue with 'move type to file' when moving a record.
  • Loading branch information
msftbot[bot] authored Feb 18, 2021
2 parents 1209b91 + e5b48ea commit 724452a
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 44 deletions.
21 changes: 21 additions & 0 deletions src/EditorFeatures/CSharpTest/CodeActions/MoveType/MoveTypeTests.MoveToNewFile.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1387,5 +1387,26 @@ class Class2
await TestMoveTypeToNewFileAsync(
code, codeAfterMove, expectedDocumentName, destinationDocumentText);
}

[WorkItem(50329, "https://github.com/dotnet/roslyn/issues/50329")]
[WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsMoveType)]
public async Task MoveRecordToNewFilePreserveUsings()
{
var code =
@"using System;
[||]record CacheContext(String Message);
class Program { }";
var codeAfterMove = @"class Program { }";

var expectedDocumentName = "CacheContext.cs";
var destinationDocumentText = @"using System;
record CacheContext(String Message);
";

await TestMoveTypeToNewFileAsync(code, codeAfterMove, expectedDocumentName, destinationDocumentText);
}
}
}
3 changes: 2 additions & 1 deletion src/EditorFeatures/DiagnosticsTestUtilities/MoveType/AbstractMoveTypeTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ protected async Task TestMoveTypeToNewFileAsync(
Assert.True(changedDocumentIds.Contains(sourceDocumentId), "source document was not changed.");

var modifiedSourceDocument = newSolution.GetDocument(sourceDocumentId);
Assert.Equal(expectedSourceTextAfterRefactoring, (await modifiedSourceDocument.GetTextAsync()).ToString());
var actualSourceTextAfterRefactoring = (await modifiedSourceDocument.GetTextAsync()).ToString();
Assert.Equal(expectedSourceTextAfterRefactoring, actualSourceTextAfterRefactoring);
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.Editor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public virtual async Task<ImmutableArray<CodeActionOperation>> GetOperationsAsyn
public static Editor GetEditor(MoveTypeOperationKind operationKind, TService service, State state, string fileName, CancellationToken cancellationToken)
=> operationKind switch
{
MoveTypeOperationKind.MoveType => (Editor)new MoveTypeEditor(service, state, fileName, cancellationToken),
MoveTypeOperationKind.MoveType => new MoveTypeEditor(service, state, fileName, cancellationToken),
MoveTypeOperationKind.RenameType => new RenameTypeEditor(service, state, fileName, cancellationToken),
MoveTypeOperationKind.RenameFile => new RenameFileEditor(service, state, fileName, cancellationToken),
MoveTypeOperationKind.MoveTypeNamespaceScope => new MoveTypeNamespaceScopeEditor(service, state, fileName, cancellationToken),
Expand Down
96 changes: 54 additions & 42 deletions ...eatures/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.MoveTypeEditor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
Expand Down Expand Up @@ -52,29 +50,68 @@ public override async Task<Solution> GetModifiedSolutionAsync()
var projectToBeUpdated = SemanticDocument.Document.Project;
var newDocumentId = DocumentId.CreateNewId(projectToBeUpdated.Id, FileName);

var documentWithMovedType = await AddNewDocumentWithSingleTypeDeclarationAndImportsAsync(newDocumentId).ConfigureAwait(false);
// We do this process in the following steps:
//
// 1. Produce the new document, with the moved type, with all the same imports as the original file.
// 2. Remove the original type from the first document, not touching the imports in it. This is
// necessary to prevent duplicate symbol errors (and other compiler issues) as we process imports.
// 3. Now that the type has been moved to the new file, remove the unnecessary imports from the new
// file. This will also tell us which imports are necessary in the new file.
// 4. Now go back to the original file and remove any unnecessary imports *if* they are in the new file.
// these imports only were needed for the moved type, and so they shouldn't stay in the original
// file.

var documentWithMovedType = await AddNewDocumentWithSingleTypeDeclarationAsync(newDocumentId).ConfigureAwait(false);

var solutionWithNewDocument = documentWithMovedType.Project.Solution;

// Get the original source document again, from the latest forked solution.
var sourceDocument = solutionWithNewDocument.GetDocument(SemanticDocument.Document.Id);
var sourceDocument = solutionWithNewDocument.GetRequiredDocument(SemanticDocument.Document.Id);

// update source document to add partial modifiers to type chain
// and/or remove type declaration from original source document.
var solutionWithBothDocumentsUpdated = await RemoveTypeFromSourceDocumentAsync(
sourceDocument, documentWithMovedType).ConfigureAwait(false);
var solutionWithBothDocumentsUpdated = await RemoveTypeFromSourceDocumentAsync(sourceDocument).ConfigureAwait(false);

return await RemoveUnnecessaryImportsAsync(solutionWithBothDocumentsUpdated, sourceDocument.Id, documentWithMovedType.Id).ConfigureAwait(false);
}

private async Task<Solution> RemoveUnnecessaryImportsAsync(
Solution solution, DocumentId sourceDocumentId, DocumentId documentWithMovedTypeId)
{
var documentWithMovedType = solution.GetRequiredDocument(documentWithMovedTypeId);

var syntaxFacts = documentWithMovedType.GetRequiredLanguageService<ISyntaxFactsService>();
var removeUnnecessaryImports = documentWithMovedType.GetRequiredLanguageService<IRemoveUnnecessaryImportsService>();

// Remove all unnecessary imports from the new document we've created.
documentWithMovedType = await removeUnnecessaryImports.RemoveUnnecessaryImportsAsync(documentWithMovedType, CancellationToken).ConfigureAwait(false);

solution = solution.WithDocumentSyntaxRoot(
documentWithMovedTypeId, await documentWithMovedType.GetRequiredSyntaxRootAsync(CancellationToken).ConfigureAwait(false));

// See which imports we kept around.
var rootWithMovedType = await documentWithMovedType.GetRequiredSyntaxRootAsync(CancellationToken).ConfigureAwait(false);
var movedImports = rootWithMovedType.DescendantNodes()
.Where(syntaxFacts.IsUsingOrExternOrImport)
.ToImmutableArray();

return solutionWithBothDocumentsUpdated;
// Now remove any unnecessary imports from the original doc that moved to the new doc.
var sourceDocument = solution.GetRequiredDocument(sourceDocumentId);
sourceDocument = await removeUnnecessaryImports.RemoveUnnecessaryImportsAsync(
sourceDocument,
n => movedImports.Contains(i => syntaxFacts.AreEquivalent(i, n)),
CancellationToken).ConfigureAwait(false);

return solution.WithDocumentSyntaxRoot(
sourceDocumentId, await sourceDocument.GetRequiredSyntaxRootAsync(CancellationToken).ConfigureAwait(false));
}

/// <summary>
/// Forks the source document, keeps required type, namespace containers
/// and adds it the solution.
/// </summary>
/// <param name="newDocumentId">id for the new document to be added</param>
/// <returns>the new solution which contains a new document with the type being moved</returns>
private async Task<Document> AddNewDocumentWithSingleTypeDeclarationAndImportsAsync(
DocumentId newDocumentId)
private async Task<Document> AddNewDocumentWithSingleTypeDeclarationAsync(DocumentId newDocumentId)
{
var document = SemanticDocument.Document;
Debug.Assert(document.Name != FileName,
Expand All @@ -93,9 +130,7 @@ private async Task<Document> AddNewDocumentWithSingleTypeDeclarationAndImportsAs
// remove things that are not being moved, from the forked document.
var membersToRemove = GetMembersToRemove(root);
foreach (var member in membersToRemove)
{
documentEditor.RemoveNode(member, SyntaxRemoveOptions.KeepNoTrivia);
}

var modifiedRoot = documentEditor.GetChangedRoot();
modifiedRoot = await AddFinalNewLineIfDesiredAsync(document, modifiedRoot).ConfigureAwait(false);
Expand All @@ -109,10 +144,7 @@ private async Task<Document> AddNewDocumentWithSingleTypeDeclarationAndImportsAs

// get the updated document, give it the minimal set of imports that the type
// inside it needs.
var service = document.GetLanguageService<IRemoveUnnecessaryImportsService>();
var newDocument = solutionWithNewDocument.GetDocument(newDocumentId);
newDocument = await service.RemoveUnnecessaryImportsAsync(newDocument, CancellationToken).ConfigureAwait(false);

var newDocument = solutionWithNewDocument.GetRequiredDocument(newDocumentId);
return newDocument;
}

Expand All @@ -129,7 +161,7 @@ private async Task<SyntaxNode> AddFinalNewLineIfDesiredAsync(Document document,
var endOfFileToken = ((ICompilationUnitSyntax)modifiedRoot).EndOfFileToken;
var previousToken = endOfFileToken.GetPreviousToken(includeZeroWidth: true, includeSkipped: true);

var syntaxFacts = document.GetLanguageService<ISyntaxFactsService>();
var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>();
if (endOfFileToken.LeadingTrivia.IsEmpty() &&
!previousToken.TrailingTrivia.Any(syntaxFacts.IsEndOfLineTrivia))
{
Expand All @@ -148,8 +180,7 @@ private async Task<SyntaxNode> AddFinalNewLineIfDesiredAsync(Document document,
/// perform other fix ups as necessary.
/// </summary>
/// <returns>an updated solution with the original document fixed up as appropriate.</returns>
private async Task<Solution> RemoveTypeFromSourceDocumentAsync(
Document sourceDocument, Document documentWithMovedType)
private async Task<Solution> RemoveTypeFromSourceDocumentAsync(Document sourceDocument)
{
var documentEditor = await DocumentEditor.CreateAsync(sourceDocument, CancellationToken).ConfigureAwait(false);

Expand All @@ -162,26 +193,6 @@ private async Task<Solution> RemoveTypeFromSourceDocumentAsync(

var updatedDocument = documentEditor.GetChangedDocument();

// Now, remove any imports that we no longer need *if* they were used in the new
// file with the moved type. Essentially, those imports were here just to serve
// that new type and we should remove them. If we have *other* imports that
// other file does not use *and* we do not use, we'll still keep those around.
// Those may be important to the user for code they're about to write, and we
// don't want to interfere with them by removing them.
var service = sourceDocument.GetLanguageService<IRemoveUnnecessaryImportsService>();

var syntaxFacts = sourceDocument.GetLanguageService<ISyntaxFactsService>();

var rootWithMovedType = await documentWithMovedType.GetSyntaxRootAsync(CancellationToken).ConfigureAwait(false);
var movedImports = rootWithMovedType.DescendantNodes()
.Where(syntaxFacts.IsUsingOrExternOrImport)
.ToImmutableArray();

updatedDocument = await service.RemoveUnnecessaryImportsAsync(
updatedDocument,
n => movedImports.Contains(i => i.IsEquivalentTo(n)),
CancellationToken).ConfigureAwait(false);

return updatedDocument.Project.Solution;
}

Expand Down Expand Up @@ -244,12 +255,13 @@ private void AddPartialModifiersToTypeChain(
bool removeAttributesAndComments,
bool removeTypeInheritance)
{
var semanticFacts = State.SemanticDocument.Document.GetLanguageService<ISemanticFactsService>();
var semanticFacts = State.SemanticDocument.Document.GetRequiredLanguageService<ISemanticFactsService>();
var typeChain = State.TypeNode.Ancestors().OfType<TTypeDeclarationSyntax>();

foreach (var node in typeChain)
{
var symbol = (ITypeSymbol)State.SemanticDocument.SemanticModel.GetDeclaredSymbol(node, CancellationToken);
var symbol = (ITypeSymbol?)State.SemanticDocument.SemanticModel.GetDeclaredSymbol(node, CancellationToken);
Contract.ThrowIfNull(symbol);
if (!semanticFacts.IsPartial(symbol, CancellationToken))
{
documentEditor.SetModifiers(node,
Expand Down Expand Up @@ -282,7 +294,7 @@ private void AddPartialModifiersToTypeChain(
private TTypeDeclarationSyntax RemoveLeadingBlankLines(
TTypeDeclarationSyntax currentTypeNode)
{
var syntaxFacts = State.SemanticDocument.Document.GetLanguageService<ISyntaxFactsService>();
var syntaxFacts = State.SemanticDocument.Document.GetRequiredLanguageService<ISyntaxFactsService>();
var withoutBlankLines = syntaxFacts.GetNodeWithoutLeadingBlankLines(currentTypeNode);

// Add an elastic marker so the formatter can add any blank lines it thinks are
Expand Down

0 comments on commit 724452a

Please sign in to comment.