From 950bb0ae8ea4b1a591b54bfefdee4c5899363d50 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Sat, 8 Jul 2017 13:32:16 -0700 Subject: [PATCH] Reenable 'Move declaration near reference' refactoring. --- .../CSharp/Portable/CSharpFeatures.csproj | 1 - ...ferenceCodeRefactoringProvider.Rewriter.cs | 47 ------------- ...rReferenceCodeRefactoringProvider.State.cs | 26 ++++--- ...ionNearReferenceCodeRefactoringProvider.cs | 67 ++++++++++++------- 4 files changed, 59 insertions(+), 82 deletions(-) delete mode 100644 src/Features/CSharp/Portable/CodeRefactorings/MoveDeclarationNearReference/MoveDeclarationNearReferenceCodeRefactoringProvider.Rewriter.cs diff --git a/src/Features/CSharp/Portable/CSharpFeatures.csproj b/src/Features/CSharp/Portable/CSharpFeatures.csproj index 1771a0034f6..69913565478 100644 --- a/src/Features/CSharp/Portable/CSharpFeatures.csproj +++ b/src/Features/CSharp/Portable/CSharpFeatures.csproj @@ -163,7 +163,6 @@ - diff --git a/src/Features/CSharp/Portable/CodeRefactorings/MoveDeclarationNearReference/MoveDeclarationNearReferenceCodeRefactoringProvider.Rewriter.cs b/src/Features/CSharp/Portable/CodeRefactorings/MoveDeclarationNearReference/MoveDeclarationNearReferenceCodeRefactoringProvider.Rewriter.cs deleted file mode 100644 index 20d0141bf5b..00000000000 --- a/src/Features/CSharp/Portable/CodeRefactorings/MoveDeclarationNearReference/MoveDeclarationNearReferenceCodeRefactoringProvider.Rewriter.cs +++ /dev/null @@ -1,47 +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.Linq; -using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Formatting; - -namespace Microsoft.CodeAnalysis.CSharp.CodeRefactorings.MoveDeclarationNearReference -{ - internal partial class MoveDeclarationNearReferenceCodeRefactoringProvider - { - private class Rewriter : CSharpSyntaxRewriter - { - private readonly BlockSyntax _oldInnermostBlock; - private readonly BlockSyntax _newInnermostBlock; - private readonly BlockSyntax _oldOutermostBlock; - private readonly LocalDeclarationStatementSyntax _declarationStatement; - - public Rewriter( - BlockSyntax oldInnermostBlock, - BlockSyntax newInnermostBlock, - BlockSyntax oldOutermostBlock, - LocalDeclarationStatementSyntax declarationStatement) - { - _oldInnermostBlock = oldInnermostBlock; - _newInnermostBlock = newInnermostBlock; - _oldOutermostBlock = oldOutermostBlock; - _declarationStatement = declarationStatement; - } - - public override SyntaxNode VisitBlock(BlockSyntax oldBlock) - { - if (oldBlock == _oldInnermostBlock) - { - return _newInnermostBlock; - } - - if (oldBlock == _oldOutermostBlock) - { - var statements = SyntaxFactory.List(oldBlock.Statements.Where(s => s != _declarationStatement).Select(this.Visit)); - return oldBlock.WithStatements(statements).WithAdditionalAnnotations(Formatter.Annotation); - } - - return base.VisitBlock(oldBlock); - } - } - } -} diff --git a/src/Features/CSharp/Portable/CodeRefactorings/MoveDeclarationNearReference/MoveDeclarationNearReferenceCodeRefactoringProvider.State.cs b/src/Features/CSharp/Portable/CodeRefactorings/MoveDeclarationNearReference/MoveDeclarationNearReferenceCodeRefactoringProvider.State.cs index 33f7d972bf4..e2c78300d88 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/MoveDeclarationNearReference/MoveDeclarationNearReferenceCodeRefactoringProvider.State.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/MoveDeclarationNearReference/MoveDeclarationNearReferenceCodeRefactoringProvider.State.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.FindSymbols; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.CodeRefactorings.MoveDeclarationNearReference { @@ -25,7 +26,7 @@ private class State public StatementSyntax FirstStatementAffectedInInnermostBlock { get; private set; } internal static async Task GenerateAsync( - SemanticDocument document, + Document document, LocalDeclarationStatementSyntax statement, CancellationToken cancellationToken) { @@ -39,7 +40,7 @@ private class State } private async Task TryInitializeAsync( - SemanticDocument document, + Document document, LocalDeclarationStatementSyntax node, CancellationToken cancellationToken) { @@ -49,16 +50,21 @@ private class State } this.DeclarationStatement = node; - if (!(this.DeclarationStatement.IsParentKind(SyntaxKind.Block) && - this.DeclarationStatement.Declaration.Variables.Count == 1)) + if (!this.DeclarationStatement.IsParentKind(SyntaxKind.Block)) { return false; } this.VariableDeclaration = this.DeclarationStatement.Declaration; this.VariableDeclarator = this.VariableDeclaration.Variables[0]; - this.OutermostBlock = (BlockSyntax)this.DeclarationStatement.Parent; - this.LocalSymbol = (ILocalSymbol)document.SemanticModel.GetDeclaredSymbol(this.VariableDeclarator, cancellationToken); + this.OutermostBlock = this.DeclarationStatement.Parent as BlockSyntax; + if (OutermostBlock == null) + { + return false; + } + + var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + this.LocalSymbol = (ILocalSymbol)semanticModel.GetDeclaredSymbol(this.VariableDeclarator, cancellationToken); if (this.LocalSymbol == null) { // This can happen in broken code, for example: "{ object x; object }" @@ -78,13 +84,13 @@ private class State return false; } - var syntaxTree = document.SyntaxTree; + var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var referencingStatements = (from r in references - let token = document.Root.FindToken(r.Location.SourceSpan.Start) + let token = syntaxRoot.FindToken(r.Location.SourceSpan.Start) let statement = token.GetAncestor() where statement != null - select statement).ToList(); + select statement).ToSet(); if (referencingStatements.Count == 0) { @@ -133,7 +139,7 @@ private class State ? this.InnermostBlock.OpenBraceToken : (SyntaxNodeOrToken)this.InnermostBlock.Statements[firstStatementIndexAffectedInBlock - 1]; var affectedSpan = TextSpan.FromBounds(previousNodeOrToken.SpanStart, FirstStatementAffectedInInnermostBlock.Span.End); - if (syntaxTree.OverlapsHiddenPosition(affectedSpan, cancellationToken)) + if (semanticModel.SyntaxTree.OverlapsHiddenPosition(affectedSpan, cancellationToken)) { return false; } diff --git a/src/Features/CSharp/Portable/CodeRefactorings/MoveDeclarationNearReference/MoveDeclarationNearReferenceCodeRefactoringProvider.cs b/src/Features/CSharp/Portable/CodeRefactorings/MoveDeclarationNearReference/MoveDeclarationNearReferenceCodeRefactoringProvider.cs index 55cd0b45414..88e7a320453 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/MoveDeclarationNearReference/MoveDeclarationNearReferenceCodeRefactoringProvider.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/MoveDeclarationNearReference/MoveDeclarationNearReferenceCodeRefactoringProvider.cs @@ -1,19 +1,23 @@ // 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.Composition; using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CodeRefactorings; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Simplification; namespace Microsoft.CodeAnalysis.CSharp.CodeRefactorings.MoveDeclarationNearReference { - // [ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = PredefinedCodeRefactoringProviderNames.MoveDeclarationNearReference)] + [ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = PredefinedCodeRefactoringProviderNames.MoveDeclarationNearReference), Shared] + [ExtensionOrder(After = PredefinedCodeFixProviderNames.AddImport)] internal partial class MoveDeclarationNearReferenceCodeRefactoringProvider : CodeRefactoringProvider { public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context) @@ -37,37 +41,51 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte return; } + var position = textSpan.Start; var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var statement = root.FindToken(textSpan.Start).GetAncestor(); - if (statement == null || !statement.Span.IntersectsWith(textSpan.Start)) + var statement = root.FindToken(position).GetAncestor(); + if (statement == null) { return; } - var semanticDocument = await SemanticDocument.CreateAsync(document, cancellationToken).ConfigureAwait(false); - var state = await State.GenerateAsync(semanticDocument, statement, cancellationToken).ConfigureAwait(false); + if (statement.Declaration.Variables.Count != 1) + { + return; + } + + // Only offer the refactoring when somewhere in the type+name of the local variable. + if (position < statement.SpanStart || position > statement.Declaration.Variables[0].Identifier.Span.End) + { + return; + } + + var state = await State.GenerateAsync(document, statement, cancellationToken).ConfigureAwait(false); if (state == null) { return; } context.RegisterRefactoring( - new MyCodeAction( - CSharpFeaturesResources.Move_declaration_near_reference, - (c) => MoveDeclarationNearReferenceAsync(document, state, c))); + new MyCodeAction(c => MoveDeclarationNearReferenceAsync(document, state, root, c))); } - private async Task MoveDeclarationNearReferenceAsync(Document document, State state, CancellationToken cancellationToken) + private async Task MoveDeclarationNearReferenceAsync( + Document document, State state, SyntaxNode root, CancellationToken cancellationToken) { - var innermostStatements = - state.InnermostBlock.Statements.Where(s => s != state.DeclarationStatement).ToList(); - var innermostAffectedIndex = innermostStatements.IndexOf(state.FirstStatementAffectedInInnermostBlock); + var editor = new SyntaxEditor(root, document.Project.Solution.Workspace); + + //var innermostStatements = + // state.InnermostBlock.Statements.Where(s => s != state.DeclarationStatement).ToList(); + //var innermostAffectedIndex = innermostStatements.IndexOf(state.FirstStatementAffectedInInnermostBlock); var crossesMeaningfulBlock = CrossesMeaningfulBlock(state); var warningAnnotation = crossesMeaningfulBlock ? WarningAnnotation.Create(CSharpFeaturesResources.Warning_colon_Declaration_changes_scope_and_may_change_meaning) : null; + editor.RemoveNode(state.DeclarationStatement); + var canMergeDeclarationAndAssignment = await CanMergeDeclarationAndAssignmentAsync(document, state, cancellationToken).ConfigureAwait(false); if (canMergeDeclarationAndAssignment) { @@ -77,29 +95,28 @@ private async Task MoveDeclarationNearReferenceAsync(Document document ? declarationStatement : declarationStatement.WithAdditionalAnnotations(warningAnnotation); - innermostStatements[innermostAffectedIndex] = declarationStatement.WithAdditionalAnnotations(Formatter.Annotation); + editor.ReplaceNode( + state.FirstStatementAffectedInInnermostBlock, + declarationStatement.WithAdditionalAnnotations(Formatter.Annotation)); } else { // If we're not merging with an existing declaration, make the declaration semantically // explicit to improve the chances that it won't break code. - var explicitDeclarationStatement = await Simplifier.ExpandAsync(state.DeclarationStatement, document, cancellationToken: cancellationToken).ConfigureAwait(false); + var explicitDeclarationStatement = await Simplifier.ExpandAsync( + state.DeclarationStatement, document, cancellationToken: cancellationToken).ConfigureAwait(false); // place the declaration above the first statement that references it. var declarationStatement = warningAnnotation == null ? explicitDeclarationStatement : explicitDeclarationStatement.WithAdditionalAnnotations(warningAnnotation); - innermostStatements.Insert(innermostAffectedIndex, declarationStatement.WithAdditionalAnnotations(Formatter.Annotation)); + editor.InsertBefore( + state.FirstStatementAffectedInInnermostBlock, + declarationStatement.WithAdditionalAnnotations(Formatter.Annotation)); } - var newInnermostBlock = state.InnermostBlock.WithStatements( - SyntaxFactory.List(innermostStatements)).WithAdditionalAnnotations(Formatter.Annotation); - - var tree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false); - var rewriter = new Rewriter(state.InnermostBlock, newInnermostBlock, state.OutermostBlock, state.DeclarationStatement); - var newRoot = rewriter.Visit(tree.GetRoot(cancellationToken)); - + var newRoot = editor.GetChangedRoot(); return document.WithSyntaxRoot(newRoot); } @@ -185,10 +202,12 @@ private StatementSyntax CreateMergedDeclarationStatement(State state, StatementS private class MyCodeAction : CodeAction.DocumentChangeAction { - public MyCodeAction(string title, Func> createChangedDocument) : - base(title, createChangedDocument) + public MyCodeAction(Func> createChangedDocument) : + base(CSharpFeaturesResources.Move_declaration_near_reference, createChangedDocument) { } + + internal override CodeActionPriority Priority => CodeActionPriority.Low; } } } -- GitLab