Don't monitor the whole workspace for config files in Visual Studio (#9601)

davidwengier · web-flow · commit 9d67a98d8005 · 2023-11-21T10:11:14.000+11:00
Fixes #9600
diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/ConfigurableLanguageServerFeatureOptions.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/ConfigurableLanguageServerFeatureOptions.cs
@@ -22,6 +22,7 @@ internal class ConfigurableLanguageServerFeatureOptions : LanguageServerFeatureO
     private readonly bool? _usePreciseSemanticTokenRanges;
     private readonly bool? _updateBuffersForClosedDocuments;
     private readonly bool? _includeProjectKeyInGeneratedFilePath;
+    private readonly bool? _monitorWorkspaceFolderForConfigurationFiles;
 
     public override bool SupportsFileManipulation => _supportsFileManipulation ?? _defaults.SupportsFileManipulation;
     public override string ProjectConfigurationFileName => _projectConfigurationFileName ?? _defaults.ProjectConfigurationFileName;
@@ -35,6 +36,7 @@ internal class ConfigurableLanguageServerFeatureOptions : LanguageServerFeatureO
     public override bool UsePreciseSemanticTokenRanges => _usePreciseSemanticTokenRanges ?? _defaults.UsePreciseSemanticTokenRanges;
     public override bool UpdateBuffersForClosedDocuments => _updateBuffersForClosedDocuments ?? _defaults.UpdateBuffersForClosedDocuments;
     public override bool IncludeProjectKeyInGeneratedFilePath => _includeProjectKeyInGeneratedFilePath ?? _defaults.IncludeProjectKeyInGeneratedFilePath;
+    public override bool MonitorWorkspaceFolderForConfigurationFiles => _monitorWorkspaceFolderForConfigurationFiles ?? _defaults.MonitorWorkspaceFolderForConfigurationFiles;
 
     public ConfigurableLanguageServerFeatureOptions(string[] args)
     {
@@ -57,6 +59,7 @@ public ConfigurableLanguageServerFeatureOptions(string[] args)
             TryProcessBoolOption(nameof(UsePreciseSemanticTokenRanges), ref _usePreciseSemanticTokenRanges, option, args, i);
             TryProcessBoolOption(nameof(UpdateBuffersForClosedDocuments), ref _updateBuffersForClosedDocuments, option, args, i);
             TryProcessBoolOption(nameof(IncludeProjectKeyInGeneratedFilePath), ref _includeProjectKeyInGeneratedFilePath, option, args, i);
+            TryProcessBoolOption(nameof(MonitorWorkspaceFolderForConfigurationFiles), ref _monitorWorkspaceFolderForConfigurationFiles, option, args, i);
         }
     }
 
diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultLanguageServerFeatureOptions.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultLanguageServerFeatureOptions.cs
@@ -38,4 +38,6 @@ public override bool ReturnCodeActionAndRenamePathsWithPrefixedSlash
     public override bool IncludeProjectKeyInGeneratedFilePath => false;
 
     public override bool UsePreciseSemanticTokenRanges => false;
+
+    public override bool MonitorWorkspaceFolderForConfigurationFiles => true;
 }
diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Extensions/IServiceCollectionExtensions.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Extensions/IServiceCollectionExtensions.cs
@@ -210,8 +210,14 @@ public static void AddDocumentManagementServices(this IServiceCollection service
         services.AddSingleton<IProjectConfigurationFileChangeListener, ProjectConfigurationStateSynchronizer>();
         services.AddSingleton<IRazorFileChangeListener, RazorFileSynchronizer>();
 
-        // File Change detectors
-        services.AddSingleton<IFileChangeDetector, ProjectConfigurationFileChangeDetector>();
+        // If we're not monitoring the whole workspace folder for configuration changes, then we don't actually need the the file change
+        // detector wired up via DI, as the razor/monitorProjectConfigurationFilePath endpoint will directly construct one. This means
+        // it can be a little simpler, and doesn't need to worry about which folders it's told to listen to.
+        if (featureOptions.MonitorWorkspaceFolderForConfigurationFiles)
+        {
+            services.AddSingleton<IFileChangeDetector, ProjectConfigurationFileChangeDetector>();
+        }
+
         services.AddSingleton<IFileChangeDetector, RazorFileChangeDetector>();
 
         // Document processed listeners
diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/IMonitorProjectConfigurationFilePathHandler.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/IMonitorProjectConfigurationFilePathHandler.cs
diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectSystem/MonitorProjectConfigurationFilePathEndpoint.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectSystem/MonitorProjectConfigurationFilePathEndpoint.cs
@@ -7,15 +7,18 @@
 using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Razor.LanguageServer.Common;
 using Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts;
 using Microsoft.AspNetCore.Razor.Utilities;
 using Microsoft.CodeAnalysis.Razor;
 using Microsoft.CodeAnalysis.Razor.Workspaces;
+using Microsoft.CommonLanguageServerProtocol.Framework;
 using Microsoft.Extensions.Logging;
 
-namespace Microsoft.AspNetCore.Razor.LanguageServer;
+namespace Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;
 
-internal class MonitorProjectConfigurationFilePathEndpoint : IMonitorProjectConfigurationFilePathHandler, IDisposable
+[LanguageServerEndpoint(LanguageServerConstants.RazorMonitorProjectConfigurationFilePathEndpoint)]
+internal class MonitorProjectConfigurationFilePathEndpoint : IRazorNotificationHandler<MonitorProjectConfigurationFilePathParams>, IDisposable
 {
     private readonly ProjectSnapshotManagerDispatcher _dispatcher;
     private readonly WorkspaceDirectoryPathResolver _workspaceDirectoryPathResolver;
@@ -81,28 +84,31 @@ public async Task HandleNotificationAsync(MonitorProjectConfigurationFilePathPar
         }
 
         var configurationDirectory = Path.GetDirectoryName(request.ConfigurationFilePath);
-        var normalizedConfigurationDirectory = FilePathNormalizer.NormalizeDirectory(configurationDirectory);
-        var workspaceDirectory = _workspaceDirectoryPathResolver.Resolve();
-        var normalizedWorkspaceDirectory = FilePathNormalizer.NormalizeDirectory(workspaceDirectory);
-
         Assumes.NotNull(configurationDirectory);
 
         var previousMonitorExists = _outputPathMonitors.TryGetValue(request.ProjectKeyId, out var entry);
 
-        if (normalizedConfigurationDirectory.StartsWith(normalizedWorkspaceDirectory, FilePathComparison.Instance))
+        if (_options.MonitorWorkspaceFolderForConfigurationFiles)
         {
-            if (previousMonitorExists)
-            {
-                _logger.LogInformation("Configuration directory changed from external directory -> internal directory for project '{0}, terminating existing monitor'.", request.ProjectKeyId);
-                RemoveMonitor(request.ProjectKeyId);
-            }
-            else
+            var normalizedConfigurationDirectory = FilePathNormalizer.NormalizeDirectory(configurationDirectory);
+            var workspaceDirectory = _workspaceDirectoryPathResolver.Resolve();
+            var normalizedWorkspaceDirectory = FilePathNormalizer.NormalizeDirectory(workspaceDirectory);
+
+            if (normalizedConfigurationDirectory.StartsWith(normalizedWorkspaceDirectory, FilePathComparison.Instance))
             {
-                _logger.LogInformation("No custom configuration directory required. The workspace directory is sufficient for '{0}'.", request.ProjectKeyId);
+                if (previousMonitorExists)
+                {
+                    _logger.LogInformation("Configuration directory changed from external directory -> internal directory for project '{0}, terminating existing monitor'.", request.ProjectKeyId);
+                    RemoveMonitor(request.ProjectKeyId);
+                }
+                else
+                {
+                    _logger.LogInformation("No custom configuration directory required. The workspace directory is sufficient for '{0}'.", request.ProjectKeyId);
+                }
+
+                // Configuration directory is already in the workspace directory. We already monitor everything in the workspace directory.
+                return;
             }
-
-            // Configuration directory is already in the workspace directory. We already monitor everything in the workspace directory.
-            return;
         }
 
         if (previousMonitorExists)
diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectSystem/MonitorProjectConfigurationFilePathParams.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/ProjectSystem/MonitorProjectConfigurationFilePathParams.cs
@@ -1,11 +1,11 @@
 ﻿// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the MIT license. See License.txt in the project root for license information.
 
-namespace Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts;
+namespace Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;
 
 internal class MonitorProjectConfigurationFilePathParams
 {
     public required string ProjectKeyId { get; set; }
 
-    public required string ConfigurationFilePath { get; set; }
+    public required string? ConfigurationFilePath { get; set; }
 }
diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLanguageServer.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLanguageServer.cs
@@ -18,6 +18,7 @@
 using Microsoft.AspNetCore.Razor.LanguageServer.LinkedEditingRange;
 using Microsoft.AspNetCore.Razor.LanguageServer.MapCode;
 using Microsoft.AspNetCore.Razor.LanguageServer.ProjectContexts;
+using Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;
 using Microsoft.AspNetCore.Razor.LanguageServer.Refactoring;
 using Microsoft.AspNetCore.Razor.LanguageServer.SignatureHelp;
 using Microsoft.AspNetCore.Razor.LanguageServer.WrapWithTag;
@@ -181,7 +182,7 @@ protected override ILspServices ConstructLspServices()
 
         static void AddHandlers(IServiceCollection services)
         {
-            // Not calling AddHandler because we want to register this endpoint as an IOnIntialized too
+            // Not calling AddHandler because we want to register this endpoint as an IOnInitialized too
             services.AddSingleton<RazorConfigurationEndpoint>();
             services.AddSingleton<IMethodHandler, RazorConfigurationEndpoint>(s => s.GetRequiredService<RazorConfigurationEndpoint>());
             // Transient because it should only be used once and I'm hoping it doesn't stick around.
diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/LanguageServerFeatureOptions.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/LanguageServerFeatureOptions.cs
@@ -34,4 +34,13 @@ internal abstract class LanguageServerFeatureOptions
     /// ensure a unique file path per project.
     /// </summary>
     public abstract bool IncludeProjectKeyInGeneratedFilePath { get; }
+
+    /// <summary>
+    /// Whether to monitor the entire workspace folder for any project.razor.bin files
+    /// </summary>
+    /// <remarks>
+    /// When this is off, the language server won't have any project knowledge unless the
+    /// razor/monitorProjectConfigurationFilePath notification is sent.
+    /// </remarks>
+    public abstract bool MonitorWorkspaceFolderForConfigurationFiles { get; }
 }
diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/MonitorProjectConfigurationFilePathParams.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/MonitorProjectConfigurationFilePathParams.cs
diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLanguageServerClient.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLanguageServerClient.cs
@@ -9,6 +9,7 @@
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Razor.LanguageServer;
 using Microsoft.AspNetCore.Razor.LanguageServer.Common;
+using Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;
 using Microsoft.AspNetCore.Razor.Telemetry;
 using Microsoft.CodeAnalysis.Host;
 using Microsoft.CodeAnalysis.Razor;
diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/VisualStudioWindowsLanguageServerFeatureOptions.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/VisualStudioWindowsLanguageServerFeatureOptions.cs
@@ -80,4 +80,6 @@ public VisualStudioWindowsLanguageServerFeatureOptions(LSPEditorFeatureDetector
     public override bool IncludeProjectKeyInGeneratedFilePath => _includeProjectKeyInGeneratedFilePath.Value;
 
     public override bool UsePreciseSemanticTokenRanges => _usePreciseSemanticTokenRanges.Value;
+
+    public override bool MonitorWorkspaceFolderForConfigurationFiles => false;
 }
diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/MonitorProjectConfigurationFilePathEndpointTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/MonitorProjectConfigurationFilePathEndpointTest.cs
@@ -1,16 +1,15 @@
 ﻿// 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.Linq;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts;
+using Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;
 using Microsoft.AspNetCore.Razor.Test.Common;
 using Microsoft.CodeAnalysis.Razor;
+using Microsoft.CodeAnalysis.Razor.Workspaces;
 using Microsoft.Extensions.Logging;
 using Moq;
 using Xunit;
@@ -135,6 +134,32 @@ public async Task Handle_InWorkspaceDirectory_Noops()
         Assert.Equal(0, detector.StartCount);
     }
 
+    [Fact]
+    public async Task Handle_InWorkspaceDirectory_MonitorsIfLanguageFeatureOptionSet()
+    {
+        // Arrange
+        var detector = new TestFileChangeDetector();
+        var configurationFileEndpoint = new TestMonitorProjectConfigurationFilePathEndpoint(
+            () => detector,
+            LegacyDispatcher,
+            _directoryPathResolver,
+            Enumerable.Empty<IProjectConfigurationFileChangeListener>(),
+            LoggerFactory,
+            options: new TestLanguageServerFeatureOptions(monitorWorkspaceFolderForConfigurationFiles: false));
+        var startRequest = new MonitorProjectConfigurationFilePathParams()
+        {
+            ProjectKeyId = TestProjectKey.Create("C:/dir/obj").Id,
+            ConfigurationFilePath = "C:/dir/obj/Debug/project.razor.bin",
+        };
+        var requestContext = CreateRazorRequestContext(documentContext: null);
+
+        // Act
+        await configurationFileEndpoint.HandleNotificationAsync(startRequest, requestContext, DisposalToken);
+
+        // Assert
+        Assert.Equal(1, detector.StartCount);
+    }
+
     [Fact]
     public async Task Handle_DuplicateMonitors_Noops()
     {
@@ -323,31 +348,18 @@ private class TestMonitorProjectConfigurationFilePathEndpoint : MonitorProjectCo
     {
         private readonly Func<IFileChangeDetector> _fileChangeDetectorFactory;
 
-        public TestMonitorProjectConfigurationFilePathEndpoint(
-            ProjectSnapshotManagerDispatcher projectSnapshotManagerDispatcher,
-            WorkspaceDirectoryPathResolver workspaceDirectoryPathResolver,
-            IEnumerable<IProjectConfigurationFileChangeListener> listeners,
-            ILoggerFactory loggerFactory)
-            : this(
-                fileChangeDetectorFactory: null,
-                projectSnapshotManagerDispatcher,
-                workspaceDirectoryPathResolver,
-                listeners,
-                loggerFactory)
-        {
-        }
-
         public TestMonitorProjectConfigurationFilePathEndpoint(
             Func<IFileChangeDetector> fileChangeDetectorFactory,
             ProjectSnapshotManagerDispatcher projectSnapshotManagerDispatcher,
             WorkspaceDirectoryPathResolver workspaceDirectoryPathResolver,
             IEnumerable<IProjectConfigurationFileChangeListener> listeners,
-            ILoggerFactory loggerFactory)
+            ILoggerFactory loggerFactory,
+            LanguageServerFeatureOptions? options = null)
             : base(
                 projectSnapshotManagerDispatcher,
                 workspaceDirectoryPathResolver,
                 listeners,
-                TestLanguageServerFeatureOptions.Instance,
+                options ?? TestLanguageServerFeatureOptions.Instance,
                 loggerFactory)
         {
             _fileChangeDetectorFactory = fileChangeDetectorFactory ?? (() => Mock.Of<IFileChangeDetector>(MockBehavior.Strict));
diff --git a/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test.Common/TestLanguageServerFeatureOptions.cs b/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test.Common/TestLanguageServerFeatureOptions.cs
@@ -9,12 +9,15 @@ internal class TestLanguageServerFeatureOptions : LanguageServerFeatureOptions
 {
     public static readonly LanguageServerFeatureOptions Instance = new TestLanguageServerFeatureOptions();
 
-    private bool _includeProjectKeyInGeneratedFilePath;
+    private readonly bool _includeProjectKeyInGeneratedFilePath;
+    private readonly bool _monitorWorkspaceFolderForConfigurationFiles;
 
     public TestLanguageServerFeatureOptions(
-        bool includeProjectKeyInGeneratedFilePath = false)
+        bool includeProjectKeyInGeneratedFilePath = false,
+        bool monitorWorkspaceFolderForConfigurationFiles = true)
     {
         _includeProjectKeyInGeneratedFilePath = includeProjectKeyInGeneratedFilePath;
+        _monitorWorkspaceFolderForConfigurationFiles = monitorWorkspaceFolderForConfigurationFiles;
     }
 
     public override bool SupportsFileManipulation => false;
@@ -40,4 +43,6 @@ public TestLanguageServerFeatureOptions(
     public override bool UpdateBuffersForClosedDocuments => false;
 
     public override bool IncludeProjectKeyInGeneratedFilePath => _includeProjectKeyInGeneratedFilePath;
+
+    public override bool MonitorWorkspaceFolderForConfigurationFiles => _monitorWorkspaceFolderForConfigurationFiles;
 }

Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,7 @@ internal class ConfigurableLanguageServerFeatureOptions : LanguageServerFeatureO`
`22`	`22`	`private readonly bool? _usePreciseSemanticTokenRanges;`
`23`	`23`	`private readonly bool? _updateBuffersForClosedDocuments;`
`24`	`24`	`private readonly bool? _includeProjectKeyInGeneratedFilePath;`
	`25`	`+ private readonly bool? _monitorWorkspaceFolderForConfigurationFiles;`
`25`	`26`
`26`	`27`	`public override bool SupportsFileManipulation => _supportsFileManipulation ?? _defaults.SupportsFileManipulation;`
`27`	`28`	`public override string ProjectConfigurationFileName => _projectConfigurationFileName ?? _defaults.ProjectConfigurationFileName;`
`@@ -35,6 +36,7 @@ internal class ConfigurableLanguageServerFeatureOptions : LanguageServerFeatureO`
`35`	`36`	`public override bool UsePreciseSemanticTokenRanges => _usePreciseSemanticTokenRanges ?? _defaults.UsePreciseSemanticTokenRanges;`
`36`	`37`	`public override bool UpdateBuffersForClosedDocuments => _updateBuffersForClosedDocuments ?? _defaults.UpdateBuffersForClosedDocuments;`
`37`	`38`	`public override bool IncludeProjectKeyInGeneratedFilePath => _includeProjectKeyInGeneratedFilePath ?? _defaults.IncludeProjectKeyInGeneratedFilePath;`
	`39`	`+ public override bool MonitorWorkspaceFolderForConfigurationFiles => _monitorWorkspaceFolderForConfigurationFiles ?? _defaults.MonitorWorkspaceFolderForConfigurationFiles;`
`38`	`40`
`39`	`41`	`public ConfigurableLanguageServerFeatureOptions(string[] args)`
`40`	`42`	`{`
`@@ -57,6 +59,7 @@ public ConfigurableLanguageServerFeatureOptions(string[] args)`
`57`	`59`	`TryProcessBoolOption(nameof(UsePreciseSemanticTokenRanges), ref _usePreciseSemanticTokenRanges, option, args, i);`
`58`	`60`	`TryProcessBoolOption(nameof(UpdateBuffersForClosedDocuments), ref _updateBuffersForClosedDocuments, option, args, i);`
`59`	`61`	`TryProcessBoolOption(nameof(IncludeProjectKeyInGeneratedFilePath), ref _includeProjectKeyInGeneratedFilePath, option, args, i);`
	`62`	`+ TryProcessBoolOption(nameof(MonitorWorkspaceFolderForConfigurationFiles), ref _monitorWorkspaceFolderForConfigurationFiles, option, args, i);`
`60`	`63`	`}`
`61`	`64`	`}`
`62`	`65`
Original file line number	Diff line number	Diff line change
`@@ -38,4 +38,6 @@ public override bool ReturnCodeActionAndRenamePathsWithPrefixedSlash`
`38`	`38`	`public override bool IncludeProjectKeyInGeneratedFilePath => false;`
`39`	`39`
`40`	`40`	`public override bool UsePreciseSemanticTokenRanges => false;`
	`41`	`+`
	`42`	`+ public override bool MonitorWorkspaceFolderForConfigurationFiles => true;`
`41`	`43`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,11 +1,11 @@`
`1`	`1`	`// Copyright (c) .NET Foundation. All rights reserved.`
`2`	`2`	`// Licensed under the MIT license. See License.txt in the project root for license information.`
`3`	`3`
`4`		`-namespace Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts;`
	`4`	`+namespace Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;`
`5`	`5`
`6`	`6`	`internal class MonitorProjectConfigurationFilePathParams`
`7`	`7`	`{`
`8`	`8`	`public required string ProjectKeyId { get; set; }`
`9`	`9`
`10`		`- public required string ConfigurationFilePath { get; set; }`
	`10`	`+ public required string? ConfigurationFilePath { get; set; }`
`11`	`11`	`}`