Fix 'sync namespace' with file scoped namespaces #55022
Conversation
| } | ||
| else if (baseNamespace is FileScopedNamespaceDeclarationSyntax fileScopedNamespace) | ||
| { | ||
| openingBuilder.AddRange(fileScopedNamespace.SemicolonToken.TrailingTrivia); |
There was a problem hiding this comment.
Can the semicolon have leading trivia here?
There was a problem hiding this comment.
yes, though that's a 1:10000 case :) i'm writing for the 99.9% cases :)
There was a problem hiding this comment.
@CyrusNajmabadi To me personally, I agree this case is very very rare. But since it's a simple change, I'd handle it. Users put comments in weird places 😄
I'd expect someone later reporting the following trivia being lost:
namespace R
/*trivia*/;
There was a problem hiding this comment.
to me it's that i can map where i expect teh trailing trivia to go. Leading trivia is something else entirely.
src/Features/CSharp/Portable/CodeRefactorings/SyncNamespace/CSharpChangeNamespaceService.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
I had already made most of these changes in #54755 😄
But since you're doing a bit more than I did (more tests and fixing move type service), I'll close my PR.
|
Can you mark the PR as "Fixes #54749" |
|
Will do! |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
ping @ryzngard |
|
I was waiting on #55020 to give this a final pass, since some of the changes overlapped. Will take a look later today |
| @@ -83,7 +83,7 @@ void Method() | |||
| { | |||
| }$$ | |||
| } | |||
| ", "Class.Method()", 2); | |||
| ", "Namespace.Class.Method()", 2); | |||
There was a problem hiding this comment.
This is correct. The namepace name shoudl be appended. but it wasn't because the walker wasn't seeing the file-scoped-namespace and was only checking for normal namespaces.
ghost
added this to the 
Fixes #54749
Relates to test plan: #49000