Fix VariablesDeclaredWalker with recursive patterns

src/Compilers/CSharp/Portable/FlowAnalysis/VariablesDeclaredWalker.cs

@@ -2,14 +2,13 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-#nullable disable
-
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
 using Microsoft.CodeAnalysis.Collections;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Text;
 using Roslyn.Utilities;
 
@@ -45,12 +44,11 @@
         protected override void Free()
         {
             base.Free();
-            _variablesDeclared = null;
+            _variablesDeclared = null!;
         }
 
         public override void VisitPattern(BoundPattern pattern)
         {
-            base.VisitPattern(pattern);
             NoteDeclaredPatternVariables(pattern);
         }
 
@@ -69,26 +67,109 @@
         /// </summary>
         private void NoteDeclaredPatternVariables(BoundPattern pattern)
         {
-            if (IsInside)
+            visitPattern(pattern);
+
+            void visitPattern(BoundPattern pattern)
             {
                 switch (pattern)
                 {
-                    case BoundObjectPattern p:
+                    case BoundDeclarationPattern declarationPattern:
+                        noteOneVariable(declarationPattern.Variable);
+                        break;
+
+                    case BoundRecursivePattern recursivePattern:
+                        foreach (var subpattern in recursivePattern.Deconstruction.NullToEmpty())
+                            visitPattern(subpattern.Pattern);
+
+                        foreach (var subpattern in recursivePattern.Properties.NullToEmpty())
+                            visitPattern(subpattern.Pattern);
+
+                        noteOneVariable(recursivePattern.Variable);
+                        break;
+
+                    case BoundITuplePattern ituplePattern:
+                        foreach (var subpattern in ituplePattern.Subpatterns)
+                            visitPattern(subpattern.Pattern);
+
+                        break;
+
+                    case BoundListPattern listPattern:
+                        foreach (var elementPattern in listPattern.Subpatterns)
+                            visitPattern(elementPattern);
+
+                        noteOneVariable(listPattern.Variable);
+                        break;
+
+                    case BoundConstantPattern constantPattern:
+                        // It is possible for the region to be the expression within a pattern.
+                        Visit(constantPattern.Value);
+                        break;
+
+                    case BoundRelationalPattern relationalPattern:
+                        // It is possible for the region to be the expression within a pattern.
+                        Visit(relationalPattern.Value);
+                        break;
+
+                    case BoundNegatedPattern negatedPattern:
+                        visitPattern(negatedPattern.Negated);
+                        break;
+
+                    case BoundSlicePattern slicePattern:
+                        if (slicePattern.Pattern != null)
+                            visitPattern(slicePattern.Pattern);
+
+                        break;
+
+                    case BoundDiscardPattern or BoundTypePattern:
+                        // Does not contain variables or expressions. Nothing to visit.
+                        break;
+
+                    case BoundBinaryPattern:
                         {
-                            // The variable may be null if it is a discard designation `_`.
-                            if (p.Variable?.Kind == SymbolKind.Local)
+                            var binaryPattern = (BoundBinaryPattern)pattern;
+                            if (binaryPattern.Left is not BoundBinaryPattern)
                             {
-                                // Because this API only returns local symbols and parameters,
-                                // we exclude pattern variables that have become fields in scripts.
-                                _variablesDeclared.Add(p.Variable);
+                                visitPattern(binaryPattern.Left);
+                                visitPattern(binaryPattern.Right);
+                                break;
                             }
+
+                            // Users (such as ourselves) can have many, many nested binary patterns. To avoid crashing, do left recursion manually.
+                            var stack = ArrayBuilder<BoundBinaryPattern>.GetInstance();
+                            do
+                            {
+                                stack.Push(binaryPattern);
+                                binaryPattern = binaryPattern.Left as BoundBinaryPattern;
+                            } while (binaryPattern is not null);
+
+                            binaryPattern = stack.Pop();
+                            visitPattern(binaryPattern.Left);
+
+                            do
+                            {
+                                visitPattern(binaryPattern.Right);
+                            } while (stack.TryPop(out binaryPattern));
+
+                            stack.Free();
+                            break;
                         }
-                        break;
+                    default:
+                        throw ExceptionUtilities.UnexpectedValue(pattern.Kind);
+                }
+            }
+
+            void noteOneVariable(Symbol? symbol)
+            {
+                if (IsInside && symbol?.Kind == SymbolKind.Local)
+                {
+                    // Because this API only returns local symbols and parameters,
+                    // we exclude pattern variables that have become fields in scripts.
+                    _variablesDeclared.Add(symbol);
                 }
             }
         }
 
-        public override BoundNode VisitLocalDeclaration(BoundLocalDeclaration node)
+        public override BoundNode? VisitLocalDeclaration(BoundLocalDeclaration node)
         {
             if (IsInside)
             {
@@ -98,7 +179,7 @@
             return base.VisitLocalDeclaration(node);
         }
 
-        public override BoundNode VisitLambda(BoundLambda node)
+        public override BoundNode? VisitLambda(BoundLambda node)
         {
             if (IsInside && !node.WasCompilerGenerated)
             {
@@ -111,7 +192,7 @@
             return base.VisitLambda(node);
         }
 
-        public override BoundNode VisitLocalFunctionStatement(BoundLocalFunctionStatement node)
+        public override BoundNode? VisitLocalFunctionStatement(BoundLocalFunctionStatement node)
         {
             if (IsInside && !node.WasCompilerGenerated)
             {
@@ -156,12 +237,12 @@
 
             base.VisitCatchBlock(catchBlock, ref finallyState);
         }
 
-        public override BoundNode VisitQueryClause(BoundQueryClause node)
+        public override BoundNode? VisitQueryClause(BoundQueryClause node)
         {
             if (IsInside)
             {
-                if ((object)node.DefinedSymbol != null)
+                if ((object?)node.DefinedSymbol != null)
                 {
                     _variablesDeclared.Add(node.DefinedSymbol);
                 }
@@ -175,7 +256,7 @@
             VisitLocal(node);
         }
 
-        public override BoundNode VisitLocal(BoundLocal node)
+        public override BoundNode? VisitLocal(BoundLocal node)
         {
             if (IsInside && node.DeclarationKind != BoundLocalDeclarationKind.None)
             {
@@ -185,4 +266,4 @@
             return null;
         }
     }
-}
+}