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

Unify user experience with disparate 'generate constructor' features #76328

Merged
merged 12 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.GenerateConstructors;
using Microsoft.CodeAnalysis.CSharp.GenerateDefaultConstructors;
using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions;
using Microsoft.CodeAnalysis.GenerateDefaultConstructors;
using Microsoft.CodeAnalysis.Test.Utilities;
using Microsoft.CodeAnalysis.Testing;
using Roslyn.Test.Utilities;
Expand All @@ -20,11 +20,11 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.GenerateDefaultConstruc

#if !CODE_STYLE
using VerifyRefactoring = CSharpCodeRefactoringVerifier<
GenerateDefaultConstructorsCodeRefactoringProvider>;
CSharpGenerateConstructorsCodeRefactoringProvider>;
#endif

[UseExportProvider]
public class GenerateDefaultConstructorsTests
public sealed class GenerateDefaultConstructorsTests
{
#if !CODE_STYLE
private static async Task TestRefactoringAsync(string source, string fixedSource, int index = 0)
Expand Down Expand Up @@ -1514,7 +1514,8 @@ protected B(int x)
{
}
}
""");
""",
index: 1);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intentional. because code actions were merged, the 0th is now the one in this group that uses the dialog to pick the membes, while the 1nth is the one that creates a delegating constructor.

}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsGenerateDefaultConstructors)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeGeneration;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Text;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,21 @@
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.GenerateConstructorFromMembers;
using Microsoft.CodeAnalysis.CSharp.GenerateConstructors;
using Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.CodeRefactorings;
using Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics.NamingStyles;
using Microsoft.CodeAnalysis.PickMembers;
using Microsoft.CodeAnalysis.Test.Utilities;
using Roslyn.Test.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.GenerateConstructorFromMembers;
namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.GenerateConstructors;

[Trait(Traits.Feature, Traits.Features.CodeActionsGenerateConstructorFromMembers)]
public sealed class GenerateConstructorFromMembersTests : AbstractCSharpCodeActionTest
public sealed class GenerateConstructorsTests : AbstractCSharpCodeActionTest
{
protected override CodeRefactoringProvider CreateCodeRefactoringProvider(EditorTestWorkspace workspace, TestParameters parameters)
=> new CSharpGenerateConstructorFromMembersCodeRefactoringProvider((IPickMembersService)parameters.fixProviderData);
=> new CSharpGenerateConstructorsCodeRefactoringProvider((IPickMembersService)parameters.fixProviderData);

private readonly NamingStylesTestOptionSets options = new(LanguageNames.CSharp);

Expand Down Expand Up @@ -74,7 +74,7 @@ class Z
public Z(int a{|Navigation:)|} => this.a = a;
}
""",
options: Option(CSharpCodeStyleOptions.PreferExpressionBodiedConstructors, CSharpCodeStyleOptions.WhenPossibleWithSilentEnforcement));
options: Option(CSharpCodeStyleOptions.PreferExpressionBodiedConstructors, CSharpCodeStyleOptions.WhenPossibleWithSilentEnforcement));
}

[Fact]
Expand All @@ -99,7 +99,7 @@ class Z
public Z(int a{|Navigation:)|} => this.a = a;
}
""",
options: Option(CSharpCodeStyleOptions.PreferExpressionBodiedConstructors, CSharpCodeStyleOptions.WhenOnSingleLineWithSilentEnforcement));
options: Option(CSharpCodeStyleOptions.PreferExpressionBodiedConstructors, CSharpCodeStyleOptions.WhenOnSingleLineWithSilentEnforcement));
}

[Fact]
Expand Down Expand Up @@ -130,7 +130,7 @@ public Z(int a, int b{|Navigation:)|}
}
}
""",
options: Option(CSharpCodeStyleOptions.PreferExpressionBodiedConstructors, CSharpCodeStyleOptions.WhenOnSingleLineWithSilentEnforcement));
options: Option(CSharpCodeStyleOptions.PreferExpressionBodiedConstructors, CSharpCodeStyleOptions.WhenOnSingleLineWithSilentEnforcement));
}

[Fact]
Expand Down Expand Up @@ -974,7 +974,7 @@ public Program(int field{|Navigation:)|}
}
}
""",
options: Option(CodeStyleOptions2.QualifyFieldAccess, CodeStyleOption2.TrueWithSuggestionEnforcement));
options: Option(CodeStyleOptions2.QualifyFieldAccess, CodeStyleOption2.TrueWithSuggestionEnforcement));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/13944")]
Expand All @@ -1001,7 +1001,7 @@ protected Contribution(string title, int number{|Navigation:)|}
public int Number { get; }
}
""",
options: Option(CodeStyleOptions2.QualifyFieldAccess, CodeStyleOption2.TrueWithSuggestionEnforcement));
options: Option(CodeStyleOptions2.QualifyFieldAccess, CodeStyleOption2.TrueWithSuggestionEnforcement));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/13944")]
Expand All @@ -1015,7 +1015,7 @@ abstract class Contribution
public int Number { get; }|]
}
""",
new TestParameters(options: Option(CodeStyleOptions2.QualifyFieldAccess, CodeStyleOption2.TrueWithSuggestionEnforcement)));
new TestParameters(options: Option(CodeStyleOptions2.QualifyFieldAccess, CodeStyleOption2.TrueWithSuggestionEnforcement)));
}

[Fact]
Expand Down Expand Up @@ -1044,7 +1044,7 @@ public Z(int a{|Navigation:)|}
}
}
""",
chosenSymbols: ["a"]);
chosenSymbols: ["a"]);
}

[Fact]
Expand Down Expand Up @@ -1072,7 +1072,7 @@ public Z(int a{|Navigation:)|}
}
}
""",
chosenSymbols: ["a"]);
chosenSymbols: ["a"]);
}

[Fact]
Expand Down Expand Up @@ -1115,7 +1115,7 @@ public Z({|Navigation:)|}
}
}
""",
chosenSymbols: []);
chosenSymbols: []);
}

[Fact]
Expand Down Expand Up @@ -1147,7 +1147,7 @@ public Z(string b, int a{|Navigation:)|}
}
}
""",
chosenSymbols: ["b", "a"]);
chosenSymbols: ["b", "a"]);
}

[Fact]
Expand Down Expand Up @@ -1181,8 +1181,8 @@ public Z(int a, string b{|Navigation:)|}
}
}
""",
chosenSymbols: ["a", "b"],
optionsCallback: options => options[0].Value = true);
chosenSymbols: ["a", "b"],
optionsCallback: options => options[0].Value = true);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/41428")]
Expand Down Expand Up @@ -1221,8 +1221,8 @@ public Z(int a, string b, string? c{|Navigation:)|}
}
}
""",
chosenSymbols: ["a", "b", "c"],
optionsCallback: options => options[0].Value = true);
chosenSymbols: ["a", "b", "c"],
optionsCallback: options => options[0].Value = true);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/41428")]
Expand Down Expand Up @@ -1261,8 +1261,8 @@ public Z(int a, string b, T? c{|Navigation:)|}
}
}
""",
chosenSymbols: ["a", "b", "c"],
optionsCallback: options => options[0].Value = true);
chosenSymbols: ["a", "b", "c"],
optionsCallback: options => options[0].Value = true);
}

[Fact]
Expand Down Expand Up @@ -1301,9 +1301,9 @@ public Z(int a, string b{|Navigation:)|}
}
}
""",
chosenSymbols: ["a", "b"],
optionsCallback: options => options[0].Value = true,
parameters: new TestParameters(options:
chosenSymbols: ["a", "b"],
optionsCallback: options => options[0].Value = true,
parameters: new TestParameters(options:
Option(CSharpCodeStyleOptions.PreferThrowExpression, CodeStyleOption2.FalseWithSilentEnforcement)));
}

Expand Down Expand Up @@ -1338,10 +1338,10 @@ public Z(int a, int? b{|Navigation:)|}
}
}
""",
chosenSymbols: ["a", "b"],
optionsCallback: options => options[0].Value = true,
parameters: new TestParameters(options:
Option(CSharpCodeStyleOptions.PreferThrowExpression, CodeStyleOption2.FalseWithSilentEnforcement)));
chosenSymbols: ["a", "b"],
optionsCallback: options => options[0].Value = true,
parameters: new TestParameters(options:
Option(CSharpCodeStyleOptions.PreferThrowExpression, CodeStyleOption2.FalseWithSilentEnforcement)));
}

[Fact]
Expand Down Expand Up @@ -1380,11 +1380,11 @@ public Z(int a, string b{|Navigation:)|}
}
}
""",
chosenSymbols: ["a", "b"],
optionsCallback: options => options[0].Value = true,
parameters: new TestParameters(
parseOptions: CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp6),
options: Option(CSharpCodeStyleOptions.PreferThrowExpression, CodeStyleOption2.FalseWithSilentEnforcement)));
chosenSymbols: ["a", "b"],
optionsCallback: options => options[0].Value = true,
parameters: new TestParameters(
parseOptions: CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp6),
options: Option(CSharpCodeStyleOptions.PreferThrowExpression, CodeStyleOption2.FalseWithSilentEnforcement)));
}

[Fact]
Expand Down Expand Up @@ -1492,7 +1492,7 @@ protected C(int prop{|Navigation:)|}
public int Prop { get; set; }
}
""",
options: Option(CodeStyleOptions2.QualifyFieldAccess, CodeStyleOption2.TrueWithSuggestionEnforcement));
options: Option(CodeStyleOptions2.QualifyFieldAccess, CodeStyleOption2.TrueWithSuggestionEnforcement));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/17643")]
Expand All @@ -1517,7 +1517,7 @@ public Program(int f{|Navigation:)|}
}
}
""",
chosenSymbols: null);
chosenSymbols: null);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/25690")]
Expand All @@ -1544,7 +1544,7 @@ public Program(int p{|Navigation:)|}
}
}
""",
chosenSymbols: null);
chosenSymbols: null);
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsGenerateEqualsAndGetHashCode)]
Expand Down Expand Up @@ -1572,7 +1572,7 @@ public Program(int p, int s{|Navigation:)|}
}
}
""",
chosenSymbols: null);
chosenSymbols: null);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/33601")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,18 @@

Imports Microsoft.CodeAnalysis.CodeRefactorings
Imports Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
Imports Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.CodeRefactorings
Imports Microsoft.CodeAnalysis.GenerateConstructorFromMembers
Imports Microsoft.CodeAnalysis.Options
Imports Microsoft.CodeAnalysis.PickMembers
Imports Microsoft.CodeAnalysis.VisualBasic.GenerateConstructorFromMembers
Imports Microsoft.CodeAnalysis.VisualBasic.GenerateConstructors

Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.GenerateConstructorFromMembers
Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.GenerateConstructors
<Trait(Traits.Feature, Traits.Features.CodeActionsGenerateConstructorFromMembers)>
Public Class GenerateConstructorFromMembersTests
Public NotInheritable Class GenerateConstructorsTests
Inherits AbstractVisualBasicCodeActionTest

Protected Overrides Function CreateCodeRefactoringProvider(workspace As EditorTestWorkspace, parameters As TestParameters) As CodeRefactoringProvider
Return New VisualBasicGenerateConstructorFromMembersCodeRefactoringProvider(DirectCast(parameters.fixProviderData, IPickMembersService))
Return New VisualBasicGenerateConstructorsCodeRefactoringProvider(DirectCast(parameters.fixProviderData, IPickMembersService))
End Function

<Fact>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,33 +10,33 @@
using Microsoft.CodeAnalysis.CodeRefactorings;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.Features.Intents;
using Microsoft.CodeAnalysis.GenerateConstructorFromMembers;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.PickMembers;
using Microsoft.CodeAnalysis.Simplification;
using Microsoft.CodeAnalysis.CSharp.Simplification;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using System.Linq;
using Microsoft.CodeAnalysis.GenerateConstructors;

namespace Microsoft.CodeAnalysis.CSharp.GenerateConstructorFromMembers;
namespace Microsoft.CodeAnalysis.CSharp.GenerateConstructors;

[ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = PredefinedCodeRefactoringProviderNames.GenerateConstructorFromMembers), Shared]
[ExtensionOrder(Before = PredefinedCodeRefactoringProviderNames.GenerateEqualsAndGetHashCodeFromMembers)]
[IntentProvider(WellKnownIntents.GenerateConstructor, LanguageNames.CSharp)]
internal sealed class CSharpGenerateConstructorFromMembersCodeRefactoringProvider
: AbstractGenerateConstructorFromMembersCodeRefactoringProvider
internal sealed class CSharpGenerateConstructorsCodeRefactoringProvider
: AbstractGenerateConstructorsCodeRefactoringProvider
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public CSharpGenerateConstructorFromMembersCodeRefactoringProvider()
public CSharpGenerateConstructorsCodeRefactoringProvider()
{
}

/// <summary>
/// For testing purposes only.
/// </summary>
[SuppressMessage("RoslynDiagnosticsReliability", "RS0034:Exported parts should have [ImportingConstructor]", Justification = "Used incorrectly by tests")]
internal CSharpGenerateConstructorFromMembersCodeRefactoringProvider(IPickMembersService pickMembersService_forTesting)
internal CSharpGenerateConstructorsCodeRefactoringProvider(IPickMembersService pickMembersService_forTesting)
: base(pickMembersService_forTesting)
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@
using Microsoft.CodeAnalysis.CodeGeneration;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.GenerateFromMembers;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.AddConstructorParametersFromMembers;

using static GenerateFromMembersHelpers;

internal sealed partial class AddConstructorParametersFromMembersCodeRefactoringProvider
{
private sealed class AddConstructorParametersCodeAction(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.GenerateFromMembers;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.Utilities;

namespace Microsoft.CodeAnalysis.AddConstructorParametersFromMembers;

using static GenerateFromMembersHelpers;

internal sealed partial class AddConstructorParametersFromMembersCodeRefactoringProvider
{
private sealed class State
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,18 @@

namespace Microsoft.CodeAnalysis.AddConstructorParametersFromMembers;

using static GenerateFromMembersHelpers;

[ExportCodeRefactoringProvider(LanguageNames.CSharp, LanguageNames.VisualBasic,
Name = PredefinedCodeRefactoringProviderNames.AddConstructorParametersFromMembers), Shared]
[ExtensionOrder(After = PredefinedCodeRefactoringProviderNames.GenerateConstructorFromMembers,
Before = PredefinedCodeRefactoringProviderNames.GenerateOverrides)]
[IntentProvider(WellKnownIntents.AddConstructorParameter, LanguageNames.CSharp)]
internal sealed partial class AddConstructorParametersFromMembersCodeRefactoringProvider : AbstractGenerateFromMembersCodeRefactoringProvider, IIntentProvider
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal sealed partial class AddConstructorParametersFromMembersCodeRefactoringProvider()
: CodeRefactoringProvider, IIntentProvider
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public AddConstructorParametersFromMembersCodeRefactoringProvider()
{
}

public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context)
{
var (document, textSpan, cancellationToken) = context;
Expand Down
Loading
Loading