Improve the locations where the Change-Signature refactoring is offered.

src/EditorFeatures/CSharpTest/CSharpEditorServicesTest.csproj

@@ -175,6 +175,7 @@
     <Compile Include="BraceHighlighting\BraceHighlightingTests.cs" />
     <Compile Include="BraceMatching\CSharpBraceMatcherTests.cs" />
     <Compile Include="CallHierarchy\CallHierarchyTests.cs" />
+    <Compile Include="ChangeSignature\ChangeSignatureTests.cs" />
     <Compile Include="ChangeSignature\ChangeSignature_CheckAllSignatureChanges.cs" />
     <Compile Include="ChangeSignature\ChangeSignature_Delegates.cs" />
     <Compile Include="ChangeSignature\ChangeSignature_Formatting.cs" />

src/EditorFeatures/CSharpTest/ChangeSignature/ChangeSignatureTests.cs

+// 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.Threading.Tasks;
+using Microsoft.CodeAnalysis.Editor.UnitTests.ChangeSignature;
+using Roslyn.Test.Utilities;
+using Xunit;
+
+namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.ChangeSignature
+{
+    public partial class ChangeSignatureTests : AbstractChangeSignatureTests
+    {
+        [WorkItem(8333, "https://github.com/dotnet/roslyn/issues/8333")]
+        [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
+        public async Task TestNotInExpressionBody()
+        {
+            var markup = @"
+class Ext
+{
+    void Foo(int a, int b) => [||]0;
+}";
+
+            await TestChangeSignatureViaCodeActionAsync(markup, expectedCodeAction: false);
+        }
+
+        [WorkItem(1905, "https://github.com/dotnet/roslyn/issues/1905")]
+        [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
+        public async Task TestAfterSemicolonForInvocationInExpressionStatement()
+        {
+            var markup = @"
+class Program
+{
+    static void Main(string[] args)
+    {
+        M1(1, 2);[||]
+        M2(1, 2, 3);
+    }
+
+    static void M1(int x, int y) { }
+
+    static void M2(int x, int y, int z) { }
+}";
+            var expectedCode = @"
+class Program
+{
+    static void Main(string[] args)
+    {
+        M1(2, 1);
+        M2(1, 2, 3);
+    }
+
+    static void M1(int y, int x) { }
+
+    static void M2(int x, int y, int z) { }
+}";
+
+            await TestChangeSignatureViaCodeActionAsync(
+                markup: markup, 
+                updatedSignature: new[] { 1, 0 },
+                expectedCode: expectedCode);
+        }
+
+        [WorkItem(17309, "https://github.com/dotnet/roslyn/issues/17309")]
+        [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
+        public async Task TestNotInLeadingWhitespace()
+        {
+            var markup = @"
+class Ext
+{
+    [||]
+    void Foo(int a, int b)
+    {
+    };
+}";
+
+            await TestChangeSignatureViaCodeActionAsync(markup, expectedCodeAction: false);
+        }
+
+        [WorkItem(17309, "https://github.com/dotnet/roslyn/issues/17309")]
+        [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
+        public async Task TestNotInLeadingTrivia()
+        {
+            var markup = @"
+class Ext
+{
+    // [||]
+    void Foo(int a, int b)
+    {
+    };
+}";
+
+            await TestChangeSignatureViaCodeActionAsync(markup, expectedCodeAction: false);
+        }
+
+        [WorkItem(17309, "https://github.com/dotnet/roslyn/issues/17309")]
+        [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
+        public async Task TestNotInLeadingTrivia2()
+        {
+            var markup = @"
+class Ext
+{
+    [||]//
+    void Foo(int a, int b)
+    {
+    };
+}";
+
+            await TestChangeSignatureViaCodeActionAsync(markup, expectedCodeAction: false);
+        }
+
+        [WorkItem(17309, "https://github.com/dotnet/roslyn/issues/17309")]
+        [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
+        public async Task TestNotInLeadingDocComment()
+        {
+            var markup = @"
+class Ext
+{
+    /// [||]
+    void Foo(int a, int b)
+    {
+    };
+}";
+
+            await TestChangeSignatureViaCodeActionAsync(markup, expectedCodeAction: false);
+        }
+
+        [WorkItem(17309, "https://github.com/dotnet/roslyn/issues/17309")]
+        [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
+        public async Task TestNotInLeadingDocComment2()
+        {
+            var markup = @"
+class Ext
+{
+    [||]///
+    void Foo(int a, int b)
+    {
+    };
+}";
+
+            await TestChangeSignatureViaCodeActionAsync(markup, expectedCodeAction: false);
+        }
+
+        [WorkItem(17309, "https://github.com/dotnet/roslyn/issues/17309")]
+        [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
+        public async Task TestNotInLeadingAttributes1()
+        {
+            var markup = @"
+class Ext
+{
+    [||][X]
+    void Foo(int a, int b)
+    {
+    };
+}";
+
+            await TestChangeSignatureViaCodeActionAsync(markup, expectedCodeAction: false);
+        }
+
+        [WorkItem(17309, "https://github.com/dotnet/roslyn/issues/17309")]
+        [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
+        public async Task TestNotInLeadingAttributes2()
+        {
+            var markup = @"
+class Ext
+{
+    [[||]X]
+    void Foo(int a, int b)
+    {
+    };
+}";
+
+            await TestChangeSignatureViaCodeActionAsync(markup, expectedCodeAction: false);
+        }
+
+        [WorkItem(17309, "https://github.com/dotnet/roslyn/issues/17309")]
+        [WpfFact, Trait(Traits.Feature, Traits.Features.ChangeSignature)]
+        public async Task TestNotInLeadingAttributes3()
+        {
+            var markup = @"
+class Ext
+{
+    [X][||]
+    void Foo(int a, int b)
+    {
+    };
+}";
+
+            await TestChangeSignatureViaCodeActionAsync(markup, expectedCodeAction: false);
+        }
+    }
+}
\ No newline at end of file

src/EditorFeatures/TestUtilities/ChangeSignature/AbstractChangeSignatureTests.cs

@@ -4,12 +4,9 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
-using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.ChangeSignature;
 using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions;
-using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
-using Microsoft.CodeAnalysis.Host;
 using Microsoft.CodeAnalysis.Notification;
 using Roslyn.Utilities;
 using Xunit;

src/Features/CSharp/Portable/ChangeSignature/CSharpChangeSignatureService.cs

@@ -22,25 +22,69 @@ namespace Microsoft.CodeAnalysis.CSharp.ChangeSignature
     [ExportLanguageService(typeof(AbstractChangeSignatureService), LanguageNames.CSharp), Shared]
     internal sealed class CSharpChangeSignatureService : AbstractChangeSignatureService
     {
+        private static readonly ImmutableArray<SyntaxKind> _invokableAncestorKinds = ImmutableArray.Create(
+                SyntaxKind.MethodDeclaration,
+                SyntaxKind.ConstructorDeclaration,
+                SyntaxKind.IndexerDeclaration,
+                SyntaxKind.InvocationExpression,
+                SyntaxKind.ElementAccessExpression,
+                SyntaxKind.ThisConstructorInitializer,
+                SyntaxKind.BaseConstructorInitializer,
+                SyntaxKind.ObjectCreationExpression,
+                SyntaxKind.Attribute,
+                SyntaxKind.NameMemberCref,
+                SyntaxKind.SimpleLambdaExpression,
+                SyntaxKind.ParenthesizedLambdaExpression,
+                SyntaxKind.DelegateDeclaration);
+
+        private static readonly ImmutableArray<SyntaxKind> _invokableAncestorInDeclarationKinds =
+            _invokableAncestorKinds.AddRange(
+                ImmutableArray.Create(SyntaxKind.Block, SyntaxKind.ArrowExpressionClause));
+
         public override async Task<ISymbol> GetInvocationSymbolAsync(
             Document document, int position, bool restrictToDeclarations, CancellationToken cancellationToken)
         {
             var tree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
-            var token = tree.GetRoot(cancellationToken).FindToken(position != tree.Length ? position : Math.Max(0, position - 1));
+            var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
+
+            var token = root.FindToken(position != tree.Length ? position : Math.Max(0, position - 1));
+
+            // Allow the user to invoke Change-Sig if they've written:   Foo(a, b, c);$$ 
+            if (token.Kind() == SyntaxKind.SemicolonToken && token.Parent is StatementSyntax)
+            {
+                token = token.GetPreviousToken();
+                position = token.Span.End;
+            }
+
+            var ancestorDeclarationKinds = restrictToDeclarations 
+                ? _invokableAncestorInDeclarationKinds
+                : _invokableAncestorKinds;
+
+            var matchingNode = token.Parent.AncestorsAndSelf().FirstOrDefault(n => ancestorDeclarationKinds.Contains(n.Kind()));
 
-            var ancestorDeclarationKinds = restrictToDeclarations ? _invokableAncestorKinds.Add(SyntaxKind.Block) : _invokableAncestorKinds;
-            SyntaxNode matchingNode = token.Parent.AncestorsAndSelf().FirstOrDefault(n => ancestorDeclarationKinds.Contains(n.Kind()));
-            if (matchingNode == null || matchingNode.IsKind(SyntaxKind.Block))
+            // If we walked up and we hit a block/expression-body, then we didn't find anything
+            // viable to reorder.  Just bail here.  This helps prevent Change-sig from appearing
+            // too aggressively inside method bodies.
+            if (matchingNode == null ||
+                matchingNode.IsKind(SyntaxKind.Block) ||
+                matchingNode.IsKind(SyntaxKind.ArrowExpressionClause))
             {
                 return null;
             }
 
-            ISymbol symbol;
-            var semanticModel = document.GetSemanticModelAsync(cancellationToken).WaitAndGetResult(cancellationToken);
-            symbol = semanticModel.GetDeclaredSymbol(matchingNode, cancellationToken);
+            // Don't show change-signature in the random whitespace/trivia for code.
+            if (!matchingNode.Span.IntersectsWith(position))
+            {
+                return null;
+            }
+
+            var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+            var symbol = semanticModel.GetDeclaredSymbol(matchingNode, cancellationToken);
             if (symbol != null)
             {
-                return symbol;
+                // If we're actually on the declaration of some symbol, ensure that we're
+                // in a good location for that symbol (i.e. not in the attributes/constraints).
+                return restrictToDeclarations && !InSymbolHeader(matchingNode, position) ? null : symbol;
             }
 
             if (matchingNode.IsKind(SyntaxKind.ObjectCreationExpression))
@@ -61,22 +105,28 @@ internal sealed class CSharpChangeSignatureService : AbstractChangeSignatureServ
             return symbolInfo.Symbol ?? symbolInfo.CandidateSymbols.FirstOrDefault();
         }
 
-        private ImmutableArray<SyntaxKind> _invokableAncestorKinds = new[]
+        private bool InSymbolHeader(SyntaxNode matchingNode, int position)
+        {
+            // Caret has to be after the attributes if the symbol has any.
+            var lastAttributes = matchingNode.ChildNodes().LastOrDefault(n => n is AttributeListSyntax);
+            var start = lastAttributes?.GetLastToken().GetNextToken().SpanStart ??
+                        matchingNode.SpanStart;
+
+            if (position < start)
             {
-                SyntaxKind.MethodDeclaration,
-                SyntaxKind.ConstructorDeclaration,
-                SyntaxKind.IndexerDeclaration,
-                SyntaxKind.InvocationExpression,
-                SyntaxKind.ElementAccessExpression,
-                SyntaxKind.ThisConstructorInitializer,
-                SyntaxKind.BaseConstructorInitializer,
-                SyntaxKind.ObjectCreationExpression,
-                SyntaxKind.Attribute,
-                SyntaxKind.NameMemberCref,
-                SyntaxKind.SimpleLambdaExpression,
-                SyntaxKind.ParenthesizedLambdaExpression,
-                SyntaxKind.DelegateDeclaration
-            }.ToImmutableArray();
+                return false;
+            }
+
+            // If the symbol has a parameter list, then the caret shouldn't be past the end of it.
+            var parameterList = matchingNode.ChildNodes().LastOrDefault(n => n is ParameterListSyntax);
+            if (parameterList != null)
+            {
+                return position <= parameterList.FullSpan.End;
+            }
+
+            // Case we haven't handled yet.  Just assume we're in the header.
+            return true;
+        }
 
         private ImmutableArray<SyntaxKind> _updatableAncestorKinds = new[]
             {

src/Features/Core/Portable/ChangeSignature/AbstractChangeSignatureService.cs

@@ -84,9 +84,11 @@ internal ChangeSignatureResult ChangeSignature(Document document, int position,
             }
         }
 
-        private async Task<ChangeSignatureAnalyzedContext> GetContextAsync(Document document, int position, bool restrictToDeclarations, CancellationToken cancellationToken)
+        private async Task<ChangeSignatureAnalyzedContext> GetContextAsync(
+            Document document, int position, bool restrictToDeclarations, CancellationToken cancellationToken)
         {
-            var symbol = await GetInvocationSymbolAsync(document, position, restrictToDeclarations, cancellationToken).ConfigureAwait(false);
+            var symbol = await GetInvocationSymbolAsync(
+                document, position, restrictToDeclarations, cancellationToken).ConfigureAwait(false);
 
             // Cross-lang symbols will show as metadata, so map it to source if possible.
             symbol = await SymbolFinder.FindSourceDefinitionAsync(symbol, document.Project.Solution, cancellationToken).ConfigureAwait(false) ?? symbol;