From e4be335994bda46cb8f673ca8f21b251aebe297e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Nov 2025 19:34:55 +0000 Subject: [PATCH 1/5] Initial plan From c2e3ae61ccb580e5fe6b6e905dc97cbd4cd1e68d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 21 Nov 2025 19:48:16 +0000 Subject: [PATCH 2/5] Fix duplicate parameter names in introduce parameter refactoring Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com> --- .../IntroduceParameterTests.cs | 138 ++++++++++++++++++ .../IntroduceParameterDocumentRewriter.cs | 24 ++- 2 files changed, 158 insertions(+), 4 deletions(-) diff --git a/src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs b/src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs index 497b08aeb9d96..f74681c4ebff3 100644 --- a/src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs +++ b/src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs @@ -1874,4 +1874,142 @@ class C }; } """); + + [Fact] + public Task TestDuplicateParameterName_Refactor() + => TestInRegularAndScriptAsync(""" + using System; + class C + { + int M(int v) + { + return [|v + 1|]; + } + } + """, """ + using System; + class C + { + int M(int v, int v1) + { + } + } + """, index: 0); + + [Fact] + public Task TestDuplicateParameterName_Trampoline() + => TestInRegularAndScriptAsync(""" + using System; + class C + { + int M(int v) + { + return [|v + 1|]; + } + + void Caller() + { + M(5); + } + } + """, """ + using System; + class C + { + int GetV1(int v) + { + return v + 1; + } + + int M(int v, int v1) + { + } + + void Caller() + { + M(5, GetV1(5)); + } + } + """, index: 1); + + [Fact] + public Task TestDuplicateParameterName_Overload() + => TestInRegularAndScriptAsync(""" + using System; + class C + { + int M(int v) + { + return [|v + 1|]; + } + + void Caller() + { + M(5); + } + } + """, """ + using System; + class C + { + int M(int v) + { + return M(v, v + 1); + } + + int M(int v, int v1) + { + } + + void Caller() + { + M(5); + } + } + """, index: 2); + + [Fact] + public Task TestDuplicateParameterName_MultipleConflicts() + => TestInRegularAndScriptAsync(""" + using System; + class C + { + int M(int v, int v1) + { + return [|v + v1 + 1|]; + } + } + """, """ + using System; + class C + { + int M(int v, int v1, int v2) + { + } + } + """, index: 0); + + [Fact] + public Task TestDuplicateParameterName_WithLocalVariable() + => TestInRegularAndScriptAsync(""" + using System; + class C + { + int M(int v) + { + int result = [|v + 1|]; + return result; + } + } + """, """ + using System; + class C + { + int M(int v, int v1) + { + int result = v1; + return result; + } + } + """, index: 0); } diff --git a/src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs b/src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs index 35663b2ee77eb..d4b9cdde54915 100644 --- a/src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs +++ b/src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs @@ -80,17 +80,33 @@ private async Task> MapExpre /// /// Gets the parameter name, if the expression's grandparent is a variable declarator then it just gets the /// local declarations name. Otherwise, it generates a name based on the context of the expression. + /// Ensures the generated name is unique and does not conflict with existing parameters. /// private async Task GetNewParameterNameAsync(CancellationToken cancellationToken) { + var semanticModel = await _originalDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + + string baseName; if (ShouldRemoveVariableDeclaratorContainingExpression(out var varDeclName, out _)) { - return varDeclName; + baseName = varDeclName; + } + else + { + baseName = _semanticFacts.GenerateNameForExpression(semanticModel, _expression, capitalize: false, cancellationToken); } - var semanticModel = await _originalDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - var semanticFacts = _originalDocument.GetRequiredLanguageService(); - return semanticFacts.GenerateNameForExpression(semanticModel, _expression, capitalize: false, cancellationToken); + // Ensure the parameter name doesn't conflict with existing parameters + var existingParameterNames = _methodSymbol.Parameters.Select(p => p.Name); + var uniqueName = _semanticFacts.GenerateUniqueName( + semanticModel, + _containerMethod, + _containerMethod, + baseName, + existingParameterNames, + cancellationToken); + + return uniqueName.ValueText; } /// From ae24b17d2fc8b796a735c93d259e867b9a5fdda8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 11:40:54 +0000 Subject: [PATCH 3/5] Add WorkItem attribute to all new tests Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com> --- .../IntroduceParameter/IntroduceParameterTests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs b/src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs index f74681c4ebff3..a6b689b4f7b6e 100644 --- a/src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs +++ b/src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs @@ -1875,7 +1875,7 @@ class C } """); - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81400")] public Task TestDuplicateParameterName_Refactor() => TestInRegularAndScriptAsync(""" using System; @@ -1896,7 +1896,7 @@ int M(int v, int v1) } """, index: 0); - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81400")] public Task TestDuplicateParameterName_Trampoline() => TestInRegularAndScriptAsync(""" using System; @@ -1932,7 +1932,7 @@ void Caller() } """, index: 1); - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81400")] public Task TestDuplicateParameterName_Overload() => TestInRegularAndScriptAsync(""" using System; @@ -1968,7 +1968,7 @@ void Caller() } """, index: 2); - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81400")] public Task TestDuplicateParameterName_MultipleConflicts() => TestInRegularAndScriptAsync(""" using System; @@ -1989,7 +1989,7 @@ int M(int v, int v1, int v2) } """, index: 0); - [Fact] + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/81400")] public Task TestDuplicateParameterName_WithLocalVariable() => TestInRegularAndScriptAsync(""" using System; From cace55403a1f755a8e8fe891a96bbe4b81383a54 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 11:48:29 +0000 Subject: [PATCH 4/5] Refactor to use conditional expression and GenerateUniqueLocalName Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com> --- .../IntroduceParameterDocumentRewriter.cs | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs b/src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs index d4b9cdde54915..fd931751fcb80 100644 --- a/src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs +++ b/src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs @@ -86,24 +86,16 @@ private async Task GetNewParameterNameAsync(CancellationToken cancellati { var semanticModel = await _originalDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - string baseName; - if (ShouldRemoveVariableDeclaratorContainingExpression(out var varDeclName, out _)) - { - baseName = varDeclName; - } - else - { - baseName = _semanticFacts.GenerateNameForExpression(semanticModel, _expression, capitalize: false, cancellationToken); - } + var baseName = ShouldRemoveVariableDeclaratorContainingExpression(out var varDeclName, out _) + ? varDeclName + : _semanticFacts.GenerateNameForExpression(semanticModel, _expression, capitalize: false, cancellationToken); // Ensure the parameter name doesn't conflict with existing parameters - var existingParameterNames = _methodSymbol.Parameters.Select(p => p.Name); - var uniqueName = _semanticFacts.GenerateUniqueName( + var uniqueName = _semanticFacts.GenerateUniqueLocalName( semanticModel, - _containerMethod, - _containerMethod, + _expression, + container: null, baseName, - existingParameterNames, cancellationToken); return uniqueName.ValueText; From 2d0ec6021117a07336eb720b0d62a313c7569d4b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 25 Nov 2025 16:16:57 +0100 Subject: [PATCH 5/5] Fixup tests --- .../IntroduceParameter/IntroduceParameterTests.cs | 15 +++++++++------ .../IntroduceParameterDocumentRewriter.cs | 8 ++++---- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs b/src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs index a6b689b4f7b6e..091ac44313292 100644 --- a/src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs +++ b/src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs @@ -1774,9 +1774,9 @@ public void M(string s) { localFunction(s); - void localFunction(string s) + void localFunction(string s1) { - _ = {|Rename:s|}.ToString(); + _ = {|Rename:s1|}.ToString(); } } } @@ -1892,6 +1892,7 @@ class C { int M(int v, int v1) { + return {|Rename:v1|}; } } """, index: 0); @@ -1916,13 +1917,14 @@ void Caller() using System; class C { - int GetV1(int v) + private int GetV1(int v) { return v + 1; } int M(int v, int v1) { + return {|Rename:v1|}; } void Caller() @@ -1952,13 +1954,14 @@ void Caller() using System; class C { - int M(int v) + private int M(int v) { return M(v, v + 1); } int M(int v, int v1) { + return {|Rename:v1|}; } void Caller() @@ -1985,6 +1988,7 @@ class C { int M(int v, int v1, int v2) { + return {|Rename:v2|}; } } """, index: 0); @@ -2005,9 +2009,8 @@ int M(int v) using System; class C { - int M(int v, int v1) + int M(int v, int result) { - int result = v1; return result; } } diff --git a/src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs b/src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs index fd931751fcb80..2a7f701219f6a 100644 --- a/src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs +++ b/src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs @@ -84,11 +84,11 @@ private async Task> MapExpre /// private async Task GetNewParameterNameAsync(CancellationToken cancellationToken) { - var semanticModel = await _originalDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + if (ShouldRemoveVariableDeclaratorContainingExpression(out var varDeclName, out _)) + return varDeclName; - var baseName = ShouldRemoveVariableDeclaratorContainingExpression(out var varDeclName, out _) - ? varDeclName - : _semanticFacts.GenerateNameForExpression(semanticModel, _expression, capitalize: false, cancellationToken); + var semanticModel = await _originalDocument.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var baseName = _semanticFacts.GenerateNameForExpression(semanticModel, _expression, capitalize: false, cancellationToken); // Ensure the parameter name doesn't conflict with existing parameters var uniqueName = _semanticFacts.GenerateUniqueLocalName(