diff --git a/src/EditorFeatures/Test/MoveType/AbstractMoveTypeTest.cs b/src/EditorFeatures/Test/MoveType/AbstractMoveTypeTest.cs index 584edf39ec33a836618b337dd9ac867468c152ab..d8f4786e49e3ff389de5ca4a7fd3fada5c16ac89 100644 --- a/src/EditorFeatures/Test/MoveType/AbstractMoveTypeTest.cs +++ b/src/EditorFeatures/Test/MoveType/AbstractMoveTypeTest.cs @@ -17,8 +17,8 @@ namespace Microsoft.CodeAnalysis.Editor.UnitTests.MoveType { public abstract class AbstractMoveTypeTest : AbstractCodeActionTest { - private string RenameFileCodeActionTitle = FeaturesResources.Renamefileto_0; - private string RenameTypeCodeActionTitle = FeaturesResources.Renametypeto_0; + private string RenameFileCodeActionTitle = FeaturesResources.Rename_file_to_0; + private string RenameTypeCodeActionTitle = FeaturesResources.Rename_type_to_0; protected override CodeRefactoringProvider CreateCodeRefactoringProvider(Workspace workspace) { diff --git a/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.MoveTypeCodeAction.cs b/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.MoveTypeCodeAction.cs index a9dca3366a6d3b37d2c90db525c6057986fe7a36..4f16811c5bd32451473a102a4a201d26dcdfb121 100644 --- a/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.MoveTypeCodeAction.cs +++ b/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.MoveTypeCodeAction.cs @@ -34,11 +34,11 @@ private string CreateDisplayText() switch (_operationKind) { case OperationKind.MoveType: - return string.Format(FeaturesResources.Movetypeto_0, _state.TargetFileNameCandidate); + return string.Format(FeaturesResources.Move_type_to_0, _state.TargetFileNameCandidate); case OperationKind.RenameType: - return string.Format(FeaturesResources.Renametypeto_0, _state.DocumentName); + return string.Format(FeaturesResources.Rename_type_to_0, _state.DocumentName); case OperationKind.RenameFile: - return string.Format(FeaturesResources.Renamefileto_0, _state.TargetFileNameCandidate); + return string.Format(FeaturesResources.Rename_file_to_0, _state.TargetFileNameCandidate); } throw ExceptionUtilities.Unreachable; diff --git a/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.MoveTypeEditor.cs b/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.MoveTypeEditor.cs index 79312e06df0c27965fa89ce3a33a1e374220f26d..348f8848f9716eaadc3f164c0048a15e6ab05a58 100644 --- a/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.MoveTypeEditor.cs +++ b/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.MoveTypeEditor.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -41,8 +42,8 @@ internal override async Task> GetOperationsAsyn /// /// The algorithm for this, is as follows: /// 1. Fork the original document that contains the type to be moved. - /// 2. Keep the type and required namespace containers, using statements - /// and remove everything else from the forked document. + /// 2. Keep the type, required namespace containers and using statements. + /// remove everything else from the forked document. /// 3. Add this forked document to the solution. /// 4. Finally, update the original document and remove the type from it. /// @@ -52,7 +53,7 @@ private async Task> MoveTypeToNewFileAsync(stri var projectToBeUpdated = SemanticDocument.Document.Project; var newDocumentId = DocumentId.CreateNewId(projectToBeUpdated.Id, documentName); - var solutionWithNewDocument = await AddNewDocumentWithTypeDeclarationAsync( + var solutionWithNewDocument = await AddNewDocumentWithSingleTypeDeclarationAndImportsAsync( SemanticDocument, documentName, newDocumentId, State.TypeNode, CancellationToken).ConfigureAwait(false); // Get the original source document again, from the latest forked solution. @@ -60,10 +61,10 @@ private async Task> MoveTypeToNewFileAsync(stri // update source document to add partial modifiers to type chain // and/or remove type declaration from original source document. - var solutionWithBothDocumentsUpdated = await UpdateSourceDocumentAsync( + var solutionWithBothDocumentsUpdated = await RemoveTypeFromSourceDocumentAsync( sourceDocument, State.TypeNode, CancellationToken).ConfigureAwait(false); - return new CodeActionOperation[] { new ApplyChangesOperation(solutionWithBothDocumentsUpdated) }; + return SpecializedCollections.SingletonEnumerable(new ApplyChangesOperation(solutionWithBothDocumentsUpdated)); } /// @@ -76,13 +77,16 @@ private async Task> MoveTypeToNewFileAsync(stri /// type to move from original document to new document /// a cancellation token /// the new solution which contains a new document with the type being moved - private async Task AddNewDocumentWithTypeDeclarationAsync( + private async Task AddNewDocumentWithSingleTypeDeclarationAndImportsAsync( SemanticDocument sourceDocument, string newDocumentName, DocumentId newDocumentId, TTypeDeclarationSyntax typeNode, CancellationToken cancellationToken) { + Debug.Assert(sourceDocument.Document.Name != newDocumentName, + $"New document name is same as old document name:{newDocumentName}"); + var root = sourceDocument.Root; var projectToBeUpdated = sourceDocument.Document.Project; var documentEditor = await DocumentEditor.CreateAsync(sourceDocument.Document, cancellationToken).ConfigureAwait(false); @@ -106,7 +110,9 @@ private async Task> MoveTypeToNewFileAsync(stri // get the updated document, perform clean up like remove unused usings. var newDocument = solutionWithNewDocument.GetDocument(newDocumentId); - return await CleanUpDocumentAsync(newDocument, cancellationToken).ConfigureAwait(false); + newDocument = await CleanUpDocumentAsync(newDocument, cancellationToken).ConfigureAwait(false); + + return newDocument.Project.Solution; } /// @@ -117,7 +123,7 @@ private async Task> MoveTypeToNewFileAsync(stri /// type that was moved to new document /// a cancellation token /// an updated solution with the original document fixed up as appropriate. - private async Task UpdateSourceDocumentAsync( + private async Task RemoveTypeFromSourceDocumentAsync( Document sourceDocument, TTypeDeclarationSyntax typeNode, CancellationToken cancellationToken) { var documentEditor = await DocumentEditor.CreateAsync(sourceDocument, cancellationToken).ConfigureAwait(false); @@ -126,7 +132,10 @@ private async Task> MoveTypeToNewFileAsync(stri documentEditor.RemoveNode(typeNode, SyntaxRemoveOptions.KeepNoTrivia); var updatedDocument = documentEditor.GetChangedDocument(); - return await CleanUpDocumentAsync(updatedDocument, cancellationToken).ConfigureAwait(false); + + updatedDocument = await CleanUpDocumentAsync(updatedDocument, cancellationToken).ConfigureAwait(false); + + return updatedDocument.Project.Solution; } /// @@ -187,18 +196,14 @@ private static bool FilterToTopLevelMembers(SyntaxNode node, SyntaxNode typeNode private void AddPartialModifiersToTypeChain(DocumentEditor documentEditor, TTypeDeclarationSyntax typeNode) { var semanticFacts = State.SemanticDocument.Document.GetLanguageService(); + var typeChain = typeNode.Ancestors().OfType(); - if (typeNode.Parent is TTypeDeclarationSyntax) + foreach (var node in typeChain) { - var typeChain = typeNode.Ancestors().OfType(); - - foreach (var node in typeChain) + var symbol = (ITypeSymbol)State.SemanticDocument.SemanticModel.GetDeclaredSymbol(node, CancellationToken); + if (!semanticFacts.IsPartial(symbol)) { - var symbol = (ITypeSymbol)State.SemanticDocument.SemanticModel.GetDeclaredSymbol(node, CancellationToken); - if (!semanticFacts.IsPartial(symbol)) - { - documentEditor.SetModifiers(node, DeclarationModifiers.Partial); - } + documentEditor.SetModifiers(node, DeclarationModifiers.Partial); } } } @@ -206,7 +211,7 @@ private void AddPartialModifiersToTypeChain(DocumentEditor documentEditor, TType /// /// Perform clean ups on a given document. /// - private async Task CleanUpDocumentAsync(Document document, CancellationToken cancellationToken) + private async Task CleanUpDocumentAsync(Document document, CancellationToken cancellationToken) { document = await document .GetLanguageService() @@ -214,7 +219,7 @@ private async Task CleanUpDocumentAsync(Document document, Cancellatio .ConfigureAwait(false); var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - return document.Project.Solution.WithDocumentSyntaxRoot(document.Id, root, PreservationMode.PreserveIdentity); + return document.WithSyntaxRoot(root); } } } diff --git a/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.RenameTypeEditor.cs b/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.RenameTypeEditor.cs index f4802287c148ff5ff1b41485461e113dc2864566..b61ee4451f5a5ecd21d77766edecef925baadd34 100644 --- a/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.RenameTypeEditor.cs +++ b/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.RenameTypeEditor.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.Rename; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CodeRefactorings.MoveType { @@ -33,7 +34,7 @@ private async Task> RenameTypeToMatchFileAsync( // if no such conflicts exist, proceed with RenameSymbolAsync. var symbol = State.SemanticDocument.SemanticModel.GetDeclaredSymbol(State.TypeNode, CancellationToken); var newSolution = await Renamer.RenameSymbolAsync(solution, symbol, State.DocumentName, SemanticDocument.Document.Options, CancellationToken).ConfigureAwait(false); - return new CodeActionOperation[] { new ApplyChangesOperation(newSolution) }; + return SpecializedCollections.SingletonEnumerable(new ApplyChangesOperation(newSolution)); } } } diff --git a/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.State.cs b/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.State.cs index 673d2cd082b1886bdc3212355eb2d0e27aeea045..543c59749b2ae17774ba33bea6084596bc7991c2 100644 --- a/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.State.cs +++ b/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.State.cs @@ -11,7 +11,7 @@ namespace Microsoft.CodeAnalysis.CodeRefactorings.MoveType { internal abstract partial class AbstractMoveTypeService { - protected class State + private class State { private readonly TService _service; public SemanticDocument SemanticDocument { get; } @@ -20,8 +20,9 @@ protected class State public string TypeName { get; set; } public string DocumentName { get; set; } public string TargetFileNameCandidate { get; set; } + public bool IsDocumentNameAValidIdentifier { get; set; } - private State(TService service,SemanticDocument document) + private State(TService service, SemanticDocument document) { this._service = service; this.SemanticDocument = document; @@ -51,7 +52,7 @@ internal static State Generate(TService service, SemanticDocument document, Text var root = this.SemanticDocument.Root; var syntaxFacts = this.SemanticDocument.Project.LanguageServices.GetService(); - var typeDeclaration = _service.GetNodetoAnalyze(root, textSpan) as TTypeDeclarationSyntax; + var typeDeclaration = _service.GetNodeToAnalyze(root, textSpan) as TTypeDeclarationSyntax; if (typeDeclaration == null) { return false; @@ -71,15 +72,16 @@ internal static State Generate(TService service, SemanticDocument document, Text TypeNode = typeDeclaration; TypeName = typeSymbol.Name; DocumentName = Path.GetFileNameWithoutExtension(this.SemanticDocument.Document.Name); + IsDocumentNameAValidIdentifier = syntaxFacts.IsValidIdentifier(DocumentName); - if (string.Equals(DocumentName, TypeName, StringComparison.CurrentCultureIgnoreCase)) + // TODO: Make this check better, it won't detect Outer.Inner.cs cases. + if (string.Equals(DocumentName, TypeName, StringComparison.CurrentCulture)) { - // if type name matches document name, we have nothing more to do. + // if type name matches document name in a case sensitive manner, we have nothing more to do. return false; } - TargetFileNameCandidate = - typeSymbol.Name + (SemanticDocument.Document.Project.Language == LanguageNames.CSharp ? ".cs" : ".vb"); + TargetFileNameCandidate = Path.Combine(typeSymbol.Name + Path.GetExtension(this.SemanticDocument.Document.Name)); return true; } diff --git a/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.cs b/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.cs index d09dbc7140165af1015d270112ce046789113183..f4a227b526dfa74727b680e926e8effe4aeac630 100644 --- a/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.cs +++ b/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.cs @@ -27,10 +27,10 @@ internal enum OperationKind public bool ShouldAnalyze(SyntaxNode root, TextSpan span) { - return GetNodetoAnalyze(root, span) is TTypeDeclarationSyntax; + return GetNodeToAnalyze(root, span) is TTypeDeclarationSyntax; } - protected virtual SyntaxNode GetNodetoAnalyze(SyntaxNode root, TextSpan span) + protected virtual SyntaxNode GetNodeToAnalyze(SyntaxNode root, TextSpan span) { return root.FindNode(span); } @@ -74,7 +74,13 @@ private List CreateActions(State state, CancellationToken cancellati // one type declaration in current document. No moving around required, just sync // document name and type name by offering rename in both directions between type and document. actions.Add(GetCodeAction(state, operationKind: OperationKind.RenameFile)); - actions.Add(GetCodeAction(state, operationKind: OperationKind.RenameType)); + + // only if the document name can be legal identifier in the language, + // offer to rename type with document name + if (state.IsDocumentNameAValidIdentifier) + { + actions.Add(GetCodeAction(state, operationKind: OperationKind.RenameType)); + } } else { diff --git a/src/Features/Core/Portable/FeaturesResources.resx b/src/Features/Core/Portable/FeaturesResources.resx index e7b51b825a51e673bf083ddd5236a489f4180a9d..e016a430072a92d6a34c2d08c347744951b44063 100644 --- a/src/Features/Core/Portable/FeaturesResources.resx +++ b/src/Features/Core/Portable/FeaturesResources.resx @@ -1035,13 +1035,13 @@ This version used in: {2} Convert to interpolated string - + Move type to {0} - + Rename file to {0} - + Rename type to {0} \ No newline at end of file diff --git a/src/Features/VisualBasic/Portable/CodeRefactorings/MoveType/VisualBasicMoveTypeService.vb b/src/Features/VisualBasic/Portable/CodeRefactorings/MoveType/VisualBasicMoveTypeService.vb index 9cd7a9fa73e4fcdd8ca5293329999eb08bce2a63..9371d0f239d48cd65b401625a61b235da5d1ae47 100644 --- a/src/Features/VisualBasic/Portable/CodeRefactorings/MoveType/VisualBasicMoveTypeService.vb +++ b/src/Features/VisualBasic/Portable/CodeRefactorings/MoveType/VisualBasicMoveTypeService.vb @@ -13,8 +13,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeRefactorings.MoveType ''' ''' Gets the TypeBlock node to analyze ''' - Protected Overrides Function GetNodetoAnalyze(root As SyntaxNode, span As TextSpan) As SyntaxNode - Dim node = MyBase.GetNodetoAnalyze(root, span) + Protected Overrides Function GetNodeToAnalyze(root As SyntaxNode, span As TextSpan) As SyntaxNode + Dim node = MyBase.GetNodeToAnalyze(root, span) If node.IsKind(SyntaxKind.ModuleStatement, SyntaxKind.ClassStatement, SyntaxKind.StructureStatement, diff --git a/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSemanticFactsService.cs b/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSemanticFactsService.cs index 82b7a02a2d94d27d5aef3ffff2e441a8863e9dd2..cf4530c249537330786cfff29f2211bfa9cc4fec 100644 --- a/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSemanticFactsService.cs +++ b/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSemanticFactsService.cs @@ -266,8 +266,7 @@ public bool IsNameOfContext(SemanticModel semanticModel, int position, Cancellat public bool IsPartial(ITypeSymbol typeSymbol) { var syntaxRefs = typeSymbol.DeclaringSyntaxReferences; - return syntaxRefs.Length > 1 - || ((BaseTypeDeclarationSyntax)syntaxRefs.Single().GetSyntax()).Modifiers.Any(SyntaxKind.PartialKeyword); + return syntaxRefs.Any(n => ((BaseTypeDeclarationSyntax)n.GetSyntax()).Modifiers.Any(SyntaxKind.PartialKeyword)); } } } diff --git a/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSemanticFactsService.vb b/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSemanticFactsService.vb index 73824ca2c3dfa7b72a834c637a91d25ba3c02de8..7433ffe7f91685f9b1a249830247f5b05ca0d36c 100644 --- a/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSemanticFactsService.vb +++ b/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSemanticFactsService.vb @@ -250,8 +250,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic Public Function IsPartial(typeSymbol As ITypeSymbol) As Boolean Implements ISemanticFactsService.IsPartial Dim syntaxRefs = typeSymbol.DeclaringSyntaxReferences - Return syntaxRefs.Length > 1 OrElse - DirectCast(syntaxRefs.Single().GetSyntax(), TypeStatementSyntax).Modifiers.Any(SyntaxKind.PartialKeyword) + Return syntaxRefs.Any( + Function(n As SyntaxReference) + Return DirectCast(n.GetSyntax(), TypeStatementSyntax).Modifiers.Any(SyntaxKind.PartialKeyword) + End Function) End Function End Class End Namespace