Merge pull request #5681 from CyrusNajmabadi/generateConstructorInDerivedType

Generate constructor in derived type

Fixes #5274

src/EditorFeatures/CSharpTest/Diagnostics/GenerateConstructor/GenerateConstructorTests.cs

@@ -636,5 +636,62 @@ public void TestGenerateConstructorInIncompleteLambda()
     @"using System . Threading . Tasks ; class C { private int v ; public C ( int v ) { this . v = v ; } C ( ) { Task . Run ( ( ) => { new C ( 0 ) } ) ; } } ");
             }
         }
+
+        [WorkItem(5274, "https://github.com/dotnet/roslyn/issues/5274")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsGenerateMethod)]
+        public void TestGenerateIntoDerivedClassWithAbstractBase()
+        {
+            Test(
+@"
+class Class1
+{
+    private void Foo(string value)
+    {
+        var rewriter = new [|Derived|](value);
+    }
+
+    private class Derived : Base
+    {
+    }
+
+    public abstract partial class Base
+    {
+        private readonly bool _val;
+
+        public Base(bool val = false)
+        {
+            _val = val;
+        }
+    }
+}",
+@"
+class Class1
+{
+    private void Foo(string value)
+    {
+        var rewriter = new Derived(value);
+    }
+
+    private class Derived : Base
+    {
+        private string value;
+
+        public Derived(string value)
+        {
+            this.value = value;
+        }
+    }
+
+    public abstract partial class Base
+    {
+        private readonly bool _val;
+
+        public Base(bool val = false)
+        {
+            _val = val;
+        }
+    }
+}");
+        }
     }
 }

src/Features/CSharp/Portable/GenerateMember/GenerateConstructor/CSharpGenerateConstructorService.cs

@@ -9,6 +9,7 @@
 using Microsoft.CodeAnalysis.CSharp.Utilities;
 using Microsoft.CodeAnalysis.GenerateMember.GenerateConstructor;
 using Microsoft.CodeAnalysis.Host.Mef;
+using Microsoft.CodeAnalysis.LanguageServices;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 
 namespace Microsoft.CodeAnalysis.CSharp.GenerateMember.GenerateConstructor
@@ -178,7 +179,13 @@ protected override bool IsConversionImplicit(Compilation compilation, ITypeSymbo
             return compilation.ClassifyConversion(sourceType, targetType).IsImplicit;
         }
 
-        internal override IMethodSymbol GetDelegatingConstructor(State state, SemanticDocument document, int argumentCount, INamedTypeSymbol namedType, ISet<IMethodSymbol> candidates, CancellationToken cancellationToken)
+        internal override IMethodSymbol GetDelegatingConstructor(
+            State state,
+            SemanticDocument document, 
+            int argumentCount, 
+            INamedTypeSymbol namedType, 
+            ISet<IMethodSymbol> candidates, 
+            CancellationToken cancellationToken)
         {
             var oldToken = state.Token;
             var tokenKind = oldToken.Kind();
@@ -207,7 +214,8 @@ internal override IMethodSymbol GetDelegatingConstructor(State state, SemanticDo
                 if (document.SemanticModel.TryGetSpeculativeSemanticModel(ctorInitializer.Span.Start, newCtorInitializer, out speculativeModel))
                 {
                     var symbolInfo = speculativeModel.GetSymbolInfo(newCtorInitializer, cancellationToken);
-                    return GenerateConstructorHelpers.GetDelegatingConstructor(symbolInfo, candidates, namedType);
+                    return GenerateConstructorHelpers.GetDelegatingConstructor(
+                        document, symbolInfo, candidates, namedType, state.ParameterTypes);
                 }
             }
             else
@@ -257,7 +265,8 @@ internal override IMethodSymbol GetDelegatingConstructor(State state, SemanticDo
                 if (speculativeModel != null)
                 {
                     var symbolInfo = speculativeModel.GetSymbolInfo(newTypeName.Parent, cancellationToken);
-                    return GenerateConstructorHelpers.GetDelegatingConstructor(symbolInfo, candidates, namedType);
+                    return GenerateConstructorHelpers.GetDelegatingConstructor(
+                        document, symbolInfo, candidates, namedType, state.ParameterTypes);
                 }
             }
 

src/Features/CSharp/Portable/GenerateType/CSharpGenerateTypeService.cs

@@ -917,8 +917,15 @@ private IPropertySymbol CreatePropertySymbol(SimpleNameSyntax propertyName, ITyp
             return true;
         }
 
-        internal override IMethodSymbol GetDelegatingConstructor(ObjectCreationExpressionSyntax objectCreation, INamedTypeSymbol namedType, SemanticModel model, ISet<IMethodSymbol> candidates, CancellationToken cancellationToken)
+        internal override IMethodSymbol GetDelegatingConstructor(
+            SemanticDocument document,
+            ObjectCreationExpressionSyntax objectCreation,
+            INamedTypeSymbol namedType,
+            ISet<IMethodSymbol> candidates,
+            CancellationToken cancellationToken)
         {
+            var model = document.SemanticModel;
+
             var oldNode = objectCreation
                     .AncestorsAndSelf(ascendOutOfTrivia: false)
                     .Where(node => SpeculationAnalyzer.CanSpeculateOnNode(node))
@@ -934,7 +941,11 @@ internal override IMethodSymbol GetDelegatingConstructor(ObjectCreationExpressio
             {
                 newObjectCreation = (ObjectCreationExpressionSyntax)newNode.GetAnnotatedNodes(s_annotation).Single();
                 var symbolInfo = speculativeModel.GetSymbolInfo(newObjectCreation, cancellationToken);
-                return GenerateConstructorHelpers.GetDelegatingConstructor(symbolInfo, candidates, namedType);
+                var parameterTypes = newObjectCreation.ArgumentList.Arguments.Select(
+                    a => speculativeModel.GetTypeInfo(a.Expression, cancellationToken).ConvertedType).ToList();
+
+                return GenerateConstructorHelpers.GetDelegatingConstructor(
+                    document, symbolInfo, candidates, namedType, parameterTypes);
             }
 
             return null;

src/Features/Core/Portable/GenerateMember/GenerateConstructor/GenerateConstructorHelpers.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.Linq;
+using Microsoft.CodeAnalysis.LanguageServices;
+using Microsoft.CodeAnalysis.Shared.Extensions;
 
 namespace Microsoft.CodeAnalysis.GenerateMember.GenerateConstructor
 {
     internal static class GenerateConstructorHelpers
     {
-        public static IMethodSymbol GetDelegatingConstructor(SymbolInfo symbolInfo, ISet<IMethodSymbol> candidateInstanceConstructors, INamedTypeSymbol containingType)
+        public static IMethodSymbol GetDelegatingConstructor(
+            SemanticDocument document,
+            SymbolInfo symbolInfo,
+            ISet<IMethodSymbol> candidateInstanceConstructors,
+            INamedTypeSymbol containingType,
+            IList<ITypeSymbol> parameterTypes)
         {
             var symbol = symbolInfo.Symbol as IMethodSymbol;
             if (symbol == null && symbolInfo.CandidateSymbols.Length == 1)
             {
-                // Even though the symbol info has a non-viable candidate symbol, we are trying to speculate a base constructor
-                // invocation from a different position then where the invocation to it would be generated.
-                // Passed in candidateInstanceConstructors actually represent all accessible and invocable constructor symbols.
-                // So, we allow candidate symbol for inaccessible OR not creatable candidate reason if it is in the given candidateInstanceConstructors.
+                // Even though the symbol info has a non-viable candidate symbol, we are trying 
+                // to speculate a base constructor invocation from a different position then 
+                // where the invocation to it would be generated. Passed in candidateInstanceConstructors 
+                // actually represent all accessible and invocable constructor symbols. So, we allow 
+                // candidate symbol for inaccessible OR not creatable candidate reason if it is in 
+                // the given candidateInstanceConstructors.
+                //
+                // Note: if we get either of these cases, we ensure that we can at least convert 
+                // the parameter types we have to the constructor parameter types.  This way we
+                // don't accidently think we delegate to a constructor in an abstract base class
+                // when the parameter types don't match.
                 if (symbolInfo.CandidateReason == CandidateReason.Inaccessible ||
                     (symbolInfo.CandidateReason == CandidateReason.NotCreatable && containingType.IsAbstract))
                 {
-                    symbol = symbolInfo.CandidateSymbols.Single() as IMethodSymbol;
+                    var method = symbolInfo.CandidateSymbols.Single() as IMethodSymbol;
+                    if (ParameterTypesMatch(document, parameterTypes, method))
+                    {
+                        symbol = method;
+                    }
                 }
             }
 
@@ -30,5 +49,37 @@ public static IMethodSymbol GetDelegatingConstructor(SymbolInfo symbolInfo, ISet
 
             return null;
         }
+
+        private static bool ParameterTypesMatch(SemanticDocument document, IList<ITypeSymbol> parameterTypes, IMethodSymbol method)
+        {
+            if (method == null)
+            {
+                return false;
+            }
+
+            if (parameterTypes.Count < method.Parameters.Length)
+            {
+                return false;
+            }
+
+            var compilation = document.SemanticModel.Compilation;
+            var semanticFactsService = document.Document.GetLanguageService<ISemanticFactsService>();
+
+            for (var i = 0; i < parameterTypes.Count; i++)
+            {
+                var type1 = parameterTypes[i];
+                if (type1 != null)
+                {
+                    var type2 = method.Parameters[i].Type;
+
+                    if (!semanticFactsService.IsAssignableTo(type1, type2, compilation))
+                    {
+                        return false;
+                    }
+                }
+            }
+
+            return true;
+        }
     }
 }

src/Features/Core/Portable/GenerateType/AbstractGenerateTypeService.GenerateNamedType.cs

@@ -14,7 +14,12 @@ namespace Microsoft.CodeAnalysis.GenerateType
 {
     internal abstract partial class AbstractGenerateTypeService<TService, TSimpleNameSyntax, TObjectCreationExpressionSyntax, TExpressionSyntax, TTypeDeclarationSyntax, TArgumentSyntax>
     {
-        internal abstract IMethodSymbol GetDelegatingConstructor(TObjectCreationExpressionSyntax objectCreation, INamedTypeSymbol namedType, SemanticModel model, ISet<IMethodSymbol> candidates, CancellationToken cancellationToken);
+        internal abstract IMethodSymbol GetDelegatingConstructor(
+            SemanticDocument document,
+            TObjectCreationExpressionSyntax objectCreation,
+            INamedTypeSymbol namedType,
+            ISet<IMethodSymbol> candidates, 
+            CancellationToken cancellationToken);
 
         private partial class Editor
         {
@@ -145,7 +150,12 @@ private void AddMembers(IList<ISymbol> members, GenerateTypeOptionsResult option
 
                     if (accessibleInstanceConstructors.Any())
                     {
-                        var delegatedConstructor = _service.GetDelegatingConstructor(_state.ObjectCreationExpressionOpt, _state.BaseTypeOrInterfaceOpt, _document.SemanticModel, accessibleInstanceConstructors, _cancellationToken);
+                        var delegatedConstructor = _service.GetDelegatingConstructor( 
+                            _document,
+                            _state.ObjectCreationExpressionOpt,
+                            _state.BaseTypeOrInterfaceOpt,
+                            accessibleInstanceConstructors,
+                            _cancellationToken);
                         if (delegatedConstructor != null)
                         {
                             // There was a best match.  Call it directly.  

src/Features/VisualBasic/Portable/GenerateMember/GenerateConstructor/VisualBasicGenerateConstructorService.vb

@@ -4,6 +4,7 @@ Imports System.Composition
 Imports System.Threading
 Imports Microsoft.CodeAnalysis.GenerateMember.GenerateConstructor
 Imports Microsoft.CodeAnalysis.Host.Mef
+Imports Microsoft.CodeAnalysis.LanguageServices
 Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
 Imports Microsoft.CodeAnalysis.VisualBasic.Utilities
 
@@ -147,7 +148,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.GenerateMember.GenerateConstructor
 
         Private Shared ReadOnly s_annotation As SyntaxAnnotation = New SyntaxAnnotation
 
-        Friend Overrides Function GetDelegatingConstructor(state As State, document As SemanticDocument, argumentCount As Integer, namedType As INamedTypeSymbol, candidates As ISet(Of IMethodSymbol), cancellationToken As CancellationToken) As IMethodSymbol
+        Friend Overrides Function GetDelegatingConstructor(state As State,
+                                                           document As SemanticDocument,
+                                                           argumentCount As Integer,
+                                                           namedType As INamedTypeSymbol,
+                                                           candidates As ISet(Of IMethodSymbol),
+                                                           cancellationToken As CancellationToken) As IMethodSymbol
             Dim oldToken = state.Token
             Dim tokenKind = oldToken.Kind()
             Dim simpleName = DirectCast(oldToken.Parent, SimpleNameSyntax)
@@ -178,7 +184,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.GenerateMember.GenerateConstructor
                 Dim speculativeModel As SemanticModel = Nothing
                 If document.SemanticModel.TryGetSpeculativeSemanticModel(invocationStatement.SpanStart, newInvocationStatement, speculativeModel) Then
                     Dim symbolInfo = speculativeModel.GetSymbolInfo(newInvocation, cancellationToken)
-                    Return GenerateConstructorHelpers.GetDelegatingConstructor(symbolInfo, candidates, namedType)
+                    Return GenerateConstructorHelpers.GetDelegatingConstructor(
+                        document, symbolInfo, candidates, namedType, state.ParameterTypes)
                 End If
             Else
                 Dim oldNode = oldToken.Parent _
@@ -218,7 +225,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.GenerateMember.GenerateConstructor
                 Dim speculativeModel = SpeculationAnalyzer.CreateSpeculativeSemanticModelForNode(oldNode, newNode, document.SemanticModel)
                 If speculativeModel IsNot Nothing Then
                     Dim symbolInfo = speculativeModel.GetSymbolInfo(newTypeName.Parent, cancellationToken)
-                    Return GenerateConstructorHelpers.GetDelegatingConstructor(symbolInfo, candidates, namedType)
+                    Return GenerateConstructorHelpers.GetDelegatingConstructor(
+                        document, symbolInfo, candidates, namedType, state.ParameterTypes)
                 End If
             End If
 

src/Features/VisualBasic/Portable/GenerateType/VisualBasicGenerateTypeService.vb

@@ -716,7 +716,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.GenerateType
             Return True
         End Function
 
-        Friend Overrides Function GetDelegatingConstructor(objectCreation As ObjectCreationExpressionSyntax, namedType As INamedTypeSymbol, model As SemanticModel, candidates As ISet(Of IMethodSymbol), cancellationToken As CancellationToken) As IMethodSymbol
+        Friend Overrides Function GetDelegatingConstructor(document As SemanticDocument,
+                                                           objectCreation As ObjectCreationExpressionSyntax,
+                                                           namedType As INamedTypeSymbol,
+                                                           candidates As ISet(Of IMethodSymbol),
+                                                           cancellationToken As CancellationToken) As IMethodSymbol
+            Dim model = document.SemanticModel
             Dim oldNode = objectCreation _
                 .AncestorsAndSelf(ascendOutOfTrivia:=False) _
                 .Where(Function(node) SpeculationAnalyzer.CanSpeculateOnNode(node)) _
@@ -731,10 +736,21 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.GenerateType
             If speculativeModel IsNot Nothing Then
                 newObjectCreation = DirectCast(newNode.GetAnnotatedNodes(s_annotation).Single(), ObjectCreationExpressionSyntax)
                 Dim symbolInfo = speculativeModel.GetSymbolInfo(newObjectCreation, cancellationToken)
-                Return GenerateConstructorHelpers.GetDelegatingConstructor(symbolInfo, candidates, namedType)
+                Dim parameterTypes As IList(Of ITypeSymbol) = GetSpeculativeArgumentTypes(speculativeModel, newObjectCreation)
+                Return GenerateConstructorHelpers.GetDelegatingConstructor(
+                    document, symbolInfo, candidates, namedType, parameterTypes)
             End If
 
             Return Nothing
         End Function
+
+        Private Shared Function GetSpeculativeArgumentTypes(model As SemanticModel, newObjectCreation As ObjectCreationExpressionSyntax) As IList(Of ITypeSymbol)
+            Return If(newObjectCreation.ArgumentList Is Nothing,
+                      SpecializedCollections.EmptyList(Of ITypeSymbol),
+                      newObjectCreation.ArgumentList.Arguments.Select(
+                          Function(a)
+                              Return If(a.GetExpression() Is Nothing, Nothing, model.GetTypeInfo(a.GetExpression()).ConvertedType)
+                          End Function).ToList())
+        End Function
     End Class
 End Namespace