Fix support for "Usings inside namespaces" in various places #55795
Conversation
| @@ -57,7 +57,9 @@ public static async Task<(Document containingDocument, SyntaxAnnotation typeAnno | |||
| CancellationToken cancellationToken) | |||
| { | |||
| var newDocumentId = DocumentId.CreateNewId(projectId, debugName: fileName); | |||
| var solutionWithInterfaceDocument = solution.AddDocument(newDocumentId, fileName, text: "", folders: folders); | |||
| var newDocumentPath = PathUtilities.CombinePaths(PathUtilities.GetDirectoryName(hintDocument.FilePath), fileName); | |||
There was a problem hiding this comment.
This was needed for the new test framework to be happy ¯\_(ツ)_/¯
There was a problem hiding this comment.
This was needed for the new test framework to be happy ¯_(ツ)_/¯
I expect if a test has failed, this most likely indicates a product bug. Does this have any effect to the end-user (ie, bug fix)? If not, I'm curious to know why the test failed.
There was a problem hiding this comment.
I'm going to guess by not setting the filepath the new test framework doesn't alter it to have a file path. Old framework might have (I'd have to dig in to see) based on the folders and filename.
There was a problem hiding this comment.
I'm curious to know why the test failed.
The new test framework didn't apply options correctly without this, because it applies options by synthesizing an .editorconfig file in the right path, but since the new document didn't have a path, the options didn't apply to it.
| // If there aren't any existing imports then make sure we honour the inside namespace preference | ||
| // for using directings if it's set | ||
| if (fallbackNode is null && PlaceImportsInsideNamespaces(options)) | ||
| fallbackNode = contextSpine.OfType<TNamespaceDeclarationSyntax>().FirstOrDefault(); |
There was a problem hiding this comment.
This is the main bit of the fix
| @@ -337,7 +337,7 @@ private static ISymbol GetSymbolsToPullUp(MemberAnalysisResult analysisResult) | |||
| node.GetCurrentNode(newDestination), | |||
| sourceImports, | |||
| generator, | |||
| options.PlaceSystemNamespaceFirst, | |||
There was a problem hiding this comment.
This is the fix for anything using MembersPuller: Nothing ever wrote to this property (and its removed entirely in a subsequent commit)
davidwengier
changed the title
davidwengier
requested review from
CyrusNajmabadi and
a team
and removed request for
a team
ghost
added this to the 
Fixes #55746
Fixes #39786
This also fixes anything that uses the AddImports service, so they all support using directive placement options, and fixes anything that uses the
MemberPullerto respect that, plus the "System Usings First" setting.Commit-by-commit might make sense because there is a fair bit of plumbing changes, but I commented where the important bits were.