dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.97k stars 4.03k forks source link

"Unmerged changes" should use merge markers to mark refactoring conflicts across multiple targets #74366

Open davkean opened 3 months ago

davkean commented 3 months ago

Summary

When encounter conflicts across a multi-target conflict, Roslyn should use Git merge markers to mark the conflicts so that the user is aware of the conflict.

Background and Motivation

I did a refactoring in a multi-targeted code yesterday where I turned a method from an instance -> static. This changed a consumption of that method in a completely different class, but ran into some conflicts in some of the targets, making the following change:

/* Unmerged change from project 'Microsoft.VisualStudio.Shell (net472)'
Before:
            if (this.IsBuiltInLocalService(serviceGuid, out Type? serviceType))
After:
            if (Package.IsBuiltInLocalService(serviceGuid, out Type? serviceType))
*/

Because I had existing changes in this file, I did not see this change until after I committed and pushed to my origin. Looking at my tree right now, I see others ran into this and those changes got merged into the tree unbeknownst to them. You can see hundreds of examples of this across Github.

Proposed Feature

I think it would a better experience here if Roslyn choose to use merge markers here to make it painfully obvious that this should be addressed before merging.

Something similar to this:

<<<<<<< Unmerged change from project 'Microsoft.VisualStudio.Shell (net472), before:
            if (this.IsBuiltInLocalService(serviceGuid, out Type? serviceType))
=======
            if (IsBuiltInLocalService(serviceGuid, out Type? serviceType))
>>>>>>> After

Roslyn already automatically recognizes these and shows an error, making a very friendly way of finding these.

CS8300  Merge conflict marker encountered   
CS8300  Merge conflict marker encountered   
CS8300  Merge conflict marker encountered   
arkalyanms commented 2 months ago

Assigning to Sam to load balance, but @CyrusNajmabadi feel free to pull if you feel so inclined.