未验证 提交 b5d33e28 编写于 作者: G Gen Lu 提交者: GitHub

Merge pull request #38754 from genlu/FixMoveNamespace

Fix crash when moving type from global namespace
......@@ -4,8 +4,6 @@
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeRefactorings;
using Microsoft.CodeAnalysis.CSharp.MoveToNamespace;
using Microsoft.CodeAnalysis.Editor.UnitTests;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.MoveToNamespace;
......@@ -1112,5 +1110,103 @@ class MyClass
Assert.Empty(actions);
}
[WpfFact, Trait(Traits.Feature, Traits.Features.MoveToNamespace)]
[WorkItem(980758, "https://devdiv.visualstudio.com/DevDiv/_workitems/edit/980758")]
public Task MoveToNamespace_MoveOnlyTypeInGlobalNamespace()
=> TestMoveToNamespaceAsync(
@"class MyClass[||]
{
}",
expectedMarkup: @"namespace {|Warning:A|}
{
class MyClass
{
}
}",
targetNamespace: "A",
expectedSymbolChanges: new Dictionary<string, string>()
{
{"MyClass", "A.MyClass" }
});
[WpfFact, Trait(Traits.Feature, Traits.Features.MoveToNamespace)]
[WorkItem(980758, "https://devdiv.visualstudio.com/DevDiv/_workitems/edit/980758")]
public async Task MoveToNamespace_MoveOnlyTypeToGlobalNamespace()
{
// We will not get "" as target namespace in VS, but the refactoring should be able
// to handle it w/o crashing.
await TestMoveToNamespaceAsync(
@"namespace A
{
class MyClass[||]
{
}
}",
expectedMarkup: @"namespace A
{
class MyClass
{
}
}",
targetNamespace: "");
}
[WpfFact, Trait(Traits.Feature, Traits.Features.MoveToNamespace)]
[WorkItem(980758, "https://devdiv.visualstudio.com/DevDiv/_workitems/edit/980758")]
public Task MoveToNamespace_MoveOneTypeInGlobalNamespace()
=> TestMoveToNamespaceAsync(
@"class MyClass1[||]
{
}
class MyClass2
{
}",
expectedSuccess: false);
[WpfFact, Trait(Traits.Feature, Traits.Features.MoveToNamespace)]
[WorkItem(980758, "https://devdiv.visualstudio.com/DevDiv/_workitems/edit/980758")]
public Task MoveToNamespace_PartialTypesInNamesapce_SelectType()
=> TestMoveToNamespaceAsync(
@"namespace NS
{
partial class MyClass[||]
{
}
partial class MyClass
{
}
}",
expectedSuccess: false);
[WpfFact, Trait(Traits.Feature, Traits.Features.MoveToNamespace)]
[WorkItem(980758, "https://devdiv.visualstudio.com/DevDiv/_workitems/edit/980758")]
public Task MoveToNamespace_PartialTypesInNamesapce_SelectNamespace()
=> TestMoveToNamespaceAsync(
@"namespace NS[||]
{
partial class MyClass
{
}
partial class MyClass
{
}
}",
expectedSuccess: false);
[WpfFact, Trait(Traits.Feature, Traits.Features.MoveToNamespace)]
[WorkItem(980758, "https://devdiv.visualstudio.com/DevDiv/_workitems/edit/980758")]
public Task MoveToNamespace_PartialTypesInGlobalNamesapce()
=> TestMoveToNamespaceAsync(
@"partial class MyClass[||]
{
}
partial class MyClass
{
}",
expectedSuccess: false);
}
}
......@@ -31,16 +31,9 @@ protected override CodeRefactoringProvider CreateCodeRefactoringProvider(Workspa
{
testParameters ??= new TestParameters();
var moveToNamespaceOptions = TestMoveToNamespaceOptionsService.DefaultOptions;
if (optionCancelled)
{
moveToNamespaceOptions = MoveToNamespaceOptionsResult.Cancelled;
}
else if (!string.IsNullOrEmpty(targetNamespace))
{
moveToNamespaceOptions = new MoveToNamespaceOptionsResult(targetNamespace);
}
var moveToNamespaceOptions = optionCancelled
? MoveToNamespaceOptionsResult.Cancelled
: new MoveToNamespaceOptionsResult(targetNamespace);
var workspace = CreateWorkspaceFromFile(markup, testParameters.Value);
using var testState = new TestState(workspace);
......@@ -61,7 +54,7 @@ protected override CodeRefactoringProvider CreateCodeRefactoringProvider(Workspa
{
var operations = await task;
if (optionCancelled)
if (optionCancelled || string.IsNullOrEmpty(targetNamespace))
{
Assert.Empty(operations);
}
......@@ -93,7 +86,7 @@ protected override CodeRefactoringProvider CreateCodeRefactoringProvider(Workspa
}
if (!optionCancelled)
if (!optionCancelled && !string.IsNullOrEmpty(targetNamespace))
{
await TestInRegularAndScriptAsync(markup, expectedMarkup);
}
......
// 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.Collections.Immutable;
using System.Composition;
using Microsoft.CodeAnalysis.LanguageServices;
using Microsoft.CodeAnalysis.MoveToNamespace;
using System.Composition;
namespace Microsoft.CodeAnalysis.Test.Utilities.MoveToNamespace
{
[Export(typeof(IMoveToNamespaceOptionsService)), Shared]
[PartNotDiscoverable]
class TestMoveToNamespaceOptionsService : IMoveToNamespaceOptionsService
internal class TestMoveToNamespaceOptionsService : IMoveToNamespaceOptionsService
{
internal static readonly MoveToNamespaceOptionsResult DefaultOptions = new MoveToNamespaceOptionsResult("TestNewNamespaceValue");
private MoveToNamespaceOptionsResult _optionsResult = DefaultOptions;
private MoveToNamespaceOptionsResult OptionsResult { get; set; }
[ImportingConstructor]
public TestMoveToNamespaceOptionsService()
......@@ -20,9 +19,9 @@ public TestMoveToNamespaceOptionsService()
}
public MoveToNamespaceOptionsResult GetChangeNamespaceOptions(string defaultNamespace, ImmutableArray<string> availableNamespaces, ISyntaxFactsService syntaxFactsService)
=> _optionsResult;
=> OptionsResult;
internal void SetOptions(MoveToNamespaceOptionsResult moveToNamespaceOptions)
=> _optionsResult = moveToNamespaceOptions;
=> OptionsResult = moveToNamespaceOptions;
}
}
......@@ -5,12 +5,13 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.MoveToNamespace;
using Roslyn.Utilities;
namespace Microsoft.CodeAnalysis.CSharp.MoveToNamespace
{
[ExportLanguageService(typeof(IMoveToNamespaceService), LanguageNames.CSharp), Shared]
internal class CSharpMoveToNamespaceService :
AbstractMoveToNamespaceService<NamespaceDeclarationSyntax, TypeDeclarationSyntax>
AbstractMoveToNamespaceService<CompilationUnitSyntax, NamespaceDeclarationSyntax, TypeDeclarationSyntax>
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
......@@ -20,19 +21,13 @@ internal class CSharpMoveToNamespaceService :
{
}
protected override string GetNamespaceName(NamespaceDeclarationSyntax namespaceSyntax)
=> namespaceSyntax.Name.ToString();
protected override string GetNamespaceName(TypeDeclarationSyntax typeDeclarationSyntax)
{
var namespaceDecl = typeDeclarationSyntax.FirstAncestorOrSelf<NamespaceDeclarationSyntax>();
if (namespaceDecl == null)
protected override string GetNamespaceName(SyntaxNode container)
=> container switch
{
return string.Empty;
}
return GetNamespaceName(namespaceDecl);
}
NamespaceDeclarationSyntax namespaceSyntax => namespaceSyntax.Name.ToString(),
CompilationUnitSyntax compilationUnit => string.Empty,
_ => throw ExceptionUtilities.UnexpectedValue(container)
};
protected override bool IsContainedInNamespaceDeclaration(NamespaceDeclarationSyntax namespaceDeclaration, int position)
{
......
......@@ -33,7 +33,10 @@ public override object GetOptions(CancellationToken cancellationToken)
protected override async Task<IEnumerable<CodeActionOperation>> ComputeOperationsAsync(object options, CancellationToken cancellationToken)
{
if (options is MoveToNamespaceOptionsResult moveToNamespaceOptions && !moveToNamespaceOptions.IsCancelled)
// We won't get an empty target namespace from VS, but still should handle it w/o crashing.
if (options is MoveToNamespaceOptionsResult moveToNamespaceOptions &&
!moveToNamespaceOptions.IsCancelled &&
!string.IsNullOrEmpty(moveToNamespaceOptions.Namespace))
{
var moveToNamespaceResult = await _moveToNamespaceService.MoveToNamespaceAsync(
_moveToNamespaceAnalysisResult,
......@@ -80,7 +83,7 @@ private static ImmutableArray<CodeActionOperation> CreateRenameOperations(MoveTo
public static AbstractMoveToNamespaceCodeAction Generate(IMoveToNamespaceService changeNamespaceService, MoveToNamespaceAnalysisResult analysisResult)
=> analysisResult.Container switch
{
MoveToNamespaceAnalysisResult.ContainerType.NamedType => (AbstractMoveToNamespaceCodeAction)new MoveTypeToNamespaceCodeAction(changeNamespaceService, analysisResult),
MoveToNamespaceAnalysisResult.ContainerType.NamedType => new MoveTypeToNamespaceCodeAction(changeNamespaceService, analysisResult),
MoveToNamespaceAnalysisResult.ContainerType.Namespace => new MoveItemsToNamespaceCodeAction(changeNamespaceService, analysisResult),
_ => throw ExceptionUtilities.UnexpectedValue(analysisResult.Container)
};
......
......@@ -13,6 +13,7 @@
using Microsoft.CodeAnalysis.LanguageServices;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;
namespace Microsoft.CodeAnalysis.MoveToNamespace
{
......@@ -25,14 +26,14 @@ internal interface IMoveToNamespaceService : ILanguageService
IMoveToNamespaceOptionsService OptionsService { get; }
}
internal abstract class AbstractMoveToNamespaceService<TNamespaceDeclarationSyntax, TNamedTypeDeclarationSyntax>
internal abstract class AbstractMoveToNamespaceService<TCompilationUnitSyntax, TNamespaceDeclarationSyntax, TNamedTypeDeclarationSyntax>
: IMoveToNamespaceService
where TCompilationUnitSyntax : SyntaxNode
where TNamespaceDeclarationSyntax : SyntaxNode
where TNamedTypeDeclarationSyntax : SyntaxNode
{
protected abstract string GetNamespaceName(TNamespaceDeclarationSyntax namespaceSyntax);
protected abstract string GetNamespaceName(TNamedTypeDeclarationSyntax namedTypeSyntax);
protected abstract string GetNamespaceName(SyntaxNode namespaceSyntax);
protected abstract bool IsContainedInNamespaceDeclaration(TNamespaceDeclarationSyntax namespaceSyntax, int position);
public IMoveToNamespaceOptionsService OptionsService { get; }
......@@ -92,42 +93,69 @@ protected AbstractMoveToNamespaceService(IMoveToNamespaceOptionsService moveToNa
return null;
}
if (ContainsNamespaceDeclaration(declarationSyntax) || ContainsMultipleNamespaceInSpine(declarationSyntax))
// The underlying ChangeNamespace service doesn't support nested namespace decalration.
if (GetNamespaceInSpineCount(declarationSyntax) == 1)
{
return MoveToNamespaceAnalysisResult.Invalid;
}
else
{
var namespaceName = GetNamespaceName(declarationSyntax);
var namespaces = await GetNamespacesAsync(document, cancellationToken).ConfigureAwait(false);
return new MoveToNamespaceAnalysisResult(document, declarationSyntax, namespaceName, namespaces.ToImmutableArray(), MoveToNamespaceAnalysisResult.ContainerType.Namespace);
var changeNamespaceService = document.GetLanguageService<IChangeNamespaceService>();
if (await changeNamespaceService.CanChangeNamespaceAsync(document, declarationSyntax, cancellationToken).ConfigureAwait(false))
{
var namespaceName = GetNamespaceName(declarationSyntax);
var namespaces = await GetNamespacesAsync(document, cancellationToken).ConfigureAwait(false);
return new MoveToNamespaceAnalysisResult(document, declarationSyntax, namespaceName, namespaces.ToImmutableArray(), MoveToNamespaceAnalysisResult.ContainerType.Namespace);
}
}
return MoveToNamespaceAnalysisResult.Invalid;
}
private async Task<MoveToNamespaceAnalysisResult> TryAnalyzeNamedTypeAsync(
Document document, SyntaxNode node, CancellationToken cancellationToken)
{
// Multiple nested namespaces are currently not supported
if (ContainsMultipleNamespaceInSpine(node) || ContainsMultipleTypesInSpine(node))
var namespaceInSpineCount = GetNamespaceInSpineCount(node);
// Nested namespaces are currently not supported by the underlying ChangeNamespace service
if (namespaceInSpineCount > 1 || ContainsMultipleTypesInSpine(node))
{
return MoveToNamespaceAnalysisResult.Invalid;
}
SyntaxNode container = null;
// Moving one of the many members declared in global namespace is not currently supported,
// but if it's the only member declared, then that's fine.
if (namespaceInSpineCount == 0)
{
container = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var syntaxFacts = document.GetLanguageService<ISyntaxFactsService>();
if (syntaxFacts.GetMembersOfCompilationUnit(container).Count > 1)
{
return MoveToNamespaceAnalysisResult.Invalid;
}
}
if (node is TNamedTypeDeclarationSyntax namedTypeDeclarationSyntax)
{
var namespaceName = GetNamespaceName(namedTypeDeclarationSyntax);
var namespaces = await GetNamespacesAsync(document, cancellationToken).ConfigureAwait(false);
return new MoveToNamespaceAnalysisResult(document, namedTypeDeclarationSyntax, namespaceName, namespaces.ToImmutableArray(), MoveToNamespaceAnalysisResult.ContainerType.NamedType);
// If we are inside a namespace declaration, then find it as the container.
container ??= GetContainingNamespace(namedTypeDeclarationSyntax);
var changeNamespaceService = document.GetLanguageService<IChangeNamespaceService>();
if (await changeNamespaceService.CanChangeNamespaceAsync(document, container, cancellationToken).ConfigureAwait(false))
{
var namespaces = await GetNamespacesAsync(document, cancellationToken).ConfigureAwait(false);
return new MoveToNamespaceAnalysisResult(document, namedTypeDeclarationSyntax, GetNamespaceName(container), namespaces.ToImmutableArray(), MoveToNamespaceAnalysisResult.ContainerType.NamedType);
}
}
return null;
return MoveToNamespaceAnalysisResult.Invalid;
}
private bool ContainsNamespaceDeclaration(SyntaxNode node)
=> node.DescendantNodes().OfType<TNamespaceDeclarationSyntax>().Any();
private static TNamespaceDeclarationSyntax GetContainingNamespace(TNamedTypeDeclarationSyntax namedTypeSyntax)
=> namedTypeSyntax.FirstAncestorOrSelf<TNamespaceDeclarationSyntax>();
private static bool ContainsMultipleNamespaceInSpine(SyntaxNode node)
=> node.AncestorsAndSelf().OfType<TNamespaceDeclarationSyntax>().Count() > 1;
private static int GetNamespaceInSpineCount(SyntaxNode node)
=> node.AncestorsAndSelf().OfType<TNamespaceDeclarationSyntax>().Count() + node.DescendantNodes().OfType<TNamespaceDeclarationSyntax>().Count();
private static bool ContainsMultipleTypesInSpine(SyntaxNode node)
=> node.AncestorsAndSelf().OfType<TNamedTypeDeclarationSyntax>().Count() > 1;
......@@ -153,17 +181,38 @@ private static bool ContainsMultipleTypesInSpine(SyntaxNode node)
}
}
private static async Task<ImmutableArray<ISymbol>> GetMemberSymbolsAsync(Document document, SyntaxNode container, CancellationToken cancellationToken)
{
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
switch (container)
{
case TNamespaceDeclarationSyntax namespaceNode:
var containerSymbol = (INamespaceSymbol)semanticModel.GetDeclaredSymbol(container);
return containerSymbol.GetMembers().SelectAsArray(m => (ISymbol)m);
case TCompilationUnitSyntax compilationUnit:
var syntaxFacts = document.GetLanguageService<ISyntaxFactsService>();
var members = syntaxFacts.GetMembersOfCompilationUnit(compilationUnit);
// We are trying to move a selected type from global namespace to the target namespace.
// This is supported if the selected type is the only member declared in the global namespace in this document.
// (See `TryAnalyzeNamedTypeAsync`)
Debug.Assert(members.Count == 1);
return members.SelectAsArray(member => semanticModel.GetDeclaredSymbol(member));
default:
throw ExceptionUtilities.UnexpectedValue(container);
}
}
private static async Task<MoveToNamespaceResult> MoveItemsInNamespaceAsync(
Document document,
SyntaxNode container,
string targetNamespace,
CancellationToken cancellationToken)
{
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var containerSymbol = (INamespaceSymbol)semanticModel.GetDeclaredSymbol(container);
var members = containerSymbol.GetMembers();
var newNameOriginalSymbolMapping = members
.ToImmutableDictionary(symbol => GetNewSymbolName(symbol, targetNamespace), symbol => (ISymbol)symbol);
var memberSymbols = await GetMemberSymbolsAsync(document, container, cancellationToken).ConfigureAwait(false);
var newNameOriginalSymbolMapping = memberSymbols
.ToImmutableDictionary(symbol => GetNewSymbolName(symbol, targetNamespace), symbol => symbol);
var changeNamespaceService = document.GetLanguageService<IChangeNamespaceService>();
if (changeNamespaceService == null)
......@@ -172,7 +221,6 @@ private static bool ContainsMultipleTypesInSpine(SyntaxNode node)
}
var originalSolution = document.Project.Solution;
var typeDeclarationsInContainer = container.DescendantNodes(syntaxNode => syntaxNode is TNamedTypeDeclarationSyntax).ToImmutableArray();
var changedSolution = await changeNamespaceService.ChangeNamespaceAsync(
document,
......@@ -211,7 +259,8 @@ private static bool ContainsMultipleTypesInSpine(SyntaxNode node)
var syntaxNode = syntaxRoot.GetAnnotatedNodes(AbstractMoveTypeService.NamespaceScopeMovedAnnotation).SingleOrDefault();
if (syntaxNode == null)
{
syntaxNode = container.FirstAncestorOrSelf<TNamespaceDeclarationSyntax>();
// The type might be declared in global namespace
syntaxNode = container.FirstAncestorOrSelf<TNamespaceDeclarationSyntax>() ?? syntaxRoot;
}
return await MoveItemsInNamespaceAsync(
......@@ -223,8 +272,13 @@ private static bool ContainsMultipleTypesInSpine(SyntaxNode node)
private static string GetNewSymbolName(ISymbol symbol, string targetNamespace)
{
Debug.Assert(symbol != null);
return targetNamespace + symbol.ToDisplayString().Substring(symbol.ContainingNamespace.ToDisplayString().Length);
Debug.Assert(symbol != null && !string.IsNullOrEmpty(targetNamespace));
var offset = symbol.ContainingNamespace.IsGlobalNamespace
? 0
: symbol.ContainingNamespace.ToDisplayString().Length + 1;
return $"{targetNamespace}.{symbol.ToDisplayString().Substring(offset)}";
}
private static readonly SymbolDisplayFormat QualifiedNamespaceFormat = new SymbolDisplayFormat(
......
......@@ -951,6 +951,9 @@ public SyntaxList<SyntaxNode> GetMembersOfTypeDeclaration(SyntaxNode typeDeclara
public SyntaxList<SyntaxNode> GetMembersOfNamespaceDeclaration(SyntaxNode namespaceDeclaration)
=> ((NamespaceDeclarationSyntax)namespaceDeclaration).Members;
public SyntaxList<SyntaxNode> GetMembersOfCompilationUnit(SyntaxNode compilationUnit)
=> ((CompilationUnitSyntax)compilationUnit).Members;
private void AppendMethodLevelMembers(SyntaxNode node, List<SyntaxNode> list)
{
foreach (var member in node.GetMembers())
......
......@@ -378,6 +378,7 @@ internal interface ISyntaxFactsService : ILanguageService
List<SyntaxNode> GetMethodLevelMembers(SyntaxNode root);
SyntaxList<SyntaxNode> GetMembersOfTypeDeclaration(SyntaxNode typeDeclaration);
SyntaxList<SyntaxNode> GetMembersOfNamespaceDeclaration(SyntaxNode namespaceDeclaration);
SyntaxList<SyntaxNode> GetMembersOfCompilationUnit(SyntaxNode compilationUnit);
bool ContainsInMemberBody(SyntaxNode node, TextSpan span);
int GetMethodLevelMemberId(SyntaxNode root, SyntaxNode node);
......
......@@ -982,6 +982,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
Return DirectCast(namespaceDeclaration, NamespaceBlockSyntax).Members
End Function
Public Function GetMembersOfCompilationUnit(compilationUnit As SyntaxNode) As SyntaxList(Of SyntaxNode) Implements ISyntaxFactsService.GetMembersOfCompilationUnit
Return DirectCast(compilationUnit, CompilationUnitSyntax).Members
End Function
Public Function IsTopLevelNodeWithMembers(node As SyntaxNode) As Boolean Implements ISyntaxFactsService.IsTopLevelNodeWithMembers
Return TypeOf node Is NamespaceBlockSyntax OrElse
TypeOf node Is TypeBlockSyntax OrElse
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册