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

Targeted DiagnosticWorker Revert #1984

Merged
merged 3 commits into from
Oct 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ internal static DiagnosticLocation ToDiagnosticLocation(this Diagnostic diagnost
internal static IEnumerable<DiagnosticLocation> DistinctDiagnosticLocationsByProject(this IEnumerable<DocumentDiagnostics> documentDiagnostic)
{
return documentDiagnostic
.SelectMany(x => x.Diagnostics, (parent, child) => (projectName: parent.Project.Name, diagnostic: child))
.SelectMany(x => x.Diagnostics, (parent, child) => (projectName: parent.ProjectName, diagnostic: child))
.Select(x => new
{
location = x.diagnostic.ToDiagnosticLocation(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public async Task<QuickFixResponse> Handle(CodeCheckRequest request)
return GetResponseFromDiagnostics(allDiagnostics, fileName: null);
}

var diagnostics = await _diagWorker.GetDiagnostics(new [] { request.FileName }.ToImmutableArray());
var diagnostics = await _diagWorker.GetDiagnostics(ImmutableArray.Create(request.FileName));

return GetResponseFromDiagnostics(diagnostics, request.FileName);
}
Expand All @@ -47,7 +47,7 @@ private static QuickFixResponse GetResponseFromDiagnostics(ImmutableArray<Docume
{
var diagnosticLocations = diagnostics
.Where(x => string.IsNullOrEmpty(fileName)
|| x.Document.FilePath == fileName)
|| x.DocumentPath == fileName)
.DistinctDiagnosticLocationsByProject()
.Where(x => x.FileName != null);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public override async Task<GetFixAllResponse> Handle(GetFixAllRequest request)

var allDiagnostics = await GetDiagnosticsAsync(request.Scope, document);
var validFixes = allDiagnostics
.GroupBy(docAndDiag => docAndDiag.Document.Project)
.GroupBy(docAndDiag => docAndDiag.ProjectId)
.SelectMany(grouping =>
{
var projectFixProviders = GetCodeFixProviders(grouping.Key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,26 +192,13 @@ public FixAllDiagnosticProvider(ICsDiagnosticWorker diagnosticWorker)
}

public override async Task<IEnumerable<Diagnostic>> GetAllDiagnosticsAsync(Project project, CancellationToken cancellationToken)
{
var diagnostics = await _diagnosticWorker.GetDiagnostics(project.Documents.ToImmutableArray());
return diagnostics.SelectMany(x => x.Diagnostics);
}
=> await _diagnosticWorker.AnalyzeProjectsAsync(project, cancellationToken);

public override async Task<IEnumerable<Diagnostic>> GetDocumentDiagnosticsAsync(Document document, CancellationToken cancellationToken)
{
var documentDiagnostics = await _diagnosticWorker.GetDiagnostics(ImmutableArray.Create(document));

if (!documentDiagnostics.Any())
return new Diagnostic[] { };

return documentDiagnostics.First().Diagnostics;
}
=> await _diagnosticWorker.AnalyzeDocumentAsync(document, cancellationToken);

public override async Task<IEnumerable<Diagnostic>> GetProjectDiagnosticsAsync(Project project, CancellationToken cancellationToken)
{
var diagnostics = await _diagnosticWorker.GetDiagnostics(project.Documents.ToImmutableArray());
return diagnostics.SelectMany(x => x.Diagnostics);
}
=> await _diagnosticWorker.AnalyzeProjectsAsync(project, cancellationToken);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ private TextSpan GetTextSpan(ICodeActionRequest request, SourceText sourceText)

private async Task CollectCodeFixesActions(Document document, TextSpan span, List<CodeAction> codeActions)
{
var diagnosticsWithProjects = await _diagnostics.GetDiagnostics(ImmutableArray.Create(document));
var diagnosticsWithProjects = await _diagnostics.GetDiagnostics(ImmutableArray.Create(document.FilePath));

var groupedBySpan = diagnosticsWithProjects
.SelectMany(x => x.Diagnostics)
Expand Down Expand Up @@ -167,7 +167,7 @@ private async Task AppendFixesAsync(Document document, TextSpan span, IEnumerabl

private List<CodeFixProvider> GetSortedCodeFixProviders(Document document)
{
return ExtensionOrderer.GetOrderedOrUnorderedList<CodeFixProvider, ExportCodeFixProviderAttribute>(_codeFixesForProject.GetAllCodeFixesForProject(document.Project), attribute => attribute.Name).ToList();
return ExtensionOrderer.GetOrderedOrUnorderedList<CodeFixProvider, ExportCodeFixProviderAttribute>(_codeFixesForProject.GetAllCodeFixesForProject(document.Project.Id), attribute => attribute.Name).ToList();
}

private List<CodeRefactoringProvider> GetSortedCodeRefactoringProviders()
Expand Down Expand Up @@ -223,14 +223,14 @@ private IEnumerable<AvailableCodeAction> ConvertToAvailableCodeAction(IEnumerabl
{
if (documentAndDiagnostics.Diagnostics.FirstOrDefault(d => d.Id == diagnosticId) is Diagnostic diagnostic)
{
return (documentAndDiagnostics.Document.Id, diagnostic);
return (documentAndDiagnostics.DocumentId, diagnostic);
}
}

return default;
}

protected ImmutableArray<CodeFixProvider> GetCodeFixProviders(Project project)
protected ImmutableArray<CodeFixProvider> GetCodeFixProviders(ProjectId project)
{
return _codeFixesForProject.GetAllCodeFixesForProject(project);
}
Expand All @@ -239,21 +239,21 @@ protected ImmutableArray<CodeFixProvider> GetCodeFixProviders(Project project)
{
// If Roslyn ever comes up with a UI for selecting what provider the user prefers, we might consider replicating.
// https://github.com/dotnet/roslyn/issues/27066
return _codeFixesForProject.GetAllCodeFixesForProject(document.Project).FirstOrDefault(provider => provider.HasFixForId(id));
return _codeFixesForProject.GetAllCodeFixesForProject(document.Project.Id).FirstOrDefault(provider => provider.HasFixForId(id));
}

protected async Task<ImmutableArray<DocumentDiagnostics>> GetDiagnosticsAsync(FixAllScope scope, Document document)
{
switch (scope)
{
case FixAllScope.Solution:
var documentsInSolution = document.Project.Solution.Projects.SelectMany(p => p.Documents).ToImmutableArray();
var documentsInSolution = document.Project.Solution.Projects.SelectMany(p => p.Documents).Select(d => d.FilePath).ToImmutableArray();
return await _diagnostics.GetDiagnostics(documentsInSolution);
case FixAllScope.Project:
var documentsInProject = document.Project.Documents.ToImmutableArray();
var documentsInProject = document.Project.Documents.Select(d => d.FilePath).ToImmutableArray();
return await _diagnostics.GetDiagnostics(documentsInProject);
case FixAllScope.Document:
return await _diagnostics.GetDiagnostics(ImmutableArray.Create(document));
return await _diagnostics.GetDiagnostics(ImmutableArray.Create(document.FilePath));
default:
throw new InvalidOperationException();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,16 @@ public CachingCodeFixProviderForProjects(ILoggerFactory loggerFactory, OmniSharp
};
}

public ImmutableArray<CodeFixProvider> GetAllCodeFixesForProject(Project project)
public ImmutableArray<CodeFixProvider> GetAllCodeFixesForProject(ProjectId projectId)
{
if (_cache.ContainsKey(project.Id))
return _cache[project.Id];
if (_cache.ContainsKey(projectId))
return _cache[projectId];

var project = _workspace.CurrentSolution.GetProject(projectId);

if (project == null)
{
_cache.TryRemove(project.Id, out _);
_cache.TryRemove(projectId, out _);
return ImmutableArray<CodeFixProvider>.Empty;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ public Queue(TimeSpan throttling)
Throttling = throttling;
}

public ImmutableHashSet<Document> WorkWaitingToExecute { get; set; } = ImmutableHashSet<Document>.Empty;
public ImmutableHashSet<Document> WorkExecuting { get; set; } = ImmutableHashSet<Document>.Empty;
public ImmutableHashSet<DocumentId> WorkWaitingToExecute { get; set; } = ImmutableHashSet<DocumentId>.Empty;
public ImmutableHashSet<DocumentId> WorkExecuting { get; set; } = ImmutableHashSet<DocumentId>.Empty;
public DateTime LastThrottlingBegan { get; set; } = DateTime.UtcNow;
public TimeSpan Throttling { get; }
public CancellationTokenSource WorkPendingToken { get; set; }
Expand All @@ -44,7 +44,7 @@ public AnalyzerWorkQueue(ILoggerFactory loggerFactory, int timeoutForPendingWork
_maximumDelayWhenWaitingForResults = timeoutForPendingWorkMs;
}

public void PutWork(IReadOnlyCollection<Document> documents, AnalyzerWorkType workType)
public void PutWork(IReadOnlyCollection<DocumentId> documentIds, AnalyzerWorkType workType)
{
lock (_queueLock)
{
Expand All @@ -56,21 +56,21 @@ public void PutWork(IReadOnlyCollection<Document> documents, AnalyzerWorkType wo
if (queue.WorkPendingToken == null)
queue.WorkPendingToken = new CancellationTokenSource();

queue.WorkWaitingToExecute = queue.WorkWaitingToExecute.Union(documents);
queue.WorkWaitingToExecute = queue.WorkWaitingToExecute.Union(documentIds);
}
}

public IReadOnlyCollection<Document> TakeWork(AnalyzerWorkType workType)
public IReadOnlyCollection<DocumentId> TakeWork(AnalyzerWorkType workType)
{
lock (_queueLock)
{
var queue = _queues[workType];

if (IsThrottlingActive(queue) || queue.WorkWaitingToExecute.IsEmpty)
return ImmutableHashSet<Document>.Empty;
return ImmutableHashSet<DocumentId>.Empty;

queue.WorkExecuting = queue.WorkWaitingToExecute;
queue.WorkWaitingToExecute = ImmutableHashSet<Document>.Empty;
queue.WorkWaitingToExecute = ImmutableHashSet<DocumentId>.Empty;
return queue.WorkExecuting;
}
}
Expand All @@ -84,12 +84,12 @@ public void WorkComplete(AnalyzerWorkType workType)
{
lock (_queueLock)
{
if (_queues[workType].WorkExecuting.IsEmpty)
if(_queues[workType].WorkExecuting.IsEmpty)
return;

_queues[workType].WorkPendingToken?.Cancel();
_queues[workType].WorkPendingToken = null;
_queues[workType].WorkExecuting = ImmutableHashSet<Document>.Empty;
_queues[workType].WorkExecuting = ImmutableHashSet<DocumentId>.Empty;
}
}

Expand All @@ -107,9 +107,15 @@ public Task WaitForegroundWorkComplete()
.ContinueWith(task => LogTimeouts(task));
}

public void QueueDocumentForeground(Document document)
public bool TryPromote(DocumentId id)
{
PutWork(new[] { document }, AnalyzerWorkType.Foreground);
if (_queues[AnalyzerWorkType.Background].WorkWaitingToExecute.Contains(id) || _queues[AnalyzerWorkType.Background].WorkExecuting.Contains(id))
{
PutWork(new[] { id }, AnalyzerWorkType.Foreground);
return true;
}

return false;
}

private void LogTimeouts(Task task)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Reactive.Concurrency;
using System.Reactive.Linq;
using System.Reactive.Subjects;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -145,25 +146,14 @@ public async Task<ImmutableArray<DocumentDiagnostics>> GetDiagnostics(ImmutableA
.Select(docPath => _workspace.GetDocumentsFromFullProjectModelAsync(docPath)))
).SelectMany(s => s);

return await GetDiagnostics(documents);
}

public Task<ImmutableArray<DocumentDiagnostics>> GetDiagnostics(ImmutableArray<Document> documents)
{
return GetDiagnostics((IEnumerable<Document>)documents);
}

private async Task<ImmutableArray<DocumentDiagnostics>> GetDiagnostics(IEnumerable<Document> documents)
{
var results = new List<DocumentDiagnostics>();
foreach (var document in documents)
{
if(document?.Project?.Name == null)
continue;

var projectName = document.Project.Name;
var diagnostics = await GetDiagnosticsForDocument(document, projectName);
results.Add(new DocumentDiagnostics(document, diagnostics));
results.Add(new DocumentDiagnostics(document.Id, document.FilePath, document.Project.Id, document.Project.Name, diagnostics));
}

return results.ToImmutableArray();
Expand All @@ -185,18 +175,18 @@ private static async Task<ImmutableArray<Diagnostic>> GetDiagnosticsForDocument(
}
}

public ImmutableArray<Document> QueueDocumentsForDiagnostics()
public ImmutableArray<DocumentId> QueueDocumentsForDiagnostics()
{
var documents = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents).ToImmutableArray();
var documents = _workspace.CurrentSolution.Projects.SelectMany(x => x.Documents);
QueueForDiagnosis(documents.Select(x => x.FilePath).ToImmutableArray());
return documents;
return documents.Select(x => x.Id).ToImmutableArray();
}

public ImmutableArray<Document> QueueDocumentsForDiagnostics(ImmutableArray<ProjectId> projectIds)
public ImmutableArray<DocumentId> QueueDocumentsForDiagnostics(ImmutableArray<ProjectId> projectIds)
{
var documents = projectIds.SelectMany(projectId => _workspace.CurrentSolution.GetProject(projectId).Documents).ToImmutableArray();
var documents = projectIds.SelectMany(projectId => _workspace.CurrentSolution.GetProject(projectId).Documents);
QueueForDiagnosis(documents.Select(x => x.FilePath).ToImmutableArray());
return documents;
return documents.Select(x => x.Id).ToImmutableArray();
}

public Task<ImmutableArray<DocumentDiagnostics>> GetAllDiagnosticsAsync()
Expand All @@ -212,5 +202,23 @@ public void Dispose()
_workspace.DocumentClosed -= OnDocumentOpened;
_disposable.Dispose();
}

public async Task<IEnumerable<Diagnostic>> AnalyzeDocumentAsync(Document document, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
return await GetDiagnosticsForDocument(document, document.Project.Name);
}

public async Task<IEnumerable<Diagnostic>> AnalyzeProjectsAsync(Project project, CancellationToken cancellationToken)
{
var diagnostics = new List<Diagnostic>();
foreach (var document in project.Documents)
{
cancellationToken.ThrowIfCancellationRequested();
diagnostics.AddRange(await GetDiagnosticsForDocument(document, project.Name));
}

return diagnostics;
}
}
}
Loading