From 2fc0ecfd685ccfd6d74916bcbd4579b6984dbddf Mon Sep 17 00:00:00 2001 From: Cheryl Borley Date: Tue, 12 Mar 2019 10:27:26 -0700 Subject: [PATCH 01/13] Fixes 33603 and 33623 --- ...ddConstructorParametersFromMembersTests.cs | 194 +++++++++++++++++- ...ider.AddConstructorParametersCodeAction.cs | 52 +++-- ...romMembersCodeRefactoringProvider.State.cs | 142 ++++++++----- ...etersFromMembersCodeRefactoringProvider.cs | 69 +++++-- .../Portable/FeaturesResources.Designer.cs | 18 ++ .../Core/Portable/FeaturesResources.resx | 6 + .../Portable/xlf/FeaturesResources.cs.xlf | 10 + .../Portable/xlf/FeaturesResources.de.xlf | 10 + .../Portable/xlf/FeaturesResources.es.xlf | 10 + .../Portable/xlf/FeaturesResources.fr.xlf | 10 + .../Portable/xlf/FeaturesResources.it.xlf | 10 + .../Portable/xlf/FeaturesResources.ja.xlf | 10 + .../Portable/xlf/FeaturesResources.ko.xlf | 10 + .../Portable/xlf/FeaturesResources.pl.xlf | 10 + .../Portable/xlf/FeaturesResources.pt-BR.xlf | 10 + .../Portable/xlf/FeaturesResources.ru.xlf | 10 + .../Portable/xlf/FeaturesResources.tr.xlf | 10 + .../xlf/FeaturesResources.zh-Hans.xlf | 10 + .../xlf/FeaturesResources.zh-Hant.xlf | 10 + 19 files changed, 517 insertions(+), 94 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs b/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs index af80dcb4fb219..a30a06f5533db 100644 --- a/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs +++ b/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs @@ -1,7 +1,9 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +// 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.Collections.Immutable; using System.Threading.Tasks; using Microsoft.CodeAnalysis.AddConstructorParametersFromMembers; +using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeRefactorings; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Test.Utilities; @@ -17,6 +19,9 @@ public class AddConstructorParametersFromMembersTests : AbstractCSharpCodeAction protected override CodeRefactoringProvider CreateCodeRefactoringProvider(Workspace workspace, TestParameters parameters) => new AddConstructorParametersFromMembersCodeRefactoringProvider(); + protected override ImmutableArray MassageActions(ImmutableArray actions) + => FlattenActions(actions); + [WorkItem(308077, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/308077")] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] public async Task TestAdd1() @@ -83,9 +88,11 @@ public Program(int i, string s = null) } [WorkItem(308077, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/308077")] + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] public async Task TestAddToConstructorWithMostMatchingParameters1() { + // behavior change with 33603, now all constructors offered await TestInRegularAndScriptAsync( @"using System.Collections.Generic; @@ -113,23 +120,26 @@ class Program string s; bool b; - public Program(int i) + public Program(int i, string s, bool b) { this.i = i; + this.s = s; + this.b = b; } - public Program(int i, string s, bool b) : this(i) + public Program(int i, string s) : this(i) { this.s = s; - this.b = b; } }"); } [WorkItem(308077, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/308077")] + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] public async Task TestAddOptionalToConstructorWithMostMatchingParameters1() { + // Behavior change with #33603, now all constructors are offered await TestInRegularAndScriptAsync( @"using System.Collections.Generic; @@ -162,7 +172,7 @@ public Program(int i) this.i = i; } - public Program(int i, string s, bool b = false) : this(i) + public Program(int i, string s, bool b) : this(i) { this.s = s; this.b = b; @@ -769,5 +779,177 @@ public C(int i, int j) }" ); } + + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] + public async Task TestMultipleConstructors() + { + var source = +@" +class C +{ + int [|l|]; + public C(int i) + { + } + public C(int i, int j) + { + } + public C(int i, int j, int k) + { + } +} +"; + var expected1 = +@" +class C +{ + int l; + public C(int i, int l) + { + this.l = l; + } + public C(int i, int j) + { + } + public C(int i, int j, int k) + { + } +} +"; + var expected2 = +@" +class C +{ + int l; + public C(int i) + { + } + public C(int i, int j, int l) + { + this.l = l; + } + public C(int i, int j, int k) + { + } +} +"; + var expected3 = +@" +class C +{ + int l; + public C(int i) + { + } + public C(int i, int j, int l = 0) + { + this.l = l; + } + public C(int i, int j, int k) + { + } +} +"; + await TestInRegularAndScriptAsync(source, expected1, 0); + await TestInRegularAndScriptAsync(source, expected2, 1); + await TestInRegularAndScriptAsync(source, expected3, 4); + } + + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] + public async Task TestOnlyOptionalSubmenuOffered() + { + var source = +@" +class C +{ + int [|l|]; + public C(int i = 1) + { + } + public C(int i = 1, int j = 2) + { + } + public C(int i = 1, int j = 2, int k = 3) + { + } +} +"; + var expected1 = +@" +class C +{ + int l; + public C(int i = 1, int l = 0) + { + this.l = l; + } + public C(int i = 1, int j = 2) + { + } + public C(int i = 1, int j = 2, int k = 3) + { + } +} +"; + var expected2 = +@" +class C +{ + int l; + public C(int i = 1) + { + } + public C(int i = 1, int j = 2, int l = 0) + { + this.l = l; + } + public C(int i = 1, int j = 2, int k = 3) + { + } +} +"; + var expected3 = +@" +class C +{ + int l; + public C(int i = 1) + { + } + public C(int i = 1, int j = 2) + { + } + public C(int i = 1, int j = 2, int k = 3, int l = 0) + { + this.l = l; + } +} +"; + await TestInRegularAndScriptAsync(source, expected1, 0); + await TestInRegularAndScriptAsync(source, expected2, 1); + await TestInRegularAndScriptAsync(source, expected3, 2); + } + + [WorkItem(33623, "https://github.com/dotnet/roslyn/issues/33623")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] + public async Task TestDeserializationConstructor() + { + await TestMissingAsync( +@" +using System; +using System.Runtime.Serialization; + +class C : ISerializable +{ + int [|i|]; + + private C(SerializationInfo info, StreamingContext context) + { + } +} +"); + } } -} \ No newline at end of file +} diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs index 95e327be5c31e..291d0790340d0 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs @@ -19,32 +19,36 @@ internal partial class AddConstructorParametersFromMembersCodeRefactoringProvide { private class AddConstructorParametersCodeAction : CodeAction { - private readonly AddConstructorParametersFromMembersCodeRefactoringProvider _service; private readonly Document _document; - private readonly State _state; - private readonly ImmutableArray _missingParameters; + private readonly ConstructorCandidate _constructorCandidate; + private readonly ISymbol _containingType; + private readonly ImmutableArray _parametersToAdd; + private readonly bool _useSubMenuName; public AddConstructorParametersCodeAction( - AddConstructorParametersFromMembersCodeRefactoringProvider service, Document document, - State state, - ImmutableArray missingParameters) + ConstructorCandidate constructorCandidate, + ISymbol containingType, + ImmutableArray parametersToAdd, + bool useSubMenuName + ) { - _service = service; _document = document; - _state = state; - _missingParameters = missingParameters; + _constructorCandidate = constructorCandidate; + _containingType = containingType; + _parametersToAdd = parametersToAdd; + _useSubMenuName = useSubMenuName; } protected override Task GetChangedDocumentAsync(CancellationToken cancellationToken) { var workspace = _document.Project.Solution.Workspace; var declarationService = _document.GetLanguageService(); - var constructor = declarationService.GetDeclarations(_state.ConstructorToAddTo).Select(r => r.GetSyntax(cancellationToken)).First(); + var constructor = declarationService.GetDeclarations(_constructorCandidate._constructor).Select(r => r.GetSyntax(cancellationToken)).First(); var newConstructor = constructor; - newConstructor = CodeGenerator.AddParameterDeclarations(newConstructor, _missingParameters, workspace); - newConstructor = CodeGenerator.AddStatements(newConstructor, CreateAssignStatements(_state), workspace) + newConstructor = CodeGenerator.AddParameterDeclarations(newConstructor, _parametersToAdd, workspace); + newConstructor = CodeGenerator.AddStatements(newConstructor, CreateAssignStatements(_constructorCandidate), workspace) .WithAdditionalAnnotations(Formatter.Annotation); var syntaxTree = constructor.SyntaxTree; @@ -53,13 +57,13 @@ protected override Task GetChangedDocumentAsync(CancellationToken canc return Task.FromResult(_document.WithSyntaxRoot(newRoot)); } - private IEnumerable CreateAssignStatements(State state) + private IEnumerable CreateAssignStatements(ConstructorCandidate constructorCandidate) { var factory = _document.GetLanguageService(); - for (var i = 0; i < _missingParameters.Length; ++i) + for (var i = 0; i < _parametersToAdd.Length; ++i) { - var memberName = _state.MissingMembers[i].Name; - var parameterName = _missingParameters[i].Name; + var memberName = constructorCandidate._missingMembers[i].Name; + var parameterName = _parametersToAdd[i].Name; yield return factory.ExpressionStatement( factory.AssignmentStatement( factory.MemberAccessExpression(factory.ThisExpression(), factory.IdentifierName(memberName)), @@ -71,14 +75,22 @@ public override string Title { get { - var parameters = _state.ConstructorToAddTo.Parameters.Select(p => p.ToDisplayString(SimpleFormat)); + var parameters = _constructorCandidate._constructor.Parameters.Select(p => p.ToDisplayString(SimpleFormat)); var parameterString = string.Join(", ", parameters); - var optional = _missingParameters[0].IsOptional; - var signature = $"{_state.ContainingType.Name}({parameterString})"; + var optional = _parametersToAdd[0].IsOptional; + var signature = $"{_containingType}({parameterString})"; + var submenu = _useSubMenuName; - return optional + if (submenu) + { + return string.Format(FeaturesResources.Add_to_0, signature); + } + else + { + return optional ? string.Format(FeaturesResources.Add_optional_parameters_to_0, signature) : string.Format(FeaturesResources.Add_parameters_to_0, signature); + } } } } diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs index 7aa4bbcaf633e..ce262cbf04733 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs @@ -1,27 +1,42 @@ // 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.Immutable; using System.Linq; +using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Shared.Utilities; namespace Microsoft.CodeAnalysis.AddConstructorParametersFromMembers { internal partial class AddConstructorParametersFromMembersCodeRefactoringProvider { + internal class ConstructorCandidate + { + internal IMethodSymbol _constructor; + internal ImmutableArray _missingMembers; + internal ImmutableArray _missingParameters; + + public ConstructorCandidate(IMethodSymbol constructor, ImmutableArray missingMembers, ImmutableArray missingParameters) + { + _constructor = constructor; + _missingMembers = missingMembers; + _missingParameters = missingParameters; + } + } private class State { - public IMethodSymbol ConstructorToAddTo { get; private set; } + public ImmutableArray ConstructorCandidates { get; private set; } public INamedTypeSymbol ContainingType { get; private set; } - public ImmutableArray MissingMembers { get; private set; } - public ImmutableArray MissingParameters { get; private set; } - public static State Generate( + public static async Task GenerateAsync( AddConstructorParametersFromMembersCodeRefactoringProvider service, - ImmutableArray selectedMembers) + ImmutableArray selectedMembers, + Document document) { var state = new State(); - if (!state.TryInitialize(service, selectedMembers)) + if (!await state.TryInitializeAsync(service, selectedMembers, document).ConfigureAwait(true)) { return null; } @@ -29,81 +44,110 @@ public static State Generate( return state; } - private bool TryInitialize( + private async Task TryInitializeAsync( AddConstructorParametersFromMembersCodeRefactoringProvider service, - ImmutableArray selectedMembers) + ImmutableArray selectedMembers, + Document document) { if (!selectedMembers.All(IsWritableInstanceFieldOrProperty)) { return false; } - this.ContainingType = selectedMembers[0].ContainingType; - if (this.ContainingType == null || this.ContainingType.TypeKind == TypeKind.Interface) + ContainingType = selectedMembers[0].ContainingType; + if (ContainingType == null || ContainingType.TypeKind == TypeKind.Interface) { return false; } - var parameters = service.DetermineParameters(selectedMembers); + var parametersForSelectedMembers = service.DetermineParameters(selectedMembers); // We are trying to add these parameters into an existing constructor's parameter list. // Comparing parameters based on names to make sure parameter list won't contains duplicate parameters after we // append the new parameters - this.ConstructorToAddTo = GetDelegatedConstructorBasedOnParameterNames(this.ContainingType, parameters); + ConstructorCandidates = await GetConstructorCandidatesInfo(ContainingType, parametersForSelectedMembers, selectedMembers, document).ConfigureAwait(false); - if (this.ConstructorToAddTo == null) + if (ConstructorCandidates.Count() == 0) { return false; } - var zippedParametersAndSelectedMembers = parameters.Zip(selectedMembers, (parameter, selectedMember) => (parameter, selectedMember)); - var missingParametersBuilder = ArrayBuilder.GetInstance(); - var missingMembersBuilder = ArrayBuilder.GetInstance(); - var constructorParamNames = this.ConstructorToAddTo.Parameters.SelectAsArray(p => p.Name); - foreach ((var parameter, var selectedMember) in zippedParametersAndSelectedMembers) - { - if (!constructorParamNames.Contains(parameter.Name)) - { - missingParametersBuilder.Add(parameter); - missingMembersBuilder.Add(selectedMember); - } - } - - this.MissingParameters = missingParametersBuilder.ToImmutableAndFree(); - this.MissingMembers = missingMembersBuilder.ToImmutableAndFree(); - - return MissingParameters.Length != 0; + return true; } /// - /// Try to find a constructor in whose parameters is the subset of by comparing name. - /// If multiple constructors meet the condition, the one with more parameters will be returned. - /// It will not consider those constructors as potential candidates if the constructor's parameter list - /// contains 'ref' or 'params' + /// Try to find all constructors in whose parameters is the subset of by comparing name. + /// These constructors will not be considered as potential candidates + /// - if the constructor's parameter list contains 'ref' or 'params' + /// - any constructor that has a params[] parameter + /// - deserialization constructor + /// - implicit default constructor /// - private IMethodSymbol GetDelegatedConstructorBasedOnParameterNames( + private async Task> GetConstructorCandidatesInfo( INamedTypeSymbol containingType, - ImmutableArray parameters) + ImmutableArray parametersForSelectedMembers, + ImmutableArray selectedMembers, + Document document) { - var parameterNames = parameters.SelectAsArray(p => p.Name); - return containingType.InstanceConstructors - .Where(constructor => IsApplicableConstructor(constructor, parameterNames)) - .OrderByDescending(constructor => constructor.Parameters.Length) - .FirstOrDefault(); + var parameterNamesForSelectedMembers = parametersForSelectedMembers.SelectAsArray(p => p.Name); + var applicableConstructors = ArrayBuilder.GetInstance(); + var constructors = containingType.InstanceConstructors; + foreach (var constructor in constructors) + { + var constructorParams = constructor.Parameters; + + if (constructorParams.Length == 2) + { + var compilation = await document.Project.GetCompilationAsync().ConfigureAwait(false); + var deserializationConstructorCheck = new DeserializationConstructorCheck(compilation); + if (deserializationConstructorCheck.IsDeserializationConstructor(constructor)) + { + continue; + } + } + + if (!constructorParams.All(parameter => parameter.RefKind == RefKind.None) || + (constructorParams.Length == 0 && constructor.IsImplicitlyDeclared) || + constructorParams.Any(p => p.IsParams) || + SelectedMembersAlreadyExistAsParameters(parameterNamesForSelectedMembers, constructorParams)) + { + continue; + } + + var missingParametersBuilder = ArrayBuilder.GetInstance(); + var missingMembersBuilder = ArrayBuilder.GetInstance(); + var constructorParamNames = constructor.Parameters.SelectAsArray(p => p.Name); + var zippedParametersAndSelectedMembers = parametersForSelectedMembers.Zip(selectedMembers, (parameter, selectedMember) => (parameter, selectedMember)); + foreach ((var parameter, var selectedMember) in zippedParametersAndSelectedMembers) + { + if (!constructorParamNames.Contains(parameter.Name)) + { + missingParametersBuilder.Add(parameter); + missingMembersBuilder.Add(selectedMember); + } + } + + if (missingParametersBuilder != null) + { + applicableConstructors.Add(new ConstructorCandidate(constructor, missingMembersBuilder.ToImmutableAndFree(), missingParametersBuilder.ToImmutableAndFree())); + } + } + + return applicableConstructors.ToImmutableAndFree(); } - private bool IsApplicableConstructor( - IMethodSymbol constructor, - ImmutableArray parametersName) + private static bool SelectedMembersAlreadyExistAsParameters(ImmutableArray parameterNamesForSelectedMembers, ImmutableArray constructorParams) { - var constructorParams = constructor.Parameters; if (constructorParams.Length == 0) { - return !constructor.IsImplicitlyDeclared; + return false; + } + + if (parameterNamesForSelectedMembers.Except(constructorParams.Select(p => p.Name)).Any()) + { + return false; } - return constructorParams.All(parameter => parameter.RefKind == RefKind.None) - && !constructorParams.Any(p => p.IsParams) - && parametersName.Except(constructorParams.Select(p => p.Name)).Any(); + return true; } } } diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs index 00fb0ff81fb9c..abc64ab327df0 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs @@ -11,6 +11,7 @@ using Microsoft.CodeAnalysis.CodeRefactorings; using Microsoft.CodeAnalysis.GenerateFromMembers; using Microsoft.CodeAnalysis.Internal.Log; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Text; namespace Microsoft.CodeAnalysis.AddConstructorParametersFromMembers @@ -32,7 +33,7 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte return; } - var actions = await this.AddConstructorParametersFromMembersAsync(document, textSpan, cancellationToken).ConfigureAwait(false); + var actions = await AddConstructorParametersFromMembersAsync(document, textSpan, cancellationToken).ConfigureAwait(false); context.RegisterRefactorings(actions); } @@ -40,11 +41,11 @@ public async Task> AddConstructorParametersFromMember { using (Logger.LogBlock(FunctionId.Refactoring_GenerateFromMembers_AddConstructorParametersFromMembers, cancellationToken)) { - var info = await this.GetSelectedMemberInfoAsync(document, textSpan, allowPartialSelection: true, cancellationToken).ConfigureAwait(false); + var info = await GetSelectedMemberInfoAsync(document, textSpan, true, cancellationToken).ConfigureAwait(false); if (info != null) { - var state = State.Generate(this, info.SelectedMembers); - if (state != null && state.ConstructorToAddTo != null) + var state = State.GenerateAsync(this, info.SelectedMembers, document).Result; + if (state != null && state.ConstructorCandidates != null) { return CreateCodeActions(document, state).AsImmutableOrNull(); } @@ -56,25 +57,55 @@ public async Task> AddConstructorParametersFromMember private IEnumerable CreateCodeActions(Document document, State state) { - var parameters = state.ConstructorToAddTo.Parameters; - if (parameters.Length == 0 || - (parameters.Length > 0 && !parameters.Last().IsOptional)) + var result = new ArrayBuilder(); + var containingType = state.ContainingType; + if (state.ConstructorCandidates.Count() <= 1) { - // return a code action to add required parameters - yield return new AddConstructorParametersCodeAction(this, document, state, state.MissingParameters); + // There will be at most 2 suggested code actions, so no need to use sub menus + var constructorCandidate = state.ConstructorCandidates[0]; + if (CanHaveRequiredParameters(state.ConstructorCandidates[0]._missingParameters)) + { + result.Add(new AddConstructorParametersCodeAction(document, constructorCandidate, containingType, constructorCandidate._missingParameters, useSubMenuName: false)); + } + result.Add(GetOptionalContructorParametersCodeAction(document, constructorCandidate, containingType, useSubMenuName: false)); } + else + { + // Create sub menus for suggested actions, one for required parameters and one for optional parameters + var requiredParameterCodeActions = new ArrayBuilder(); + var optionalPrameterCodeActions = new ArrayBuilder(); + foreach (var constructor in state.ConstructorCandidates) + { + if (CanHaveRequiredParameters(constructor._missingParameters)) + { + requiredParameterCodeActions.Add(new AddConstructorParametersCodeAction(document, constructor, containingType, constructor._missingParameters, useSubMenuName: true)); + } + optionalPrameterCodeActions.Add(GetOptionalContructorParametersCodeAction(document, constructor, containingType, useSubMenuName: true)); + } - var missingParameters = state.MissingParameters.SelectAsArray(p => CodeGenerationSymbolFactory.CreateParameterSymbol( - attributes: default, - refKind: p.RefKind, - isParams: p.IsParams, - type: p.Type, - name: p.Name, - isOptional: true, - hasDefaultValue: true)); + result.Add(new CodeAction.CodeActionWithNestedActions(FeaturesResources.Add_parameter_to_constructor, requiredParameterCodeActions.ToImmutableAndFree(), isInlinable: false)); + result.Add(new CodeAction.CodeActionWithNestedActions(FeaturesResources.Add_optional_parameter_to_constructor, optionalPrameterCodeActions.ToImmutableAndFree(), isInlinable: false)); + } + + return result; + + // local functions + static CodeAction GetOptionalContructorParametersCodeAction(Document document, ConstructorCandidate constructorCandidate, INamedTypeSymbol containingType, bool useSubMenuName) + { + var missingOptionalParameters = constructorCandidate._missingParameters.SelectAsArray(p => CodeGenerationSymbolFactory.CreateParameterSymbol( + attributes: default, + refKind: p.RefKind, + isParams: p.IsParams, + type: p.Type, + name: p.Name, + isOptional: true, + hasDefaultValue: true)); + + return new AddConstructorParametersCodeAction(document, constructorCandidate, containingType, missingOptionalParameters, useSubMenuName); + } - // return a code action to add optional parameters - yield return new AddConstructorParametersCodeAction(this, document, state, missingParameters); + static bool CanHaveRequiredParameters(ImmutableArray parameters) + => parameters.Length == 0 || (parameters.Length > 0 && !parameters.Last().IsOptional); } } } diff --git a/src/Features/Core/Portable/FeaturesResources.Designer.cs b/src/Features/Core/Portable/FeaturesResources.Designer.cs index 157fcdb62a7c7..e87b8f50bc9a0 100644 --- a/src/Features/Core/Portable/FeaturesResources.Designer.cs +++ b/src/Features/Core/Portable/FeaturesResources.Designer.cs @@ -232,6 +232,15 @@ internal static string Add_null_checks { } } + /// + /// Looks up a localized string similar to Add optional parameter to constructor. + /// + internal static string Add_optional_parameter_to_constructor { + get { + return ResourceManager.GetString("Add_optional_parameter_to_constructor", resourceCulture); + } + } + /// /// Looks up a localized string similar to Add optional parameters to '{0}'. /// @@ -259,6 +268,15 @@ internal static string Add_parameter_to_0_and_overrides_implementations { } } + /// + /// Looks up a localized string similar to Add parameter to constructor. + /// + internal static string Add_parameter_to_constructor { + get { + return ResourceManager.GetString("Add_parameter_to_constructor", resourceCulture); + } + } + /// /// Looks up a localized string similar to Add parameters to '{0}'. /// diff --git a/src/Features/Core/Portable/FeaturesResources.resx b/src/Features/Core/Portable/FeaturesResources.resx index 742a13ff31f36..b8aaae85d618f 100644 --- a/src/Features/Core/Portable/FeaturesResources.resx +++ b/src/Features/Core/Portable/FeaturesResources.resx @@ -1641,4 +1641,10 @@ This version used in: {2} Make readonly fields writable {Locked="readonly"} "readonly" is C# keyword and should not be localized. + + Add optional parameter to constructor + + + Add parameter to constructor + \ No newline at end of file diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf index ac23ea65a8bf3..9a11c3951246c 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf @@ -7,11 +7,21 @@ Add member name + + Add optional parameter to constructor + Add optional parameter to constructor + + Add parameter to '{0}' (and overrides/implementations) Přidat parametr do {0} (a přepsání/implementace) + + Add parameter to constructor + Add parameter to constructor + + Add project reference to '{0}'. Přidat odkaz na projekt do {0} diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf index 784fb1e3051a7..4e0f6876a2252 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf @@ -7,11 +7,21 @@ Add member name + + Add optional parameter to constructor + Add optional parameter to constructor + + Add parameter to '{0}' (and overrides/implementations) Parameter zu "{0}" (und Außerkraftsetzungen/Implementierungen) hinzufügen + + Add parameter to constructor + Add parameter to constructor + + Add project reference to '{0}'. Fügen Sie zu "{0}" einen Projektverweis hinzu. diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf index 784b42d1913bd..9e84ab23cac42 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf @@ -7,11 +7,21 @@ Add member name + + Add optional parameter to constructor + Add optional parameter to constructor + + Add parameter to '{0}' (and overrides/implementations) Agregar un parámetro a “{0}” (y reemplazos/implementaciones) + + Add parameter to constructor + Add parameter to constructor + + Add project reference to '{0}'. Agregue referencia de proyecto a '{0}'. diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf index ff8df6d1d7286..551fe0fe97f43 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf @@ -7,11 +7,21 @@ Add member name + + Add optional parameter to constructor + Add optional parameter to constructor + + Add parameter to '{0}' (and overrides/implementations) Ajouter un paramètre à '{0}' (et aux remplacements/implémentations) + + Add parameter to constructor + Add parameter to constructor + + Add project reference to '{0}'. Ajoutez une référence de projet à '{0}'. diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf index f5ff423de9da3..088ac02159871 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf @@ -7,11 +7,21 @@ Add member name + + Add optional parameter to constructor + Add optional parameter to constructor + + Add parameter to '{0}' (and overrides/implementations) Aggiungi il parametro a '{0}' (e override/implementazioni) + + Add parameter to constructor + Add parameter to constructor + + Add project reference to '{0}'. Aggiunge il riferimento al progetto a '{0}'. diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf index e5e0cf35cc760..3b52e91a32813 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf @@ -7,11 +7,21 @@ Add member name + + Add optional parameter to constructor + Add optional parameter to constructor + + Add parameter to '{0}' (and overrides/implementations) パラメーターを '{0}' に追加します (オーバーライド/実装も行います) + + Add parameter to constructor + Add parameter to constructor + + Add project reference to '{0}'. プロジェクト参照を '{0}' に追加します。 diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf index dd1c5e23d2060..c7a919c993d15 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf @@ -7,11 +7,21 @@ Add member name + + Add optional parameter to constructor + Add optional parameter to constructor + + Add parameter to '{0}' (and overrides/implementations) '{0}'(및 재정의/구현)에 매개 변수 추가 + + Add parameter to constructor + Add parameter to constructor + + Add project reference to '{0}'. 프로젝트 참조를 '{0}'에 추가합니다. diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf index a984c7f4217b0..e8c191cacc11b 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf @@ -7,11 +7,21 @@ Add member name + + Add optional parameter to constructor + Add optional parameter to constructor + + Add parameter to '{0}' (and overrides/implementations) Dodaj parametr do elementu „{0}” (oraz przesłonięć/implementacji) + + Add parameter to constructor + Add parameter to constructor + + Add project reference to '{0}'. Dodaj odwołanie do projektu do elementu „{0}” diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf index 1389604c8799f..16218c82a7659 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf @@ -7,11 +7,21 @@ Add member name + + Add optional parameter to constructor + Add optional parameter to constructor + + Add parameter to '{0}' (and overrides/implementations) Adicionar parâmetro ao '{0}' (e substituições/implementações) + + Add parameter to constructor + Add parameter to constructor + + Add project reference to '{0}'. Adicione referência de projeto a "{0}". diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf index f45b14045d84d..badde3ae0c0d1 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf @@ -7,11 +7,21 @@ Add member name + + Add optional parameter to constructor + Add optional parameter to constructor + + Add parameter to '{0}' (and overrides/implementations) Добавить параметр в "{0}" (а также переопределения или реализации) + + Add parameter to constructor + Add parameter to constructor + + Add project reference to '{0}'. Добавьте ссылку на проект в "{0}". diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf index 8729bbd82791f..a27eb12394862 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf @@ -7,11 +7,21 @@ Add member name + + Add optional parameter to constructor + Add optional parameter to constructor + + Add parameter to '{0}' (and overrides/implementations) '{0}' öğesine (ve geçersiz kılmalara/uygulamalara) parametre ekle + + Add parameter to constructor + Add parameter to constructor + + Add project reference to '{0}'. {0}' üzerine proje başvurusu ekleyin. diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hans.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hans.xlf index 79da3fffdaeb3..cd25d900b6f7f 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hans.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hans.xlf @@ -7,11 +7,21 @@ Add member name + + Add optional parameter to constructor + Add optional parameter to constructor + + Add parameter to '{0}' (and overrides/implementations) 将参数添加到“{0}”(和重写/实现) + + Add parameter to constructor + Add parameter to constructor + + Add project reference to '{0}'. 将参数引用添加到“{0}”。 diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hant.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hant.xlf index a79cb00b65c6a..890800c341fab 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hant.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hant.xlf @@ -7,11 +7,21 @@ Add member name + + Add optional parameter to constructor + Add optional parameter to constructor + + Add parameter to '{0}' (and overrides/implementations) 將參數新增至 '{0}' (以及覆寫/執行) + + Add parameter to constructor + Add parameter to constructor + + Add project reference to '{0}'. 加入對 '{0}' 的專案參考。 From 29334741cda9a0dc4cd13888c55ac7464102353f Mon Sep 17 00:00:00 2001 From: Cheryl Borley Date: Thu, 14 Mar 2019 11:07:56 -0700 Subject: [PATCH 02/13] Respond to feedback --- ...ddConstructorParametersFromMembersTests.cs | 392 +++++++++++++++--- ...ddConstructorParametersFromMembersTests.vb | 261 +++++++++++- ...ider.AddConstructorParametersCodeAction.cs | 22 +- ...efactoringProvider.ConstructorCandidate.cs | 23 + ...romMembersCodeRefactoringProvider.State.cs | 15 +- ...etersFromMembersCodeRefactoringProvider.cs | 20 +- 6 files changed, 651 insertions(+), 82 deletions(-) create mode 100644 src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.ConstructorCandidate.cs diff --git a/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs b/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs index a30a06f5533db..71917950698a9 100644 --- a/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs +++ b/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs @@ -131,7 +131,52 @@ public Program(int i, string s) : this(i) { this.s = s; } -}"); +}", index: 0); + } + + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] + public async Task TestAddToConstructorWithMostMatchingParameters2() + { + // behavior change with 33603, now all constructors offered + await TestInRegularAndScriptAsync( +@"using System.Collections.Generic; + +class Program +{ + [|int i; + string s; + bool b;|] + + public Program(int i) + { + this.i = i; + } + + public Program(int i, string s) : this(i) + { + this.s = s; + } +}", +@"using System.Collections.Generic; + +class Program +{ + int i; + string s; + bool b; + + public Program(int i) + { + this.i = i; + } + + public Program(int i, string s, bool b) : this(i) + { + this.s = s; + this.b = b; + } +}", index: 1); } [WorkItem(308077, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/308077")] @@ -782,7 +827,7 @@ public C(int i, int j) [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] - public async Task TestMultipleConstructors() + public async Task TestMultipleConstructors_FirstofThree() { var source = @" @@ -798,9 +843,8 @@ public C(int i, int j) public C(int i, int j, int k) { } -} -"; - var expected1 = +}"; + var expected = @" class C { @@ -815,9 +859,30 @@ public C(int i, int j) public C(int i, int j, int k) { } -} -"; - var expected2 = +}"; + await TestInRegularAndScriptAsync(source, expected, index: 0); + } + + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] + public async Task TestMultipleConstructors_SecondOfThree() + { + var source = +@" +class C +{ + int [|l|]; + public C(int i) + { + } + public C(int i, int j) + { + } + public C(int i, int j, int k) + { + } +}"; + var expected = @" class C { @@ -832,9 +897,31 @@ public C(int i, int j, int l) public C(int i, int j, int k) { } -} -"; - var expected3 = +}"; + await TestInRegularAndScriptAsync(source, expected, index: 1); + } + + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] + public async Task TestMultipleConstructors_ThirdOfThree() + { + var source = +@" +class C +{ + int [|l|]; + public C(int i) + { + } + public C(int i, int j) + { + } + public C(int i, int j, int k) + { + } +}"; + + var expected = @" class C { @@ -842,94 +929,305 @@ class C public C(int i) { } - public C(int i, int j, int l = 0) + public C(int i, int j) + { + } + public C(int i, int j, int k, int l) + { + this.l = l; + } +}"; + await TestInRegularAndScriptAsync(source, expected, index: 2); + } + + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] + public async Task TestMultipleConstructors_FirstOptionalOfThree() + { + var source = +@" +class C +{ + int [|l|]; + public C(int i) + { + } + public C(int i, int j) + { + } + public C(int i, int j, int k) + { + } +}"; + var expected = +@" +class C +{ + int l; + public C(int i, int l = 0) { this.l = l; } + public C(int i, int j) + { + } public C(int i, int j, int k) { } -} -"; - await TestInRegularAndScriptAsync(source, expected1, 0); - await TestInRegularAndScriptAsync(source, expected2, 1); - await TestInRegularAndScriptAsync(source, expected3, 4); +}"; + await TestInRegularAndScriptAsync(source, expected, 3); } [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] - public async Task TestOnlyOptionalSubmenuOffered() + public async Task TestMultipleConstructorsSecondOptionalOfThree() { var source = @" class C { int [|l|]; - public C(int i = 1) + public C(int i) { } - public C(int i = 1, int j = 2) + public C(int i, int j) { } - public C(int i = 1, int j = 2, int k = 3) + public C(int i, int j, int k) { } -} -"; - var expected1 = +}"; + var expected = @" class C { - int l; - public C(int i = 1, int l = 0) + int [|l|]; + public C(int i) + { + } + public C(int i, int j, int l = 0) { this.l = l; } - public C(int i = 1, int j = 2) + public C(int i, int j, int k) { } - public C(int i = 1, int j = 2, int k = 3) +}"; + await TestInRegularAndScriptAsync(source, expected, index: 4); + } + + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] + public async Task TestMultipleConstructors_ThirdOptionalOfThree() + { + var source = +@" +class C +{ + int [|l|]; + public C(int i) { } -} -"; - var expected2 = + public C(int i, int j) + { + } + public C(int i, int j, int k) + { + } +}"; + var expected = @" class C { - int l; - public C(int i = 1) + int [|l|]; + public C(int i) + { + } + public C(int i, int j) { } - public C(int i = 1, int j = 2, int l = 0) + public C(int i, int j, int k, int l = 0) { this.l = l; } - public C(int i = 1, int j = 2, int k = 3) +}"; + await TestInRegularAndScriptAsync(source, expected, index: 5); + } + + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] + public async Task TestMultipleConstructors_OneMustBeOptional() + { + var source = +@" +class C +{ + int [|l|]; + + // index 0, and 2 as optional + public C(int i) + { + } + + // index 3 as optional + public C(int i, double j = 0) { } -} -"; - var expected3 = + + // index 1, and 4 as optional + public C(int i, double j, int k) + { + } +}"; + var expected = @" class C { - int l; - public C(int i = 1) + int [|l|]; + + // index 0, and 2 as optional + public C(int i) { } - public C(int i = 1, int j = 2) + + // index 3 as optional + public C(int i, double j = 0) { } - public C(int i = 1, int j = 2, int k = 3, int l = 0) + + // index 1, and 4 as optional + public C(int i, double j, int k, int l) { this.l = l; } -} -"; - await TestInRegularAndScriptAsync(source, expected1, 0); - await TestInRegularAndScriptAsync(source, expected2, 1); - await TestInRegularAndScriptAsync(source, expected3, 2); +}"; + await TestInRegularAndScriptAsync(source, expected, index: 1); + } + + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] + public async Task TestMultipleConstructors_OneMustBeOptional2() + { + var source = +@" +class C +{ + int [|l|]; + + // index 0, and 2 as optional + public C(int i) + { + } + + // index 3 as optional + public C(int i, double j = 0) + { + } + + // index 1, and 4 as optional + public C(int i, double j, int k) + { + } +}"; + var expected = +@" +class C +{ + int [|l|]; + + // index 0, and 2 as optional + public C(int i) + { + } + + // index 3 as optional + public C(int i, double j = 0, int l = 0) + { + this.l = l; + } + + // index 1, and 4 as optional + public C(int i, double j, int k) + { + } +}"; + await TestInRegularAndScriptAsync(source, expected, index: 3); + } + + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] + public async Task TestMultipleConstructors_AllMustBeOptional() + { + var source = +@" +class C +{ + int [|p|]; + public C(int i = 0) + { + } + public C(double j, int k = 0) + { + } + public C(int l, double m, int n = 0) + { + } +}"; + var expected = +@" +class C +{ + int [|p|]; + public C(int i = 0, int p = 0) + { + this.p = p; + } + public C(double j, int k = 0) + { + } + public C(int l, double m, int n = 0) + { + } +}"; + await TestInRegularAndScriptAsync(source, expected, index: 0); + } + + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] + public async Task TestMultipleConstructors_AllMustBeOptional2() + { + var source = +@" +class C +{ + int [|p|]; + public C(int i = 0) + { + } + public C(double j, int k = 0) + { + } + public C(int l, double m, int n = 0) + { + } +}"; + var expected = +@" +class C +{ + int [|p|]; + public C(int i = 0) + { + } + public C(double j, int k = 0) + { + } + public C(int l, double m, int n = 0, int p = 0) + { + this.p = p; + } +}"; + await TestInRegularAndScriptAsync(source, expected, index: 2); } [WorkItem(33623, "https://github.com/dotnet/roslyn/issues/33623")] diff --git a/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb b/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb index 94aac4825a3ce..0225a90399671 100644 --- a/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb +++ b/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb @@ -1,6 +1,8 @@ -' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +Imports System.Collections.Immutable Imports Microsoft.CodeAnalysis.AddConstructorParametersFromMembers +Imports Microsoft.CodeAnalysis.CodeActions Imports Microsoft.CodeAnalysis.CodeRefactorings Imports Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.CodeRefactorings @@ -12,6 +14,10 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.AddConstructorPara Return New AddConstructorParametersFromMembersCodeRefactoringProvider() End Function + Protected Overrides Function MassageActions(actions As ImmutableArray(Of CodeAction)) As ImmutableArray(Of CodeAction) + Return FlattenActions(actions) + End Function + Public Async Function TestAdd1() As Task @@ -56,8 +62,10 @@ index:=1) End Function + Public Async Function TestAddToConstructorWithMostMatchingParameters1() As Task + ' behavior change with 33603, now all constructors offered Await TestInRegularAndScriptAsync( "Class Program [|Private i As Integer @@ -83,12 +91,14 @@ End Class", Me.s = s Me.b = b End Sub -End Class") +End Class", index:=1) End Function + Public Async Function TestAddOptionalToConstructorWithMostMatchingParameters1() As Task + ' behavior change with 33603, now all constructors offered Await TestInRegularAndScriptAsync( "Class Program [|Private i As Integer @@ -115,7 +125,7 @@ End Class", Me.b = b End Sub End Class", -index:=1) +index:=3) End Function @@ -304,5 +314,248 @@ End Class", End Sub End Class") End Function + + + + Public Async Function TestMultipleConstructors_FirstOfThree() As Task + Await TestInRegularAndScriptAsync( +"Class Program + Private [|l|] As Integer + + Public Sub New(i As Integer) + Me.i = i + End Sub + + Public Sub New(i As Integer, j As Integer) + End Sub + + Public Sub New(i As Integer, j As Integer, k As Integer) + End Sub +End Class", +"Class Program + Private [|l|] As Integer + + Public Sub New(i As Integer, l As Integer) + Me.i = i + Me.l = l + End Sub + + Public Sub New(i As Integer, j As Integer) + End Sub + + Public Sub New(i As Integer, j As Integer, k As Integer) + End Sub +End Class", index:=0) + End Function + + + + Public Async Function TestMultipleConstructors_SecondOfThree() As Task + Await TestInRegularAndScriptAsync( +"Class Program + Private [|l|] As Integer + + Public Sub New(i As Integer) + Me.i = i + End Sub + + Public Sub New(i As Integer, j As Integer) + End Sub + + Public Sub New(i As Integer, j As Integer, k As Integer) + End Sub +End Class", +"Class Program + Private [|l|] As Integer + + Public Sub New(i As Integer) + Me.i = i + End Sub + + Public Sub New(i As Integer, j As Integer, l As Integer) + Me.l = l + End Sub + + Public Sub New(i As Integer, j As Integer, k As Integer) + End Sub +End Class", index:=1) + End Function + + + + Public Async Function TestMultipleConstructors_ThirdOfThree() As Task + Await TestInRegularAndScriptAsync( +"Class Program + Private [|l|] As Integer + + Public Sub New(i As Integer) + Me.i = i + End Sub + + Public Sub New(i As Integer, j As Integer) + End Sub + + Public Sub New(i As Integer, j As Integer, k As Integer) + End Sub +End Class", +"Class Program + Private [|l|] As Integer + + Public Sub New(i As Integer) + Me.i = i + End Sub + + Public Sub New(i As Integer, j As Integer) + End Sub + + Public Sub New(i As Integer, j As Integer, k As Integer, l As Integer) + Me.l = l + End Sub +End Class", index:=2) + End Function + + + + Public Async Function TestMultipleConstructors_OneMustBeOptional() As Task + Await TestInRegularAndScriptAsync( +"Class Program + Private [|l|] As Integer + + ' index 0, and 2 as optional + Public Sub New(i As Integer) + Me.i = i + End Sub + + ' index 3 as optional + Public Sub New(Optional j As Double = Nothing) + End Sub + + ' index 1, and 4 as optional + Public Sub New(i As Integer, j As Double) + End Sub +End Class", +"Class Program + Private [|l|] As Integer + + ' index 0, and 2 as optional + Public Sub New(i As Integer) + Me.i = i + End Sub + + ' index 3 as optional + Public Sub New(Optional j As Double = Nothing) + End Sub + + ' index 1, and 4 as optional + Public Sub New(i As Integer, j As Double, l As Integer) + Me.l = l + End Sub +End Class", index:=1) + End Function + + + + Public Async Function TestMultipleConstructors_OneMustBeOptional2() As Task + Await TestInRegularAndScriptAsync( +"Class Program + Private [|l|] As Integer + + ' index 0, and 2 as optional + Public Sub New(i As Integer) + Me.i = i + End Sub + + ' index 3 as optional + Public Sub New(Optional j As Double = Nothing) + End Sub + + ' index 1, and 4 as optional + Public Sub New(i As Integer, j As Double) + End Sub +End Class", +"Class Program + Private [|l|] As Integer + + ' index 0, and 2 as optional + Public Sub New(i As Integer) + Me.i = i + End Sub + + ' index 3 as optional + Public Sub New(Optional j As Double = Nothing, Optional l As Integer = Nothing) + Me.l = l + End Sub + + ' index 1, and 4 as optional + Public Sub New(i As Integer, j As Double) + End Sub +End Class", index:=3) + End Function + + + + Public Async Function TestMultipleConstructors_AllMustBeOptional1() As Task + Await TestInRegularAndScriptAsync( +"Class Program + Private [|p|] As Integer + + Public Sub New(Optional i As Integer = Nothing) + Me.i = i + End Sub + + Public Sub New(j As Double, Optional k As Double = Nothing) + End Sub + + Public Sub New(l As Integer, m As Integer, Optional n As Integer = Nothing) + End Sub +End Class", +"Class Program + Private p As Integer + + Public Sub New(Optional i As Integer = Nothing, Optional p As Integer = Nothing) + Me.i = i + Me.p = p + End Sub + + Public Sub New(j As Double, Optional k As Double = Nothing) + End Sub + + Public Sub New(l As Integer, m As Integer, Optional n As Integer = Nothing) + End Sub +End Class", index:=0) + End Function + + + + Public Async Function TestMultipleConstructors_AllMustBeOptional2() As Task + Await TestInRegularAndScriptAsync( +"Class Program + Private [|p|] As Integer + + Public Sub New(Optional i As Integer = Nothing) + Me.i = i + End Sub + + Public Sub New(j As Double, Optional k As Double = Nothing) + End Sub + + Public Sub New(l As Integer, m As Integer, Optional n As Integer = Nothing) + End Sub +End Class", +"Class Program + Private p As Integer + + Public Sub New(Optional i As Integer = Nothing) + Me.i = i + End Sub + + Public Sub New(j As Double, Optional k As Double = Nothing) + End Sub + + Public Sub New(l As Integer, m As Integer, Optional n As Integer = Nothing, Optional p As Integer = Nothing) + Me.p = p + End Sub +End Class", index:=2) + End Function End Class -End Namespace \ No newline at end of file +End Namespace diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs index 291d0790340d0..0c79421aaa168 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs @@ -23,6 +23,12 @@ private class AddConstructorParametersCodeAction : CodeAction private readonly ConstructorCandidate _constructorCandidate; private readonly ISymbol _containingType; private readonly ImmutableArray _parametersToAdd; + + /// + /// If there is more than one constructor, the suggested actions will be split into two sub menus, + /// one for regular parameters and one for optional. This boolean is used by the Title property + /// to determine if the code action should be given the complete title or the sub menu title + /// private readonly bool _useSubMenuName; public AddConstructorParametersCodeAction( @@ -30,8 +36,7 @@ public AddConstructorParametersCodeAction( ConstructorCandidate constructorCandidate, ISymbol containingType, ImmutableArray parametersToAdd, - bool useSubMenuName - ) + bool useSubMenuName) { _document = document; _constructorCandidate = constructorCandidate; @@ -44,7 +49,7 @@ protected override Task GetChangedDocumentAsync(CancellationToken canc { var workspace = _document.Project.Solution.Workspace; var declarationService = _document.GetLanguageService(); - var constructor = declarationService.GetDeclarations(_constructorCandidate._constructor).Select(r => r.GetSyntax(cancellationToken)).First(); + var constructor = declarationService.GetDeclarations(_constructorCandidate.Constructor).Select(r => r.GetSyntax(cancellationToken)).First(); var newConstructor = constructor; newConstructor = CodeGenerator.AddParameterDeclarations(newConstructor, _parametersToAdd, workspace); @@ -62,7 +67,7 @@ private IEnumerable CreateAssignStatements(ConstructorCandidate cons var factory = _document.GetLanguageService(); for (var i = 0; i < _parametersToAdd.Length; ++i) { - var memberName = constructorCandidate._missingMembers[i].Name; + var memberName = constructorCandidate.MissingMembers[i].Name; var parameterName = _parametersToAdd[i].Name; yield return factory.ExpressionStatement( factory.AssignmentStatement( @@ -75,9 +80,8 @@ public override string Title { get { - var parameters = _constructorCandidate._constructor.Parameters.Select(p => p.ToDisplayString(SimpleFormat)); + var parameters = _constructorCandidate.Constructor.Parameters.Select(p => p.ToDisplayString(SimpleFormat)); var parameterString = string.Join(", ", parameters); - var optional = _parametersToAdd[0].IsOptional; var signature = $"{_containingType}({parameterString})"; var submenu = _useSubMenuName; @@ -87,9 +91,9 @@ public override string Title } else { - return optional - ? string.Format(FeaturesResources.Add_optional_parameters_to_0, signature) - : string.Format(FeaturesResources.Add_parameters_to_0, signature); + return _parametersToAdd[0].IsOptional + ? string.Format(FeaturesResources.Add_optional_parameters_to_0, signature) + : string.Format(FeaturesResources.Add_parameters_to_0, signature); } } } diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.ConstructorCandidate.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.ConstructorCandidate.cs new file mode 100644 index 0000000000000..b8398dd24b72b --- /dev/null +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.ConstructorCandidate.cs @@ -0,0 +1,23 @@ +// 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.Collections.Immutable; + +namespace Microsoft.CodeAnalysis.AddConstructorParametersFromMembers +{ + internal partial class AddConstructorParametersFromMembersCodeRefactoringProvider + { + private readonly struct ConstructorCandidate + { + public readonly IMethodSymbol Constructor; + public readonly ImmutableArray MissingMembers; + public readonly ImmutableArray MissingParameters; + + public ConstructorCandidate(IMethodSymbol constructor, ImmutableArray missingMembers, ImmutableArray missingParameters) + { + Constructor = constructor; + MissingMembers = missingMembers; + MissingParameters = missingParameters; + } + } + } +} diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs index ce262cbf04733..e255525874019 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +// Copyright (c) Microsoft. All Rfights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; using System.Collections.Immutable; @@ -12,19 +12,6 @@ namespace Microsoft.CodeAnalysis.AddConstructorParametersFromMembers { internal partial class AddConstructorParametersFromMembersCodeRefactoringProvider { - internal class ConstructorCandidate - { - internal IMethodSymbol _constructor; - internal ImmutableArray _missingMembers; - internal ImmutableArray _missingParameters; - - public ConstructorCandidate(IMethodSymbol constructor, ImmutableArray missingMembers, ImmutableArray missingParameters) - { - _constructor = constructor; - _missingMembers = missingMembers; - _missingParameters = missingParameters; - } - } private class State { public ImmutableArray ConstructorCandidates { get; private set; } diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs index abc64ab327df0..507ad124ae062 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs @@ -63,9 +63,9 @@ private IEnumerable CreateCodeActions(Document document, State state { // There will be at most 2 suggested code actions, so no need to use sub menus var constructorCandidate = state.ConstructorCandidates[0]; - if (CanHaveRequiredParameters(state.ConstructorCandidates[0]._missingParameters)) + if (CanHaveRequiredParameters(state.ConstructorCandidates[0].MissingParameters)) { - result.Add(new AddConstructorParametersCodeAction(document, constructorCandidate, containingType, constructorCandidate._missingParameters, useSubMenuName: false)); + result.Add(new AddConstructorParametersCodeAction(document, constructorCandidate, containingType, constructorCandidate.MissingParameters, useSubMenuName: false)); } result.Add(GetOptionalContructorParametersCodeAction(document, constructorCandidate, containingType, useSubMenuName: false)); } @@ -74,16 +74,20 @@ private IEnumerable CreateCodeActions(Document document, State state // Create sub menus for suggested actions, one for required parameters and one for optional parameters var requiredParameterCodeActions = new ArrayBuilder(); var optionalPrameterCodeActions = new ArrayBuilder(); - foreach (var constructor in state.ConstructorCandidates) + foreach (var constructorCandidate in state.ConstructorCandidates) { - if (CanHaveRequiredParameters(constructor._missingParameters)) + if (CanHaveRequiredParameters(constructorCandidate.Constructor.Parameters)) { - requiredParameterCodeActions.Add(new AddConstructorParametersCodeAction(document, constructor, containingType, constructor._missingParameters, useSubMenuName: true)); + requiredParameterCodeActions.Add(new AddConstructorParametersCodeAction(document, constructorCandidate, containingType, constructorCandidate.MissingParameters, useSubMenuName: true)); } - optionalPrameterCodeActions.Add(GetOptionalContructorParametersCodeAction(document, constructor, containingType, useSubMenuName: true)); + optionalPrameterCodeActions.Add(GetOptionalContructorParametersCodeAction(document, constructorCandidate, containingType, useSubMenuName: true)); + } + + if (requiredParameterCodeActions.Count > 0) + { + result.Add(new CodeAction.CodeActionWithNestedActions(FeaturesResources.Add_parameter_to_constructor, requiredParameterCodeActions.ToImmutableAndFree(), isInlinable: false)); } - result.Add(new CodeAction.CodeActionWithNestedActions(FeaturesResources.Add_parameter_to_constructor, requiredParameterCodeActions.ToImmutableAndFree(), isInlinable: false)); result.Add(new CodeAction.CodeActionWithNestedActions(FeaturesResources.Add_optional_parameter_to_constructor, optionalPrameterCodeActions.ToImmutableAndFree(), isInlinable: false)); } @@ -92,7 +96,7 @@ private IEnumerable CreateCodeActions(Document document, State state // local functions static CodeAction GetOptionalContructorParametersCodeAction(Document document, ConstructorCandidate constructorCandidate, INamedTypeSymbol containingType, bool useSubMenuName) { - var missingOptionalParameters = constructorCandidate._missingParameters.SelectAsArray(p => CodeGenerationSymbolFactory.CreateParameterSymbol( + var missingOptionalParameters = constructorCandidate.MissingParameters.SelectAsArray(p => CodeGenerationSymbolFactory.CreateParameterSymbol( attributes: default, refKind: p.RefKind, isParams: p.IsParams, From 2e64ad90da2c9255480ff75665eb9a39b5b5b71c Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Thu, 14 Mar 2019 14:53:39 -0700 Subject: [PATCH 03/13] Fix spelling/random characters Co-Authored-By: chborl --- ...tructorParametersFromMembersCodeRefactoringProvider.State.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs index e255525874019..b59eca4806a21 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft. All Rfights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +// 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.Immutable; From 73001c9ed082a1bd5cd29d3098f2105ae775c359 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Thu, 14 Mar 2019 14:54:32 -0700 Subject: [PATCH 04/13] Use IsEmpty property Co-Authored-By: chborl --- ...tructorParametersFromMembersCodeRefactoringProvider.State.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs index b59eca4806a21..15c1de4900056 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs @@ -53,7 +53,7 @@ private async Task TryInitializeAsync( // append the new parameters ConstructorCandidates = await GetConstructorCandidatesInfo(ContainingType, parametersForSelectedMembers, selectedMembers, document).ConfigureAwait(false); - if (ConstructorCandidates.Count() == 0) + if (ConstructorCandidates.IsEmpty) { return false; } From 6c86d40e90d5657ab32dfbc274d908f4bb4bd06d Mon Sep 17 00:00:00 2001 From: Cheryl Borley Date: Thu, 14 Mar 2019 16:04:39 -0700 Subject: [PATCH 05/13] remove unneeded null check --- ...ctorParametersFromMembersCodeRefactoringProvider.State.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs index e255525874019..ddb7c39d22a59 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs @@ -113,10 +113,7 @@ private async Task> GetConstructorCandidate } } - if (missingParametersBuilder != null) - { - applicableConstructors.Add(new ConstructorCandidate(constructor, missingMembersBuilder.ToImmutableAndFree(), missingParametersBuilder.ToImmutableAndFree())); - } + applicableConstructors.Add(new ConstructorCandidate(constructor, missingMembersBuilder.ToImmutableAndFree(), missingParametersBuilder.ToImmutableAndFree())); } return applicableConstructors.ToImmutableAndFree(); From 39fc4b5729c63d3dcdc613d820edc6a5a920e224 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Fri, 15 Mar 2019 15:43:48 -0700 Subject: [PATCH 06/13] Configure await false Co-Authored-By: chborl --- ...tructorParametersFromMembersCodeRefactoringProvider.State.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs index d62d5a4777c71..121f36d45f9cc 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs @@ -23,7 +23,7 @@ public static async Task GenerateAsync( Document document) { var state = new State(); - if (!await state.TryInitializeAsync(service, selectedMembers, document).ConfigureAwait(true)) + if (!await state.TryInitializeAsync(service, selectedMembers, document).ConfigureAwait(false)) { return null; } From 1f40165a53b1419440d9c8d098d84f887dd5b843 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Fri, 15 Mar 2019 15:45:14 -0700 Subject: [PATCH 07/13] refactor null check Co-Authored-By: chborl --- ...ddConstructorParametersFromMembersCodeRefactoringProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs index 507ad124ae062..844ef886e9910 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs @@ -45,7 +45,7 @@ public async Task> AddConstructorParametersFromMember if (info != null) { var state = State.GenerateAsync(this, info.SelectedMembers, document).Result; - if (state != null && state.ConstructorCandidates != null) + if (state?.ConstructorCandidates != null) { return CreateCodeActions(document, state).AsImmutableOrNull(); } From a0fada41121dc0ba52a35a2849e859617b22762b Mon Sep 17 00:00:00 2001 From: Cheryl Borley Date: Wed, 20 Mar 2019 16:50:31 -0700 Subject: [PATCH 08/13] Respond to feedback and refactor --- ...ddConstructorParametersFromMembersTests.cs | 82 +++-------- .../AbstractCodeActionOrUserDiagnosticTest.cs | 21 ++- ...ddConstructorParametersFromMembersTests.vb | 26 ++-- ...ider.AddConstructorParametersCodeAction.cs | 23 ++-- ...romMembersCodeRefactoringProvider.State.cs | 127 +++++++++--------- ...etersFromMembersCodeRefactoringProvider.cs | 30 ++--- 6 files changed, 132 insertions(+), 177 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs b/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs index 71917950698a9..aa2e19200b66f 100644 --- a/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs +++ b/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs @@ -23,6 +23,7 @@ protected override ImmutableArray MassageActions(ImmutableArray FlattenActions(actions); [WorkItem(308077, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/308077")] + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] public async Task TestAdd1() { @@ -51,10 +52,11 @@ public Program(int i, string s) this.i = i; this.s = s; } -}"); +}", title: string.Format(FeaturesResources.Add_parameters_to_0, "Program(int)")); } [WorkItem(308077, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/308077")] + [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] public async Task TestAddOptional1() { @@ -83,8 +85,7 @@ public Program(int i, string s = null) this.i = i; this.s = s; } -}", -index: 1); +}", index: 1, title: string.Format(FeaturesResources.Add_optional_parameters_to_0, "Program(int)")); } [WorkItem(308077, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/308077")] @@ -114,52 +115,6 @@ public Program(int i, string s) : this(i) }", @"using System.Collections.Generic; -class Program -{ - int i; - string s; - bool b; - - public Program(int i, string s, bool b) - { - this.i = i; - this.s = s; - this.b = b; - } - - public Program(int i, string s) : this(i) - { - this.s = s; - } -}", index: 0); - } - - [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] - public async Task TestAddToConstructorWithMostMatchingParameters2() - { - // behavior change with 33603, now all constructors offered - await TestInRegularAndScriptAsync( -@"using System.Collections.Generic; - -class Program -{ - [|int i; - string s; - bool b;|] - - public Program(int i) - { - this.i = i; - } - - public Program(int i, string s) : this(i) - { - this.s = s; - } -}", -@"using System.Collections.Generic; - class Program { int i; @@ -176,7 +131,7 @@ public Program(int i, string s, bool b) : this(i) this.s = s; this.b = b; } -}", index: 1); +}", index: 1, title: string.Format(FeaturesResources.Add_to_0, "Program(int, string)")); } [WorkItem(308077, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/308077")] @@ -217,13 +172,12 @@ public Program(int i) this.i = i; } - public Program(int i, string s, bool b) : this(i) + public Program(int i, string s, bool b = false) : this(i) { this.s = s; this.b = b; } -}", -index: 1); +}", index: 3, title: string.Format(FeaturesResources.Add_to_0, "Program(int, string)")); } [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] @@ -860,7 +814,7 @@ public C(int i, int j, int k) { } }"; - await TestInRegularAndScriptAsync(source, expected, index: 0); + await TestInRegularAndScriptAsync(source, expected, index: 0, title: string.Format(FeaturesResources.Add_to_0, "C(int)")); } [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] @@ -898,7 +852,7 @@ public C(int i, int j, int k) { } }"; - await TestInRegularAndScriptAsync(source, expected, index: 1); + await TestInRegularAndScriptAsync(source, expected, index: 1, title: string.Format(FeaturesResources.Add_to_0, "C(int, int)")); } [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] @@ -937,7 +891,7 @@ public C(int i, int j, int k, int l) this.l = l; } }"; - await TestInRegularAndScriptAsync(source, expected, index: 2); + await TestInRegularAndScriptAsync(source, expected, index: 2, title: string.Format(FeaturesResources.Add_to_0, "C(int, int, int)")); } [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] @@ -975,12 +929,12 @@ public C(int i, int j, int k) { } }"; - await TestInRegularAndScriptAsync(source, expected, 3); + await TestInRegularAndScriptAsync(source, expected, index: 3, title: string.Format(FeaturesResources.Add_to_0, "C(int)")); } [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)] - public async Task TestMultipleConstructorsSecondOptionalOfThree() + public async Task TestMultipleConstructors_SecondOptionalOfThree() { var source = @" @@ -1013,7 +967,7 @@ public C(int i, int j, int k) { } }"; - await TestInRegularAndScriptAsync(source, expected, index: 4); + await TestInRegularAndScriptAsync(source, expected, index: 4, title: string.Format(FeaturesResources.Add_to_0, "C(int, int)")); } [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] @@ -1051,7 +1005,7 @@ public C(int i, int j, int k, int l = 0) this.l = l; } }"; - await TestInRegularAndScriptAsync(source, expected, index: 5); + await TestInRegularAndScriptAsync(source, expected, index: 5, title: string.Format(FeaturesResources.Add_to_0, "C(int, int, int)")); } [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] @@ -1101,7 +1055,7 @@ public C(int i, double j, int k, int l) this.l = l; } }"; - await TestInRegularAndScriptAsync(source, expected, index: 1); + await TestInRegularAndScriptAsync(source, expected, index: 1, title: string.Format(FeaturesResources.Add_to_0, "C(int, double, int)")); } [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] @@ -1151,7 +1105,7 @@ public C(int i, double j, int k) { } }"; - await TestInRegularAndScriptAsync(source, expected, index: 3); + await TestInRegularAndScriptAsync(source, expected, index: 3, title: string.Format(FeaturesResources.Add_to_0, "C(int, double)")); } [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] @@ -1189,7 +1143,7 @@ public C(int l, double m, int n = 0) { } }"; - await TestInRegularAndScriptAsync(source, expected, index: 0); + await TestInRegularAndScriptAsync(source, expected, index: 0, title: string.Format(FeaturesResources.Add_to_0, "C(int)")); } [WorkItem(33603, "https://github.com/dotnet/roslyn/issues/33603")] @@ -1227,7 +1181,7 @@ public C(int l, double m, int n = 0, int p = 0) this.p = p; } }"; - await TestInRegularAndScriptAsync(source, expected, index: 2); + await TestInRegularAndScriptAsync(source, expected, index: 2, title: string.Format(FeaturesResources.Add_to_0, "C(int, double, int)")); } [WorkItem(33623, "https://github.com/dotnet/roslyn/issues/33623")] diff --git a/src/EditorFeatures/TestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs b/src/EditorFeatures/TestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs index 3e44cbb8b626a..0020db53d5bf4 100644 --- a/src/EditorFeatures/TestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs +++ b/src/EditorFeatures/TestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs @@ -37,6 +37,7 @@ public struct TestParameters internal readonly int index; internal readonly CodeActionPriority? priority; internal readonly bool retainNonFixableDiagnostics; + internal readonly string title; internal TestParameters( ParseOptions parseOptions = null, @@ -45,7 +46,8 @@ internal TestParameters( object fixProviderData = null, int index = 0, CodeActionPriority? priority = null, - bool retainNonFixableDiagnostics = false) + bool retainNonFixableDiagnostics = false, + string title = null) { this.parseOptions = parseOptions; this.compilationOptions = compilationOptions; @@ -54,16 +56,17 @@ internal TestParameters( this.index = index; this.priority = priority; this.retainNonFixableDiagnostics = retainNonFixableDiagnostics; + this.title = title; } public TestParameters WithParseOptions(ParseOptions parseOptions) - => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority); + => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, title: title); public TestParameters WithFixProviderData(object fixProviderData) - => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority); + => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, title: title); public TestParameters WithIndex(int index) - => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority); + => new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, title: title); } protected abstract string GetLanguage(); @@ -323,11 +326,12 @@ internal Task TestInRegularAndScriptAsync( CompilationOptions compilationOptions = null, IDictionary options = null, object fixProviderData = null, - ParseOptions parseOptions = null) + ParseOptions parseOptions = null, + string title = null) { return TestInRegularAndScript1Async( initialMarkup, expectedMarkup, index, priority, - new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index)); + new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, title: title)); } internal async Task TestInRegularAndScript1Async( @@ -524,6 +528,11 @@ internal static Task> VerifyActionAndGetOper Assert.Equal(parameters.priority.Value, action.Priority); } + if (parameters.title != null) + { + Assert.Equal(parameters.title, action.Title); + } + return action.GetOperationsAsync(CancellationToken.None); } diff --git a/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb b/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb index 0225a90399671..2fc08b7041678 100644 --- a/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb +++ b/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb @@ -19,6 +19,7 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.AddConstructorPara End Function + Public Async Function TestAdd1() As Task Await TestInRegularAndScriptAsync( @@ -36,10 +37,11 @@ End Class", Me.i = i Me.s = s End Sub -End Class") +End Class", title:=String.Format(FeaturesResources.Add_parameters_to_0, "Program(Integer)")) End Function + Public Async Function TestAddOptional1() As Task Await TestInRegularAndScriptAsync( @@ -57,8 +59,7 @@ End Class", Me.i = i Me.s = s End Sub -End Class", -index:=1) +End Class", index:=1, title:=String.Format(FeaturesResources.Add_optional_parameters_to_0, "Program(Integer)")) End Function @@ -91,7 +92,7 @@ End Class", Me.s = s Me.b = b End Sub -End Class", index:=1) +End Class", index:=1, title:=String.Format(FeaturesResources.Add_to_0, "Program(Integer, String)")) End Function @@ -124,8 +125,7 @@ End Class", Me.s = s Me.b = b End Sub -End Class", -index:=3) +End Class", index:=3, title:=String.Format(FeaturesResources.Add_to_0, "Program(Integer, String)")) End Function @@ -345,7 +345,7 @@ End Class", Public Sub New(i As Integer, j As Integer, k As Integer) End Sub -End Class", index:=0) +End Class", index:=0, title:=String.Format(FeaturesResources.Add_to_0, "Program(Integer)")) End Function @@ -378,7 +378,7 @@ End Class", Public Sub New(i As Integer, j As Integer, k As Integer) End Sub -End Class", index:=1) +End Class", index:=1, title:=String.Format(FeaturesResources.Add_to_0, "Program(Integer, Integer)")) End Function @@ -411,7 +411,7 @@ End Class", Public Sub New(i As Integer, j As Integer, k As Integer, l As Integer) Me.l = l End Sub -End Class", index:=2) +End Class", index:=2, title:=String.Format(FeaturesResources.Add_to_0, "Program(Integer, Integer, Integer)")) End Function @@ -450,7 +450,7 @@ End Class", Public Sub New(i As Integer, j As Double, l As Integer) Me.l = l End Sub -End Class", index:=1) +End Class", index:=1, title:=String.Format(FeaturesResources.Add_to_0, "Program(Integer, Double)")) End Function @@ -489,7 +489,7 @@ End Class", ' index 1, and 4 as optional Public Sub New(i As Integer, j As Double) End Sub -End Class", index:=3) +End Class", index:=3, title:=String.Format(FeaturesResources.Add_to_0, "Program(Double)")) End Function @@ -522,7 +522,7 @@ End Class", Public Sub New(l As Integer, m As Integer, Optional n As Integer = Nothing) End Sub -End Class", index:=0) +End Class", index:=0, title:=String.Format(FeaturesResources.Add_to_0, "Program(Integer)")) End Function @@ -555,7 +555,7 @@ End Class", Public Sub New(l As Integer, m As Integer, Optional n As Integer = Nothing, Optional p As Integer = Nothing) Me.p = p End Sub -End Class", index:=2) +End Class", index:=2, title:=String.Format(FeaturesResources.Add_to_0, "Program(Integer, Integer, Integer)")) End Function End Class End Namespace diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs index 0c79421aaa168..a7f464fc47992 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs @@ -22,11 +22,11 @@ private class AddConstructorParametersCodeAction : CodeAction private readonly Document _document; private readonly ConstructorCandidate _constructorCandidate; private readonly ISymbol _containingType; - private readonly ImmutableArray _parametersToAdd; + private readonly ImmutableArray _missingParameters; /// - /// If there is more than one constructor, the suggested actions will be split into two sub menus, - /// one for regular parameters and one for optional. This boolean is used by the Title property + /// If there is more than one constructor, the suggested actions will be split into two sub menus, + /// one for regular parameters and one for optional. This boolean is used by the Title property /// to determine if the code action should be given the complete title or the sub menu title /// private readonly bool _useSubMenuName; @@ -35,13 +35,13 @@ public AddConstructorParametersCodeAction( Document document, ConstructorCandidate constructorCandidate, ISymbol containingType, - ImmutableArray parametersToAdd, + ImmutableArray missingParameters, bool useSubMenuName) { _document = document; _constructorCandidate = constructorCandidate; _containingType = containingType; - _parametersToAdd = parametersToAdd; + _missingParameters = missingParameters; _useSubMenuName = useSubMenuName; } @@ -52,7 +52,7 @@ protected override Task GetChangedDocumentAsync(CancellationToken canc var constructor = declarationService.GetDeclarations(_constructorCandidate.Constructor).Select(r => r.GetSyntax(cancellationToken)).First(); var newConstructor = constructor; - newConstructor = CodeGenerator.AddParameterDeclarations(newConstructor, _parametersToAdd, workspace); + newConstructor = CodeGenerator.AddParameterDeclarations(newConstructor, _missingParameters, workspace); newConstructor = CodeGenerator.AddStatements(newConstructor, CreateAssignStatements(_constructorCandidate), workspace) .WithAdditionalAnnotations(Formatter.Annotation); @@ -65,10 +65,10 @@ protected override Task GetChangedDocumentAsync(CancellationToken canc private IEnumerable CreateAssignStatements(ConstructorCandidate constructorCandidate) { var factory = _document.GetLanguageService(); - for (var i = 0; i < _parametersToAdd.Length; ++i) + for (var i = 0; i < _missingParameters.Length; ++i) { var memberName = constructorCandidate.MissingMembers[i].Name; - var parameterName = _parametersToAdd[i].Name; + var parameterName = _missingParameters[i].Name; yield return factory.ExpressionStatement( factory.AssignmentStatement( factory.MemberAccessExpression(factory.ThisExpression(), factory.IdentifierName(memberName)), @@ -82,16 +82,15 @@ public override string Title { var parameters = _constructorCandidate.Constructor.Parameters.Select(p => p.ToDisplayString(SimpleFormat)); var parameterString = string.Join(", ", parameters); - var signature = $"{_containingType}({parameterString})"; - var submenu = _useSubMenuName; + var signature = $"{_containingType.Name}({parameterString})"; - if (submenu) + if (_useSubMenuName) { return string.Format(FeaturesResources.Add_to_0, signature); } else { - return _parametersToAdd[0].IsOptional + return _missingParameters[0].IsOptional ? string.Format(FeaturesResources.Add_optional_parameters_to_0, signature) : string.Format(FeaturesResources.Add_parameters_to_0, signature); } diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs index 121f36d45f9cc..4960f53ffe4af 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs @@ -1,8 +1,8 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +// 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.Immutable; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.PooledObjects; @@ -20,10 +20,12 @@ private class State public static async Task GenerateAsync( AddConstructorParametersFromMembersCodeRefactoringProvider service, ImmutableArray selectedMembers, - Document document) + Document document, + CancellationToken cancellationToken) { var state = new State(); - if (!await state.TryInitializeAsync(service, selectedMembers, document).ConfigureAwait(false)) + if (!await state.TryInitializeAsync( + service, selectedMembers, document, cancellationToken).ConfigureAwait(false)) { return null; } @@ -34,104 +36,95 @@ public static async Task GenerateAsync( private async Task TryInitializeAsync( AddConstructorParametersFromMembersCodeRefactoringProvider service, ImmutableArray selectedMembers, - Document document) + Document document, + CancellationToken cancellationToken) { - if (!selectedMembers.All(IsWritableInstanceFieldOrProperty)) - { - return false; - } - ContainingType = selectedMembers[0].ContainingType; - if (ContainingType == null || ContainingType.TypeKind == TypeKind.Interface) + if (!selectedMembers.All(IsWritableInstanceFieldOrProperty) || + ContainingType == null || + ContainingType.TypeKind == TypeKind.Interface) { return false; } - var parametersForSelectedMembers = service.DetermineParameters(selectedMembers); - // We are trying to add these parameters into an existing constructor's parameter list. - // Comparing parameters based on names to make sure parameter list won't contains duplicate parameters after we - // append the new parameters - ConstructorCandidates = await GetConstructorCandidatesInfo(ContainingType, parametersForSelectedMembers, selectedMembers, document).ConfigureAwait(false); - - if (ConstructorCandidates.IsEmpty) - { - return false; - } + ConstructorCandidates = await GetConstructorCandidatesInfoAsync( + ContainingType, service, selectedMembers, document, cancellationToken).ConfigureAwait(false); - return true; + return !ConstructorCandidates.IsEmpty; } /// - /// Try to find all constructors in whose parameters is the subset of by comparing name. - /// These constructors will not be considered as potential candidates + /// Try to find all constructors in whose parameters + /// are a subset of by comparing name. + /// These constructors will not be considered as potential candidates: /// - if the constructor's parameter list contains 'ref' or 'params' /// - any constructor that has a params[] parameter /// - deserialization constructor /// - implicit default constructor /// - private async Task> GetConstructorCandidatesInfo( + private async Task> GetConstructorCandidatesInfoAsync( INamedTypeSymbol containingType, - ImmutableArray parametersForSelectedMembers, + AddConstructorParametersFromMembersCodeRefactoringProvider service, ImmutableArray selectedMembers, - Document document) + Document document, + CancellationToken cancellationToken) { - var parameterNamesForSelectedMembers = parametersForSelectedMembers.SelectAsArray(p => p.Name); + var parametersForSelectedMembers = service.DetermineParameters(selectedMembers); var applicableConstructors = ArrayBuilder.GetInstance(); - var constructors = containingType.InstanceConstructors; - foreach (var constructor in constructors) - { - var constructorParams = constructor.Parameters; - if (constructorParams.Length == 2) + foreach (var constructor in containingType.InstanceConstructors) + { + if (await IsApplicableConstructorAsync( + constructor, document, parametersForSelectedMembers.SelectAsArray(p => p.Name), cancellationToken).ConfigureAwait(false)) { - var compilation = await document.Project.GetCompilationAsync().ConfigureAwait(false); - var deserializationConstructorCheck = new DeserializationConstructorCheck(compilation); - if (deserializationConstructorCheck.IsDeserializationConstructor(constructor)) - { - continue; - } + applicableConstructors.Add(CreateConstructorCandidate(parametersForSelectedMembers, selectedMembers, constructor)); } + } - if (!constructorParams.All(parameter => parameter.RefKind == RefKind.None) || - (constructorParams.Length == 0 && constructor.IsImplicitlyDeclared) || - constructorParams.Any(p => p.IsParams) || - SelectedMembersAlreadyExistAsParameters(parameterNamesForSelectedMembers, constructorParams)) - { - continue; - } + return applicableConstructors.ToImmutableAndFree(); + } + + private static async Task IsApplicableConstructorAsync(IMethodSymbol constructor, Document document, ImmutableArray parameterNamesForSelectedMembers, CancellationToken cancellationToken) + { + var constructorParams = constructor.Parameters; - var missingParametersBuilder = ArrayBuilder.GetInstance(); - var missingMembersBuilder = ArrayBuilder.GetInstance(); - var constructorParamNames = constructor.Parameters.SelectAsArray(p => p.Name); - var zippedParametersAndSelectedMembers = parametersForSelectedMembers.Zip(selectedMembers, (parameter, selectedMember) => (parameter, selectedMember)); - foreach ((var parameter, var selectedMember) in zippedParametersAndSelectedMembers) + if (constructorParams.Length == 2) + { + var compilation = await document.Project.GetCompilationAsync(cancellationToken).ConfigureAwait(false); + var deserializationConstructorCheck = new DeserializationConstructorCheck(compilation); + if (deserializationConstructorCheck.IsDeserializationConstructor(constructor)) { - if (!constructorParamNames.Contains(parameter.Name)) - { - missingParametersBuilder.Add(parameter); - missingMembersBuilder.Add(selectedMember); - } + return false; } - - applicableConstructors.Add(new ConstructorCandidate(constructor, missingMembersBuilder.ToImmutableAndFree(), missingParametersBuilder.ToImmutableAndFree())); } - return applicableConstructors.ToImmutableAndFree(); + return constructorParams.All(parameter => parameter.RefKind == RefKind.None) && + !constructor.IsImplicitlyDeclared && + !constructorParams.Any(p => p.IsParams) && + !SelectedMembersAlreadyExistAsParameters(parameterNamesForSelectedMembers, constructorParams); } private static bool SelectedMembersAlreadyExistAsParameters(ImmutableArray parameterNamesForSelectedMembers, ImmutableArray constructorParams) - { - if (constructorParams.Length == 0) - { - return false; - } + => constructorParams.Length != 0 && !parameterNamesForSelectedMembers.Except(constructorParams.Select(p => p.Name)).Any(); - if (parameterNamesForSelectedMembers.Except(constructorParams.Select(p => p.Name)).Any()) + private static ConstructorCandidate CreateConstructorCandidate(ImmutableArray parametersForSelectedMembers, ImmutableArray selectedMembers, IMethodSymbol constructor) + { + var missingParametersBuilder = ArrayBuilder.GetInstance(); + var missingMembersBuilder = ArrayBuilder.GetInstance(); + var constructorParamNames = constructor.Parameters.SelectAsArray(p => p.Name); + var zippedParametersAndSelectedMembers = + parametersForSelectedMembers.Zip(selectedMembers, (parameter, selectedMember) => (parameter, selectedMember)); + foreach (var (parameter, selectedMember) in zippedParametersAndSelectedMembers) { - return false; + if (!constructorParamNames.Contains(parameter.Name)) + { + missingParametersBuilder.Add(parameter); + missingMembersBuilder.Add(selectedMember); + } } - return true; + return new ConstructorCandidate( + constructor, missingMembersBuilder.ToImmutableAndFree(), missingParametersBuilder.ToImmutableAndFree()); } } } diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs index 844ef886e9910..776d6ac7679ec 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs @@ -41,13 +41,13 @@ public async Task> AddConstructorParametersFromMember { using (Logger.LogBlock(FunctionId.Refactoring_GenerateFromMembers_AddConstructorParametersFromMembers, cancellationToken)) { - var info = await GetSelectedMemberInfoAsync(document, textSpan, true, cancellationToken).ConfigureAwait(false); + var info = await GetSelectedMemberInfoAsync(document, textSpan, allowPartialSelection: true, cancellationToken).ConfigureAwait(false); if (info != null) { - var state = State.GenerateAsync(this, info.SelectedMembers, document).Result; - if (state?.ConstructorCandidates != null) + var state = await State.GenerateAsync(this, info.SelectedMembers, document, cancellationToken).ConfigureAwait(false); + if (state?.ConstructorCandidates != null && !state.ConstructorCandidates.IsEmpty) { - return CreateCodeActions(document, state).AsImmutableOrNull(); + return CreateCodeActions(document, state); } } @@ -55,11 +55,11 @@ public async Task> AddConstructorParametersFromMember } } - private IEnumerable CreateCodeActions(Document document, State state) + private ImmutableArray CreateCodeActions(Document document, State state) { - var result = new ArrayBuilder(); + var result = ArrayBuilder.GetInstance(); var containingType = state.ContainingType; - if (state.ConstructorCandidates.Count() <= 1) + if (state.ConstructorCandidates.Length == 1) { // There will be at most 2 suggested code actions, so no need to use sub menus var constructorCandidate = state.ConstructorCandidates[0]; @@ -72,15 +72,15 @@ private IEnumerable CreateCodeActions(Document document, State state else { // Create sub menus for suggested actions, one for required parameters and one for optional parameters - var requiredParameterCodeActions = new ArrayBuilder(); - var optionalPrameterCodeActions = new ArrayBuilder(); + var requiredParameterCodeActions = ArrayBuilder.GetInstance(); + var optionalParameterCodeActions = ArrayBuilder.GetInstance(); foreach (var constructorCandidate in state.ConstructorCandidates) { if (CanHaveRequiredParameters(constructorCandidate.Constructor.Parameters)) { requiredParameterCodeActions.Add(new AddConstructorParametersCodeAction(document, constructorCandidate, containingType, constructorCandidate.MissingParameters, useSubMenuName: true)); } - optionalPrameterCodeActions.Add(GetOptionalContructorParametersCodeAction(document, constructorCandidate, containingType, useSubMenuName: true)); + optionalParameterCodeActions.Add(GetOptionalContructorParametersCodeAction(document, constructorCandidate, containingType, useSubMenuName: true)); } if (requiredParameterCodeActions.Count > 0) @@ -88,12 +88,15 @@ private IEnumerable CreateCodeActions(Document document, State state result.Add(new CodeAction.CodeActionWithNestedActions(FeaturesResources.Add_parameter_to_constructor, requiredParameterCodeActions.ToImmutableAndFree(), isInlinable: false)); } - result.Add(new CodeAction.CodeActionWithNestedActions(FeaturesResources.Add_optional_parameter_to_constructor, optionalPrameterCodeActions.ToImmutableAndFree(), isInlinable: false)); + result.Add(new CodeAction.CodeActionWithNestedActions(FeaturesResources.Add_optional_parameter_to_constructor, optionalParameterCodeActions.ToImmutableAndFree(), isInlinable: false)); } - return result; + return result.AsImmutableOrNull(); // local functions + static bool CanHaveRequiredParameters(ImmutableArray parameters) + => parameters.Length == 0 || !parameters.Last().IsOptional; + static CodeAction GetOptionalContructorParametersCodeAction(Document document, ConstructorCandidate constructorCandidate, INamedTypeSymbol containingType, bool useSubMenuName) { var missingOptionalParameters = constructorCandidate.MissingParameters.SelectAsArray(p => CodeGenerationSymbolFactory.CreateParameterSymbol( @@ -107,9 +110,6 @@ static CodeAction GetOptionalContructorParametersCodeAction(Document document, C return new AddConstructorParametersCodeAction(document, constructorCandidate, containingType, missingOptionalParameters, useSubMenuName); } - - static bool CanHaveRequiredParameters(ImmutableArray parameters) - => parameters.Length == 0 || (parameters.Length > 0 && !parameters.Last().IsOptional); } } } From 96b9f4e80cd22c058a8b21c0e64a9696913d892f Mon Sep 17 00:00:00 2001 From: Cheryl Borley Date: Wed, 20 Mar 2019 17:11:00 -0700 Subject: [PATCH 09/13] Wrap long lines --- ...ider.AddConstructorParametersCodeAction.cs | 3 +- ...romMembersCodeRefactoringProvider.State.cs | 8 ++- ...etersFromMembersCodeRefactoringProvider.cs | 63 ++++++++++++++----- 3 files changed, 54 insertions(+), 20 deletions(-) diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs index a7f464fc47992..3384d7843bf0d 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.AddConstructorParametersCodeAction.cs @@ -49,7 +49,8 @@ protected override Task GetChangedDocumentAsync(CancellationToken canc { var workspace = _document.Project.Solution.Workspace; var declarationService = _document.GetLanguageService(); - var constructor = declarationService.GetDeclarations(_constructorCandidate.Constructor).Select(r => r.GetSyntax(cancellationToken)).First(); + var constructor = declarationService.GetDeclarations( + _constructorCandidate.Constructor).Select(r => r.GetSyntax(cancellationToken)).First(); var newConstructor = constructor; newConstructor = CodeGenerator.AddParameterDeclarations(newConstructor, _missingParameters, workspace); diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs index 4960f53ffe4af..68ce9aa183882 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.State.cs @@ -55,7 +55,7 @@ private async Task TryInitializeAsync( /// /// Try to find all constructors in whose parameters - /// are a subset of by comparing name. + /// are a subset of the selected members by comparing name. /// These constructors will not be considered as potential candidates: /// - if the constructor's parameter list contains 'ref' or 'params' /// - any constructor that has a params[] parameter @@ -105,15 +105,17 @@ private static async Task IsApplicableConstructorAsync(IMethodSymbol const } private static bool SelectedMembersAlreadyExistAsParameters(ImmutableArray parameterNamesForSelectedMembers, ImmutableArray constructorParams) - => constructorParams.Length != 0 && !parameterNamesForSelectedMembers.Except(constructorParams.Select(p => p.Name)).Any(); + => constructorParams.Length != 0 && + !parameterNamesForSelectedMembers.Except(constructorParams.Select(p => p.Name)).Any(); private static ConstructorCandidate CreateConstructorCandidate(ImmutableArray parametersForSelectedMembers, ImmutableArray selectedMembers, IMethodSymbol constructor) { var missingParametersBuilder = ArrayBuilder.GetInstance(); var missingMembersBuilder = ArrayBuilder.GetInstance(); var constructorParamNames = constructor.Parameters.SelectAsArray(p => p.Name); - var zippedParametersAndSelectedMembers = + var zippedParametersAndSelectedMembers = parametersForSelectedMembers.Zip(selectedMembers, (parameter, selectedMember) => (parameter, selectedMember)); + foreach (var (parameter, selectedMember) in zippedParametersAndSelectedMembers) { if (!constructorParamNames.Contains(parameter.Name)) diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs index 776d6ac7679ec..30c8b81a12371 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs @@ -41,7 +41,12 @@ public async Task> AddConstructorParametersFromMember { using (Logger.LogBlock(FunctionId.Refactoring_GenerateFromMembers_AddConstructorParametersFromMembers, cancellationToken)) { - var info = await GetSelectedMemberInfoAsync(document, textSpan, allowPartialSelection: true, cancellationToken).ConfigureAwait(false); + var info = await GetSelectedMemberInfoAsync( + document, + textSpan, + allowPartialSelection: true, + cancellationToken).ConfigureAwait(false); + if (info != null) { var state = await State.GenerateAsync(this, info.SelectedMembers, document, cancellationToken).ConfigureAwait(false); @@ -65,9 +70,18 @@ private ImmutableArray CreateCodeActions(Document document, State st var constructorCandidate = state.ConstructorCandidates[0]; if (CanHaveRequiredParameters(state.ConstructorCandidates[0].MissingParameters)) { - result.Add(new AddConstructorParametersCodeAction(document, constructorCandidate, containingType, constructorCandidate.MissingParameters, useSubMenuName: false)); + result.Add(new AddConstructorParametersCodeAction( + document, + constructorCandidate, + containingType, + constructorCandidate.MissingParameters, + useSubMenuName: false)); } - result.Add(GetOptionalContructorParametersCodeAction(document, constructorCandidate, containingType, useSubMenuName: false)); + result.Add(GetOptionalContructorParametersCodeAction( + document, + constructorCandidate, + containingType, + useSubMenuName: false)); } else { @@ -78,17 +92,32 @@ private ImmutableArray CreateCodeActions(Document document, State st { if (CanHaveRequiredParameters(constructorCandidate.Constructor.Parameters)) { - requiredParameterCodeActions.Add(new AddConstructorParametersCodeAction(document, constructorCandidate, containingType, constructorCandidate.MissingParameters, useSubMenuName: true)); + requiredParameterCodeActions.Add(new AddConstructorParametersCodeAction( + document, + constructorCandidate, + containingType, + constructorCandidate.MissingParameters, + useSubMenuName: true)); } - optionalParameterCodeActions.Add(GetOptionalContructorParametersCodeAction(document, constructorCandidate, containingType, useSubMenuName: true)); + optionalParameterCodeActions.Add(GetOptionalContructorParametersCodeAction( + document, + constructorCandidate, + containingType, + useSubMenuName: true)); } if (requiredParameterCodeActions.Count > 0) { - result.Add(new CodeAction.CodeActionWithNestedActions(FeaturesResources.Add_parameter_to_constructor, requiredParameterCodeActions.ToImmutableAndFree(), isInlinable: false)); + result.Add(new CodeAction.CodeActionWithNestedActions( + FeaturesResources.Add_parameter_to_constructor, + requiredParameterCodeActions.ToImmutableAndFree(), + isInlinable: false)); } - result.Add(new CodeAction.CodeActionWithNestedActions(FeaturesResources.Add_optional_parameter_to_constructor, optionalParameterCodeActions.ToImmutableAndFree(), isInlinable: false)); + result.Add(new CodeAction.CodeActionWithNestedActions( + FeaturesResources.Add_optional_parameter_to_constructor, + optionalParameterCodeActions.ToImmutableAndFree(), + isInlinable: false)); } return result.AsImmutableOrNull(); @@ -99,16 +128,18 @@ static bool CanHaveRequiredParameters(ImmutableArray parameter static CodeAction GetOptionalContructorParametersCodeAction(Document document, ConstructorCandidate constructorCandidate, INamedTypeSymbol containingType, bool useSubMenuName) { - var missingOptionalParameters = constructorCandidate.MissingParameters.SelectAsArray(p => CodeGenerationSymbolFactory.CreateParameterSymbol( - attributes: default, - refKind: p.RefKind, - isParams: p.IsParams, - type: p.Type, - name: p.Name, - isOptional: true, - hasDefaultValue: true)); + var missingOptionalParameters = constructorCandidate.MissingParameters.SelectAsArray( + p => CodeGenerationSymbolFactory.CreateParameterSymbol( + attributes: default, + refKind: p.RefKind, + isParams: p.IsParams, + type: p.Type, + name: p.Name, + isOptional: true, + hasDefaultValue: true)); - return new AddConstructorParametersCodeAction(document, constructorCandidate, containingType, missingOptionalParameters, useSubMenuName); + return new AddConstructorParametersCodeAction( + document, constructorCandidate, containingType, missingOptionalParameters, useSubMenuName); } } } From 362e4ec5b4dae73332a4466edc0345696aed67db Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Thu, 21 Mar 2019 10:33:33 -0700 Subject: [PATCH 10/13] Update test comments Co-Authored-By: chborl --- .../AddConstructorParametersFromMembersTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs b/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs index aa2e19200b66f..5a54daf91119b 100644 --- a/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs +++ b/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs @@ -1018,7 +1018,8 @@ class C { int [|l|]; - // index 0, and 2 as optional + // index 0 as required + // index 2 as optional public C(int i) { } From 5653782392b6b76dbed0ac81410c56c56d768a0f Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Thu, 21 Mar 2019 10:34:46 -0700 Subject: [PATCH 11/13] Change .AsImmutableOrNull to .ToImmutableAndFree Co-Authored-By: chborl --- ...ddConstructorParametersFromMembersCodeRefactoringProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs index 30c8b81a12371..f37d917c34ee1 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs @@ -120,7 +120,7 @@ private ImmutableArray CreateCodeActions(Document document, State st isInlinable: false)); } - return result.AsImmutableOrNull(); + return result.ToImmutableAndFree(); // local functions static bool CanHaveRequiredParameters(ImmutableArray parameters) From 8a95222c0f60e00591eba2772ff5bad30354dd4a Mon Sep 17 00:00:00 2001 From: Cheryl Borley Date: Mon, 25 Mar 2019 07:17:54 -0700 Subject: [PATCH 12/13] Fix test doc comments --- .../AddConstructorParametersFromMembersTests.cs | 9 ++++++--- .../AddConstructorParametersFromMembersTests.vb | 12 ++++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs b/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs index 5a54daf91119b..9f5ebf5920a23 100644 --- a/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs +++ b/src/EditorFeatures/CSharpTest/GenerateFromMembers/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.cs @@ -1029,7 +1029,8 @@ public C(int i, double j = 0) { } - // index 1, and 4 as optional + // index 1 as required + // index 4 as optional public C(int i, double j, int k) { } @@ -1040,7 +1041,8 @@ class C { int [|l|]; - // index 0, and 2 as optional + // index 0 as required + // index 2 as optional public C(int i) { } @@ -1050,7 +1052,8 @@ public C(int i, double j = 0) { } - // index 1, and 4 as optional + // index 1 as required + // index 4 as optional public C(int i, double j, int k, int l) { this.l = l; diff --git a/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb b/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb index 2fc08b7041678..3b730ff0187f1 100644 --- a/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb +++ b/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb @@ -421,7 +421,8 @@ End Class", index:=2, title:=String.Format(FeaturesResources.Add_to_0, "Program( "Class Program Private [|l|] As Integer - ' index 0, and 2 as optional + ' index 0 as regular + ' index 2 as optional Public Sub New(i As Integer) Me.i = i End Sub @@ -430,14 +431,16 @@ End Class", index:=2, title:=String.Format(FeaturesResources.Add_to_0, "Program( Public Sub New(Optional j As Double = Nothing) End Sub - ' index 1, and 4 as optional + ' index 1 as regular + ' index 4 as optional Public Sub New(i As Integer, j As Double) End Sub End Class", "Class Program Private [|l|] As Integer - ' index 0, and 2 as optional + ' index 0 as regular + ' index 2 as optional Public Sub New(i As Integer) Me.i = i End Sub @@ -446,7 +449,8 @@ End Class", Public Sub New(Optional j As Double = Nothing) End Sub - ' index 1, and 4 as optional + ' index 1 as regular + ' index 4 as optional Public Sub New(i As Integer, j As Double, l As Integer) Me.l = l End Sub From 3c9c22e56c5104548fafb88ac11ad2f8c54ac609 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Mon, 25 Mar 2019 09:00:17 -0700 Subject: [PATCH 13/13] Apply suggestions from code review Update doc comment Co-Authored-By: chborl --- .../AddConstructorParametersFromMembersTests.vb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb b/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb index 3b730ff0187f1..14371e68c67d7 100644 --- a/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb +++ b/src/EditorFeatures/VisualBasicTest/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersTests.vb @@ -421,7 +421,7 @@ End Class", index:=2, title:=String.Format(FeaturesResources.Add_to_0, "Program( "Class Program Private [|l|] As Integer - ' index 0 as regular + ' index 0 as required ' index 2 as optional Public Sub New(i As Integer) Me.i = i @@ -431,7 +431,7 @@ End Class", index:=2, title:=String.Format(FeaturesResources.Add_to_0, "Program( Public Sub New(Optional j As Double = Nothing) End Sub - ' index 1 as regular + ' index 1 as required ' index 4 as optional Public Sub New(i As Integer, j As Double) End Sub @@ -439,7 +439,7 @@ End Class", "Class Program Private [|l|] As Integer - ' index 0 as regular + ' index 0 as required ' index 2 as optional Public Sub New(i As Integer) Me.i = i @@ -449,7 +449,7 @@ End Class", Public Sub New(Optional j As Double = Nothing) End Sub - ' index 1 as regular + ' index 1 as required ' index 4 as optional Public Sub New(i As Integer, j As Double, l As Integer) Me.l = l