Move to a listener model to invert the relationship between the capability check and notifier

Author: davidwengier

src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/IProjectCapabilityListener.cs

@@ -0,0 +1,9 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.VisualStudio.Razor;
+
+internal interface IProjectCapabilityListener
+{
+    void OnProjectCapabilityMatched(string projectFilePath, string capability, bool isMatch);
+}

src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/IProjectCapabilityResolver.cs

@@ -9,12 +9,4 @@ internal interface IProjectCapabilityResolver
     /// Determines whether the project associated with the specified document has the given <paramref name="capability"/>.
     /// </summary>
     bool ResolveCapability(string capability, string documentFilePath);
-
-    /// <summary>
-    /// Tries to return a cached value for the capability check, if a previous call to <see cref="ResolveCapability(string, string)" /> has been made for the same project and capability.
-    /// </summary>
-    /// <remarks>
-    /// This method is intended purely for performance optimization. It should not be used to determine if a capability is supported, as it may return false negatives in many circumstances.
-    /// </remarks>
-    bool TryGetCachedCapabilityMatch(string projectFilePath, string capability, out bool isMatch);
 }

src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/IncompatibleProjectNotifier.cs

@@ -15,33 +15,64 @@
 
 namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost;
 
+[Export(typeof(IProjectCapabilityListener))]
 [Export(typeof(IIncompatibleProjectNotifier))]
 [method: ImportingConstructor]
 internal sealed class IncompatibleProjectNotifier(
-    IProjectCapabilityResolver projectCapabilityResolver,
-    ILoggerFactory loggerFactory) : IIncompatibleProjectNotifier
+    ILoggerFactory loggerFactory) : IIncompatibleProjectNotifier, IProjectCapabilityListener
 {
-    private readonly IProjectCapabilityResolver _projectCapabilityResolver = projectCapabilityResolver;
     private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<IncompatibleProjectNotifier>();
 
+    private readonly HashSet<string> _frameworkProjects = new(PathUtilities.OSSpecificPathComparer);
+
     public void NotifyMiscFilesDocument(TextDocument textDocument)
     {
         _logger.Log(LogLevel.Error, $"{WorkspacesSR.FormatIncompatibleProject_MiscFiles(Path.GetFileName(textDocument.FilePath))}");
     }
 
     public void NotifyMissingDocument(Project project, string filePath)
     {
-        // When this document was opened, we will have checked if it was a .NET Framework project. If so, then we can avoid
-        // notifying the user because they are not using the LSP editor, even though we get the odd request.
-        // If this check returns a false positive, the fallout is only one log message, so nothing to be concerned about.
-        if (_projectCapabilityResolver.TryGetCachedCapabilityMatch(project.FilePath.AssumeNotNull(), WellKnownProjectCapabilities.DotNetCoreCSharp, out var isMatch) && !isMatch)
+        // When this document was opened, we will have checked if it was a .NET Framework project, and we listened for that below.
+        // Since this method is only called when we receive an LSP request for a document, and LSP only works on open documents,
+        // we know that the capability check must have happened before this method was called, so our cache is as up to date as
+        // possible for the specific file being asked about.
+        lock (_frameworkProjects)
         {
-            return;
+            if (_frameworkProjects.Contains(project.FilePath.AssumeNotNull()))
+            {
+                // This project doesn't have the .NET Core C# capability, so it's a .NET Framework project and we don't want
+                // to notify the user, as those projects use a different editor.
+                return;
+            }
         }
 
         _logger.Log(LogLevel.Error, $"{(
             project.AdditionalDocuments.Any(d => d.FilePath is not null && d.FilePath.IsRazorFilePath())
                 ? WorkspacesSR.FormatIncompatibleProject_NotAnAdditionalFile(Path.GetFileName(filePath), project.Name)
                 : WorkspacesSR.FormatIncompatibleProject_NoAdditionalFiles(Path.GetFileName(filePath), project.Name))}");
     }
+
+    public void OnProjectCapabilityMatched(string projectFilePath, string capability, bool isMatch)
+    {
+        // We only track the .NET Core capability
+        if (capability != WellKnownProjectCapabilities.DotNetCoreCSharp)
+        {
+            return;
+        }
+
+        lock (_frameworkProjects)
+        {
+            if (isMatch)
+            {
+                // The project is a .NET Core project, so we don't care, but just in case it used to be .NET Framework,
+                // let's clean up.
+                _frameworkProjects.Remove(projectFilePath);
+            }
+            else
+            {
+                // The project is not a .NET Core project, so add it to our list of framework projects.
+                _frameworkProjects.Add(projectFilePath);
+            }
+        }
+    }
 }

src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/ProjectCapabilityResolver.cs

@@ -22,22 +22,22 @@ namespace Microsoft.VisualStudio.Razor;
 internal sealed class ProjectCapabilityResolver : IProjectCapabilityResolver, IDisposable
 {
     private readonly ILiveShareSessionAccessor _liveShareSessionAccessor;
+    private readonly IEnumerable<IProjectCapabilityListener> _projectCapabilityListeners;
     private readonly AsyncLazy<IVsUIShellOpenDocument> _lazyVsUIShellOpenDocument;
     private readonly ILogger _logger;
     private readonly JoinableTaskFactory _jtf;
     private readonly CancellationTokenSource _disposeTokenSource;
 
-    private readonly Dictionary<(string ProjectFilePath, string Capability), bool> _cachedCapabilities = [];
-    private readonly object _gate = new();
-
     [ImportingConstructor]
     public ProjectCapabilityResolver(
         ILiveShareSessionAccessor liveShareSessionAccessor,
         IVsService<SVsUIShellOpenDocument, IVsUIShellOpenDocument> vsUIShellOpenDocumentService,
+        [ImportMany] IEnumerable<IProjectCapabilityListener> projectCapabilityListeners,
         ILoggerFactory loggerFactory,
         JoinableTaskContext joinableTaskContext)
     {
         _liveShareSessionAccessor = liveShareSessionAccessor;
+        _projectCapabilityListeners = projectCapabilityListeners;
         _jtf = joinableTaskContext.Factory;
         _logger = loggerFactory.GetOrCreateLogger<ProjectCapabilityResolver>();
         _disposeTokenSource = new();
@@ -131,9 +131,10 @@ private bool ContainingProjectHasCapability(string capability, string documentFi
 
             if (vsHierarchy.GetProjectFilePath(_jtf) is { } projectFilePath)
             {
-                lock (_gate)
+                foreach (var listener in _projectCapabilityListeners)
                 {
-                    _cachedCapabilities[(projectFilePath, capability)] = isMatch;
+                    // Notify all listeners of the capability match.
+                    listener.OnProjectCapabilityMatched(projectFilePath, capability, isMatch);
                 }
             }
         }
@@ -149,12 +150,4 @@ private bool ContainingProjectHasCapability(string capability, string documentFi
 
         return isMatch;
     }
-
-    public bool TryGetCachedCapabilityMatch(string projectFilePath, string capability, out bool isMatch)
-    {
-        lock (_gate)
-        {
-            return _cachedCapabilities.TryGetValue((projectFilePath, capability), out isMatch);
-        }
-    }
 }