diff --git a/src/EditorFeatures/CSharpTest/MoveToNamespace/MoveToNamespaceTests.cs b/src/EditorFeatures/CSharpTest/MoveToNamespace/MoveToNamespaceTests.cs index 28a7662763198fe46d8925fc0ca4a001c7bbe4ca..17e16978f5583fd33b027865626d484603e7a82f 100644 --- a/src/EditorFeatures/CSharpTest/MoveToNamespace/MoveToNamespaceTests.cs +++ b/src/EditorFeatures/CSharpTest/MoveToNamespace/MoveToNamespaceTests.cs @@ -7,6 +7,7 @@ using Microsoft.CodeAnalysis.Editor.UnitTests; using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces; using Microsoft.CodeAnalysis.MoveToNamespace; +using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Test.Utilities; using Microsoft.CodeAnalysis.Test.Utilities.MoveToNamespace; using Microsoft.VisualStudio.Composition; @@ -1226,5 +1227,67 @@ partial class MyClass { }", expectedSuccess: false); + + [Fact, Trait(Traits.Feature, Traits.Features.MoveToNamespace)] + [WorkItem(39234, "https://github.com/dotnet/roslyn/issues/39234")] + public async Task TestMultiTargetingProject() + { + // Create two projects with same project file path and single linked document to simulate a multi-targeting project. + var input = @" + + +namespace A +{ + public class Class1 + { + } + + public class Class2[||] + { + } +} + + + + + +"; + + var expected = +@"namespace A +{ + public class Class1 + { + } +} + +namespace B +{ + public class Class2 + { + } +}"; + using var workspace = TestWorkspace.Create(System.Xml.Linq.XElement.Parse(input), exportProvider: ExportProviderFactory.CreateExportProvider()); + + // Set the target namespace to "B" + var testDocument = workspace.Projects.Single(p => p.Name == "Proj1").Documents.Single(); + var document = workspace.CurrentSolution.GetDocument(testDocument.Id); + var movenamespaceService = document.GetLanguageService(); + var moveToNamespaceOptions = new MoveToNamespaceOptionsResult("B"); + ((TestMoveToNamespaceOptionsService)movenamespaceService.OptionsService).SetOptions(moveToNamespaceOptions); + + var (_, action) = await GetCodeActionsAsync(workspace, default); + var operations = await VerifyActionAndGetOperationsAsync(workspace, action, default); + var result = ApplyOperationsAndGetSolution(workspace, operations); + + // Make sure both linked documents are changed. + foreach (var id in workspace.Documents.Select(d => d.Id)) + { + var changedDocument = result.Item2.GetDocument(id); + var changedRoot = await changedDocument.GetSyntaxRootAsync(); + var actualText = changedRoot.ToFullString(); + Assert.Equal(expected, actualText); + } + } } } diff --git a/src/EditorFeatures/TestUtilities/MoveToNamespace/AbstractMoveToNamespaceTests.cs b/src/EditorFeatures/TestUtilities/MoveToNamespace/AbstractMoveToNamespaceTests.cs index 8f5f6207a2644ef3fd66918eb948bfc0ff1835de..4dc07e0e776625fbc2bb8a03a64c43388bbe9440 100644 --- a/src/EditorFeatures/TestUtilities/MoveToNamespace/AbstractMoveToNamespaceTests.cs +++ b/src/EditorFeatures/TestUtilities/MoveToNamespace/AbstractMoveToNamespaceTests.cs @@ -83,7 +83,6 @@ protected override CodeRefactoringProvider CreateCodeRefactoringProvider(Workspa checkedCodeActions.Add(codeAction); } } - } if (!optionCancelled && !string.IsNullOrEmpty(targetNamespace)) diff --git a/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpChangeNamespaceService.cs b/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpChangeNamespaceService.cs index 4219c01e6446675f6c407bb545efec78d2db0f12..1262750d56bb100aae53bf9ea3dddba12a5177f7 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpChangeNamespaceService.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpChangeNamespaceService.cs @@ -99,11 +99,11 @@ protected override SyntaxList GetMemberDeclarationsInCo } /// - /// Try to get a new node to replace given node, which is a reference to a top-level type declared inside the namespce to be changed. + /// Try to get a new node to replace given node, which is a reference to a top-level type declared inside the namespace to be changed. /// If this reference is the right side of a qualified name, the new node returned would be the entire qualified name. Depends on /// whether is provided, the name in the new node might be qualified with this new namespace instead. /// - /// A reference to a type declared inside the namespce to be changed, which is calculated based on results from + /// A reference to a type declared inside the namespace to be changed, which is calculated based on results from /// `SymbolFinder.FindReferencesAsync`. /// If specified, and the reference is qualified with namespace, the namespace part of original reference /// will be replaced with given namespace in the new node. @@ -275,7 +275,7 @@ private static CompilationUnitSyntax MoveMembersFromNamespaceToGlobal(Compilatio var eofToken = root.EndOfFileToken .WithAdditionalAnnotations(WarningAnnotation); - // Try to preserve trivia from original namesapce declaration. + // Try to preserve trivia from original namespace declaration. // If there's any member inside the declaration, we attach them to the // first and last member, otherwise, simply attach all to the EOF token. if (members.Count > 0) @@ -362,6 +362,11 @@ private static CompilationUnitSyntax MoveMembersFromGlobalToNamespace(Compilatio { // Otherwise, the span should contain a namespace declaration node, which must be the only one // in the entire syntax spine to enable the change namespace operation. + if (!compilationUnit.Span.Contains(span)) + { + return null; + } + var node = compilationUnit.FindNode(span, getInnermostNodeForTie: true); var namespaceDecl = node.AncestorsAndSelf().OfType().SingleOrDefault(); diff --git a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractChangeNamespaceService.cs b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractChangeNamespaceService.cs index 4d34329f265de9ef8493b78976c40332b7cf1b02..166b67ba508df4c5c73f67b613f75f9db82f4d9d 100644 --- a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractChangeNamespaceService.cs +++ b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractChangeNamespaceService.cs @@ -33,11 +33,11 @@ internal abstract class AbstractChangeNamespaceService : IChangeNamespaceService /// /// Try to get a new node to replace given node, which is a reference to a top-level type declared inside the - /// namespce to be changed. If this reference is the right side of a qualified name, the new node returned would + /// namespace to be changed. If this reference is the right side of a qualified name, the new node returned would /// be the entire qualified name. Depends on whether is provided, the name /// in the new node might be qualified with this new namespace instead. /// - /// A reference to a type declared inside the namespce to be changed, which is calculated + /// A reference to a type declared inside the namespace to be changed, which is calculated /// based on results from `SymbolFinder.FindReferencesAsync`. /// If specified, the namespace of original reference will be replaced with given /// namespace in the replacement node. @@ -164,7 +164,7 @@ await ChangeNamespaceInSingleDocumentAsync(solutionAfterNamespaceChange, documen // After changing documents, we still need to remove unnecessary imports related to our change. // We don't try to remove all imports that might become unnecessary/invalid after the namespace change, - // just ones that fully matche the old/new namespace. Because it's hard to get it right and will almost + // just ones that fully match the old/new namespace. Because it's hard to get it right and will almost // certainly cause perf issue. // For example, if we are changing namespace `Foo.Bar` (which is the only namespace declaration with such name) // to `A.B`, the using of name `Bar` in a different file below would remain untouched, even it's no longer valid: @@ -287,7 +287,7 @@ protected static bool IsSupportedLinkedDocument(Document document, out Immutable // TODO: figure out how to properly determine if and how a document is linked using project system. - // If we found a linked document which is part of a project with differenct project file, + // If we found a linked document which is part of a project with different project file, // then it's an actual linked file (i.e. not a multi-targeting project). We don't support that for now. if (linkedDocumentIds.Any(id => !PathUtilities.PathsEqual(solution.GetDocument(id).Project.FilePath, document.Project.FilePath))) diff --git a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractSyncNamespaceCodeRefactoringProvider.cs b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractSyncNamespaceCodeRefactoringProvider.cs index 6f3d01cf489e61b3ee80afd3e34acfc1ea48d5ab..e4e11acdbe64b1792a9cda7f9176b8bb330d8c9e 100644 --- a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractSyncNamespaceCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractSyncNamespaceCodeRefactoringProvider.cs @@ -36,7 +36,7 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte { // These code actions try to move file to a new location based on declared namespace // and the default namespace of the project. The new location is a list of folders - // determined by the relateive part of the declared namespace compare to the default namespace. + // determined by the relative part of the declared namespace compare to the default namespace. // // For example, if he default namespace is `A.B.C`, file path is // "[project root dir]\Class1.cs" and declared namespace in the file is @@ -81,7 +81,7 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte /// Try to get the node that can be used to trigger the refactoring based on current cursor position. /// /// - /// (1) a node of type node, if curosr in the name and it's the + /// (1) a node of type node, if cursor in the name and it's the /// only namespace declaration in the document. /// (2) a node of type node, if the cursor is in the name of first /// declaration in global namespace and there's no namespace declaration in this document. diff --git a/src/Features/Core/Portable/MoveToNamespace/AbstractMoveToNamespaceService.cs b/src/Features/Core/Portable/MoveToNamespace/AbstractMoveToNamespaceService.cs index d628cb52920ed0c30835d9c8b58dc189607140da..721db5d5b01699d2620c981e4755c16fe6e5cf21 100644 --- a/src/Features/Core/Portable/MoveToNamespace/AbstractMoveToNamespaceService.cs +++ b/src/Features/Core/Portable/MoveToNamespace/AbstractMoveToNamespaceService.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.ChangeNamespace; using Microsoft.CodeAnalysis.CodeRefactorings.MoveType; +using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.Shared.Extensions; @@ -93,7 +94,7 @@ protected AbstractMoveToNamespaceService(IMoveToNamespaceOptionsService moveToNa return null; } - // The underlying ChangeNamespace service doesn't support nested namespace decalration. + // The underlying ChangeNamespace service doesn't support nested namespace declaration. if (GetNamespaceInSpineCount(declarationSyntax) == 1) { var changeNamespaceService = document.GetLanguageService(); @@ -252,11 +253,16 @@ private static async Task> GetMemberSymbolsAsync(Documen moveSpan, MoveTypeOperationKind.MoveTypeNamespaceScope, cancellationToken).ConfigureAwait(false); - var modifiedDocument = modifiedSolution.GetDocument(document.Id); - var syntaxRoot = await modifiedDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + // Since MoveTypeService doesn't handle linked files, we need to merge the diff ourselves, + // otherwise, we will end up with multiple linked documents with different content. + var mergedSolution = await PropagateChangeToLinkedDocuments(modifiedDocument, cancellationToken).ConfigureAwait(false); + var mergedDocument = mergedSolution.GetDocument(document.Id); + + var syntaxRoot = await mergedDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var syntaxNode = syntaxRoot.GetAnnotatedNodes(AbstractMoveTypeService.NamespaceScopeMovedAnnotation).SingleOrDefault(); + if (syntaxNode == null) { // The type might be declared in global namespace @@ -264,12 +270,27 @@ private static async Task> GetMemberSymbolsAsync(Documen } return await MoveItemsInNamespaceAsync( - modifiedDocument, + mergedDocument, syntaxNode, targetNamespace, cancellationToken).ConfigureAwait(false); } + private static async Task PropagateChangeToLinkedDocuments(Document document, CancellationToken cancellationToken) + { + // Need to make sure elastic trivia is formatted properly before pushing the text to other documents. + var formattedDocument = await Formatter.FormatAsync(document, SyntaxAnnotation.ElasticAnnotation, cancellationToken: cancellationToken).ConfigureAwait(false); + var formattedText = await formattedDocument.GetTextAsync(cancellationToken).ConfigureAwait(false); + var solution = formattedDocument.Project.Solution; + + foreach (var documentId in formattedDocument.GetLinkedDocumentIds()) + { + solution = solution.WithDocumentText(documentId, formattedText); + } + + return solution; + } + private static string GetNewSymbolName(ISymbol symbol, string targetNamespace) { Debug.Assert(symbol != null && !string.IsNullOrEmpty(targetNamespace)); diff --git a/src/Workspaces/CoreTestUtilities/MEF/ExportProviderCache.cs b/src/Workspaces/CoreTestUtilities/MEF/ExportProviderCache.cs index 59a0e2ddbc7834593ca64590b1d8742090fb849d..15dfbecd0fedb700ca0f043a387034cf5ce01d0b 100644 --- a/src/Workspaces/CoreTestUtilities/MEF/ExportProviderCache.cs +++ b/src/Workspaces/CoreTestUtilities/MEF/ExportProviderCache.cs @@ -275,7 +275,7 @@ private static void RequireForSingleExportProvider(bool condition) // method attempting to create a default ExportProvider which did not match the one assigned to // the test. // * A test attempted to perform multiple test sequences in the context of a single test method, - // rather than break up the test into distict tests for each case. + // rather than break up the test into distinct tests for each case. // * A test referenced different predefined ExportProvider instances within the context of a test. // Each test is expected to use the same ExportProvider throughout the test. throw new InvalidOperationException($"Only one {nameof(ExportProvider)} can be created in the context of a single test.");