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

Add "Active file" analysis scope for background analysis in the IDE #39699

Merged
merged 21 commits into from
Nov 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
e62aba5
Add "Active file" analysis scope for background analysis in the IDE
mavasani Sep 17, 2019
edb2504
Fix build for unit tests
mavasani Nov 5, 2019
4c66dd8
Couple more test fixes
mavasani Nov 6, 2019
3f20333
Fix integration tests
mavasani Nov 6, 2019
70e280d
Revert the change to DesignerAttributeIncrementalAnalyzer to bail out…
mavasani Nov 12, 2019
98b46aa
Improve comment for TODO incremental analyzer
mavasani Nov 12, 2019
3a6892d
Use lock for _lazyAnalyzers field
mavasani Nov 12, 2019
5de0b84
Address some more feedback
mavasani Nov 12, 2019
3cb849e
Tweak ReanalyzeOnOptionChange
mavasani Nov 12, 2019
014ac5c
Use lock for _lasActiveDocument field.
mavasani Nov 12, 2019
14606d7
Use current solution for options.
mavasani Nov 12, 2019
347f8e6
Add comment
mavasani Nov 12, 2019
262abdd
Revert an unnecessary change - we already handle the bail out for non…
mavasani Nov 12, 2019
2c07649
Update documentation and implementation for changes to BackgroundParser
mavasani Nov 12, 2019
382df2e
Update Obsolete attribute strings
mavasani Nov 12, 2019
cc090d1
Add comment
mavasani Nov 12, 2019
e5b8a1b
Add work coordinator unit tests for different background analysis scopes
mavasani Nov 13, 2019
111ec04
Address PM feedback and change the user facing resource strings to ma…
mavasani Nov 15, 2019
be5bd27
Switch the error list scope combo box to match the newly selected bac…
mavasani Nov 18, 2019
1c43467
Revert "Switch the error list scope combo box to match the newly sele…
mavasani Nov 19, 2019
c935dba
Revert the hard coded name check for SBD incremental analyzer as unit…
mavasani Nov 20, 2019
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 @@ -10,6 +10,7 @@
using Microsoft.CodeAnalysis.Editor.Shared.Options;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.Options;
using Microsoft.CodeAnalysis.SolutionCrawler;
using Microsoft.CodeAnalysis.Text;
using Microsoft.CodeAnalysis.TodoComments;
Expand Down Expand Up @@ -56,38 +57,49 @@ public async Task AnalyzeSyntaxAsync(Document document, InvocationReasons reason
return;
}

// Compute and persist the TODO comments for this document.
var (existingData, newData) = await ComputeAndPersistTodoCommentsAsync(document, _state, _todoCommentTokens, cancellationToken).ConfigureAwait(false);

// Now update the task list if there are any changes from the previously computed TODO comments, if any.
// NOTE: We don't check for cancellation here to ensure the task list view and our latest
// computed state are identical.
if (existingData == null || existingData.Items.Length != newData.Items.Length)
Copy link
Member

Choose a reason for hiding this comment

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

Can existingData and newData has different items but same length?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. However, this hasn't change as part of this PR, I am just refactoring the code in this PR. I will play around a bit with this and see if I can get a repro to drive this change.

{
Debug.Assert(_workspace == document.Project.Solution.Workspace);
RaiseTaskListUpdated(_workspace, document.Project.Solution, document.Id, newData.Items);
}
}

private static async Task<(Data existingData, Data newData)> ComputeAndPersistTodoCommentsAsync(
Document document,
TodoCommentState state,
TodoCommentTokens todoCommentTokens,
CancellationToken cancellationToken)
{
// use tree version so that things like compiler option changes are considered
var textVersion = await document.GetTextVersionAsync(cancellationToken).ConfigureAwait(false);
var syntaxVersion = await document.GetSyntaxVersionAsync(cancellationToken).ConfigureAwait(false);

var existingData = await _state.TryGetExistingDataAsync(document, cancellationToken).ConfigureAwait(false);
var existingData = await state.TryGetExistingDataAsync(document, cancellationToken).ConfigureAwait(false);
if (existingData != null)
{
// check whether we can use the data as it is (can happen when re-using persisted data from previous VS session)
if (CheckVersions(document, textVersion, syntaxVersion, existingData))
{
Debug.Assert(_workspace == document.Project.Solution.Workspace);
RaiseTaskListUpdated(_workspace, document.Project.Solution, document.Id, existingData.Items);
return;
return (existingData, existingData);
}
}

var tokens = _todoCommentTokens.GetTokens(document);
var tokens = todoCommentTokens.GetTokens(document);
var comments = await GetTodoCommentsAsync(document, tokens, cancellationToken).ConfigureAwait(false);
var items = await CreateItemsAsync(document, comments, cancellationToken).ConfigureAwait(false);

var data = new Data(textVersion, syntaxVersion, items);
await _state.PersistAsync(document, data, cancellationToken).ConfigureAwait(false);

// * NOTE * cancellation can't throw after this point.
if (existingData == null || existingData.Items.Length > 0 || data.Items.Length > 0)
{
Debug.Assert(_workspace == document.Project.Solution.Workspace);
RaiseTaskListUpdated(_workspace, document.Project.Solution, document.Id, data.Items);
}
await state.PersistAsync(document, data, cancellationToken).ConfigureAwait(false);
mavasani marked this conversation as resolved.
Show resolved Hide resolved
return (existingData, data);
}

private async Task<IList<TodoComment>> GetTodoCommentsAsync(Document document, IList<TodoCommentDescriptor> tokens, CancellationToken cancellationToken)
private static async Task<IList<TodoComment>> GetTodoCommentsAsync(Document document, IList<TodoCommentDescriptor> tokens, CancellationToken cancellationToken)
{
var service = document.GetLanguageService<ITodoCommentService>();
if (service == null)
Expand All @@ -99,7 +111,7 @@ private async Task<IList<TodoComment>> GetTodoCommentsAsync(Document document, I
return await service.GetTodoCommentsAsync(document, tokens, cancellationToken).ConfigureAwait(false);
}

private async Task<ImmutableArray<TodoItem>> CreateItemsAsync(Document document, IList<TodoComment> comments, CancellationToken cancellationToken)
private static async Task<ImmutableArray<TodoItem>> CreateItemsAsync(Document document, IList<TodoComment> comments, CancellationToken cancellationToken)
{
var items = ImmutableArray.CreateBuilder<TodoItem>();
if (comments != null)
Expand All @@ -116,7 +128,7 @@ private async Task<ImmutableArray<TodoItem>> CreateItemsAsync(Document document,
return items.ToImmutable();
}

private TodoItem CreateItem(Document document, SourceText text, SyntaxTree tree, TodoComment comment)
private static TodoItem CreateItem(Document document, SourceText text, SyntaxTree tree, TodoComment comment)
{
// make sure given position is within valid text range.
var textSpan = new TextSpan(Math.Min(text.Length, Math.Max(0, comment.Position)), 0);
Expand Down Expand Up @@ -148,13 +160,8 @@ public ImmutableArray<TodoItem> GetTodoItems(Workspace workspace, DocumentId id,
// TODO let's think about what to do here. for now, let call it synchronously. also, there is no actual async-ness for the
// TryGetExistingDataAsync, API just happen to be async since our persistent API is async API. but both caller and implementor are
// actually not async.
var existingData = _state.TryGetExistingDataAsync(document, cancellationToken).WaitAndGetResult_CanCallOnBackground(cancellationToken);
if (existingData == null)
{
return ImmutableArray<TodoItem>.Empty;
}

return existingData.Items;
var (_, newData) = ComputeAndPersistTodoCommentsAsync(document, _state, _todoCommentTokens, cancellationToken).WaitAndGetResult_CanCallOnBackground(cancellationToken);
return newData.Items;
}

public IEnumerable<UpdatedEventArgs> GetTodoItemsUpdatedEventArgs(Workspace workspace)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ internal static class InternalFeatureOnOffOptions
public static readonly Option<bool> DesignerAttributes = new Option<bool>(nameof(InternalFeatureOnOffOptions), nameof(DesignerAttributes), defaultValue: true,
storageLocations: new LocalUserProfileStorageLocation(LocalRegistryPath + "Designer Attribute"));

public static readonly Option<bool> FullSolutionAnalysisMemoryMonitor = new Option<bool>(nameof(InternalFeatureOnOffOptions), nameof(FullSolutionAnalysisMemoryMonitor), defaultValue: true,
public static readonly Option<bool> BackgroundAnalysisMemoryMonitor = new Option<bool>(nameof(InternalFeatureOnOffOptions), "FullSolutionAnalysisMemoryMonitor", defaultValue: true,
storageLocations: new LocalUserProfileStorageLocation(LocalRegistryPath + "Full Solution Analysis Memory Monitor"));

public static readonly Option<bool> ProjectReferenceConversion = new Option<bool>(nameof(InternalFeatureOnOffOptions), nameof(ProjectReferenceConversion), defaultValue: true,
Expand Down Expand Up @@ -97,7 +97,7 @@ public InternalFeatureOnOffOptionsProvider()
InternalFeatureOnOffOptions.Snippets,
InternalFeatureOnOffOptions.TodoComments,
InternalFeatureOnOffOptions.DesignerAttributes,
InternalFeatureOnOffOptions.FullSolutionAnalysisMemoryMonitor,
InternalFeatureOnOffOptions.BackgroundAnalysisMemoryMonitor,
InternalFeatureOnOffOptions.ProjectReferenceConversion);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public async Task TestHasSuccessfullyLoadedBeingFalse()
public async Task TestHasSuccessfullyLoadedBeingFalseFSAOn()
{
var workspace = new AdhocWorkspace();
workspace.Options = workspace.Options.WithChangedOption(ServiceFeatureOnOffOptions.ClosedFileDiagnostic, LanguageNames.CSharp, true);
workspace.Options = workspace.Options.WithChangedOption(SolutionCrawlerOptions.BackgroundAnalysisScopeOption, BackgroundAnalysisScope.FullSolution);
var document = GetDocumentFromIncompleteProject(workspace);

// open document
Expand Down Expand Up @@ -90,7 +90,7 @@ public async Task TestHasSuccessfullyLoadedBeingFalseWhenFileOpenedWithCompilerA
public async Task TestHasSuccessfullyLoadedBeingFalseWithCompilerAnalyzerFSAOn()
{
var workspace = new AdhocWorkspace();
workspace.Options = workspace.Options.WithChangedOption(ServiceFeatureOnOffOptions.ClosedFileDiagnostic, LanguageNames.CSharp, true);
workspace.Options = workspace.Options.WithChangedOption(SolutionCrawlerOptions.BackgroundAnalysisScopeOption, BackgroundAnalysisScope.FullSolution);
var document = GetDocumentFromIncompleteProject(workspace);

await TestAnalyzerAsync(workspace, document, new CSharpCompilerDiagnosticAnalyzer(), CompilerAnalyzerResultSetter, expectedSyntax: true, expectedSemantic: false);
Expand Down Expand Up @@ -305,7 +305,7 @@ public async Task TestHostAnalyzerErrorNotLeaking()
loader: TextLoader.From(TextAndVersion.Create(SourceText.From("class A {}"), VersionStamp.Create(), filePath: "test.cs")),
filePath: "test.cs")}));

workspace.Options = workspace.Options.WithChangedOption(ServiceFeatureOnOffOptions.ClosedFileDiagnostic, project.Language, true);
workspace.Options = workspace.Options.WithChangedOption(SolutionCrawlerOptions.BackgroundAnalysisScopeOption, BackgroundAnalysisScope.FullSolution);

// create listener/service/analyzer
var listener = new AsynchronousOperationListener();
Expand Down
2 changes: 1 addition & 1 deletion src/EditorFeatures/Test/Diagnostics/DiagnosticDataTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public async Task DiagnosticData_GetText6()
await VerifyTextSpanAsync(code, 1, 30, 2, 40, new TextSpan(code.Length, 0));
}

[Fact, Trait(Test.Utilities.Traits.Feature, Test.Utilities.Traits.Features.Diagnostics)]
[Fact, Trait(Traits.Feature, Traits.Features.Diagnostics)]
public async Task DiagnosticData_GetText7()
{
var code = @"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// 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.Immutable;
using System.Composition;
using Microsoft.CodeAnalysis.Host.Mef;

namespace Microsoft.CodeAnalysis.Editor.Test
{
[ExportWorkspaceService(typeof(IDocumentTrackingService)), Shared]
internal sealed class TestDocumentTrackingService : IDocumentTrackingService
{
private readonly object _gate = new object();
private event EventHandler<DocumentId> _activeDocumentChangedEventHandler;
private DocumentId _activeDocumentId;

[ImportingConstructor]
public TestDocumentTrackingService()
{
}

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

remove
{
lock (_gate)
{
_activeDocumentChangedEventHandler -= value;
}
}
}

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

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

public DocumentId TryGetActiveDocument()
{
return _activeDocumentId;
}

public ImmutableArray<DocumentId> GetVisibleDocuments()
{
return _activeDocumentId != null ? ImmutableArray.Create(_activeDocumentId) : ImmutableArray<DocumentId>.Empty;
}
}
}
Loading