diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/GenerateConstructor/GenerateConstructorTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/GenerateConstructor/GenerateConstructorTests.cs index c8d212cc2c1cfe9c24cad90cedce73a787ed11c8..19484ecc6115964fd46f33ee8a3aa53f28f3864b 100644 --- a/src/EditorFeatures/CSharpTest/Diagnostics/GenerateConstructor/GenerateConstructorTests.cs +++ b/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; + } + } +}"); + } } } diff --git a/src/Features/CSharp/Portable/GenerateMember/GenerateConstructor/CSharpGenerateConstructorService.cs b/src/Features/CSharp/Portable/GenerateMember/GenerateConstructor/CSharpGenerateConstructorService.cs index 1d986ff5ec94e51fcdf19bbc5d419829c10be572..2e3d00d64fb9e1d08706a611691dd3d96d29df99 100644 --- a/src/Features/CSharp/Portable/GenerateMember/GenerateConstructor/CSharpGenerateConstructorService.cs +++ b/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 candidates, CancellationToken cancellationToken) + internal override IMethodSymbol GetDelegatingConstructor( + State state, + SemanticDocument document, + int argumentCount, + INamedTypeSymbol namedType, + ISet 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); } } diff --git a/src/Features/CSharp/Portable/GenerateType/CSharpGenerateTypeService.cs b/src/Features/CSharp/Portable/GenerateType/CSharpGenerateTypeService.cs index 7734d2907b25ff0c4e67f170ad072f19a1c6ab55..644810cbe70a9d06fc2edf6fdbacf1fe019e33a4 100644 --- a/src/Features/CSharp/Portable/GenerateType/CSharpGenerateTypeService.cs +++ b/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 candidates, CancellationToken cancellationToken) + internal override IMethodSymbol GetDelegatingConstructor( + SemanticDocument document, + ObjectCreationExpressionSyntax objectCreation, + INamedTypeSymbol namedType, + ISet 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; diff --git a/src/Features/Core/Portable/GenerateMember/GenerateConstructor/GenerateConstructorHelpers.cs b/src/Features/Core/Portable/GenerateMember/GenerateConstructor/GenerateConstructorHelpers.cs index f37f8543d0fdafc531ecd4a009ee129a97162e51..0f24419ad63c57ed957ec1a96327fd2d540e666e 100644 --- a/src/Features/Core/Portable/GenerateMember/GenerateConstructor/GenerateConstructorHelpers.cs +++ b/src/Features/Core/Portable/GenerateMember/GenerateConstructor/GenerateConstructorHelpers.cs @@ -1,25 +1,44 @@ // 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 candidateInstanceConstructors, INamedTypeSymbol containingType) + public static IMethodSymbol GetDelegatingConstructor( + SemanticDocument document, + SymbolInfo symbolInfo, + ISet candidateInstanceConstructors, + INamedTypeSymbol containingType, + IList 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 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(); + + 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; + } } } diff --git a/src/Features/Core/Portable/GenerateType/AbstractGenerateTypeService.GenerateNamedType.cs b/src/Features/Core/Portable/GenerateType/AbstractGenerateTypeService.GenerateNamedType.cs index 572db2ac54e15e0a4f39df851b027c00f0e211ee..6ccf953759ec52967800453905c87b1947cfc207 100644 --- a/src/Features/Core/Portable/GenerateType/AbstractGenerateTypeService.GenerateNamedType.cs +++ b/src/Features/Core/Portable/GenerateType/AbstractGenerateTypeService.GenerateNamedType.cs @@ -14,7 +14,12 @@ namespace Microsoft.CodeAnalysis.GenerateType { internal abstract partial class AbstractGenerateTypeService { - internal abstract IMethodSymbol GetDelegatingConstructor(TObjectCreationExpressionSyntax objectCreation, INamedTypeSymbol namedType, SemanticModel model, ISet candidates, CancellationToken cancellationToken); + internal abstract IMethodSymbol GetDelegatingConstructor( + SemanticDocument document, + TObjectCreationExpressionSyntax objectCreation, + INamedTypeSymbol namedType, + ISet candidates, + CancellationToken cancellationToken); private partial class Editor { @@ -145,7 +150,12 @@ private void AddMembers(IList 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. diff --git a/src/Features/VisualBasic/Portable/GenerateMember/GenerateConstructor/VisualBasicGenerateConstructorService.vb b/src/Features/VisualBasic/Portable/GenerateMember/GenerateConstructor/VisualBasicGenerateConstructorService.vb index 992d5abeb7fe4b31077f6c63ef7c881e2fdbc617..90c22992f2d2890fa3d0545d97621769667aa67b 100644 --- a/src/Features/VisualBasic/Portable/GenerateMember/GenerateConstructor/VisualBasicGenerateConstructorService.vb +++ b/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 diff --git a/src/Features/VisualBasic/Portable/GenerateType/VisualBasicGenerateTypeService.vb b/src/Features/VisualBasic/Portable/GenerateType/VisualBasicGenerateTypeService.vb index c61b89c4d44648768559a50e1ae617125fb0d155..a83a80c0a8815b537e0df8661f48fdaf0542dd3c 100644 --- a/src/Features/VisualBasic/Portable/GenerateType/VisualBasicGenerateTypeService.vb +++ b/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