From 17c380e69543c93865debc50b47a52506a1e81e0 Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Fri, 13 Sep 2019 13:54:28 -0700 Subject: [PATCH 1/9] Modify rename to always annotate symbol declarations that were renamed. This allows IVsRefactorNotify to be handled automatically when calling Solution.TryApplyChanges --- .../NamingStyles/NamingStylesTests.cs | 53 ++++++- .../MoveToNamespace/MoveToNamespaceTests.cs | 3 +- .../Test2/Rename/RenameEngineResult.vb | 26 +++ .../AbstractMoveToNamespaceTests.cs | 136 ++++++++++------ .../Workspaces/SymbolRenameEventArgs.cs | 20 +++ .../Workspaces/TestRefactorNotify.cs | 29 ++++ ...eActionOperationFactoryWorkspaceService.cs | 39 ----- .../TestUtilities/Workspaces/TestWorkspace.cs | 48 ++++++ .../CSharpChangeNamespaceService.cs | 28 +++- ...arpSyncNamespaceCodeRefactoringProvider.cs | 3 - .../NamingStyle/NamingStyleCodeFixProvider.cs | 43 +---- .../MoveType/AbstractMoveTypeService.cs | 23 +-- .../AbstractChangeNamespaceService.cs | 148 ++++++++++++++---- ...cNamespaceCodeRefactoringProvider.State.cs | 74 ++------- ...actSyncNamespaceCodeRefactoringProvider.cs | 2 - .../SyncNamespace/IChangeNamespaceService.cs | 7 + ...eActionOperationFactoryWorkspaceService.cs | 11 -- .../AbstractMoveToNamespaceCodeAction.cs | 21 --- .../VisualBasicChangeNamespaceService.vb | 6 +- .../Impl/CodeModel/CSharpCodeModelService.cs | 4 +- .../CSharpCodeModelServiceFactory.cs | 4 +- .../VisualStudioWorkspaceImpl.cs | 50 +++++- ...isualStudioRenameTrackingDismissService.cs | 26 +++ ...eActionOperationFactoryWorkspaceService.cs | 82 ---------- ...osoft.VisualStudio.LanguageServices.csproj | 3 + .../CodeModel/AbstractCodeModelService.cs | 12 -- ...alStudio.LanguageServices.UnitTests.vbproj | 3 + .../CodeModel/VisualBasicCodeModelService.vb | 3 +- .../VisualBasicCodeModelServiceFactory.vb | 5 +- .../CSharpSyntaxFactsService.cs | 3 + .../SyntaxFactsService/ISyntaxFactsService.cs | 1 + .../ConflictEngine/ConflictResolution.cs | 1 + .../ConflictResolver.Session.cs | 82 ++++++---- .../Portable/Rename/RenameSymbolAnnotation.cs | 107 +++++++++++++ .../VisualBasicSyntaxFactsService.vb | 4 + 35 files changed, 692 insertions(+), 418 deletions(-) create mode 100644 src/EditorFeatures/TestUtilities/Workspaces/SymbolRenameEventArgs.cs create mode 100644 src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs delete mode 100644 src/EditorFeatures/TestUtilities/Workspaces/TestSymbolRenamedCodeActionOperationFactoryWorkspaceService.cs delete mode 100644 src/Features/Core/Portable/CodeRefactorings/WorkspaceServices/ISymbolRenamedCodeActionOperationFactoryWorkspaceService.cs create mode 100644 src/VisualStudio/Core/Def/Implementation/VisualStudioRenameTrackingDismissService.cs delete mode 100644 src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioSymbolRenamedCodeActionOperationFactoryWorkspaceService.cs create mode 100644 src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/NamingStyles/NamingStylesTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/NamingStyles/NamingStylesTests.cs index 87d247a37e7ee..fb579e4402614 100644 --- a/src/EditorFeatures/CSharpTest/Diagnostics/NamingStyles/NamingStylesTests.cs +++ b/src/EditorFeatures/CSharpTest/Diagnostics/NamingStyles/NamingStylesTests.cs @@ -8,9 +8,12 @@ using Microsoft.CodeAnalysis.CSharp.Diagnostics.NamingStyles; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Diagnostics.Analyzers.NamingStyles; +using Microsoft.CodeAnalysis.Editor.UnitTests; using Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics.NamingStyles; using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces; using Microsoft.CodeAnalysis.Test.Utilities; +using Microsoft.CodeAnalysis.Test.Utilities.Workspaces; +using Microsoft.VisualStudio.Composition; using Roslyn.Test.Utilities; using Xunit; @@ -20,6 +23,17 @@ public class NamingStylesTests : AbstractCSharpDiagnosticProviderBasedUserDiagno { private readonly NamingStylesTestOptionSets options = new NamingStylesTestOptionSets(LanguageNames.CSharp); + private static readonly IExportProviderFactory ExportProviderFactory = + ExportProviderCache.GetOrCreateExportProviderFactory( + TestExportProvider.EntireAssemblyCatalogWithCSharpAndVisualBasic.WithPart(typeof(TestRefactorNotify))); + + protected override TestWorkspace CreateWorkspaceFromFile(string initialMarkup, TestParameters parameters) + => CreateWorkspaceFromFile(initialMarkup, parameters, ExportProviderFactory); + + protected TestWorkspace CreateWorkspaceFromFile(string initialMarkup, TestParameters parameters, IExportProviderFactory exportProviderFactory) + => TestWorkspace.CreateCSharp(initialMarkup, parameters.parseOptions, parameters.compilationOptions, exportProvider: exportProviderFactory.CreateExportProvider()); + + internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace) => (new CSharpNamingStyleDiagnosticAnalyzer(), new NamingStyleCodeFixProvider()); @@ -1217,17 +1231,42 @@ public async Task TestRefactorNotify() var testParameters = new TestParameters(options: options.ClassNamesArePascalCase); using var workspace = CreateWorkspaceFromOptions(markup, testParameters); + + var refactorNotify = workspace.GetService() as TestRefactorNotify; + Assert.NotNull(refactorNotify); + + var beforeCalled = false; + var afterCalled = false; + TestRefactorNotify.SymbolRenamedEventHandler beforeRename = (args) => + { + Assert.Equal("C", args.NewName); + Assert.False(beforeCalled); + beforeCalled = true; + }; + + TestRefactorNotify.SymbolRenamedEventHandler afterRename = (args) => + { + Assert.Equal("C", args.NewName); + Assert.False(afterCalled); + afterCalled = true; + }; + + refactorNotify.OnBeforeRename += beforeRename; + refactorNotify.OnAfterRename += afterRename; + var (_, action) = await GetCodeActionsAsync(workspace, testParameters); + var operations = await action.GetOperationsAsync(CancellationToken.None); - var previewOperations = await action.GetPreviewOperationsAsync(CancellationToken.None); - Assert.Empty(previewOperations.OfType()); + foreach (var operation in operations) + { + operation.Apply(workspace, CancellationToken.None); + } - var commitOperations = await action.GetOperationsAsync(CancellationToken.None); - Assert.Equal(2, commitOperations.Length); + Assert.True(beforeCalled); + Assert.True(afterCalled); - var symbolRenamedOperation = (TestSymbolRenamedCodeActionOperationFactoryWorkspaceService.Operation)commitOperations[1]; - Assert.Equal("c", symbolRenamedOperation._symbol.Name); - Assert.Equal("C", symbolRenamedOperation._newName); + refactorNotify.OnBeforeRename -= beforeRename; + refactorNotify.OnAfterRename -= afterRename; } } } diff --git a/src/EditorFeatures/CSharpTest/MoveToNamespace/MoveToNamespaceTests.cs b/src/EditorFeatures/CSharpTest/MoveToNamespace/MoveToNamespaceTests.cs index 17e16978f5583..12f6b4b8adfd7 100644 --- a/src/EditorFeatures/CSharpTest/MoveToNamespace/MoveToNamespaceTests.cs +++ b/src/EditorFeatures/CSharpTest/MoveToNamespace/MoveToNamespaceTests.cs @@ -10,6 +10,7 @@ using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Test.Utilities; using Microsoft.CodeAnalysis.Test.Utilities.MoveToNamespace; +using Microsoft.CodeAnalysis.Test.Utilities.Workspaces; using Microsoft.VisualStudio.Composition; using Roslyn.Test.Utilities; using Xunit; @@ -21,7 +22,7 @@ public class MoveToNamespaceTests : AbstractMoveToNamespaceTests { private static readonly IExportProviderFactory ExportProviderFactory = ExportProviderCache.GetOrCreateExportProviderFactory( - TestExportProvider.EntireAssemblyCatalogWithCSharpAndVisualBasic.WithPart(typeof(TestMoveToNamespaceOptionsService))); + TestExportProvider.EntireAssemblyCatalogWithCSharpAndVisualBasic.WithPart(typeof(TestMoveToNamespaceOptionsService)).WithPart(typeof(TestRefactorNotify))); protected override TestWorkspace CreateWorkspaceFromFile(string initialMarkup, TestParameters parameters) => CreateWorkspaceFromFile(initialMarkup, parameters, ExportProviderFactory); diff --git a/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb b/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb index 926b62a072d69..a9966955fe73f 100644 --- a/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb +++ b/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb @@ -8,6 +8,7 @@ Imports Microsoft.VisualStudio.Text Imports Xunit.Sdk Imports Microsoft.CodeAnalysis.Options Imports Xunit.Abstractions +Imports Roslyn.Utilities Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename ''' @@ -81,6 +82,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename engineResult = New RenameEngineResult(workspace, result, renameTo) engineResult.AssertUnlabeledSpansRenamedAndHaveNoConflicts() + engineResult.AssertChangedSymbolsAreAnnotated(symbol) Catch ' Something blew up, so we still own the test workspace If engineResult IsNot Nothing Then @@ -143,6 +145,30 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename Next End Sub + Public Sub AssertChangedSymbolsAreAnnotated(symbol As ISymbol) + + If Not RenameSymbolAnnotation.ShouldAnnotateSymbol(symbol) Then + Return + End If + + For Each location In _resolution.RelatedLocations + Dim documentId = location.DocumentId + Dim document = _resolution.NewSolution.GetDocument(documentId) + Dim root = document.GetSyntaxRootSynchronously(CancellationToken.None) + + Dim node = root.FindNode(location.ComplexifiedTargetSpan) + + Dim renameAnnotatedNodes = root.GetAnnotatedNodes(RenameSymbolAnnotation.RenameSymbolKind) + + ' Not all related locations will get annotated, we just need one + If renameAnnotatedNodes.Any() Then + Return + End If + Next + + Throw New Exception("Did not find any rename annotated nodes") + End Sub + Private Function GetLabeledLocations(label As String) As IEnumerable(Of Location) Dim locations As New List(Of Location) diff --git a/src/EditorFeatures/TestUtilities/MoveToNamespace/AbstractMoveToNamespaceTests.cs b/src/EditorFeatures/TestUtilities/MoveToNamespace/AbstractMoveToNamespaceTests.cs index 4dc07e0e77662..e151a66d2299e 100644 --- a/src/EditorFeatures/TestUtilities/MoveToNamespace/AbstractMoveToNamespaceTests.cs +++ b/src/EditorFeatures/TestUtilities/MoveToNamespace/AbstractMoveToNamespaceTests.cs @@ -5,10 +5,14 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeRefactorings; +using Microsoft.CodeAnalysis.Editor; using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions; using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces; using Microsoft.CodeAnalysis.MoveToNamespace; +using Microsoft.CodeAnalysis.Test.Utilities.Workspaces; +using Roslyn.Utilities; using Xunit; namespace Microsoft.CodeAnalysis.Test.Utilities.MoveToNamespace @@ -25,9 +29,7 @@ public async Task TestMoveToNamespaceAsync( TestParameters? testParameters = null, string targetNamespace = null, bool optionCancelled = false, - bool testAnalysis = false, - IReadOnlyDictionary expectedSymbolChanges = null - ) + IReadOnlyDictionary expectedSymbolChanges = null) { testParameters ??= new TestParameters(); @@ -41,58 +43,96 @@ public async Task TestMoveToNamespaceAsync( testState.TestMoveToNamespaceOptionsService.SetOptions(moveToNamespaceOptions); if (expectedSuccess) { - var actions = await testState.MoveToNamespaceService.GetCodeActionsAsync( + if (!optionCancelled && !string.IsNullOrEmpty(targetNamespace)) + { + await TestRefactorNotifyAsync(testState, workspace, targetNamespace, optionCancelled, expectedSymbolChanges); + await TestInRegularAndScriptAsync(markup, expectedMarkup); + } + } + else + { + await TestMissingInRegularAndScriptAsync(markup, parameters: testParameters.Value); + } + } + + private async Task TestRefactorNotifyAsync( + TestState testState, + TestWorkspace workspace, + string targetNamespace = null, + bool optionCancelled = false, + IReadOnlyDictionary expectedSymbolChanges = null) + { + var actions = await testState.MoveToNamespaceService.GetCodeActionsAsync( testState.InvocationDocument, testState.TestInvocationDocument.SelectedSpans.Single(), CancellationToken.None); - var operationTasks = actions - .Cast() - .Select(action => action.GetOperationsAsync(action.GetOptions(CancellationToken.None), CancellationToken.None)); + var operationTasks = actions + .Cast() + .Select(action => action.GetOperationsAsync(action.GetOptions(CancellationToken.None), CancellationToken.None)); + + var operations = new List(); + + foreach (var task in operationTasks) + { + var taskOperations = await task; - foreach (var task in operationTasks) + if (optionCancelled || string.IsNullOrEmpty(targetNamespace)) { - var operations = await task; - - if (optionCancelled || string.IsNullOrEmpty(targetNamespace)) - { - Assert.Empty(operations); - } - else - { - Assert.NotEmpty(operations); - var renamedCodeActionsOperations = operations - .Where(operation => operation is TestSymbolRenamedCodeActionOperationFactoryWorkspaceService.Operation) - .Cast() - .ToImmutableArray(); - - Assert.NotEmpty(renamedCodeActionsOperations); - - Assert.NotNull(expectedSymbolChanges); - - var checkedCodeActions = new HashSet(renamedCodeActionsOperations.Length); - foreach (var kvp in expectedSymbolChanges) - { - var originalName = kvp.Key; - var newName = kvp.Value; - - var codeAction = renamedCodeActionsOperations.FirstOrDefault(a => a._symbol.ToDisplayString() == originalName); - Assert.Equal(newName, codeAction?._newName); - Assert.False(checkedCodeActions.Contains(codeAction)); - - checkedCodeActions.Add(codeAction); - } - } + Assert.Empty(taskOperations); } - - if (!optionCancelled && !string.IsNullOrEmpty(targetNamespace)) + else { - await TestInRegularAndScriptAsync(markup, expectedMarkup); + Assert.NotEmpty(taskOperations); } + + operations.AddRange(taskOperations); } - else + + var refactorNotify = workspace.GetService() as TestRefactorNotify; + + var expectedRenames = expectedSymbolChanges.Select(kvp => new RenameTracking() { - await TestMissingInRegularAndScriptAsync(markup, parameters: testParameters.Value); + Original = kvp.Key, + New = kvp.Value, + OnBeforeCalled = false, + OnAfterCalled = false + }).ToImmutableArray(); + + TestRefactorNotify.SymbolRenamedEventHandler beforeRename = (args) => + { + var expectedRenameTracking = expectedRenames.First(r => r.New == args.NewName); + Assert.False(expectedRenameTracking.OnBeforeCalled); + expectedRenameTracking.OnBeforeCalled = true; + }; + + TestRefactorNotify.SymbolRenamedEventHandler afterRename = (args) => + { + var expectedRenameTracking = expectedRenames.First(r => r.New == args.NewName); + expectedRenameTracking.OnAfterCalled = true; + }; + + if (refactorNotify is object) + { + refactorNotify.OnBeforeRename += beforeRename; + refactorNotify.OnAfterRename += afterRename; + } + + foreach (var operation in operations) + { + operation.Apply(workspace, CancellationToken.None); + } + + if (refactorNotify is object) + { + refactorNotify.OnBeforeRename -= beforeRename; + refactorNotify.OnAfterRename -= afterRename; + } + + foreach (var expectedRename in expectedRenames) + { + Assert.True(expectedRename.OnBeforeCalled, $"{expectedRename.Original} => {expectedRename.New} :: on before was not called"); + Assert.True(expectedRename.OnAfterCalled, $"{expectedRename.Original} => {expectedRename.New} :: on after was not called"); } } @@ -112,5 +152,13 @@ public async Task TestMoveToNamespaceAnalysisAsync(string markup, string expecte } public Task TestCancelledOption(string markup) => TestMoveToNamespaceAsync(markup, expectedMarkup: markup, optionCancelled: true); + + private class RenameTracking + { + public string Original { get; set; } + public string New { get; set; } + public bool OnBeforeCalled { get; set; } + public bool OnAfterCalled { get; set; } + } } } diff --git a/src/EditorFeatures/TestUtilities/Workspaces/SymbolRenameEventArgs.cs b/src/EditorFeatures/TestUtilities/Workspaces/SymbolRenameEventArgs.cs new file mode 100644 index 0000000000000..9340b1cc88c2a --- /dev/null +++ b/src/EditorFeatures/TestUtilities/Workspaces/SymbolRenameEventArgs.cs @@ -0,0 +1,20 @@ +using System.Collections.Generic; + +namespace Microsoft.CodeAnalysis.Test.Utilities.Workspaces +{ + internal class SymbolRenameEventArgs + { + public SymbolRenameEventArgs(Workspace workspace, IEnumerable documentIds, ISymbol symbol, string newName) + { + Workspace = workspace; + DocumentIds = documentIds; + Symbol = symbol; + NewName = newName; + } + + public Workspace Workspace { get; } + public IEnumerable DocumentIds { get; } + public ISymbol Symbol { get; } + public string NewName { get; } + } +} diff --git a/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs b/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs new file mode 100644 index 0000000000000..33977a5f4c47d --- /dev/null +++ b/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs @@ -0,0 +1,29 @@ +// 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.Generic; +using System.Composition; +using Microsoft.CodeAnalysis.Editor; + +namespace Microsoft.CodeAnalysis.Test.Utilities.Workspaces +{ + [Export(typeof(IRefactorNotifyService)), Shared] + [PartNotDiscoverable] + internal class TestRefactorNotify : IRefactorNotifyService + { + public delegate void SymbolRenamedEventHandler(SymbolRenameEventArgs args); + public event SymbolRenamedEventHandler OnAfterRename; + public event SymbolRenamedEventHandler OnBeforeRename; + + public bool TryOnAfterGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, ISymbol symbol, string newName, bool throwOnFailure) + { + OnAfterRename?.Invoke(new SymbolRenameEventArgs(workspace, changedDocumentIDs, symbol, newName)); + return true; + } + + public bool TryOnBeforeGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, ISymbol symbol, string newName, bool throwOnFailure) + { + OnBeforeRename?.Invoke(new SymbolRenameEventArgs(workspace, changedDocumentIDs, symbol, newName)); + return true; + } + } +} diff --git a/src/EditorFeatures/TestUtilities/Workspaces/TestSymbolRenamedCodeActionOperationFactoryWorkspaceService.cs b/src/EditorFeatures/TestUtilities/Workspaces/TestSymbolRenamedCodeActionOperationFactoryWorkspaceService.cs deleted file mode 100644 index b6bdcc446b55c..0000000000000 --- a/src/EditorFeatures/TestUtilities/Workspaces/TestSymbolRenamedCodeActionOperationFactoryWorkspaceService.cs +++ /dev/null @@ -1,39 +0,0 @@ -// 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.Composition; -using Microsoft.CodeAnalysis.CodeActions; -using Microsoft.CodeAnalysis.CodeActions.WorkspaceServices; -using Microsoft.CodeAnalysis.Host.Mef; - -namespace Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces -{ - [ExportWorkspaceService(typeof(ISymbolRenamedCodeActionOperationFactoryWorkspaceService), WorkspaceKind.Test), Shared] - public class TestSymbolRenamedCodeActionOperationFactoryWorkspaceService : ISymbolRenamedCodeActionOperationFactoryWorkspaceService - { - [ImportingConstructor] - public TestSymbolRenamedCodeActionOperationFactoryWorkspaceService() - { - } - - public CodeActionOperation CreateSymbolRenamedOperation(ISymbol symbol, string newName, Solution startingSolution, Solution updatedSolution) - { - return new Operation(symbol, newName, startingSolution, updatedSolution); - } - - public class Operation : CodeActionOperation - { - public ISymbol _symbol; - public string _newName; - public Solution _startingSolution; - public Solution _updatedSolution; - - public Operation(ISymbol symbol, string newName, Solution startingSolution, Solution updatedSolution) - { - _symbol = symbol; - _newName = newName; - _startingSolution = startingSolution; - _updatedSolution = updatedSolution; - } - } - } -} diff --git a/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace.cs b/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace.cs index 56aa7b16bfe4b..b59f9fe53f7bf 100644 --- a/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace.cs +++ b/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace.cs @@ -13,6 +13,7 @@ using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.Shared.Utilities; using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.Text.Shared.Extensions; using Microsoft.VisualStudio.Composition; @@ -46,6 +47,7 @@ public partial class TestWorkspace : Workspace private readonly IMetadataAsSourceFileService _metadataAsSourceFileService; private readonly Dictionary _createdTextBuffers = new Dictionary(); + private readonly IEnumerable _refactorNotifyServices; public TestWorkspace() : this(TestExportProvider.ExportProviderWithCSharpAndVisualBasic, WorkspaceKind.Test) @@ -70,6 +72,7 @@ public TestWorkspace(ExportProvider exportProvider, string? workspaceKind = null _backgroundParser.Start(); _metadataAsSourceFileService = exportProvider.GetExportedValues().FirstOrDefault(); + _refactorNotifyServices = exportProvider.GetExportedValues(); RegisterDocumentOptionProviders(exportProvider.GetExports()); } @@ -704,5 +707,50 @@ internal ITextBuffer GetOrCreateBufferForPath(string? filePath, IContentType con return textBuffer; }); } + + internal override bool TryApplyChanges( + Solution newSolution, + IProgressTracker progressTracker) + { + var oldSolution = CurrentSolution; + if (base.TryApplyChanges(newSolution, progressTracker)) + { + var projectChanges = newSolution.GetChanges(oldSolution).GetProjectChanges().ToList(); + var changedDocs = projectChanges.SelectMany(pd => pd.GetChangedDocuments(true).Concat(pd.GetChangedAdditionalDocuments())).ToList(); + + NotifyRefactorChanges(changedDocs, newSolution, oldSolution); + return true; + } + + return false; + } + + private void NotifyRefactorChanges( + IEnumerable changedDocumentIds, + Solution newSolution, + Solution oldSolution, + CancellationToken cancellationToken = default) + { + // Without any services to notify there's no need to dig through the documents + if (!_refactorNotifyServices.Any()) + { + return; + } + + var changedSymbols = Rename.RenameSymbolAnnotation + .GatherChangedSymbolsInDocumentsAsync(changedDocumentIds, newSolution, oldSolution, cancellationToken) + .WaitAndGetResult_CanCallOnBackground(cancellationToken); + + foreach (var (oldSymbol, newSymbol) in changedSymbols) + { + foreach (var refactorNotifyService in _refactorNotifyServices) + { + if (refactorNotifyService.TryOnBeforeGlobalSymbolRenamed(this, changedDocumentIds, oldSymbol, newSymbol.ToDisplayString(), throwOnFailure: false)) + { + refactorNotifyService.TryOnAfterGlobalSymbolRenamed(this, changedDocumentIds, oldSymbol, newSymbol.ToDisplayString(), throwOnFailure: false); + } + } + } + } } } diff --git a/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpChangeNamespaceService.cs b/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpChangeNamespaceService.cs index 5a2bc9cf40f71..33eef43cd27e5 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpChangeNamespaceService.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpChangeNamespaceService.cs @@ -16,6 +16,7 @@ using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Rename; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; @@ -232,19 +233,22 @@ private static bool TryGetGlobalQualifiedName( /// - if target namespace is "", then we try to move all members in declared /// namespace to global namespace (i.e. remove the namespace declaration). /// - protected override CompilationUnitSyntax ChangeNamespaceDeclaration( - CompilationUnitSyntax root, + protected override async Task ChangeNamespaceDeclarationAsync( + Document document, ImmutableArray declaredNamespaceParts, - ImmutableArray targetNamespaceParts) + ImmutableArray targetNamespaceParts, + CancellationToken cancellationToken) { Debug.Assert(!declaredNamespaceParts.IsDefault && !targetNamespaceParts.IsDefault); + var root = (CompilationUnitSyntax)await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); var container = root.GetAnnotatedNodes(ContainerAnnotation).Single(); if (container is CompilationUnitSyntax compilationUnit) { // Move everything from global namespace to a namespace declaration Debug.Assert(IsGlobalNamespace(declaredNamespaceParts)); - return MoveMembersFromGlobalToNamespace(compilationUnit, targetNamespaceParts); + return MoveMembersFromGlobalToNamespace(compilationUnit, targetNamespaceParts, semanticModel); } if (container is NamespaceDeclarationSyntax namespaceDecl) @@ -260,8 +264,9 @@ protected override CompilationUnitSyntax ChangeNamespaceDeclaration( namespaceDecl, namespaceDecl.WithName( CreateNamespaceAsQualifiedName(targetNamespaceParts, aliasQualifier: null, targetNamespaceParts.Length - 1) - .WithTriviaFrom(namespaceDecl.Name).WithAdditionalAnnotations(WarningAnnotation)) - .WithoutAnnotations(ContainerAnnotation)); // Make sure to remove the annotation we added + .WithTriviaFrom(namespaceDecl.Name) + .WithAdditionalAnnotations(WarningAnnotation)) + .WithoutAnnotations(ContainerAnnotation)); // Make sure to remove the annotation we added } throw ExceptionUtilities.Unreachable; @@ -313,9 +318,15 @@ private static CompilationUnitSyntax MoveMembersFromNamespaceToGlobal(Compilatio eofToken); } - private static CompilationUnitSyntax MoveMembersFromGlobalToNamespace(CompilationUnitSyntax compilationUnit, ImmutableArray targetNamespaceParts) + private static CompilationUnitSyntax MoveMembersFromGlobalToNamespace( + CompilationUnitSyntax compilationUnit, + ImmutableArray targetNamespaceParts, + SemanticModel semanticModel) { Debug.Assert(!compilationUnit.Members.Any(m => m is NamespaceDeclarationSyntax)); + compilationUnit = compilationUnit.ReplaceNodes( + compilationUnit.Members, + (original, current) => semanticModel.GetDeclaredSymbol(current) is null ? current : current.WithRenameSymbolAnnotation(semanticModel)); var targetNamespaceDecl = SyntaxFactory.NamespaceDeclaration( name: CreateNamespaceAsQualifiedName(targetNamespaceParts, aliasQualifier: null, targetNamespaceParts.Length - 1) @@ -479,5 +490,8 @@ private static (ImmutableArray openingTrivia, ImmutableArray identifier.EscapeIdentifier(); } } diff --git a/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpSyncNamespaceCodeRefactoringProvider.cs b/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpSyncNamespaceCodeRefactoringProvider.cs index 638449848139e..e85e72af8005d 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpSyncNamespaceCodeRefactoringProvider.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpSyncNamespaceCodeRefactoringProvider.cs @@ -58,8 +58,5 @@ protected override async Task TryGetApplicableInvocationNodeAsync(Do return null; } - - protected override string EscapeIdentifier(string identifier) - => identifier.EscapeIdentifier(); } } diff --git a/src/Features/Core/Portable/CodeFixes/NamingStyle/NamingStyleCodeFixProvider.cs b/src/Features/Core/Portable/CodeFixes/NamingStyle/NamingStyleCodeFixProvider.cs index 16ca2d8f8175c..faad9a68daf18 100644 --- a/src/Features/Core/Portable/CodeFixes/NamingStyle/NamingStyleCodeFixProvider.cs +++ b/src/Features/Core/Portable/CodeFixes/NamingStyle/NamingStyleCodeFixProvider.cs @@ -78,9 +78,6 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) var solution = context.Document.Project.Solution; context.RegisterCodeFix( new FixNameCodeAction( - solution, - symbol, - fixedName, string.Format(FeaturesResources.Fix_Name_Violation_colon_0, fixedName), c => FixAsync(document, symbol, fixedName, c), equivalenceKey: nameof(NamingStyleCodeFixProvider)), @@ -97,51 +94,15 @@ await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false), cancellationToken).ConfigureAwait(false); } - private class FixNameCodeAction : CodeAction + private class FixNameCodeAction : CodeAction.SolutionChangeAction { - private readonly Solution _startingSolution; - private readonly ISymbol _symbol; - private readonly string _newName; - private readonly string _title; - private readonly Func> _createChangedSolutionAsync; - private readonly string _equivalenceKey; - public FixNameCodeAction( - Solution startingSolution, - ISymbol symbol, - string newName, string title, Func> createChangedSolutionAsync, string equivalenceKey) + : base(title, createChangedSolutionAsync, equivalenceKey) { - _startingSolution = startingSolution; - _symbol = symbol; - _newName = newName; - _title = title; - _createChangedSolutionAsync = createChangedSolutionAsync; - _equivalenceKey = equivalenceKey; } - - protected override async Task> ComputePreviewOperationsAsync(CancellationToken cancellationToken) - { - return SpecializedCollections.SingletonEnumerable( - new ApplyChangesOperation(await _createChangedSolutionAsync(cancellationToken).ConfigureAwait(false))); - } - - protected override async Task> ComputeOperationsAsync(CancellationToken cancellationToken) - { - var factory = _startingSolution.Workspace.Services.GetService(); - var newSolution = await _createChangedSolutionAsync(cancellationToken).ConfigureAwait(false); - return new CodeActionOperation[] - { - new ApplyChangesOperation(newSolution), - factory.CreateSymbolRenamedOperation(_symbol, _newName, _startingSolution, newSolution) - }.AsEnumerable(); - } - - public override string Title => _title; - - public override string EquivalenceKey => _equivalenceKey; } } } diff --git a/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.cs b/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.cs index 8d27f753acba9..c55c9cc236929 100644 --- a/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.cs +++ b/src/Features/Core/Portable/CodeRefactorings/MoveType/AbstractMoveTypeService.cs @@ -58,15 +58,20 @@ public override async Task GetModifiedSolutionAsync(Document document, return document.Project.Solution; } - var suggestedFileNames = GetSuggestedFileNames( - state.TypeNode, - IsNestedType(state.TypeNode), - state.TypeName, - state.SemanticDocument.Document.Name, - state.SemanticDocument.SemanticModel, - cancellationToken); - - var editor = Editor.GetEditor(operationKind, (TService)this, state, suggestedFileNames.FirstOrDefault(), cancellationToken); + var suggestedFileName = operationKind switch + { + MoveTypeOperationKind.RenameType => Path.GetFileNameWithoutExtension(document.Name), + MoveTypeOperationKind.MoveTypeNamespaceScope => document.Name, + _ => GetSuggestedFileNames( + state.TypeNode, + IsNestedType(state.TypeNode), + state.TypeName, + state.SemanticDocument.Document.Name, + state.SemanticDocument.SemanticModel, + cancellationToken).FirstOrDefault() + }; + + var editor = Editor.GetEditor(operationKind, (TService)this, state, suggestedFileName, cancellationToken); var modifiedSolution = await editor.GetModifiedSolutionAsync().ConfigureAwait(false); return modifiedSolution ?? document.Project.Solution; } diff --git a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractChangeNamespaceService.cs b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractChangeNamespaceService.cs index 166b67ba508df..f64886c7ad67f 100644 --- a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractChangeNamespaceService.cs +++ b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractChangeNamespaceService.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +#nullable enable + using System; using System.Collections.Generic; using System.Collections.Immutable; @@ -7,6 +9,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.AddImports; using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.FindSymbols; @@ -15,6 +18,7 @@ using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.RemoveUnnecessaryImports; +using Microsoft.CodeAnalysis.Rename; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Simplification; using Microsoft.CodeAnalysis.Text; @@ -30,6 +34,7 @@ internal abstract class AbstractChangeNamespaceService : IChangeNamespaceService public abstract Task CanChangeNamespaceAsync(Document document, SyntaxNode container, CancellationToken cancellationToken); public abstract Task ChangeNamespaceAsync(Document document, SyntaxNode container, string targetNamespace, CancellationToken cancellationToken); + public abstract string? GetTargetNamespaceFromDocument(Document document); /// /// Try to get a new node to replace given node, which is a reference to a top-level type declared inside the @@ -43,7 +48,30 @@ internal abstract class AbstractChangeNamespaceService : IChangeNamespaceService /// namespace in the replacement node. /// The node to be replaced. This might be an ancestor of original /// The replacement node. - public abstract bool TryGetReplacementReferenceSyntax(SyntaxNode reference, ImmutableArray newNamespaceParts, ISyntaxFactsService syntaxFacts, out SyntaxNode old, out SyntaxNode @new); + public abstract bool TryGetReplacementReferenceSyntax(SyntaxNode reference, ImmutableArray newNamespaceParts, ISyntaxFactsService syntaxFacts, out SyntaxNode? old, out SyntaxNode? @new); + + public static string? GetDefaultNamespace(Document document, ISyntaxFactsService syntaxFacts) + { + var solution = document.Project.Solution; + var linkedIds = document.GetLinkedDocumentIds(); + var documents = linkedIds.SelectAsArray(id => solution.GetDocument(id)).Add(document); + + // For all projects containing all the linked documents, bail if + // 1. Any of them doesn't have default namespace, or + // 2. Multiple default namespace are found. (this might be possible by tweaking project file). + // The refactoring depends on a single default namespace to operate. + var defaultNamespaceFromProjects = new HashSet( + documents.Select(d => d?.Project.DefaultNamespace), + syntaxFacts.StringComparer); + + if (defaultNamespaceFromProjects.Count != 1 + || defaultNamespaceFromProjects.First() == null) + { + return default; + } + + return defaultNamespaceFromProjects.Single(); + } } internal abstract class AbstractChangeNamespaceService @@ -63,14 +91,15 @@ internal abstract class AbstractChangeNamespaceService declaredNamespaceParts, ImmutableArray targetNamespaceParts); + protected abstract Task ChangeNamespaceDeclarationAsync( + Document document, ImmutableArray declaredNamespaceParts, ImmutableArray targetNamespaceParts, CancellationToken cancellationToken); protected abstract SyntaxList GetMemberDeclarationsInContainer(SyntaxNode compilationUnitOrNamespaceDecl); - protected abstract Task TryGetApplicableContainerFromSpanAsync(Document document, TextSpan span, CancellationToken cancellationToken); + protected abstract Task TryGetApplicableContainerFromSpanAsync(Document document, TextSpan span, CancellationToken cancellationToken); protected abstract string GetDeclaredNamespace(SyntaxNode container); + protected abstract string EscapeIdentifier(string identifier); /// /// Decide if we can change the namespace for provided based on the criteria listed for @@ -107,7 +136,7 @@ public override async Task ChangeNamespaceAsync( CancellationToken cancellationToken) { // Make sure given namespace name is valid, "" means global namespace. - var syntaxFacts = document.GetLanguageService(); + var syntaxFacts = document.GetRequiredLanguageService(); if (targetNamespace == null || (targetNamespace.Length > 0 && !targetNamespace.Split(s_dotSeparator).All(syntaxFacts.IsValidIdentifier))) { @@ -202,6 +231,50 @@ await ChangeNamespaceInSingleDocumentAsync(solutionAfterNamespaceChange, documen } } + public override string? GetTargetNamespaceFromDocument(Document document) + { + var syntaxFacts = document.GetRequiredLanguageService(); + + // We can't determine what the expected namespace would be without knowing the default namespace. + var defaultNamespace = GetDefaultNamespace(document, syntaxFacts); + if (defaultNamespace == null) + { + return null; + } + + var namespaceFromFolders = TryBuildNamespaceFromFolders(document.Folders, syntaxFacts); + return namespaceFromFolders == null + ? null + : ConcatNamespace(defaultNamespace, namespaceFromFolders); + } + + private static string ConcatNamespace(string rootNamespace, string namespaceSuffix) + { + if (namespaceSuffix.Length == 0) + { + return rootNamespace; + } + else if (rootNamespace.Length == 0) + { + return namespaceSuffix; + } + else + { + return rootNamespace + "." + namespaceSuffix; + } + } + + /// + /// Create a qualified identifier as the suffix of namespace based on a list of folder names. + /// + private string? TryBuildNamespaceFromFolders( + IEnumerable folders, + ISyntaxFactsService syntaxFacts) + { + var parts = folders.SelectMany(folder => folder.Split(s_dotSeparator).SelectAsArray(this.EscapeIdentifier)); + return parts.All(syntaxFacts.IsValidIdentifier) ? string.Join(".", parts) : null; + } + protected async Task> TryGetApplicableContainersFromAllDocumentsAsync( Solution solution, ImmutableArray ids, @@ -219,6 +292,8 @@ await ChangeNamespaceInSingleDocumentAsync(solutionAfterNamespaceChange, documen foreach (var document in documents) { + if (document is null) continue; + var container = await TryGetApplicableContainerFromSpanAsync(document, span, cancellationToken).ConfigureAwait(false); if (container is TNamespaceDeclarationSyntax) @@ -261,8 +336,8 @@ protected async Task ContainsPartialTypeWithMultipleDeclarationsAsync( Document document, SyntaxNode container, CancellationToken cancellationToken) { var memberDecls = GetMemberDeclarationsInContainer(container); - var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - var semanticFacts = document.GetLanguageService(); + var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var semanticFacts = document.GetRequiredLanguageService(); foreach (var memberDecl in memberDecls) { @@ -290,7 +365,7 @@ protected static bool IsSupportedLinkedDocument(Document document, out Immutable // 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))) + !PathUtilities.PathsEqual(solution.GetRequiredDocument(id).Project.FilePath, document.Project.FilePath))) { allDocumentIds = default; return false; @@ -305,7 +380,7 @@ private async Task> GetDeclaredSymbolsInContainerAsync( SyntaxNode container, CancellationToken cancellationToken) { - var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); var declarations = GetMemberDeclarationsInContainer(container); var builder = ArrayBuilder.GetInstance(); @@ -372,8 +447,8 @@ private static SyntaxNode CreateImport(SyntaxGenerator syntaxGenerator, string n string newNamespace, CancellationToken cancellationToken) { - var document = solution.GetDocument(id); - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var document = solution.GetRequiredDocument(id); + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var container = root.GetAnnotatedNodes(ContainerAnnotation).Single(); // Get types declared in the changing namespace, because we need to fix all references to them, @@ -413,14 +488,14 @@ private static SyntaxNode CreateImport(SyntaxGenerator syntaxGenerator, string n var fixedDocuments = await Task.WhenAll( refLocationGroups.Select(refInOneDocument => FixReferencingDocumentAsync( - solutionWithChangedNamespace.GetDocument(refInOneDocument.Key), + solutionWithChangedNamespace.GetRequiredDocument(refInOneDocument.Key), refInOneDocument, newNamespace, cancellationToken))).ConfigureAwait(false); var solutionWithFixedReferences = await MergeDocumentChangesAsync(solutionWithChangedNamespace, fixedDocuments, cancellationToken).ConfigureAwait(false); - return (solutionWithFixedReferences, refLocationGroups.SelectAsArray(g => g.Key)); + return (solutionWithFixedReferences, refLocationGroups.SelectAsArray, DocumentId>(g => g.Key)); } private static async Task MergeDocumentChangesAsync(Solution originalSolution, Document[] changedDocuments, CancellationToken cancellationToken) @@ -429,7 +504,7 @@ private static async Task MergeDocumentChangesAsync(Solution originalS { originalSolution = originalSolution.WithDocumentSyntaxRoot( document.Id, - await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false)); + await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false)); } return originalSolution; @@ -512,8 +587,6 @@ private async Task FixDeclarationDocumentAsync( string newNamespace, CancellationToken cancellationToken) { - Debug.Assert(newNamespace != null); - // 1. Fix references to the affected types in this document if necessary. // 2. Add usings for containing namespaces, in case we have references // relying on old namespace declaration for resolution. @@ -532,7 +605,7 @@ private async Task FixDeclarationDocumentAsync( // 3. Change namespace declaration to target namespace. // 4. Simplify away unnecessary qualifications. - var addImportService = document.GetLanguageService(); + var addImportService = document.GetRequiredLanguageService(); ImmutableArray containersToAddImports; var oldNamespaceParts = GetNamespaceParts(oldNamespace); @@ -547,7 +620,7 @@ private async Task FixDeclarationDocumentAsync( { // If there's no reference to types declared in this document, // we will use root node as import container. - containersToAddImports = ImmutableArray.Create(await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false)); + containersToAddImports = ImmutableArray.Create(await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false)); } Debug.Assert(containersToAddImports.Length > 0); @@ -568,10 +641,9 @@ private async Task FixDeclarationDocumentAsync( placeSystemNamespaceFirst, cancellationToken).ConfigureAwait(false); - var root = await documentWithAddedImports.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - - root = ChangeNamespaceDeclaration((TCompilationUnitSyntax)root, oldNamespaceParts, newNamespaceParts) - .WithAdditionalAnnotations(Formatter.Annotation); + var documentWithRenameAnnotations = await GetRenameSymbolAnnotatedRootAsync(documentWithAddedImports, cancellationToken).ConfigureAwait(false); + var compilationUnit = await ChangeNamespaceDeclarationAsync(documentWithRenameAnnotations, oldNamespaceParts, newNamespaceParts, cancellationToken).ConfigureAwait(false); + SyntaxNode root = compilationUnit.WithAdditionalAnnotations(Formatter.Annotation); // Need to invoke formatter explicitly since we are doing the diff merge ourselves. root = Formatter.Format(root, Formatter.Annotation, documentWithAddedImports.Project.Solution.Workspace, optionSet, cancellationToken); @@ -581,6 +653,18 @@ private async Task FixDeclarationDocumentAsync( return await Simplifier.ReduceAsync(formattedDocument, optionSet, cancellationToken).ConfigureAwait(false); } + private async Task GetRenameSymbolAnnotatedRootAsync(Document document, CancellationToken cancellationToken) + { + var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var syntaxFacts = document.GetRequiredLanguageService(); + + return document.WithSyntaxRoot( + root.ReplaceNodes( + root.GetAnnotatedNodes(ContainerAnnotation).SelectMany(c => c is TCompilationUnitSyntax ? syntaxFacts.GetMembersOfCompilationUnit(c) : syntaxFacts.GetMembersOfNamespaceDeclaration(c)), + (original, current) => current.WithRenameSymbolAnnotation(semanticModel))); + } + private async Task FixReferencingDocumentAsync( Document document, IEnumerable refLocations, @@ -598,8 +682,8 @@ private async Task FixReferencingDocumentAsync( // 2. Add using of new namespace (for each reference's container). // 3. Try to simplify qualified names introduced from step(1). - var addImportService = document.GetLanguageService(); - var changeNamespaceService = document.GetLanguageService(); + var addImportService = document.GetRequiredLanguageService(); + var changeNamespaceService = document.GetRequiredLanguageService(); var newNamespaceParts = GetNamespaceParts(newNamespace); @@ -646,7 +730,7 @@ await FixReferencesAsync(document, changeNamespaceService, addImportService, ref var containers = PooledHashSet.GetInstance(); var generator = SyntaxGenerator.GetGenerator(document); - var syntaxFacts = document.GetLanguageService(); + var syntaxFacts = document.GetRequiredLanguageService(); // We need a dummy import to figure out the container for given reference. var dummyImport = CreateImport(generator, "Dummy", withFormatterAnnotation: false); @@ -698,8 +782,8 @@ await FixReferencesAsync(document, changeNamespaceService, addImportService, ref } var fixedDocument = editor.GetChangedDocument(); - root = await fixedDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var result = (fixedDocument, containers.SelectAsArray(c => root.GetCurrentNode(c))); + root = await fixedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var result = (fixedDocument, containers.SelectAsArray(c => root.GetCurrentNode(c))); containers.Free(); return result; @@ -721,7 +805,7 @@ private async Task RemoveUnnecessaryImportsAsync( continue; } - var document = solution.GetDocument(id); + var document = solution.GetRequiredDocument(id); LinkedDocumentsToSkip.AddRange(document.GetLinkedDocumentIds()); documentsToProcessBuilder.Add(document); @@ -748,8 +832,8 @@ Task RemoveUnnecessaryImportsWorker( IEnumerable importsToRemove, CancellationToken token) { - var removeImportService = doc.GetLanguageService(); - var syntaxFacts = doc.GetLanguageService(); + var removeImportService = doc.GetRequiredLanguageService(); + var syntaxFacts = doc.GetRequiredLanguageService(); return removeImportService.RemoveUnnecessaryImportsAsync( doc, @@ -787,8 +871,8 @@ private async Task AddImportsInContainersAsync( ? container.DescendantNodes().First() : container; - var compilation = await document.Project.GetCompilationAsync(cancellationToken).ConfigureAwait(false); - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var compilation = await document.Project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false); + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); root = addImportService.AddImports(compilation, root, contextLocation, imports, placeSystemNamespaceFirst, cancellationToken); document = document.WithSyntaxRoot(root); } diff --git a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractSyncNamespaceCodeRefactoringProvider.State.cs b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractSyncNamespaceCodeRefactoringProvider.State.cs index b7c4c268674d1..48200ff0974c6 100644 --- a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractSyncNamespaceCodeRefactoringProvider.State.cs +++ b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractSyncNamespaceCodeRefactoringProvider.State.cs @@ -99,13 +99,6 @@ public static async Task CreateAsync( var syntaxFacts = document.GetLanguageService(); var solution = document.Project.Solution; - // We can't determine what the expected namespace would be without knowing the default namespace. - var defaultNamespace = GetDefaultNamespace(document, syntaxFacts); - if (defaultNamespace == null) - { - return null; - } - string declaredNamespace; if (applicableNode is TCompilationUnitSyntax) { @@ -121,12 +114,15 @@ public static async Task CreateAsync( throw ExceptionUtilities.Unreachable; } - // Namespace can't be changed if we can't construct a valid qualified identifier from folder names. - // In this case, we might still be able to provide refactoring to move file to new location. - var namespaceFromFolders = TryBuildNamespaceFromFolders(provider, document.Folders, syntaxFacts); - var targetNamespace = namespaceFromFolders == null - ? null - : ConcatNamespace(defaultNamespace, namespaceFromFolders); + var defaultNamespace = AbstractChangeNamespaceService.GetDefaultNamespace(document, syntaxFacts); + if (defaultNamespace == null) + { + return null; + } + + + var changeNamespaceService = document.GetRequiredLanguageService(); + var targetNamespace = changeNamespaceService.GetTargetNamespaceFromDocument(document); // No action required if namespace already matches folders. if (syntaxFacts.StringComparer.Equals(targetNamespace, declaredNamespace)) @@ -160,58 +156,6 @@ private static bool IsDocumentPathRootedInProjectFolder(Document document) return PathUtilities.PathsEqual(absoluteDircetoryPath, logicalDirectoryPath); } - private static string GetDefaultNamespace(Document document, ISyntaxFactsService syntaxFacts) - { - var solution = document.Project.Solution; - var linkedIds = document.GetLinkedDocumentIds(); - var documents = linkedIds.SelectAsArray(id => solution.GetDocument(id)).Add(document); - - // For all projects containing all the linked documents, bail if - // 1. Any of them doesn't have default namespace, or - // 2. Multiple default namespace are found. (this might be possible by tweaking project file). - // The refactoring depends on a single default namespace to operate. - var defaultNamespaceFromProjects = new HashSet( - documents.Select(d => d.Project.DefaultNamespace), - syntaxFacts.StringComparer); - - if (defaultNamespaceFromProjects.Count != 1 - || defaultNamespaceFromProjects.First() == null) - { - return default; - } - - return defaultNamespaceFromProjects.Single(); - } - - /// - /// Create a qualified identifier as the suffix of namespace based on a list of folder names. - /// - private static string TryBuildNamespaceFromFolders( - AbstractSyncNamespaceCodeRefactoringProvider service, - IEnumerable folders, - ISyntaxFactsService syntaxFacts) - { - var parts = folders.SelectMany(folder => folder.Split(new[] { '.' }).SelectAsArray(service.EscapeIdentifier)); - return parts.All(syntaxFacts.IsValidIdentifier) ? string.Join(".", parts) : null; - } - - private static string ConcatNamespace(string rootNamespace, string namespaceSuffix) - { - Debug.Assert(rootNamespace != null && namespaceSuffix != null); - if (namespaceSuffix.Length == 0) - { - return rootNamespace; - } - else if (rootNamespace.Length == 0) - { - return namespaceSuffix; - } - else - { - return rootNamespace + "." + namespaceSuffix; - } - } - /// /// Try get the relative namespace for based on , /// if is the containing namespace of . Otherwise, diff --git a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractSyncNamespaceCodeRefactoringProvider.cs b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractSyncNamespaceCodeRefactoringProvider.cs index e4e11acdbe64b..0adfb905a9db2 100644 --- a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractSyncNamespaceCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractSyncNamespaceCodeRefactoringProvider.cs @@ -89,8 +89,6 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte /// protected abstract Task TryGetApplicableInvocationNodeAsync(Document document, TextSpan span, CancellationToken cancellationToken); - protected abstract string EscapeIdentifier(string identifier); - private class ChangeNamespaceCodeAction : SolutionChangeAction { public ChangeNamespaceCodeAction(string title, Func> createChangedSolution) diff --git a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/IChangeNamespaceService.cs b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/IChangeNamespaceService.cs index 0a2ace8b39f5b..16ba3a20a9be3 100644 --- a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/IChangeNamespaceService.cs +++ b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/IChangeNamespaceService.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +#nullable enable + using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host; @@ -51,5 +53,10 @@ internal interface IChangeNamespaceService : ILanguageService /// a no-op and original solution will be returned. /// Task ChangeNamespaceAsync(Document document, SyntaxNode container, string targetNamespace, CancellationToken cancellationToken); + + /// + /// Gets the target namespace for a given document by using the path and any root namespace + /// + string? GetTargetNamespaceFromDocument(Document document); } } diff --git a/src/Features/Core/Portable/CodeRefactorings/WorkspaceServices/ISymbolRenamedCodeActionOperationFactoryWorkspaceService.cs b/src/Features/Core/Portable/CodeRefactorings/WorkspaceServices/ISymbolRenamedCodeActionOperationFactoryWorkspaceService.cs deleted file mode 100644 index 67e52d65bb1d8..0000000000000 --- a/src/Features/Core/Portable/CodeRefactorings/WorkspaceServices/ISymbolRenamedCodeActionOperationFactoryWorkspaceService.cs +++ /dev/null @@ -1,11 +0,0 @@ -// 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 Microsoft.CodeAnalysis.Host; - -namespace Microsoft.CodeAnalysis.CodeActions.WorkspaceServices -{ - internal interface ISymbolRenamedCodeActionOperationFactoryWorkspaceService : IWorkspaceService - { - CodeActionOperation CreateSymbolRenamedOperation(ISymbol symbol, string newName, Solution startingSolution, Solution updatedSolution); - } -} diff --git a/src/Features/Core/Portable/MoveToNamespace/AbstractMoveToNamespaceCodeAction.cs b/src/Features/Core/Portable/MoveToNamespace/AbstractMoveToNamespaceCodeAction.cs index 1f84b83c19f79..ea590a700007f 100644 --- a/src/Features/Core/Portable/MoveToNamespace/AbstractMoveToNamespaceCodeAction.cs +++ b/src/Features/Core/Portable/MoveToNamespace/AbstractMoveToNamespaceCodeAction.cs @@ -1,13 +1,11 @@ // 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.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; -using Microsoft.CodeAnalysis.CodeActions.WorkspaceServices; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.MoveToNamespace @@ -58,25 +56,6 @@ private static ImmutableArray CreateRenameOperations(MoveTo var operations = PooledObjects.ArrayBuilder.GetInstance(); operations.Add(new ApplyChangesOperation(moveToNamespaceResult.UpdatedSolution)); - - var symbolRenameCodeActionOperationFactory = moveToNamespaceResult.UpdatedSolution.Workspace.Services.GetService(); - - // It's possible we're not in a host context providing this service, in which case - // just provide a code action that won't notify of the symbol rename. - // Without the symbol rename operation, code generators (like WPF) may not - // know to regenerate code correctly. - if (symbolRenameCodeActionOperationFactory != null) - { - foreach (var (newName, symbol) in moveToNamespaceResult.NewNameOriginalSymbolMapping) - { - operations.Add(symbolRenameCodeActionOperationFactory.CreateSymbolRenamedOperation( - symbol, - newName, - moveToNamespaceResult.OriginalSolution, - moveToNamespaceResult.UpdatedSolution)); - } - } - return operations.ToImmutableAndFree(); } diff --git a/src/Features/VisualBasic/Portable/CodeRefactorings/SyncNamespace/VisualBasicChangeNamespaceService.vb b/src/Features/VisualBasic/Portable/CodeRefactorings/SyncNamespace/VisualBasicChangeNamespaceService.vb index 2ef9b5decec77..6acc80cf6b8c7 100644 --- a/src/Features/VisualBasic/Portable/CodeRefactorings/SyncNamespace/VisualBasicChangeNamespaceService.vb +++ b/src/Features/VisualBasic/Portable/CodeRefactorings/SyncNamespace/VisualBasicChangeNamespaceService.vb @@ -59,7 +59,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ChangeNamespace End Function ' This is only reachable when called from a VB service, which is not implemented yet. - Protected Overrides Function ChangeNamespaceDeclaration(root As CompilationUnitSyntax, declaredNamespaceParts As ImmutableArray(Of String), targetNamespaceParts As ImmutableArray(Of String)) As CompilationUnitSyntax + Protected Overrides Function ChangeNamespaceDeclarationAsync(document As Document, declaredNamespaceParts As ImmutableArray(Of String), targetNamespaceParts As ImmutableArray(Of String), cancellationToken As CancellationToken) As Task(Of CompilationUnitSyntax) Throw ExceptionUtilities.Unreachable End Function @@ -99,5 +99,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ChangeNamespace Return SyntaxFactory.SimpleMemberAccessExpression(CreateNamespaceAsMemberAccess(namespaceParts, index - 1), namePiece) End If End Function + + Protected Overrides Function EscapeIdentifier(identifier As String) As String + Return identifier.EscapeIdentifier() + End Function End Class End Namespace diff --git a/src/VisualStudio/CSharp/Impl/CodeModel/CSharpCodeModelService.cs b/src/VisualStudio/CSharp/Impl/CodeModel/CSharpCodeModelService.cs index 076af341c0a37..44c4995d078ae 100644 --- a/src/VisualStudio/CSharp/Impl/CodeModel/CSharpCodeModelService.cs +++ b/src/VisualStudio/CSharp/Impl/CodeModel/CSharpCodeModelService.cs @@ -35,11 +35,9 @@ internal partial class CSharpCodeModelService : AbstractCodeModelService { internal CSharpCodeModelService( HostLanguageServices languageServiceProvider, - IEditorOptionsFactoryService editorOptionsFactoryService, - IEnumerable refactorNotifyServices) + IEditorOptionsFactoryService editorOptionsFactoryService) : base(languageServiceProvider, editorOptionsFactoryService, - refactorNotifyServices, BlankLineInGeneratedMethodFormattingRule.Instance, EndRegionFormattingRule.Instance) { diff --git a/src/VisualStudio/CSharp/Impl/CodeModel/CSharpCodeModelServiceFactory.cs b/src/VisualStudio/CSharp/Impl/CodeModel/CSharpCodeModelServiceFactory.cs index 85b0ef128b556..6cf6bed8d4ae0 100644 --- a/src/VisualStudio/CSharp/Impl/CodeModel/CSharpCodeModelServiceFactory.cs +++ b/src/VisualStudio/CSharp/Impl/CodeModel/CSharpCodeModelServiceFactory.cs @@ -15,7 +15,6 @@ namespace Microsoft.VisualStudio.LanguageServices.CSharp.CodeModel internal partial class CSharpCodeModelServiceFactory : ILanguageServiceFactory { private readonly IEditorOptionsFactoryService _editorOptionsFactoryService; - private readonly IEnumerable _refactorNotifyServices; [ImportingConstructor] public CSharpCodeModelServiceFactory( @@ -23,12 +22,11 @@ public CSharpCodeModelServiceFactory( [ImportMany] IEnumerable refactorNotifyServices) { _editorOptionsFactoryService = editorOptionsFactoryService; - _refactorNotifyServices = refactorNotifyServices; } public ILanguageService CreateLanguageService(HostLanguageServices provider) { - return new CSharpCodeModelService(provider, _editorOptionsFactoryService, _refactorNotifyServices); + return new CSharpCodeModelService(provider, _editorOptionsFactoryService); } } } diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs index 87361ba016565..b8fce38b358e9 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs @@ -11,11 +11,13 @@ using EnvDTE; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Editor; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; using Microsoft.CodeAnalysis.Emit; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Rename; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.Shared.Utilities; @@ -102,6 +104,7 @@ internal abstract partial class VisualStudioWorkspaceImpl : VisualStudioWorkspac private readonly Lazy _projectCodeModelFactory; private readonly IEnumerable> _documentOptionsProviderFactories; + private readonly IEnumerable> _refactorNotifyServices; private bool _documentOptionsProvidersInitialized = false; private readonly Dictionary _projectCompilationOutputs = new Dictionary(); private readonly object _projectCompilationOutputsGuard = new object(); @@ -119,6 +122,7 @@ public VisualStudioWorkspaceImpl(ExportProvider exportProvider, IAsyncServicePro _projectionBufferFactoryService = exportProvider.GetExportedValue(); _projectCodeModelFactory = exportProvider.GetExport(); _documentOptionsProviderFactories = exportProvider.GetExports(); + _refactorNotifyServices = exportProvider.GetExports(); // We fetch this lazily because VisualStudioProjectFactory depends on VisualStudioWorkspaceImpl -- we have a circularity. Since this // exists right now as a compat shim, we'll just do this. @@ -366,7 +370,7 @@ public override EnvDTE.FileCodeModel GetFileCodeModel(DocumentId documentId) } internal override bool TryApplyChanges( - Microsoft.CodeAnalysis.Solution newSolution, + CodeAnalysis.Solution newSolution, IProgressTracker progressTracker) { if (!_foregroundObject.IsForeground()) @@ -384,7 +388,22 @@ internal override bool TryApplyChanges( this.EnsureEditableDocuments(changedDocs); } - return base.TryApplyChanges(newSolution, progressTracker); + var changedSymbols = RenameSymbolAnnotation + .GatherChangedSymbolsInDocumentsAsync(changedDocs, CurrentSolution, currentSolution, CancellationToken.None) + .WaitAndGetResult_CanCallOnBackground(CancellationToken.None); + + var notifyRefactorOnBeforeSucceeded = TryNotifyRefactorBeforeChanges(changedSymbols, changedDocs); + + if (base.TryApplyChanges(newSolution, progressTracker)) + { + if (notifyRefactorOnBeforeSucceeded) + { + NotifyRefactorAfterChanges(changedSymbols, changedDocs); + } + return true; + } + + return false; bool CanApplyChange(DocumentId documentId) { @@ -399,6 +418,33 @@ bool CanApplyChange(DocumentId documentId) } } + private void NotifyRefactorAfterChanges(ImmutableDictionary changedSymbols, List changedDocumentIds) + { + foreach (var (oldSymbol, newSymbol) in changedSymbols) + { + foreach (var refactorNotifyService in _refactorNotifyServices) + { + refactorNotifyService.Value.TryOnAfterGlobalSymbolRenamed(this, changedDocumentIds, oldSymbol, newSymbol.ToDisplayString(), throwOnFailure: false); + } + } + } + + private bool TryNotifyRefactorBeforeChanges(ImmutableDictionary changedSymbols, IEnumerable changedDocumentIds) + { + foreach (var (oldSymbol, newSymbol) in changedSymbols) + { + foreach (var refactorNotifyService in _refactorNotifyServices) + { + if (!refactorNotifyService.Value.TryOnBeforeGlobalSymbolRenamed(this, changedDocumentIds, oldSymbol, newSymbol.ToDisplayString(), throwOnFailure: false)) + { + return false; + } + } + } + + return true; + } + public override bool CanOpenDocuments { get diff --git a/src/VisualStudio/Core/Def/Implementation/VisualStudioRenameTrackingDismissService.cs b/src/VisualStudio/Core/Def/Implementation/VisualStudioRenameTrackingDismissService.cs new file mode 100644 index 0000000000000..cedaac451d5a6 --- /dev/null +++ b/src/VisualStudio/Core/Def/Implementation/VisualStudioRenameTrackingDismissService.cs @@ -0,0 +1,26 @@ +// 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.Generic; +using System.Composition; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Editor; +using Microsoft.CodeAnalysis.Editor.Shared.Utilities; + +namespace Microsoft.VisualStudio.LanguageServices.Implementation +{ + /// + /// Handles dismissing the rename tracker when a symbol has changed + /// + [Export(typeof(IRefactorNotifyService)), Shared] + internal class VisualStudioRenameTrackingDismissService : IRefactorNotifyService + { + public bool TryOnBeforeGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, ISymbol symbol, string newName, bool throwOnFailure) + => true; + + public bool TryOnAfterGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, ISymbol symbol, string newName, bool throwOnFailure) + { + RenameTrackingDismisser.DismissRenameTracking(workspace, changedDocumentIDs); + return true; + } + } +} diff --git a/src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioSymbolRenamedCodeActionOperationFactoryWorkspaceService.cs b/src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioSymbolRenamedCodeActionOperationFactoryWorkspaceService.cs deleted file mode 100644 index f8e9fca6831ff..0000000000000 --- a/src/VisualStudio/Core/Def/Implementation/Workspace/VisualStudioSymbolRenamedCodeActionOperationFactoryWorkspaceService.cs +++ /dev/null @@ -1,82 +0,0 @@ -// 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.Collections.Generic; -using System.Composition; -using System.Linq; -using System.Threading; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeActions; -using Microsoft.CodeAnalysis.CodeActions.WorkspaceServices; -using Microsoft.CodeAnalysis.Editor; -using Microsoft.CodeAnalysis.Host.Mef; - -namespace Microsoft.VisualStudio.LanguageServices.Implementation -{ - using Workspace = Microsoft.CodeAnalysis.Workspace; - - [ExportWorkspaceService(typeof(ISymbolRenamedCodeActionOperationFactoryWorkspaceService), ServiceLayer.Host), Shared] - internal sealed class VisualStudioSymbolRenamedCodeActionOperationFactoryWorkspaceService : ISymbolRenamedCodeActionOperationFactoryWorkspaceService - { - private readonly IEnumerable _refactorNotifyServices; - - [ImportingConstructor] - public VisualStudioSymbolRenamedCodeActionOperationFactoryWorkspaceService( - [ImportMany] IEnumerable refactorNotifyServices) - { - _refactorNotifyServices = refactorNotifyServices; - } - - public CodeActionOperation CreateSymbolRenamedOperation(ISymbol symbol, string newName, Solution startingSolution, Solution updatedSolution) - { - return new RenameSymbolOperation( - _refactorNotifyServices, - symbol ?? throw new ArgumentNullException(nameof(symbol)), - newName ?? throw new ArgumentNullException(nameof(newName)), - startingSolution ?? throw new ArgumentNullException(nameof(startingSolution)), - updatedSolution ?? throw new ArgumentNullException(nameof(updatedSolution))); - } - - private class RenameSymbolOperation : CodeActionOperation - { - private readonly IEnumerable _refactorNotifyServices; - private readonly ISymbol _symbol; - private readonly string _newName; - private readonly Solution _startingSolution; - private readonly Solution _updatedSolution; - - public RenameSymbolOperation( - IEnumerable refactorNotifyServices, - ISymbol symbol, - string newName, - Solution startingSolution, - Solution updatedSolution) - { - _refactorNotifyServices = refactorNotifyServices; - _symbol = symbol; - _newName = newName; - _startingSolution = startingSolution; - _updatedSolution = updatedSolution; - } - - public override void Apply(Workspace workspace, CancellationToken cancellationToken = default) - { - var updatedDocumentIds = _updatedSolution.GetChanges(_startingSolution).GetProjectChanges().SelectMany(p => p.GetChangedDocuments()); - - foreach (var refactorNotifyService in _refactorNotifyServices) - { - // If something goes wrong and some language service rejects the rename, we - // can't really do anything about it because we're potentially in the middle of - // some unknown set of CodeActionOperations. This is a best effort approach. - - if (refactorNotifyService.TryOnBeforeGlobalSymbolRenamed(workspace, updatedDocumentIds, _symbol, _newName, throwOnFailure: false)) - { - refactorNotifyService.TryOnAfterGlobalSymbolRenamed(workspace, updatedDocumentIds, _symbol, _newName, throwOnFailure: false); - } - } - } - - public override string Title => string.Format(EditorFeaturesResources.Rename_0_to_1, _symbol.Name, _newName); - } - } -} diff --git a/src/VisualStudio/Core/Def/Microsoft.VisualStudio.LanguageServices.csproj b/src/VisualStudio/Core/Def/Microsoft.VisualStudio.LanguageServices.csproj index 0a68311f0e141..aa596a997f1ea 100644 --- a/src/VisualStudio/Core/Def/Microsoft.VisualStudio.LanguageServices.csproj +++ b/src/VisualStudio/Core/Def/Microsoft.VisualStudio.LanguageServices.csproj @@ -273,5 +273,8 @@ + + + diff --git a/src/VisualStudio/Core/Impl/CodeModel/AbstractCodeModelService.cs b/src/VisualStudio/Core/Impl/CodeModel/AbstractCodeModelService.cs index 3360c3d1b62ee..5a13501edb2c4 100644 --- a/src/VisualStudio/Core/Impl/CodeModel/AbstractCodeModelService.cs +++ b/src/VisualStudio/Core/Impl/CodeModel/AbstractCodeModelService.cs @@ -46,7 +46,6 @@ internal abstract partial class AbstractCodeModelService : ICodeModelService private readonly AbstractNodeNameGenerator _nodeNameGenerator; private readonly AbstractNodeLocator _nodeLocator; private readonly AbstractCodeModelEventCollector _eventCollector; - private readonly IEnumerable _refactorNotifyServices; private readonly AbstractFormattingRule _lineAdjustmentFormattingRule; private readonly AbstractFormattingRule _endRegionFormattingRule; @@ -54,7 +53,6 @@ internal abstract partial class AbstractCodeModelService : ICodeModelService protected AbstractCodeModelService( HostLanguageServices languageServiceProvider, IEditorOptionsFactoryService editorOptionsFactoryService, - IEnumerable refactorNotifyServices, AbstractFormattingRule lineAdjustmentFormattingRule, AbstractFormattingRule endRegionFormattingRule) { @@ -66,7 +64,6 @@ protected AbstractCodeModelService( _editorOptionsFactoryService = editorOptionsFactoryService; _lineAdjustmentFormattingRule = lineAdjustmentFormattingRule; _endRegionFormattingRule = endRegionFormattingRule; - _refactorNotifyServices = refactorNotifyServices; _nodeNameGenerator = CreateNodeNameGenerator(); _nodeLocator = CreateNodeLocator(); _eventCollector = CreateCodeModelEventCollector(); @@ -496,10 +493,6 @@ public void Rename(ISymbol symbol, string newName, Workspace workspace, ProjectC // Rename symbol. var oldSolution = workspace.CurrentSolution; var newSolution = Renamer.RenameSymbolAsync(oldSolution, symbol, newName, oldSolution.Options).WaitAndGetResult_CodeModel(CancellationToken.None); - var changedDocuments = newSolution.GetChangedDocuments(oldSolution); - - // Notify third parties of the coming rename operation and let exceptions propagate out - _refactorNotifyServices.TryOnBeforeGlobalSymbolRenamed(workspace, changedDocuments, symbol, newName, throwOnFailure: true); // Update the workspace. if (!workspace.TryApplyChanges(newSolution)) @@ -507,11 +500,6 @@ public void Rename(ISymbol symbol, string newName, Workspace workspace, ProjectC throw Exceptions.ThrowEFail(); } - // Notify third parties of the completed rename operation and let exceptions propagate out - _refactorNotifyServices.TryOnAfterGlobalSymbolRenamed(workspace, changedDocuments, symbol, newName, throwOnFailure: true); - - RenameTrackingDismisser.DismissRenameTracking(workspace, changedDocuments); - // Update the node keys. nodeKeyValidation.RestoreKeys(); } diff --git a/src/VisualStudio/Core/Test/Microsoft.VisualStudio.LanguageServices.UnitTests.vbproj b/src/VisualStudio/Core/Test/Microsoft.VisualStudio.LanguageServices.UnitTests.vbproj index 0c302d023513b..69025871338d7 100644 --- a/src/VisualStudio/Core/Test/Microsoft.VisualStudio.LanguageServices.UnitTests.vbproj +++ b/src/VisualStudio/Core/Test/Microsoft.VisualStudio.LanguageServices.UnitTests.vbproj @@ -87,4 +87,7 @@ + + + \ No newline at end of file diff --git a/src/VisualStudio/VisualBasic/Impl/CodeModel/VisualBasicCodeModelService.vb b/src/VisualStudio/VisualBasic/Impl/CodeModel/VisualBasicCodeModelService.vb index ea209391112b5..a889c099c0736 100644 --- a/src/VisualStudio/VisualBasic/Impl/CodeModel/VisualBasicCodeModelService.vb +++ b/src/VisualStudio/VisualBasic/Impl/CodeModel/VisualBasicCodeModelService.vb @@ -31,11 +31,10 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.CodeModel Private ReadOnly _commitBufferManagerFactory As CommitBufferManagerFactory - Friend Sub New(provider As HostLanguageServices, editorOptionsFactoryService As IEditorOptionsFactoryService, refactorNotifyServices As IEnumerable(Of IRefactorNotifyService), commitBufferManagerFactory As CommitBufferManagerFactory) + Friend Sub New(provider As HostLanguageServices, editorOptionsFactoryService As IEditorOptionsFactoryService, commitBufferManagerFactory As CommitBufferManagerFactory) MyBase.New( provider, editorOptionsFactoryService, - refactorNotifyServices, LineAdjustmentFormattingRule.Instance, EndRegionFormattingRule.Instance) diff --git a/src/VisualStudio/VisualBasic/Impl/CodeModel/VisualBasicCodeModelServiceFactory.vb b/src/VisualStudio/VisualBasic/Impl/CodeModel/VisualBasicCodeModelServiceFactory.vb index da155f63d5e40..1c53668283418 100644 --- a/src/VisualStudio/VisualBasic/Impl/CodeModel/VisualBasicCodeModelServiceFactory.vb +++ b/src/VisualStudio/VisualBasic/Impl/CodeModel/VisualBasicCodeModelServiceFactory.vb @@ -15,20 +15,17 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.CodeModel Implements ILanguageServiceFactory Private ReadOnly _editorOptionsFactoryService As IEditorOptionsFactoryService - Private ReadOnly _refactorNotifyServices As IEnumerable(Of IRefactorNotifyService) Private ReadOnly _commitBufferManagerFactory As CommitBufferManagerFactory Public Sub New(editorOptionsFactoryService As IEditorOptionsFactoryService, - refactorNotifyServices As IEnumerable(Of IRefactorNotifyService), commitBufferManagerFactory As CommitBufferManagerFactory) Me._editorOptionsFactoryService = editorOptionsFactoryService - Me._refactorNotifyServices = refactorNotifyServices Me._commitBufferManagerFactory = commitBufferManagerFactory End Sub Public Function CreateLanguageService(provider As HostLanguageServices) As ILanguageService Implements ILanguageServiceFactory.CreateLanguageService - Return New VisualBasicCodeModelService(provider, Me._editorOptionsFactoryService, _refactorNotifyServices, _commitBufferManagerFactory) + Return New VisualBasicCodeModelService(provider, Me._editorOptionsFactoryService, _commitBufferManagerFactory) End Function End Class End Namespace diff --git a/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs b/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs index f768476f82d91..d606bb3bcfc90 100644 --- a/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs +++ b/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs @@ -1442,6 +1442,9 @@ public bool IsDeclaration(SyntaxNode node) public bool IsTypeDeclaration(SyntaxNode node) => SyntaxFacts.IsTypeDeclaration(node.Kind()); + public bool IsNamespaceDeclaration(SyntaxNode node) + => node.IsKind(SyntaxKind.NamespaceDeclaration); + private static readonly SyntaxAnnotation s_annotation = new SyntaxAnnotation(); public void AddFirstMissingCloseBrace( diff --git a/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs b/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs index c3db71729ef2c..6d91aeab27e72 100644 --- a/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs +++ b/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs @@ -96,6 +96,7 @@ internal interface ISyntaxFactsService : ILanguageService bool IsGlobalAttribute(SyntaxNode node); bool IsDeclaration(SyntaxNode node); bool IsTypeDeclaration(SyntaxNode node); + bool IsNamespaceDeclaration(SyntaxNode node); bool IsRegularComment(SyntaxTrivia trivia); bool IsDocumentationComment(SyntaxTrivia trivia); diff --git a/src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolution.cs b/src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolution.cs index 3659d25e13108..3ea8702edbc96 100644 --- a/src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolution.cs +++ b/src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolution.cs @@ -1,5 +1,6 @@ // 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.Collections.Generic; using System.Collections.ObjectModel; using System.IO; diff --git a/src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolver.Session.cs b/src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolver.Session.cs index b12d906d7dd1a..20bf51368e650 100644 --- a/src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolver.Session.cs +++ b/src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolver.Session.cs @@ -120,14 +120,13 @@ public async Task ResolveConflictsAsync() var documentIdsThatGetsAnnotatedAndRenamed = new HashSet(documentsByProject); using (baseSolution.Services.CacheService?.EnableCaching(documentsByProject.Key)) { - // Rename is going to be in 5 phases. - // 1st phase - Does a simple token replacement + // + // For the first few 3 iterations, the solution is updated gradually as changes are made. + // Iterative changes are broken into "phases": // If the 1st phase results in conflict then we perform then: // 2nd phase is to expand and simplify only the reference locations with conflicts // 3rd phase is to expand and simplify all the conflict locations (both reference and non-reference) - // If there are unresolved Conflicts after the 3rd phase then in 4th phase, - // We complexify and resolve locations that were resolvable and for the other locations we perform the normal token replacement like the first the phase. - // If the OptionSet has RenameFile to true, we rename files with the type declaration + // for (var phase = 0; phase < 4; phase++) { // Step 1: @@ -203,6 +202,11 @@ public async Task ResolveConflictsAsync() } } + // + // If there are unresolved Conflicts after the iterations, + // we complexify and resolve locations that were resolvable and for the other locations we perform the normal token replacement like the first the phase. + // + // This rename could break implicit references of this symbol (e.g. rename MoveNext on a collection like type in a // foreach/for each statement var renamedSymbolInNewSolution = await GetRenamedSymbolInCurrentSolutionAsync(conflictResolution).ConfigureAwait(false); @@ -231,22 +235,25 @@ await conflictResolution.NewSolution.GetDocument(_documentIdOfRenameSymbolDeclar await DebugVerifyNoErrorsAsync(conflictResolution, _documentsIdsToBeCheckedForConflict).ConfigureAwait(false); #endif - // Step 5: Rename declaration files + // + // If the OptionSet has RenameFile to true, we rename files with the type declaration + // if (_optionSet.GetOption(RenameOptions.RenameFile)) { var definitionLocations = _renameLocationSet.Symbol.Locations; - var definitionDocuments = definitionLocations - .Select(l => conflictResolution.OldSolution.GetDocument(l.SourceTree)) - .Distinct(); + var definitionsAndOriginalDocuments = definitionLocations + .Select(l => (l, conflictResolution.OldSolution.GetDocument(l.SourceTree))); - if (definitionDocuments.Count() == 1) + // At the moment, only single document renaming is allowed + if (definitionsAndOriginalDocuments.Count() == 1) { - // At the moment, only single document renaming is allowed - conflictResolution.RenameDocumentToMatchNewSymbol(definitionDocuments.Single()); + conflictResolution.RenameDocumentToMatchNewSymbol(definitionsAndOriginalDocuments.Single().Item2); } } - // Step 6: Drop changes made in unchangeable documents + // + // Drop changes made in unchangeable documents + // var (containsDisallowedChange, updatedSolution) = await conflictResolution.NewSolution.ExcludeDisallowedDocumentTextChangesAsync(conflictResolution.OldSolution, _cancellationToken).ConfigureAwait(false); if (containsDisallowedChange) { @@ -276,9 +283,11 @@ private async Task DebugVerifyNoErrorsAsync(ConflictResolution conflictResolutio } // We want to ignore few error message introduced by rename because the user is wantedly doing it. - var ignoreErrorCodes = new List(); - ignoreErrorCodes.Add("BC30420"); // BC30420 - Sub Main missing in VB Project - ignoreErrorCodes.Add("CS5001"); // CS5001 - Missing Main in C# Project + var ignoreErrorCodes = new List + { + "BC30420", // BC30420 - Sub Main missing in VB Project + "CS5001" // CS5001 - Missing Main in C# Project + }; // only check if rename thinks it was successful if (conflictResolution.ReplacementTextValid && conflictResolution.RelatedLocations.All(loc => (loc.Type & RelatedLocationType.UnresolvableConflict) == 0)) @@ -324,7 +333,7 @@ private async Task IdentifyConflictsAsync( var newDocument = conflictResolution.NewSolution.GetDocument(documentId); var syntaxRoot = await newDocument.GetSyntaxRootAsync(_cancellationToken).ConfigureAwait(false); - var nodesOrTokensWithConflictCheckAnnotations = GetNodesOrTokensToCheckForConflicts(documentId, syntaxRoot); + var nodesOrTokensWithConflictCheckAnnotations = GetNodesOrTokensToCheckForConflicts(syntaxRoot); foreach (var (syntax, annotation) in nodesOrTokensWithConflictCheckAnnotations) { if (annotation.IsRenameLocation) @@ -351,7 +360,7 @@ private async Task IdentifyConflictsAsync( var syntaxFactsService = newDocument.Project.LanguageServices.GetService(); // Get all tokens that need conflict check - var nodesOrTokensWithConflictCheckAnnotations = GetNodesOrTokensToCheckForConflicts(documentId, syntaxRoot); + var nodesOrTokensWithConflictCheckAnnotations = GetNodesOrTokensToCheckForConflicts(syntaxRoot); var complexifiedLocationSpanForThisDocument = _conflictLocations @@ -380,7 +389,7 @@ private async Task IdentifyConflictsAsync( // to the new position for which we use the renameSpanTracker, which was tracking // & mapping the old span -> new span during rename hasConflict = _hasConflictCallback?.Invoke(newReferencedSymbols) ?? - await CheckForConflictAsync(conflictResolution, renamedSymbolInNewSolution, newDocument, conflictAnnotation, newReferencedSymbols).ConfigureAwait(false); + await CheckForConflictAsync(conflictResolution, renamedSymbolInNewSolution, conflictAnnotation, newReferencedSymbols).ConfigureAwait(false); } if (!hasConflict && !conflictAnnotation.IsInvocationExpression) @@ -440,7 +449,7 @@ private async Task IdentifyConflictsAsync( var baseDocument = conflictResolution.OldSolution.GetDocument(unprocessedDocumentIdWithPotentialDeclarationConflicts); var baseSyntaxTree = await baseDocument.GetSyntaxTreeAsync(_cancellationToken).ConfigureAwait(false); - var nodesOrTokensWithConflictCheckAnnotations = GetNodesOrTokensToCheckForConflicts(unprocessedDocumentIdWithPotentialDeclarationConflicts, syntaxRoot); + var nodesOrTokensWithConflictCheckAnnotations = GetNodesOrTokensToCheckForConflicts(syntaxRoot); foreach (var (syntax, annotation) in nodesOrTokensWithConflictCheckAnnotations) { var tokenOrNode = syntax; @@ -466,9 +475,8 @@ await AddDeclarationConflictsAsync( /// /// Gets the list of the nodes that were annotated for a conflict check /// - private IEnumerable<(SyntaxNodeOrToken syntax, RenameActionAnnotation annotation)> GetNodesOrTokensToCheckForConflicts( - DocumentId documentId, - SyntaxNode syntaxRoot) + private IEnumerable<(SyntaxNodeOrToken syntax, RenameActionAnnotation annotation)> + GetNodesOrTokensToCheckForConflicts(SyntaxNode syntaxRoot) { return syntaxRoot.DescendantNodesAndTokens(descendIntoTrivia: true) .Where(s => _renameAnnotations.HasAnnotations(s)) @@ -478,7 +486,6 @@ await AddDeclarationConflictsAsync( private async Task CheckForConflictAsync( ConflictResolution conflictResolution, ISymbol renamedSymbolInNewSolution, - Document newDocument, RenameActionAnnotation conflictAnnotation, IEnumerable newReferencedSymbols) { @@ -737,7 +744,7 @@ private async Task AddDocumentsWithPotentialConflicts(IEnumerable docu // The rename process and annotation for the bookkeeping is performed in one-step private async Task AnnotateAndRename_WorkerAsync( - Solution originalSolution, + Solution solution, Solution partiallyRenamedSolution, HashSet documentIdsToRename, ISet renameLocations, @@ -746,11 +753,32 @@ private async Task AnnotateAndRename_WorkerAsync( { try { + // Add the RenameSymbolAnnotation to the declarations + foreach (var syntaxReference in _renameLocationSet.Symbol.DeclaringSyntaxReferences) + { + var document = solution.GetDocument(syntaxReference.SyntaxTree); + if (document is null) + { + continue; + } + + var node = await syntaxReference.GetSyntaxAsync().ConfigureAwait(false); + var root = await document.GetSyntaxRootAsync().ConfigureAwait(false); + + var newRoot = root.ReplaceNode( + node, + node.WithoutAnnotations(RenameSymbolAnnotation.RenameSymbolKind) + .WithAdditionalAnnotations(RenameSymbolAnnotation.Create(_renameLocationSet.Symbol))); + + var annotatedDocument = document.WithSyntaxRoot(newRoot); + solution = annotatedDocument.Project.Solution; + } + foreach (var documentId in documentIdsToRename.ToList()) { _cancellationToken.ThrowIfCancellationRequested(); - var document = originalSolution.GetDocument(documentId); + var document = solution.GetDocument(documentId); var semanticModel = await document.GetSemanticModelAsync(_cancellationToken).ConfigureAwait(false); var originalSyntaxRoot = await semanticModel.SyntaxTree.GetRootAsync(_cancellationToken).ConfigureAwait(false); @@ -783,7 +811,7 @@ private async Task AnnotateAndRename_WorkerAsync( allTextSpansInSingleSourceTree, stringAndCommentTextSpansInSingleSourceTree, conflictLocationSpans, - originalSolution, + solution, _renameLocationSet.Symbol, replacementTextValid, renameSpansTracker, diff --git a/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs b/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs new file mode 100644 index 0000000000000..c134534afb7bb --- /dev/null +++ b/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs @@ -0,0 +1,107 @@ +// 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.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Shared.Extensions; + +#nullable enable + +namespace Microsoft.CodeAnalysis.Rename +{ + /// + /// Provides a way to produce and consume annotations for rename + /// that contain the previous symbol serialized as a . + /// This annotations is used by + /// in some cases to notify the workspace host of refactorings. + /// + /// + /// This annotation is applied to a symbol that has been renamed. + /// When Workspace.TryApplyChanges happens in Visual Studio, we raise rename events for that symbol. + /// + internal static class RenameSymbolAnnotation + { + public const string RenameSymbolKind = nameof(RenameSymbolAnnotation); + + public static bool ShouldAnnotateSymbol(ISymbol symbol) + => symbol.DeclaringSyntaxReferences.Any(); + + public static SyntaxAnnotation? Create(ISymbol oldSymbol) + { + return ShouldAnnotateSymbol(oldSymbol) + ? new SyntaxAnnotation(RenameSymbolKind, SerializeData(oldSymbol)) + : null; + } + + public static TNode WithRenameSymbolAnnotation(this TNode node, SemanticModel semanticModel) + where TNode : SyntaxNode + => node.WithAdditionalAnnotations(Create(semanticModel.GetDeclaredSymbol(node))); + + public static async Task> GatherChangedSymbolsInDocumentsAsync( + IEnumerable changedDocumentIds, + Solution newSolution, Solution oldSolution, + CancellationToken cancellationToken) + { + var changedSymbols = ImmutableDictionary.CreateBuilder(); + + foreach (var changedDocId in changedDocumentIds) + { + var newDocument = newSolution.GetDocument(changedDocId); + + // Documents without syntax tree won't have the annotations attached + if (newDocument is null || !(newDocument.SupportsSyntaxTree)) + { + continue; + } + + var syntaxRoot = await newDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var symbolRenameNodes = syntaxRoot.GetAnnotatedNodes(RenameSymbolKind); + + foreach (var node in symbolRenameNodes) + { + foreach (var annotation in node.GetAnnotations(RenameSymbolKind)) + { + var oldDocument = oldSolution.GetDocument(changedDocId); + if (oldDocument is null) + { + continue; + } + + var newSemanticModel = await newDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var currentSemanticModel = await oldDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + + var oldSymbol = ResolveSymbol(annotation, currentSemanticModel.Compilation); + var newSymbol = newSemanticModel.GetDeclaredSymbol(node); + + changedSymbols.Add(oldSymbol, newSymbol); + } + } + } + + return changedSymbols.ToImmutable(); + } + + internal static ISymbol ResolveSymbol(SyntaxAnnotation annotation, Compilation oldCompilation) + { + if (annotation.Kind != RenameSymbolKind) + { + throw new InvalidOperationException($"'{annotation}' is not of kind {RenameSymbolKind}"); + } + + if (string.IsNullOrEmpty(annotation.Data)) + { + throw new InvalidOperationException($"'{annotation}' has no data"); + } + + var oldSymbolKey = SymbolKey.ResolveString(annotation.Data, oldCompilation); + + return oldSymbolKey.Symbol; + } + + private static string SerializeData(ISymbol symbol) + => symbol.GetSymbolKey().ToString(); + } +} diff --git a/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb b/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb index 226f59d2be97a..e9e9bbcc45237 100644 --- a/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb +++ b/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb @@ -1525,6 +1525,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic newContextNode = contextNode End Sub + Public Function IsNamespaceDeclaration(node As SyntaxNode) As Boolean Implements ISyntaxFactsService.IsNamespaceDeclaration + Return node.IsKind(SyntaxKind.NamespaceBlock) + End Function + Public Function GetObjectCreationInitializer(node As SyntaxNode) As SyntaxNode Implements ISyntaxFactsService.GetObjectCreationInitializer Return DirectCast(node, ObjectCreationExpressionSyntax).Initializer End Function From fb201ddafc239ed45302707fbd6875f7057cf69d Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Thu, 6 Feb 2020 12:22:52 -0800 Subject: [PATCH 2/9] Small cleanups from PR feedback --- .../NamingStyles/NamingStylesTests.cs | 9 ++-- .../RenameTracking/RenameTrackingTestState.cs | 43 +++++++++--------- .../Test2/Rename/RenameTestHelpers.vb | 5 +-- .../MockRefactorNotifyService.cs | 45 ------------------- .../Workspaces/TestRefactorNotify.cs | 23 +++++++--- .../Portable/Rename/RenameSymbolAnnotation.cs | 6 +-- 6 files changed, 52 insertions(+), 79 deletions(-) delete mode 100644 src/EditorFeatures/TestUtilities/RenameTracking/MockRefactorNotifyService.cs diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/NamingStyles/NamingStylesTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/NamingStyles/NamingStylesTests.cs index aac2d6539c2b2..f8598f7b8cc18 100644 --- a/src/EditorFeatures/CSharpTest/Diagnostics/NamingStyles/NamingStylesTests.cs +++ b/src/EditorFeatures/CSharpTest/Diagnostics/NamingStyles/NamingStylesTests.cs @@ -35,7 +35,6 @@ protected override TestWorkspace CreateWorkspaceFromFile(string initialMarkup, T protected TestWorkspace CreateWorkspaceFromFile(string initialMarkup, TestParameters parameters, IExportProviderFactory exportProviderFactory) => TestWorkspace.CreateCSharp(initialMarkup, parameters.parseOptions, parameters.compilationOptions, exportProvider: exportProviderFactory.CreateExportProvider()); - internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace) => (new CSharpNamingStyleDiagnosticAnalyzer(), new NamingStyleCodeFixProvider()); @@ -1234,8 +1233,7 @@ public async Task TestRefactorNotify() using var workspace = CreateWorkspaceFromOptions(markup, testParameters); - var refactorNotify = workspace.GetService() as TestRefactorNotify; - Assert.NotNull(refactorNotify); + var refactorNotify = (TestRefactorNotify)workspace.GetService(); var beforeCalled = false; var afterCalled = false; @@ -1244,13 +1242,18 @@ public async Task TestRefactorNotify() Assert.Equal("C", args.NewName); Assert.False(beforeCalled); beforeCalled = true; + + return true; }; TestRefactorNotify.SymbolRenamedEventHandler afterRename = (args) => { Assert.Equal("C", args.NewName); + Assert.True(beforeCalled); Assert.False(afterCalled); afterCalled = true; + + return true; }; refactorNotify.OnBeforeRename += beforeRename; diff --git a/src/EditorFeatures/Test/RenameTracking/RenameTrackingTestState.cs b/src/EditorFeatures/Test/RenameTracking/RenameTrackingTestState.cs index 2a4710441150b..7148ba1fab3ed 100644 --- a/src/EditorFeatures/Test/RenameTracking/RenameTrackingTestState.cs +++ b/src/EditorFeatures/Test/RenameTracking/RenameTrackingTestState.cs @@ -19,6 +19,7 @@ using Microsoft.CodeAnalysis.Notification; using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.Shared.Utilities; +using Microsoft.CodeAnalysis.Test.Utilities.Workspaces; using Microsoft.CodeAnalysis.Text.Shared.Extensions; using Microsoft.CodeAnalysis.UnitTests.Diagnostics; using Microsoft.VisualStudio.Composition; @@ -41,14 +42,10 @@ internal sealed class RenameTrackingTestState : IDisposable private readonly ITextUndoHistoryRegistry _historyRegistry; private string _notificationMessage = null; - private readonly TestHostDocument _hostDocument; - public TestHostDocument HostDocument { get { return _hostDocument; } } + public TestHostDocument HostDocument { get; } - private readonly IEditorOperations _editorOperations; - public IEditorOperations EditorOperations { get { return _editorOperations; } } - - private readonly MockRefactorNotifyService _mockRefactorNotifyService; - public MockRefactorNotifyService RefactorNotifyService { get { return _mockRefactorNotifyService; } } + public IEditorOperations EditorOperations { get; } + public TestRefactorNotify RefactorNotifyService { get; } private readonly CodeFixProvider _codeFixProvider; private readonly RenameTrackingCancellationCommandHandler _commandHandler = new RenameTrackingCancellationCommandHandler(); @@ -84,15 +81,21 @@ public RenameTrackingTestState( { this.Workspace = workspace; - _hostDocument = Workspace.Documents.First(); - _view = _hostDocument.GetTextView(); - _view.Caret.MoveTo(new SnapshotPoint(_view.TextSnapshot, _hostDocument.CursorPosition.Value)); - _editorOperations = Workspace.GetService().GetEditorOperations(_view); + HostDocument = Workspace.Documents.First(); + _view = HostDocument.GetTextView(); + _view.Caret.MoveTo(new SnapshotPoint(_view.TextSnapshot, HostDocument.CursorPosition.Value)); + EditorOperations = Workspace.GetService().GetEditorOperations(_view); _historyRegistry = Workspace.ExportProvider.GetExport().Value; - _mockRefactorNotifyService = new MockRefactorNotifyService + RefactorNotifyService = new TestRefactorNotify(); + + RefactorNotifyService.OnBeforeRename += (_) => + { + return onBeforeGlobalSymbolRenamedReturnValue; + }; + + RefactorNotifyService.OnAfterRename += (_) => { - OnBeforeSymbolRenamedReturnValue = onBeforeGlobalSymbolRenamedReturnValue, - OnAfterSymbolRenamedReturnValue = onAfterGlobalSymbolRenamedReturnValue + return onAfterGlobalSymbolRenamedReturnValue; }; // Mock the action taken by the workspace INotificationService @@ -106,22 +109,22 @@ public RenameTrackingTestState( Workspace.ExportProvider.GetExport().Value, Workspace.ExportProvider.GetExport().Value, Workspace.ExportProvider.GetExport().Value, - SpecializedCollections.SingletonEnumerable(_mockRefactorNotifyService), + SpecializedCollections.SingletonEnumerable(RefactorNotifyService), Workspace.ExportProvider.GetExportedValue()); - _tagger = tracker.CreateTagger(_hostDocument.GetTextBuffer()); + _tagger = tracker.CreateTagger(HostDocument.GetTextBuffer()); if (languageName == LanguageNames.CSharp) { _codeFixProvider = new CSharpRenameTrackingCodeFixProvider( _historyRegistry, - SpecializedCollections.SingletonEnumerable(_mockRefactorNotifyService)); + SpecializedCollections.SingletonEnumerable(RefactorNotifyService)); } else if (languageName == LanguageNames.VisualBasic) { _codeFixProvider = new VisualBasicRenameTrackingCodeFixProvider( _historyRegistry, - SpecializedCollections.SingletonEnumerable(_mockRefactorNotifyService)); + SpecializedCollections.SingletonEnumerable(RefactorNotifyService)); } else { @@ -175,7 +178,7 @@ public async Task AssertNoTag() public async Task> GetDocumentDiagnosticsAsync(Document document = null) { - document ??= this.Workspace.CurrentSolution.GetDocument(_hostDocument.Id); + document ??= this.Workspace.CurrentSolution.GetDocument(HostDocument.Id); var analyzer = new RenameTrackingDiagnosticAnalyzer(); return (await DiagnosticProviderTestUtilities.GetDocumentDiagnosticsAsync(analyzer, document, (await document.GetSyntaxRootAsync()).FullSpan)).ToList(); @@ -192,7 +195,7 @@ public async Task AssertTag(string expectedFromName, string expectedToName, bool var tag = tags.Single(); - var document = this.Workspace.CurrentSolution.GetDocument(_hostDocument.Id); + var document = this.Workspace.CurrentSolution.GetDocument(HostDocument.Id); var diagnostics = await GetDocumentDiagnosticsAsync(document); // There should be a single rename tracking diagnostic diff --git a/src/EditorFeatures/Test2/Rename/RenameTestHelpers.vb b/src/EditorFeatures/Test2/Rename/RenameTestHelpers.vb index 241b57e52352e..dafb50c0cfd6a 100644 --- a/src/EditorFeatures/Test2/Rename/RenameTestHelpers.vb +++ b/src/EditorFeatures/Test2/Rename/RenameTestHelpers.vb @@ -9,11 +9,10 @@ Imports Microsoft.CodeAnalysis.Editor.Host Imports Microsoft.CodeAnalysis.Editor.Implementation.InlineRename Imports Microsoft.CodeAnalysis.Editor.Implementation.RenameTracking Imports Microsoft.CodeAnalysis.Editor.Shared.Utilities -Imports Microsoft.CodeAnalysis.Editor.UnitTests.RenameTracking Imports Microsoft.CodeAnalysis.Editor.UnitTests.Utilities.GoToHelpers Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces -Imports Microsoft.CodeAnalysis.Experiments Imports Microsoft.CodeAnalysis.Shared.TestHooks +Imports Microsoft.CodeAnalysis.Test.Utilities.Workspaces Imports Microsoft.CodeAnalysis.Text Imports Microsoft.CodeAnalysis.Text.Shared.Extensions Imports Microsoft.VisualStudio.Composition @@ -132,7 +131,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename workspace.ExportProvider.GetExport(Of IWaitIndicator)().Value, workspace.ExportProvider.GetExport(Of IInlineRenameService)().Value, workspace.ExportProvider.GetExport(Of IDiagnosticAnalyzerService)().Value, - {New MockRefactorNotifyService()}, + {New TestRefactorNotify()}, workspace.ExportProvider.GetExportedValue(Of IAsynchronousOperationListenerProvider)) Return tracker.CreateTagger(Of RenameTrackingTag)(document.GetTextBuffer()) diff --git a/src/EditorFeatures/TestUtilities/RenameTracking/MockRefactorNotifyService.cs b/src/EditorFeatures/TestUtilities/RenameTracking/MockRefactorNotifyService.cs deleted file mode 100644 index 508369c62ba9d..0000000000000 --- a/src/EditorFeatures/TestUtilities/RenameTracking/MockRefactorNotifyService.cs +++ /dev/null @@ -1,45 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System.Collections.Generic; -using System.Runtime.InteropServices; - -namespace Microsoft.CodeAnalysis.Editor.UnitTests.RenameTracking -{ - public sealed class MockRefactorNotifyService : IRefactorNotifyService - { - private int _onBeforeSymbolRenamedCount = 0; - private int _onAfterSymbolRenamedCount = 0; - - public int OnBeforeSymbolRenamedCount { get { return _onBeforeSymbolRenamedCount; } } - public int OnAfterSymbolRenamedCount { get { return _onAfterSymbolRenamedCount; } } - - public bool OnBeforeSymbolRenamedReturnValue { get; set; } - public bool OnAfterSymbolRenamedReturnValue { get; set; } - - public bool TryOnBeforeGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, ISymbol symbol, string newName, bool throwOnFailure) - { - _onBeforeSymbolRenamedCount++; - - if (throwOnFailure && !OnBeforeSymbolRenamedReturnValue) - { - Marshal.ThrowExceptionForHR(unchecked((int)0x80004004)); // E_ABORT - } - - return OnBeforeSymbolRenamedReturnValue; - } - - public bool TryOnAfterGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, ISymbol symbol, string newName, bool throwOnFailure) - { - _onAfterSymbolRenamedCount++; - - if (throwOnFailure && !OnAfterSymbolRenamedReturnValue) - { - Marshal.ThrowExceptionForHR(unchecked((int)0x80004004)); // E_ABORT - } - - return OnAfterSymbolRenamedReturnValue; - } - } -} diff --git a/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs b/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs index 33977a5f4c47d..0b931218e543c 100644 --- a/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs +++ b/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Composition; +using System.Runtime.InteropServices; using Microsoft.CodeAnalysis.Editor; namespace Microsoft.CodeAnalysis.Test.Utilities.Workspaces @@ -10,20 +11,32 @@ namespace Microsoft.CodeAnalysis.Test.Utilities.Workspaces [PartNotDiscoverable] internal class TestRefactorNotify : IRefactorNotifyService { - public delegate void SymbolRenamedEventHandler(SymbolRenameEventArgs args); + public delegate bool SymbolRenamedEventHandler(SymbolRenameEventArgs args); public event SymbolRenamedEventHandler OnAfterRename; public event SymbolRenamedEventHandler OnBeforeRename; public bool TryOnAfterGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, ISymbol symbol, string newName, bool throwOnFailure) { - OnAfterRename?.Invoke(new SymbolRenameEventArgs(workspace, changedDocumentIDs, symbol, newName)); - return true; + var succeeded = OnAfterRename?.Invoke(new SymbolRenameEventArgs(workspace, changedDocumentIDs, symbol, newName)) ?? true; + + if (throwOnFailure && !succeeded) + { + Marshal.ThrowExceptionForHR(unchecked((int)0x80004004)); // E_ABORT + } + + return succeeded; } public bool TryOnBeforeGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, ISymbol symbol, string newName, bool throwOnFailure) { - OnBeforeRename?.Invoke(new SymbolRenameEventArgs(workspace, changedDocumentIDs, symbol, newName)); - return true; + var succeeded = OnBeforeRename?.Invoke(new SymbolRenameEventArgs(workspace, changedDocumentIDs, symbol, newName)) ?? true; + + if (throwOnFailure && !succeeded) + { + Marshal.ThrowExceptionForHR(unchecked((int)0x80004004)); // E_ABORT + } + + return succeeded; } } } diff --git a/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs b/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs index c134534afb7bb..85520d8e47144 100644 --- a/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs +++ b/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs @@ -19,7 +19,7 @@ namespace Microsoft.CodeAnalysis.Rename /// in some cases to notify the workspace host of refactorings. /// /// - /// This annotation is applied to a symbol that has been renamed. + /// This annotation is applied to the declaring syntax of a symbol that has been renamed. /// When Workspace.TryApplyChanges happens in Visual Studio, we raise rename events for that symbol. /// internal static class RenameSymbolAnnotation @@ -71,9 +71,9 @@ public static async Task> GatherChangedSym } var newSemanticModel = await newDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - var currentSemanticModel = await oldDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var oldSemanticModel = await oldDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - var oldSymbol = ResolveSymbol(annotation, currentSemanticModel.Compilation); + var oldSymbol = ResolveSymbol(annotation, oldSemanticModel.Compilation); var newSymbol = newSemanticModel.GetDeclaredSymbol(node); changedSymbols.Add(oldSymbol, newSymbol); From 181f210cc52e3ed26e1da7a8f20d49cde9052943 Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Thu, 6 Feb 2020 12:35:19 -0800 Subject: [PATCH 3/9] More missed cleanup places --- .../MoveToNamespace/AbstractMoveToNamespaceTests.cs | 4 ++++ .../TestUtilities/Workspaces/TestRefactorNotify.cs | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/src/EditorFeatures/TestUtilities/MoveToNamespace/AbstractMoveToNamespaceTests.cs b/src/EditorFeatures/TestUtilities/MoveToNamespace/AbstractMoveToNamespaceTests.cs index d1d512d5b21d4..79c0213ebb9cf 100644 --- a/src/EditorFeatures/TestUtilities/MoveToNamespace/AbstractMoveToNamespaceTests.cs +++ b/src/EditorFeatures/TestUtilities/MoveToNamespace/AbstractMoveToNamespaceTests.cs @@ -106,12 +106,16 @@ private async Task TestRefactorNotifyAsync( var expectedRenameTracking = expectedRenames.First(r => r.New == args.NewName); Assert.False(expectedRenameTracking.OnBeforeCalled); expectedRenameTracking.OnBeforeCalled = true; + + return true; }; TestRefactorNotify.SymbolRenamedEventHandler afterRename = (args) => { var expectedRenameTracking = expectedRenames.First(r => r.New == args.NewName); expectedRenameTracking.OnAfterCalled = true; + + return true; }; if (refactorNotify is object) diff --git a/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs b/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs index 0b931218e543c..feb539e06339d 100644 --- a/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs +++ b/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs @@ -15,8 +15,13 @@ internal class TestRefactorNotify : IRefactorNotifyService public event SymbolRenamedEventHandler OnAfterRename; public event SymbolRenamedEventHandler OnBeforeRename; + public int OnBeforeSymbolRenamedCount { get; private set; } + public int OnAfterSymbolRenamedCount {get; private set;} + public bool TryOnAfterGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, ISymbol symbol, string newName, bool throwOnFailure) { + OnBeforeSymbolRenamedCount++; + var succeeded = OnAfterRename?.Invoke(new SymbolRenameEventArgs(workspace, changedDocumentIDs, symbol, newName)) ?? true; if (throwOnFailure && !succeeded) @@ -29,6 +34,8 @@ public bool TryOnAfterGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, ISymbol symbol, string newName, bool throwOnFailure) { + OnAfterSymbolRenamedCount++; + var succeeded = OnBeforeRename?.Invoke(new SymbolRenameEventArgs(workspace, changedDocumentIDs, symbol, newName)) ?? true; if (throwOnFailure && !succeeded) From 9f4c76efb98d6d0187674d9e6fb46398e03b84ca Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Thu, 6 Feb 2020 13:20:20 -0800 Subject: [PATCH 4/9] Use GetRequiredDocument --- src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs b/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs index 85520d8e47144..a8b7af564cee9 100644 --- a/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs +++ b/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs @@ -49,10 +49,10 @@ public static async Task> GatherChangedSym foreach (var changedDocId in changedDocumentIds) { - var newDocument = newSolution.GetDocument(changedDocId); + var newDocument = newSolution.GetRequiredDocument(changedDocId); // Documents without syntax tree won't have the annotations attached - if (newDocument is null || !(newDocument.SupportsSyntaxTree)) + if (!newDocument.SupportsSyntaxTree) { continue; } From 14b6c26a99123fd22c56c2ceb29110cdcd32d891 Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Thu, 6 Feb 2020 13:23:29 -0800 Subject: [PATCH 5/9] Update header to MIT license --- .../TestUtilities/Workspaces/TestRefactorNotify.cs | 6 ++++-- .../VisualStudioRenameTrackingDismissService.cs | 4 +++- .../Core/Portable/Rename/RenameSymbolAnnotation.cs | 4 +++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs b/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs index feb539e06339d..ccaefff9be076 100644 --- a/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs +++ b/src/EditorFeatures/TestUtilities/Workspaces/TestRefactorNotify.cs @@ -1,4 +1,6 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. using System.Collections.Generic; using System.Composition; @@ -16,7 +18,7 @@ internal class TestRefactorNotify : IRefactorNotifyService public event SymbolRenamedEventHandler OnBeforeRename; public int OnBeforeSymbolRenamedCount { get; private set; } - public int OnAfterSymbolRenamedCount {get; private set;} + public int OnAfterSymbolRenamedCount { get; private set; } public bool TryOnAfterGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, ISymbol symbol, string newName, bool throwOnFailure) { diff --git a/src/VisualStudio/Core/Def/Implementation/VisualStudioRenameTrackingDismissService.cs b/src/VisualStudio/Core/Def/Implementation/VisualStudioRenameTrackingDismissService.cs index cedaac451d5a6..0b8a5eea4656b 100644 --- a/src/VisualStudio/Core/Def/Implementation/VisualStudioRenameTrackingDismissService.cs +++ b/src/VisualStudio/Core/Def/Implementation/VisualStudioRenameTrackingDismissService.cs @@ -1,4 +1,6 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. using System.Collections.Generic; using System.Composition; diff --git a/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs b/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs index a8b7af564cee9..82e77907f6cb1 100644 --- a/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs +++ b/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs @@ -1,4 +1,6 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. using System; using System.Collections.Generic; From de7f62a5e2d7fd4a6018c5656a5f3576510191d8 Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Thu, 6 Feb 2020 17:44:37 -0800 Subject: [PATCH 6/9] Cleanup a lot of uses of IRefactorNotifyService still lingering --- .../CSharpRenameTrackingCodeFixProvider.cs | 7 +-- .../NamingStyles/NamingStylesTests.cs | 44 +++++-------- ...torInlineRenameService.SymbolRenameInfo.cs | 15 ----- .../AbstractEditorInlineRenameService.cs | 12 +--- .../CSharpEditorInlineRenameService.cs | 6 -- .../InlineRename/InlineRenameService.cs | 4 -- .../InlineRename/InlineRenameSession.cs | 22 ------- .../VisualBasicEditorInlineRenameService.cs | 6 -- .../IEditorInlineRenameService.cs | 12 ---- .../AbstractRenameTrackingCodeFixProvider.cs | 7 +-- ...TaggerProvider.RenameTrackingCodeAction.cs | 6 +- ...gTaggerProvider.RenameTrackingCommitter.cs | 25 -------- .../RenameTrackingTaggerProvider.Tagger.cs | 5 +- .../RenameTrackingTaggerProvider.cs | 8 +-- .../RenameTrackingTaggerProviderTests.cs | 63 ------------------- .../RenameTracking/RenameTrackingTestState.cs | 11 +--- .../Test/Utilities/EditorServicesUtil.cs | 4 +- .../Test2/Rename/RenameTestHelpers.vb | 1 - .../Utilities/TestRenameSymbolAnnotation.cs | 27 ++++++++ .../RenameTrackingCodeFixProvider.vb | 5 +- .../VisualStudioWorkspaceImpl.cs | 4 +- .../Portable/Rename/RenameSymbolAnnotation.cs | 51 +++++++++------ 22 files changed, 93 insertions(+), 252 deletions(-) create mode 100644 src/EditorFeatures/TestUtilities/Utilities/TestRenameSymbolAnnotation.cs diff --git a/src/EditorFeatures/CSharp/RenameTracking/CSharpRenameTrackingCodeFixProvider.cs b/src/EditorFeatures/CSharp/RenameTracking/CSharpRenameTrackingCodeFixProvider.cs index c4860baa8cc14..79138951f7af8 100644 --- a/src/EditorFeatures/CSharp/RenameTracking/CSharpRenameTrackingCodeFixProvider.cs +++ b/src/EditorFeatures/CSharp/RenameTracking/CSharpRenameTrackingCodeFixProvider.cs @@ -2,10 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Collections.Generic; using System.Composition; using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.Editor.Host; using Microsoft.CodeAnalysis.Editor.Implementation.RenameTracking; using Microsoft.VisualStudio.Text.Operations; @@ -36,9 +34,8 @@ internal sealed class CSharpRenameTrackingCodeFixProvider : AbstractRenameTracki { [ImportingConstructor] public CSharpRenameTrackingCodeFixProvider( - ITextUndoHistoryRegistry undoHistoryRegistry, - [ImportMany] IEnumerable refactorNotifyServices) - : base(undoHistoryRegistry, refactorNotifyServices) + ITextUndoHistoryRegistry undoHistoryRegistry) + : base(undoHistoryRegistry) { } } diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/NamingStyles/NamingStylesTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/NamingStyles/NamingStylesTests.cs index f8598f7b8cc18..d146c1a752297 100644 --- a/src/EditorFeatures/CSharpTest/Diagnostics/NamingStyles/NamingStylesTests.cs +++ b/src/EditorFeatures/CSharpTest/Diagnostics/NamingStyles/NamingStylesTests.cs @@ -2,7 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Collections.Generic; using System.Collections.Immutable; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeFixes; @@ -13,6 +15,8 @@ using Microsoft.CodeAnalysis.Editor.UnitTests; using Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics.NamingStyles; using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces; +using Microsoft.CodeAnalysis.Rename; +using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Test.Utilities; using Microsoft.CodeAnalysis.Test.Utilities.Workspaces; using Microsoft.VisualStudio.Composition; @@ -1226,38 +1230,14 @@ internal interface [Fact, Trait(Traits.Feature, Traits.Features.NamingStyle)] [WorkItem(16562, "https://github.com/dotnet/roslyn/issues/16562")] - public async Task TestRefactorNotify() + public async Task TestRenameSymbolAnnotation() { var markup = @"public class [|c|] { }"; var testParameters = new TestParameters(options: options.ClassNamesArePascalCase); using var workspace = CreateWorkspaceFromOptions(markup, testParameters); - var refactorNotify = (TestRefactorNotify)workspace.GetService(); - - var beforeCalled = false; - var afterCalled = false; - TestRefactorNotify.SymbolRenamedEventHandler beforeRename = (args) => - { - Assert.Equal("C", args.NewName); - Assert.False(beforeCalled); - beforeCalled = true; - - return true; - }; - - TestRefactorNotify.SymbolRenamedEventHandler afterRename = (args) => - { - Assert.Equal("C", args.NewName); - Assert.True(beforeCalled); - Assert.False(afterCalled); - afterCalled = true; - - return true; - }; - - refactorNotify.OnBeforeRename += beforeRename; - refactorNotify.OnAfterRename += afterRename; + var originalSolution = workspace.CurrentSolution; var (_, action) = await GetCodeActionsAsync(workspace, testParameters); var operations = await action.GetOperationsAsync(CancellationToken.None); @@ -1267,11 +1247,15 @@ public async Task TestRefactorNotify() operation.Apply(workspace, CancellationToken.None); } - Assert.True(beforeCalled); - Assert.True(afterCalled); + var newSolution = workspace.CurrentSolution; - refactorNotify.OnBeforeRename -= beforeRename; - refactorNotify.OnAfterRename -= afterRename; + await Test.Utilities.Utilities.TestRenameSymbolAnnotation.ValidateRenameSymbolAnnotationsAsync( + originalSolution, + newSolution, + new Dictionary() + { + {"c", "C" } + }.ToImmutableDictionary()); } } } diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/AbstractEditorInlineRenameService.SymbolRenameInfo.cs b/src/EditorFeatures/Core.Wpf/InlineRename/AbstractEditorInlineRenameService.SymbolRenameInfo.cs index 37d678f2529b8..f1228742719b8 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/AbstractEditorInlineRenameService.SymbolRenameInfo.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/AbstractEditorInlineRenameService.SymbolRenameInfo.cs @@ -34,7 +34,6 @@ private partial class SymbolInlineRenameInfo : IInlineRenameInfoWithFileRename private readonly object _gate = new object(); private readonly Document _document; - private readonly IEnumerable _refactorNotifyServices; private Task _underlyingFindRenameLocationsTask; @@ -55,7 +54,6 @@ private partial class SymbolInlineRenameInfo : IInlineRenameInfoWithFileRename public ISymbol RenameSymbol => RenameSymbolAndProjectId.Symbol; public SymbolInlineRenameInfo( - IEnumerable refactorNotifyServices, Document document, TextSpan triggerSpan, SymbolAndProjectId renameSymbolAndProjectId, @@ -64,7 +62,6 @@ public SymbolInlineRenameInfo( { this.CanRename = true; - _refactorNotifyServices = refactorNotifyServices; _document = document; this.RenameSymbolAndProjectId = renameSymbolAndProjectId; @@ -232,18 +229,6 @@ private async Task GetLocationSet(Task changedDocumentIDs, string replacementText) - { - return _refactorNotifyServices.TryOnBeforeGlobalSymbolRenamed(workspace, changedDocumentIDs, RenameSymbol, - this.GetFinalSymbolName(replacementText), throwOnFailure: false); - } - - public bool TryOnAfterGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, string replacementText) - { - return _refactorNotifyServices.TryOnAfterGlobalSymbolRenamed(workspace, changedDocumentIDs, RenameSymbol, - this.GetFinalSymbolName(replacementText), throwOnFailure: false); - } - public InlineRenameFileRenameInfo GetFileRenameInfo() { if (RenameSymbol.Kind == SymbolKind.NamedType && diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/AbstractEditorInlineRenameService.cs b/src/EditorFeatures/Core.Wpf/InlineRename/AbstractEditorInlineRenameService.cs index 80277a5b37543..2284414d61491 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/AbstractEditorInlineRenameService.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/AbstractEditorInlineRenameService.cs @@ -21,13 +21,6 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename { internal abstract partial class AbstractEditorInlineRenameService : IEditorInlineRenameService { - private readonly IEnumerable _refactorNotifyServices; - - protected AbstractEditorInlineRenameService(IEnumerable refactorNotifyServices) - { - _refactorNotifyServices = refactorNotifyServices; - } - public Task GetRenameInfoAsync(Document document, int position, CancellationToken cancellationToken) { // This is unpleasant, but we do everything synchronously. That's because we end up @@ -50,11 +43,10 @@ private IInlineRenameInfo GetRenameInfo(Document document, int position, Cancell return new FailureInlineRenameInfo(EditorFeaturesResources.You_must_rename_an_identifier); } - return GetRenameInfo(_refactorNotifyServices, document, triggerToken, cancellationToken); + return GetRenameInfo(document, triggerToken, cancellationToken); } internal static IInlineRenameInfo GetRenameInfo( - IEnumerable refactorNotifyServices, Document document, SyntaxToken triggerToken, CancellationToken cancellationToken) { var syntaxFactsService = document.GetLanguageService(); @@ -219,7 +211,7 @@ internal static IInlineRenameInfo GetRenameInfo( } return new SymbolInlineRenameInfo( - refactorNotifyServices, document, triggerToken.Span, + document, triggerToken.Span, symbolAndProjectId, forceRenameOverloads, cancellationToken); } diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/CSharpEditorInlineRenameService.cs b/src/EditorFeatures/Core.Wpf/InlineRename/CSharpEditorInlineRenameService.cs index aaee7d9e7f3d1..365e593bf2a65 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/CSharpEditorInlineRenameService.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/CSharpEditorInlineRenameService.cs @@ -2,7 +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. -using System.Collections.Generic; using System.Composition; using Microsoft.CodeAnalysis.Editor.Implementation.InlineRename; using Microsoft.CodeAnalysis.Host.Mef; @@ -12,10 +11,5 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.InlineRename [ExportLanguageService(typeof(IEditorInlineRenameService), LanguageNames.CSharp), Shared] internal class CSharpEditorInlineRenameService : AbstractEditorInlineRenameService { - [ImportingConstructor] - public CSharpEditorInlineRenameService( - [ImportMany]IEnumerable refactorNotifyServices) : base(refactorNotifyServices) - { - } } } diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/InlineRenameService.cs b/src/EditorFeatures/Core.Wpf/InlineRename/InlineRenameService.cs index 9019ea681570a..e56e9fbe8f534 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/InlineRenameService.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/InlineRenameService.cs @@ -27,7 +27,6 @@ internal class InlineRenameService : IInlineRenameService private readonly IWaitIndicator _waitIndicator; private readonly ITextBufferAssociatedViewService _textBufferAssociatedViewService; private readonly IAsynchronousOperationListener _asyncListener; - private readonly IEnumerable _refactorNotifyServices; private readonly ITextBufferFactoryService _textBufferFactoryService; private readonly IFeatureServiceFactory _featureServiceFactory; private InlineRenameSession _activeRenameSession; @@ -40,7 +39,6 @@ public InlineRenameService( ITextBufferAssociatedViewService textBufferAssociatedViewService, ITextBufferFactoryService textBufferFactoryService, IFeatureServiceFactory featureServiceFactory, - [ImportMany] IEnumerable refactorNotifyServices, IAsynchronousOperationListenerProvider listenerProvider) { _threadingContext = threadingContext; @@ -48,7 +46,6 @@ public InlineRenameService( _textBufferAssociatedViewService = textBufferAssociatedViewService; _textBufferFactoryService = textBufferFactoryService; _featureServiceFactory = featureServiceFactory; - _refactorNotifyServices = refactorNotifyServices; _asyncListener = listenerProvider.GetListener(FeatureAttribute.Rename); } @@ -80,7 +77,6 @@ public InlineRenameSessionInfo StartInlineSession( _textBufferAssociatedViewService, _textBufferFactoryService, _featureServiceFactory, - _refactorNotifyServices, _asyncListener); return new InlineRenameSessionInfo(ActiveSession); diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/InlineRenameSession.cs b/src/EditorFeatures/Core.Wpf/InlineRename/InlineRenameSession.cs index 24fd9308f5e77..73b2e731e9563 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/InlineRenameSession.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/InlineRenameSession.cs @@ -41,7 +41,6 @@ internal partial class InlineRenameSession : ForegroundThreadAffinitizedObject, private readonly ITextBufferFactoryService _textBufferFactoryService; private readonly IFeatureService _featureService; private readonly IFeatureDisableToken _completionDisabledToken; - private readonly IEnumerable _refactorNotifyServices; private readonly IDebuggingWorkspaceService _debuggingWorkspaceService; private readonly IAsynchronousOperationListener _asyncListener; private readonly Solution _baseSolution; @@ -113,7 +112,6 @@ public InlineRenameSession( ITextBufferAssociatedViewService textBufferAssociatedViewService, ITextBufferFactoryService textBufferFactoryService, IFeatureServiceFactory featureServiceFactory, - IEnumerable refactorNotifyServices, IAsynchronousOperationListener asyncListener) : base(threadingContext, assertIsForeground: true) { @@ -141,7 +139,6 @@ public InlineRenameSession( _renameService = renameService; _waitIndicator = waitIndicator; - _refactorNotifyServices = refactorNotifyServices; _asyncListener = asyncListener; _triggerView = textBufferAssociatedViewService.GetAssociatedTextViews(triggerSpan.Snapshot.TextBuffer).FirstOrDefault(v => v.HasAggregateFocus) ?? textBufferAssociatedViewService.GetAssociatedTextViews(triggerSpan.Snapshot.TextBuffer).First(); @@ -762,16 +759,6 @@ private void ApplyRename(Solution newSolution, IWaitContext waitContext) var changes = _baseSolution.GetChanges(newSolution); var changedDocumentIDs = changes.GetProjectChanges().SelectMany(c => c.GetChangedDocuments()).ToList(); - if (!_renameInfo.TryOnBeforeGlobalSymbolRenamed(_workspace, changedDocumentIDs, this.ReplacementText)) - { - var notificationService = _workspace.Services.GetService(); - notificationService.SendNotification( - EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid, - EditorFeaturesResources.Rename_Symbol, - NotificationSeverity.Error); - return; - } - using var undoTransaction = _workspace.OpenGlobalUndoTransaction(EditorFeaturesResources.Inline_Rename); var finalSolution = newSolution.Workspace.CurrentSolution; foreach (var id in changedDocumentIDs) @@ -811,15 +798,6 @@ private void ApplyRename(Solution newSolution, IWaitContext waitContext) .SelectMany(c => c.GetChangedDocuments().Concat(c.GetAddedDocuments())) .ToList(); - if (!_renameInfo.TryOnAfterGlobalSymbolRenamed(_workspace, finalChangedIds, this.ReplacementText)) - { - var notificationService = _workspace.Services.GetService(); - notificationService.SendNotification( - EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated, - EditorFeaturesResources.Rename_Symbol, - NotificationSeverity.Information); - } - undoTransaction.Commit(); } } diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/VisualBasicEditorInlineRenameService.cs b/src/EditorFeatures/Core.Wpf/InlineRename/VisualBasicEditorInlineRenameService.cs index 3293326f0bf45..f77d9d5514bfc 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/VisualBasicEditorInlineRenameService.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/VisualBasicEditorInlineRenameService.cs @@ -2,7 +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. -using System.Collections.Generic; using System.Composition; using Microsoft.CodeAnalysis.Editor.Implementation.InlineRename; using Microsoft.CodeAnalysis.Host.Mef; @@ -12,10 +11,5 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.InlineRename [ExportLanguageService(typeof(IEditorInlineRenameService), LanguageNames.VisualBasic), Shared] internal class VisualBasicEditorInlineRenameService : AbstractEditorInlineRenameService { - [ImportingConstructor] - public VisualBasicEditorInlineRenameService( - [ImportMany]IEnumerable refactorNotifyServices) : base(refactorNotifyServices) - { - } } } diff --git a/src/EditorFeatures/Core/Implementation/InlineRename/IEditorInlineRenameService.cs b/src/EditorFeatures/Core/Implementation/InlineRename/IEditorInlineRenameService.cs index 634e799ef7373..1fcdbe566af24 100644 --- a/src/EditorFeatures/Core/Implementation/InlineRename/IEditorInlineRenameService.cs +++ b/src/EditorFeatures/Core/Implementation/InlineRename/IEditorInlineRenameService.cs @@ -220,18 +220,6 @@ internal interface IInlineRenameInfo /// locations to rename, as well as any time the rename options are changed by the user. /// Task FindRenameLocationsAsync(OptionSet optionSet, CancellationToken cancellationToken); - - /// - /// Called before the rename is applied to the specified documents in the workspace. Return - /// if rename should proceed, or if it should be canceled. - /// - bool TryOnBeforeGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, string replacementText); - - /// - /// Called after the rename is applied to the specified documents in the workspace. Return - /// if this operation succeeded, or if it failed. - /// - bool TryOnAfterGlobalSymbolRenamed(Workspace workspace, IEnumerable changedDocumentIDs, string replacementText); } internal interface IInlineRenameInfoWithFileRename : IInlineRenameInfo diff --git a/src/EditorFeatures/Core/Implementation/RenameTracking/AbstractRenameTrackingCodeFixProvider.cs b/src/EditorFeatures/Core/Implementation/RenameTracking/AbstractRenameTrackingCodeFixProvider.cs index 664e236798a0c..3411b0cd2db78 100644 --- a/src/EditorFeatures/Core/Implementation/RenameTracking/AbstractRenameTrackingCodeFixProvider.cs +++ b/src/EditorFeatures/Core/Implementation/RenameTracking/AbstractRenameTrackingCodeFixProvider.cs @@ -14,14 +14,11 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.RenameTracking internal abstract class AbstractRenameTrackingCodeFixProvider : CodeFixProvider { private readonly ITextUndoHistoryRegistry _undoHistoryRegistry; - private readonly IEnumerable _refactorNotifyServices; protected AbstractRenameTrackingCodeFixProvider( - ITextUndoHistoryRegistry undoHistoryRegistry, - IEnumerable refactorNotifyServices) + ITextUndoHistoryRegistry undoHistoryRegistry) { _undoHistoryRegistry = undoHistoryRegistry; - _refactorNotifyServices = refactorNotifyServices; } public sealed override ImmutableArray FixableDiagnosticIds @@ -40,7 +37,7 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) // any fixes. if (RenameTrackingTaggerProvider.CanInvokeRename(document)) { - var action = RenameTrackingTaggerProvider.CreateCodeAction(document, diagnostic, _refactorNotifyServices, _undoHistoryRegistry); + var action = RenameTrackingTaggerProvider.CreateCodeAction(document, diagnostic, _undoHistoryRegistry); context.RegisterCodeFix(action, diagnostic); } diff --git a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCodeAction.cs b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCodeAction.cs index b5cbd8358df5a..b596217fabad2 100644 --- a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCodeAction.cs +++ b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCodeAction.cs @@ -20,15 +20,13 @@ private class RenameTrackingCodeAction : CodeAction { private readonly string _title; private readonly Document _document; - private readonly IEnumerable _refactorNotifyServices; private readonly ITextUndoHistoryRegistry _undoHistoryRegistry; private RenameTrackingCommitter _renameTrackingCommitter; - public RenameTrackingCodeAction(Document document, string title, IEnumerable refactorNotifyServices, ITextUndoHistoryRegistry undoHistoryRegistry) + public RenameTrackingCodeAction(Document document, string title, ITextUndoHistoryRegistry undoHistoryRegistry) { _document = document; _title = title; - _refactorNotifyServices = refactorNotifyServices; _undoHistoryRegistry = undoHistoryRegistry; } @@ -84,7 +82,7 @@ private bool TryInitializeRenameTrackingCommitter(CancellationToken cancellation var snapshotSpan = stateMachine.TrackingSession.TrackingSpan.GetSpan(stateMachine.Buffer.CurrentSnapshot); var newName = snapshotSpan.GetText(); var displayText = string.Format(EditorFeaturesResources.Rename_0_to_1, stateMachine.TrackingSession.OriginalName, newName); - _renameTrackingCommitter = new RenameTrackingCommitter(stateMachine, snapshotSpan, _refactorNotifyServices, _undoHistoryRegistry, displayText); + _renameTrackingCommitter = new RenameTrackingCommitter(stateMachine, snapshotSpan, _undoHistoryRegistry, displayText); return true; } } diff --git a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs index 97b315beccab9..a7f4e201c522c 100644 --- a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs +++ b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs @@ -27,7 +27,6 @@ private class RenameTrackingCommitter : ForegroundThreadAffinitizedObject { private readonly StateMachine _stateMachine; private readonly SnapshotSpan _snapshotSpan; - private readonly IEnumerable _refactorNotifyServices; private readonly ITextUndoHistoryRegistry _undoHistoryRegistry; private readonly string _displayText; private readonly AsyncLazy _renameSymbolResultGetter; @@ -35,14 +34,12 @@ private class RenameTrackingCommitter : ForegroundThreadAffinitizedObject public RenameTrackingCommitter( StateMachine stateMachine, SnapshotSpan snapshotSpan, - IEnumerable refactorNotifyServices, ITextUndoHistoryRegistry undoHistoryRegistry, string displayText) : base(stateMachine.ThreadingContext) { _stateMachine = stateMachine; _snapshotSpan = snapshotSpan; - _refactorNotifyServices = refactorNotifyServices; _undoHistoryRegistry = undoHistoryRegistry; _displayText = displayText; _renameSymbolResultGetter = new AsyncLazy(c => RenameSymbolWorkerAsync(c), cacheResult: true); @@ -143,19 +140,6 @@ private bool ApplyChangesToWorkspace(CancellationToken cancellationToken) var trackingSessionId = _stateMachine.StoreCurrentTrackingSessionAndGenerateId(); UpdateWorkspaceForResetOfTypedIdentifier(workspace, renameTrackingSolutionSet.OriginalSolution, trackingSessionId); - // Now that the solution is back in its original state, notify third parties about - // the coming rename operation. - if (!_refactorNotifyServices.TryOnBeforeGlobalSymbolRenamed(workspace, changedDocuments, renameTrackingSolutionSet.Symbol, newName, throwOnFailure: false)) - { - var notificationService = workspace.Services.GetService(); - notificationService.SendNotification( - EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid, - EditorFeaturesResources.Rename_Symbol, - NotificationSeverity.Error); - - return true; - } - // move all changes to final solution based on the workspace's current solution, since the current solution // got updated when we reset it above. var finalSolution = workspace.CurrentSolution; @@ -274,15 +258,6 @@ private void UpdateWorkspaceForGlobalIdentifierRename( Contract.Fail("Rename Tracking could not update solution."); } - if (!_refactorNotifyServices.TryOnAfterGlobalSymbolRenamed(workspace, changedDocuments, symbol, newName, throwOnFailure: false)) - { - var notificationService = workspace.Services.GetService(); - notificationService.SendNotification( - EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated, - EditorFeaturesResources.Rename_Symbol, - NotificationSeverity.Information); - } - // Never resume tracking session on redo var undoPrimitiveAfter = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: false); localUndoTransaction.AddUndo(undoPrimitiveAfter); diff --git a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.Tagger.cs b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.Tagger.cs index 952b7e148d9e5..ebfd7f2926e0d 100644 --- a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.Tagger.cs +++ b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.Tagger.cs @@ -21,15 +21,13 @@ private class Tagger : ITagger, ITagger, IDisposab private readonly StateMachine _stateMachine; private readonly ITextUndoHistoryRegistry _undoHistoryRegistry; private readonly IWaitIndicator _waitIndicator; - private readonly IEnumerable _refactorNotifyServices; public event EventHandler TagsChanged = delegate { }; public Tagger( StateMachine stateMachine, ITextUndoHistoryRegistry undoHistoryRegistry, - IWaitIndicator waitIndicator, - IEnumerable refactorNotifyServices) + IWaitIndicator waitIndicator) { _stateMachine = stateMachine; _stateMachine.Connect(); @@ -37,7 +35,6 @@ public Tagger( _stateMachine.TrackingSessionCleared += StateMachine_TrackingSessionCleared; _undoHistoryRegistry = undoHistoryRegistry; _waitIndicator = waitIndicator; - _refactorNotifyServices = refactorNotifyServices; } private void StateMachine_TrackingSessionCleared(ITrackingSpan trackingSpanToClear) diff --git a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.cs b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.cs index 25bc6963e3bde..7b648c8bb1171 100644 --- a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.cs +++ b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.cs @@ -41,7 +41,6 @@ internal sealed partial class RenameTrackingTaggerProvider : ITaggerProvider private readonly IAsynchronousOperationListener _asyncListener; private readonly IWaitIndicator _waitIndicator; private readonly IInlineRenameService _inlineRenameService; - private readonly IEnumerable _refactorNotifyServices; private readonly IDiagnosticAnalyzerService _diagnosticAnalyzerService; [ImportingConstructor] @@ -51,14 +50,12 @@ public RenameTrackingTaggerProvider( IWaitIndicator waitIndicator, IInlineRenameService inlineRenameService, IDiagnosticAnalyzerService diagnosticAnalyzerService, - [ImportMany] IEnumerable refactorNotifyServices, IAsynchronousOperationListenerProvider listenerProvider) { _threadingContext = threadingContext; _undoHistoryRegistry = undoHistoryRegistry; _waitIndicator = waitIndicator; _inlineRenameService = inlineRenameService; - _refactorNotifyServices = refactorNotifyServices; _diagnosticAnalyzerService = diagnosticAnalyzerService; _asyncListener = listenerProvider.GetListener(FeatureAttribute.RenameTracking); } @@ -66,7 +63,7 @@ public RenameTrackingTaggerProvider( public ITagger CreateTagger(ITextBuffer buffer) where T : ITag { var stateMachine = buffer.Properties.GetOrCreateSingletonProperty(() => new StateMachine(_threadingContext, buffer, _inlineRenameService, _asyncListener, _diagnosticAnalyzerService)); - return new Tagger(stateMachine, _undoHistoryRegistry, _waitIndicator, _refactorNotifyServices) as ITagger; + return new Tagger(stateMachine, _undoHistoryRegistry, _waitIndicator) as ITagger; } internal static void ResetRenameTrackingState(Workspace workspace, DocumentId documentId) @@ -138,7 +135,6 @@ internal static Diagnostic TryGetDiagnostic(SyntaxTree tree, DiagnosticDescripto internal static CodeAction CreateCodeAction( Document document, Diagnostic diagnostic, - IEnumerable refactorNotifyServices, ITextUndoHistoryRegistry undoHistoryRegistry) { // This can run on a background thread. @@ -148,7 +144,7 @@ internal static CodeAction CreateCodeAction( diagnostic.Properties[RenameTrackingDiagnosticAnalyzer.RenameFromPropertyKey], diagnostic.Properties[RenameTrackingDiagnosticAnalyzer.RenameToPropertyKey]); - return new RenameTrackingCodeAction(document, message, refactorNotifyServices, undoHistoryRegistry); + return new RenameTrackingCodeAction(document, message, undoHistoryRegistry); } internal static bool IsRenamableIdentifier(Task isRenamableIdentifierTask, bool waitForResult, CancellationToken cancellationToken) diff --git a/src/EditorFeatures/Test/RenameTracking/RenameTrackingTaggerProviderTests.cs b/src/EditorFeatures/Test/RenameTracking/RenameTrackingTaggerProviderTests.cs index e5460954d20ad..8e81025f005c3 100644 --- a/src/EditorFeatures/Test/RenameTracking/RenameTrackingTaggerProviderTests.cs +++ b/src/EditorFeatures/Test/RenameTracking/RenameTrackingTaggerProviderTests.cs @@ -746,69 +746,6 @@ public Cats() await state.AssertNoTag(); } - [WpfFact] - [Trait(Traits.Feature, Traits.Features.RenameTracking)] - public async Task RenameTrackingHonorsThirdPartyRequestsForCancellationBeforeRename() - { - var code = @" -class Cat$$ -{ - public Cat() - { - } -}"; - using var state = RenameTrackingTestState.Create(code, LanguageNames.CSharp, onBeforeGlobalSymbolRenamedReturnValue: false); - state.EditorOperations.InsertText("s"); - await state.AssertTag("Cat", "Cats", invokeAction: true); - Assert.Equal(1, state.RefactorNotifyService.OnBeforeSymbolRenamedCount); - - // Make sure the rename didn't proceed - Assert.Equal(0, state.RefactorNotifyService.OnAfterSymbolRenamedCount); - await state.AssertNoTag(); - - var expectedCode = @" -class Cat -{ - public Cat() - { - } -}"; - Assert.Equal(expectedCode, state.HostDocument.GetTextBuffer().CurrentSnapshot.GetText()); - - state.AssertNotificationMessage(); - } - - [WpfFact] - [Trait(Traits.Feature, Traits.Features.RenameTracking)] - public async Task RenameTrackingAlertsAboutThirdPartyRequestsForCancellationAfterRename() - { - var code = @" -class Cat$$ -{ - public Cat() - { - } -}"; - using var state = RenameTrackingTestState.Create(code, LanguageNames.CSharp, onAfterGlobalSymbolRenamedReturnValue: false); - state.EditorOperations.InsertText("s"); - await state.AssertTag("Cat", "Cats", invokeAction: true); - - Assert.Equal(1, state.RefactorNotifyService.OnBeforeSymbolRenamedCount); - Assert.Equal(1, state.RefactorNotifyService.OnAfterSymbolRenamedCount); - state.AssertNotificationMessage(); - - // Make sure the rename completed - var expectedCode = @" -class Cats -{ - public Cats() - { - } -}"; - Assert.Equal(expectedCode, state.HostDocument.GetTextBuffer().CurrentSnapshot.GetText()); - await state.AssertNoTag(); - } - [WpfFact, WorkItem(530469, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/530469")] [Trait(Traits.Feature, Traits.Features.RenameTracking)] public async Task RenameTrackingNotWhenStartedFromTextualWordInTrivia() diff --git a/src/EditorFeatures/Test/RenameTracking/RenameTrackingTestState.cs b/src/EditorFeatures/Test/RenameTracking/RenameTrackingTestState.cs index 7148ba1fab3ed..b353e3144b594 100644 --- a/src/EditorFeatures/Test/RenameTracking/RenameTrackingTestState.cs +++ b/src/EditorFeatures/Test/RenameTracking/RenameTrackingTestState.cs @@ -86,7 +86,7 @@ public RenameTrackingTestState( _view.Caret.MoveTo(new SnapshotPoint(_view.TextSnapshot, HostDocument.CursorPosition.Value)); EditorOperations = Workspace.GetService().GetEditorOperations(_view); _historyRegistry = Workspace.ExportProvider.GetExport().Value; - RefactorNotifyService = new TestRefactorNotify(); + RefactorNotifyService = (TestRefactorNotify)Workspace.GetService(); RefactorNotifyService.OnBeforeRename += (_) => { @@ -109,22 +109,17 @@ public RenameTrackingTestState( Workspace.ExportProvider.GetExport().Value, Workspace.ExportProvider.GetExport().Value, Workspace.ExportProvider.GetExport().Value, - SpecializedCollections.SingletonEnumerable(RefactorNotifyService), Workspace.ExportProvider.GetExportedValue()); _tagger = tracker.CreateTagger(HostDocument.GetTextBuffer()); if (languageName == LanguageNames.CSharp) { - _codeFixProvider = new CSharpRenameTrackingCodeFixProvider( - _historyRegistry, - SpecializedCollections.SingletonEnumerable(RefactorNotifyService)); + _codeFixProvider = new CSharpRenameTrackingCodeFixProvider(_historyRegistry); } else if (languageName == LanguageNames.VisualBasic) { - _codeFixProvider = new VisualBasicRenameTrackingCodeFixProvider( - _historyRegistry, - SpecializedCollections.SingletonEnumerable(RefactorNotifyService)); + _codeFixProvider = new VisualBasicRenameTrackingCodeFixProvider(_historyRegistry); } else { diff --git a/src/EditorFeatures/Test/Utilities/EditorServicesUtil.cs b/src/EditorFeatures/Test/Utilities/EditorServicesUtil.cs index b2288cc5f8100..5a9dd0ae31ad1 100644 --- a/src/EditorFeatures/Test/Utilities/EditorServicesUtil.cs +++ b/src/EditorFeatures/Test/Utilities/EditorServicesUtil.cs @@ -4,6 +4,7 @@ using System; using Microsoft.CodeAnalysis.Test.Utilities; +using Microsoft.CodeAnalysis.Test.Utilities.Workspaces; using Microsoft.VisualStudio.Composition; namespace Microsoft.CodeAnalysis.Editor.UnitTests.Utilities @@ -17,7 +18,8 @@ public static class EditorServicesUtil private static IExportProviderFactory CreateExportProviderFactory() { var catalog = TestExportProvider.GetCSharpAndVisualBasicAssemblyCatalog() - .WithParts(ExportProviderCache.GetOrCreateAssemblyCatalog(new[] { typeof(EditorServicesUtil).Assembly }, ExportProviderCache.CreateResolver())); + .WithParts(ExportProviderCache.GetOrCreateAssemblyCatalog(new[] { typeof(EditorServicesUtil).Assembly }, ExportProviderCache.CreateResolver())) + .WithPart(typeof(TestRefactorNotify)); return ExportProviderCache.GetOrCreateExportProviderFactory(catalog); } } diff --git a/src/EditorFeatures/Test2/Rename/RenameTestHelpers.vb b/src/EditorFeatures/Test2/Rename/RenameTestHelpers.vb index dafb50c0cfd6a..8c9c7bda3c0c9 100644 --- a/src/EditorFeatures/Test2/Rename/RenameTestHelpers.vb +++ b/src/EditorFeatures/Test2/Rename/RenameTestHelpers.vb @@ -131,7 +131,6 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename workspace.ExportProvider.GetExport(Of IWaitIndicator)().Value, workspace.ExportProvider.GetExport(Of IInlineRenameService)().Value, workspace.ExportProvider.GetExport(Of IDiagnosticAnalyzerService)().Value, - {New TestRefactorNotify()}, workspace.ExportProvider.GetExportedValue(Of IAsynchronousOperationListenerProvider)) Return tracker.CreateTagger(Of RenameTrackingTag)(document.GetTextBuffer()) diff --git a/src/EditorFeatures/TestUtilities/Utilities/TestRenameSymbolAnnotation.cs b/src/EditorFeatures/TestUtilities/Utilities/TestRenameSymbolAnnotation.cs new file mode 100644 index 0000000000000..7b29147588f2a --- /dev/null +++ b/src/EditorFeatures/TestUtilities/Utilities/TestRenameSymbolAnnotation.cs @@ -0,0 +1,27 @@ +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.CodeAnalysis.Test.Utilities.Utilities +{ + internal static class TestRenameSymbolAnnotation + { + public static async Task ValidateRenameSymbolAnnotationsAsync(Solution originalSolution, Solution newSolution, ImmutableDictionary expectedSymbolChanges, CancellationToken cancellationToken = default) + { + var changes = newSolution.GetChanges(originalSolution); + var changedDocumentIds = changes.GetProjectChanges().SelectMany(p => p.GetChangedDocuments()); + + var changedSymbols = await Rename.RenameSymbolAnnotation.GatherChangedSymbolsInDocumentsAsync(changedDocumentIds, newSolution, originalSolution, cancellationToken); + + Assert.Equal(expectedSymbolChanges.Count(), changedSymbols.Length); + + foreach (var (originalSymbol, currentSymbol) in changedSymbols) + { + Assert.True(expectedSymbolChanges.ContainsKey(originalSymbol.Name)); + Assert.Equal(expectedSymbolChanges[originalSymbol.Name], currentSymbol.Name); + } + } + } +} diff --git a/src/EditorFeatures/VisualBasic/RenameTracking/RenameTrackingCodeFixProvider.vb b/src/EditorFeatures/VisualBasic/RenameTracking/RenameTrackingCodeFixProvider.vb index 0a968c12047e0..c56f86d73ff06 100644 --- a/src/EditorFeatures/VisualBasic/RenameTracking/RenameTrackingCodeFixProvider.vb +++ b/src/EditorFeatures/VisualBasic/RenameTracking/RenameTrackingCodeFixProvider.vb @@ -4,7 +4,6 @@ Imports System.Composition Imports Microsoft.CodeAnalysis.CodeFixes -Imports Microsoft.CodeAnalysis.Editor.Host Imports Microsoft.CodeAnalysis.Editor.Implementation.RenameTracking Imports Microsoft.VisualStudio.Text.Operations @@ -34,8 +33,8 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.RenameTracking Inherits AbstractRenameTrackingCodeFixProvider - Public Sub New(undoHistoryRegistry As ITextUndoHistoryRegistry, refactorNotifyServices As IEnumerable(Of IRefactorNotifyService)) - MyBase.New(undoHistoryRegistry, refactorNotifyServices) + Public Sub New(undoHistoryRegistry As ITextUndoHistoryRegistry) + MyBase.New(undoHistoryRegistry) End Sub End Class End Namespace diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs index 150f96dc08137..ce526c7bf6151 100644 --- a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs +++ b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioWorkspaceImpl.cs @@ -421,7 +421,7 @@ bool CanApplyChange(DocumentId documentId) } } - private void NotifyRefactorAfterChanges(ImmutableDictionary changedSymbols, List changedDocumentIds) + private void NotifyRefactorAfterChanges(ImmutableArray<(ISymbol originalSymbol, ISymbol newSymbol)> changedSymbols, List changedDocumentIds) { foreach (var (oldSymbol, newSymbol) in changedSymbols) { @@ -432,7 +432,7 @@ private void NotifyRefactorAfterChanges(ImmutableDictionary ch } } - private bool TryNotifyRefactorBeforeChanges(ImmutableDictionary changedSymbols, IEnumerable changedDocumentIds) + private bool TryNotifyRefactorBeforeChanges(ImmutableArray<(ISymbol originalSymbol, ISymbol newSymbol)> changedSymbols, IEnumerable changedDocumentIds) { foreach (var (oldSymbol, newSymbol) in changedSymbols) { diff --git a/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs b/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs index 82e77907f6cb1..f76c667b34aed 100644 --- a/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs +++ b/src/Workspaces/Core/Portable/Rename/RenameSymbolAnnotation.cs @@ -9,8 +9,9 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Shared.Extensions; +using Roslyn.Utilities; -#nullable enable +#nullable enable namespace Microsoft.CodeAnalysis.Rename { @@ -42,43 +43,36 @@ public static TNode WithRenameSymbolAnnotation(this TNode node, SemanticM where TNode : SyntaxNode => node.WithAdditionalAnnotations(Create(semanticModel.GetDeclaredSymbol(node))); - public static async Task> GatherChangedSymbolsInDocumentsAsync( + public static async Task> GatherChangedSymbolsInDocumentsAsync( IEnumerable changedDocumentIds, Solution newSolution, Solution oldSolution, CancellationToken cancellationToken) { - var changedSymbols = ImmutableDictionary.CreateBuilder(); + var changedSymbols = ImmutableArray.CreateBuilder<(ISymbol originalSymbol, ISymbol newSymbol)>(); - foreach (var changedDocId in changedDocumentIds) + foreach (var documentId in changedDocumentIds) { - var newDocument = newSolution.GetRequiredDocument(changedDocId); + var newDocument = newSolution.GetRequiredDocument(documentId); + var oldDocument = oldSolution.GetDocument(documentId); - // Documents without syntax tree won't have the annotations attached - if (!newDocument.SupportsSyntaxTree) + if (oldDocument is null) { continue; } - var syntaxRoot = await newDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var symbolRenameNodes = syntaxRoot.GetAnnotatedNodes(RenameSymbolKind); + var newSemanticModel = await newDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var oldSemanticModel = await oldDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - foreach (var node in symbolRenameNodes) + var renamedNodes = await GatherAnnotatedNodesAsync(newDocument, cancellationToken).ConfigureAwait(false); + + foreach (var node in renamedNodes) { foreach (var annotation in node.GetAnnotations(RenameSymbolKind)) { - var oldDocument = oldSolution.GetDocument(changedDocId); - if (oldDocument is null) - { - continue; - } - - var newSemanticModel = await newDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - var oldSemanticModel = await oldDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - var oldSymbol = ResolveSymbol(annotation, oldSemanticModel.Compilation); var newSymbol = newSemanticModel.GetDeclaredSymbol(node); - changedSymbols.Add(oldSymbol, newSymbol); + changedSymbols.Add((oldSymbol, newSymbol)); } } } @@ -86,6 +80,23 @@ public static async Task> GatherChangedSym return changedSymbols.ToImmutable(); } + public static async Task> GatherAnnotatedNodesAsync(Document document, CancellationToken cancellationToken) + { + if (!document.SupportsSyntaxTree) + { + return ImmutableArray.Create(); + } + + var changedSymbols = ImmutableArray.CreateBuilder(); + + var syntaxRoot = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var symbolRenameNodes = syntaxRoot.GetAnnotatedNodes(RenameSymbolKind); + + changedSymbols.AddRange(symbolRenameNodes); + + return changedSymbols.ToImmutable(); + } + internal static ISymbol ResolveSymbol(SyntaxAnnotation annotation, Compilation oldCompilation) { if (annotation.Kind != RenameSymbolKind) From 718ed6b4dc073ae4b9a4b43c96659c3956d93bcd Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Thu, 6 Feb 2020 18:19:20 -0800 Subject: [PATCH 7/9] Remove more refactorNotify code --- ...tainedLanguage.IVsContainedLanguageCodeSupport.cs | 2 +- .../Venus/ContainedLanguageCodeSupport.cs | 12 ------------ .../Venus/CSharpContainedLanguageSupportTests.vb | 5 ----- .../VisualBasicContainedLanguageSupportTests.vb | 5 ----- 4 files changed, 1 insertion(+), 23 deletions(-) diff --git a/src/VisualStudio/Core/Def/Implementation/Venus/ContainedLanguage.IVsContainedLanguageCodeSupport.cs b/src/VisualStudio/Core/Def/Implementation/Venus/ContainedLanguage.IVsContainedLanguageCodeSupport.cs index cfa42cdeaac48..31c9e663220ac 100644 --- a/src/VisualStudio/Core/Def/Implementation/Venus/ContainedLanguage.IVsContainedLanguageCodeSupport.cs +++ b/src/VisualStudio/Core/Def/Implementation/Venus/ContainedLanguage.IVsContainedLanguageCodeSupport.cs @@ -188,7 +188,7 @@ public int OnRenamed(ContainedLanguageRenameType clrt, string bstrOldID, string { var refactorNotifyServices = this.ComponentModel.DefaultExportProvider.GetExportedValues(); - if (!ContainedLanguageCodeSupport.TryRenameElement(GetThisDocument(), clrt, bstrOldID, bstrNewID, refactorNotifyServices, c.CancellationToken)) + if (!ContainedLanguageCodeSupport.TryRenameElement(GetThisDocument(), clrt, bstrOldID, bstrNewID, c.CancellationToken)) { result = s_CONTAINEDLANGUAGE_CANNOTFINDITEM; } diff --git a/src/VisualStudio/Core/Def/Implementation/Venus/ContainedLanguageCodeSupport.cs b/src/VisualStudio/Core/Def/Implementation/Venus/ContainedLanguageCodeSupport.cs index 0e775f3145a9c..9e04e2996a85e 100644 --- a/src/VisualStudio/Core/Def/Implementation/Venus/ContainedLanguageCodeSupport.cs +++ b/src/VisualStudio/Core/Def/Implementation/Venus/ContainedLanguageCodeSupport.cs @@ -11,14 +11,11 @@ using Microsoft.CodeAnalysis.CodeGeneration; using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Editor; -using Microsoft.CodeAnalysis.Editor.Shared.Extensions; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; using Microsoft.CodeAnalysis.Editor.Undo; using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.Formatting.Rules; -using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.LanguageServices; -using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Rename; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.Utilities; @@ -309,7 +306,6 @@ public static bool TryRenameElement( ContainedLanguageRenameType clrt, string oldFullyQualifiedName, string newFullyQualifiedName, - IEnumerable refactorNotifyServices, CancellationToken cancellationToken) { var symbol = FindSymbol(document, clrt, oldFullyQualifiedName, cancellationToken); @@ -328,19 +324,11 @@ public static bool TryRenameElement( var undoTitle = string.Format(EditorFeaturesResources.Rename_0_to_1, symbol.Name, newName); using (var workspaceUndoTransaction = workspace.OpenGlobalUndoTransaction(undoTitle)) { - // Notify third parties about the coming rename operation on the workspace, and let - // any exceptions propagate through - refactorNotifyServices.TryOnBeforeGlobalSymbolRenamed(workspace, changedDocuments, symbol, newName, throwOnFailure: true); - if (!workspace.TryApplyChanges(newSolution)) { Exceptions.ThrowEFail(); } - // Notify third parties about the completed rename operation on the workspace, and - // let any exceptions propagate through - refactorNotifyServices.TryOnAfterGlobalSymbolRenamed(workspace, changedDocuments, symbol, newName, throwOnFailure: true); - workspaceUndoTransaction.Commit(); } diff --git a/src/VisualStudio/Core/Test/Venus/CSharpContainedLanguageSupportTests.vb b/src/VisualStudio/Core/Test/Venus/CSharpContainedLanguageSupportTests.vb index 6b442ee870b45..21ffddae46252 100644 --- a/src/VisualStudio/Core/Test/Venus/CSharpContainedLanguageSupportTests.vb +++ b/src/VisualStudio/Core/Test/Venus/CSharpContainedLanguageSupportTests.vb @@ -833,7 +833,6 @@ public partial class _Default clrt:=ContainedLanguageRenameType.CLRT_CLASSMEMBER, oldFullyQualifiedName:="_Default.Page_Load", newFullyQualifiedName:="_Default.Page_Load1", - refactorNotifyServices:=SpecializedCollections.EmptyEnumerable(Of IRefactorNotifyService), cancellationToken:=Nothing) Assert.True(renameSucceeded) @@ -860,7 +859,6 @@ public partial class _Default clrt:=ContainedLanguageRenameType.CLRT_CLASSMEMBER, oldFullyQualifiedName:="_Default.Fictional", newFullyQualifiedName:="_Default.Fictional1", - refactorNotifyServices:=SpecializedCollections.EmptyEnumerable(Of IRefactorNotifyService), cancellationToken:=Nothing) Assert.False(renameSucceeded) @@ -878,7 +876,6 @@ public partial class _Default clrt:=ContainedLanguageRenameType.CLRT_CLASS, oldFullyQualifiedName:="Goo", newFullyQualifiedName:="Bar", - refactorNotifyServices:=SpecializedCollections.EmptyEnumerable(Of IRefactorNotifyService), cancellationToken:=Nothing) Assert.True(renameSucceeded) @@ -896,7 +893,6 @@ public partial class _Default clrt:=ContainedLanguageRenameType.CLRT_NAMESPACE, oldFullyQualifiedName:="Goo", newFullyQualifiedName:="Bar", - refactorNotifyServices:=SpecializedCollections.EmptyEnumerable(Of IRefactorNotifyService), cancellationToken:=Nothing) Assert.True(renameSucceeded) @@ -929,7 +925,6 @@ public class _Default clrt:=ContainedLanguageRenameType.CLRT_CLASSMEMBER, oldFullyQualifiedName:="_Default.button", newFullyQualifiedName:="_Default.button1", - refactorNotifyServices:=SpecializedCollections.EmptyEnumerable(Of IRefactorNotifyService), cancellationToken:=Nothing) Assert.True(renameSucceeded) diff --git a/src/VisualStudio/Core/Test/Venus/VisualBasicContainedLanguageSupportTests.vb b/src/VisualStudio/Core/Test/Venus/VisualBasicContainedLanguageSupportTests.vb index 862d3e62d7c32..418e6aced1a28 100644 --- a/src/VisualStudio/Core/Test/Venus/VisualBasicContainedLanguageSupportTests.vb +++ b/src/VisualStudio/Core/Test/Venus/VisualBasicContainedLanguageSupportTests.vb @@ -870,7 +870,6 @@ End Class.Value clrt:=ContainedLanguageRenameType.CLRT_CLASSMEMBER, oldFullyQualifiedName:="_Default.Page_Load", newFullyQualifiedName:="_Default.Page_Load1", - refactorNotifyServices:=SpecializedCollections.EmptyEnumerable(Of IRefactorNotifyService), cancellationToken:=Nothing) Assert.True(renameSucceeded) @@ -895,7 +894,6 @@ End Class.Value clrt:=ContainedLanguageRenameType.CLRT_CLASSMEMBER, oldFullyQualifiedName:="_Default.Fictional", newFullyQualifiedName:="_Default.Fictional1", - refactorNotifyServices:=SpecializedCollections.EmptyEnumerable(Of IRefactorNotifyService), cancellationToken:=Nothing) Assert.False(renameSucceeded) @@ -915,7 +913,6 @@ End Class.Value clrt:=ContainedLanguageRenameType.CLRT_CLASS, oldFullyQualifiedName:="Goo", newFullyQualifiedName:="Bar", - refactorNotifyServices:=SpecializedCollections.EmptyEnumerable(Of IRefactorNotifyService), cancellationToken:=Nothing) Assert.True(renameSucceeded) @@ -934,7 +931,6 @@ End Namespace.Value clrt:=ContainedLanguageRenameType.CLRT_NAMESPACE, oldFullyQualifiedName:="Goo", newFullyQualifiedName:="Bar", - refactorNotifyServices:=SpecializedCollections.EmptyEnumerable(Of IRefactorNotifyService), cancellationToken:=Nothing) Assert.True(renameSucceeded) @@ -964,7 +960,6 @@ End Class.Value clrt:=ContainedLanguageRenameType.CLRT_CLASSMEMBER, oldFullyQualifiedName:="_Default.button", newFullyQualifiedName:="_Default.button1", - refactorNotifyServices:=SpecializedCollections.EmptyEnumerable(Of IRefactorNotifyService), cancellationToken:=Nothing) Assert.True(renameSucceeded) From 3efc4d0ca0c2447106d659eb1fd7e83a70c6357f Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Fri, 7 Feb 2020 15:49:43 -0800 Subject: [PATCH 8/9] Fix duplicate annotation --- .../SyncNamespace/CSharpChangeNamespaceService.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpChangeNamespaceService.cs b/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpChangeNamespaceService.cs index fb5c51ef3cb99..70cdaa5627887 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpChangeNamespaceService.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpChangeNamespaceService.cs @@ -328,9 +328,6 @@ private static CompilationUnitSyntax MoveMembersFromGlobalToNamespace( SemanticModel semanticModel) { Debug.Assert(!compilationUnit.Members.Any(m => m is NamespaceDeclarationSyntax)); - compilationUnit = compilationUnit.ReplaceNodes( - compilationUnit.Members, - (original, current) => semanticModel.GetDeclaredSymbol(current) is null ? current : current.WithRenameSymbolAnnotation(semanticModel)); var targetNamespaceDecl = SyntaxFactory.NamespaceDeclaration( name: CreateNamespaceAsQualifiedName(targetNamespaceParts, aliasQualifier: null, targetNamespaceParts.Length - 1) From 2f1bcb4f2939bc76d9790cf33638ba876642a782 Mon Sep 17 00:00:00 2001 From: "Andrew Hall (METAL)" Date: Thu, 20 Feb 2020 13:29:27 -0800 Subject: [PATCH 9/9] Nullability fixes --- .../SyncNamespace/AbstractChangeNamespaceService.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractChangeNamespaceService.cs b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractChangeNamespaceService.cs index 6d5f406a9f3dc..ad2dd1c510dc0 100644 --- a/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractChangeNamespaceService.cs +++ b/src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractChangeNamespaceService.cs @@ -364,7 +364,7 @@ protected static bool IsSupportedLinkedDocument(Document document, out Immutable // 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.GetRequiredDocument(id).Project.FilePath, document.Project.FilePath))) + !PathUtilities.PathsEqual(solution.GetRequiredDocument(id).Project.FilePath!, document.Project.FilePath!))) { allDocumentIds = default; return false; @@ -473,7 +473,7 @@ private static SyntaxNode CreateImport(SyntaxGenerator syntaxGenerator, string n } else { - Debug.Assert(!PathUtilities.PathsEqual(refLocation.Document.FilePath, document.FilePath)); + Debug.Assert(!PathUtilities.PathsEqual(refLocation.Document.FilePath!, document.FilePath!)); refLocationsInOtherDocuments.Add(refLocation); } } @@ -863,9 +863,9 @@ private async Task AddImportsInContainersAsync( ? container.DescendantNodes().First() : container; - var compilation = await document.Project.GetCompilationAsync(cancellationToken).ConfigureAwait(false); - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var generator = document.GetLanguageService(); + var compilation = await document.Project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false); + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var generator = document.GetRequiredLanguageService(); root = addImportService.AddImports(compilation, root, contextLocation, imports, generator, placeSystemNamespaceFirst, cancellationToken); document = document.WithSyntaxRoot(root); }