Merge pull request #45667 from davidwengier/TupleToStructFixArgumentNameCase

Fix "Tuple to Struct" named parameters being cased incorrectly

src/EditorFeatures/CSharpTest/ConvertTupleToStruct/ConvertTupleToStructTests.cs

@@ -106,6 +106,73 @@ public static implicit operator (int a, int b)(NewStruct value)
             await TestInRegularAndScriptAsync(text, expected, options: GetPreferImplicitTypeOptions(host));
         }
 
+        [WorkItem(45451, "https://github.com/dotnet/roslyn/issues/45451")]
+        [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.CodeActionsConvertTupleToStruct)]
+        public async Task ConvertSingleTupleType_ChangeArgumentNameCase(TestHost host)
+        {
+            var text = @"
+class Test
+{
+    void Method()
+    {
+        var t1 = [||](A: 1, B: 2);
+    }
+}
+";
+            var expected = @"
+class Test
+{
+    void Method()
+    {
+        var t1 = new {|Rename:NewStruct|}(a: 1, b: 2);
+    }
+}
+
+internal struct NewStruct
+{
+    public int A;
+    public int B;
+
+    public NewStruct(int a, int b)
+    {
+        A = a;
+        B = b;
+    }
+
+    public override bool Equals(object obj)
+    {
+        return obj is NewStruct other &&
+               A == other.A &&
+               B == other.B;
+    }
+
+    public override int GetHashCode()
+    {
+        var hashCode = -1817952719;
+        hashCode = hashCode * -1521134295 + A.GetHashCode();
+        hashCode = hashCode * -1521134295 + B.GetHashCode();
+        return hashCode;
+    }
+
+    public void Deconstruct(out int a, out int b)
+    {
+        a = A;
+        b = B;
+    }
+
+    public static implicit operator (int A, int B)(NewStruct value)
+    {
+        return (value.A, value.B);
+    }
+
+    public static implicit operator NewStruct((int A, int B) value)
+    {
+        return new NewStruct(value.A, value.B);
+    }
+}";
+            await TestInRegularAndScriptAsync(text, expected, options: GetPreferImplicitTypeOptions(host));
+        }
+
         [WorkItem(39916, "https://github.com/dotnet/roslyn/issues/39916")]
         [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.CodeActionsConvertTupleToStruct)]
         public async Task ConvertSingleTupleType_Explicit(TestHost host)
@@ -1991,8 +2058,8 @@ void Method()
         var t1 = new {|Rename:NewStruct|}(1, 2);
         var t2 = new NewStruct(1, 2);
         var t3 = (a: 1, b: 2);
-        var t4 = new NewStruct(Item1: 1, Item2: 2);
-        var t5 = new NewStruct(Item1: 1, Item2: 2);
+        var t4 = new NewStruct(item1: 1, item2: 2);
+        var t5 = new NewStruct(item1: 1, item2: 2);
     }
 }
 
@@ -2069,8 +2136,8 @@ void Method()
         var t1 = new NewStruct(1, 2);
         var t2 = new NewStruct(1, 2);
         var t3 = (a: 1, b: 2);
-        var t4 = new {|Rename:NewStruct|}(Item1: 1, Item2: 2);
-        var t5 = new NewStruct(Item1: 1, Item2: 2);
+        var t4 = new {|Rename:NewStruct|}(item1: 1, item2: 2);
+        var t5 = new NewStruct(item1: 1, item2: 2);
     }
 }
 

src/EditorFeatures/VisualBasicTest/ConvertTupleToStruct/ConvertTupleToStructTests.vb

@@ -19,7 +19,7 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.ConvertTupleToStru
     Public Class ConvertTupleToStructTests
         Inherits AbstractVisualBasicCodeActionTest
 
-        Protected Overrides Function CreateCodeRefactoringProvider(Workspace As Workspace, parameters As TestParameters) As CodeRefactoringProvider
+        Protected Overrides Function CreateCodeRefactoringProvider(workspace As Workspace, parameters As TestParameters) As CodeRefactoringProvider
             Return New VisualBasicConvertTupleToStructCodeRefactoringProvider()
         End Function
 
@@ -89,6 +89,63 @@ End Structure
             Await TestInRegularAndScriptAsync(text, expected, options:=GetTestOptions(host))
         End Function
 
+        <WorkItem(45451, "https://github.com/dotnet/roslyn/issues/45451")>
+        <Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.CodeActionsConvertTupleToStruct)>
+        Public Async Function ConvertSingleTupleType_ChangeArgumentNameCase(host As TestHost) As Task
+            Dim text = "
+class Test
+    sub Method()
+        dim t1 = [||](A:=1, B:=2)
+    end sub
+end class
+"
+            Dim expected = "
+class Test
+    sub Method()
+        dim t1 = New {|Rename:NewStruct|}(a:=1, b:=2)
+    end sub
+end class
+
+Friend Structure NewStruct
+    Public A As Integer
+    Public B As Integer
+
+    Public Sub New(a As Integer, b As Integer)
+        Me.A = a
+        Me.B = b
+    End Sub
+
+    Public Overrides Function Equals(obj As Object) As Boolean
+        If Not (TypeOf obj Is NewStruct) Then
+            Return False
+        End If
+
+        Dim other = DirectCast(obj, NewStruct)
+        Return A = other.A AndAlso
+               B = other.B
+    End Function
+
+    Public Overrides Function GetHashCode() As Integer
+        Return (A, B).GetHashCode()
+    End Function
+
+    Public Sub Deconstruct(ByRef a As Integer, ByRef b As Integer)
+        a = Me.A
+        b = Me.B
+    End Sub
+
+    Public Shared Widening Operator CType(value As NewStruct) As (A As Integer, B As Integer)
+        Return (value.A, value.B)
+    End Operator
+
+    Public Shared Widening Operator CType(value As (A As Integer, B As Integer)) As NewStruct
+        Return New NewStruct(value.A, value.B)
+    End Operator
+End Structure
+"
+            Await TestInRegularAndScriptAsync(text, expected, options:=GetTestOptions(host))
+        End Function
+
         <Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.CodeActionsConvertTupleToStruct)>
         Public Async Function ConvertSingleTupleTypeNoNames(host As TestHost) As Task
             Dim text = "
@@ -671,7 +728,7 @@ end class"
 class Test
     sub Method()
         dim t1 = New {|Rename:NewStruct|}(a:=1, b:=2)
-        dim t2 = New NewStruct(A:=3, B:=4)
+        dim t2 = New NewStruct(a:=3, b:=4)
     end sub
 end class
 
@@ -843,10 +900,6 @@ End Structure
             Await TestInRegularAndScriptAsync(text, expected, options:=GetTestOptions(host))
         End Function
 
-        Sub foo(a As Integer, b As Integer)
-
-        End Sub
-
         <Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.CodeActionsConvertTupleToStruct)>
         Public Async Function TestFixAllMatchesInSingleMethod(host As TestHost) As Task
             Dim text = "
@@ -975,8 +1028,8 @@ End Structure
             Await TestInRegularAndScriptAsync(text, expected, options:=GetTestOptions(host))
         End Function
 
-        <Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.CodeActionsConvertTupleToStruct)>
-        Public Async Function NotIfReferencesAnonymousTypeInternally(host As TestHost) As Task
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsConvertTupleToStruct)>
+        Public Async Function NotIfReferencesAnonymousTypeInternally() As Task
             Dim text = "
 class Test
     sub Method()
@@ -1547,7 +1600,7 @@ class Test
         dim t1 = New {|Rename:NewStruct|}(1, 2)
         dim t2 = New NewStruct(1, 2)
         dim t3 = (a:=1, b:=2)
-        dim t4 = New NewStruct(Item1:=1, Item2:=2)
+        dim t4 = New NewStruct(item1:=1, item2:=2)
         dim t5 = New NewStruct(item1:=1, item2:=2)
     end sub
 end class
@@ -1616,7 +1669,7 @@ class Test
         dim t1 = New NewStruct(1, 2)
         dim t2 = New NewStruct(1, 2)
         dim t3 = (a:=1, b:=2)
-        dim t4 = New {|Rename:NewStruct|}(Item1:=1, Item2:=2)
+        dim t4 = New {|Rename:NewStruct|}(item1:=1, item2:=2)
         dim t5 = New NewStruct(item1:=1, item2:=2)
     end sub
 end class
@@ -1829,7 +1882,7 @@ End Structure
                 FeaturesResources.updating_usages_in_containing_member,
                 FeaturesResources.updating_usages_in_containing_type
             })
-            Await TestInRegularAndScriptAsync(text, expected, index:=1)
+            Await TestInRegularAndScriptAsync(text, expected, index:=1, options:=GetTestOptions(host))
         End Function
 
         <Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.CodeActionsConvertTupleToStruct)>
@@ -1906,7 +1959,7 @@ Friend Structure NewStruct
     End Operator
 End Structure
 "
-            Await TestInRegularAndScriptAsync(text, expected, index:=1)
+            Await TestInRegularAndScriptAsync(text, expected, index:=1, options:=GetTestOptions(host))
         End Function
 
         <Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.CodeActionsConvertTupleToStruct)>
@@ -1985,7 +2038,7 @@ Friend Structure NewStruct
     End Operator
 End Structure
 "
-            Await TestInRegularAndScriptAsync(text, expected, index:=1)
+            Await TestInRegularAndScriptAsync(text, expected, index:=1, options:=GetTestOptions(host))
         End Function
 
         <Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.CodeActionsConvertTupleToStruct)>
@@ -2101,7 +2154,7 @@ end class
         </Document>
     </Project>
 </Workspace>"
-            Await TestInRegularAndScriptAsync(text, expected, index:=1)
+            Await TestInRegularAndScriptAsync(text, expected, index:=1, options:=GetTestOptions(host))
         End Function
 
 #End Region
@@ -2225,7 +2278,7 @@ end class
         </Document>
     </Project>
 </Workspace>"
-            Await TestInRegularAndScriptAsync(text, expected, index:=2)
+            Await TestInRegularAndScriptAsync(text, expected, index:=2, options:=GetTestOptions(host))
         End Function
 
 #End Region
@@ -2338,7 +2391,7 @@ end class
         </Document>
     </Project>
 </Workspace>"
-            Await TestInRegularAndScriptAsync(text, expected, index:=3)
+            Await TestInRegularAndScriptAsync(text, expected, index:=3, options:=GetTestOptions(host))
         End Function
 
         <Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.CodeActionsConvertTupleToStruct)>
@@ -2445,7 +2498,7 @@ end class
         </Document>
     </Project>
 </Workspace>"
-            Await TestInRegularAndScriptAsync(text, expected, index:=3)
+            Await TestInRegularAndScriptAsync(text, expected, index:=3, options:=GetTestOptions(host))
         End Function
 
 #End Region

src/Features/CSharp/Portable/ConvertTupleToStruct/CSharpConvertTupleToStructCodeRefactoringProvider.cs

@@ -9,6 +9,8 @@
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Host.Mef;
 
+#nullable enable
+
 namespace Microsoft.CodeAnalysis.CSharp.ConvertTupleToStruct
 {
     [ExtensionOrder(Before = PredefinedCodeRefactoringProviderNames.IntroduceVariable)]
@@ -32,5 +34,19 @@ internal class CSharpConvertTupleToStructCodeRefactoringProvider :
         public CSharpConvertTupleToStructCodeRefactoringProvider()
         {
         }
+
+        protected override ArgumentSyntax GetArgumentWithChangedName(ArgumentSyntax argument, string name)
+            => argument.WithNameColon(ChangeName(argument.NameColon, name));
+
+        private static NameColonSyntax? ChangeName(NameColonSyntax? nameColon, string name)
+        {
+            if (nameColon == null)
+            {
+                return null;
+            }
+
+            var newName = SyntaxFactory.IdentifierName(name).WithTriviaFrom(nameColon.Name);
+            return nameColon.WithName(newName);
+        }
     }
 }

src/Features/Core/Portable/ConvertTupleToStruct/AbstractConvertTupleToStructCodeRefactoringProvider.cs

@@ -53,6 +53,8 @@ internal abstract partial class AbstractConvertTupleToStructCodeRefactoringProvi
         where TTypeBlockSyntax : SyntaxNode
         where TNamespaceDeclarationSyntax : SyntaxNode
     {
+        protected abstract TArgumentSyntax GetArgumentWithChangedName(TArgumentSyntax argument, string name);
+
         public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context)
         {
             var (document, textSpan, cancellationToken) = context;
@@ -203,7 +205,7 @@ private static string GetTitle(Scope scope)
             return document.WithSyntaxRoot(newRoot).Project.Solution;
         }
 
-        private static async Task<Solution> ConvertToStructInCurrentProcessAsync(
+        private async Task<Solution> ConvertToStructInCurrentProcessAsync(
             Document document, TextSpan span, Scope scope, CancellationToken cancellationToken)
         {
             var (tupleExprOrTypeNode, tupleType) = await TryGetTupleInfoAsync(
@@ -259,7 +261,7 @@ private static string GetTitle(Scope scope)
             return updatedSolution;
         }
 
-        private static async Task ReplaceExpressionAndTypesInScopeAsync(
+        private async Task ReplaceExpressionAndTypesInScopeAsync(
             Dictionary<Document, SyntaxEditor> documentToEditorMap,
             ImmutableArray<DocumentToUpdate> documentsToUpdate,
             SyntaxNode tupleExprOrTypeNode, INamedTypeSymbol tupleType,
@@ -305,7 +307,7 @@ private static string GetTitle(Scope scope)
                     // We should only ever get a default array (meaning, update the root), or a
                     // non-empty array.  We should never be asked to update exactly '0' nodes.
                     Debug.Assert(documentToUpdate.NodesToUpdate.IsDefault ||
-                                 documentToUpdate.NodesToUpdate.Length >= 1);
+                                 !documentToUpdate.NodesToUpdate.IsEmpty);
 
                     // If we were given specific nodes to update, only update those.  Otherwise
                     // updated everything from the root down.
@@ -347,7 +349,7 @@ private static string GetTitle(Scope scope)
                 structNameToken = structNameToken.WithAdditionalAnnotations(RenameAnnotation.Create());
             }
 
-            return typeParameters.Length == 0
+            return typeParameters.IsEmpty
                 ? (TNameSyntax)generator.IdentifierName(structNameToken)
                 : (TNameSyntax)generator.GenericName(structNameToken, typeParameters.Select(tp => generator.IdentifierName(tp.Name)));
         }
@@ -546,7 +548,7 @@ private static bool InfoProbablyContainsTupleFieldNames(SyntaxTreeIndex info, Im
             return currentSolution;
         }
 
-        private static async Task<bool> ReplaceTupleExpressionsAndTypesInDocumentAsync(
+        private async Task<bool> ReplaceTupleExpressionsAndTypesInDocumentAsync(
             Document document, SyntaxEditor editor, SyntaxNode startingNode,
             INamedTypeSymbol tupleType, TNameSyntax fullyQualifiedStructName,
             string structName, ImmutableArray<ITypeParameterSymbol> typeParameters,
@@ -566,7 +568,7 @@ private static bool InfoProbablyContainsTupleFieldNames(SyntaxTreeIndex info, Im
             return changed;
         }
 
-        private static async Task<bool> ReplaceMatchingTupleExpressionsAsync(
+        private async Task<bool> ReplaceMatchingTupleExpressionsAsync(
             Document document, SyntaxEditor editor, SyntaxNode startingNode,
             INamedTypeSymbol tupleType, TNameSyntax qualifiedTypeName,
             string typeName, ImmutableArray<ITypeParameterSymbol> typeParameters,
@@ -622,7 +624,7 @@ private static bool AreEquivalent(StringComparer comparer, INamedTypeSymbol tupl
             return true;
         }
 
-        private static void ReplaceWithObjectCreation(
+        private void ReplaceWithObjectCreation(
             SyntaxEditor editor, string typeName, ImmutableArray<ITypeParameterSymbol> typeParameters,
             TNameSyntax qualifiedTypeName, SyntaxNode startingCreationNode, TTupleExpressionSyntax childCreation)
         {
@@ -649,27 +651,31 @@ private static bool AreEquivalent(StringComparer comparer, INamedTypeSymbol tupl
                 });
         }
 
-        private static SeparatedSyntaxList<TArgumentSyntax> ConvertArguments(SyntaxGenerator generator, SeparatedSyntaxList<TArgumentSyntax> arguments)
+        private SeparatedSyntaxList<TArgumentSyntax> ConvertArguments(SyntaxGenerator generator, SeparatedSyntaxList<TArgumentSyntax> arguments)
             => generator.SeparatedList<TArgumentSyntax>(ConvertArguments(generator, arguments.GetWithSeparators()));
 
-        private static SyntaxNodeOrTokenList ConvertArguments(SyntaxGenerator generator, SyntaxNodeOrTokenList list)
+        private SyntaxNodeOrTokenList ConvertArguments(SyntaxGenerator generator, SyntaxNodeOrTokenList list)
             => new SyntaxNodeOrTokenList(list.Select(v => ConvertArgumentOrToken(generator, v)));
 
-        private static SyntaxNodeOrToken ConvertArgumentOrToken(SyntaxGenerator generator, SyntaxNodeOrToken arg)
+        private SyntaxNodeOrToken ConvertArgumentOrToken(SyntaxGenerator generator, SyntaxNodeOrToken arg)
             => arg.IsToken
                 ? arg
                 : ConvertArgument(generator, (TArgumentSyntax)arg.AsNode());
 
-        private static TArgumentSyntax ConvertArgument(
+        private TArgumentSyntax ConvertArgument(
             SyntaxGenerator generator, TArgumentSyntax argument)
         {
-            // Keep named arguments for literal args.  It helps keep the code self-documenting.
+            // If the original arguments had names then we keep them, but convert the case to match the
+            // the constructor parameters they now refer to. It helps keep the code self-documenting.
             // Remove for complex args as it's most likely just clutter a person doesn't need
             // when instantiating their new type.
             var expr = generator.SyntaxFacts.GetExpressionOfArgument(argument);
             if (expr is TLiteralExpressionSyntax)
             {
-                return argument;
+                var argumentName = generator.SyntaxFacts.GetNameForArgument(argument);
+                var newArgumentName = GetConstructorParameterName(argumentName);
+
+                return GetArgumentWithChangedName(argument, newArgumentName);
             }
 
             return (TArgumentSyntax)generator.Argument(expr).WithTriviaFrom(argument);
@@ -800,9 +806,9 @@ private static SyntaxNodeOrToken ConvertArgumentOrToken(SyntaxGenerator generato
             SyntaxGenerator generator, ArrayBuilder<ISymbol> members,
             INamedTypeSymbol tupleType, INamedTypeSymbol structType)
         {
-            const string valueName = "value";
+            const string ValueName = "value";
 
-            var valueNode = generator.IdentifierName(valueName);
+            var valueNode = generator.IdentifierName(ValueName);
             var arguments = tupleType.TupleElements.SelectAsArray<IFieldSymbol, SyntaxNode>(
                 field => generator.Argument(
                     generator.MemberAccessExpression(valueNode, field.Name)));
@@ -818,7 +824,7 @@ private static SyntaxNodeOrToken ConvertArgumentOrToken(SyntaxGenerator generato
                 Accessibility.Public,
                 DeclarationModifiers.Static,
                 tupleType,
-                CodeGenerationSymbolFactory.CreateParameterSymbol(structType, valueName),
+                CodeGenerationSymbolFactory.CreateParameterSymbol(structType, ValueName),
                 isImplicit: true,
                 ImmutableArray.Create(convertToTupleStatement)));
             members.Add(CodeGenerationSymbolFactory.CreateConversionSymbol(
@@ -826,7 +832,7 @@ private static SyntaxNodeOrToken ConvertArgumentOrToken(SyntaxGenerator generato
                 Accessibility.Public,
                 DeclarationModifiers.Static,
                 structType,
-                CodeGenerationSymbolFactory.CreateParameterSymbol(tupleType, valueName),
+                CodeGenerationSymbolFactory.CreateParameterSymbol(tupleType, ValueName),
                 isImplicit: true,
                 ImmutableArray.Create(convertToStructStatement)));
         }
@@ -854,7 +860,7 @@ private static SyntaxNodeOrToken ConvertArgumentOrToken(SyntaxGenerator generato
             var parameters = fields.SelectAsArray<IFieldSymbol, IParameterSymbol>(field =>
             {
                 var parameter = CodeGenerationSymbolFactory.CreateParameterSymbol(
-                    field.Type, field.Name.ToCamelCase(trimLeadingTypePrefix: false));
+                    field.Type, GetConstructorParameterName(field.Name));
 
                 parameterToPropMap[parameter.Name] = field;
 
@@ -872,6 +878,9 @@ private static SyntaxNodeOrToken ConvertArgumentOrToken(SyntaxGenerator generato
             return constructor;
         }
 
+        private static string GetConstructorParameterName(string name)
+            => name.ToCamelCase(trimLeadingTypePrefix: false); // TODO: This is the common case, but should ideally match the users style preference
+
         private class MyCodeAction : CodeAction.SolutionChangeAction
         {
             public MyCodeAction(string title, Func<CancellationToken, Task<Solution>> createChangedSolution)

src/Features/VisualBasic/Portable/ConvertTupleToStruct/VisualBasicConvertTupleToStructCodeRefactoringProvider.vb

@@ -30,5 +30,26 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ConvertTupleToStruct
         <SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification:="Used in test code: https://github.com/dotnet/roslyn/issues/42814")>
         Public Sub New()
         End Sub
+
+        Protected Overrides Function GetArgumentWithChangedName(argument As ArgumentSyntax, name As String) As ArgumentSyntax
+            Dim simpleArgument = TryCast(argument, SimpleArgumentSyntax)
+
+            If simpleArgument Is Nothing Then
+                Return argument
+            End If
+
+            Dim nameColonEquals = simpleArgument.NameColonEquals
+
+            Return simpleArgument.WithNameColonEquals(ChangeName(nameColonEquals, name))
+        End Function
+
+        Private Shared Function ChangeName(nameColonEquals As NameColonEqualsSyntax, name As String) As NameColonEqualsSyntax
+            If nameColonEquals Is Nothing Then
+                Return Nothing
+            End If
+
+            Dim newName = SyntaxFactory.IdentifierName(name).WithTriviaFrom(nameColonEquals.Name)
+            Return nameColonEquals.WithName(newName)
+        End Function
     End Class
 End Namespace