Skip to content

Commit

Permalink
fix open file only diagnostics behavior
Browse files Browse the repository at this point in the history
diagnostics reported by open file only diagnostic analyzers such as simplify type names
didn't get cleaned up properly when a file is closed.

this makes sure those diagnostics only exist while the file they originated from is open.
  • Loading branch information
heejaechang committed Dec 9, 2016
1 parent 2133504 commit 1140434
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
23 changes: 21 additions & 2 deletions src/Features/Core/Portable/Diagnostics/AnalyzerHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,31 @@ public static bool IsBuiltInAnalyzer(this DiagnosticAnalyzer analyzer)
return analyzer is IBuiltInAnalyzer || analyzer.IsWorkspaceDiagnosticAnalyzer() || analyzer.IsCompilerAnalyzer();
}

public static bool ShouldRunForFullProject(this DiagnosticAnalyzerService service, DiagnosticAnalyzer analyzer, Project project)
public static bool IsOpenFileOnly(this DiagnosticAnalyzer analyzer, Workspace workspace)
{
var builtInAnalyzer = analyzer as IBuiltInAnalyzer;
if (builtInAnalyzer != null)
{
return !builtInAnalyzer.OpenFileOnly(project.Solution.Workspace);
return builtInAnalyzer.OpenFileOnly(workspace);
}

return false;
}

public static bool ShouldRunForFullProject(this DiagnosticAnalyzerService service, DiagnosticAnalyzer analyzer, Project project)
{
if (analyzer.IsBuiltInAnalyzer())
{
// always return true for builtin analyzer. we can't use
// descriptor check since many builtin analyzer always return
// hidden descriptor regardless what descriptor it actually
// return on runtime. they do this so that they can control
// severity through option page rather than rule set editor.
// this is special behavior only ide analyzer can do. we hope
// once we support editorconfig fully, third party can use this
// ability as well and we can remove this kind special treatment on builtin
// analyzer.
return true;
}

if (analyzer.IsWorkspaceDiagnosticAnalyzer())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,11 @@ public async Task MergeAsync(ActiveFileState state, Document document)
// keep from build flag if full analysis is off
var fromBuild = fullAnalysis ? false : lastResult.FromBuild;

// if it is allowed to keep project state, check versions and if they are same, bail out
var openFileOnlyAnalyzer = _owner.Analyzer.IsOpenFileOnly(document.Project.Solution.Workspace);

// if it is allowed to keep project state, check versions and if they are same, bail out.
// if full solution analysis is off or we are asked to reset document state, we always merge.
if (fullAnalysis &&
if (fullAnalysis && !openFileOnlyAnalyzer &&
syntax.Version != VersionStamp.Default &&
syntax.Version == semantic.Version &&
syntax.Version == lastResult.Version)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,13 @@ public override async Task AnalyzeProjectAsync(Project project, bool semanticsCh
{
var stateSets = GetStateSetsForFullSolutionAnalysis(_stateManager.GetOrUpdateStateSets(project), project).ToList();

// PERF: get analyzers that are not suppressed.
// PERF: get analyzers that are not suppressed and marked as open file only
// this is perf optimization. we cache these result since we know the result. (no diagnostics)
// REVIEW: IsAnalyzerSuppressed call seems can be quite expensive in certain condition. is there any other way to do this?
var activeAnalyzers = stateSets
.Select(s => s.Analyzer)
.Where(a => !Owner.IsAnalyzerSuppressed(a, project));
.Where(a => !Owner.IsAnalyzerSuppressed(a, project) &&
!a.IsOpenFileOnly(project.Solution.Workspace));

// get driver only with active analyzers.
var includeSuppressedDiagnostics = true;
Expand All @@ -92,6 +93,8 @@ public override async Task AnalyzeProjectAsync(Project project, bool semanticsCh
}

// no cancellation after this point.
// any analyzer that doesn't have result will be treated as returned empty set
// which means we will remove those from error list
foreach (var stateSet in stateSets)
{
var state = stateSet.GetProjectState(project.Id);
Expand Down

0 comments on commit 1140434

Please sign in to comment.