提交 e43a1b41 编写于 作者: G Gen Lu

Address review comments

上级 0f9a0009
......@@ -23,7 +23,7 @@ namespace Microsoft.CodeAnalysis.CSharp.ChangeNamespace
internal sealed class CSharpChangeNamespaceService :
AbstractChangeNamespaceService<NamespaceDeclarationSyntax, CompilationUnitSyntax, MemberDeclarationSyntax>
{
protected override async Task<ImmutableArray<(DocumentId, SyntaxNode)>> CanChangeNamespaceWorkerAsync(
protected override async Task<ImmutableArray<(DocumentId, SyntaxNode)>> GetValidContainersFromAllLinkedDocumentsAsync(
Document document,
SyntaxNode container,
CancellationToken cancellationToken)
......@@ -206,22 +206,13 @@ protected override SyntaxList<MemberDeclarationSyntax> GetMemberDeclarationsInCo
ImmutableArray<string> targetNamespaceParts)
{
Debug.Assert(!declaredNamespaceParts.IsDefault && !targetNamespaceParts.IsDefault);
var container = root.GetAnnotatedNodes(ContainerAnnotation[0]).Single();
var container = root.GetAnnotatedNodes(ContainerAnnotation).Single();
// Move everything from global namespace to a namespace declaration
if (container is CompilationUnitSyntax compilationUnit)
{
Debug.Assert(!compilationUnit.Members.Any(m => m is NamespaceDeclarationSyntax));
// Move everything from global namespace to a namespace declaration
Debug.Assert(IsGlobalNamespace(declaredNamespaceParts));
var targetNamespaceDecl = SyntaxFactory.NamespaceDeclaration(
name: CreateNameSyntax(targetNamespaceParts, aliasQualifier: null, targetNamespaceParts.Length - 1)
.WithAdditionalAnnotations(WarningAnnotation),
externs: default,
usings: default,
members: compilationUnit.Members);
return compilationUnit.WithMembers(new SyntaxList<MemberDeclarationSyntax>(targetNamespaceDecl))
.WithoutAnnotations(ContainerAnnotation);
return MoveMembersFromGlobalToNamespace(compilationUnit, targetNamespaceParts);
}
if (container is NamespaceDeclarationSyntax namespaceDecl)
......@@ -229,48 +220,7 @@ protected override SyntaxList<MemberDeclarationSyntax> GetMemberDeclarationsInCo
// Move everything to global namespace
if (IsGlobalNamespace(targetNamespaceParts))
{
var (namespaceOpeningTrivia, namespaceClosingTrivia) =
GetOpeningAndClosingTriviaOfNamespaceDeclaration(namespaceDecl);
var members = namespaceDecl.Members;
var eofToken = root.EndOfFileToken
.WithAdditionalAnnotations(WarningAnnotation);
// Try to preserve trivia from original namesapce 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)
{
var first = members.First();
var firstWithTrivia = first.WithPrependedLeadingTrivia(namespaceOpeningTrivia);
members = members.Replace(first, firstWithTrivia);
var last = members.Last();
var lastWithTrivia = last.WithAppendedTrailingTrivia(namespaceClosingTrivia);
members = members.Replace(last, lastWithTrivia);
}
else
{
eofToken = eofToken.WithPrependedLeadingTrivia(
namespaceOpeningTrivia.Concat(namespaceClosingTrivia));
}
// Moving inner imports out of the namespace declaration can lead to a break in semantics.
// For example:
//
// namespace A.B.C
// {
// using D.E.F;
// }
//
// The using of D.E.F is looked up with in the context of A.B.C first. If it's moved outside,
// it may fail to resolve.
return root.Update(
root.Externs.AddRange(namespaceDecl.Externs),
root.Usings.AddRange(namespaceDecl.Usings),
root.AttributeLists,
root.Members.ReplaceRange(namespaceDecl, members),
eofToken);
return MoveMembersFromNamespaceToGlobal(root, namespaceDecl);
}
// Change namespace name
......@@ -279,12 +229,72 @@ protected override SyntaxList<MemberDeclarationSyntax> GetMemberDeclarationsInCo
namespaceDecl.WithName(
CreateNameSyntax(targetNamespaceParts, aliasQualifier: null, targetNamespaceParts.Length - 1)
.WithTriviaFrom(namespaceDecl.Name).WithAdditionalAnnotations(WarningAnnotation))
.WithoutAnnotations(ContainerAnnotation));
.WithoutAnnotations(ContainerAnnotation)); // Make sure to remove the annotation we added
}
throw ExceptionUtilities.Unreachable;
}
private static CompilationUnitSyntax MoveMembersFromNamespaceToGlobal(CompilationUnitSyntax root, NamespaceDeclarationSyntax namespaceDecl)
{
var (namespaceOpeningTrivia, namespaceClosingTrivia) =
GetOpeningAndClosingTriviaOfNamespaceDeclaration(namespaceDecl);
var members = namespaceDecl.Members;
var eofToken = root.EndOfFileToken
.WithAdditionalAnnotations(WarningAnnotation);
// Try to preserve trivia from original namesapce 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)
{
var first = members.First();
var firstWithTrivia = first.WithPrependedLeadingTrivia(namespaceOpeningTrivia);
members = members.Replace(first, firstWithTrivia);
var last = members.Last();
var lastWithTrivia = last.WithAppendedTrailingTrivia(namespaceClosingTrivia);
members = members.Replace(last, lastWithTrivia);
}
else
{
eofToken = eofToken.WithPrependedLeadingTrivia(
namespaceOpeningTrivia.Concat(namespaceClosingTrivia));
}
// Moving inner imports out of the namespace declaration can lead to a break in semantics.
// For example:
//
// namespace A.B.C
// {
// using D.E.F;
// }
//
// The using of D.E.F is looked up with in the context of A.B.C first. If it's moved outside,
// it may fail to resolve.
return root.Update(
root.Externs.AddRange(namespaceDecl.Externs),
root.Usings.AddRange(namespaceDecl.Usings),
root.AttributeLists,
root.Members.ReplaceRange(namespaceDecl, members),
eofToken);
}
private static CompilationUnitSyntax MoveMembersFromGlobalToNamespace(CompilationUnitSyntax compilationUnit, ImmutableArray<string> targetNamespaceParts)
{
Debug.Assert(!compilationUnit.Members.Any(m => m is NamespaceDeclarationSyntax));
var targetNamespaceDecl = SyntaxFactory.NamespaceDeclaration(
name: CreateNameSyntax(targetNamespaceParts, aliasQualifier: null, targetNamespaceParts.Length - 1)
.WithAdditionalAnnotations(WarningAnnotation),
externs: default,
usings: default,
members: compilationUnit.Members);
return compilationUnit.WithMembers(new SyntaxList<MemberDeclarationSyntax>(targetNamespaceDecl))
.WithoutAnnotations(ContainerAnnotation); // Make sure to remove the annotation we added
}
/// <summary>
/// For the node specified by <paramref name="span"/> to be applicable container, it must be a namespace
/// declaration or a compilation unit, contain no partial declarations and meet the following additional
......@@ -299,7 +309,7 @@ protected override SyntaxList<MemberDeclarationSyntax> GetMemberDeclarationsInCo
/// </summary>
protected override async Task<SyntaxNode> TryGetApplicableContainerFromSpanAsync(Document document, TextSpan span, CancellationToken cancellationToken)
{
var compilationUnit = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false) as CompilationUnitSyntax;
var compilationUnit = (CompilationUnitSyntax)await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
SyntaxNode container = null;
// Empty span means that user wants to move all types declared in the document to a new namespace.
......@@ -307,10 +317,7 @@ protected override async Task<SyntaxNode> TryGetApplicableContainerFromSpanAsync
// which we use the number of namespace declaration nodes to decide.
if (span.IsEmpty)
{
var hasNamespaceDecl = compilationUnit.DescendantNodes(IsCompilationUnitOrNamespaceDeclaration)
.OfType<NamespaceDeclarationSyntax>().Any();
if (hasNamespaceDecl)
if (ContainsNamespaceDeclaration(compilationUnit))
{
return null;
}
......@@ -324,12 +331,17 @@ protected override async Task<SyntaxNode> TryGetApplicableContainerFromSpanAsync
var node = compilationUnit.FindNode(span, getInnermostNodeForTie: true);
var namespaceDecl = node.AncestorsAndSelf().OfType<NamespaceDeclarationSyntax>().SingleOrDefault();
if (namespaceDecl?.Name.GetDiagnostics().All(diag => diag.DefaultSeverity != DiagnosticSeverity.Error) != true)
if (namespaceDecl == null)
{
return null;
}
if (namespaceDecl.Name.GetDiagnostics().Any(diag => diag.DefaultSeverity == DiagnosticSeverity.Error))
{
return null;
}
if (node.DescendantNodes(IsCompilationUnitOrNamespaceDeclaration).OfType<NamespaceDeclarationSyntax>().Any())
if (ContainsNamespaceDeclaration(node))
{
return null;
}
......@@ -347,8 +359,9 @@ protected override async Task<SyntaxNode> TryGetApplicableContainerFromSpanAsync
return container;
bool IsCompilationUnitOrNamespaceDeclaration(SyntaxNode n)
=> n is CompilationUnitSyntax || n is NamespaceDeclarationSyntax;
bool ContainsNamespaceDeclaration(SyntaxNode node)
=> node.DescendantNodes(n => n is CompilationUnitSyntax || n is NamespaceDeclarationSyntax)
.OfType<NamespaceDeclarationSyntax>().Any();
}
private static bool IsGlobalNamespace(ImmutableArray<string> parts)
......@@ -371,7 +384,7 @@ private static string GetAliasQualifierOpt(SyntaxNode name)
}
}
private NameSyntax CreateNameSyntax(ImmutableArray<string> namespaceParts, string aliasQualifier, int index)
private static NameSyntax CreateNameSyntax(ImmutableArray<string> namespaceParts, string aliasQualifier, int index)
{
var part = namespaceParts[index].EscapeIdentifier();
Debug.Assert(part.Length > 0);
......
......@@ -26,7 +26,7 @@ protected override async Task<SyntaxNode> TryGetApplicableInvocationNodeAsync(Do
var position = span.Start;
var compilationUnit = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false) as CompilationUnitSyntax;
var compilationUnit = (CompilationUnitSyntax)await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var namespaceDecls = compilationUnit.DescendantNodes(n => n is CompilationUnitSyntax || n is NamespaceDeclarationSyntax)
.OfType<NamespaceDeclarationSyntax>().ToImmutableArray();
......
......@@ -55,7 +55,7 @@ internal abstract class AbstractChangeNamespaceService<TNamespaceDeclarationSynt
/// <summary>
/// The annotation used to track applicable container in each document to be fixed.
/// </summary>
protected static SyntaxAnnotation[] ContainerAnnotation { get; } = new[] { new SyntaxAnnotation() };
protected static SyntaxAnnotation ContainerAnnotation { get; } = new SyntaxAnnotation();
protected static SyntaxAnnotation WarningAnnotation { get; }
= CodeActions.WarningAnnotation.Create(
......@@ -70,11 +70,20 @@ internal abstract class AbstractChangeNamespaceService<TNamespaceDeclarationSynt
protected abstract string GetDeclaredNamespace(SyntaxNode container);
protected abstract Task<ImmutableArray<(DocumentId id, SyntaxNode container)>> CanChangeNamespaceWorkerAsync(Document document, SyntaxNode container, CancellationToken cancellationToken);
/// <summary>
/// Decide if we can change the namespace for provided <paramref name="container"/> based on the criteria listed for
/// <see cref="IChangeNamespaceService.CanChangeNamespaceAsync(Document, SyntaxNode, CancellationToken)"/>
/// </summary>
/// <returns>
/// If namespace can be changed, returns a list of documents that linked to the provided document (including itself)
/// and the corresponding container nodes in each document, which will later be used for annotation. Otherwise, a
/// default ImmutableArray is returned. Currently we only support linked document in multi-targeting project scenario.
/// </returns>
protected abstract Task<ImmutableArray<(DocumentId id, SyntaxNode container)>> GetValidContainersFromAllLinkedDocumentsAsync(Document document, SyntaxNode container, CancellationToken cancellationToken);
public override async Task<bool> CanChangeNamespaceAsync(Document document, SyntaxNode container, CancellationToken cancellationToken)
{
var applicableContainers = await CanChangeNamespaceWorkerAsync(document, container, cancellationToken).ConfigureAwait(false);
var applicableContainers = await GetValidContainersFromAllLinkedDocumentsAsync(document, container, cancellationToken).ConfigureAwait(false);
return !applicableContainers.IsDefault;
}
......@@ -94,7 +103,7 @@ public override async Task<bool> CanChangeNamespaceAsync(Document document, Synt
return solution;
}
var containersFromAllDocuments = await CanChangeNamespaceWorkerAsync(document, container, cancellationToken).ConfigureAwait(false);
var containersFromAllDocuments = await GetValidContainersFromAllLinkedDocumentsAsync(document, container, cancellationToken).ConfigureAwait(false);
if (containersFromAllDocuments.IsDefault)
{
return solution;
......@@ -107,6 +116,7 @@ public override async Task<bool> CanChangeNamespaceAsync(Document document, Synt
return solution;
}
// Annotate the container nodes so we can still find and modify them after syntax tree has changed.
var annotatedSolution = await AnnotateContainersAsync(solution, containersFromAllDocuments, cancellationToken).ConfigureAwait(false);
// Here's the entire process for changing namespace:
......@@ -117,16 +127,16 @@ public override async Task<bool> CanChangeNamespaceAsync(Document document, Synt
//
// The reason for doing explicit diff merge twice is so merging after remove unnecessary imports can be correctly handled.
var ids = containersFromAllDocuments.SelectAsArray(pair => pair.id);
var documentIds = containersFromAllDocuments.SelectAsArray(pair => pair.id);
var solutionAfterNamespaceChange = annotatedSolution;
var referenceDocuments = PooledHashSet<DocumentId>.GetInstance();
try
{
foreach (var id in ids)
foreach (var documentId in documentIds)
{
var (newSolution, refDocumentIds) =
await ChangeNamespaceInSingleDocumentAsync(solutionAfterNamespaceChange, id, declaredNamespace, targetNamespace, cancellationToken)
await ChangeNamespaceInSingleDocumentAsync(solutionAfterNamespaceChange, documentId, declaredNamespace, targetNamespace, cancellationToken)
.ConfigureAwait(false);
solutionAfterNamespaceChange = newSolution;
referenceDocuments.AddRange(refDocumentIds);
......@@ -153,7 +163,7 @@ await ChangeNamespaceInSingleDocumentAsync(solutionAfterNamespaceChange, id, dec
var solutionAfterImportsRemoved = await RemoveUnnecessaryImportsAsync(
solutionAfterFirstMerge,
containersFromAllDocuments.SelectAsArray(pair => pair.id),
documentIds,
CreateAllContainingNamespaces(declaredNamespace),
cancellationToken).ConfigureAwait(false);
......@@ -191,7 +201,6 @@ await ChangeNamespaceInSingleDocumentAsync(solutionAfterNamespaceChange, id, dec
foreach (var document in documents)
{
var container = await TryGetApplicableContainerFromSpanAsync(document, span, cancellationToken).ConfigureAwait(false);
containers.Add((document.Id, container));
if (container is TNamespaceDeclarationSyntax)
{
......@@ -207,6 +216,8 @@ await ChangeNamespaceInSingleDocumentAsync(solutionAfterNamespaceChange, id, dec
{
return default;
}
containers.Add((document.Id, container));
}
return spanForContainers.Count == 1 ? containers.ToImmutable() : default;
......@@ -229,6 +240,7 @@ protected async Task<Solution> AnnotateContainersAsync(Solution solution, Immuta
var documentEditor = await solutionEditor.GetDocumentEditorAsync(id, cancellationToken).ConfigureAwait(false);
documentEditor.ReplaceNode(container, container.WithAdditionalAnnotations(ContainerAnnotation));
}
return solutionEditor.GetChangedSolution();
}
......@@ -251,6 +263,7 @@ protected async Task<Solution> AnnotateContainersAsync(Solution solution, Immuta
return true;
}
}
return false;
}
......@@ -328,6 +341,7 @@ private static SyntaxNode CreateImport(SyntaxGenerator syntaxGenerator, string n
{
import = import.WithAdditionalAnnotations(Formatter.Annotation);
}
return import;
}
......@@ -345,7 +359,7 @@ private static SyntaxNode CreateImport(SyntaxGenerator syntaxGenerator, string n
{
var document = solution.GetDocument(id);
var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var container = root.GetAnnotatedNodes(ContainerAnnotation[0]).Single();
var container = root.GetAnnotatedNodes(ContainerAnnotation).Single();
// Get types declared in the changing namespace, because we need to fix all references to them,
// e.g. change the namespace for qualified name, add imports to proper containers, etc.
......
......@@ -214,7 +214,8 @@ private static string ConcatNamespace(string rootNamespace, string namespaceSuff
/// <summary>
/// Try get the relative namespace for <paramref name="namespace"/> based on <paramref name="relativeTo"/>,
/// if <paramref name="relativeTo"/> is the containing namespace of <paramref name="namespace"/>.
/// if <paramref name="relativeTo"/> is the containing namespace of <paramref name="namespace"/>. Otherwise,
/// Returns null.
/// For example:
/// - If <paramref name="relativeTo"/> is "A.B" and <paramref name="namespace"/> is "A.B.C.D", then
/// the relative namespace is "C.D".
......
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.ChangeNamespace;
......@@ -62,9 +63,13 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
// root directory, and no namespace declaration in the document, respectively.
var service = document.GetLanguageService<IChangeNamespaceService>();
var solutionChangeAction = new SolutionChangeAction(
ChangeNamespaceActionTitle(state),
var solutionChangeAction = new ChangeNamespaceCodeAction(
state.TargetNamespace.Length == 0
? FeaturesResources.Change_to_global_namespace
: string.Format(FeaturesResources.Change_namespace_to_0, state.TargetNamespace),
token => service.ChangeNamespaceAsync(document, state.Container, state.TargetNamespace, token));
context.RegisterRefactoring(solutionChangeAction);
}
}
......@@ -83,9 +88,12 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
protected abstract string EscapeIdentifier(string identifier);
private static string ChangeNamespaceActionTitle(State state)
=> state.TargetNamespace.Length == 0
? FeaturesResources.Change_to_global_namespace
: string.Format(FeaturesResources.Change_namespace_to_0, state.TargetNamespace);
private class ChangeNamespaceCodeAction : SolutionChangeAction
{
public ChangeNamespaceCodeAction(string title, Func<CancellationToken, Task<Solution>> createChangedSolution) :
base(title, createChangedSolution)
{
}
}
}
}
......@@ -33,7 +33,7 @@ internal interface IChangeNamespaceService : ILanguageService
/// Change namespace for given <paramref name="container"/> to the name specified by <paramref name="targetNamespace"/>.
/// Everything declared in the <paramref name="container"/> will be moved to the new namespace.
/// Change will only be made if <see cref="CanChangeNamespaceAsync"/> returns <see langword="true"/> and <paramref name="targetNamespace"/>
/// is a valid name for namespace (we use "" to specify global namespace).
/// is a valid name for namespace. Use "" for <paramref name="targetNamespace"/> to specify the global namespace.
/// </summary>
Task<Solution> ChangeNamespaceAsync(Document document, SyntaxNode container, string targetNamespace, CancellationToken cancellationToken);
}
......
......@@ -39,7 +39,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ChangeNamespace
End Function
' TODO: Implement the service for VB
Protected Overrides Function CanChangeNamespaceWorkerAsync(document As Document, container As SyntaxNode, cancellationToken As CancellationToken) As Task(Of ImmutableArray(Of (DocumentId, SyntaxNode)))
Protected Overrides Function GetValidContainersFromAllLinkedDocumentsAsync(document As Document, container As SyntaxNode, cancellationToken As CancellationToken) As Task(Of ImmutableArray(Of (DocumentId, SyntaxNode)))
Return Task.FromResult(CType(Nothing, ImmutableArray(Of (DocumentId, SyntaxNode))))
End Function
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册