Skip to content

Remove the PreviewWorkspace SolutionCrawler functionality #67027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mavasani merged 4 commits into from
Feb 24, 2023
Merged

Remove the PreviewWorkspace SolutionCrawler functionality #67027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
214bde6
Remove the PreviewWorkspace SolutionCrawler functionality
CyrusNajmabadi Feb 23, 2023
04da588
Remove unnecessary test
CyrusNajmabadi Feb 23, 2023
af582bc
Update test to be pull based.
CyrusNajmabadi Feb 24, 2023
0c8dd2d
Fixup test to be pull not push
CyrusNajmabadi Feb 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 0 additions & 147 deletions src/EditorFeatures/Core/Shared/Preview/PreviewSolutionCrawlerRegistrationService.cs
View file
Open in desktop

This file was deleted.

68 changes: 20 additions & 48 deletions src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@
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;
using Microsoft.VisualStudio.Text.Tagging;
using Roslyn.Test.Utilities;
using Roslyn.Utilities;
using Xunit;
using Microsoft.CodeAnalysis.Storage;

namespace Microsoft.CodeAnalysis.Editor.UnitTests.Preview
{
Expand Down Expand Up @@ -125,52 +125,15 @@ public async Task TestPreviewServices()
{
using var previewWorkspace = new PreviewWorkspace(EditorTestCompositions.EditorFeatures.GetHostServices());
var service = previewWorkspace.Services.GetService<ISolutionCrawlerRegistrationService>();
Assert.IsType<PreviewSolutionCrawlerRegistrationServiceFactory.Service>(service);
var registrationService = Assert.IsType<SolutionCrawlerRegistrationService>(service);
Assert.False(registrationService.Register(previewWorkspace));

var persistentService = previewWorkspace.Services.SolutionServices.GetPersistentStorageService();

await using var storage = await persistentService.GetStorageAsync(SolutionKey.ToSolutionKey(previewWorkspace.CurrentSolution), CancellationToken.None);
Assert.IsType<NoOpPersistentStorage>(storage);
}

[WorkItem(923196, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/923196")]
[WpfFact]
public async Task TestPreviewDiagnostic()
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Feb 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pull tagger test is below. this one was testing the push/solution-crawler version.

{
var hostServices = EditorTestCompositions.EditorFeatures.GetHostServices();
var exportProvider = (IMefHostExportProvider)hostServices;

var diagnosticService = (IDiagnosticUpdateSource)exportProvider.GetExportedValue<IDiagnosticAnalyzerService>();
RoslynDebug.AssertNotNull(diagnosticService);
var globalOptions = exportProvider.GetExportedValue<IGlobalOptionService>();

var taskSource = new TaskCompletionSource<DiagnosticsUpdatedArgs>();
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()
{
Expand Down Expand Up @@ -201,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<IDiagnosticService>();
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Feb 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was previously using push. updated to pull below.


var hostDocument = workspace.Projects.First().Documents.First();

// make a change to remove squiggle
Expand All @@ -221,18 +181,30 @@ public async Task TestPreviewDiagnosticTaggerInPreviewPane()

var listenerProvider = workspace.ExportProvider.GetExportedValue<AsynchronousOperationListenerProvider>();

// 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<ITaggerProvider>().OfType<DiagnosticsSquiggleTaggerProvider>().Single();
var leftTagger = provider.CreateTagger<IErrorTag>(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<IDiagnosticAnalyzerService>();
var incrementalAnalyzer = (IIncrementalAnalyzerProvider)diagnosticAnalyzer;
incrementalAnalyzer.CreateIncrementalAnalyzer(leftDocument.Project.Solution.Workspace);
incrementalAnalyzer.CreateIncrementalAnalyzer(rightDocument.Project.Solution.Workspace);

var leftTagger = provider.CreateTagger<IErrorTag>(leftBuffer);
var rightTagger = provider.CreateTagger<IErrorTag>(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);

Expand Down
17 changes: 15 additions & 2 deletions src/Features/Core/Portable/SolutionCrawler/SolutionCrawlerRegistrationService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/// <summary>
/// make sure solution cralwer is registered for the given workspace.
Expand Down
18 changes: 5 additions & 13 deletions ...uageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_IncrementalAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Feb 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same logic a before. i just rejiggered. in both paths below we were adding hte item to the map. i pulled that above, and then just changed the logic if the NoOp analyzer is returned or the actual instance.

}

public void ShutdownAnalyzerFrom(Workspace workspace)
Expand Down