Use new document formatter to format when a type is moved to a new document #58977
Conversation
| // established for the existing type prior to being moved | ||
| var newDocumentOptions = (DocumentOptionSet)originalOptions.WithChangedOption(new Options.OptionKey(CodeStyleOptions2.RequireAccessibilityModifiers, unformattedDocumentWitMovedType.Project.Language), AccessibilityModifiersRequired.Never); | ||
| var newDocumentFormattingService = unformattedDocumentWitMovedType.GetRequiredLanguageService<INewDocumentFormattingService>(); | ||
| var documentWithMovedType = await newDocumentFormattingService.FormatNewDocumentAsync(unformattedDocumentWitMovedType, newDocumentOptions, SemanticDocument.Document, default).ConfigureAwait(false); |
There was a problem hiding this comment.
Unsure. I think whatever we decide should be same for "extract base" and "extract static" as well, since those are similar features that take existing code and move to a new document.
I believe this is the right approach, but happy to hear Sam's thoughts on the matter.
There was a problem hiding this comment.
For this feature, it's fine to apply formatting as long as there are absolutely no changes (not even whitespace changes) from the first line of the type that moved to its closing }. A test should be added showing that an incorrectly formatted type is not reformatted when it moves to a new file.
There was a problem hiding this comment.
(not even whitespace changes)
This could get tricky with namespace block changes for indentation.
| await TestMoveTypeToNewFileAsync( | ||
| originalCode: @" | ||
| using System; | ||
|
|
||
| class A | ||
| { | ||
| } | ||
|
|
||
| class [|B|] | ||
| { | ||
| }", | ||
| expectedSourceTextAfterRefactoring: @" | ||
| using System; | ||
|
|
||
| class A | ||
| { | ||
| }", | ||
| expectedDocumentName: "B.cs", | ||
| destinationDocumentText: @" | ||
| class B | ||
| { | ||
| }"); |
There was a problem hiding this comment.
The linked issue mentions file headers, this is not tested currently.
It also needs some thought. f we completely removed the file header, then we're regressing a good behavior for many developers. I think a reasonable behavior is to consider whatever exists in file_header_template. If it's not set, then don't add any header.
Fixes #58660