From 9672d4c158d793cbd6aa55e2e3d2e86a311b5acf Mon Sep 17 00:00:00 2001 From: Jonathon Marolf Date: Thu, 14 Jan 2016 17:36:42 -0800 Subject: [PATCH] Correctly detect existing constructors during code generation --- .../GenerateConstructorTests.cs | 7 ++ .../CrefCompletionProviderTests.cs | 5 ++ .../CrefCompletionProviderTests.vb | 4 + .../GenerateConstructorTests.vb | 18 +++++ ...bstractGenerateConstructorService.State.cs | 73 ++++++++++++++++--- .../AbstractGenerateConstructorService.cs | 1 - .../CSharpSyntaxFactsService.cs | 10 +++ .../SyntaxFactsService/ISyntaxFactsService.cs | 6 ++ .../VisualBasicSyntaxFactsService.vb | 10 ++- 9 files changed, 121 insertions(+), 13 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/CodeActions/GenerateFromMembers/GenerateConstructor/GenerateConstructorTests.cs b/src/EditorFeatures/CSharpTest/CodeActions/GenerateFromMembers/GenerateConstructor/GenerateConstructorTests.cs index 6487ac68f5e..c423ea2bc65 100644 --- a/src/EditorFeatures/CSharpTest/CodeActions/GenerateFromMembers/GenerateConstructor/GenerateConstructorTests.cs +++ b/src/EditorFeatures/CSharpTest/CodeActions/GenerateFromMembers/GenerateConstructor/GenerateConstructorTests.cs @@ -173,5 +173,12 @@ public async Task TestContextualKeywordName() @"class Program { [|int yield ;|] } ", @"class Program { int yield ; public Program ( int yield ) { this . yield = yield ; } } "); } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsGenerateConstructor)] + public async Task TestGenerateConstructorNotOfferedForDuplicate() + { + await TestMissingAsync( +"using System ; class X { public X ( string v ) { } static void Test ( ) { new X ( new [|string|] ( ) ) ; } } "); + } } } diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/CrefCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/CrefCompletionProviderTests.cs index 62b117459f6..2a035b54792 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/CrefCompletionProviderTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/CrefCompletionProviderTests.cs @@ -910,6 +910,11 @@ public TextSpan GetInactiveRegionSpanAroundPosition(SyntaxTree tree, int positio { throw new NotImplementedException(); } + + public string GetNameForArgument(SyntaxNode argument) + { + throw new NotImplementedException(); + } } } } diff --git a/src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/CrefCompletionProviderTests.vb b/src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/CrefCompletionProviderTests.vb index 773b3e8fd62..f54430dd9d4 100644 --- a/src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/CrefCompletionProviderTests.vb +++ b/src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/CrefCompletionProviderTests.vb @@ -816,6 +816,10 @@ End Class]]>.Value.NormalizeLineEndings() Public Function GetInactiveRegionSpanAroundPosition(tree As SyntaxTree, position As Integer, cancellationToken As CancellationToken) As TextSpan Implements ISyntaxFactsService.GetInactiveRegionSpanAroundPosition Throw New NotImplementedException() End Function + + Public Function GetNameForArgument(argument As SyntaxNode) As String Implements ISyntaxFactsService.GetNameForArgument + Throw New NotImplementedException() + End Function End Class End Class End Namespace diff --git a/src/EditorFeatures/VisualBasicTest/Diagnostics/GenerateConstructor/GenerateConstructorTests.vb b/src/EditorFeatures/VisualBasicTest/Diagnostics/GenerateConstructor/GenerateConstructorTests.vb index 06fa0238886..345577bb677 100644 --- a/src/EditorFeatures/VisualBasicTest/Diagnostics/GenerateConstructor/GenerateConstructorTests.vb +++ b/src/EditorFeatures/VisualBasicTest/Diagnostics/GenerateConstructor/GenerateConstructorTests.vb @@ -578,6 +578,24 @@ End Class Public Class [|;;|]Derived Inherits Base +End Class") + End Function + + + Public Async Function TestGenerateConstructorNotOfferedForDuplicate() As Task + Await TestMissingAsync( +"Imports System + +Class X + Private v As String + + Public Sub New(v As String) + Me.v = v + End Sub + + Sub Test() + Dim x As X = New X(New [|String|]()) + End Sub End Class") End Function End Class diff --git a/src/Features/Core/Portable/GenerateMember/GenerateConstructor/AbstractGenerateConstructorService.State.cs b/src/Features/Core/Portable/GenerateMember/GenerateConstructor/AbstractGenerateConstructorService.State.cs index b99034c36b0..1ba451ba5e1 100644 --- a/src/Features/Core/Portable/GenerateMember/GenerateConstructor/AbstractGenerateConstructorService.State.cs +++ b/src/Features/Core/Portable/GenerateMember/GenerateConstructor/AbstractGenerateConstructorService.State.cs @@ -1,5 +1,6 @@ // 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 System.Threading; @@ -10,7 +11,6 @@ using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.Utilities; -using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.GenerateMember.GenerateConstructor { @@ -93,23 +93,74 @@ private State() this.ParameterTypes = this.ParameterTypes ?? GetParameterTypes(service, document, cancellationToken); this.ParameterRefKinds = this.ParameterRefKinds ?? this.Arguments.Select(service.GetRefKind).ToList(); - return !ClashesWithExistingConstructor(service, document, cancellationToken); + return !ClashesWithExistingConstructor(document, cancellationToken); } - private bool ClashesWithExistingConstructor(TService service, SemanticDocument document, CancellationToken cancellationToken) + private bool ClashesWithExistingConstructor(SemanticDocument document, CancellationToken cancellationToken) { - var parameters = this.ParameterTypes.Zip(this.ParameterRefKinds, (t, r) => CodeGenerationSymbolFactory.CreateParameterSymbol( - attributes: null, - refKind: r, - isParams: false, - type: t, - name: string.Empty)).ToList(); + if (this.ParameterTypes == null || + this.ParameterRefKinds == null) + { + return false; + } + + if(this.ParameterTypes.Count != this.ParameterRefKinds.Count) + { + return false; + } + + if (!this.TypeToGenerateIn.InstanceConstructors.Any(c => c.Parameters.Length == this.ParameterTypes.Count)) + { + return false; + } var destinationProvider = document.Project.Solution.Workspace.Services.GetLanguageServices(this.TypeToGenerateIn.Language); var syntaxFacts = destinationProvider.GetService(); + for (int i = 0; i < ParameterTypes.Count; i++) + { + var compareParameterName = false; + var type = this.ParameterTypes[i]; + var refKind = this.ParameterRefKinds[i]; + string parameterName = GetParameterName(syntaxFacts, i); + if (!string.IsNullOrEmpty(parameterName)) + { + compareParameterName = true; + } + var parameterSymbol = CodeGenerationSymbolFactory.CreateParameterSymbol( + attributes: null, + refKind: refKind, + isParams: false, + type: type, + name: parameterName); + var result = this.TypeToGenerateIn.InstanceConstructors.Any(c => + { + if (i >= c.Parameters.Length) + { + return false; + } + + return SymbolEquivalenceComparer + .Instance + .ParameterEquivalenceComparer + .Equals(parameterSymbol, c.Parameters[i], compareParameterName, syntaxFacts.IsCaseSensitive); + }); + if (result == false) + { + return false; + } + } + + return true; + } + + private string GetParameterName(ISyntaxFactsService service, int i) + { + if (i >= this.Arguments?.Count) + { + return string.Empty; + } - return this.TypeToGenerateIn.InstanceConstructors.Any( - c => SignatureComparer.Instance.HaveSameSignature(parameters, c.Parameters, compareParameterName: true, isCaseSensitive: syntaxFacts.IsCaseSensitive)); + return service.GetNameForArgument(this.Arguments?[i]); } internal List GetParameterTypes( diff --git a/src/Features/Core/Portable/GenerateMember/GenerateConstructor/AbstractGenerateConstructorService.cs b/src/Features/Core/Portable/GenerateMember/GenerateConstructor/AbstractGenerateConstructorService.cs index 16eeb1c2d8f..ada6d65ee6f 100644 --- a/src/Features/Core/Portable/GenerateMember/GenerateConstructor/AbstractGenerateConstructorService.cs +++ b/src/Features/Core/Portable/GenerateMember/GenerateConstructor/AbstractGenerateConstructorService.cs @@ -28,7 +28,6 @@ protected AbstractGenerateConstructorService() protected abstract bool TryInitializeClassDeclarationGenerationState(SemanticDocument document, SyntaxNode classDeclaration, CancellationToken cancellationToken, out SyntaxToken token, out IMethodSymbol constructor, out INamedTypeSymbol typeToGenerateIn); protected abstract bool TryInitializeConstructorInitializerGeneration(SemanticDocument document, SyntaxNode constructorInitializer, CancellationToken cancellationToken, out SyntaxToken token, out IList arguments, out INamedTypeSymbol typeToGenerateIn); protected abstract bool TryInitializeSimpleAttributeNameGenerationState(SemanticDocument document, SyntaxNode simpleName, CancellationToken cancellationToken, out SyntaxToken token, out IList arguments, out IList attributeArguments, out INamedTypeSymbol typeToGenerateIn); - protected abstract IList GenerateParameterNames(SemanticModel semanticModel, IEnumerable arguments, IList reservedNames = null); protected virtual IList GenerateParameterNames(SemanticModel semanticModel, IEnumerable arguments, IList reservedNames = null) { return null; } protected abstract string GenerateNameForArgument(SemanticModel semanticModel, TArgumentSyntax argument); diff --git a/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs b/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs index b917b88a35d..d9ba65fee49 100644 --- a/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs +++ b/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs @@ -1429,5 +1429,15 @@ public TextSpan GetInactiveRegionSpanAroundPosition(SyntaxTree syntaxTree, int p return default(TextSpan); } + + public string GetNameForArgument(SyntaxNode argument) + { + if ((argument as ArgumentSyntax)?.NameColon != null) + { + return (argument as ArgumentSyntax).NameColon.Name.Identifier.ValueText; + } + + return string.Empty; + } } } diff --git a/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs b/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs index abdea1375f7..b5d4c1b985c 100644 --- a/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs +++ b/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs @@ -156,6 +156,12 @@ internal interface ISyntaxFactsService : ILanguageService IEnumerable GetConstructors(SyntaxNode root, CancellationToken cancellationToken); bool TryGetCorrespondingOpenBrace(SyntaxToken token, out SyntaxToken openBrace); + + /// + /// Given a , that represents and argument return the string representation of + /// that arguments name. + /// + string GetNameForArgument(SyntaxNode argument); } [Flags] diff --git a/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb b/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb index df64296cf60..fe1d35ec881 100644 --- a/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb +++ b/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb @@ -1177,5 +1177,13 @@ Namespace Microsoft.CodeAnalysis.VisualBasic Return Nothing End Function - End Class + + Public Function GetNameForArgument(argument As SyntaxNode) As String Implements ISyntaxFactsService.GetNameForArgument + If TryCast(argument, ArgumentSyntax)?.IsNamed Then + Return DirectCast(argument, SimpleArgumentSyntax).NameColonEquals.Name.Identifier.ValueText + End If + + Return String.Empty + End Function + End Class End Namespace -- GitLab