Switch to primary constructor

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/FileWatching/DelegatingFileChangeWatcher.cs

@@ -2,54 +2,44 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Collections.Immutable;
 using System.Composition;
+using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.CodeAnalysis.LanguageServer.LanguageServer;
 using Microsoft.CodeAnalysis.ProjectSystem;
-using Microsoft.Extensions.Logging;
-using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.CodeAnalysis.Shared.TestHooks;
-using System.Collections.Immutable;
+using Microsoft.Extensions.Logging;
 
 namespace Microsoft.CodeAnalysis.LanguageServer.HostWorkspace.FileWatching;
 
 /// <summary>
-/// A MEF export for <see cref="IFileChangeWatcher" />. This checks if we're able to create an <see cref="LspFileChangeWatcher" /> if the client supports
-/// file watching. If we do, we create that and delegate to it. Otherwise we use a <see cref="SimpleFileChangeWatcher" />.
+/// A MEF export for <see cref="IFileChangeWatcher" />. This checks if we're able to create an <see
+/// cref="LspFileChangeWatcher" /> if the client supports file watching. If we do, we create that and delegate to it.
+/// Otherwise we use a <see cref="SimpleFileChangeWatcher" />.
 /// </summary>
 /// <remarks>
-/// LSP clients don't always support file watching; this allows us to be flexible and use it when we can, but fall back to something else if we can't.
+/// LSP clients don't always support file watching; this allows us to be flexible and use it when we can, but fall back
+/// to something else if we can't.
 /// </remarks>
 [Export(typeof(IFileChangeWatcher)), Shared]
-internal sealed class DelegatingFileChangeWatcher : IFileChangeWatcher
+[method: ImportingConstructor]
+[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
+internal sealed class DelegatingFileChangeWatcher(
+    ILoggerFactory loggerFactory,
+    IAsynchronousOperationListenerProvider asynchronousOperationListenerProvider)
+    : IFileChangeWatcher
 {
-    private readonly ILoggerFactory _loggerFactory;
-    private readonly IAsynchronousOperationListenerProvider _asynchronousOperationListenerProvider;
-    private readonly Lazy<IFileChangeWatcher> _underlyingFileWatcher;
-
-    [ImportingConstructor]
-    [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
-    public DelegatingFileChangeWatcher(ILoggerFactory loggerFactory, IAsynchronousOperationListenerProvider asynchronousOperationListenerProvider)
-    {
-        _loggerFactory = loggerFactory;
-        _asynchronousOperationListenerProvider = asynchronousOperationListenerProvider;
-        _underlyingFileWatcher = new Lazy<IFileChangeWatcher>(CreateFileWatcher);
-    }
-
-    private IFileChangeWatcher CreateFileWatcher()
+    private readonly Lazy<IFileChangeWatcher> _underlyingFileWatcher = new(() =>
     {
         // Do we already have an LSP client that we can confirm works for us?
         var instance = LanguageServerHost.Instance;
 
         if (instance != null && LspFileChangeWatcher.SupportsLanguageServerHost(instance))
-        {
-            return new LspFileChangeWatcher(instance, _asynchronousOperationListenerProvider);
-        }
-        else
-        {
-            _loggerFactory.CreateLogger<DelegatingFileChangeWatcher>().LogWarning("We are unable to use LSP file watching; falling back to our in-process watcher.");
-            return new SimpleFileChangeWatcher();
-        }
-    }
+            return new LspFileChangeWatcher(instance, asynchronousOperationListenerProvider);
+
+        loggerFactory.CreateLogger<DelegatingFileChangeWatcher>().LogWarning("We are unable to use LSP file watching; falling back to our in-process watcher.");
+        return new SimpleFileChangeWatcher();
+    });
 
     public IFileChangeContext CreateContext(ImmutableArray<WatchedDirectory> watchedDirectories)
         => _underlyingFileWatcher.Value.CreateContext(watchedDirectories);

src/Workspaces/Core/Portable/Workspace/ProjectSystem/FileWatchedPortableExecutableReferenceFactory.cs

@@ -5,12 +5,10 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
-using System.Linq;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.CodeAnalysis.Host;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.ProjectSystem;
@@ -20,29 +18,32 @@ internal sealed class FileWatchedPortableExecutableReferenceFactory
     private readonly object _gate = new();
 
     /// <summary>
-    /// A file change context used to watch metadata references. This is lazy to avoid creating this immediately during our LSP process startup, when we
-    /// don't yet know the LSP client's capabilities.
+    /// A file change context used to watch metadata references. This is lazy to avoid creating this immediately during
+    /// our LSP process startup, when we don't yet know the LSP client's capabilities.
     /// </summary>
     private readonly Lazy<IFileChangeContext> _fileReferenceChangeContext;
 
     /// <summary>
-    /// File watching tokens from <see cref="_fileReferenceChangeContext"/> that are watching metadata references. These are only created once we are actually applying a batch because
-    /// we don't determine until the batch is applied if the file reference will actually be a file reference or it'll be a converted project reference.
+    /// File watching tokens from <see cref="_fileReferenceChangeContext"/> that are watching metadata references. These
+    /// are only created once we are actually applying a batch because we don't determine until the batch is applied if
+    /// the file reference will actually be a file reference or it'll be a converted project reference.
     /// </summary>
     private readonly Dictionary<PortableExecutableReference, (IWatchedFile Token, int RefCount)> _metadataReferenceFileWatchingTokens = [];
 
     /// <summary>
-    /// Stores the caller for a previous disposal of a reference produced by this class, to track down a double-dispose issue.
+    /// Stores the caller for a previous disposal of a reference produced by this class, to track down a double-dispose
+    /// issue.
     /// </summary>
     /// <remarks>
     /// This can be removed once https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1843611 is fixed.
     /// </remarks>
     private readonly ConditionalWeakTable<PortableExecutableReference, string> _previousDisposalLocations = new();
 
     /// <summary>
-    /// <see cref="CancellationTokenSource"/>s for in-flight refreshing of metadata references. When we see a file change, we wait a bit before trying to actually
-    /// update the workspace. We need cancellation tokens for those so we can cancel them either when a flurry of events come in (so we only do the delay after the last
-    /// modification), or when we know the project is going away entirely.
+    /// <see cref="CancellationTokenSource"/>s for in-flight refreshing of metadata references. When we see a file
+    /// change, we wait a bit before trying to actually update the workspace. We need cancellation tokens for those so
+    /// we can cancel them either when a flurry of events come in (so we only do the delay after the last modification),
+    /// or when we know the project is going away entirely.
     /// </summary>
     private readonly Dictionary<string, CancellationTokenSource> _metadataReferenceRefreshCancellationTokenSources = [];
 
@@ -53,11 +54,12 @@ public FileWatchedPortableExecutableReferenceFactory(
         {
             var referenceDirectories = new HashSet<string>();
 
-            // On each platform, there is a place that reference assemblies for the framework are installed. These are rarely going to be changed
-            // but are the most common places that we're going to create file watches. Rather than either creating a huge number of file watchers
-            // for every single file, or eventually realizing we should just watch these directories, we just create the single directory watchers now.
-            // We'll collect this from two places: constructing it from known environment variables, and also for the defaults where those environment
-            // variables would usually point, as a fallback.
+            // On each platform, there is a place that reference assemblies for the framework are installed. These are
+            // rarely going to be changed but are the most common places that we're going to create file watches. Rather
+            // than either creating a huge number of file watchers for every single file, or eventually realizing we
+            // should just watch these directories, we just create the single directory watchers now. We'll collect this
+            // from two places: constructing it from known environment variables, and also for the defaults where those
+            // environment variables would usually point, as a fallback.
 
             if (Environment.GetEnvironmentVariable("DOTNET_ROOT") is string dotnetRoot && !string.IsNullOrEmpty(dotnetRoot))
             {
@@ -79,9 +81,9 @@ public FileWatchedPortableExecutableReferenceFactory(
                 referenceDirectories.Add("/usr/local/share/dotnet/packs");
             }
 
-            // Also watch the NuGet restore path; we don't do this (yet) on Windows due to potential concerns about whether
-            // this creates additional overhead responding to changes during a restore.
-            // TODO: remove this condition
+            // Also watch the NuGet restore path; we don't do this (yet) on Windows due to potential concerns about
+            // whether this creates additional overhead responding to changes during a restore. TODO: remove this
+            // condition
             if (!PlatformInformation.IsWindows)
             {
                 referenceDirectories.Add(Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".nuget", "packages"));
@@ -97,9 +99,9 @@ public FileWatchedPortableExecutableReferenceFactory(
     public event EventHandler<string>? ReferenceChanged;
 
     /// <summary>
-    /// Starts watching a particular PortableExecutableReference for changes to the file.
-    /// If this is already being watched , the reference count will be incremented.
-    /// This is *not* safe to attempt to call multiple times for the same project and reference (e.g. in applying workspace updates)
+    /// Starts watching a particular PortableExecutableReference for changes to the file. If this is already being
+    /// watched , the reference count will be incremented. This is *not* safe to attempt to call multiple times for the
+    /// same project and reference (e.g. in applying workspace updates)
     /// </summary>
     public void StartWatchingReference(PortableExecutableReference reference, string fullFilePath)
     {
@@ -115,9 +117,9 @@ public void StartWatchingReference(PortableExecutableReference reference, string
     }
 
     /// <summary>
-    /// Decrements the reference count for the given PortableExecutableReference.
-    /// When the reference count reaches 0, the file watcher will be stopped.
-    /// This is *not* safe to attempt to call multiple times for the same project and reference (e.g. in applying workspace updates)
+    /// Decrements the reference count for the given PortableExecutableReference. When the reference count reaches 0,
+    /// the file watcher will be stopped. This is *not* safe to attempt to call multiple times for the same project and
+    /// reference (e.g. in applying workspace updates)
     /// </summary>
     public void StopWatchingReference(PortableExecutableReference reference, [CallerFilePath] string callerFilePath = "", [CallerLineNumber] int callerLineNumber = 0)
     {
@@ -147,19 +149,17 @@ public void StopWatchingReference(PortableExecutableReference reference, [Caller
                 _metadataReferenceFileWatchingTokens[reference] = (watchedFileReference.Token, newRefCount);
             }
 
-            // Note we still potentially have an outstanding change that we haven't raised a notification
-            // for due to the delay we use. We could cancel the notification for that file path,
-            // but we may still have another outstanding PortableExecutableReference that isn't this one
-            // that does want that notification. We're OK just leaving the delay still running for two
-            // reasons:
+            // Note we still potentially have an outstanding change that we haven't raised a notification for due to the
+            // delay we use. We could cancel the notification for that file path, but we may still have another
+            // outstanding PortableExecutableReference that isn't this one that does want that notification. We're OK
+            // just leaving the delay still running for two reasons:
             //
-            // 1. Technically, we did see a file change before the call to StopWatchingReference, so
-            //    arguably we should still raise it.
-            // 2. Since we raise the notification for a file path, it's up to the consumer of this to still
-            //    track down which actual reference needs to be changed. That'll automatically handle any
-            //    race where the event comes late, which is a scenario this must always deal with no matter
-            //    what -- another thread might already be gearing up to notify the caller of this reference
-            //    and we can't stop it.
+            // 1. Technically, we did see a file change before the call to StopWatchingReference, so arguably we should
+            //    still raise it.
+            // 2. Since we raise the notification for a file path, it's up to the consumer of this to still track down
+            //    which actual reference needs to be changed. That'll automatically handle any race where the event
+            //    comes late, which is a scenario this must always deal with no matter what -- another thread might
+            //    already be gearing up to notify the caller of this reference and we can't stop it.
         }
     }
 

src/Workspaces/Core/Portable/Workspace/ProjectSystem/ProjectSystemProjectFactory.ProjectUpdateState.cs

@@ -10,26 +10,27 @@ namespace Microsoft.CodeAnalysis.Workspaces.ProjectSystem;
 internal sealed partial class ProjectSystemProjectFactory
 {
     /// <summary>
-    /// Immutable data type that holds the current state of the project system factory as well as
-    /// storing any incremental state changes in the current workspace update.
+    /// Immutable data type that holds the current state of the project system factory as well as storing any
+    /// incremental state changes in the current workspace update.
     /// 
-    /// This state is updated by various project system update operations under the <see cref="_gate"/>.
-    /// Importantly, this immutable type allows us to discard updates to the state that fail to apply
-    /// due to interceding workspace operations.
+    /// This state is updated by various project system update operations under the <see cref="_gate"/>. Importantly,
+    /// this immutable type allows us to discard updates to the state that fail to apply due to interceding workspace
+    /// operations.
     /// 
     /// There are two kinds of state that this type holds that need to support discarding:
-    ///   1.  Global state for the <see cref="ProjectSystemProjectFactory"/> (various maps of project information).
-    ///       This state must be saved between different changes.
-    ///   2.  Incremental state for the current change being processed.  This state has information that is
-    ///       cannot be resilient to being applied multiple times during the workspace update, so is saved
-    ///       to be applied only once the workspace update is successful.
-    ///
+    /// <list type="number">
+    /// <item>Global state for the <see cref="ProjectSystemProjectFactory"/> (various maps of project information). This
+    /// state must be saved between different changes.</item>
+    /// <item>Incremental state for the current change being processed.  This state has information that is cannot be
+    /// resilient to being applied multiple times during the workspace update, so is saved to be applied only once the
+    /// workspace update is successful.</item>
+    /// </list>
     /// </summary>
     /// <param name="ProjectsByOutputPath">
-    /// Global state representing a multimap from an output path to the project outputting to it. Ideally, this shouldn't ever
-    /// actually be a true multimap, since we shouldn't have two projects outputting to the same path, but
-    /// any bug by a project adding the wrong output path means we could end up with some duplication.
-    /// In that case, we'll temporarily have two until (hopefully) somebody removes it.
+    /// Global state representing a multimap from an output path to the project outputting to it. Ideally, this
+    /// shouldn't ever actually be a true multimap, since we shouldn't have two projects outputting to the same path,
+    /// but any bug by a project adding the wrong output path means we could end up with some duplication. In that case,
+    /// we'll temporarily have two until (hopefully) somebody removes it.
     /// </param>
     /// <param name="ProjectReferenceInfos">
     /// Global state containing output paths and converted project reference information for each project.