Skip to content

Add support for compilation end code fix. #1132

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
heejaechang merged 5 commits into from
Mar 10, 2015
Merged

Add support for compilation end code fix. #1132

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dc8cbac
Add support for compilation end code fix.
heejaechang Mar 6, 2015
0b4aa1f
Merge branch 'master' of https://github.com/dotnet/roslyn into Compil…
heejaechang Mar 10, 2015
b0ab5b6
move some of code to new C# language feature and comment fix up
heejaechang Mar 10, 2015
ce7fdf6
fix invalid assert and more spelling fix
heejaechang Mar 10, 2015
c16f92f
Merge branch 'master' of https://github.com/dotnet/roslyn into Compil…
heejaechang Mar 10, 2015
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
2 changes: 1 addition & 1 deletion src/EditorFeatures/Core/Implementation/TodoComment/TodoCommentState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ protected override void WriteTo(Stream stream, Data data, CancellationToken canc
public ImmutableArray<ITaskItem> GetItems_TestingOnly(DocumentId documentId)
{
Data data;
if (this.DataCache.TryGetValue(documentId, out data))
if (this.DataCache.TryGetValue(documentId, out data) && data != null)
{
return data.Items;
}
Expand Down
15 changes: 11 additions & 4 deletions src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Imports Microsoft.CodeAnalysis.Diagnostics
Imports Microsoft.CodeAnalysis.Diagnostics.EngineV1
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
Imports Microsoft.CodeAnalysis.Options
Imports Microsoft.CodeAnalysis.Test.Utilities
Imports Microsoft.CodeAnalysis.Text
Imports Roslyn.Utilities
Expand Down Expand Up @@ -594,6 +595,11 @@ class AnonymousFunctions

Using workspace = TestWorkspaceFactory.CreateWorkspace(test)
Dim project = workspace.CurrentSolution.Projects.Single()

' turn off heuristic
Dim options = workspace.Services.GetService(Of IOptionService)()
options.SetOptions(options.GetOptions.WithChangedOption(InternalDiagnosticsOptions.UseCompilationEndCodeFixHueristic, False))

@msJohnHamby msJohnHamby Mar 9, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"Heuristic" is the expected spelling. What is the heuristic? How reliable is it?

@heejaechang heejaechang Mar 10, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@JohnHamby heuristic is that, if there is existing compilation end diagnostic, we show code fix for it, otherwise, we won't.

but this behavior is controlled by an option you can change from Tools/Options/Roslyn/Diagnostics

until we have async support from platform, we probably need to test several different experiences. this is just one of them.


Dim analyzer = New CompilationEndedAnalyzer
Dim analyzerReference = New AnalyzerImageReference(ImmutableArray.Create(Of DiagnosticAnalyzer)(analyzer))
project = project.AddAnalyzerReference(analyzerReference)
Expand All @@ -604,17 +610,18 @@ class AnonymousFunctions
Dim descriptorsMap = diagnosticService.GetDiagnosticDescriptors(project)
Assert.Equal(1, descriptorsMap.Count)

' Ask for document diagnostics multiple times, and verify compilation diagnostics are not reported.
' Ask for document diagnostics multiple times, and verify compilation diagnostics are reported.
Dim document = project.Documents.Single()

Dim fullSpan = document.GetSyntaxRootAsync().WaitAndGetResult(CancellationToken.None).FullSpan
Dim diagnostics = diagnosticService.GetDiagnosticsForSpanAsync(document, fullSpan, CancellationToken.None).WaitAndGetResult(CancellationToken.None)
Assert.Equal(0, diagnostics.Count())
Assert.Equal(1, diagnostics.Count())

@mavasani mavasani Mar 9, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should change this to now verify that the diagnostic returned is the one with Location in this document. Probably all the last code block at end of this method into a separate method and invoke that at 3 places where we verify diagnosticService.GetDiagnosticsForSpanAsync

@heejaechang heejaechang Mar 10, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mavasani not sure I understood what you are asking.

@mavasani mavasani Mar 10, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Basically this analyzer reports 2 diagnostics, one with location and one without location (see projectDiagnostics below). We should verify that the single diagnostic returned by GetDiagnosticsForSpanAsync is same as withDocumentLocationDiagnostic.

@heejaechang heejaechang Mar 10, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah, sure :)


diagnostics = diagnosticService.GetDiagnosticsForSpanAsync(document, fullSpan, CancellationToken.None).WaitAndGetResult(CancellationToken.None)
Assert.Equal(0, diagnostics.Count())
Assert.Equal(1, diagnostics.Count())

diagnostics = diagnosticService.GetDiagnosticsForSpanAsync(document, fullSpan, CancellationToken.None).WaitAndGetResult(CancellationToken.None)
Assert.Equal(0, diagnostics.Count())
Assert.Equal(1, diagnostics.Count())

' Verify compilation diagnostics are reported with correct location info when asked for project diagnostics.
Dim projectDiagnostics = diagnosticService.GetProjectDiagnosticsForIdsAsync(project.Solution, project.Id).WaitAndGetResult(CancellationToken.None)
Expand Down
5 changes: 0 additions & 5 deletions src/Features/Core/Diagnostics/EngineV1/DiagnosticAnalyzerDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ internal class DiagnosticAnalyzerDriver
private readonly AbstractHostDiagnosticUpdateSource _hostDiagnosticUpdateSource;
private readonly CancellationToken _cancellationToken;
private readonly ISyntaxNodeAnalyzerService _syntaxNodeAnalyzerService;
private readonly Dictionary<SyntaxNode, ImmutableArray<SyntaxNode>> _descendantExecutableNodesMap;
private readonly ISyntaxFactsService _syntaxFacts;
private readonly IGeneratedCodeRecognitionService _generatedCodeService;
private readonly IAnalyzerDriverService _analyzerDriverService;

Expand Down Expand Up @@ -74,8 +72,6 @@ internal DiagnosticAnalyzerDriver(
_syntaxNodeAnalyzerService = syntaxNodeAnalyzerService;
_hostDiagnosticUpdateSource = hostDiagnosticUpdateSource;
_cancellationToken = cancellationToken;
_descendantExecutableNodesMap = new Dictionary<SyntaxNode, ImmutableArray<SyntaxNode>>();
_syntaxFacts = document.Project.LanguageServices.GetService<ISyntaxFactsService>();
_generatedCodeService = document.Project.Solution.Workspace.Services.GetService<IGeneratedCodeRecognitionService>();
_analyzerDriverService = document.Project.LanguageServices.GetService<IAnalyzerDriverService>();
_analyzerOptions = new WorkspaceAnalyzerOptions(_project.AnalyzerOptions, _project.Solution.Workspace);
Expand All @@ -97,7 +93,6 @@ internal DiagnosticAnalyzerDriver(
_generatedCodeService = project.Solution.Workspace.Services.GetService<IGeneratedCodeRecognitionService>();
_analyzerDriverService = project.LanguageServices.GetService<IAnalyzerDriverService>();
_hostDiagnosticUpdateSource = hostDiagnosticUpdateSource;
_descendantExecutableNodesMap = null;
_analyzerOptions = new WorkspaceAnalyzerOptions(_project.AnalyzerOptions, _project.Solution.Workspace);
_onAnalyzerException = overriddenOnAnalyzerException ?? Default_OnAnalyzerException;
_onAnalyzerException_NoTelemetryLogging = overriddenOnAnalyzerException ?? Default_OnAnalyzerException_NoTelemetryLogging;
Expand Down
5 changes: 2 additions & 3 deletions src/Features/Core/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer.DiagnosticState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,14 @@ private AnalysisData TryGetExistingData(Stream stream, Project project, Document
var textVersion = VersionStamp.ReadFrom(reader);
var dataVersion = VersionStamp.ReadFrom(reader);

// textversion can be default for document from project analysis.
if (dataVersion == VersionStamp.Default)
if (textVersion == VersionStamp.Default || dataVersion == VersionStamp.Default)
{
return null;
}

AppendItems(reader, project, document, list, cancellationToken);

return new AnalysisData(textVersion, dataVersion, list.ToImmutableArray<DiagnosticData>());
return new AnalysisData(textVersion, dataVersion, list.ToImmutableArray());
}
}
catch (Exception)
Expand Down
27 changes: 15 additions & 12 deletions ...iagnostics/EngineV1/DiagnosticIncrementalAnalyzer.IncrementalAnalyzer.AnalyzerExecutor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,13 @@ public async Task<AnalysisData> GetProjectAnalysisDataAsync(DiagnosticAnalyzerDr
var state = stateSet.GetState(StateType.Project);
var existingData = await GetExistingProjectAnalysisDataAsync(project, state, cancellationToken).ConfigureAwait(false);

// TODO:
// if there is any document level result, we can't ever use cache since we can't track those changes in current design
// hopely v2 engine, either don't care this at all, or can deal with this better
if (CheckSemanticVersions(project, existingData, versions) && !existingData.Items.Any(d => d.DocumentId != null))
if (CheckSemanticVersions(project, existingData, versions))
{
return existingData;
}

var diagnosticData = await GetProjectDiagnosticsAsync(analyzerDriver, stateSet.Analyzer, _owner.ForceAnalyzeAllDocuments).ConfigureAwait(false);
return new AnalysisData(VersionStamp.Default, versions.DataVersion, GetExistingItems(existingData), diagnosticData.AsImmutableOrEmpty());
return new AnalysisData(versions.TextVersion, versions.DataVersion, GetExistingItems(existingData), diagnosticData.AsImmutableOrEmpty());
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
{
Expand All @@ -140,19 +137,22 @@ public async Task<AnalysisData> GetProjectAnalysisDataAsync(DiagnosticAnalyzerDr

private async Task<AnalysisData> GetExistingProjectAnalysisDataAsync(Project project, DiagnosticState state, CancellationToken cancellationToken)
{
var dataVersion = VersionStamp.Default;
var existingData = await state.TryGetExistingDataAsync(project, cancellationToken).ConfigureAwait(false);

// quick path.
if (existingData == null || existingData.Items.Length == 0)
// quick bail out
if (state.Count == 0)
{
return existingData;
return null;
}

var textVersion = VersionStamp.Default;
var dataVersion = VersionStamp.Default;
var existingData = await state.TryGetExistingDataAsync(project, cancellationToken).ConfigureAwait(false);

var builder = ImmutableArray.CreateBuilder<DiagnosticData>();
if (existingData != null)
{
textVersion = existingData.TextVersion;
dataVersion = existingData.DataVersion;

builder.AddRange(existingData.Items);
}

Expand All @@ -169,16 +169,19 @@ private async Task<AnalysisData> GetExistingProjectAnalysisDataAsync(Project pro
continue;
}

textVersion = existingData.TextVersion;
dataVersion = existingData.DataVersion;

builder.AddRange(existingData.Items);
}

if (dataVersion == VersionStamp.Default)
{
Contract.Requires(textVersion == VersionStamp.Default);
return null;
}

return new AnalysisData(VersionStamp.Default, dataVersion, builder.ToImmutable());
return new AnalysisData(textVersion, dataVersion, builder.ToImmutable());

@mavasani mavasani Mar 9, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor: Assert text version is not default here or probably in ctor of AnalysisData?

@heejaechang heejaechang Mar 10, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

}

private bool CanUseDocumentState(AnalysisData existingData, VersionStamp textVersion, VersionStamp dataVersion)
Expand Down
4 changes: 2 additions & 2 deletions src/Features/Core/Diagnostics/EngineV1/DiagnosticIncrementalAnalyzer.StateSet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ public DiagnosticState GetState(StateType stateType)
return _state[(int)stateType];
}

public void Remove(object key)
public void Remove(object documentOrProjectId)
{
for (var stateType = 0; stateType < s_stateTypeCount; stateType++)
{
_state[stateType].Remove(key);
_state[stateType].Remove(documentOrProjectId);
}
}

Expand Down
Loading