Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 5, 2025

Fix: "Replace with method" refactoring for partial properties

Problem

When using "Replace property with method" refactoring on a partial property:

  • The partial modifier was not preserved on generated methods
  • Both declaration and implementation parts were generated with bodies, causing compilation errors (CS0161 and CS0111)

Solution

This PR fixes the issue by:

  1. Updating DeclarationModifiers.From to correctly detect partial methods, properties, and events
  2. Modifying the method generation logic to create declaration-only methods for partial definitions
  3. Adding comprehensive test coverage

Changes Made

1. DeclarationModifiers.From (src/Workspaces/Core/Portable/Editing/DeclarationModifiers.cs)

// A method, property, or event is partial if it's a partial definition, has a partial definition part, or has a partial implementation part
var isPartial = (method?.IsPartialDefinition == true || method?.PartialDefinitionPart != null || method?.PartialImplementationPart != null) ||
               (property?.IsPartialDefinition == true || property?.PartialDefinitionPart != null || property?.PartialImplementationPart != null) ||
               (@event?.IsPartialDefinition == true || @event?.PartialDefinitionPart != null || @event?.PartialImplementationPart != null);
  • Detects partial symbols by checking IsPartialDefinition, PartialDefinitionPart, and PartialImplementationPart
  • Now includes support for partial events in addition to methods and properties
  • Ensures the partial modifier is added to both declaration and implementation methods

2. CSharpReplacePropertyWithMethodsService (src/Features/CSharp/Portable/ReplacePropertyWithMethods/CSharpReplacePropertyWithMethodsService.cs)

  • Added early return in GetGetMethodWorker when getMethod.IsPartialDefinition is true
  • Added early return in GetSetMethodWorker when setMethod.IsPartialDefinition is true
  • These early returns prevent body generation for partial definition methods

3. Test Coverage (src/Features/CSharpTest/ReplacePropertyWithMethods/ReplacePropertyWithMethodsTests.cs)

  • TestPartialPropertyWithImplementation - Verifies getter-only partial property transformation
  • TestPartialPropertyWithBody - Verifies partial property with getter and setter transformation

Example Transformation

Before:

partial class C
{
    public partial int P { get; }
}

partial class C
{
    public partial int P => 0;
}

After (Expected):

partial class C
{
    public partial int GetP();  // ✅ partial modifier, no body
}

partial class C
{
    public partial int GetP()  // ✅ partial modifier, with body
    {
        return 0;
    }
}

Testing

  • Built successfully without errors
  • Added unit tests for partial property scenarios
  • Addressed code review feedback to include events and all partial checks

Fixes #81068

Original prompt

This section details on the original issue you should resolve

<issue_title>"Replace with method" on partial property generates two implementation parts and does not add partial modifier</issue_title>
<issue_description>Version Used: VS 18.0.0 Insiders

Steps to Reproduce:
On the first P symbol, open the lightbulb menu and select "Replace 'P' with method".

partial class C
{
    public partial int P { get; }
}

partial class C
{
    public partial int P => 0;
}

Expected Behavior:

partial class C
{
    public partial int GetP(); // <-- partial modifier; declaration part without body
}

partial class C
{
    public partial int GetP() // <-- partial modifier
    {
        return 0;
    }
}

Actual Behavior:

partial class C
{
    public int GetP() // CS0161: 'C.GetP()': not all code paths return a value
    {
    }
}

partial class C
{
    public int GetP() // CS0111: Type 'C' already defines a member called 'GetP' with the same parameter types
    {
        return 0;
    }
}

</issue_description>

<agent_instructions>@copilot Update 'replace with method' to preserve 'partial' from teh property to the method. If we had a partial-definition property (no actual body) we should get the same sort of partial-def method. And similarly if we started with the partial-impl. The information should be available on the symbols to do the right thing.</agent_instructions>

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.

…ctoring

Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
… parts

Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix partial property method generation issue Fix "Replace with method" refactoring for partial properties Nov 5, 2025
Copilot finished work on behalf of CyrusNajmabadi November 5, 2025 22:27
Copilot AI requested a review from CyrusNajmabadi November 5, 2025 22:27

// A method or property is partial if it's either a partial definition or has a partial definition part
var isPartial = (method?.IsPartialDefinition == true || method?.PartialDefinitionPart != null) ||
(property?.IsPartialDefinition == true || property?.PartialDefinitionPart != null);
Copy link
Member

Choose a reason for hiding this comment

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

@copilot check for events as well. Also, i think you need to check for any of PartialDefinitionPart, PartialImplementationPart or IsPartialDefinition

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Added event support and now checking IsPartialDefinition, PartialDefinitionPart, and PartialImplementationPart for all three symbol types (methods, properties, and events). See commit f266974.

…ection

Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
Copilot AI requested a review from CyrusNajmabadi November 5, 2025 22:40
Copilot finished work on behalf of CyrusNajmabadi November 5, 2025 22:40
@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review November 6, 2025 00:37
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner November 6, 2025 00:37
@CyrusNajmabadi
Copy link
Member

Superceded by #81067

@CyrusNajmabadi CyrusNajmabadi deleted the copilot/fix-partial-class-implementation branch November 6, 2025 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Replace with method" on partial property generates two implementation parts and does not add partial modifier

2 participants