Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code cleanup on save while resolving merge conflicts #60818

Closed
vsfeedback opened this issue Apr 18, 2022 · 64 comments
Closed

Code cleanup on save while resolving merge conflicts #60818

vsfeedback opened this issue Apr 18, 2022 · 64 comments
Labels
Area-IDE Bug Developer Community The issue was originally reported on https://developercommunity.visualstudio.com Resolution-External The behavior lies outside the functionality covered by this repository
Milestone

Comments

@vsfeedback
Copy link

vsfeedback commented Apr 18, 2022

This issue has been moved from a ticket on Developer Community.


[severity:It's more difficult to complete my work] [regression] [worked-in:VS2019]
With turned on feature 'Code Cleanup on Save', while resolving conflicts after merging branches, after applying changes all 'using' directives are gone.


Original Comments

Feedback Bot on 4/18/2022, 01:41 AM:

(private comment, text removed)


Original Solutions

(no solutions)

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 18, 2022
@jinujoseph jinujoseph added Bug Investigation Required Developer Community The issue was originally reported on https://developercommunity.visualstudio.com and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 19, 2022
@jinujoseph jinujoseph modified the milestones: Backlog, 17.3 Apr 19, 2022
@kakone
Copy link

kakone commented Jun 17, 2022

Very annoying, we must deactivate "Code Cleanup on Save" before resolving conflicts.

@DavidJohnWilliams
Copy link

I don't know if this is the only time it happens, but I have noticed that if you have conflicts in a project file it causes the project to be unloaded. If you are then reviewing conflicts in a file within that project then it loads the files from outside of the project i.e. with the path to file on disk. Which means the code cleanup does not have context and removes all using statements. This happens even in the diff window not just the merge window.

As a workaround, it may be possible that resolving project conflicts first prevents this from happening.

@MatthieuTruchet-VA
Copy link

Hi,

We have the same behavior.

Any news on this issue ?

@kakone
Copy link

kakone commented Aug 9, 2022

As a workaround, it may be possible that resolving project conflicts first prevents this from happening.

Unfortunately not. I didn't find any workaround apart from deactivating "Code Cleanup on Save".

@AbdullahAlRana
Copy link

I am also facing the same problem, I found code cleanup feature very handy but for this problem I had to unable it.

@alexandresmoraes
Copy link

i'm having the same behavior

@waitwhereami
Copy link

Can confirm this is still a problem as of VS 2022 17.3.4. Very annoying bug.

@arkalyanms arkalyanms modified the milestones: 17.3, 17.4 Oct 3, 2022
@l-e-f
Copy link

l-e-f commented Oct 31, 2022

Confirming this is still an issue

@LiviuD
Copy link

LiviuD commented Nov 5, 2022

Still an issue, quite a big one (VS2022 17.3.6).

@mwasson74
Copy link

Same issue using VS2022 17.4.1

@mblichowski
Copy link

Very annoying

@fredrikbergman
Copy link

Same for me in VS2022 17.4.2.. 🙁

@andreatosato
Copy link

Same here

@cremor
Copy link

cremor commented Dec 6, 2022

Note that this not only removes all using directives, but also breaks the code style of the merged file by ignoring rules defined in the EditorConfig file.

If someone needs steps to reproduce:

  1. Enable Options - Text Editor - Code Cleanup - Run Code Cleanup profile on Save
  2. Make sure that the chosen Code Cleanup profile formats the document and removes unnecessary usings.
  3. Do a Git merge that results in a conflict in a C# file.
  4. Resolve the conflict. Note that the preview of the resulting file in the lower part of the conflict window shows everything as it should be.
  5. Open the file after resolving the conflict.
  6. Note that all usings are removed from the file, which breaks the code.
  7. Also note that the formatting of the file was reset to the Visual Studio settings instead of the settings defined in the EditorConfig file.

@EngWagner
Copy link

Same problem here, in VS 2022 version 17.4.2

@randyshoopman
Copy link

Our team is also seeing this issue. It's a big pain. Please fix. VS 17.4.0

@Eli-Black-Work
Copy link

@JoeRobich Can we get a quick update on this? 🙂 Seems like lots of people are running into this (including myself! 😄)

@nomadedge
Copy link

Visual Studio 17.5.1 the issue is still a thing. It's almost a year now since the issue was created...

@EngWagner
Copy link

This bug is almost having a birthday lol

@Eli-Black-Work
Copy link

I see that sharwell removed JoeRobich and marked this as help wanted. Thanks, @sharwell 🙂

@ashishranjan2011
Copy link

Very annoying bug

@DarkGraySun
Copy link

Please fix the bug. My teammates are already using alternative merge tools, I can't get them back to the Visual Studio standard.
(Simple "Enable Code Cleanup" checkbox only in the merge window that I uncheck?)

@mdesn
Copy link

mdesn commented Apr 20, 2023

Yes, please fix this issue as soon as possible. We are also using an alternative merge tool because of this annoying and major issue.

@waitwhereami
Copy link

1 year later, pretty obvious MS doesn't really care about this bug. Maybe it'll be fixed in VS 2024...

@wokket
Copy link

wokket commented May 5, 2023

I've been tearing my hair out over why merges where losing usings, finding out MS has known about this for over a year and hasn't even responded on the ticket 😡

@21r8390
Copy link

21r8390 commented May 24, 2023

Can please @CyrusNajmabadi @mavasani @sharwell @JoeRobich @Cosifne @genlu @beccamc @jasonmalinowski or someone else have more clarity on why the second most updated bug is still not fixed after a year?

@Eli-Black-Work
Copy link

I feel like part of the problem is that this seems like a VS issue, but it's logged in the Roslyn repo.

@cremor
Copy link

cremor commented May 24, 2023

All Visual Studio bug reports (on the MS Developer Community) were closed as duplicates of this GitHub issue.

@Eli-Black-Work
Copy link

Eli-Black-Work commented May 24, 2023

That's part of what concerns me 😅

@sharwell
Copy link
Member

The IDE just instructs the language service to "run cleanup" with a given profile. The actual changes being made in response to that instruction are defined by the language service, which in this case is Roslyn. We haven't had the chance to investigate what makes a merge situation different from other situations. If someone wanted to investigate, this is the specific block of code which is responsible for removing using directives during this operation:

if (organizeUsingsSet.IsRemoveUnusedImportEnabled &&
document.GetLanguageService<IRemoveUnnecessaryImportsService>() is { } removeUsingsService)
{
using (Logger.LogBlock(FunctionId.CodeCleanup_RemoveUnusedImports, cancellationToken))
{
var formattingOptions = await document.GetSyntaxFormattingOptionsAsync(fallbackOptions, cancellationToken).ConfigureAwait(false);
document = await removeUsingsService.RemoveUnnecessaryImportsAsync(document, formattingOptions, cancellationToken).ConfigureAwait(false);
}
}

@cremor
Copy link

cremor commented May 24, 2023

If someone wanted to investigate, this is the specific block of code which is responsible for removing using directives during this operation

As I've explained in my comment above, this issue is not just about the removal of using statements. So the bug might be somewhere else (or there are multiple bugs).

@Eli-Black-Work
Copy link

Thanks, @sharwell, good to know 🙂

@luccawilli
Copy link

Thank you @sharwell for the code ref.

As far as i can tell, the problem lies in how the cleanup works.
You can see it pretty well in the CSharpUnnecessaryImportsProvider class.
The semantic model gets diagnosed and the resulting diagnostic output will then be filtered for "CS8019" warnings. And then all usings which triggered such a warning will be removed.

Presumably not everything is loaded in the semantic model. Otherwise no warnings would arise.

You can see the same behavior when using the "Compare with unmodified..." command in the git changes panel or in the merge conflict view. If you try to use IntelliSense to find a local class (or some class in a imported namespace) you will be disappointed, nothing will be suggested.

For me there are now two options, first removing of unnecessary imports should be disabled in this "compare/merge views" or secondly the imported namespaces must also be loaded in the "compare/merge views".
But I am unsure if the necessary changes can be made here in the roslyn repository.

@Kraego
Copy link

Kraego commented May 25, 2023

Imho I would go a step further than @luccawilli pointed out in his first suggestion. For me it would be okay to disable the format on save completely in the merge view.

@tim-davies-oneadvanced
Copy link

Please fix it!

@holmes-mike
Copy link

This is becoming increasingly annoying, (i.e. forgetting to restore using prior to a force push and discovering CI build failures).
I'm now having to resort to VS code for rebasing/merge conflict resolution...

@vmelamed
Copy link

vmelamed commented Jul 5, 2023

There are many people here who are experiencing this extremely annoying and counterproductive bug. I wonder if we can collect enough bribe money to have someone fix this 15-month-old bug...?

@olegtk
Copy link
Contributor

olegtk commented Jul 7, 2023

Moved to VS Editor, tracked internally as https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1847852, tracked externally as https://developercommunity.visualstudio.com/t/Code-cleanup-on-save-while-resolving/10015181 (please vote on it).

@sharwell sharwell closed this as not planned Won't fix, can't repro, duplicate, stale Jul 8, 2023
@sharwell sharwell added Resolution-External The behavior lies outside the functionality covered by this repository and removed help wanted The issue is "up for grabs" - add a comment if you are interested in working on it Investigation Required labels Jul 8, 2023
@mwasson74
Copy link

Closed and back to the original Developer Community issue? Can someone please help me understand?

@cremor
Copy link

cremor commented Jul 8, 2023

I assume someone analyzed the bug in detail and noticed that it's not a bug in Roslyn, but in VS itself.

@waitwhereami
Copy link

Bully work, all of us ignored for 15 months because of some good old fashioned buck passing at MS. Couldn't be arsed to verify where the issue really was in 2022 when it was buck-passed to Roslyn, so here we are again.

@sharwell
Copy link
Member

We did a bunch of work last week to consolidate feedback on this issue to the team that now has ownership of it. Please place additional feedback on the tracking issue, as feedback here will not be seen by the team working on a fix for the issue.

@dotnet dotnet locked as too heated and limited conversation to collaborators Jul 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area-IDE Bug Developer Community The issue was originally reported on https://developercommunity.visualstudio.com Resolution-External The behavior lies outside the functionality covered by this repository
Projects
None yet
Development

No branches or pull requests