Skip to content

Fix duplicate parameter names in introduce parameter refactoring#81404

Merged
CyrusNajmabadi merged 6 commits intomainfrom
copilot/fix-duplicate-parameters
Nov 25, 2025
Merged

Fix duplicate parameter names in introduce parameter refactoring#81404
CyrusNajmabadi merged 6 commits intomainfrom
copilot/fix-duplicate-parameters

Conversation

Copy link
Contributor

Copilot AI commented Nov 21, 2025 •
edited
Loading

Fix "Introduce parameter" duplicate parameter names

This PR fixes an issue where the "Introduce parameter" refactoring could generate duplicate or ambiguous parameter names when the generated name conflicts with an existing parameter.

Problem:
When using the "Introduce parameter" refactoring on an expression like v + 1 in a method that already has a parameter named v, the refactoring would generate a duplicate parameter v, leading to compiler errors:

  • CS0100: The parameter name 'v' is a duplicate
  • CS0229: Ambiguity between 'int v' and 'int v'

Solution:
Modified GetNewParameterNameAsync in IntroduceParameterDocumentRewriter.cs to use ISemanticFactsService.GenerateUniqueLocalName to ensure the generated parameter name doesn't conflict with existing parameters. The method now:

  1. Generates a base name for the parameter (from variable declarator or expression) using a conditional expression
  2. Uses GenerateUniqueLocalName which automatically determines conflicts from the expression location
  3. Returns a unique name by appending numerical suffixes (v1, v2, etc.) when needed

Testing:
Added comprehensive tests covering all three code action options:

  • Direct refactoring
  • Trampoline method extraction
  • Method overload generation

Status:

  • Explore the repository and understand the issue
  • Locate the relevant code in IntroduceParameterDocumentRewriter.cs
  • Identify the GetNewParameterNameAsync method that generates parameter names
  • Modify GetNewParameterNameAsync to use ISemanticFactsService.GenerateUniqueLocalName
  • Add comprehensive tests to verify the fix works for all three code action options
  • Build and verify the changes compile successfully
  • Run code review - PASSED with no comments
  • Run CodeQL security check - PASSED with no issues
  • Add WorkItem attributes to all new tests
  • Refactor to use conditional expression and simplified logic

Fixes #81400

Original prompt

This section details on the original issue you should resolve

<issue_title>"Introduce parameter" may generate duplicate/ambiguous parameters</issue_title>
<issue_description>Version Used: VS 26 Insiders [11218.70]

Steps to Reproduce:
Select the marked range, open the lightbulb menu and "Introduce parameter for v+1" -> "into new overload"

class C
{
    int M(int v)
    {
        return $v + 1$;
    }
}

Expected Behavior:
The new parameter name is unambiguous and not a duplicate, e.g. by adding a numerical suffix, like v1.

Actual Behavior:
The new parameter has the same name as the existing parameter, leading to conflicts:

int M(int v, int v) // CS0100: The parameter name 'v' is a duplicate
{
    return v; // CS0229: Ambiguity between 'int v' and 'int v'
}

All three options for "Introduce parameter" have this issue in some way.</issue_description>

Comments on the Issue (you are @copilot in this section)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@CyrusNajmabadi
Fix duplicate parameter names in introduce parameter refactoring
c2e3ae6 
Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix issue with ambiguous parameter names in overloads Fix duplicate parameter names in introduce parameter refactoring Nov 21, 2025
Copilot AI requested a review from CyrusNajmabadi November 21, 2025 19:55
CyrusNajmabadi
CyrusNajmabadi reviewed Nov 24, 2025
View reviewed changes
src/Features/CSharpTest/IntroduceParameter/IntroduceParameterTests.cs Outdated
}
""");

[Fact]
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi Nov 24, 2025

Choose a reason for hiding this comment

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

@copilot add WorkItem("https://github.com/dotnet/roslyn/issues/81400") for all tests you've added.

Copy link
Contributor Author

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

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

Done in ae24b17.

CyrusNajmabadi
CyrusNajmabadi reviewed Nov 24, 2025
View reviewed changes
src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs Outdated
else
{
baseName = _semanticFacts.GenerateNameForExpression(semanticModel, _expression, capitalize: false, cancellationToken);
}
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi Nov 24, 2025

Choose a reason for hiding this comment

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

@copilot change this to a conditional expression like so:

var baseName = ShouldRemove(...)
    ? varDeclName
    : _semanticFacts.GenerateNameForExpression(...);

Copy link
Contributor Author

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

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

Done in cace554.

CyrusNajmabadi
CyrusNajmabadi reviewed Nov 24, 2025
View reviewed changes
src/Features/Core/Portable/IntroduceParameter/IntroduceParameterDocumentRewriter.cs Outdated
_containerMethod,
_containerMethod,
baseName,
existingParameterNames,
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi Nov 24, 2025

Choose a reason for hiding this comment

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

@copilot do you need to pass existingParameterNames along? i think GenerateUniqueName figures it out from the locations you pass in. Also, use GenerateUniqueLocalName. I don't think you need the container if you have the _expression as that's the location we can use to determine the names of things in scope.

Copy link
Contributor Author

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

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

Done in cace554. Changed to use GenerateUniqueLocalName with _expression as the location and container: null, following the pattern used in IntroduceVariable. This automatically determines conflicts without needing to pass existing parameter names.

@CyrusNajmabadi
Add WorkItem attribute to all new tests
ae24b17 
Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
Copilot AI requested a review from CyrusNajmabadi November 24, 2025 11:42
@CyrusNajmabadi
Refactor to use conditional expression and GenerateUniqueLocalName
cace554 
Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review November 25, 2025 15:17
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner November 25, 2025 15:17
@CyrusNajmabadi CyrusNajmabadi merged commit 6adcb60 into main Nov 25, 2025
26 checks passed
@CyrusNajmabadi CyrusNajmabadi deleted the copilot/fix-duplicate-parameters branch November 25, 2025 17:56
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Nov 25, 2025
@dotnet-maestro dotnet-maestro bot mentioned this pull request Nov 26, 2025
Merged
@dotnet-bot dotnet-bot mentioned this pull request Nov 27, 2025
Closed
@dotnet-bot dotnet-bot mentioned this pull request Dec 9, 2025
Closed
@dotnet-maestro dotnet-maestro bot mentioned this pull request Dec 18, 2025
Merged
@davidwengier davidwengier modified the milestones: Next, 18.3 Jan 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akhera99 akhera99 akhera99 approved these changes

@CyrusNajmabadi CyrusNajmabadi Awaiting requested review from CyrusNajmabadi

Assignees

@CyrusNajmabadi CyrusNajmabadi

Copilot code review Copilot

Labels

VSCode

Projects

None yet

Milestone

18.3

Development

Successfully merging this pull request may close these issues.

"Introduce parameter" may generate duplicate/ambiguous parameters

4 participants

@CyrusNajmabadi @akhera99 @davidwengier