Skip to content

Commit

Permalink
Merge pull request dotnet#1402 from heejaechang/staleerror
Browse files Browse the repository at this point in the history
change the way build and live diagnostics are merged
  • Loading branch information
heejaechang committed Mar 27, 2015
2 parents 6d33966 + 79f3481 commit 0ab8e70
Show file tree
Hide file tree
Showing 17 changed files with 618 additions and 294 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ internal override ImmutableArray<int> GetSupportedErrorCodes()
// We don't support configuring WRN_ALinkWarn. See comments in method "CSharpDiagnosticFilter.Filter" for more details.
continue;

case (int)ErrorCode.WRN_UnreferencedField:
// unused field. current live error doesn't support this.
continue;

default:
builder.Add(errorCode);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ public sealed override void Initialize(AnalysisContext context)
c.RegisterSyntaxTreeAction(analyzer.AnalyzeSyntaxTree);
c.RegisterSemanticModelAction(CompilationAnalyzer.AnalyzeSemanticModel);
});

context.RegisterCompilationAction(CompilationAnalyzer.AnalyzeCompilation);
}
}
}
26 changes: 26 additions & 0 deletions src/Features/Core/Diagnostics/BaseDiagnosticIncrementalAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,32 @@ protected BaseDiagnosticIncrementalAnalyzer(DiagnosticAnalyzerService owner, Wor
public abstract Task<IEnumerable<DiagnosticData>> GetDiagnosticsForSpanAsync(Document document, TextSpan range, CancellationToken cancellationToken);
#endregion

#region build error synchronization
/// <summary>
/// Callback from build listener.
///
/// Given diagnostics are errors host got from explicit build.
/// It is up to each incremental analyzer how they will merge this information with live diagnostic info.
///
/// this API doesn't have cancellationToken since it can't be cancelled.
///
/// given diagnostics are project wide diagnsotics that doesn't contain a source location.
/// </summary>
public abstract Task SynchronizeWithBuildAsync(Project project, ImmutableArray<DiagnosticData> diagnostics);

/// <summary>
/// Callback from build listener.
///
/// Given diagnostics are errors host got from explicit build.
/// It is up to each incremental analyzer how they will merge this information with live diagnostic info
///
/// this API doesn't have cancellationToken since it can't be cancelled.
///
/// given diagnostics are ones that has a source location.
/// </summary>
public abstract Task SynchronizeWithBuildAsync(Document document, ImmutableArray<DiagnosticData> diagnostics);
#endregion

internal DiagnosticAnalyzerService Owner { get; }
internal Workspace Workspace { get; }
internal AbstractHostDiagnosticUpdateSource HostDiagnosticUpdateSource { get; }
Expand Down
5 changes: 5 additions & 0 deletions src/Features/Core/Diagnostics/DiagnosticAnalyzerService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,11 @@ public Task<ImmutableArray<DiagnosticData>> GetProjectDiagnosticsForIdsAsync(
return SpecializedTasks.EmptyImmutableArray<DiagnosticData>();
}

public bool IsCompilerDiagnostic(string language, DiagnosticData diagnostic)
{
return _hostAnalyzerManager.IsCompilerDiagnostic(language, diagnostic);
}

// virtual for testing purposes.
internal virtual Action<Exception, DiagnosticAnalyzer, Diagnostic> GetOnAnalyzerException(ProjectId projectId, DiagnosticLogAggregator diagnosticLogAggregator)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Immutable;
using System.Threading.Tasks;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Diagnostics
{
internal partial class DiagnosticAnalyzerService
{
/// <summary>
/// Synchronize build errors with live error.
///
/// no cancellationToken since this can't be cancelled
/// </summary>
public Task SynchronizeWithBuildAsync(Project project, ImmutableArray<DiagnosticData> diagnostics)
{
BaseDiagnosticIncrementalAnalyzer analyzer;
if (_map.TryGetValue(project.Solution.Workspace, out analyzer))
{
return analyzer.SynchronizeWithBuildAsync(project, diagnostics);
}

return SpecializedTasks.EmptyTask;
}

/// <summary>
/// Synchronize build errors with live error
///
/// no cancellationToken since this can't be cancelled
/// </summary>
public Task SynchronizeWithBuildAsync(Document document, ImmutableArray<DiagnosticData> diagnostics)
{
BaseDiagnosticIncrementalAnalyzer analyzer;
if (_map.TryGetValue(document.Project.Solution.Workspace, out analyzer))
{
return analyzer.SynchronizeWithBuildAsync(document, diagnostics);
}

return SpecializedTasks.EmptyTask;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Diagnostics.Log;
using Microsoft.CodeAnalysis.Internal.Log;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Shared.Options;
Expand Down Expand Up @@ -170,6 +168,18 @@ public override Task<IEnumerable<DiagnosticData>> GetDiagnosticsForSpanAsync(Doc
}
#endregion

#region build synchronization
public override Task SynchronizeWithBuildAsync(Project project, ImmutableArray<DiagnosticData> diagnostics)
{
return Analyzer.SynchronizeWithBuildAsync(project, diagnostics);
}

public override Task SynchronizeWithBuildAsync(Document document, ImmutableArray<DiagnosticData> diagnostics)
{
return Analyzer.SynchronizeWithBuildAsync(document, diagnostics);
}
#endregion

public void TurnOff(bool useV2)
{
var turnedOffAnalyzer = GetAnalyzer(!useV2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ private async Task<AnalysisData> GetExistingProjectAnalysisDataAsync(Project pro
return null;
}

Contract.Requires(textVersion != VersionStamp.Default);
return new AnalysisData(textVersion, dataVersion, builder.ToImmutable());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ private AnalysisData TryGetExistingData(Stream stream, Project project, Document
var textVersion = VersionStamp.ReadFrom(reader);
var dataVersion = VersionStamp.ReadFrom(reader);

if (textVersion == VersionStamp.Default || dataVersion == VersionStamp.Default)
if (dataVersion == VersionStamp.Default)
{
return null;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Globalization;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Diagnostics.EngineV1
{
internal partial class DiagnosticIncrementalAnalyzer
{
public override async Task SynchronizeWithBuildAsync(Project project, ImmutableArray<DiagnosticData> diagnostics)
{
if (!PreferBuildErrors(project.Solution.Workspace))
{
// prefer live errors over build errors
return;
}

using (var poolObject = SharedPools.Default<HashSet<string>>().GetPooledObject())
{
var lookup = CreateDiagnosticIdLookup(diagnostics);

foreach (var stateSet in _stateManger.GetStateSets(project))
{
var descriptors = HostAnalyzerManager.GetDiagnosticDescriptors(stateSet.Analyzer);
var liveDiagnostics = ConvertToLiveDiagnostics(lookup, descriptors, poolObject.Object);

// we are using Default so that things like LB can't use cached information
var projectTextVersion = VersionStamp.Default;
var semanticVersion = await project.GetDependentSemanticVersionAsync(CancellationToken.None).ConfigureAwait(false);

var state = stateSet.GetState(StateType.Project);
var existingDiagnostics = await state.TryGetExistingDataAsync(project, CancellationToken.None).ConfigureAwait(false);

var mergedDiagnostics = MergeDiagnostics(liveDiagnostics, GetExistingDiagnostics(existingDiagnostics));
await state.PersistAsync(project, new AnalysisData(projectTextVersion, semanticVersion, mergedDiagnostics), CancellationToken.None).ConfigureAwait(false);
RaiseDiagnosticsUpdated(StateType.Project, project.Id, stateSet.Analyzer, new SolutionArgument(project), mergedDiagnostics);
}
}
}

public override async Task SynchronizeWithBuildAsync(Document document, ImmutableArray<DiagnosticData> diagnostics)
{
if (!PreferBuildErrors(document.Project.Solution.Workspace))
{
// prefer live errors over build errors
return;
}

using (var poolObject = SharedPools.Default<HashSet<string>>().GetPooledObject())
{
var lookup = CreateDiagnosticIdLookup(diagnostics);

foreach (var stateSet in _stateManger.GetStateSets(document.Project))
{
// we are using Default so that things like LB can't use cached information
var textVersion = VersionStamp.Default;
var semanticVersion = await document.Project.GetDependentSemanticVersionAsync(CancellationToken.None).ConfigureAwait(false);

// clear document and project live errors
await PersistAndReportAsync(stateSet, StateType.Project, document, textVersion, semanticVersion, ImmutableArray<DiagnosticData>.Empty).ConfigureAwait(false);
await PersistAndReportAsync(stateSet, StateType.Syntax, document, textVersion, semanticVersion, ImmutableArray<DiagnosticData>.Empty).ConfigureAwait(false);

var descriptors = HostAnalyzerManager.GetDiagnosticDescriptors(stateSet.Analyzer);
var liveDiagnostics = ConvertToLiveDiagnostics(lookup, descriptors, poolObject.Object);

// REVIEW: for now, we are putting build error in document state rather than creating its own state.
// reason is so that live error can take over it as soon as possible
// this also means there can be slight race where it will clean up eventually. if live analysis runs for syntax but didn't run
// for document yet, then we can have duplicated entries in the error list until live analysis catch.
await PersistAndReportAsync(stateSet, StateType.Document, document, textVersion, semanticVersion, liveDiagnostics).ConfigureAwait(false);
}
}
}

private bool PreferBuildErrors(Workspace workspace)
{
return workspace.Options.GetOption(InternalDiagnosticsOptions.BuildErrorIsTheGod) || workspace.Options.GetOption(InternalDiagnosticsOptions.BuildErrorWinLiveError);
}

private async Task PersistAndReportAsync(
StateSet stateSet, StateType stateType, Document document, VersionStamp textVersion, VersionStamp semanticVersion, ImmutableArray<DiagnosticData> diagnostics)
{
var state = stateSet.GetState(stateType);
var existingDiagnostics = await state.TryGetExistingDataAsync(document, CancellationToken.None).ConfigureAwait(false);

var mergedDiagnostics = MergeDiagnostics(diagnostics, GetExistingDiagnostics(existingDiagnostics));
await state.PersistAsync(document, new AnalysisData(textVersion, semanticVersion, mergedDiagnostics), CancellationToken.None).ConfigureAwait(false);
RaiseDiagnosticsUpdated(stateType, document.Id, stateSet.Analyzer, new SolutionArgument(document), mergedDiagnostics);
}

private static ImmutableArray<DiagnosticData> GetExistingDiagnostics(AnalysisData analysisData)
{
if (analysisData == null)
{
return ImmutableArray<DiagnosticData>.Empty;
}

return analysisData.Items;
}

private ImmutableArray<DiagnosticData> MergeDiagnostics(ImmutableArray<DiagnosticData> liveDiagnostics, ImmutableArray<DiagnosticData> existingDiagnostics)
{
ImmutableArray<DiagnosticData>.Builder builder = null;

if (liveDiagnostics.Length > 0)
{
builder = ImmutableArray.CreateBuilder<DiagnosticData>();
builder.AddRange(liveDiagnostics);
}

if (existingDiagnostics.Length > 0)
{
builder = builder ?? ImmutableArray.CreateBuilder<DiagnosticData>();
builder.AddRange(existingDiagnostics.Where(d => d.Severity == DiagnosticSeverity.Hidden));
}

return builder == null ? ImmutableArray<DiagnosticData>.Empty : builder.ToImmutable();
}

private static ILookup<string, DiagnosticData> CreateDiagnosticIdLookup(ImmutableArray<DiagnosticData> diagnostics)
{
if (diagnostics.Length == 0)
{
return null;
}

return diagnostics.ToLookup(d => d.Id);
}

private ImmutableArray<DiagnosticData> ConvertToLiveDiagnostics(
ILookup<string, DiagnosticData> lookup, ImmutableArray<DiagnosticDescriptor> descriptors, HashSet<string> seen)
{
if (lookup == null)
{
return ImmutableArray<DiagnosticData>.Empty;
}

ImmutableArray<DiagnosticData>.Builder builder = null;
foreach (var descriptor in descriptors)
{
// make sure we don't report same id to multiple different analyzers
if (!seen.Add(descriptor.Id))
{
// TODO: once we have information where diagnostic came from, we probably doesnt need this.
continue;
}

var items = lookup[descriptor.Id];
if (items == null)
{
continue;
}

builder = builder ?? ImmutableArray.CreateBuilder<DiagnosticData>();
builder.AddRange(items.Select(d => CreateLiveDiagnostic(descriptor, d)));
}

return builder == null ? ImmutableArray<DiagnosticData>.Empty : builder.ToImmutable();
}

private static DiagnosticData CreateLiveDiagnostic(DiagnosticDescriptor descriptor, DiagnosticData diagnostic)
{
return new DiagnosticData(
descriptor.Id,
descriptor.Category,
diagnostic.Message,
descriptor.MessageFormat.ToString(DiagnosticData.USCultureInfo),
diagnostic.Severity,
descriptor.DefaultSeverity,
descriptor.IsEnabledByDefault,
diagnostic.WarningLevel,
descriptor.CustomTags.ToImmutableArray(),
diagnostic.Properties,
diagnostic.Workspace,
diagnostic.ProjectId,
diagnostic.DocumentId,
diagnostic.HasTextSpan ? diagnostic.TextSpan : (TextSpan?)null,
diagnostic.MappedFilePath, diagnostic.MappedStartLine, diagnostic.MappedStartColumn, diagnostic.MappedEndLine, diagnostic.MappedEndColumn,
diagnostic.OriginalFilePath, diagnostic.OriginalStartLine, diagnostic.OriginalStartColumn, diagnostic.OriginalEndLine, diagnostic.OriginalEndColumn,
descriptor.Title.ToString(CultureInfo.CurrentUICulture),
descriptor.Description.ToString(CultureInfo.CurrentUICulture),
descriptor.HelpLinkUri);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace Microsoft.CodeAnalysis.Diagnostics.EngineV2
internal class DiagnosticIncrementalAnalyzer : BaseDiagnosticIncrementalAnalyzer
{
private readonly int _correlationId;

public DiagnosticIncrementalAnalyzer(DiagnosticAnalyzerService owner, int correlationId, Workspace workspace, HostAnalyzerManager hostAnalyzerManager, AbstractHostDiagnosticUpdateSource hostDiagnosticUpdateSource)
: base(owner, workspace, hostAnalyzerManager, hostDiagnosticUpdateSource)
{
Expand Down Expand Up @@ -181,6 +181,22 @@ private IEnumerable<DiagnosticData> GetDiagnosticData(Project project, Immutable
}
}

public override Task SynchronizeWithBuildAsync(Project project, ImmutableArray<DiagnosticData> diagnostics)
{
// V2 engine doesn't do anything.
// it means live error always win over build errors. build errors that can't be reported by live analyzer
// are already taken cared by engine
return SpecializedTasks.EmptyTask;
}

public override Task SynchronizeWithBuildAsync(Document document, ImmutableArray<DiagnosticData> diagnostics)
{
// V2 engine doesn't do anything.
// it means live error always win over build errors. build errors that can't be reported by live analyzer
// are already taken cared by engine
return SpecializedTasks.EmptyTask;
}

private void RaiseEvents(Project project, ImmutableArray<DiagnosticData> diagnostics)
{
var groups = diagnostics.GroupBy(d => d.DocumentId);
Expand Down
Loading

0 comments on commit 0ab8e70

Please sign in to comment.