Skip to content

Commit

Permalink
Fix LSP File watching so it correctly reports the baseUri if there's …
Browse files Browse the repository at this point in the history
…a trailing slash (#73203)

Fix LSP File watching so it correctly reports the baseUri if there's a trailing slash
  • Loading branch information
ryzngard authored Apr 26, 2024
1 parent de4d586 commit 3eefb58
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public async Task CreatingDirectoryWatchRequestsDirectoryWatch()

var watcher = GetSingleFileWatcher(dynamicCapabilitiesRpcTarget);

Assert.Equal(tempDirectory.Path + Path.DirectorySeparatorChar, watcher.GlobPattern.BaseUri.LocalPath);
Assert.Equal(tempDirectory.Path, watcher.GlobPattern.BaseUri.LocalPath);
Assert.Equal("**/*", watcher.GlobPattern.Pattern);

// Get rid of the registration and it should be gone again
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ private class FileChangeContext : IFileChangeContext
/// The list of file paths we're watching manually that were outside the directories being watched. The count in this case counts
/// the number of
/// </summary>
private readonly Dictionary<string, int> _watchedFiles = new Dictionary<string, int>(_stringComparer);
private static readonly StringComparer _stringComparer = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? StringComparer.Ordinal : StringComparer.OrdinalIgnoreCase;
private readonly Dictionary<string, int> _watchedFiles = new Dictionary<string, int>(s_stringComparer);
private static readonly StringComparer s_stringComparer = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? StringComparer.Ordinal : StringComparer.OrdinalIgnoreCase;
private static readonly StringComparison s_stringComparison = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? StringComparison.Ordinal : StringComparison.OrdinalIgnoreCase;

public FileChangeContext(ImmutableArray<WatchedDirectory> watchedDirectories, LspFileChangeWatcher lspFileChangeWatcher)
{
Expand All @@ -80,7 +81,7 @@ public FileChangeContext(ImmutableArray<WatchedDirectory> watchedDirectories, Ls
{
GlobPattern = new RelativePattern
{
BaseUri = ProtocolConversions.CreateAbsoluteUri(d.Path),
BaseUri = ProtocolConversions.CreateRelativePatternBaseUri(d.Path),
Pattern = d.ExtensionFilter is not null ? "**/*" + d.ExtensionFilter : "**/*"
}
}).ToArray();
Expand All @@ -99,7 +100,7 @@ private void WatchedFilesHandler_OnNotificationRaised(object? sender, DidChangeW

// Unfortunately the LSP protocol doesn't give us any hint of which of the file watches we might have sent to the client
// was the one that registered for this change, so we have to check paths to see if this one we should respond to.
if (WatchedDirectory.FilePathCoveredByWatchedDirectories(_watchedDirectories, filePath, StringComparison.Ordinal))
if (WatchedDirectory.FilePathCoveredByWatchedDirectories(_watchedDirectories, filePath, s_stringComparison))
{
FileChanged?.Invoke(this, filePath);
}
Expand Down Expand Up @@ -128,7 +129,7 @@ public void Dispose()
public IWatchedFile EnqueueWatchingFile(string filePath)
{
// If we already have this file under our path, we may not have to do additional watching
if (WatchedDirectory.FilePathCoveredByWatchedDirectories(_watchedDirectories, filePath, StringComparison.OrdinalIgnoreCase))
if (WatchedDirectory.FilePathCoveredByWatchedDirectories(_watchedDirectories, filePath, s_stringComparison))
return NoOpWatchedFile.Instance;

// Record that we're now watching this file
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,22 @@ public static Uri CreateAbsoluteUri(string absolutePath)
}
}

internal static Uri CreateRelativePatternBaseUri(string path)
{
// According to VSCode LSP RelativePattern spec,
// found at https://github.com/microsoft/vscode/blob/9e1974682eb84eebb073d4ae775bad1738c281f6/src/vscode-dts/vscode.d.ts#L2226
// the baseUri should not end in a trailing separator, nor should it
// have any relative segmeents (., ..)
if (path[^1] == System.IO.Path.DirectorySeparatorChar)
{
path = path[..^1];
}

Debug.Assert(!path.Split(System.IO.Path.DirectorySeparatorChar).Any(p => p == "." || p == ".."));

return CreateAbsoluteUri(path);
}

// Implements workaround for https://github.com/dotnet/runtime/issues/89538:
internal static string GetAbsoluteUriString(string absolutePath)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,47 @@ public void CreateAbsoluteUri_LocalPaths_Unix(string filePath, string expectedAb
Assert.Equal(filePath, uri.LocalPath);
}

[ConditionalTheory(typeof(WindowsOnly))]
[InlineData("C:\\a\\b", "file:///C:/a/b")]
[InlineData("C:\\a\\b\\", "file:///C:/a/b")]
[InlineData("C:\\a\\\\b", "file:///C:/a//b")]
[InlineData("C:\\%25\ue25b/a\\b", "file:///C:/%2525%EE%89%9B/a/b")]
[InlineData("C:\\%25\ue25b/a\\\\b", "file:///C:/%2525%EE%89%9B/a//b")]
[InlineData("C:\\\u0089\uC7BD", "file:///C:/%C2%89%EC%9E%BD")]
[InlineData("/\\server\ue25b\\%25\ue25b\\b", "file://server/%2525%EE%89%9B/b")]
[InlineData("\\\\server\ue25b\\%25\ue25b\\b", "file://server/%2525%EE%89%9B/b")]
[InlineData("\\\\server\ue25b\\%25\ue25b\\b\\", "file://server/%2525%EE%89%9B/b")]
[InlineData("C:\\ !$&'()+,-;=@[]_~#", "file:///C:/%20!$&'()+,-;=@[]_~%23")]
[InlineData("C:\\ !$&'()+,-;=@[]_~#\ue25b", "file:///C:/%20!$&'()+,-;=@[]_~%23%EE%89%9B")]
[InlineData("C:\\\u0073\u0323\u0307", "file:///C:/s%CC%A3%CC%87")] // combining marks
[InlineData("A:/\\\u200e//", "file:///A://%E2%80%8E//")] // cases from https://github.com/dotnet/runtime/issues/1487
[InlineData("B:\\/\u200e", "file:///B://%E2%80%8E")]
[InlineData("C:/\\\\\r", "file:///C:///-%C4%80%0D")]
[InlineData("D:\\\\\\\\\\\u200e", "file:///D://///%E2%80%8E")]
public void CreateRelativePatternBaseUri_LocalPaths_Windows(string filePath, string expectedUri)
{
var uri = ProtocolConversions.CreateRelativePatternBaseUri(filePath);
Assert.Equal(expectedUri, uri.AbsoluteUri);
}

[ConditionalTheory(typeof(UnixLikeOnly))]
[InlineData("/", "file://")]
[InlineData("/u", "file:///u")]
[InlineData("/unix/", "file:///unix")]
[InlineData("/unix/path", "file:///unix/path")]
[InlineData("/%25\ue25b/\u0089\uC7BD", "file:///%2525%EE%89%9B/%C2%89%EC%9E%BD")]
[InlineData("/!$&'()+,-;=@[]_~#", "file:///!$&'()+,-;=@[]_~%23")]
[InlineData("/!$&'()+,-;=@[]_~#", "file:///!$&'()+,-;=@[]_~%23%EE%89%9B")]
[InlineData("/\\\u200e//", "file:////%E2%80%8E//")] // cases from https://github.com/dotnet/runtime/issues/1487
[InlineData("\\/\u200e", "file:////%E2%80%8E")]
[InlineData("/\\\\\r", "file://///-%C4%80%0D")]
[InlineData("\\\\\\\\\\\u200e", "file:///////%E2%80%8E")]
public void CreateRelativePatternBaseUri_LocalPaths_Unix(string filePath, string expectedRelativeUri)
{
var uri = ProtocolConversions.CreateRelativePatternBaseUri(filePath);
Assert.Equal(expectedRelativeUri, uri.AbsoluteUri);
}

[ConditionalTheory(typeof(UnixLikeOnly))]
[InlineData("/a/./b", "file:///a/./b", "file:///a/b")]
[InlineData("/a/../b", "file:///a/../b", "file:///b")]
Expand Down

0 comments on commit 3eefb58

Please sign in to comment.