From 39b4e71578b249f262cffc3260b55505e1346a77 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 9 Apr 2024 17:33:48 -0700 Subject: [PATCH 1/5] Restore case insensitivity with file paths and related doc ids in the workspace --- .../Core/Portable/Workspace/Solution/SolutionState.cs | 8 +++++++- .../Portable/Workspace/Solution/TextDocumentStates.cs | 7 +++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs index b12b5c203b900..f2836780f7ef4 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs @@ -33,6 +33,12 @@ internal readonly record struct StateChange( /// internal sealed partial class SolutionState { + /// + /// Note: this insensitive comparer is bused on many systems. But we do things this way for copmat with the logic + /// we've had on windows since forever. + /// + public static readonly StringComparer FilePathComparer = StringComparer.OrdinalIgnoreCase; + // the version of the workspace this solution is from public int WorkspaceVersion { get; } public string? WorkspaceKind { get; } @@ -47,7 +53,7 @@ internal sealed partial class SolutionState // holds on data calculated based on the AnalyzerReferences list private readonly Lazy _lazyAnalyzers; - private ImmutableDictionary> _lazyFilePathToRelatedDocumentIds = ImmutableDictionary>.Empty; + private ImmutableDictionary> _lazyFilePathToRelatedDocumentIds = ImmutableDictionary>.Empty.WithComparers(FilePathComparer); private SolutionState( string? workspaceKind, diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/TextDocumentStates.cs b/src/Workspaces/Core/Portable/Workspace/Solution/TextDocumentStates.cs index 535371c95a56a..d1740d26d7500 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/TextDocumentStates.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/TextDocumentStates.cs @@ -25,6 +25,8 @@ namespace Microsoft.CodeAnalysis; internal sealed class TextDocumentStates where TState : TextDocumentState { + private static readonly ObjectPool>> s_filePathPool = new(() => new(SolutionState.FilePathComparer)); + public static readonly TextDocumentStates Empty = new([], ImmutableSortedDictionary.Create(DocumentIdComparer.Instance), FrozenDictionary>.Empty); @@ -323,7 +325,8 @@ public void AddDocumentIdsWithFilePath(ref TemporaryArray temporaryA private FrozenDictionary> ComputeFilePathToDocumentIds() { - using var _ = PooledDictionary>.GetInstance(out var result); + using var pooledDictionary = s_filePathPool.GetPooledObject(); + var result = pooledDictionary.Object; foreach (var (documentId, state) in _map) { @@ -336,6 +339,6 @@ private FrozenDictionary> ComputeFilePathToDocumen : OneOrMany.Create(documentId); } - return result.ToFrozenDictionary(); + return result.ToFrozenDictionary(SolutionState.FilePathComparer); } } From 814ced31e75df136450d006ba8b2c0ac8c0757b6 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 9 Apr 2024 17:38:01 -0700 Subject: [PATCH 2/5] Update src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs --- .../Core/Portable/Workspace/Solution/SolutionState.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs index f2836780f7ef4..4d3f278d7554a 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.cs @@ -34,7 +34,7 @@ internal readonly record struct StateChange( internal sealed partial class SolutionState { /// - /// Note: this insensitive comparer is bused on many systems. But we do things this way for copmat with the logic + /// Note: this insensitive comparer is busted on many systems. But we do things this way for compat with the logic /// we've had on windows since forever. /// public static readonly StringComparer FilePathComparer = StringComparer.OrdinalIgnoreCase; From c5bdcc047f9f9a74702566bdb9784c7428114ca7 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 9 Apr 2024 17:41:11 -0700 Subject: [PATCH 3/5] Add tests --- .../CoreTest/SolutionTests/SolutionTests.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs b/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs index 394b0b7152e85..db2774e7943e2 100644 --- a/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs +++ b/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs @@ -4845,6 +4845,23 @@ public void GetRelatedDocumentsDoesNotReturnOtherTypesOfDocuments() Assert.Single(solution.GetRelatedDocumentIds(regularDocumentId)); } + [Theory, CombinatorialData] + public void GetRelatedDocumentsCaseInsensitive( + [CombinatorialValues("file", "File", "FILE", "FiLe")] string prefix, + [CombinatorialValues("cs", "Cs", "cS", "CS")] string extension) + { + using var workspace = CreateWorkspace(); + + var solution = workspace.CurrentSolution + .AddProject("TestProject1", "TestProject1", LanguageNames.CSharp) + .AddDocument("File.cs", "", filePath: "File.cs").Project.Solution + .AddProject("TestProject2", "TestProject2", LanguageNames.CSharp) + .AddDocument("file.cs", "", filePath: "file.cs").Project.Solution; + + // GetDocumentIdsWithFilePath should return two, since it'll count all types of documents + Assert.Equal(2, solution.GetDocumentIdsWithFilePath($"{prefix}.{extension}").Length); + } + [Fact] public async Task TestFrozenPartialSolution1() { From 231baba8a8759479cf684d5efcfe8ed655fa1603 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 9 Apr 2024 17:47:28 -0700 Subject: [PATCH 4/5] add lsp test --- .../ProtocolUnitTests/UriTests.cs | 45 ++++++++++++------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/Features/LanguageServer/ProtocolUnitTests/UriTests.cs b/src/Features/LanguageServer/ProtocolUnitTests/UriTests.cs index 863040cb99abe..35830fe1c1a3f 100644 --- a/src/Features/LanguageServer/ProtocolUnitTests/UriTests.cs +++ b/src/Features/LanguageServer/ProtocolUnitTests/UriTests.cs @@ -79,15 +79,17 @@ public async Task TestWorkspaceDocument_WithFileScheme(bool mutatingLspWorkspace { var documentFilePath = @"C:\A.cs"; var markup = -@$" - - - public class A - {{ - }} - - -"; + $$""" + + + + public class A + { + } + + + + """; await using var testLspServer = await CreateXmlTestLspServerAsync(markup, mutatingLspWorkspace); var workspaceDocument = testLspServer.TestWorkspace.CurrentSolution.Projects.Single().Documents.Single(); @@ -95,12 +97,25 @@ public class A await testLspServer.OpenDocumentAsync(expectedDocumentUri).ConfigureAwait(false); - // Verify file is added to the misc file workspace. - var (workspace, _, document) = await testLspServer.GetManager().GetLspDocumentInfoAsync(new LSP.TextDocumentIdentifier { Uri = expectedDocumentUri }, CancellationToken.None); - Assert.False(workspace is LspMiscellaneousFilesWorkspace); - AssertEx.NotNull(document); - Assert.Equal(expectedDocumentUri, document.GetURI()); - Assert.Equal(documentFilePath, document.FilePath); + // Verify file is not added to the misc file workspace. + { + var (workspace, _, document) = await testLspServer.GetManager().GetLspDocumentInfoAsync(new LSP.TextDocumentIdentifier { Uri = expectedDocumentUri }, CancellationToken.None); + Assert.False(workspace is LspMiscellaneousFilesWorkspace); + AssertEx.NotNull(document); + Assert.Equal(expectedDocumentUri, document.GetURI()); + Assert.Equal(documentFilePath, document.FilePath); + } + + // Try again, this time with a uri with different case sensitivity + { + var lowercaseUri = ProtocolConversions.CreateAbsoluteUri(documentFilePath.ToLowerInvariant()); + Assert.NotEqual(expectedDocumentUri.AbsolutePath, lowercaseUri.AbsolutePath); + var (workspace, _, document) = await testLspServer.GetManager().GetLspDocumentInfoAsync(new LSP.TextDocumentIdentifier { Uri = lowercaseUri }, CancellationToken.None); + Assert.False(workspace is LspMiscellaneousFilesWorkspace); + AssertEx.NotNull(document); + Assert.Equal(expectedDocumentUri, document.GetURI()); + Assert.Equal(documentFilePath, document.FilePath); + } } [Theory, CombinatorialData] From 16d68d3655478d8f2379e3a5db5a92e262b33d90 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 9 Apr 2024 17:48:01 -0700 Subject: [PATCH 5/5] Add docs --- src/Features/LanguageServer/ProtocolUnitTests/UriTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/LanguageServer/ProtocolUnitTests/UriTests.cs b/src/Features/LanguageServer/ProtocolUnitTests/UriTests.cs index 35830fe1c1a3f..853bae0808727 100644 --- a/src/Features/LanguageServer/ProtocolUnitTests/UriTests.cs +++ b/src/Features/LanguageServer/ProtocolUnitTests/UriTests.cs @@ -106,7 +106,7 @@ public class A Assert.Equal(documentFilePath, document.FilePath); } - // Try again, this time with a uri with different case sensitivity + // Try again, this time with a uri with different case sensitivity. This is supported, and is needed by Xaml. { var lowercaseUri = ProtocolConversions.CreateAbsoluteUri(documentFilePath.ToLowerInvariant()); Assert.NotEqual(expectedDocumentUri.AbsolutePath, lowercaseUri.AbsolutePath);