Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Offer add parameter to constructor refactoring for all applicable constructors #34041

Merged
merged 14 commits into from
Mar 26, 2019
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -17,6 +19,9 @@ public class AddConstructorParametersFromMembersTests : AbstractCSharpCodeAction
protected override CodeRefactoringProvider CreateCodeRefactoringProvider(Workspace workspace, TestParameters parameters)
=> new AddConstructorParametersFromMembersCodeRefactoringProvider();

protected override ImmutableArray<CodeAction> MassageActions(ImmutableArray<CodeAction> actions)
=> FlattenActions(actions);

[WorkItem(308077, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/308077")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddConstructorParametersFromMembers)]
public async Task TestAdd1()
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
}");
}
sharwell marked this conversation as resolved.
Show resolved Hide resolved

[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;

Expand Down Expand Up @@ -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)
sharwell marked this conversation as resolved.
Show resolved Hide resolved
{
this.s = s;
this.b = b;
Expand Down Expand Up @@ -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);
sharwell marked this conversation as resolved.
Show resolved Hide resolved
}

[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);
sharwell marked this conversation as resolved.
Show resolved Hide resolved
}

[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)
{
}
}
");
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<IParameterSymbol> _missingParameters;
private readonly ConstructorCandidate _constructorCandidate;
private readonly ISymbol _containingType;
private readonly ImmutableArray<IParameterSymbol> _parametersToAdd;
private readonly bool _useSubMenuName;
sharwell marked this conversation as resolved.
Show resolved Hide resolved

public AddConstructorParametersCodeAction(
AddConstructorParametersFromMembersCodeRefactoringProvider service,
Document document,
State state,
ImmutableArray<IParameterSymbol> missingParameters)
ConstructorCandidate constructorCandidate,
ISymbol containingType,
ImmutableArray<IParameterSymbol> parametersToAdd,
bool useSubMenuName
)
sharwell marked this conversation as resolved.
Show resolved Hide resolved
{
_service = service;
_document = document;
_state = state;
_missingParameters = missingParameters;
_constructorCandidate = constructorCandidate;
_containingType = containingType;
_parametersToAdd = parametersToAdd;
_useSubMenuName = useSubMenuName;
}

protected override Task<Document> GetChangedDocumentAsync(CancellationToken cancellationToken)
{
var workspace = _document.Project.Solution.Workspace;
var declarationService = _document.GetLanguageService<ISymbolDeclarationService>();
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;
Expand All @@ -53,13 +57,13 @@ protected override Task<Document> GetChangedDocumentAsync(CancellationToken canc
return Task.FromResult(_document.WithSyntaxRoot(newRoot));
}

private IEnumerable<SyntaxNode> CreateAssignStatements(State state)
private IEnumerable<SyntaxNode> CreateAssignStatements(ConstructorCandidate constructorCandidate)
{
var factory = _document.GetLanguageService<SyntaxGenerator>();
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)),
Expand All @@ -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;
sharwell marked this conversation as resolved.
Show resolved Hide resolved
var signature = $"{_containingType}({parameterString})";
var submenu = _useSubMenuName;
sharwell marked this conversation as resolved.
Show resolved Hide resolved

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);
sharwell marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}
Expand Down
Loading