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,13 +44,13 @@ internal VariablesDeclaredWalker(CSharpCompilation compilation, Symbol member, B
         protected override void Free()
         {
             base.Free();
-            _variablesDeclared = null;
+            _variablesDeclared = null!;
         }
 
         public override void VisitPattern(BoundPattern pattern)
         {
-            base.VisitPattern(pattern);
             NoteDeclaredPatternVariables(pattern);
+            base.VisitPattern(pattern);
         }
 
         protected override void VisitSwitchSection(BoundSwitchSection node, bool isLastSection)
@@ -69,26 +68,104 @@ protected override void VisitSwitchSection(BoundSwitchSection node, bool isLastS
         /// </summary>
         private void NoteDeclaredPatternVariables(BoundPattern pattern)
         {
-            if (IsInside)
+            switch (pattern)
             {
-                switch (pattern)
-                {
-                    case BoundObjectPattern p:
+                case BoundDeclarationPattern declarationPattern:
+                    noteOneVariable(declarationPattern.Variable);
+                    break;
+
+                case BoundRecursivePattern recursivePattern:
+                    foreach (var subpattern in recursivePattern.Deconstruction.NullToEmpty())
+                        NoteDeclaredPatternVariables(subpattern.Pattern);
+
+                    foreach (var subpattern in recursivePattern.Properties.NullToEmpty())
+                        NoteDeclaredPatternVariables(subpattern.Pattern);
+
+                    noteOneVariable(recursivePattern.Variable);
+                    break;
+
+                case BoundITuplePattern ituplePattern:
+                    foreach (var subpattern in ituplePattern.Subpatterns)
+                        NoteDeclaredPatternVariables(subpattern.Pattern);
+
+                    break;
+
+                case BoundListPattern listPattern:
+                    foreach (var elementPattern in listPattern.Subpatterns)
+                        NoteDeclaredPatternVariables(elementPattern);
+
+                    noteOneVariable(listPattern.Variable);
+                    break;
+
+                case BoundConstantPattern constantPattern:
+                    // It is possible for the region to be the expression within a pattern.
+                    VisitRvalue(constantPattern.Value);
+                    break;
+
+                case BoundRelationalPattern relationalPattern:
+                    // It is possible for the region to be the expression within a pattern.
+                    VisitRvalue(relationalPattern.Value);
+                    break;
+
+                case BoundNegatedPattern negatedPattern:
+                    NoteDeclaredPatternVariables(negatedPattern.Negated);
+                    break;
+
+                case BoundSlicePattern slicePattern:
+                    if (slicePattern.Pattern != null)
+                        NoteDeclaredPatternVariables(slicePattern.Pattern);
+
+                    break;
+
+                case BoundDiscardPattern or BoundTypePattern:
+                    // Does not contain variables or expressions. Nothing to visit.
+                    break;
+
+                case BoundBinaryPattern:
+                    {
+                        var binaryPattern = (BoundBinaryPattern)pattern;
+                        if (binaryPattern.Left is not BoundBinaryPattern)
                         {
-                            // The variable may be null if it is a discard designation `_`.
-                            if (p.Variable?.Kind == SymbolKind.Local)
-                            {
-                                // Because this API only returns local symbols and parameters,
-                                // we exclude pattern variables that have become fields in scripts.
-                                _variablesDeclared.Add(p.Variable);
-                            }
+                            NoteDeclaredPatternVariables(binaryPattern.Left);
+                            NoteDeclaredPatternVariables(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();
+                        NoteDeclaredPatternVariables(binaryPattern.Left);
+
+                        do
+                        {
+                            NoteDeclaredPatternVariables(binaryPattern.Right);
+                        } while (stack.TryPop(out binaryPattern));
+
+                        stack.Free();
                         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 +175,7 @@ public override BoundNode VisitLocalDeclaration(BoundLocalDeclaration node)
             return base.VisitLocalDeclaration(node);
         }
 
-        public override BoundNode VisitLambda(BoundLambda node)
+        public override BoundNode? VisitLambda(BoundLambda node)
         {
             if (IsInside && !node.WasCompilerGenerated)
             {
@@ -111,7 +188,7 @@ public override BoundNode VisitLambda(BoundLambda node)
             return base.VisitLambda(node);
         }
 
-        public override BoundNode VisitLocalFunctionStatement(BoundLocalFunctionStatement node)
+        public override BoundNode? VisitLocalFunctionStatement(BoundLocalFunctionStatement node)
         {
             if (IsInside && !node.WasCompilerGenerated)
             {
@@ -142,7 +219,7 @@ public override void VisitForEachIterationVariables(BoundForEachStatement node)
             }
         }
 
-        public override BoundNode VisitCatchBlock(BoundCatchBlock catchBlock)
+        public override BoundNode? VisitCatchBlock(BoundCatchBlock catchBlock)
         {
             if (IsInside)
             {
@@ -159,11 +236,11 @@ public override BoundNode VisitCatchBlock(BoundCatchBlock catchBlock)
             return null;
         }
 
-        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);
                 }
@@ -177,7 +254,7 @@ protected override void VisitLvalue(BoundLocal node)
             VisitLocal(node);
         }
 
-        public override BoundNode VisitLocal(BoundLocal node)
+        public override BoundNode? VisitLocal(BoundLocal node)
         {
             if (IsInside && node.DeclarationKind != BoundLocalDeclarationKind.None)
             {
@@ -187,4 +264,4 @@ public override BoundNode VisitLocal(BoundLocal node)
             return null;
         }
     }
-}
+}