Skip to content
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.

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

Fix state bugs in active file analysis #52807

Merged
merged 11 commits into from
Apr 27, 2021

Large diffs are not rendered by default.

248 changes: 124 additions & 124 deletions src/EditorFeatures/CSharpTest/NavigateTo/NavigateToTests.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -645,9 +645,15 @@ internal async Task TestAdditionalFileAnalyzer(bool registerFromInitialize, bool

await ((AsynchronousOperationListener)service.Listener).ExpeditedWaitAsync();

var expectedCount = !testMultiple
? 1
: analysisScope == BackgroundAnalysisScope.FullSolution ? 4 : 2;
var expectedCount = (analysisScope, testMultiple) switch
{
(BackgroundAnalysisScope.ActiveFile, _) => 0,
(BackgroundAnalysisScope.OpenFilesAndProjects or BackgroundAnalysisScope.FullSolution, false) => 1,
(BackgroundAnalysisScope.OpenFilesAndProjects, true) => 2,
(BackgroundAnalysisScope.FullSolution, true) => 4,
_ => throw ExceptionUtilities.Unreachable,
};

Assert.Equal(expectedCount, diagnostics.Count);

for (var i = 0; i < analyzers.Length; i++)
Expand All @@ -658,7 +664,11 @@ internal async Task TestAdditionalFileAnalyzer(bool registerFromInitialize, bool
var applicableDiagnostics = diagnostics.Where(
d => d.Id == analyzer.Descriptor.Id && d.DataLocation.OriginalFilePath == additionalDoc.FilePath);

if (analysisScope != BackgroundAnalysisScope.FullSolution &&
if (analysisScope == BackgroundAnalysisScope.ActiveFile)
{
Assert.Empty(applicableDiagnostics);
}
else if (analysisScope == BackgroundAnalysisScope.OpenFilesAndProjects &&
firstAdditionalDocument != additionalDoc)
{
Assert.Empty(applicableDiagnostics);
Expand Down
291 changes: 178 additions & 113 deletions src/EditorFeatures/Test/SolutionCrawler/WorkCoordinatorTests.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ namespace Microsoft.CodeAnalysis.Editor.UnitTests.NavigateTo
[UseExportProvider]
public abstract class AbstractNavigateToTests
{
private static readonly TestComposition s_composition = EditorTestCompositions.EditorFeatures.AddParts(typeof(TestDocumentTrackingServiceFactory));
protected static readonly TestComposition DefaultComposition = EditorTestCompositions.EditorFeatures;
protected static readonly TestComposition FirstVisibleComposition = EditorTestCompositions.EditorFeatures.AddParts(typeof(FirstDocIsVisibleDocumentTrackingService.Factory));
protected static readonly TestComposition FirstActiveAndVisibleComposition = EditorTestCompositions.EditorFeatures.AddParts(typeof(FirstDocIsActiveAndVisibleDocumentTrackingService.Factory));

protected INavigateToItemProvider _provider;
protected NavigateToTestAggregator _aggregator;
Expand Down Expand Up @@ -66,31 +68,40 @@ public abstract class AbstractNavigateToTests
protected abstract TestWorkspace CreateWorkspace(string content, ExportProvider exportProvider);
protected abstract string Language { get; }

protected async Task TestAsync(TestHost testHost, string content, Func<TestWorkspace, Task> body)
public enum Composition
{
await TestAsync(content, body, testHost, null);
await TestAsync(content, body, testHost, w => new FirstDocIsVisibleDocumentTrackingService(w.Workspace));
await TestAsync(content, body, testHost, w => new FirstDocIsActiveAndVisibleDocumentTrackingService(w.Workspace));
Default,
FirstVisible,
FirstActiveAndVisible,
}

protected async Task TestAsync(TestHost testHost, Composition composition, string content, Func<TestWorkspace, Task> body)
{
var testComposition = composition switch
{
Composition.Default => DefaultComposition,
Composition.FirstVisible => FirstVisibleComposition,
Composition.FirstActiveAndVisible => FirstActiveAndVisibleComposition,
_ => throw ExceptionUtilities.UnexpectedValue(composition),
};

await TestAsync(content, body, testHost, testComposition);
}

private async Task TestAsync(
string content, Func<TestWorkspace, Task> body, TestHost testHost,
Func<HostWorkspaceServices, IDocumentTrackingService> createTrackingService)
TestComposition composition)
{
using var workspace = CreateWorkspace(content, testHost, createTrackingService);
using var workspace = CreateWorkspace(content, testHost, composition);
await body(workspace);
}

private protected TestWorkspace CreateWorkspace(
XElement workspaceElement,
TestHost testHost,
Func<HostWorkspaceServices, IDocumentTrackingService> createTrackingService)
TestComposition composition)
{
var exportProvider = s_composition.WithTestHostParts(testHost).ExportProviderFactory.CreateExportProvider();

// must be set before the workspace is created since the constructor accesses IDocumentTrackingService
var documentTrackingServiceFactory = exportProvider.GetExportedValue<TestDocumentTrackingServiceFactory>();
documentTrackingServiceFactory.FactoryMethod = createTrackingService;
var exportProvider = composition.WithTestHostParts(testHost).ExportProviderFactory.CreateExportProvider();

var workspace = TestWorkspace.Create(workspaceElement, exportProvider: exportProvider);
InitializeWorkspace(workspace);
Expand All @@ -100,13 +111,9 @@ private protected TestWorkspace CreateWorkspace(
private protected TestWorkspace CreateWorkspace(
string content,
TestHost testHost,
Func<HostWorkspaceServices, IDocumentTrackingService> createTrackingService)
TestComposition composition)
{
var exportProvider = s_composition.WithTestHostParts(testHost).ExportProviderFactory.CreateExportProvider();

// must be set before the workspace is created since the constructor accesses IDocumentTrackingService
var documentTrackingServiceFactory = exportProvider.GetExportedValue<TestDocumentTrackingServiceFactory>();
documentTrackingServiceFactory.FactoryMethod = createTrackingService;
var exportProvider = composition.WithTestHostParts(testHost).ExportProviderFactory.CreateExportProvider();

var workspace = CreateWorkspace(content, exportProvider);
InitializeWorkspace(workspace);
Expand Down Expand Up @@ -194,7 +201,8 @@ private class FirstDocIsVisibleDocumentTrackingService : IDocumentTrackingServic
{
private readonly Workspace _workspace;

public FirstDocIsVisibleDocumentTrackingService(Workspace workspace)
[Obsolete(MefConstruction.FactoryMethodMessage, error: true)]
private FirstDocIsVisibleDocumentTrackingService(Workspace workspace)
=> _workspace = workspace;

public event EventHandler<DocumentId> ActiveDocumentChanged { add { } remove { } }
Expand All @@ -205,13 +213,28 @@ public DocumentId TryGetActiveDocument()

public ImmutableArray<DocumentId> GetVisibleDocuments()
=> ImmutableArray.Create(_workspace.CurrentSolution.Projects.First().DocumentIds.First());

[ExportWorkspaceServiceFactory(typeof(IDocumentTrackingService), ServiceLayer.Test), Shared, PartNotDiscoverable]
public class Factory : IWorkspaceServiceFactory
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public Factory()
{
}

[Obsolete(MefConstruction.FactoryMethodMessage, error: true)]
public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
=> new FirstDocIsVisibleDocumentTrackingService(workspaceServices.Workspace);
}
}

private class FirstDocIsActiveAndVisibleDocumentTrackingService : IDocumentTrackingService
{
private readonly Workspace _workspace;

public FirstDocIsActiveAndVisibleDocumentTrackingService(Workspace workspace)
[Obsolete(MefConstruction.FactoryMethodMessage, error: true)]
private FirstDocIsActiveAndVisibleDocumentTrackingService(Workspace workspace)
=> _workspace = workspace;

public event EventHandler<DocumentId> ActiveDocumentChanged { add { } remove { } }
Expand All @@ -222,26 +245,20 @@ public DocumentId TryGetActiveDocument()

public ImmutableArray<DocumentId> GetVisibleDocuments()
=> ImmutableArray.Create(_workspace.CurrentSolution.Projects.First().DocumentIds.First());
}

[Export]
[ExportWorkspaceServiceFactory(typeof(IDocumentTrackingService), ServiceLayer.Test), Shared, PartNotDiscoverable]
public sealed class TestDocumentTrackingServiceFactory : IWorkspaceServiceFactory
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public TestDocumentTrackingServiceFactory()
=> FactoryMethod = null;

internal Func<HostWorkspaceServices, IDocumentTrackingService> FactoryMethod
[ExportWorkspaceServiceFactory(typeof(IDocumentTrackingService), ServiceLayer.Test), Shared, PartNotDiscoverable]
public class Factory : IWorkspaceServiceFactory
{
get;
set;
}
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public Factory()
{
}

[Obsolete(MefConstruction.FactoryMethodMessage, error: true)]
public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
=> FactoryMethod?.Invoke(workspaceServices);
[Obsolete(MefConstruction.FactoryMethodMessage, error: true)]
public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
=> new FirstDocIsActiveAndVisibleDocumentTrackingService(workspaceServices.Workspace);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System;
using System.Collections.Immutable;
using System.Composition;
Expand All @@ -14,48 +12,29 @@ namespace Microsoft.CodeAnalysis.Editor.Test
[ExportWorkspaceService(typeof(IDocumentTrackingService), ServiceLayer.Test), Shared, PartNotDiscoverable]
internal sealed class TestDocumentTrackingService : IDocumentTrackingService
{
private readonly object _gate = new object();
private event EventHandler<DocumentId> _activeDocumentChangedEventHandler;
private DocumentId _activeDocumentId;
private DocumentId? _activeDocumentId;

[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public TestDocumentTrackingService()
{
}

public event EventHandler<DocumentId> ActiveDocumentChanged
{
add
{
lock (_gate)
{
_activeDocumentChangedEventHandler += value;
}
}

remove
{
lock (_gate)
{
_activeDocumentChangedEventHandler -= value;
}
}
}
public event EventHandler<DocumentId?>? ActiveDocumentChanged;

public event EventHandler<EventArgs> NonRoslynBufferTextChanged
{
add { }
remove { }
}

public void SetActiveDocument(DocumentId newActiveDocumentId)
public void SetActiveDocument(DocumentId? newActiveDocumentId)
{
_activeDocumentId = newActiveDocumentId;
_activeDocumentChangedEventHandler?.Invoke(this, newActiveDocumentId);
ActiveDocumentChanged?.Invoke(this, newActiveDocumentId);
}

public DocumentId TryGetActiveDocument()
public DocumentId? TryGetActiveDocument()
=> _activeDocumentId;

public ImmutableArray<DocumentId> GetVisibleDocuments()
Expand Down
Loading