From 3c11ec059d3f4d092081f454e0ba027bf7c3dfd0 Mon Sep 17 00:00:00 2001
From: Manish Vasani <mavasani@microsoft.com>
Date: Mon, 6 Aug 2018 15:28:11 -0700
Subject: [PATCH] Avoid stack overflow in
 AvoidUninstantiatedInternalClassesAnalyzer (CA1812) by keeping track of
 processed types.

Fixes #1739
---
 .../AvoidUninstantiatedInternalClasses.cs     | 22 +++++++++++--------
 ...AvoidUninstantiatedInternalClassesTests.cs | 20 +++++++++++++++++
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/Maintainability/AvoidUninstantiatedInternalClasses.cs b/src/Microsoft.CodeQuality.Analyzers/Core/Maintainability/AvoidUninstantiatedInternalClasses.cs
index caf7439556..2decb51524 100644
--- a/src/Microsoft.CodeQuality.Analyzers/Core/Maintainability/AvoidUninstantiatedInternalClasses.cs
+++ b/src/Microsoft.CodeQuality.Analyzers/Core/Maintainability/AvoidUninstantiatedInternalClasses.cs
@@ -45,8 +45,8 @@ public override void Initialize(AnalysisContext analysisContext)
 
             analysisContext.RegisterCompilationStartAction(startContext =>
             {
-                var instantiatedTypes = new ConcurrentBag<INamedTypeSymbol>();
-                var internalTypes = new ConcurrentBag<INamedTypeSymbol>();
+                var instantiatedTypes = new ConcurrentDictionary<INamedTypeSymbol, object>();
+                var internalTypes = new ConcurrentDictionary<INamedTypeSymbol, object>();
 
                 var compilation = startContext.Compilation;
 
@@ -77,7 +77,7 @@ public override void Initialize(AnalysisContext analysisContext)
                     var expr = (IObjectCreationOperation)context.Operation;
                     if (expr.Type is INamedTypeSymbol namedType)
                     {
-                        instantiatedTypes.Add(namedType);
+                        instantiatedTypes.TryAdd(namedType, null);
                     }
                 }, OperationKind.ObjectCreation);
 
@@ -94,13 +94,13 @@ public override void Initialize(AnalysisContext analysisContext)
                             mef1ExportAttributeSymbol,
                             mef2ExportAttributeSymbol))
                     {
-                        internalTypes.Add(type);
+                        internalTypes.TryAdd(type, null);
                     }
 
                     // Instantiation from the subtype constructor initializer.
                     if (type.BaseType != null)
                     {
-                        instantiatedTypes.Add(type.BaseType);
+                        instantiatedTypes.TryAdd(type.BaseType, null);
                     }
                 }, SymbolKind.NamedType);
 
@@ -115,7 +115,11 @@ void ProcessGenericTypes(IEnumerable<(ITypeParameterSymbol param, ITypeSymbol ar
                         {
                             void ProcessNamedTypeParamConstraint(INamedTypeSymbol namedTypeArg)
                             {
-                                instantiatedTypes.Add(namedTypeArg);
+                                if (!instantiatedTypes.TryAdd(namedTypeArg, null))
+                                {
+                                    // Already processed.
+                                    return;
+                                }
 
                                 // We need to handle if this type param also has type params that have a generic constraint. Take the following example:
                                 // new Factory1<Factory2<InstantiatedType>>();
@@ -185,9 +189,9 @@ IEnumerable<INamedTypeSymbol> GetAllNamedTypeConstraints(ITypeParameterSymbol t)
                 startContext.RegisterCompilationEndAction(context =>
                 {
                     var uninstantiatedInternalTypes = internalTypes
-                        .Select(it => it.OriginalDefinition)
-                        .Except(instantiatedTypes.Select(it => it.OriginalDefinition))
-                        .Where(type => !HasInstantiatedNestedType(type, instantiatedTypes));
+                        .Select(it => it.Key.OriginalDefinition)
+                        .Except(instantiatedTypes.Select(it => it.Key.OriginalDefinition))
+                        .Where(type => !HasInstantiatedNestedType(type, instantiatedTypes.Keys));
 
                     foreach (var type in uninstantiatedInternalTypes)
                     {
diff --git a/src/Microsoft.CodeQuality.Analyzers/UnitTests/Maintainability/AvoidUninstantiatedInternalClassesTests.cs b/src/Microsoft.CodeQuality.Analyzers/UnitTests/Maintainability/AvoidUninstantiatedInternalClassesTests.cs
index a378b68f9b..04aba18609 100644
--- a/src/Microsoft.CodeQuality.Analyzers/UnitTests/Maintainability/AvoidUninstantiatedInternalClassesTests.cs
+++ b/src/Microsoft.CodeQuality.Analyzers/UnitTests/Maintainability/AvoidUninstantiatedInternalClassesTests.cs
@@ -1211,6 +1211,26 @@ End Sub
 End Module");
         }
 
+        [Fact, WorkItem(1739, "https://github.com/dotnet/roslyn-analyzers/issues/1739")]
+        public void CA1812_CSharp_NoDiagnostic_GenericTypeWithRecursiveConstraint()
+        {
+            VerifyCSharp(@"
+public abstract class JobStateBase<TState>
+    where TState : JobStateBase<TState>, new()
+{
+    public void SomeFunction ()
+    {
+        new JobStateChangeHandler<TState>();
+    }
+}
+
+public class JobStateChangeHandler<TState>
+    where TState : JobStateBase<TState>, new()
+{
+}
+");
+        }
+
         protected override DiagnosticAnalyzer GetBasicDiagnosticAnalyzer()
         {
             return new AvoidUninstantiatedInternalClassesAnalyzer();