Merge pull request #42965 from CyrusNajmabadi/codeModelAnalyzer

Remove the CodeModel incremental analyzer.

src/VisualStudio/CSharp/Test/CodeModel/FileCodeModelTestHelpers.cs

@@ -12,6 +12,8 @@
 using Microsoft.VisualStudio.LanguageServices.UnitTests;
 using static Microsoft.VisualStudio.LanguageServices.UnitTests.CodeModel.CodeModelTestHelpers;
 using Microsoft.VisualStudio.LanguageServices.UnitTests.CodeModel.Mocks;
+using Microsoft.CodeAnalysis.Editor;
+using Microsoft.CodeAnalysis.Shared.TestHooks;
 
 namespace Microsoft.VisualStudio.LanguageServices.CSharp.UnitTests.CodeModel
 {
@@ -38,13 +40,20 @@ internal static class FileCodeModelTestHelpers
 
                 var visualStudioWorkspaceMock = new MockVisualStudioWorkspace(workspace);
                 var threadingContext = workspace.ExportProvider.GetExportedValue<IThreadingContext>();
+                var notificationService = workspace.ExportProvider.GetExportedValue<IForegroundNotificationService>();
+                var listenerProvider = workspace.ExportProvider.GetExportedValue<AsynchronousOperationListenerProvider>();
 
                 var state = new CodeModelState(
                     threadingContext,
                     serviceProvider,
                     project.LanguageServices,
                     visualStudioWorkspaceMock,
-                    new ProjectCodeModelFactory(visualStudioWorkspaceMock, serviceProvider, threadingContext));
+                    new ProjectCodeModelFactory(
+                        visualStudioWorkspaceMock,
+                        serviceProvider,
+                        threadingContext,
+                        notificationService,
+                        listenerProvider));
 
                 var codeModel = FileCodeModel.Create(state, null, document, new MockTextManagerAdapter()).Handle;
 

src/VisualStudio/Core/Def/Packaging/PackageInstallerServiceFactory.cs

@@ -181,8 +181,7 @@ protected override void StartWorking()
             }
 
             OnSourceProviderSourcesChanged(this, EventArgs.Empty);
-            OnWorkspaceChanged(null, new WorkspaceChangeEventArgs(
-                WorkspaceChangeKind.SolutionAdded, null, null));
+            OnWorkspaceChanged(localSolutionChanged: true, localChangedProject: null);
         }
 
         private void OnSourceProviderSourcesChanged(object sender, EventArgs e)
@@ -348,8 +347,8 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e)
         {
             ThisCanBeCalledOnAnyThread();
 
-            var localSolutionChanged = false;
-            ProjectId localChangedProject = null;
+            var solutionChanged = false;
+            ProjectId chnagedProject = null;
             switch (e.Kind)
             {
                 default:
@@ -360,7 +359,7 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e)
                 case WorkspaceChangeKind.ProjectChanged:
                 case WorkspaceChangeKind.ProjectReloaded:
                 case WorkspaceChangeKind.ProjectRemoved:
-                    localChangedProject = e.ProjectId;
+                    chnagedProject = e.ProjectId;
                     break;
 
                 case WorkspaceChangeKind.SolutionAdded:
@@ -368,10 +367,15 @@ private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e)
                 case WorkspaceChangeKind.SolutionCleared:
                 case WorkspaceChangeKind.SolutionReloaded:
                 case WorkspaceChangeKind.SolutionRemoved:
-                    localSolutionChanged = true;
+                    solutionChanged = true;
                     break;
             }
 
+            this.OnWorkspaceChanged(solutionChanged, chnagedProject);
+        }
+
+        private void OnWorkspaceChanged(bool localSolutionChanged, ProjectId localChangedProject)
+        {
             lock (_gate)
             {
                 // Augment the data that the foreground thread will process.

src/VisualStudio/Core/Impl/CodeModel/CodeModelIncrementalAnalyzer.cs

-// Licensed to the .NET Foundation under one or more agreements.
-// 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;
-using System.Collections.Generic;
-using System.Composition;
-using System.Threading;
-using System.Threading.Tasks;
-using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.Editor;
-using Microsoft.CodeAnalysis.Host.Mef;
-using Microsoft.CodeAnalysis.Options;
-using Microsoft.CodeAnalysis.Shared.TestHooks;
-using Microsoft.CodeAnalysis.SolutionCrawler;
-using Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem;
-using Roslyn.Utilities;
-
-namespace Microsoft.VisualStudio.LanguageServices.Implementation.CodeModel
-{
-    [ExportIncrementalAnalyzerProvider(nameof(CodeModelIncrementalAnalyzerProvider), new[] { WorkspaceKind.Host }), Shared]
-    internal class CodeModelIncrementalAnalyzerProvider : IIncrementalAnalyzerProvider
-    {
-        private readonly IAsynchronousOperationListener _listener;
-        private readonly IForegroundNotificationService _notificationService;
-        private readonly ProjectCodeModelFactory _projectCodeModelFactory;
-
-        [ImportingConstructor]
-        [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
-        public CodeModelIncrementalAnalyzerProvider(
-            IForegroundNotificationService notificationService,
-            IAsynchronousOperationListenerProvider listenerProvider,
-            ProjectCodeModelFactory projectCodeModelFactory)
-        {
-            _listener = listenerProvider.GetListener(FeatureAttribute.CodeModel);
-            _notificationService = notificationService;
-            _projectCodeModelFactory = projectCodeModelFactory;
-        }
-
-        public IIncrementalAnalyzer CreateIncrementalAnalyzer(Workspace workspace)
-        {
-            if (!(workspace is VisualStudioWorkspace visualStudioWorkspace))
-            {
-                return null;
-            }
-
-            return new Analyzer(_notificationService, _listener, visualStudioWorkspace, _projectCodeModelFactory);
-        }
-
-        private class Analyzer : IIncrementalAnalyzer
-        {
-            private readonly IForegroundNotificationService _notificationService;
-            private readonly IAsynchronousOperationListener _listener;
-            private readonly VisualStudioWorkspace _workspace;
-            private readonly ProjectCodeModelFactory _projectCodeModelFactory;
-
-            public Analyzer(IForegroundNotificationService notificationService, IAsynchronousOperationListener listener, VisualStudioWorkspace workspace, ProjectCodeModelFactory projectCodeModelFactory)
-            {
-                _notificationService = notificationService;
-                _listener = listener;
-                _workspace = workspace;
-                _projectCodeModelFactory = projectCodeModelFactory;
-            }
-
-            public Task AnalyzeSyntaxAsync(Document document, InvocationReasons reasons, CancellationToken cancellationToken)
-            {
-                FireEvents(document.Id, cancellationToken);
-
-                return Task.CompletedTask;
-            }
-
-            public Task RemoveDocumentAsync(DocumentId documentId, CancellationToken cancellationToken)
-            {
-                // REVIEW: do we need to fire events when a document is removed from the solution?
-                FireEvents(documentId, CancellationToken.None);
-                return Task.CompletedTask;
-            }
-
-            public void FireEvents(DocumentId documentId, CancellationToken cancellationToken)
-            {
-                _notificationService.RegisterNotification(() =>
-                {
-                    var projectCodeModel = _projectCodeModelFactory.TryGetProjectCodeModel(documentId.ProjectId);
-
-                    if (projectCodeModel == null)
-                    {
-                        return false;
-                    }
-
-                    var filename = _workspace.GetFilePath(documentId);
-                    if (filename == null)
-                    {
-                        return false;
-                    }
-
-                    if (!projectCodeModel.TryGetCachedFileCodeModel(filename, out var fileCodeModelHandle))
-                    {
-                        return false;
-                    }
-
-                    var codeModel = fileCodeModelHandle.Object;
-                    return codeModel.FireEvents();
-                },
-                _listener.BeginAsyncOperation("CodeModelEvent"),
-                cancellationToken);
-            }
-
-            #region unused
-            public Task NewSolutionSnapshotAsync(Solution solution, CancellationToken cancellationToken)
-                => Task.CompletedTask;
-
-            public Task DocumentOpenAsync(Document document, CancellationToken cancellationToken)
-                => Task.CompletedTask;
-
-            public Task DocumentCloseAsync(Document document, CancellationToken cancellationToken)
-                => Task.CompletedTask;
-
-            public Task DocumentResetAsync(Document document, CancellationToken cancellationToken)
-                => Task.CompletedTask;
-
-            public bool NeedsReanalysisOnOptionChanged(object sender, OptionChangedEventArgs e)
-                => false;
-
-            public Task AnalyzeDocumentAsync(Document document, SyntaxNode bodyOpt, InvocationReasons reasons, CancellationToken cancellationToken)
-                => Task.CompletedTask;
-
-            public Task AnalyzeProjectAsync(Project project, bool semanticsChanged, InvocationReasons reasons, CancellationToken cancellationToken)
-                => Task.CompletedTask;
-
-            public Task RemoveProjectAsync(ProjectId projectId, CancellationToken cancellationToken)
-                => Task.CompletedTask;
-            #endregion
-        }
-    }
-}

src/VisualStudio/Core/Impl/CodeModel/FileCodeModel_Events.cs

@@ -20,11 +20,9 @@ public sealed partial class FileCodeModel
 
         public bool FireEvents()
         {
-            var needMoreTime = false;
-
             _codeElementTable.CleanUpDeadObjects();
-            needMoreTime = _codeElementTable.NeedsCleanUp;
 
+            var needMoreTime = _codeElementTable.NeedsCleanUp;
             if (this.IsZombied)
             {
                 // file is removed from the solution. this can happen if a fireevent is enqueued to foreground notification service
@@ -71,6 +69,8 @@ public bool FireEvents()
             }
 
             var extensibility = (EnvDTE80.IVsExtensibility2)this.State.ServiceProvider.GetService(typeof(EnvDTE.IVsExtensibility));
+            if (extensibility == null)
+                return false;
 
             foreach (var codeModelEvent in eventQueue)
             {

src/VisualStudio/Core/Impl/CodeModel/ProjectCodeModelFactory.cs

@@ -2,15 +2,24 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+#nullable enable
+
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.ComponentModel.Composition;
 using System.Diagnostics.CodeAnalysis;
+using System.Threading;
+using EnvDTE80;
 using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Editor;
 using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
+using Microsoft.CodeAnalysis.Shared.TestHooks;
+using Microsoft.CodeAnalysis.SolutionCrawler;
 using Microsoft.VisualStudio.Shell;
 using Microsoft.VisualStudio.Threading;
+using Roslyn.Utilities;
 
 namespace Microsoft.VisualStudio.LanguageServices.Implementation.CodeModel
 {
@@ -32,15 +41,125 @@ internal sealed class ProjectCodeModelFactory : IProjectCodeModelFactory, IDispo
         /// </summary>
         private readonly JoinableTaskCollection _deferredCleanupTasks;
 
+        /// <summary>
+        /// Cancellation token that controls existing async work that we have kicked off.  Canceled when we're finally
+        /// disposed.
+        /// </summary>
+        private readonly CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource();
+
+        private readonly IForegroundNotificationService _notificationService;
+        private readonly IAsynchronousOperationListener _listener;
+        private readonly AsyncBatchingWorkQueue<DocumentId> _documentsToFireEventsFor;
+
         [ImportingConstructor]
         [SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
-        public ProjectCodeModelFactory(VisualStudioWorkspace visualStudioWorkspace, [Import(typeof(SVsServiceProvider))] IServiceProvider serviceProvider, IThreadingContext threadingContext)
+        public ProjectCodeModelFactory(
+            VisualStudioWorkspace visualStudioWorkspace,
+            [Import(typeof(SVsServiceProvider))] IServiceProvider serviceProvider,
+            IThreadingContext threadingContext,
+            IForegroundNotificationService notificationService,
+            IAsynchronousOperationListenerProvider listenerProvider)
         {
             _visualStudioWorkspace = visualStudioWorkspace;
             _serviceProvider = serviceProvider;
             _threadingContext = threadingContext;
             _deferredCleanupTasks = new JoinableTaskCollection(threadingContext.JoinableTaskContext);
             _deferredCleanupTasks.DisplayName = nameof(ProjectCodeModelFactory) + "." + nameof(_deferredCleanupTasks);
+
+            _notificationService = notificationService;
+            _listener = listenerProvider.GetListener(FeatureAttribute.CodeModel);
+
+            // Queue up notifications we hear about docs changing.  that way we don't have to fire events multiple times
+            // for the same documents.  Once enough time has passed, take the documents that were changed and run
+            // through them, firing their latest events.
+            _documentsToFireEventsFor = new AsyncBatchingWorkQueue<DocumentId>(
+                TimeSpan.FromMilliseconds(visualStudioWorkspace.Options.GetOption(InternalSolutionCrawlerOptions.AllFilesWorkerBackOffTimeSpanInMS)),
+                ProcessNextDocumentBatchAsync,
+                // We only care about unique doc-ids, so pass in this comparer to collapse streams of changes for a
+                // single document down to one notification.
+                EqualityComparer<DocumentId>.Default,
+                _listener,
+                _cancellationTokenSource.Token);
+
+            _visualStudioWorkspace.WorkspaceChanged += OnWorkspaceChanged;
+        }
+
+        private System.Threading.Tasks.Task ProcessNextDocumentBatchAsync(
+            ImmutableArray<DocumentId> documentIds, CancellationToken cancellationToken)
+        {
+            foreach (var documentId in documentIds)
+            {
+                // Now, enqueue foreground work to actually process these documents in a serialized and incremental
+                // fashion.  FireEventsForDocument will actually limit how much time it spends firing events so that it
+                // doesn't saturate  the UI thread.
+                _notificationService.RegisterNotification(
+                    () => FireEventsForDocument(documentId),
+                    _listener.BeginAsyncOperation("CodeModelEvent"),
+                    cancellationToken);
+            }
+
+            return System.Threading.Tasks.Task.CompletedTask;
+
+            bool FireEventsForDocument(DocumentId documentId)
+            {
+                // If we've been asked to shutdown, don't bother reporting any more events.
+                if (_cancellationTokenSource.IsCancellationRequested)
+                    return false;
+
+                var projectCodeModel = this.TryGetProjectCodeModel(documentId.ProjectId);
+                if (projectCodeModel == null)
+                    return false;
+
+                var filename = _visualStudioWorkspace.GetFilePath(documentId);
+                if (filename == null)
+                    return false;
+
+                if (!projectCodeModel.TryGetCachedFileCodeModel(filename, out var fileCodeModelHandle))
+                    return false;
+
+                var codeModel = fileCodeModelHandle.Object;
+                return codeModel.FireEvents();
+            }
+        }
+
+        private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e)
+        {
+            // Events that can't change existing code model items.  Can just ignore them.
+            switch (e.Kind)
+            {
+                case WorkspaceChangeKind.SolutionAdded:
+                case WorkspaceChangeKind.ProjectAdded:
+                case WorkspaceChangeKind.DocumentAdded:
+                case WorkspaceChangeKind.AdditionalDocumentAdded:
+                case WorkspaceChangeKind.AdditionalDocumentRemoved:
+                case WorkspaceChangeKind.AdditionalDocumentReloaded:
+                case WorkspaceChangeKind.AdditionalDocumentChanged:
+                case WorkspaceChangeKind.AnalyzerConfigDocumentAdded:
+                case WorkspaceChangeKind.AnalyzerConfigDocumentRemoved:
+                case WorkspaceChangeKind.AnalyzerConfigDocumentReloaded:
+                case WorkspaceChangeKind.AnalyzerConfigDocumentChanged:
+                    return;
+                case WorkspaceChangeKind.DocumentRemoved:
+                case WorkspaceChangeKind.DocumentChanged:
+                    // Fast path when we know we affected a document that could have had code model elements in it.  No
+                    // need to do a solution diff in this case.
+                    _documentsToFireEventsFor.AddWork(e.DocumentId!);
+                    return;
+            }
+
+            // Other type of event that could indicate a doc change/removal. Have to actually analyze the change to
+            // determine what we should do here.
+
+            var changes = e.OldSolution.GetChanges(e.NewSolution);
+
+            foreach (var project in changes.GetRemovedProjects())
+                _documentsToFireEventsFor.AddWork(project.DocumentIds);
+
+            foreach (var projectChange in changes.GetProjectChanges())
+            {
+                _documentsToFireEventsFor.AddWork(projectChange.GetRemovedDocuments());
+                _documentsToFireEventsFor.AddWork(projectChange.GetChangedDocuments());
+            }
         }
 
         public IProjectCodeModel CreateProjectCodeModel(ProjectId id, ICodeModelInstanceFactory codeModelInstanceFactory)
@@ -83,6 +202,12 @@ public void ScheduleDeferredCleanupTask(Action a)
             => _deferredCleanupTasks.Add(_threadingContext.JoinableTaskFactory.StartOnIdle(a, VsTaskRunContext.UIThreadNormalPriority));
 
         void IDisposable.Dispose()
-            => _deferredCleanupTasks.Join();
+        {
+            // Stop any outstanding BG work we've queued up.
+            _cancellationTokenSource.Cancel();
+
+            // Now wait for all our cleanup tasks to finish before we return.
+            _deferredCleanupTasks.Join();
+        }
     }
 }

src/VisualStudio/TestUtilities2/CodeModel/CodeModelTestHelpers.vb

@@ -5,9 +5,13 @@
 Imports System.Runtime.CompilerServices
 Imports System.Runtime.ExceptionServices
 Imports System.Runtime.InteropServices
+Imports EnvDTE
+Imports EnvDTE80
 Imports Microsoft.CodeAnalysis
+Imports Microsoft.CodeAnalysis.Editor
 Imports Microsoft.CodeAnalysis.Editor.Shared.Utilities
 Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
+Imports Microsoft.CodeAnalysis.Shared.TestHooks
 Imports Microsoft.CodeAnalysis.Test.Utilities
 Imports Microsoft.VisualStudio.ComponentModelHost
 Imports Microsoft.VisualStudio.LanguageServices.Implementation.CodeModel
@@ -52,13 +56,20 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.CodeModel
                 Dim project = workspace.CurrentSolution.Projects.Single()
 
                 Dim threadingContext = workspace.ExportProvider.GetExportedValue(Of IThreadingContext)
+                Dim notificationService = workspace.ExportProvider.GetExportedValue(Of IForegroundNotificationService)
+                Dim listenerProvider = workspace.ExportProvider.GetExportedValue(Of AsynchronousOperationListenerProvider)()
 
                 Dim state = New CodeModelState(
                     threadingContext,
                     mockServiceProvider,
                     project.LanguageServices,
                     mockVisualStudioWorkspace,
-                    New ProjectCodeModelFactory(mockVisualStudioWorkspace, mockServiceProvider, threadingContext))
+                    New ProjectCodeModelFactory(
+                        mockVisualStudioWorkspace,
+                        mockServiceProvider,
+                        threadingContext,
+                        notificationService,
+                        listenerProvider))
 
                 Dim projectCodeModel = DirectCast(state.ProjectCodeModelFactory.CreateProjectCodeModel(project.Id, Nothing), ProjectCodeModel)
 
@@ -88,7 +99,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.CodeModel
             Private ReadOnly _componentModel As MockComponentModel
 
             Public Sub New(componentModel As MockComponentModel)
-                Me._componentModel = componentModel
+                _componentModel = componentModel
             End Sub
 
             Public Function GetService(serviceType As Type) As Object Implements IServiceProvider.GetService
@@ -96,6 +107,10 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.CodeModel
                     Return Me._componentModel
                 End If
 
+                If serviceType = GetType(EnvDTE.IVsExtensibility) Then
+                    Return Nothing
+                End If
+
                 Throw New NotImplementedException($"No service exists for {serviceType.FullName}")
             End Function
         End Class
@@ -132,7 +147,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.CodeModel
         End Class
 
         <Extension()>
-        Public Function GetDocumentAtCursor(state As CodeModelTestState) As Document
+        Public Function GetDocumentAtCursor(state As CodeModelTestState) As Microsoft.CodeAnalysis.Document
             Dim cursorDocument = state.Workspace.Documents.First(Function(d) d.CursorPosition.HasValue)
 
             Dim document = state.Workspace.CurrentSolution.GetDocument(cursorDocument.Id)

src/Workspaces/Core/Portable/SemanticModelWorkspaceService/SemanticModelWorkspaceServiceFactory.SemanticModelWorkspaceService.cs

@@ -292,10 +292,10 @@ private void OnWorkspaceChanged(object? sender, WorkspaceChangeEventArgs e)
                     case WorkspaceChangeKind.ProjectRemoved:
                     case WorkspaceChangeKind.ProjectChanged:
                     case WorkspaceChangeKind.ProjectReloaded:
-                        ClearVersionMap(e.NewSolution.Workspace, e.ProjectId);
+                        ClearVersionMap(e.NewSolution.Workspace, e.ProjectId!);
                         break;
                     case WorkspaceChangeKind.DocumentRemoved:
-                        ClearVersionMap(e.NewSolution.Workspace, e.DocumentId);
+                        ClearVersionMap(e.NewSolution.Workspace, e.DocumentId!);
                         break;
                     case WorkspaceChangeKind.DocumentAdded:
                     case WorkspaceChangeKind.DocumentReloaded:

src/Workspaces/Core/Portable/Workspace/WorkspaceChangeEventArgs.cs

@@ -2,6 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+#nullable enable
+
 using System;
 
 namespace Microsoft.CodeAnalysis
@@ -21,25 +23,55 @@ public class WorkspaceChangeEventArgs : EventArgs
         public WorkspaceChangeKind Kind { get; }
 
         /// <remarks>
-        /// If linked documents are being changed, there may be multiple events with the same
-        /// <see cref="OldSolution"/> and <see cref="NewSolution"/>.
+        /// If linked documents are being changed, there may be multiple events with the same <see cref="OldSolution"/>
+        /// and <see cref="NewSolution"/>.  Note that the workspace starts with its solution set to an empty solution.
+        /// <see cref="WorkspaceChangeKind.SolutionAdded"/> replaces the previous solution, which might be the empty
+        /// one.
         /// </remarks>
         public Solution OldSolution { get; }
 
         /// <remarks>
-        /// If linked documents are being changed, there may be multiple events with the same
-        /// <see cref="OldSolution"/> and <see cref="NewSolution"/>.
+        /// If linked documents are being changed, there may be multiple events with the same <see cref="OldSolution"/>
+        /// and <see cref="NewSolution"/>. Note <see cref="WorkspaceChangeKind.SolutionRemoved"/> replaces the previous
+        /// solution with the empty one.
         /// </remarks>
         public Solution NewSolution { get; }
 
-        public ProjectId ProjectId { get; }
-        public DocumentId DocumentId { get; }
+        /// <summary>
+        /// The id of the affected <see cref="Project"/>.  Can be <see langword="null"/> if this is an change unrelated
+        /// to a project (for example <see cref="WorkspaceChangeKind.SolutionReloaded"/>.  Should be non-<see
+        /// langword="null"/> for:
+        /// <list type="bullet">
+        /// <item><see cref="WorkspaceChangeKind.ProjectAdded"/></item>
+        /// <item><see cref="WorkspaceChangeKind.ProjectChanged"/></item>
+        /// <item><see cref="WorkspaceChangeKind.ProjectReloaded"/></item>
+        /// <item><see cref="WorkspaceChangeKind.ProjectRemoved"/></item>
+        /// </list>
+        /// </summary>
+        public ProjectId? ProjectId { get; }
+
+        /// <summary>
+        /// The id of the affected <see cref="Document"/>.  Can be <see langword="null"/> if this is an change unrelated
+        /// to a document (for example <see cref="WorkspaceChangeKind.ProjectAdded"/>. Should be non-<see
+        /// langword="null"/> for:
+        /// <list type="bullet">
+        /// <item><see cref="WorkspaceChangeKind.DocumentAdded"/></item>
+        /// <item><see cref="WorkspaceChangeKind.DocumentChanged"/></item>
+        /// <item><see cref="WorkspaceChangeKind.DocumentInfoChanged"/></item>
+        /// <item><see cref="WorkspaceChangeKind.DocumentReloaded"/></item>
+        /// <item><see cref="WorkspaceChangeKind.DocumentRemoved"/></item>
+        /// </list>
+        /// </summary>
+        public DocumentId? DocumentId { get; }
 
-        public WorkspaceChangeEventArgs(WorkspaceChangeKind kind, Solution oldSolution, Solution newSolution, ProjectId projectId = null, DocumentId documentId = null)
+        public WorkspaceChangeEventArgs(WorkspaceChangeKind kind, Solution oldSolution, Solution newSolution, ProjectId? projectId = null, DocumentId? documentId = null)
         {
+            if (!kind.IsValid())
+                throw new ArgumentOutOfRangeException(nameof(kind));
+
             this.Kind = kind;
-            this.OldSolution = oldSolution;
-            this.NewSolution = newSolution;
+            this.OldSolution = oldSolution ?? throw new ArgumentNullException(nameof(oldSolution));
+            this.NewSolution = newSolution ?? throw new ArgumentNullException(nameof(newSolution));
             this.ProjectId = projectId;
             this.DocumentId = documentId;
         }

src/Workspaces/Core/Portable/Workspace/WorkspaceChangeKind.cs

@@ -124,6 +124,11 @@ public enum WorkspaceChangeKind
         /// An analyzer config document in the current solution was changed.
         /// </summary>
         AnalyzerConfigDocumentChanged = 21,
+    }
 
+    internal static class WorkspaceChangeKindExtensions
+    {
+        public static bool IsValid(this WorkspaceChangeKind kind)
+            => kind >= WorkspaceChangeKind.SolutionChanged && kind <= WorkspaceChangeKind.AnalyzerConfigDocumentChanged;
     }
 }