From 3eefb58ea69229a9374d14a51704c011877ead52 Mon Sep 17 00:00:00 2001 From: Andrew Hall Date: Fri, 26 Apr 2024 14:06:16 -0700 Subject: [PATCH] Fix LSP File watching so it correctly reports the baseUri if there's a trailing slash (#73203) Fix LSP File watching so it correctly reports the baseUri if there's a trailing slash --- .../LspFileChangeWatcherTests.cs | 2 +- .../FileWatching/LspFileChangeWatcher.cs | 11 ++--- .../Extensions/ProtocolConversions.cs | 16 ++++++++ .../ProtocolConversionsTests.cs | 41 +++++++++++++++++++ 4 files changed, 64 insertions(+), 6 deletions(-) diff --git a/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/LspFileChangeWatcherTests.cs b/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/LspFileChangeWatcherTests.cs index c7b1b6b940865..4be80e009e63a 100644 --- a/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/LspFileChangeWatcherTests.cs +++ b/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/LspFileChangeWatcherTests.cs @@ -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 diff --git a/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/FileWatching/LspFileChangeWatcher.cs b/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/FileWatching/LspFileChangeWatcher.cs index b2da22f5e8157..254707da542a7 100644 --- a/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/FileWatching/LspFileChangeWatcher.cs +++ b/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/FileWatching/LspFileChangeWatcher.cs @@ -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 /// - private readonly Dictionary _watchedFiles = new Dictionary(_stringComparer); - private static readonly StringComparer _stringComparer = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? StringComparer.Ordinal : StringComparer.OrdinalIgnoreCase; + private readonly Dictionary _watchedFiles = new Dictionary(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 watchedDirectories, LspFileChangeWatcher lspFileChangeWatcher) { @@ -80,7 +81,7 @@ public FileChangeContext(ImmutableArray watchedDirectories, Ls { GlobPattern = new RelativePattern { - BaseUri = ProtocolConversions.CreateAbsoluteUri(d.Path), + BaseUri = ProtocolConversions.CreateRelativePatternBaseUri(d.Path), Pattern = d.ExtensionFilter is not null ? "**/*" + d.ExtensionFilter : "**/*" } }).ToArray(); @@ -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); } @@ -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 diff --git a/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs b/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs index a06cee02e928e..b46fbf4e76b96 100644 --- a/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs +++ b/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs @@ -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) { diff --git a/src/Features/LanguageServer/ProtocolUnitTests/ProtocolConversionsTests.cs b/src/Features/LanguageServer/ProtocolUnitTests/ProtocolConversionsTests.cs index 8685d125fc2a5..a9d20fa0ccee6 100644 --- a/src/Features/LanguageServer/ProtocolUnitTests/ProtocolConversionsTests.cs +++ b/src/Features/LanguageServer/ProtocolUnitTests/ProtocolConversionsTests.cs @@ -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")]