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

Restore case insensitivity with file paths and related doc ids in the workspace #72965

Merged
merged 6 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions src/Features/LanguageServer/ProtocolUnitTests/UriTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,28 +79,43 @@ public async Task TestWorkspaceDocument_WithFileScheme(bool mutatingLspWorkspace
{
var documentFilePath = @"C:\A.cs";
var markup =
@$"<Workspace>
<Project Language=""C#"" Name=""CSProj1"" CommonReferences=""true"" FilePath=""C:\CSProj1.csproj"">
<Document FilePath=""{documentFilePath}"">
public class A
{{
}}
</Document>
</Project>
</Workspace>";
$$"""
<Workspace>
<Project Language="C#" Name="CSProj1" CommonReferences="true" FilePath="C:\CSProj1.csproj">
<Document FilePath="{{documentFilePath}}">
public class A
{
}
</Document>
</Project>
</Workspace>
""";
await using var testLspServer = await CreateXmlTestLspServerAsync(markup, mutatingLspWorkspace);

var workspaceDocument = testLspServer.TestWorkspace.CurrentSolution.Projects.Single().Documents.Single();
var expectedDocumentUri = ProtocolConversions.CreateAbsoluteUri(documentFilePath);

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. This is supported, and is needed by Xaml.
Copy link
Member

@dibarbet dibarbet Apr 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to clarify - its needed by all files (including cs files) - C# devkit is adding them as upper case drive letters (but the URIs from the client are lowercase drive letters).

{
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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ internal readonly record struct StateChange(
/// </summary>
internal sealed partial class SolutionState
{
/// <summary>
/// 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.
/// </summary>
public static readonly StringComparer FilePathComparer = StringComparer.OrdinalIgnoreCase;

// the version of the workspace this solution is from
public int WorkspaceVersion { get; }
public string? WorkspaceKind { get; }
Expand All @@ -47,7 +53,7 @@ internal sealed partial class SolutionState
// holds on data calculated based on the AnalyzerReferences list
private readonly Lazy<HostDiagnosticAnalyzers> _lazyAnalyzers;

private ImmutableDictionary<string, ImmutableArray<DocumentId>> _lazyFilePathToRelatedDocumentIds = ImmutableDictionary<string, ImmutableArray<DocumentId>>.Empty;
private ImmutableDictionary<string, ImmutableArray<DocumentId>> _lazyFilePathToRelatedDocumentIds = ImmutableDictionary<string, ImmutableArray<DocumentId>>.Empty.WithComparers(FilePathComparer);

private SolutionState(
string? workspaceKind,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ namespace Microsoft.CodeAnalysis;
internal sealed class TextDocumentStates<TState>
where TState : TextDocumentState
{
private static readonly ObjectPool<Dictionary<string, OneOrMany<DocumentId>>> s_filePathPool = new(() => new(SolutionState.FilePathComparer));

public static readonly TextDocumentStates<TState> Empty =
new([], ImmutableSortedDictionary.Create<DocumentId, TState>(DocumentIdComparer.Instance), FrozenDictionary<string, OneOrMany<DocumentId>>.Empty);

Expand Down Expand Up @@ -323,7 +325,8 @@ public void AddDocumentIdsWithFilePath(ref TemporaryArray<DocumentId> temporaryA

private FrozenDictionary<string, OneOrMany<DocumentId>> ComputeFilePathToDocumentIds()
{
using var _ = PooledDictionary<string, OneOrMany<DocumentId>>.GetInstance(out var result);
using var pooledDictionary = s_filePathPool.GetPooledObject();
var result = pooledDictionary.Object;

foreach (var (documentId, state) in _map)
{
Expand All @@ -336,6 +339,6 @@ private FrozenDictionary<string, OneOrMany<DocumentId>> ComputeFilePathToDocumen
: OneOrMany.Create(documentId);
}

return result.ToFrozenDictionary();
return result.ToFrozenDictionary(SolutionState.FilePathComparer);
}
}
17 changes: 17 additions & 0 deletions src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
Loading