From 214bde6dfea751a83f0740a77e409a239a5ab611 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 23 Feb 2023 15:18:31 -0800 Subject: [PATCH 1/4] Remove the PreviewWorkspace SolutionCrawler functionality --- ...eviewSolutionCrawlerRegistrationService.cs | 147 ------------------ .../Test/Preview/PreviewWorkspaceTests.cs | 3 +- .../SolutionCrawlerRegistrationService.cs | 17 +- 3 files changed, 17 insertions(+), 150 deletions(-) delete mode 100644 src/EditorFeatures/Core/Shared/Preview/PreviewSolutionCrawlerRegistrationService.cs diff --git a/src/EditorFeatures/Core/Shared/Preview/PreviewSolutionCrawlerRegistrationService.cs b/src/EditorFeatures/Core/Shared/Preview/PreviewSolutionCrawlerRegistrationService.cs deleted file mode 100644 index 739d664e23a6a..0000000000000 --- a/src/EditorFeatures/Core/Shared/Preview/PreviewSolutionCrawlerRegistrationService.cs +++ /dev/null @@ -1,147 +0,0 @@ -// 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.Immutable; -using System.Composition; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Diagnostics; -using Microsoft.CodeAnalysis.Host; -using Microsoft.CodeAnalysis.Host.Mef; -using Microsoft.CodeAnalysis.Shared.Extensions; -using Microsoft.CodeAnalysis.Shared.TestHooks; -using Microsoft.CodeAnalysis.SolutionCrawler; -using Roslyn.Utilities; - -namespace Microsoft.CodeAnalysis.Editor.Shared.Preview -{ - [ExportWorkspaceServiceFactory(typeof(ISolutionCrawlerRegistrationService), WorkspaceKind.Preview), Shared] - internal class PreviewSolutionCrawlerRegistrationServiceFactory : IWorkspaceServiceFactory - { - private readonly DiagnosticAnalyzerService _analyzerService; - private readonly IAsynchronousOperationListener _listener; - - [ImportingConstructor] - [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] - public PreviewSolutionCrawlerRegistrationServiceFactory(IDiagnosticAnalyzerService analyzerService, IAsynchronousOperationListenerProvider listenerProvider) - { - // this service is directly tied to DiagnosticAnalyzerService and - // depends on its implementation. - _analyzerService = (DiagnosticAnalyzerService)analyzerService; - - _listener = listenerProvider.GetListener(FeatureAttribute.DiagnosticService); - } - - public IWorkspaceService? CreateService(HostWorkspaceServices workspaceServices) - { - // to make life time management easier, just create new service per new workspace - return new Service(this, workspaceServices.Workspace); - } - - // internal for testing - internal class Service : ISolutionCrawlerRegistrationService - { - private readonly PreviewSolutionCrawlerRegistrationServiceFactory _owner; - private readonly Workspace _workspace; - private readonly CancellationTokenSource _source = new(); - - // since we now have one service for each one specific instance of workspace, - // we can have states for this specific workspace. - private Task? _analyzeTask; - - public Service(PreviewSolutionCrawlerRegistrationServiceFactory owner, Workspace workspace) - { - _owner = owner; - _workspace = workspace; - } - - public void Register(Workspace workspace) - { - // given workspace must be owner of this workspace service - Contract.ThrowIfFalse(workspace == _workspace); - - // this can't be called twice - Contract.ThrowIfFalse(_analyzeTask == null); - - var asyncToken = _owner._listener.BeginAsyncOperation(nameof(PreviewSolutionCrawlerRegistrationServiceFactory) + "." + nameof(Service) + "." + nameof(Register)); - _analyzeTask = AnalyzeAsync().CompletesAsyncOperation(asyncToken); - } - - private async Task AnalyzeAsync() - { - var workerBackOffTimeSpan = SolutionCrawlerTimeSpan.PreviewBackOff; - var incrementalAnalyzer = _owner._analyzerService.CreateIncrementalAnalyzer(_workspace); - - var solution = _workspace.CurrentSolution; - var documentIds = _workspace.GetOpenDocumentIds().ToImmutableArray(); - - try - { - foreach (var documentId in documentIds) - { - var textDocument = solution.GetTextDocument(documentId); - if (textDocument == null) - { - continue; - } - - // delay analyzing - await _owner._listener.Delay(workerBackOffTimeSpan, _source.Token).ConfigureAwait(false); - - // do actual analysis - if (textDocument is Document document) - { - await incrementalAnalyzer.AnalyzeSyntaxAsync(document, InvocationReasons.Empty, _source.Token).ConfigureAwait(false); - await incrementalAnalyzer.AnalyzeDocumentAsync(document, bodyOpt: null, reasons: InvocationReasons.Empty, cancellationToken: _source.Token).ConfigureAwait(false); - } - else - { - await incrementalAnalyzer.AnalyzeNonSourceDocumentAsync(textDocument, InvocationReasons.Empty, _source.Token).ConfigureAwait(false); - } - - // don't call project one. - } - } - catch (OperationCanceledException) - { - // do nothing - } - } - - public void Unregister(Workspace workspace, bool blockingShutdown = false) - => _ = UnregisterAsync(workspace); - - private async Task UnregisterAsync(Workspace workspace) - { - Contract.ThrowIfFalse(workspace == _workspace); - - _source.Cancel(); - - try - { - var analyzeTask = _analyzeTask; - if (analyzeTask != null) - { - // wait for analyzer work to be finished - await analyzeTask.ConfigureAwait(false); - } - } - finally - { - _source.Dispose(); - - // ask it to reset its stages for the given workspace - _owner._analyzerService.ShutdownAnalyzerFrom(_workspace); - } - } - - public void AddAnalyzerProvider(IIncrementalAnalyzerProvider provider, IncrementalAnalyzerProviderMetadata metadata) - { - // preview solution crawler doesn't support adding and removing analyzer dynamically - throw new NotSupportedException(); - } - } - } -} diff --git a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs index 77ddea0583827..ebd9ea2ebc768 100644 --- a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs +++ b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs @@ -125,7 +125,8 @@ public async Task TestPreviewServices() { using var previewWorkspace = new PreviewWorkspace(EditorTestCompositions.EditorFeatures.GetHostServices()); var service = previewWorkspace.Services.GetService(); - Assert.IsType(service); + var registrationService = Assert.IsType(service); + Assert.False(registrationService.Register(previewWorkspace)); var persistentService = previewWorkspace.Services.SolutionServices.GetPersistentStorageService(); diff --git a/src/Features/Core/Portable/SolutionCrawler/SolutionCrawlerRegistrationService.cs b/src/Features/Core/Portable/SolutionCrawler/SolutionCrawlerRegistrationService.cs index d38dd2a41326b..dbed8666710ca 100644 --- a/src/Features/Core/Portable/SolutionCrawler/SolutionCrawlerRegistrationService.cs +++ b/src/Features/Core/Portable/SolutionCrawler/SolutionCrawlerRegistrationService.cs @@ -44,8 +44,21 @@ public SolutionCrawlerRegistrationService( _listener = listenerProvider.GetListener(FeatureAttribute.SolutionCrawlerLegacy); } - public void Register(Workspace workspace) - => EnsureRegistration(workspace, initializeLazily: true); + void ISolutionCrawlerRegistrationService.Register(Workspace workspace) + => Register(workspace); + + public bool Register(Workspace workspace) + { + // Do not crawl the preview workspace. It's pure overhead and serves no purpose. Diagnostics for the + // preview workspace are provided either through Roslyn-Native-Pull-Tagging (which does not need solution + // crawler). Or will be something LSP needs to handle if Native-Pull-Tagging is off and + // LSP-Pull-Diagnostics is on. + if (workspace.Kind == WorkspaceKind.Preview) + return false; + + EnsureRegistration(workspace, initializeLazily: true); + return true; + } /// /// make sure solution cralwer is registered for the given workspace. From 04da5887cc93a8c26c5c46994af9a1321a20e57b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 23 Feb 2023 15:31:06 -0800 Subject: [PATCH 2/4] Remove unnecessary test --- .../Test/Preview/PreviewWorkspaceTests.cs | 40 +------------------ 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs index ebd9ea2ebc768..3854f0573653d 100644 --- a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs +++ b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs @@ -18,6 +18,7 @@ using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.SolutionCrawler; +using Microsoft.CodeAnalysis.Storage; using Microsoft.CodeAnalysis.Test.Utilities; using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.Text.Shared.Extensions; @@ -25,7 +26,6 @@ using Roslyn.Test.Utilities; using Roslyn.Utilities; using Xunit; -using Microsoft.CodeAnalysis.Storage; namespace Microsoft.CodeAnalysis.Editor.UnitTests.Preview { @@ -134,44 +134,6 @@ public async Task TestPreviewServices() Assert.IsType(storage); } - [WorkItem(923196, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/923196")] - [WpfFact] - public async Task TestPreviewDiagnostic() - { - var hostServices = EditorTestCompositions.EditorFeatures.GetHostServices(); - var exportProvider = (IMefHostExportProvider)hostServices; - - var diagnosticService = (IDiagnosticUpdateSource)exportProvider.GetExportedValue(); - RoslynDebug.AssertNotNull(diagnosticService); - var globalOptions = exportProvider.GetExportedValue(); - - var taskSource = new TaskCompletionSource(); - diagnosticService.DiagnosticsUpdated += (s, a) => taskSource.TrySetResult(a); - - using var previewWorkspace = new PreviewWorkspace(hostServices); - - var solution = previewWorkspace.CurrentSolution - .WithAnalyzerReferences(new[] { DiagnosticExtensions.GetCompilerDiagnosticAnalyzerReference(LanguageNames.CSharp) }) - .AddProject("project", "project.dll", LanguageNames.CSharp) - .AddDocument("document", "class { }") - .Project - .Solution; - - Assert.True(previewWorkspace.TryApplyChanges(solution)); - - var document = previewWorkspace.CurrentSolution.Projects.First().Documents.Single(); - - previewWorkspace.OpenDocument(document.Id, (await document.GetTextAsync()).Container); - previewWorkspace.EnableSolutionCrawler(); - - // wait 20 seconds - taskSource.Task.Wait(20000); - Assert.True(taskSource.Task.IsCompleted); - - var args = taskSource.Task.Result; - Assert.True(args.Diagnostics.Length > 0); - } - [WpfFact] public async Task TestPreviewDiagnosticTagger() { From af582bc53da5ec523ba5664ef3c46bea2da543d9 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 23 Feb 2023 16:30:25 -0800 Subject: [PATCH 3/4] Update test to be pull based. --- .../Test/Preview/PreviewWorkspaceTests.cs | 22 ++++++++++++++----- ...sticAnalyzerService_IncrementalAnalyzer.cs | 18 +++++---------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs index 3854f0573653d..e72737b78e233 100644 --- a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs +++ b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs @@ -184,18 +184,30 @@ public async Task TestPreviewDiagnosticTaggerInPreviewPane() var listenerProvider = workspace.ExportProvider.GetExportedValue(); - // set up tagger for both buffers - var leftBuffer = diffView.Viewer.LeftView.BufferGraph.GetTextBuffers(t => t.ContentType.IsOfType(ContentTypeNames.CSharpContentType)).First(); var provider = workspace.ExportProvider.GetExportedValues().OfType().Single(); - var leftTagger = provider.CreateTagger(leftBuffer); - Contract.ThrowIfNull(leftTagger); - using var leftDisposable = leftTagger as IDisposable; + // set up tagger for both buffers + var leftBuffer = diffView.Viewer.LeftView.BufferGraph.GetTextBuffers(t => t.ContentType.IsOfType(ContentTypeNames.CSharpContentType)).First(); var rightBuffer = diffView.Viewer.RightView.BufferGraph.GetTextBuffers(t => t.ContentType.IsOfType(ContentTypeNames.CSharpContentType)).First(); + + var leftDocument = leftBuffer.GetRelatedDocuments().Single(); + var rightDocument = rightBuffer.GetRelatedDocuments().Single(); + + // Diagnostic analyzer service, which provides pull capabilities (and not to be confused with + // IDiagnosticService, which is push), doesn't normally register for test workspace. So do it explicitly. + var diagnosticAnalyzer = workspace.ExportProvider.GetExportedValue(); + var incrementalAnalyzer = (IIncrementalAnalyzerProvider)diagnosticAnalyzer; + incrementalAnalyzer.CreateIncrementalAnalyzer(leftDocument.Project.Solution.Workspace); + incrementalAnalyzer.CreateIncrementalAnalyzer(rightDocument.Project.Solution.Workspace); + + var leftTagger = provider.CreateTagger(leftBuffer); var rightTagger = provider.CreateTagger(rightBuffer); + Contract.ThrowIfNull(leftTagger); Contract.ThrowIfNull(rightTagger); + using var leftDisposable = leftTagger as IDisposable; using var rightDisposable = rightTagger as IDisposable; + // wait for diagnostics and taggers await listenerProvider.WaitAllDispatcherOperationAndTasksAsync(workspace, FeatureAttribute.DiagnosticService, FeatureAttribute.ErrorSquiggles); diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_IncrementalAnalyzer.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_IncrementalAnalyzer.cs index 87bf00589f38a..fd996268a34eb 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_IncrementalAnalyzer.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_IncrementalAnalyzer.cs @@ -3,12 +3,9 @@ // See the LICENSE file in the project root for more information. using System; -using System.Threading; -using System.Threading.Tasks; using Microsoft.CodeAnalysis.Diagnostics.EngineV2; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Internal.Log; -using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.SolutionCrawler; using Roslyn.Utilities; @@ -21,17 +18,12 @@ internal partial class DiagnosticAnalyzerService : IIncrementalAnalyzerProvider { public IIncrementalAnalyzer CreateIncrementalAnalyzer(Workspace workspace) { - if (GlobalOptions.IsLspPullDiagnostics()) - { - // We rely on LSP to query us for diagnostics when things have changed and poll us for changes that might - // have happened to the project or closed files outside of VS. - // However, we still need to create the analyzer so that the map contains the analyzer to run when pull diagnostics asks. - _ = _map.GetValue(workspace, _createIncrementalAnalyzer); - - return NoOpIncrementalAnalyzer.Instance; - } + var analyzer = _map.GetValue(workspace, _createIncrementalAnalyzer); - return _map.GetValue(workspace, _createIncrementalAnalyzer); + // We rely on LSP to query us for diagnostics when things have changed and poll us for changes that might + // have happened to the project or closed files outside of VS. However, we still need to create the analyzer + // so that the map contains the analyzer to run when pull diagnostics asks. + return GlobalOptions.IsLspPullDiagnostics() ? NoOpIncrementalAnalyzer.Instance : analyzer; } public void ShutdownAnalyzerFrom(Workspace workspace) From 0c8dd2d92a06d31c892e87e421f570694cfeb4b6 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 23 Feb 2023 16:32:08 -0800 Subject: [PATCH 4/4] Fixup test to be pull not push --- src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs index e72737b78e233..0024cd12e7041 100644 --- a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs +++ b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs @@ -164,9 +164,6 @@ public async Task TestPreviewDiagnosticTaggerInPreviewPane() workspace.TryApplyChanges(workspace.CurrentSolution.WithAnalyzerReferences(new[] { DiagnosticExtensions.GetCompilerDiagnosticAnalyzerReference(LanguageNames.CSharp) })); - // set up listener to wait until diagnostic finish running - _ = workspace.ExportProvider.GetExportedValue(); - var hostDocument = workspace.Projects.First().Documents.First(); // make a change to remove squiggle