Merge pull request #1761 from mavasani/Issue1739

Avoid stack overflow in AvoidUninstantiatedInternalClassesAnalyzer (C…
dotnet · Aug 7, 2018 · ed182a5 · ed182a5
2 parents 0ac5b43 + 3c11ec0
commit ed182a5
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 9 deletions.
diff --git a/...icrosoft.CodeQuality.Analyzers/Core/Maintainability/AvoidUninstantiatedInternalClasses.cs b/...icrosoft.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/...odeQuality.Analyzers/UnitTests/Maintainability/AvoidUninstantiatedInternalClassesTests.cs b/...odeQuality.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();