Merge pull request #767 from mavasani/SupportedDiagnosticsFix

My prior change #673 made to address issues with analyzer exception diagnostics introduced a few leaks in product and test code. The primary reason was that I attempted to use static events to track state as the existing AnalyzerDriverHelper type which did the core analyzer execution was a static type with all static methods. Additionally, I added a static event to the new host diagnostic update source added for reporting analyzer specific diagnostics. These were holding onto projects/workspaces in test runs causing leaks.

I have reverted the approach of static events and instead refactored the code in AnalyzerDriver project to simplify the whole design:
 1. Renamed AnalyzerDriverHelper to AnalyzerExecutor and made it a non-static type, which has instance fields for all the configuration parameters for analyzer callbacks.
 2. Move all the core analyzer callbacks (actions/Initialize method/supported diagnostics) into AnalyzerExecutor. Command line compiler just creates a single instance of the executor, while IDE driver creates instances per analyzer. 
 3. Both these changes simplified the API a lot, and I just had to add an additional field "Action addExceptionDiagnostic" to AnalyzerExecutor to configure how to handle exception diagnostics.
 4. MEF import AbstractHostDiagnosticUpdateSource in DiagnosticAnalyzerService and thread it down to IDE analyzer driver.
 5. Changes 3 and 4 above meant that in the IDE driver, delegate "addExceptionDiagnostic" just asks the HostDiagnosticUpdateSource to report the exception diagnostic produced by the AnalyzerExecutor.

This change also re-enables the tests skipped by #761 and fixes #759.

src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs

@@ -2924,9 +2924,9 @@ public override IEnumerable<ISymbol> GetSymbolsWithName(Func<string, bool> predi
 
         #endregion
 
-        internal override AnalyzerDriver AnalyzerForLanguage(ImmutableArray<DiagnosticAnalyzer> analyzers, AnalyzerOptions options, AnalyzerManager analyzerManager, Func<Exception, DiagnosticAnalyzer, bool> continueOnAnalyzerException, CancellationToken cancellationToken)
+        internal override AnalyzerDriver AnalyzerForLanguage(ImmutableArray<DiagnosticAnalyzer> analyzers, AnalyzerManager analyzerManager, CancellationToken cancellationToken)
         {
-            return new AnalyzerDriver<SyntaxKind>(analyzers, n => n.Kind(), options, analyzerManager, continueOnAnalyzerException, cancellationToken);
+            return new AnalyzerDriver<SyntaxKind>(analyzers, n => n.Kind(), analyzerManager, cancellationToken);
         }
 
         internal void SymbolDeclaredEvent(Symbol symbol)

src/Compilers/CSharp/Test/Semantic/Diagnostics/DiagnosticAnalyzerTests.AllInOne.cs

@@ -81,7 +81,7 @@ public class C
 
         #endregion
 
-        [Fact(Skip = "GitHub Issue 759")]
+        [Fact]
         [WorkItem(759)]
         public void AnalyzerDriverIsSafeAgainstAnalyzerExceptions()
         {

src/Compilers/Core/AnalyzerDriver/AnalyzerDriver.projitems

@@ -9,8 +9,7 @@
     <Import_RootNamespace>AnalyzerDriver</Import_RootNamespace>
   </PropertyGroup>
   <ItemGroup>
-    <Compile Include="$(MSBuildThisFileDirectory)AnalyzerDriverHelper.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)AnalyzerExceptionDiagnosticArgs.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)AnalyzerExecutor.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)AnalyzerManager.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)DeclarationComputer.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)DeclarationInfo.cs" />

src/Compilers/Core/AnalyzerDriver/AnalyzerManager.cs

@@ -12,7 +12,8 @@
 namespace Microsoft.CodeAnalysis.Diagnostics
 {
     /// <summary>
-    /// Manages properties of analyzers (such as registered actions, supported diagnostics) for analyzer host's lifetime.
+    /// Manages properties of analyzers (such as registered actions, supported diagnostics) for analyzer host's lifetime
+    /// and executes the callbacks into the analyzers.
     /// 
     /// It ensures the following for the lifetime of analyzer host:
     /// 1) <see cref="DiagnosticAnalyzer.Initialize(AnalysisContext)"/> is invoked only once per-analyzer.
@@ -21,11 +22,14 @@ namespace Microsoft.CodeAnalysis.Diagnostics
     /// </summary>
     internal class AnalyzerManager
     {
-        public static readonly AnalyzerManager Default = new AnalyzerManager();
+        /// <summary>
+        /// Gets the default instance of the AnalyzerManager for the lifetime of the analyzer host process.
+        /// </summary>
+        public static readonly AnalyzerManager Instance = new AnalyzerManager();
 
         // This map stores the tasks to compute HostSessionStartAnalysisScope for session wide analyzer actions, i.e. AnalyzerActions registered by analyzer's Initialize method.
         // These are run only once per every analyzer.
-        private ConditionalWeakTable<DiagnosticAnalyzer, Task<HostSessionStartAnalysisScope>> _sessionScopeMap =
+        private readonly ConditionalWeakTable<DiagnosticAnalyzer, Task<HostSessionStartAnalysisScope>> _sessionScopeMap =
             new ConditionalWeakTable<DiagnosticAnalyzer, Task<HostSessionStartAnalysisScope>>();
 
         // This map stores the tasks to compute HostCompilationStartAnalysisScope for per-compilation analyzer actions, i.e. AnalyzerActions registered by analyzer's CompilationStartActions.
@@ -40,71 +44,61 @@ internal class AnalyzerManager
         /// </summary>
         private readonly ConditionalWeakTable<DiagnosticAnalyzer, IReadOnlyList<DiagnosticDescriptor>> _descriptorCache =
             new ConditionalWeakTable<DiagnosticAnalyzer, IReadOnlyList<DiagnosticDescriptor>>();
-
+        
         private Task<HostCompilationStartAnalysisScope> GetCompilationAnalysisScopeCoreAsync(
             DiagnosticAnalyzer analyzer,
             HostSessionStartAnalysisScope sessionScope,
-            Compilation compilation,
-            AnalyzerOptions analyzerOptions,
-            Func<Exception, DiagnosticAnalyzer, bool> continueOnAnalyzerException,
-            CancellationToken cancellationToken)
+            AnalyzerExecutor analyzerExecutor)
         {
             Func<DiagnosticAnalyzer, Task<HostCompilationStartAnalysisScope>> getTask = a =>
             {
                 return Task.Run(() =>
                 {
                     var compilationAnalysisScope = new HostCompilationStartAnalysisScope(sessionScope);
-                    AnalyzerDriverHelper.ExecuteCompilationStartActions(sessionScope.CompilationStartActions, compilationAnalysisScope, compilation,
-                        analyzerOptions, continueOnAnalyzerException, cancellationToken);
+                    analyzerExecutor.ExecuteCompilationStartActions(sessionScope.CompilationStartActions, compilationAnalysisScope);
                     return compilationAnalysisScope;
-                }, cancellationToken);
+                }, analyzerExecutor.CancellationToken);
             };
 
-            var compilationActionsMap = _compilationScopeMap.GetOrCreateValue(compilation);
+            var compilationActionsMap = _compilationScopeMap.GetOrCreateValue(analyzerExecutor.Compilation);
             return compilationActionsMap.GetOrAdd(analyzer, getTask);
         }
 
         private async Task<HostCompilationStartAnalysisScope> GetCompilationAnalysisScopeAsync(
-            DiagnosticAnalyzer analyzer,
+            DiagnosticAnalyzer analyzer, 
             HostSessionStartAnalysisScope sessionScope,
-            Compilation compilation,
-            AnalyzerOptions analyzerOptions,
-            Func<Exception, DiagnosticAnalyzer, bool> continueOnAnalyzerException,
-            CancellationToken cancellationToken)
+            AnalyzerExecutor analyzerExecutor)
         {
             try
             {
-                return await GetCompilationAnalysisScopeCoreAsync(analyzer, sessionScope,
-                    compilation, analyzerOptions, continueOnAnalyzerException, cancellationToken).ConfigureAwait(false);
+                return await GetCompilationAnalysisScopeCoreAsync(analyzer, sessionScope, analyzerExecutor).ConfigureAwait(false);
             }
             catch (OperationCanceledException)
             {
                 // Task to compute the scope was cancelled.
                 // Clear the entry in scope map for analyzer, so we can attempt a retry.
-                var compilationActionsMap = _compilationScopeMap.GetOrCreateValue(compilation);
+                var compilationActionsMap = _compilationScopeMap.GetOrCreateValue(analyzerExecutor.Compilation);
                 Task<HostCompilationStartAnalysisScope> cancelledTask;
                 compilationActionsMap.TryRemove(analyzer, out cancelledTask);
 
-                cancellationToken.ThrowIfCancellationRequested();
-                return await GetCompilationAnalysisScopeAsync(analyzer, sessionScope,
-                    compilation, analyzerOptions, continueOnAnalyzerException, cancellationToken).ConfigureAwait(false);
+                analyzerExecutor.CancellationToken.ThrowIfCancellationRequested();
+                return await GetCompilationAnalysisScopeAsync(analyzer, sessionScope, analyzerExecutor).ConfigureAwait(false);
 
             }
         }
 
         private Task<HostSessionStartAnalysisScope> GetSessionAnalysisScopeCoreAsync(
-            DiagnosticAnalyzer analyzer,
-            Func<Exception, DiagnosticAnalyzer, bool> continueOnAnalyzerException,
-            CancellationToken cancellationToken)
+            DiagnosticAnalyzer analyzer, 
+            AnalyzerExecutor analyzerExecutor)
         {
             Func<DiagnosticAnalyzer, Task<HostSessionStartAnalysisScope>> getTask = a =>
             {
                 return Task.Run(() =>
                 {
                     var sessionScope = new HostSessionStartAnalysisScope();
-                    AnalyzerDriverHelper.ExecuteInitializeMethod(a, sessionScope, continueOnAnalyzerException, cancellationToken);
+                    analyzerExecutor.ExecuteInitializeMethod(a, sessionScope);
                     return sessionScope;
-                }, cancellationToken);
+                }, analyzerExecutor.CancellationToken);
             };
 
             var callback = new ConditionalWeakTable<DiagnosticAnalyzer, Task<HostSessionStartAnalysisScope>>.CreateValueCallback(getTask);
@@ -113,21 +107,20 @@ internal class AnalyzerManager
 
         private async Task<HostSessionStartAnalysisScope> GetSessionAnalysisScopeAsync(
             DiagnosticAnalyzer analyzer,
-            Func<Exception, DiagnosticAnalyzer, bool> continueOnAnalyzerException,
-            CancellationToken cancellationToken)
+            AnalyzerExecutor analyzerExecutor)
         {
             try
             {
-                return await GetSessionAnalysisScopeCoreAsync(analyzer, continueOnAnalyzerException, cancellationToken).ConfigureAwait(false);
+                return await GetSessionAnalysisScopeCoreAsync(analyzer, analyzerExecutor).ConfigureAwait(false);
             }
             catch (OperationCanceledException)
             {
                 // Task to compute the scope was cancelled.
                 // Clear the entry in scope map for analyzer, so we can attempt a retry.
                 _sessionScopeMap.Remove(analyzer);
 
-                cancellationToken.ThrowIfCancellationRequested();
-                return await GetSessionAnalysisScopeAsync(analyzer, continueOnAnalyzerException, cancellationToken).ConfigureAwait(false);
+                analyzerExecutor.CancellationToken.ThrowIfCancellationRequested();
+                return await GetSessionAnalysisScopeAsync(analyzer, analyzerExecutor).ConfigureAwait(false);
             }
         }
 
@@ -136,18 +129,12 @@ internal class AnalyzerManager
         /// The returned actions include the actions registered during <see cref="DiagnosticAnalyzer.Initialize(AnalysisContext)"/> method as well as
         /// the actions registered during <see cref="CompilationStartAnalyzerAction"/> for the given compilation.
         /// </summary>
-        public async Task<AnalyzerActions> GetAnalyzerActionsAsync(
-            DiagnosticAnalyzer analyzer,
-            Compilation compilation,
-            AnalyzerOptions analyzerOptions,
-            Func<Exception, DiagnosticAnalyzer, bool> continueOnAnalyzerException,
-            CancellationToken cancellationToken)
+        public async Task<AnalyzerActions> GetAnalyzerActionsAsync(DiagnosticAnalyzer analyzer, AnalyzerExecutor analyzerExecutor)
         {
-            var sessionScope = await GetSessionAnalysisScopeAsync(analyzer, continueOnAnalyzerException, cancellationToken).ConfigureAwait(false);
-            if (sessionScope.CompilationStartActions.Length > 0 && compilation != null)
+            var sessionScope = await GetSessionAnalysisScopeAsync(analyzer, analyzerExecutor).ConfigureAwait(false);
+            if (sessionScope.CompilationStartActions.Length > 0 && analyzerExecutor.Compilation != null)
             {
-                var compilationScope = await GetCompilationAnalysisScopeAsync(analyzer, sessionScope,
-                    compilation, analyzerOptions, continueOnAnalyzerException, cancellationToken).ConfigureAwait(false);
+                var compilationScope = await GetCompilationAnalysisScopeAsync(analyzer, sessionScope, analyzerExecutor).ConfigureAwait(false);
                 return compilationScope.GetAnalyzerActions(analyzer);
             }
 
@@ -159,20 +146,98 @@ internal class AnalyzerManager
         /// </summary>
         public ImmutableArray<DiagnosticDescriptor> GetSupportedDiagnosticDescriptors(
             DiagnosticAnalyzer analyzer,
-            Func<Exception, DiagnosticAnalyzer, bool> continueOnAnalyzerException,
-            CancellationToken cancellationToken)
+            AnalyzerExecutor analyzerExecutor)
         {
             var descriptors = _descriptorCache.GetValue(analyzer, key =>
             {
                 var supportedDiagnostics = ImmutableArray<DiagnosticDescriptor>.Empty;
 
                 // Catch Exception from analyzer.SupportedDiagnostics
-                AnalyzerDriverHelper.ExecuteAndCatchIfThrows(analyzer, continueOnAnalyzerException, () => { supportedDiagnostics = analyzer.SupportedDiagnostics; }, cancellationToken);
+                analyzerExecutor.ExecuteAndCatchIfThrows(analyzer, () => { supportedDiagnostics = analyzer.SupportedDiagnostics; });
 
                 return supportedDiagnostics;
             });
 
             return (ImmutableArray<DiagnosticDescriptor>)descriptors;
         }
+
+        /// <summary>
+        /// Returns true if all the diagnostics that can be produced by this analyzer are suppressed through options.
+        /// </summary>
+        internal bool IsDiagnosticAnalyzerSuppressed(
+            DiagnosticAnalyzer analyzer,
+            CompilationOptions options,
+            Func<DiagnosticAnalyzer, bool> isCompilerAnalyzer,
+            AnalyzerExecutor analyzerExecutor)
+        {
+            if (isCompilerAnalyzer(analyzer))
+            {
+                // Compiler analyzer must always be executed for compiler errors, which cannot be suppressed or filtered.
+                return false;
+            }
+
+            var supportedDiagnostics = GetSupportedDiagnosticDescriptors(analyzer, analyzerExecutor);
+            var diagnosticOptions = options.SpecificDiagnosticOptions;
+
+            foreach (var diag in supportedDiagnostics)
+            {
+                if (HasNotConfigurableTag(diag.CustomTags))
+                {
+                    // If diagnostic descriptor is not configurable, then diagnostics created through it cannot be suppressed.
+                    return false;
+                }
+
+                // Is this diagnostic suppressed by default (as written by the rule author)
+                var isSuppressed = !diag.IsEnabledByDefault;
+
+                // If the user said something about it, that overrides the author.
+                if (diagnosticOptions.ContainsKey(diag.Id))
+                {
+                    isSuppressed = diagnosticOptions[diag.Id] == ReportDiagnostic.Suppress;
+                }
+
+                if (isSuppressed)
+                {
+                    continue;
+                }
+                else
+                {
+                    return false;
+                }
+            }
+
+            return true;
+        }
+
+        internal static bool HasNotConfigurableTag(IEnumerable<string> customTags)
+        {
+            foreach (var customTag in customTags)
+            {
+                if (customTag == WellKnownDiagnosticTags.NotConfigurable)
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        internal static bool IsAnalyzerExceptionDiagnostic(Diagnostic diagnostic)
+        {
+            if (diagnostic.Id == AnalyzerExecutor.DiagnosticId)
+            {
+#pragma warning disable RS0013 // Its ok to realize the Descriptor for analyzer exception diagnostics, which are descriptor based and also rare.
+                foreach (var tag in diagnostic.Descriptor.CustomTags)
+#pragma warning restore RS0013
+                {
+                    if (tag == WellKnownDiagnosticTags.AnalyzerException)
+                    {
+                        return true;
+                    }
+                }
+            }
+
+            return false;
+        }
     }
 }

src/Compilers/Core/Desktop/CommandLine/CommonCompiler.cs

@@ -333,15 +333,15 @@ private int RunCore(TextWriter consoleOutput, CancellationToken cancellationToke
             var analyzerOptions = new AnalyzerOptions(ImmutableArray.Create<AdditionalText, AdditionalTextFile>(additionalTextFiles));
 
             AnalyzerDriver analyzerDriver = null;
+            AnalyzerManager analyzerManager = null;
             ConcurrentSet<Diagnostic> analyzerExceptionDiagnostics = null;
-            EventHandler<AnalyzerExceptionDiagnosticArgs> analyzerExceptionDiagnosticsHandler = null;
             if (!analyzers.IsDefaultOrEmpty)
             {
+                analyzerManager = new AnalyzerManager();
                 analyzerExceptionDiagnostics = new ConcurrentSet<Diagnostic>();
-                analyzerExceptionDiagnosticsHandler = AnalyzerDriverHelper.RegisterAnalyzerExceptionDiagnosticHandler(analyzers, analyzerExceptionDiagnostics.Add);
-
-                var analyzerManager = new AnalyzerManager();
-                analyzerDriver = AnalyzerDriver.Create(compilation, analyzers, analyzerOptions, analyzerManager, out compilation, cancellationToken);
+                Action<Diagnostic> addExceptionDiagnostic = diagnostic => analyzerExceptionDiagnostics.Add(diagnostic);
+
+                analyzerDriver = AnalyzerDriver.Create(compilation, analyzers, analyzerOptions, analyzerManager, addExceptionDiagnostic, out compilation, cancellationToken);
             }
 
             // Print the diagnostics produced during the parsing stage and exit if there were any errors.
@@ -450,8 +450,7 @@ private int RunCore(TextWriter consoleOutput, CancellationToken cancellationToke
                 {
                     var analyzerDiagnostics = analyzerDriver.GetDiagnosticsAsync().Result;
                     var allAnalyzerDiagnostics = analyzerDiagnostics.AddRange(analyzerExceptionDiagnostics);
-                    AnalyzerDriverHelper.UnregisterAnalyzerExceptionDiagnosticHandler(analyzerExceptionDiagnosticsHandler);
-
+
                     if (PrintErrors(allAnalyzerDiagnostics, consoleOutput))
                     {
                         return Failed;

src/Compilers/Core/Portable/Compilation/Compilation.cs

@@ -81,7 +81,7 @@ public abstract partial class Compilation
             }
         }
 
-        internal abstract AnalyzerDriver AnalyzerForLanguage(ImmutableArray<DiagnosticAnalyzer> analyzers, AnalyzerOptions options, AnalyzerManager analyzerManager, Func<Exception, DiagnosticAnalyzer, bool> continueOnAnalyzerException, CancellationToken cancellationToken);
+        internal abstract AnalyzerDriver AnalyzerForLanguage(ImmutableArray<DiagnosticAnalyzer> analyzers, AnalyzerManager analyzerManager, CancellationToken cancellationToken);
 
         /// <summary>
         /// Gets the source language ("C#" or "Visual Basic").

src/Compilers/Core/Portable/Diagnostic/Diagnostic.cs

@@ -7,6 +7,7 @@
 using Microsoft.CodeAnalysis.Text;
 using Roslyn.Utilities;
 using System.Collections.Immutable;
+using Microsoft.CodeAnalysis.Diagnostics;
 
 namespace Microsoft.CodeAnalysis
 {
@@ -407,7 +408,7 @@ internal static int GetDefaultWarningLevel(DiagnosticSeverity severity)
         /// </summary>
         internal virtual bool IsNotConfigurable()
         {
-            return DiagnosticDescriptor.IsNotConfigurable(this.CustomTags);
+            return AnalyzerManager.HasNotConfigurableTag(this.CustomTags);
         }
     }
 }