Issues around analyzer exception diagnostics getting suppressed or not reported

[srivatsn]

Ported from TFS WorkItem: 1114327

---

Repro Steps:

From: Manish Vasani
Sent: Friday, January 23, 2015 3:34 PM
To: HeeJae Chang; John Hamby; Srivatsn Narayanan
Subject: RE: Code Review: Fix couple of issues in IDE diagnostic service for project diagnostics (mavasani)

 

Based on my exercise, I too feel that carrying these diagnostics along with regular diagnostics will only lead to more bugs. Probably we should have an event to report these diagnostics and a handler in IDE and command line compiler that report/clear out these diagnostics. I’ll open a bug to track this work as well.

I went down this path as I tried to write a test (for the original issue on this thread) with an analyzer that throws when called twice on same symbol/node for a compilation, and the exception or diagnostic never showed up J

 

From: HeeJae Chang
Sent: Friday, January 23, 2015 3:25 PM
To: Manish Vasani; John Hamby; Srivatsn Narayanan
Subject: RE: Code Review: Fix couple of issues in IDE diagnostic service for project diagnostics (mavasani)

 

Yesterday, I went through some of this code for Watson and wondered by exception diagnostic is handled same way as regular diagnostics from analyzer. Since in this way, unless analyzer consistently throws, the diagnostic will come and go as code is changed.

 

Shouldn’t those diagnostic reported differently?

 

-          heejae

 

From: Manish Vasani
Sent: Friday, January 23, 2015 3:12 PM
To: John Hamby; HeeJae Chang; Srivatsn Narayanan
Subject: RE: Code Review: Fix couple of issues in IDE diagnostic service for project diagnostics (mavasani)

 

So investigating this further has exposed a bunch of other issues:

 

1)      Swallowing analyzer exception diagnostics: There seem to a bunch of places where the IDE diagnostic service is swallowing diagnostics from analyzer exception (in fact any diagnostic reported without a location). The most glaring one is if we are running document analysis, the action might throw and we generate a no-location diagnostic. The DiagnosticAnalyzerDriver returns this diagnostic, but the DiagnosticIncrementalAnalyzer filters out all diagnostics without a span, hence even the exception diagnostic and it never shows up in error list.

Even ReanalyzeAllDocuments which does force analysis of all documents prior to running compilation end actions, throws away all diagnostics, including analyzer exception diagnostics.

I tried couple of approaches to fix this:

(a)    By adding a static conditional weak table from compilation to project diagnostics on DiagnosticAnalyzerDriver, such that any no-location diagnostics generated during document analysis are stored away on the map, and these get reported when GetProjectDiagnostics is invoked. This works fine, except for the case where document analysis for some document executes after project diagnostics have been fetched. This is very much possible if our solution crawler low pri queue picks up a project before all of its documents have been processed by high/medium pri processor.

(b)   Fix DiagnosticAnalyzerService handling of methods that fetch or compute document diagnostics to not filter out no-location diagnostics during document analysis itself. These seems weird from implementation perspective as we will pass around diagnostics without a span, when specifically asked for diagnostics within a document span. I am not sure what all will break with this approach, but I am reluctant to go this path.

I am now confused what is the best way to approach this problem.

 

2)      We suppress duplicate AnalyzerException diagnostics per compilation over here. See the first if statement. However, this seems wrong as this method might be invoked twice on the same diagnostic, and we will report it as not suppressed on first invocation, but suppressed on subsequent invocation (blame me, I wrote that!). I am going to remove all the code related to faultyAnalyzerMessages there and open a bug to ensure it gets fixed the right way.

 

[mavasani]

This should now be fixed with #673. However, we still don't report diagnostics from SupportedDiagnostics invoked within WorkspaceAnalyzerManager. We have capability of reporting workspace specific diagnostics now in Features layer. I'll address this in a subsequent change.

[mavasani]

SupportedDiagnostics invocation has now been fixed in the IDE to catch and report diagnostics for exceptions.
The only pending issue in this area is now captured by #887, I am closing this issue.