More cohost code actions tests! (#11179)

I couldn't originally test some code actions because they used the file
system, so I intended this PR to fix that by abstracting the file system
away. It is that, but I also found some more tests that we lacked
coverage for in cohosting, and since I find the cohosting tests to be
just so wonderful (not biased at all, obviously) I thought I'd be a bit
extra.

I deliberately didn't try to fix any of the code actions, but I think
they could probably all do with some nicer whitespace handling. Managed
to avoid the self-nerd-snipe this time though.

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DirectoryHelper.cs → src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/IFileSystemExtensions.cs

@@ -7,10 +7,11 @@
 using System.Linq;
 using Microsoft.CodeAnalysis.Razor;
 using Microsoft.CodeAnalysis.Razor.Logging;
+using Microsoft.CodeAnalysis.Razor.Workspaces;
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer;
 
-internal static class DirectoryHelper
+internal static class IFileSystemExtensions
 {
     /// <summary>
     /// Finds all the files in a directory which meet the given criteria.
@@ -23,36 +24,12 @@ internal static class DirectoryHelper
     /// <returns>A list of files within the given directory that meet the search criteria.</returns>
     /// <remarks>This method is needed to avoid problematic folders such as "node_modules" which are known not to yield the desired results or may cause performance issues.</remarks>
     internal static IEnumerable<string> GetFilteredFiles(
+        this IFileSystem fileSystem,
         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)
+        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;
         }
 
@@ -117,33 +94,11 @@ internal static IEnumerable<string> GetFilteredFiles(
             var directory = Path.GetFileName(path);
             if (!ignoredDirectories.Contains(directory, FilePathComparer.Instance))
             {
-                foreach (var result in GetFilteredFiles(path, searchPattern, ignoredDirectories, fileSystem, logger))
+                foreach (var result in GetFilteredFiles(fileSystem, path, searchPattern, ignoredDirectories, logger))
                 {
                     yield return result;
                 }
             }
         }
     }
-
-    [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);
-        }
-    }
 }

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 = _fileSystem.GetFilteredFiles(workspaceDirectory, "*" + extension, s_ignoredDirectories, _logger);
             result.AddRange(existingFiles);
         }
 

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);
 

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/CodeActions/Razor/ComponentAccessibilityCodeActionProvider.cs

@@ -25,8 +25,10 @@ namespace Microsoft.CodeAnalysis.Razor.CodeActions;
 
 using SyntaxNode = Microsoft.AspNetCore.Razor.Language.Syntax.SyntaxNode;
 
-internal class ComponentAccessibilityCodeActionProvider : IRazorCodeActionProvider
+internal class ComponentAccessibilityCodeActionProvider(IFileSystem fileSystem) : IRazorCodeActionProvider
 {
+    private readonly IFileSystem _fileSystem = fileSystem;
+
     public async Task<ImmutableArray<RazorVSInternalCodeAction>> ProvideAsync(RazorCodeActionContext context, CancellationToken cancellationToken)
     {
         // Locate cursor
@@ -89,7 +91,7 @@ private static bool IsApplicableTag(IStartTagSyntaxNode startTag)
         return true;
     }
 
-    private static void AddCreateComponentFromTag(RazorCodeActionContext context, IStartTagSyntaxNode startTag, List<RazorVSInternalCodeAction> container)
+    private void AddCreateComponentFromTag(RazorCodeActionContext context, IStartTagSyntaxNode startTag, List<RazorVSInternalCodeAction> container)
     {
         if (!context.SupportsFileCreation)
         {
@@ -103,7 +105,7 @@ private static void AddCreateComponentFromTag(RazorCodeActionContext context, IS
         Assumes.NotNull(directoryName);
 
         var newComponentPath = Path.Combine(directoryName, $"{startTag.Name.Content}.razor");
-        if (File.Exists(newComponentPath))
+        if (_fileSystem.FileExists(newComponentPath))
         {
             return;
         }

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/CodeActions/Razor/GenerateMethodCodeActionResolver.cs

@@ -19,6 +19,7 @@
 using Microsoft.CodeAnalysis.Razor.Formatting;
 using Microsoft.CodeAnalysis.Razor.ProjectSystem;
 using Microsoft.CodeAnalysis.Razor.Protocol;
+using Microsoft.CodeAnalysis.Razor.Workspaces;
 using Microsoft.VisualStudio.LanguageServer.Protocol;
 using CSharpSyntaxFactory = Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
 
@@ -27,11 +28,13 @@ namespace Microsoft.CodeAnalysis.Razor.CodeActions.Razor;
 internal class GenerateMethodCodeActionResolver(
     IRoslynCodeActionHelpers roslynCodeActionHelpers,
     IDocumentMappingService documentMappingService,
-    IRazorFormattingService razorFormattingService) : IRazorCodeActionResolver
+    IRazorFormattingService razorFormattingService,
+    IFileSystem fileSystem) : IRazorCodeActionResolver
 {
     private readonly IRoslynCodeActionHelpers _roslynCodeActionHelpers = roslynCodeActionHelpers;
     private readonly IDocumentMappingService _documentMappingService = documentMappingService;
     private readonly IRazorFormattingService _razorFormattingService = razorFormattingService;
+    private readonly IFileSystem _fileSystem = fileSystem;
 
     private const string ReturnType = "$$ReturnType$$";
     private const string MethodName = "$$MethodName$$";
@@ -58,7 +61,7 @@ internal class GenerateMethodCodeActionResolver(
         var razorClassName = Path.GetFileNameWithoutExtension(uriPath);
         var codeBehindPath = $"{uriPath}.cs";
 
-        if (!File.Exists(codeBehindPath) ||
+        if (!_fileSystem.FileExists(codeBehindPath) ||
             razorClassName is null ||
             !code.TryComputeNamespace(fallbackToRootNamespace: true, out var razorNamespace))
         {
@@ -72,7 +75,7 @@ razorClassName is null ||
                 cancellationToken).ConfigureAwait(false);
         }
 
-        var content = File.ReadAllText(codeBehindPath);
+        var content = _fileSystem.ReadFile(codeBehindPath);
         if (GetCSharpClassDeclarationSyntax(content, razorNamespace, razorClassName) is not { } @class)
         {
             // The code behind file is malformed, generate the code in the razor file instead.

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/FileSystem.cs

@@ -0,0 +1,22 @@
+// 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 sealed class FileSystem : IFileSystem
+{
+    public IEnumerable<string> GetFiles(string workspaceDirectory, string searchPattern, SearchOption searchOption)
+        => Directory.GetFiles(workspaceDirectory, searchPattern, searchOption);
+
+    public IEnumerable<string> GetDirectories(string workspaceDirectory)
+        => Directory.GetDirectories(workspaceDirectory);
+
+    public bool FileExists(string filePath)
+        => File.Exists(filePath);
+
+    public string ReadFile(string filePath)
+        => File.ReadAllText(filePath);
+}

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/IFileSystem.cs

@@ -0,0 +1,18 @@
+// 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);
+
+    bool FileExists(string filePath);
+
+    string ReadFile(string filePath);
+}

src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/CodeActions/RemoteServices.cs

@@ -43,7 +43,8 @@ internal sealed class OOPExtractToCodeBehindCodeActionProvider(ILoggerFactory lo
 internal sealed class OOPExtractToComponentCodeActionProvider : ExtractToComponentCodeActionProvider;
 
 [Export(typeof(IRazorCodeActionProvider)), Shared]
-internal sealed class OOPComponentAccessibilityCodeActionProvider : ComponentAccessibilityCodeActionProvider;
+[method: ImportingConstructor]
+internal sealed class OOPComponentAccessibilityCodeActionProvider(IFileSystem fileSystem) : ComponentAccessibilityCodeActionProvider(fileSystem);
 
 [Export(typeof(IRazorCodeActionProvider)), Shared]
 internal sealed class OOPGenerateMethodCodeActionProvider : GenerateMethodCodeActionProvider;
@@ -84,8 +85,9 @@ internal sealed class OOPAddUsingsCodeActionResolver : AddUsingsCodeActionResolv
 internal sealed class OOPGenerateMethodCodeActionResolver(
     IRoslynCodeActionHelpers roslynCodeActionHelpers,
     IDocumentMappingService documentMappingService,
-    IRazorFormattingService razorFormattingService)
-    : GenerateMethodCodeActionResolver(roslynCodeActionHelpers, documentMappingService, razorFormattingService);
+    IRazorFormattingService razorFormattingService,
+    IFileSystem fileSystem)
+    : GenerateMethodCodeActionResolver(roslynCodeActionHelpers, documentMappingService, razorFormattingService, fileSystem);
 
 [Export(typeof(ICSharpCodeActionResolver)), Shared]
 [method: ImportingConstructor]

src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/RemoteFileSystem.cs

@@ -0,0 +1,37 @@
+// 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.Composition;
+using System.IO;
+using Microsoft.CodeAnalysis.Razor.Workspaces;
+
+namespace Microsoft.CodeAnalysis.Remote.Razor;
+
+[Export(typeof(IFileSystem)), Shared]
+internal class RemoteFileSystem : IFileSystem
+{
+    private IFileSystem _fileSystem = new FileSystem();
+
+    public bool FileExists(string filePath)
+        => _fileSystem.FileExists(filePath);
+
+    public string ReadFile(string filePath)
+        => _fileSystem.ReadFile(filePath);
+
+    public IEnumerable<string> GetDirectories(string workspaceDirectory)
+        => _fileSystem.GetDirectories(workspaceDirectory);
+
+    public IEnumerable<string> GetFiles(string workspaceDirectory, string searchPattern, SearchOption searchOption)
+        => _fileSystem.GetFiles(workspaceDirectory, searchPattern, searchOption);
+
+    internal TestAccessor GetTestAccessor() => new(this);
+
+    internal readonly struct TestAccessor(RemoteFileSystem instance)
+    {
+        public void SetFileSystem(IFileSystem fileSystem)
+        {
+            instance._fileSystem = fileSystem;
+        }
+    }
+}

src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/CodeActions/CodeActionEndToEndTestBase.NetFx.cs

@@ -333,6 +333,7 @@ internal GenerateMethodCodeActionResolver[] CreateRazorCodeActionResolvers(
                 new GenerateMethodCodeActionResolver(
                         roslynCodeActionHelpers,
                         new LspDocumentMappingService(FilePathService, new TestDocumentContextFactory(), LoggerFactory),
-                        razorFormattingService)
+                        razorFormattingService,
+                        new FileSystem())
             ];
 }