Make IFileSystem a real, non-obsolete, DIed service

davidwengier · davidwengier · commit 5fe03dc8693d · 2024-11-08T08:57:09.000+11:00
diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DirectoryHelper.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DirectoryHelper.cs
@@ -7,6 +7,7 @@
 using System.Linq;
 using Microsoft.CodeAnalysis.Razor;
 using Microsoft.CodeAnalysis.Razor.Logging;
+using Microsoft.CodeAnalysis.Razor.Workspaces;
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer;
 
@@ -26,33 +27,9 @@ internal static IEnumerable<string> GetFilteredFiles(
         string workspaceDirectory,
         string searchPattern,
         IReadOnlyCollection<string> ignoredDirectories,
-#pragma warning disable CS0618 // Type or member is obsolete
-        IFileSystem? fileSystem = null,
-#pragma warning restore CS0618 // Type or member is obsolete
-        ILogger? logger = null)
+        IFileSystem fileSystem,
+        ILogger logger)
     {
-        if (workspaceDirectory is null)
-        {
-            throw new ArgumentNullException(nameof(workspaceDirectory));
-        }
-
-        if (searchPattern is null)
-        {
-            throw new ArgumentNullException(nameof(searchPattern));
-        }
-
-        if (ignoredDirectories is null || ignoredDirectories.Count == 0)
-        {
-            throw new ArgumentNullException(nameof(ignoredDirectories));
-        }
-
-        if (fileSystem is null)
-        {
-#pragma warning disable CS0618 // Type or member is obsolete
-            fileSystem = new DefaultFileSystem();
-#pragma warning restore CS0618 // Type or member is obsolete
-        }
-
         IEnumerable<string> files;
         try
         {
@@ -62,21 +39,21 @@ internal static IEnumerable<string> GetFilteredFiles(
         {
             // The filesystem may have deleted the directory between us finding it and searching for files in it.
             // This can also happen if the directory is too long for windows.
-            files = Array.Empty<string>();
+            files = [];
         }
         catch (UnauthorizedAccessException ex)
         {
-            logger?.LogWarning($"UnauthorizedAccess: {ex.Message}");
+            logger.LogWarning($"UnauthorizedAccess: {ex.Message}");
             yield break;
         }
         catch (PathTooLongException ex)
         {
-            logger?.LogWarning($"PathTooLong: {ex.Message}");
+            logger.LogWarning($"PathTooLong: {ex.Message}");
             yield break;
         }
         catch (IOException ex)
         {
-            logger?.LogWarning($"IOException: {ex.Message}");
+            logger.LogWarning($"IOException: {ex.Message}");
             yield break;
         }
 
@@ -94,21 +71,21 @@ internal static IEnumerable<string> GetFilteredFiles(
         {
             // The filesystem may have deleted the directory between us finding it and searching for directories in it.
             // This can also happen if the directory is too long for windows.
-            directories = Array.Empty<string>();
+            directories = [];
         }
         catch (UnauthorizedAccessException ex)
         {
-            logger?.LogWarning($"UnauthorizedAccess: {ex.Message}");
+            logger.LogWarning($"UnauthorizedAccess: {ex.Message}");
             yield break;
         }
         catch (PathTooLongException ex)
         {
-            logger?.LogWarning($"PathTooLong: {ex.Message}");
+            logger.LogWarning($"PathTooLong: {ex.Message}");
             yield break;
         }
         catch (IOException ex)
         {
-            logger?.LogWarning($"IOException: {ex.Message}");
+            logger.LogWarning($"IOException: {ex.Message}");
             yield break;
         }
 
@@ -124,26 +101,4 @@ internal static IEnumerable<string> GetFilteredFiles(
             }
         }
     }
-
-    [Obsolete("This only exists to enable testing, do not use it outside of tests for this class")]
-    internal interface IFileSystem
-    {
-        public IEnumerable<string> GetFiles(string workspaceDirectory, string searchPattern, SearchOption searchOption);
-
-        public IEnumerable<string> GetDirectories(string workspaceDirectory);
-    }
-
-    [Obsolete("This only exists to enable testing, do not use it outside of tests for this class")]
-    private class DefaultFileSystem : IFileSystem
-    {
-        public IEnumerable<string> GetFiles(string workspaceDirectory, string searchPattern, SearchOption searchOption)
-        {
-            return Directory.GetFiles(workspaceDirectory, searchPattern, searchOption);
-        }
-
-        public IEnumerable<string> GetDirectories(string workspaceDirectory)
-        {
-            return Directory.GetDirectories(workspaceDirectory);
-        }
-    }
 }
diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorFileChangeDetector.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorFileChangeDetector.cs
@@ -13,6 +13,8 @@
 using Microsoft.AspNetCore.Razor.PooledObjects;
 using Microsoft.AspNetCore.Razor.Utilities;
 using Microsoft.CodeAnalysis.Razor;
+using Microsoft.CodeAnalysis.Razor.Logging;
+using Microsoft.CodeAnalysis.Razor.Workspaces;
 using Microsoft.VisualStudio.Threading;
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer;
@@ -30,13 +32,15 @@ internal partial class RazorFileChangeDetector : IFileChangeDetector, IDisposabl
     private readonly Dictionary<string, (RazorFileChangeKind kind, int index)> _filePathToChangeMap;
     private readonly HashSet<int> _indicesToSkip;
     private readonly List<FileSystemWatcher> _watchers;
+    private readonly IFileSystem _fileSystem;
+    private readonly ILogger _logger;
 
-    public RazorFileChangeDetector(IEnumerable<IRazorFileChangeListener> listeners)
-        : this(listeners, s_delay)
+    public RazorFileChangeDetector(IEnumerable<IRazorFileChangeListener> listeners, IFileSystem fileSystem, ILoggerFactory loggerFactory)
+        : this(listeners, fileSystem, loggerFactory, s_delay)
     {
     }
 
-    protected RazorFileChangeDetector(IEnumerable<IRazorFileChangeListener> listeners, TimeSpan delay)
+    protected RazorFileChangeDetector(IEnumerable<IRazorFileChangeListener> listeners, IFileSystem fileSystem, ILoggerFactory loggerFactory, TimeSpan delay)
     {
         _listeners = listeners.ToImmutableArray();
 
@@ -45,6 +49,8 @@ protected RazorFileChangeDetector(IEnumerable<IRazorFileChangeListener> listener
         _filePathToChangeMap = new(FilePathComparer.Instance);
         _indicesToSkip = [];
         _watchers = new List<FileSystemWatcher>(s_razorFileExtensions.Length);
+        _fileSystem = fileSystem;
+        _logger = loggerFactory.GetOrCreateLogger<RazorFileChangeDetector>();
     }
 
     public void Dispose()
@@ -218,7 +224,7 @@ protected virtual ImmutableArray<string> GetExistingRazorFiles(string workspaceD
 
         foreach (var extension in s_razorFileExtensions)
         {
-            var existingFiles = DirectoryHelper.GetFilteredFiles(workspaceDirectory, "*" + extension, s_ignoredDirectories);
+            var existingFiles = DirectoryHelper.GetFilteredFiles(workspaceDirectory, "*" + extension, s_ignoredDirectories, _fileSystem, _logger);
             result.AddRange(existingFiles);
         }
 
diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLanguageServer.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLanguageServer.cs
@@ -123,6 +123,7 @@ protected override ILspServices ConstructLspServices()
         services.AddSingleton(featureOptions);
 
         services.AddSingleton<IFilePathService, LSPFilePathService>();
+        services.AddSingleton<IFileSystem, FileSystem>();
 
         services.AddLifeCycleServices(this, _clientConnection, _lspServerActivationTracker);
 
diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/FileSystem.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/FileSystem.cs
@@ -0,0 +1,20 @@
+﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the MIT license. See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using System.IO;
+
+namespace Microsoft.CodeAnalysis.Razor.Workspaces;
+
+internal class FileSystem : IFileSystem
+{
+    public IEnumerable<string> GetFiles(string workspaceDirectory, string searchPattern, SearchOption searchOption)
+    {
+        return Directory.GetFiles(workspaceDirectory, searchPattern, searchOption);
+    }
+
+    public IEnumerable<string> GetDirectories(string workspaceDirectory)
+    {
+        return Directory.GetDirectories(workspaceDirectory);
+    }
+}
diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/IFileSystem.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/IFileSystem.cs
@@ -0,0 +1,14 @@
+﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the MIT license. See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using System.IO;
+
+namespace Microsoft.CodeAnalysis.Razor.Workspaces;
+
+internal interface IFileSystem
+{
+    public IEnumerable<string> GetFiles(string workspaceDirectory, string searchPattern, SearchOption searchOption);
+
+    public IEnumerable<string> GetDirectories(string workspaceDirectory);
+}
diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/RemoteFileSystem.cs b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/RemoteFileSystem.cs
@@ -0,0 +1,12 @@
+﻿// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the MIT license. See License.txt in the project root for license information.
+
+using System.Composition;
+using Microsoft.CodeAnalysis.Razor.Workspaces;
+
+namespace Microsoft.CodeAnalysis.Remote.Razor;
+
+[Export(typeof(IFileSystem)), Shared]
+internal class RemoteFileSystem : FileSystem
+{
+}
diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DirectoryHelperTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DirectoryHelperTest.cs
@@ -1,16 +1,13 @@
 ﻿// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the MIT license. See License.txt in the project root for license information.
 
-#nullable disable
-
-using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using Microsoft.AspNetCore.Razor.LanguageServer.Completion;
+using Microsoft.CodeAnalysis.Razor.Workspaces;
 using Xunit;
 using Xunit.Abstractions;
-using static Microsoft.AspNetCore.Razor.LanguageServer.DirectoryHelper;
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer.Test;
 
@@ -26,24 +23,22 @@ public void GetFilteredFiles_FindsFiles()
         var workspaceDirectory = Path.Combine("LowerDirectory");
         var searchPattern = "project.razor.bin";
         var ignoredDirectories = new[] { "node_modules" };
-        var fileResults = new Dictionary<string, IEnumerable<string>>() {
-            { "HigherDirectory", new []{ firstProjectRazorJson } },
-            { "RealDirectory", new []{ secondProjectRazorJson } },
-            { "LongDirectory", new[]{ "LONGPATH", "LONGPATH\\project.razor.bin" } },
+        var fileResults = new Dictionary<string, string[]?>() {
+            { "HigherDirectory", [firstProjectRazorJson] },
+            { "RealDirectory", [secondProjectRazorJson] },
+            { "LongDirectory", ["LONGPATH", "LONGPATH\\project.razor.bin"] },
             { "node_modules", null },
         };
-        var directoryResults = new Dictionary<string, IEnumerable<string>>() {
-            { "LowerDirectory", new[]{ "HigherDirectory" } },
-            { "HigherDirectory", new[]{ "node_modules", "RealDirectory", "FakeDirectory", "LongDirectory" } },
+        var directoryResults = new Dictionary<string, string[]?>() {
+            { "LowerDirectory", ["HigherDirectory"] },
+            { "HigherDirectory", ["node_modules", "RealDirectory", "FakeDirectory", "LongDirectory"] },
             { "node_modules", null },
         };
 
-#pragma warning disable CS0612 // Type or member is obsolete
         var fileSystem = new TestFileSystem(fileResults, directoryResults);
-#pragma warning restore CS0612 // Type or member is obsolete
 
         // Act
-        var files = DirectoryHelper.GetFilteredFiles(workspaceDirectory, searchPattern, ignoredDirectories, fileSystem);
+        var files = DirectoryHelper.GetFilteredFiles(workspaceDirectory, searchPattern, ignoredDirectories, fileSystem, Logger);
 
         // Assert
         Assert.Collection(files,
@@ -52,23 +47,13 @@ public void GetFilteredFiles_FindsFiles()
         );
     }
 
-    [Obsolete]
-    private class TestFileSystem : IFileSystem
+    private class TestFileSystem(
+        Dictionary<string, string[]?> fileResults,
+        Dictionary<string, string[]?> directoryResults) : IFileSystem
     {
-        private readonly IDictionary<string, IEnumerable<string>> _fileResults;
-        private readonly IDictionary<string, IEnumerable<string>> _directoryResults;
-
-        public TestFileSystem(
-            IDictionary<string, IEnumerable<string>> fileResults,
-            IDictionary<string, IEnumerable<string>> directoryResults)
-        {
-            _fileResults = fileResults;
-            _directoryResults = directoryResults;
-        }
-
         public IEnumerable<string> GetDirectories(string workspaceDirectory)
         {
-            var success = _directoryResults.TryGetValue(workspaceDirectory, out var results);
+            var success = directoryResults.TryGetValue(workspaceDirectory, out var results);
             if (success)
             {
                 if (results is null)
@@ -91,7 +76,7 @@ public IEnumerable<string> GetDirectories(string workspaceDirectory)
 
         public IEnumerable<string> GetFiles(string workspaceDirectory, string searchPattern, SearchOption searchOption)
         {
-            var success = _fileResults.TryGetValue(workspaceDirectory, out var results);
+            var success = fileResults.TryGetValue(workspaceDirectory, out var results);
             if (success)
             {
                 if (results is null)
diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/RazorFileChangeDetectorTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/RazorFileChangeDetectorTest.cs
@@ -9,6 +9,8 @@
 using Microsoft.AspNetCore.Razor.LanguageServer.Common;
 using Microsoft.AspNetCore.Razor.Test.Common;
 using Microsoft.AspNetCore.Razor.Test.Common.LanguageServer;
+using Microsoft.CodeAnalysis.Razor.Logging;
+using Microsoft.CodeAnalysis.Razor.Workspaces;
 using Moq;
 using Xunit;
 using Xunit.Abstractions;
@@ -38,7 +40,8 @@ public async Task StartAsync_NotifiesListenersOfExistingRazorFiles()
         ImmutableArray<string> existingRazorFiles = ["c:/path/to/index.razor", "c:/other/path/_Host.cshtml"];
         using var detector = new InitializationSkippingRazorFileChangeDetector(
             [listenerMock1.Object, listenerMock2.Object],
-            existingRazorFiles);
+            existingRazorFiles,
+            LoggerFactory);
 
         // Act
         await detector.StartAsync("/some/workspacedirectory", DisposalToken);
@@ -79,7 +82,7 @@ internal async Task TestNotificationBehavior((string, RazorFileChangeKind)[] wor
             .Returns(Task.CompletedTask)
             .Callback((string filePath, RazorFileChangeKind kind, CancellationToken _) => actual.Add((filePath, kind)));
 
-        using var detector = new TestRazorFileChangeDetector([listenerMock.Object], TimeSpan.FromMilliseconds(1));
+        using var detector = new TestRazorFileChangeDetector([listenerMock.Object], TimeSpan.FromMilliseconds(1), LoggerFactory);
         var detectorAccessor = detector.GetTestAccessor();
 
         detectorAccessor.AddWork(work);
@@ -114,14 +117,17 @@ public static TheoryData NotificationBehaviorData
 
     private class TestRazorFileChangeDetector(
         IEnumerable<IRazorFileChangeListener> listeners,
-        TimeSpan delay)
-        : RazorFileChangeDetector(listeners, delay)
+        TimeSpan delay,
+        ILoggerFactory loggerFactory)
+        : RazorFileChangeDetector(listeners, new FileSystem(), loggerFactory, delay)
     {
     }
 
     private class InitializationSkippingRazorFileChangeDetector(
         IEnumerable<IRazorFileChangeListener> listeners,
-        ImmutableArray<string> existingProjectFiles) : RazorFileChangeDetector(listeners)
+        ImmutableArray<string> existingProjectFiles,
+        ILoggerFactory loggerFactory)
+        : RazorFileChangeDetector(listeners, new FileSystem(), loggerFactory)
     {
         private readonly ImmutableArray<string> _existingProjectFiles = existingProjectFiles;
 
