From ec03d317e85fa75ab31cbebdd995f0d43b231e0f Mon Sep 17 00:00:00 2001 From: Heejae Chang Date: Thu, 12 Feb 2015 07:11:27 -0800 Subject: [PATCH 1/4] bare minimum implementation of diagnostic analyzer that uses compiler analyzer driver --- .../EngineV2/DiagnosticIncrementalAnalyzer.cs | 137 ++++++++++++++++-- 1 file changed, 121 insertions(+), 16 deletions(-) diff --git a/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs b/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs index 6f825124d1e58..b6fca5ada1592 100644 --- a/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs +++ b/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Collections.Immutable; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Text; @@ -30,9 +31,11 @@ public override Task AnalyzeDocumentAsync(Document document, SyntaxNode bodyOpt, return SpecializedTasks.EmptyTask; } - public override Task AnalyzeProjectAsync(Project project, bool semanticsChanged, CancellationToken cancellationToken) + public override async Task AnalyzeProjectAsync(Project project, bool semanticsChanged, CancellationToken cancellationToken) { - return SpecializedTasks.EmptyTask; + var diagnostics = await GetDiagnosticsAsync(project.Solution, project.Id, null, cancellationToken).ConfigureAwait(false); + + RaiseEvetns(project, diagnostics); } public override Task AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken) @@ -57,51 +60,153 @@ public override Task NewSolutionSnapshotAsync(Solution solution, CancellationTok public override void RemoveDocument(DocumentId documentId) { + _owner.RaiseDiagnosticsUpdated( + this, new DiagnosticsUpdatedArgs(ValueTuple.Create(this, documentId), null, null, null, null, ImmutableArray.Empty)); } public override void RemoveProject(ProjectId projectId) { + _owner.RaiseDiagnosticsUpdated( + this, new DiagnosticsUpdatedArgs(ValueTuple.Create(this, projectId), null, null, null, null, ImmutableArray.Empty)); } #endregion public override Task> GetCachedDiagnosticsAsync(Solution solution, ProjectId projectId = null, DocumentId documentId = null, CancellationToken cancellationToken = default(CancellationToken)) { - return SpecializedTasks.EmptyImmutableArray(); + return GetDiagnosticsAsync(solution, projectId, documentId, cancellationToken); } public override Task> GetSpecificCachedDiagnosticsAsync(Solution solution, object id, CancellationToken cancellationToken) { - return SpecializedTasks.EmptyImmutableArray(); + return GetSpecificDiagnosticsAsync(solution, id, cancellationToken); } - public override Task> GetDiagnosticsAsync(Solution solution, ProjectId projectId = null, DocumentId documentId = null, CancellationToken cancellationToken = default(CancellationToken)) + public override async Task> GetDiagnosticsAsync(Solution solution, ProjectId projectId = null, DocumentId documentId = null, CancellationToken cancellationToken = default(CancellationToken)) { - return SpecializedTasks.EmptyImmutableArray(); + if (documentId != null) + { + var diagnostics = await GetProjectDiagnosticsAsync(solution.GetProject(projectId), cancellationToken).ConfigureAwait(false); + return diagnostics.Where(d => d.DocumentId == documentId).ToImmutableArrayOrEmpty(); + } + + if (projectId != null) + { + return await GetProjectDiagnosticsAsync(solution.GetProject(projectId), cancellationToken).ConfigureAwait(false); + } + + var builder = ImmutableArray.CreateBuilder(); + foreach (var project in solution.Projects) + { + builder.AddRange(await GetProjectDiagnosticsAsync(project, cancellationToken).ConfigureAwait(false)); + } + + return builder.ToImmutable(); } - public override Task> GetSpecificDiagnosticsAsync(Solution solution, object id, CancellationToken cancellationToken) + public override async Task> GetSpecificDiagnosticsAsync(Solution solution, object id, CancellationToken cancellationToken) { - return SpecializedTasks.EmptyImmutableArray(); + if (id is ValueTuple) + { + var key = (ValueTuple)id; + return await GetDiagnosticsAsync(solution, key.Item2.ProjectId, key.Item2, cancellationToken).ConfigureAwait(false); + } + + if (id is ValueTuple) + { + var key = (ValueTuple)id; + var diagnostics = await GetDiagnosticsAsync(solution, key.Item2, null, cancellationToken).ConfigureAwait(false); + return diagnostics.Where(d => d.DocumentId == null).ToImmutableArray(); + } + + return ImmutableArray.Empty; } - public override Task> GetDiagnosticsForIdsAsync(Solution solution, ProjectId projectId = null, DocumentId documentId = null, ImmutableHashSet diagnosticIds = null, CancellationToken cancellationToken = default(CancellationToken)) + public override async Task> GetDiagnosticsForIdsAsync(Solution solution, ProjectId projectId = null, DocumentId documentId = null, ImmutableHashSet diagnosticIds = null, CancellationToken cancellationToken = default(CancellationToken)) { - return SpecializedTasks.EmptyImmutableArray(); + var diagnostics = await GetDiagnosticsAsync(solution, projectId, documentId, cancellationToken).ConfigureAwait(false); + return diagnostics.Where(d => diagnosticIds.Contains(d.Id)).ToImmutableArrayOrEmpty(); } - public override Task> GetProjectDiagnosticsForIdsAsync(Solution solution, ProjectId projectId = null, ImmutableHashSet diagnosticIds = null, CancellationToken cancellationToken = default(CancellationToken)) + public override async Task> GetProjectDiagnosticsForIdsAsync(Solution solution, ProjectId projectId = null, ImmutableHashSet diagnosticIds = null, CancellationToken cancellationToken = default(CancellationToken)) { - return SpecializedTasks.EmptyImmutableArray(); + var diagnostics = await GetDiagnosticsForIdsAsync(solution, projectId, null, diagnosticIds, cancellationToken).ConfigureAwait(false); + return diagnostics.Where(d => d.DocumentId == null).ToImmutableArray(); } - public override Task TryAppendDiagnosticsForSpanAsync(Document document, TextSpan range, List diagnostics, CancellationToken cancellationToken) + public override async Task TryAppendDiagnosticsForSpanAsync(Document document, TextSpan range, List result, CancellationToken cancellationToken) { - return SpecializedTasks.False; + result.AddRange(await GetDiagnosticsForSpanAsync(document, range, cancellationToken).ConfigureAwait(false)); + return true; } - public override Task> GetDiagnosticsForSpanAsync(Document document, TextSpan range, CancellationToken cancellationToken) + public override async Task> GetDiagnosticsForSpanAsync(Document document, TextSpan range, CancellationToken cancellationToken) { - return SpecializedTasks.EmptyEnumerable(); + var diagnostics = await GetDiagnosticsAsync(document.Project.Solution, document.Project.Id, document.Id, cancellationToken).ConfigureAwait(false); + return diagnostics.Where(d => range.IntersectsWith(d.TextSpan)); + } + + private async Task> GetProjectDiagnosticsAsync(Project project, CancellationToken cancellationToken) + { + if (project == null) + { + return ImmutableArray.Empty; + } + + var compilation = await project.GetCompilationAsync(cancellationToken).ConfigureAwait(false); + + // TODO: this should move to _analyzerManager + var analyzers = _analyzerManager.GetHostDiagnosticAnalyzersPerReference(project.Language) + .SelectMany(kv => kv.Value) + .Concat(project.AnalyzerReferences.SelectMany(r => r.GetAnalyzers(project.Language))); + + var compilationWithAnalyzer = compilation.WithAnalyzers(analyzers.ToImmutableArray(), project.AnalyzerOptions, cancellationToken); + + // REVIEW: this API is a bit strange. + // if getting diagnostic is cancelled, it has to create new compilation and do everything from scretch again? + return GetDiagnosticData(project, await compilationWithAnalyzer.GetAnalyzerDiagnosticsAsync().ConfigureAwait(false)).ToImmutableArrayOrEmpty(); + } + + private IEnumerable GetDiagnosticData(Project project, ImmutableArray diagnostics) + { + foreach (var diagnostic in diagnostics) + { + if (diagnostic.Location == Location.None) + { + yield return DiagnosticData.Create(project, diagnostic); + continue; + } + + var document = project.GetDocument(diagnostic.Location.SourceTree); + if (document == null) + { + continue; + } + + yield return DiagnosticData.Create(document, diagnostic); + } + } + + private void RaiseEvetns(Project project, ImmutableArray diagnostics) + { + var groups = diagnostics.GroupBy(d => d.DocumentId); + + var solution = project.Solution; + var workspace = solution.Workspace; + + foreach (var kv in groups) + { + if (kv.Key == null) + { + _owner.RaiseDiagnosticsUpdated( + this, new DiagnosticsUpdatedArgs( + ValueTuple.Create(this, project.Id), workspace, solution, project.Id, null, kv.ToImmutableArrayOrEmpty())); + continue; + } + + _owner.RaiseDiagnosticsUpdated( + this, new DiagnosticsUpdatedArgs( + ValueTuple.Create(this, kv.Key), workspace, solution, project.Id, kv.Key, kv.ToImmutableArrayOrEmpty())); + } } } } From a3e0ca9b03b475c8fdf687fcfc7df701253bb63b Mon Sep 17 00:00:00 2001 From: Heejae Chang Date: Thu, 12 Feb 2015 19:48:06 -0800 Subject: [PATCH 2/4] fix reporting issue and more fix on de-duplication fixed reporting issue and de-duplication issues. --- .../Core/Diagnostics/AnalyzerManager.cs | 72 ++++++++++++++----- .../EngineV2/DiagnosticIncrementalAnalyzer.cs | 10 ++- 2 files changed, 59 insertions(+), 23 deletions(-) diff --git a/src/Features/Core/Diagnostics/AnalyzerManager.cs b/src/Features/Core/Diagnostics/AnalyzerManager.cs index 17f636fe5e99c..9090ed528cf7d 100644 --- a/src/Features/Core/Diagnostics/AnalyzerManager.cs +++ b/src/Features/Core/Diagnostics/AnalyzerManager.cs @@ -113,11 +113,28 @@ public ImmutableDictionary> GetHost /// Get identity and s map for given /// public ImmutableDictionary> GetDiagnosticDescriptorsPerReference(Project project) + { + return GetDiagnosticDescriptorsPerReference(GetDiagnosticAnalyzersPerReference(project)); + } + + /// + /// Get identity and s map for given + /// + public ImmutableDictionary> GetDiagnosticAnalyzersPerReference(Project project) { var hostAnalyzerReferences = GetHostDiagnosticAnalyzersPerReference(project.Language); - var projectAnalyzerReferences = CreateDiagnosticAnalyzersPerReferenceMap(CreateAnalyzerReferencesMap(project.AnalyzerReferences), project.Language); + var projectAnalyzerReferences = CreateDiagnosticAnalyzersPerReferenceMap(CreateAnalyzerReferencesMap(project.AnalyzerReferences.Where(CheckAnalyzerReferenceIdentity)), project.Language); - return GetDiagnosticDescriptorsPerReference(hostAnalyzerReferences.Concat(projectAnalyzerReferences)); + return MergeDiagnosticAnalyzerMap(hostAnalyzerReferences, projectAnalyzerReferences); + } + + /// + /// Get s for given + /// + public ImmutableArray GetDiagnosticAnalyzers(Project project) + { + var analyzersPerReferences = GetDiagnosticAnalyzersPerReference(project); + return analyzersPerReferences.SelectMany(kv => kv.Value).ToImmutableArray(); } /// @@ -129,33 +146,22 @@ public ImmutableDictionary> GetHostDi } private ImmutableDictionary> GetDiagnosticDescriptorsPerReference( - IEnumerable>> analyzersMap) + ImmutableDictionary> analyzersMap) { - var seen = new HashSet(); var builder = ImmutableDictionary.CreateBuilder>(); foreach (var kv in analyzersMap) { var referenceId = kv.Key; var analyzers = kv.Value; - // this can happen if same analyzer exist in both host and projects. - if (builder.ContainsKey(referenceId)) - { - continue; - } - var descriptors = ImmutableArray.CreateBuilder(); foreach (var analyzer in analyzers) { - // don't put duplicated analyzers - if (analyzer == null || !seen.Add(analyzer)) - { - continue; - } - + // given map should be in good shape. no duplication. no null and etc descriptors.AddRange(GetDiagnosticDescriptors(analyzer)); } + // there can't be duplication since _hostAnalyzerReferenceMap is already de-duplicated. builder.Add(referenceId, descriptors.ToImmutable()); } @@ -190,6 +196,16 @@ private static string GetAnalyzerReferenceId(AnalyzerReference reference) return reference.Display ?? FeaturesResources.Unknown; } + private bool CheckAnalyzerReferenceIdentity(AnalyzerReference reference) + { + if (reference == null) + { + return false; + } + + return !_hostAnalyzerReferencesMap.ContainsKey(GetAnalyzerReferenceId(reference)); + } + private static ImmutableDictionary> CreateDiagnosticAnalyzersPerReferenceMap( IDictionary analyzerReferencesMap, string languageOpt = null) { @@ -204,7 +220,7 @@ private static ImmutableDictionary> C } // input "analyzerReferencesMap" is a dictionary, so there will be no duplication here. - builder.Add(reference.Key, analyzers); + builder.Add(reference.Key, analyzers.WhereNotNull().ToImmutableArray()); } return builder.ToImmutable(); @@ -248,5 +264,27 @@ private static ImmutableArray CreateAnalyzerReferencesFromAss return builder.ToImmutable(); } + + private static ImmutableDictionary> MergeDiagnosticAnalyzerMap( + ImmutableDictionary> map1, ImmutableDictionary> map2) + { + var current = map1; + var seen = new HashSet(map1.Values.SelectMany(v => v)); + + foreach (var kv in map2) + { + var referenceIdentity = kv.Key; + var analyzers = kv.Value; + + if (map1.ContainsKey(referenceIdentity)) + { + continue; + } + + current = current.Add(referenceIdentity, analyzers.Where(a => seen.Add(a)).ToImmutableArray()); + } + + return current; + } } } diff --git a/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs b/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs index b6fca5ada1592..7844d3f05791e 100644 --- a/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs +++ b/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs @@ -61,13 +61,13 @@ public override Task NewSolutionSnapshotAsync(Solution solution, CancellationTok public override void RemoveDocument(DocumentId documentId) { _owner.RaiseDiagnosticsUpdated( - this, new DiagnosticsUpdatedArgs(ValueTuple.Create(this, documentId), null, null, null, null, ImmutableArray.Empty)); + this, new DiagnosticsUpdatedArgs(ValueTuple.Create(this, documentId), _workspace, null, null, null, ImmutableArray.Empty)); } public override void RemoveProject(ProjectId projectId) { _owner.RaiseDiagnosticsUpdated( - this, new DiagnosticsUpdatedArgs(ValueTuple.Create(this, projectId), null, null, null, null, ImmutableArray.Empty)); + this, new DiagnosticsUpdatedArgs(ValueTuple.Create(this, projectId), _workspace, null, null, null, ImmutableArray.Empty)); } #endregion @@ -155,11 +155,9 @@ private async Task> GetProjectDiagnosticsAsync(Pr var compilation = await project.GetCompilationAsync(cancellationToken).ConfigureAwait(false); // TODO: this should move to _analyzerManager - var analyzers = _analyzerManager.GetHostDiagnosticAnalyzersPerReference(project.Language) - .SelectMany(kv => kv.Value) - .Concat(project.AnalyzerReferences.SelectMany(r => r.GetAnalyzers(project.Language))); + var analyzers = _analyzerManager.GetDiagnosticAnalyzers(project); - var compilationWithAnalyzer = compilation.WithAnalyzers(analyzers.ToImmutableArray(), project.AnalyzerOptions, cancellationToken); + var compilationWithAnalyzer = compilation.WithAnalyzers(analyzers, project.AnalyzerOptions, cancellationToken); // REVIEW: this API is a bit strange. // if getting diagnostic is cancelled, it has to create new compilation and do everything from scretch again? From dfaa0f57f93a6cd867c996156f2047ec32df5e8e Mon Sep 17 00:00:00 2001 From: Heejae Chang Date: Thu, 12 Feb 2015 20:49:03 -0800 Subject: [PATCH 3/4] code cleanup fixed typo and removed stale comments --- .../Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs b/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs index 7844d3f05791e..eea403ea70a41 100644 --- a/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs +++ b/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs @@ -35,7 +35,7 @@ public override async Task AnalyzeProjectAsync(Project project, bool semanticsCh { var diagnostics = await GetDiagnosticsAsync(project.Solution, project.Id, null, cancellationToken).ConfigureAwait(false); - RaiseEvetns(project, diagnostics); + RaiseEvents(project, diagnostics); } public override Task AnalyzeSyntaxAsync(Document document, CancellationToken cancellationToken) @@ -154,7 +154,6 @@ private async Task> GetProjectDiagnosticsAsync(Pr var compilation = await project.GetCompilationAsync(cancellationToken).ConfigureAwait(false); - // TODO: this should move to _analyzerManager var analyzers = _analyzerManager.GetDiagnosticAnalyzers(project); var compilationWithAnalyzer = compilation.WithAnalyzers(analyzers, project.AnalyzerOptions, cancellationToken); @@ -184,7 +183,7 @@ private IEnumerable GetDiagnosticData(Project project, Immutable } } - private void RaiseEvetns(Project project, ImmutableArray diagnostics) + private void RaiseEvents(Project project, ImmutableArray diagnostics) { var groups = diagnostics.GroupBy(d => d.DocumentId); From 2f07a386b9f5543547735c2aac8ad5d90af14d9c Mon Sep 17 00:00:00 2001 From: Heejae Chang Date: Thu, 12 Feb 2015 21:05:33 -0800 Subject: [PATCH 4/4] rename some of method to more clearly indicate what can be cached in manager and what must be cached by caller. GetXXX method is used for information that manager can cache or already cached. CreateXXX method is used for information that manager can't cache. --- .../Core/Diagnostics/AnalyzerManager.cs | 38 +++++++++---------- .../Diagnostics/DiagnosticAnalyzerService.cs | 2 +- .../EngineV2/DiagnosticIncrementalAnalyzer.cs | 2 +- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/Features/Core/Diagnostics/AnalyzerManager.cs b/src/Features/Core/Diagnostics/AnalyzerManager.cs index 9090ed528cf7d..564963b38695b 100644 --- a/src/Features/Core/Diagnostics/AnalyzerManager.cs +++ b/src/Features/Core/Diagnostics/AnalyzerManager.cs @@ -102,25 +102,33 @@ public ImmutableArray GetDiagnosticDescriptors(DiagnosticA } /// - /// Get identity and s map + /// Get identity and s map for given + /// + public ImmutableDictionary> GetHostDiagnosticAnalyzersPerReference(string language) + { + return _hostDiagnosticAnalyzersPerLanguageMap.GetOrAdd(language, CreateHostDiagnosticAnalyzers); + } + + /// + /// Create identity and s map /// public ImmutableDictionary> GetHostDiagnosticDescriptorsPerReference() { - return GetDiagnosticDescriptorsPerReference(_lazyHostDiagnosticAnalyzersPerReferenceMap.Value); + return CreateDiagnosticDescriptorsPerReference(_lazyHostDiagnosticAnalyzersPerReferenceMap.Value); } /// - /// Get identity and s map for given + /// Create identity and s map for given /// - public ImmutableDictionary> GetDiagnosticDescriptorsPerReference(Project project) + public ImmutableDictionary> CreateDiagnosticDescriptorsPerReference(Project project) { - return GetDiagnosticDescriptorsPerReference(GetDiagnosticAnalyzersPerReference(project)); + return CreateDiagnosticDescriptorsPerReference(CreateDiagnosticAnalyzersPerReference(project)); } /// - /// Get identity and s map for given + /// Create identity and s map for given /// - public ImmutableDictionary> GetDiagnosticAnalyzersPerReference(Project project) + public ImmutableDictionary> CreateDiagnosticAnalyzersPerReference(Project project) { var hostAnalyzerReferences = GetHostDiagnosticAnalyzersPerReference(project.Language); var projectAnalyzerReferences = CreateDiagnosticAnalyzersPerReferenceMap(CreateAnalyzerReferencesMap(project.AnalyzerReferences.Where(CheckAnalyzerReferenceIdentity)), project.Language); @@ -129,23 +137,15 @@ public ImmutableDictionary> GetDiagno } /// - /// Get s for given + /// Create s collection for given /// - public ImmutableArray GetDiagnosticAnalyzers(Project project) + public ImmutableArray CreateDiagnosticAnalyzers(Project project) { - var analyzersPerReferences = GetDiagnosticAnalyzersPerReference(project); + var analyzersPerReferences = CreateDiagnosticAnalyzersPerReference(project); return analyzersPerReferences.SelectMany(kv => kv.Value).ToImmutableArray(); } - /// - /// Get identity and s map for given - /// - public ImmutableDictionary> GetHostDiagnosticAnalyzersPerReference(string language) - { - return _hostDiagnosticAnalyzersPerLanguageMap.GetOrAdd(language, CreateHostDiagnosticAnalyzers); - } - - private ImmutableDictionary> GetDiagnosticDescriptorsPerReference( + private ImmutableDictionary> CreateDiagnosticDescriptorsPerReference( ImmutableDictionary> analyzersMap) { var builder = ImmutableDictionary.CreateBuilder>(); diff --git a/src/Features/Core/Diagnostics/DiagnosticAnalyzerService.cs b/src/Features/Core/Diagnostics/DiagnosticAnalyzerService.cs index dca90e8d361ce..5433f3841ce9e 100644 --- a/src/Features/Core/Diagnostics/DiagnosticAnalyzerService.cs +++ b/src/Features/Core/Diagnostics/DiagnosticAnalyzerService.cs @@ -43,7 +43,7 @@ public ImmutableDictionary> GetDiag return _analyzerManager.GetHostDiagnosticDescriptorsPerReference(); } - return _analyzerManager.GetDiagnosticDescriptorsPerReference(projectOpt); + return _analyzerManager.CreateDiagnosticDescriptorsPerReference(projectOpt); } public ImmutableArray GetDiagnosticDescriptors(DiagnosticAnalyzer analyzer) diff --git a/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs b/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs index eea403ea70a41..b00c9d701af1c 100644 --- a/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs +++ b/src/Features/Core/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs @@ -154,7 +154,7 @@ private async Task> GetProjectDiagnosticsAsync(Pr var compilation = await project.GetCompilationAsync(cancellationToken).ConfigureAwait(false); - var analyzers = _analyzerManager.GetDiagnosticAnalyzers(project); + var analyzers = _analyzerManager.CreateDiagnosticAnalyzers(project); var compilationWithAnalyzer = compilation.WithAnalyzers(analyzers, project.AnalyzerOptions, cancellationToken);