Add parameter CodeFixProvider: Add support for `this` and `base` constructor...

Add parameter CodeFixProvider: Add support for `this` and `base` constructor initializers invocations. (#29061)

* Add support for this and base constructor initializers invocations.

* Add WorkItem attribute to tests.

* Fix error comments on tests.

* Added tests for VB.

* Refactor RegisterCodeFixesAsync.

* Change visibility of some methods from protected to private and make some methods static.

* Remove Async prefix from synchronous methods.

* Simplify method signatures of "GetDataForFix_" methods.

* Incorporated code review.

* Return empty fixData to exit the loop if a known but unfixable node is found.

* Handle VBs Me.New() calls properly.

* Move method to make the PR diff easier to read.

* Unify removal of constructor candidates.

* Used named args for bool.

* Added copyright banner.

src/EditorFeatures/CSharpTest/AddParameter/AddParameterTests.cs

@@ -2407,5 +2407,76 @@ public class C {
 }";
             await TestMissingAsync(code);
         }
+
+        [WorkItem(29061, "https://github.com/dotnet/roslyn/issues/29061")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddParameter)]
+        public async Task TestThis_DontOfferToFixTheConstructorWithTheDiagnosticOnIt()
+        {
+            // error CS1729: 'C' does not contain a constructor that takes 1 arguments
+            var code =
+@"
+public class C {
+    
+    public C(): [|this|](1)
+    { }
+}";
+            await TestMissingAsync(code);
+        }
+
+        [WorkItem(29061, "https://github.com/dotnet/roslyn/issues/29061")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddParameter)]
+        public async Task TestThis_Fix_IfACandidateIsAvailable()
+        {
+            // error CS1729: 'C' does not contain a constructor that takes 2 arguments
+            var code =
+@"
+class C 
+{
+    public C(int i) { }
+    
+    public C(): [|this|](1, 1)
+    { }
+}";
+            var fix0 =
+@"
+class C 
+{
+    public C(int i, int v) { }
+    
+    public C(): this(1, 1)
+    { }
+}";
+            await TestInRegularAndScriptAsync(code, fix0, index: 0);
+            await TestActionCountAsync(code, 1);
+        }
+
+        [WorkItem(29061, "https://github.com/dotnet/roslyn/issues/29061")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddParameter)]
+        public async Task TestBase_Fix_IfACandidateIsAvailable()
+        {
+            // error CS1729: 'B' does not contain a constructor that takes 1 arguments
+            var code =
+@"
+public class B
+{
+    B() { }
+}
+public class C : B
+{
+    public C(int i) : [|base|](i) { }
+}";
+            var fix0 =
+@"
+public class B
+{
+    B(int i) { }
+}
+public class C : B
+{
+    public C(int i) : base(i) { }
+}";
+            await TestInRegularAndScriptAsync(code, fix0, index: 0);
+            await TestActionCountAsync(code, 1);
+        }
     }
 }

src/EditorFeatures/VisualBasicTest/AddParameter/AddParameterTests.vb

@@ -830,7 +830,7 @@ Public Class C
         M(new System.Exception(), 2)
     End Sub
 End Class"
-            Await TestInRegularAndScriptAsync(code, Fix)
+            Await TestInRegularAndScriptAsync(code, fix)
         End Function
 
         <WorkItem(21446, "https://github.com/dotnet/roslyn/issues/21446")>
@@ -1040,5 +1040,85 @@ End Class
 "
             Await TestMissingAsync(code)
         End Function
+
+        <WorkItem(29061, "https://github.com/dotnet/roslyn/issues/29061")>
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddParameter)>
+        Public Async Function TestConstructorInitializer_DontOfferFixForConstructorWithDiagnostic() As Task
+            ' Error BC30057: Too many arguments to 'Public Sub New()'.
+            Dim code =
+"
+Public Class C
+    Public Sub New()
+        Me.New([|1|])
+    End Sub
+End Class
+"
+            Await TestMissingAsync(code)
+        End Function
+
+        <WorkItem(29061, "https://github.com/dotnet/roslyn/issues/29061")>
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddParameter)>
+        Public Async Function TestConstructorInitializer_OfferFixForOtherConstructors() As Task
+            ' Error BC30516: Overload resolution failed because no accessible 'New' accepts this number of arguments.
+            Dim code =
+"
+Public Class C
+    Public Sub New(i As Integer)
+    End Sub
+    
+    Public Sub New()
+        Me.[|New|](1,1)
+    End Sub
+End Class
+"
+            Dim fix0 =
+"
+Public Class C
+    Public Sub New(i As Integer, v As Integer)
+    End Sub
+    
+    Public Sub New()
+        Me.New(1,1)
+    End Sub
+End Class
+"
+            Await TestInRegularAndScriptAsync(code, fix0, index:=0)
+            Await TestActionCountAsync(code, 1)
+        End Function
+
+        <WorkItem(29061, "https://github.com/dotnet/roslyn/issues/29061")>
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddParameter)>
+        Public Async Function TestConstructorInitializer_OfferFixForBaseConstrcutors() As Task
+            ' error BC30057: Too many arguments to 'Public Sub New()'.
+            Dim code =
+"
+Public Class B
+    Public Sub New()
+    End Sub
+End Class
+
+Public Class C
+    Inherits B
+    Public Sub New()
+        MyBase.New([|1|])
+    End Sub
+End Class
+"
+            Dim fix0 =
+"
+Public Class B
+    Public Sub New(v As Integer)
+    End Sub
+End Class
+
+Public Class C
+    Inherits B
+    Public Sub New()
+        MyBase.New(1)
+    End Sub
+End Class
+"
+            Await TestInRegularAndScriptAsync(code, fix0, index:=0)
+        End Function
     End Class
 End Namespace

src/Features/CSharp/Portable/AddParameter/CSharpAddParameterCodeFixProvider.cs

@@ -2,11 +2,14 @@
 
 using System.Collections.Immutable;
 using System.Composition;
+using System.Threading;
 using Microsoft.CodeAnalysis.AddParameter;
 using Microsoft.CodeAnalysis.CodeFixes;
 using Microsoft.CodeAnalysis.CSharp.GenerateConstructor;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.LanguageServices;
+using Microsoft.CodeAnalysis.Shared.Extensions;
 
 namespace Microsoft.CodeAnalysis.CSharp.AddParameter
 {
@@ -39,5 +42,34 @@ protected override ImmutableArray<string> TooManyArgumentsDiagnosticIds
 
         protected override ImmutableArray<string> CannotConvertDiagnosticIds
             => GenerateConstructorDiagnosticIds.CannotConvertDiagnosticIds;
+
+        protected override RegisterFixData<ArgumentSyntax> TryGetLanguageSpecificFixInfo(
+            SemanticModel semanticModel,
+            SyntaxNode node,
+            CancellationToken cancellationToken)
+        {
+            if (node is ConstructorInitializerSyntax constructorInitializer)
+            {
+                var constructorDeclaration = constructorInitializer.Parent;
+                if (semanticModel.GetDeclaredSymbol(constructorDeclaration, cancellationToken) is IMethodSymbol constructorSymbol)
+                {
+                    var type = constructorSymbol.ContainingType;
+                    if (constructorInitializer.IsKind(SyntaxKind.BaseConstructorInitializer))
+                    {
+                        // Search for fixable constructors in the base class.
+                        type = type?.BaseType;
+                    }
+
+                    if (type != null && type.IsFromSource())
+                    {
+                        var methodCandidates = type.InstanceConstructors;
+                        var arguments = constructorInitializer.ArgumentList.Arguments;
+                        return new RegisterFixData<ArgumentSyntax>(arguments, methodCandidates, isConstructorInitializer: true);
+                    }
+                }
+            }
+
+            return null;
+        }
     }
 }

src/Features/Core/Portable/AddParameter/AbstractAddParameterCodeFixProvider.cs

@@ -38,6 +38,12 @@ internal abstract class AbstractAddParameterCodeFixProvider<
         protected abstract ImmutableArray<string> TooManyArgumentsDiagnosticIds { get; }
         protected abstract ImmutableArray<string> CannotConvertDiagnosticIds { get; }
 
+        protected virtual RegisterFixData<TArgumentSyntax> TryGetLanguageSpecificFixInfo(
+            SemanticModel semanticModel,
+            SyntaxNode node,
+            CancellationToken cancellationToken)
+            => null;
+
         public override async Task RegisterCodeFixesAsync(CodeFixContext context)
         {
             var cancellationToken = context.CancellationToken;
@@ -47,19 +53,33 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
             var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
 
             var initialNode = root.FindNode(diagnostic.Location.SourceSpan);
+            var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+            var syntaxFacts = document.GetLanguageService<ISyntaxFactsService>();
 
             for (var node = initialNode; node != null; node = node.Parent)
             {
-                if (node is TObjectCreationExpressionSyntax objectCreation)
-                {
-                    var argumentOpt = TryGetRelevantArgument(initialNode, node, diagnostic);
-                    await HandleObjectCreationExpressionAsync(context, objectCreation, argumentOpt).ConfigureAwait(false);
-                    return;
-                }
-                else if (node is TInvocationExpressionSyntax invocationExpression)
+                var fixData =
+                    TryGetInvocationExpressionFixInfo(semanticModel, syntaxFacts, node, cancellationToken) ??
+                    TryGetObjectCreationFixInfo(semanticModel, syntaxFacts, node, cancellationToken) ??
+                    TryGetLanguageSpecificFixInfo(semanticModel, node, cancellationToken);
+
+                if (fixData != null)
                 {
+                    var candidates = fixData.MethodCandidates;
+                    if (fixData.IsConstructorInitializer)
+                    {
+                        // The invocation is a :this() or :base() call. In  the 'this' case we need to exclude the 
+                        // method with the diagnostic because otherwise we might introduce a call to itself (which is forbidden).
+                        if (semanticModel.GetEnclosingSymbol(node.SpanStart, cancellationToken) is IMethodSymbol methodWithDiagnostic)
+                        {
+                            candidates = candidates.Remove(methodWithDiagnostic);
+                        }
+                    }
+
                     var argumentOpt = TryGetRelevantArgument(initialNode, node, diagnostic);
-                    await HandleInvocationExpressionAsync(context, invocationExpression, argumentOpt).ConfigureAwait(false);
+                    var argumentInsertPositionInMethodCandidates = GetArgumentInsertPositionForMethodCandidates(
+                        argumentOpt, semanticModel, syntaxFacts, fixData.Arguments, candidates);
+                    RegisterFixForMethodOverloads(context, fixData.Arguments, argumentInsertPositionInMethodCandidates);
                     return;
                 }
             }
@@ -87,63 +107,65 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
                               .LastOrDefault(a => a.AncestorsAndSelf().Contains(node));
         }
 
-        private async Task HandleInvocationExpressionAsync(
-            CodeFixContext context, TInvocationExpressionSyntax invocationExpression, TArgumentSyntax argumentOpt)
+        private static RegisterFixData<TArgumentSyntax> TryGetInvocationExpressionFixInfo(
+            SemanticModel semanticModel,
+            ISyntaxFactsService syntaxFacts,
+            SyntaxNode node,
+            CancellationToken cancellationToken)
         {
-            var document = context.Document;
-            var cancellationToken = context.CancellationToken;
-            var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
-            var syntaxFacts = document.GetLanguageService<ISyntaxFactsService>();
+            if (node is TInvocationExpressionSyntax invocationExpression)
+            {
+                var expression = syntaxFacts.GetExpressionOfInvocationExpression(invocationExpression);
+                var candidates = semanticModel.GetMemberGroup(expression, cancellationToken).OfType<IMethodSymbol>().ToImmutableArray();
+                var arguments = (SeparatedSyntaxList<TArgumentSyntax>)syntaxFacts.GetArgumentsOfInvocationExpression(invocationExpression);
 
-            var expression = syntaxFacts.GetExpressionOfInvocationExpression(invocationExpression);
-            var candidates = semanticModel.GetMemberGroup(expression, cancellationToken).OfType<IMethodSymbol>().ToImmutableArray();
+                // In VB a constructor calls other constructor overloads via a Me.New(..) invocation.
+                // If the candidates are MethodKind.Constructor than these are the equivalent the a C# ConstructorInitializer.
+                var isConstructorInitializer = candidates.All(m => m.MethodKind == MethodKind.Constructor);
+                return new RegisterFixData<TArgumentSyntax>(arguments, candidates, isConstructorInitializer);
+            }
 
-            var arguments = (SeparatedSyntaxList<TArgumentSyntax>)syntaxFacts.GetArgumentsOfInvocationExpression(invocationExpression);
-            var argumentInsertPositionInMethodCandidates = GetArgumentInsertPositionForMethodCandidates(
-                argumentOpt, semanticModel, syntaxFacts, arguments, candidates);
-            RegisterFixForMethodOverloads(context, arguments, argumentInsertPositionInMethodCandidates);
+            return null;
         }
 
-        private async Task HandleObjectCreationExpressionAsync(
-            CodeFixContext context,
-            TObjectCreationExpressionSyntax objectCreation,
-            TArgumentSyntax argumentOpt)
+        private static RegisterFixData<TArgumentSyntax> TryGetObjectCreationFixInfo(
+            SemanticModel semanticModel,
+            ISyntaxFactsService syntaxFacts,
+            SyntaxNode node,
+            CancellationToken cancellationToken)
         {
-            var document = context.Document;
-            var cancellationToken = context.CancellationToken;
-            var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
-            var syntaxFacts = document.GetLanguageService<ISyntaxFactsService>();
-
-            // Not supported if this is "new { ... }" (as there are no parameters at all.
-            var typeNode = syntaxFacts.GetObjectCreationType(objectCreation);
-            if (typeNode == null)
+            if (node is TObjectCreationExpressionSyntax objectCreation)
             {
-                return;
-            }
 
-            // If we can't figure out the type being created, or the type isn't in source,
-            // then there's nothing we can do.
-            var type = semanticModel.GetSymbolInfo(typeNode, cancellationToken).GetAnySymbol() as INamedTypeSymbol;
-            if (type == null)
-            {
-                return;
-            }
+                // Not supported if this is "new { ... }" (as there are no parameters at all.
+                var typeNode = syntaxFacts.GetObjectCreationType(objectCreation);
+                if (typeNode == null)
+                {
+                    return new RegisterFixData<TArgumentSyntax>();
+                }
 
-            if (!type.IsNonImplicitAndFromSource())
-            {
-                return;
-            }
+                // If we can't figure out the type being created, or the type isn't in source,
+                // then there's nothing we can do.
+                if (!(semanticModel.GetSymbolInfo(typeNode, cancellationToken).GetAnySymbol() is INamedTypeSymbol type))
+                {
+                    return new RegisterFixData<TArgumentSyntax>();
+                }
+
+                if (!type.IsNonImplicitAndFromSource())
+                {
+                    return new RegisterFixData<TArgumentSyntax>();
+                }
 
-            var arguments = (SeparatedSyntaxList<TArgumentSyntax>)syntaxFacts.GetArgumentsOfObjectCreationExpression(objectCreation);
-            var methodCandidates = type.InstanceConstructors;
+                var arguments = (SeparatedSyntaxList<TArgumentSyntax>)syntaxFacts.GetArgumentsOfObjectCreationExpression(objectCreation);
+                var methodCandidates = type.InstanceConstructors;
 
-            var insertionData = GetArgumentInsertPositionForMethodCandidates(
-                argumentOpt, semanticModel, syntaxFacts, arguments, methodCandidates);
+                return new RegisterFixData<TArgumentSyntax>(arguments, methodCandidates, isConstructorInitializer: false);
+            }
 
-            RegisterFixForMethodOverloads(context, arguments, insertionData);
+            return null;
         }
 
-        private ImmutableArray<ArgumentInsertPositionData<TArgumentSyntax>> GetArgumentInsertPositionForMethodCandidates(
+        private static ImmutableArray<ArgumentInsertPositionData<TArgumentSyntax>> GetArgumentInsertPositionForMethodCandidates(
             TArgumentSyntax argumentOpt,
             SemanticModel semanticModel,
             ISyntaxFactsService syntaxFacts,
@@ -186,7 +208,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
             return methodsAndArgumentToAdd.ToImmutableAndFree();
         }
 
-        private int NonParamsParameterCount(IMethodSymbol method)
+        private static int NonParamsParameterCount(IMethodSymbol method)
             => method.IsParams() ? method.Parameters.Length - 1 : method.Parameters.Length;
 
         private void RegisterFixForMethodOverloads(
@@ -677,7 +699,7 @@ private static async Task<(ITypeSymbol, RefKind)> GetArgumentTypeAndRefKindAsync
                         parameterOptions: SymbolDisplayParameterOptions.IncludeParamsRefOut | SymbolDisplayParameterOptions.IncludeType,
                         miscellaneousOptions: SymbolDisplayMiscellaneousOptions.UseSpecialTypes);
 
-        private TArgumentSyntax DetermineFirstArgumentToAdd(
+        private static TArgumentSyntax DetermineFirstArgumentToAdd(
             SemanticModel semanticModel,
             ISyntaxFactsService syntaxFacts,
             StringComparer comparer,
@@ -759,7 +781,7 @@ private static async Task<(ITypeSymbol, RefKind)> GetArgumentTypeAndRefKindAsync
             return null;
         }
 
-        private bool TypeInfoMatchesWithParamsExpansion(
+        private static bool TypeInfoMatchesWithParamsExpansion(
             TypeInfo argumentTypeInfo, IParameterSymbol parameter,
             bool isNullLiteral, bool isDefaultLiteral)
         {
@@ -774,7 +796,7 @@ private static async Task<(ITypeSymbol, RefKind)> GetArgumentTypeAndRefKindAsync
             return false;
         }
 
-        private bool TypeInfoMatchesType(
+        private static bool TypeInfoMatchesType(
             TypeInfo argumentTypeInfo, ITypeSymbol type,
             bool isNullLiteral, bool isDefaultLiteral)
         {

src/Features/Core/Portable/AddParameter/RegisterFixData.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;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Text;
+
+namespace Microsoft.CodeAnalysis.AddParameter
+{
+    internal class RegisterFixData<TArgumentSyntax>
+        where TArgumentSyntax : SyntaxNode
+    {
+        public RegisterFixData() : this(new SeparatedSyntaxList<TArgumentSyntax>(), ImmutableArray<IMethodSymbol>.Empty, false)
+        {
+        }
+
+        public RegisterFixData(SeparatedSyntaxList<TArgumentSyntax> arguments, ImmutableArray<IMethodSymbol> methodCandidates, bool isConstructorInitializer)
+        {
+            Arguments = arguments;
+            MethodCandidates = methodCandidates;
+            IsConstructorInitializer = isConstructorInitializer;
+        }
+
+        public SeparatedSyntaxList<TArgumentSyntax> Arguments { get; }
+        public ImmutableArray<IMethodSymbol> MethodCandidates { get; }
+        public bool IsConstructorInitializer { get; }
+    }
+}