changed GetOperation to always return one from root operation tree and never...

changed GetOperation to always return one from root operation tree and never use recovery mode of semantic model.

src/Compilers/CSharp/Portable/BoundTree/Expression.cs

@@ -128,4 +128,14 @@ partial class BoundThrowExpression
     {
         protected override ImmutableArray<BoundNode> Children => ImmutableArray.Create<BoundNode>(this.Expression);
     }
+
+    internal partial class BoundMethodGroup
+    {
+        protected override ImmutableArray<BoundNode> Children => ImmutableArray.Create<BoundNode>(this.ReceiverOpt);
+    }
+
+    internal partial class BoundPropertyGroup
+    {
+        protected override ImmutableArray<BoundNode> Children => ImmutableArray.Create<BoundNode>(this.ReceiverOpt);
+    }
 }

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

@@ -961,6 +961,18 @@ public override SyntaxTree SyntaxTree
 
         internal override IOperation GetOperationWorker(CSharpSyntaxNode node, GetOperationOptions options, CancellationToken cancellationToken)
         {
+            var bindingRoot = GetBindingRoot(node);
+
+            var statementOrRootOperation = GetStatementOrRootOperation(bindingRoot, options, cancellationToken);
+
+            // we might optimize it later
+            return statementOrRootOperation.DescendantsAndSelf().FirstOrDefault(o => o.Syntax == node);
+        }
+
+        private IOperation GetStatementOrRootOperation(CSharpSyntaxNode node, GetOperationOptions options, CancellationToken cancellationToken)
+        {
+            Debug.Assert(node == GetBindingRoot(node));
+
             CSharpSyntaxNode bindableNode;
 
             BoundNode lowestBoundNode;
@@ -1293,17 +1305,24 @@ private CSharpSyntaxNode GetBindingRoot(CSharpSyntaxNode node)
 
             StatementSyntax enclosingStatement = null;
 
+#if DEBUG
             for (CSharpSyntaxNode current = node; current != this.Root; current = current.ParentOrStructuredTriviaParent)
             {
+                // make sure we never go out of Root
                 Debug.Assert(current != null, "How did we get outside the root?");
+            }
+#endif
 
-                if (enclosingStatement == null)
+            for (CSharpSyntaxNode current = node; current != this.Root; current = current.ParentOrStructuredTriviaParent)
+            {
+                enclosingStatement = current as StatementSyntax;
+                if (enclosingStatement != null)
                 {
-                    enclosingStatement = current as StatementSyntax;
+                    return enclosingStatement;
                 }
             }
 
-            return enclosingStatement ?? this.Root;
+            return this.Root;
         }
 
         // We want the binder in which this syntax node is going to be bound, NOT the binder which
@@ -1551,7 +1570,7 @@ private static Binder GetQueryEnclosingBinder(int position, CSharpSyntaxNode sta
             }
             while (node != null);
 
-done:
+            done:
             return GetEnclosingBinderInternalWithinRoot(AdjustStartingNodeAccordingToNewRoot(startingNode, queryClause.Syntax),
                                       position, queryClause.Binder, queryClause.Syntax);
         }

src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs

@@ -7,7 +7,6 @@
 using System.Linq;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
-using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Roslyn.Utilities;
 
@@ -18,9 +17,6 @@ internal sealed partial class CSharpOperationFactory
         private readonly ConcurrentDictionary<BoundNode, IOperation> _cache =
             new ConcurrentDictionary<BoundNode, IOperation>(concurrencyLevel: 2, capacity: 10);
 
-        private readonly ConcurrentDictionary<BoundLocalDeclaration, IVariableDeclaration> _variableDeclarationCache =
-            new ConcurrentDictionary<BoundLocalDeclaration, IVariableDeclaration>(concurrencyLevel: 2, capacity: 10);
-
         private readonly SemanticModel _semanticModel;
 
         public CSharpOperationFactory(SemanticModel semanticModel)
@@ -35,40 +31,16 @@ public IOperation Create(BoundNode boundNode)
                 return null;
             }
 
-            // this should be removed once this issue is fixed
-            // https://github.com/dotnet/roslyn/issues/21187
-            if (IsIgnoredNode(boundNode))
+            // implicit receiver can be shared between multiple bound nodes.
+            // always return cloned one
+            if (boundNode.Kind == BoundKind.ImplicitReceiver)
             {
-                // due to how IOperation is set up, some of C# BoundNode must be ignored
-                // while generating IOperation. otherwise, 2 different IOperation trees will be created
-                // for nodes under same sub tree
-                return null;
+                return _semanticModel.CloneOperation(CreateInternal(boundNode));
             }
 
             return _cache.GetOrAdd(boundNode, n => CreateInternal(n));
         }
 
-        private static bool IsIgnoredNode(BoundNode boundNode)
-        {
-            // since boundNode doesn't have parent pointer, it can't just look around using bound node
-            // it needs to use syntax node. we ignore these because this will return its own operation tree
-            // that don't belong to its parent operation tree.
-            switch (boundNode.Kind)
-            {
-                case BoundKind.LocalDeclaration:
-                    return boundNode.Syntax.Kind() == SyntaxKind.VariableDeclarator &&
-                           boundNode.Syntax.Parent?.Kind() == SyntaxKind.VariableDeclaration &&
-                           boundNode.Syntax.Parent?.Parent?.Kind() == SyntaxKind.LocalDeclarationStatement;
-                case BoundKind.EventAccess:
-                    // related issue - https://github.com/dotnet/roslyn/pull/20960
-                    return boundNode.Syntax.Kind() == SyntaxKind.IdentifierName &&
-                           (boundNode.Syntax.Parent?.Kind() == SyntaxKind.AddAssignmentExpression ||
-                            boundNode.Syntax.Parent?.Kind() == SyntaxKind.SubtractAssignmentExpression);
-            }
-
-            return false;
-        }
-
         private IOperation CreateInternal(BoundNode boundNode)
         {
             switch (boundNode.Kind)
@@ -274,6 +246,11 @@ private ImmutableArray<IOperation> GetIOperationChildren(BoundNode boundNode)
             foreach (var childNode in boundNodeWithChildren.Children)
             {
                 var operation = Create(childNode);
+                if (operation == null)
+                {
+                    continue;
+                }
+
                 builder.Add(operation);
             }
 
@@ -292,7 +269,7 @@ private IPlaceholderExpression CreateBoundDeconstructValuePlaceholderOperation(B
         private IInvocationExpression CreateBoundCallOperation(BoundCall boundCall)
         {
             IMethodSymbol targetMethod = boundCall.Method;
-            Lazy<IOperation> instance = new Lazy<IOperation>(() => Create(((object)boundCall.Method == null || boundCall.Method.IsStatic) ? null : boundCall.ReceiverOpt));
+            Lazy<IOperation> instance = new Lazy<IOperation>(() => CreateBoundCallInstanceOperation(boundCall));
             bool isVirtual = (object)boundCall.Method != null &&
                         boundCall.ReceiverOpt != null &&
                         (boundCall.Method.IsVirtual || boundCall.Method.IsAbstract || boundCall.Method.IsOverride) &&
@@ -398,22 +375,6 @@ private IEventReferenceExpression CreateBoundEventAccessOperation(BoundEventAcce
             return new LazyEventReferenceExpression(@event, instance, member, _semanticModel, syntax, type, constantValue, isImplicit);
         }
 
-        private IEventReferenceExpression CreateBoundEventAccessOperation(BoundEventAssignmentOperator boundEventAssignmentOperator)
-        {
-            SyntaxNode syntax = boundEventAssignmentOperator.Syntax;
-            // BoundEventAssignmentOperator doesn't hold on to BoundEventAccess provided during binding.
-            // Based on the implementation of those two bound node types, the following data can be retrieved w/o changing BoundEventAssignmentOperator:
-            //  1. the type of BoundEventAccess is the type of the event symbol.
-            //  2. the constant value of BoundEventAccess is always null.
-            //  3. the syntax of the boundEventAssignmentOperator is always AssignmentExpressionSyntax, so the syntax for the event reference would be the LHS of the assignment.
-            IEventSymbol @event = boundEventAssignmentOperator.Event;
-            Lazy<IOperation> instance = new Lazy<IOperation>(() => Create(boundEventAssignmentOperator.Event.IsStatic ? null : boundEventAssignmentOperator.ReceiverOpt));
-            SyntaxNode eventAccessSyntax = ((AssignmentExpressionSyntax)syntax).Left;
-            bool isImplicit = boundEventAssignmentOperator.WasCompilerGenerated;
-
-            return new LazyEventReferenceExpression(@event, instance, @event, _semanticModel, eventAccessSyntax, @event.Type, ConvertToOptional(null), isImplicit);
-        }
-
         private IEventAssignmentExpression CreateBoundEventAssignmentOperatorOperation(BoundEventAssignmentOperator boundEventAssignmentOperator)
         {
             Lazy<IEventReferenceExpression> eventReference = new Lazy<IEventReferenceExpression>(() => CreateBoundEventAccessOperation(boundEventAssignmentOperator));
@@ -426,7 +387,7 @@ private IEventAssignmentExpression CreateBoundEventAssignmentOperatorOperation(B
             return new LazyEventAssignmentExpression(eventReference, handlerValue, adds, _semanticModel, syntax, type, constantValue, isImplicit);
         }
 
-            private IParameterReferenceExpression CreateBoundParameterOperation(BoundParameter boundParameter)
+        private IParameterReferenceExpression CreateBoundParameterOperation(BoundParameter boundParameter)
         {
             IParameterSymbol parameter = boundParameter.ParameterSymbol;
             SyntaxNode syntax = boundParameter.Syntax;
@@ -634,9 +595,8 @@ private IOperation CreateUnboundLambdaOperation(UnboundLambda unboundLambda)
             // nodes for the lambda expression. So, we ask the semantic model for the IOperation node for the unbound lambda syntax.
             // We are counting on the fact that will do the error recovery and actually create the BoundLambda node appropriate for
             // this syntax node.
-            var lambdaOperation = _semanticModel.GetOperationInternal(unboundLambda.Syntax);
-            Debug.Assert(lambdaOperation.Kind == OperationKind.AnonymousFunctionExpression);
-            return lambdaOperation;
+            var boundLambda = unboundLambda.BindForErrorRecovery();
+            return CreateInternal(boundLambda);
         }
 
         private IAnonymousFunctionExpression CreateBoundLambdaOperation(BoundLambda boundLambda)
@@ -1319,29 +1279,29 @@ private IInvalidStatement CreateBoundBadStatementOperation(BoundBadStatement bou
             return new LazyInvalidStatement(children, _semanticModel, syntax, type, constantValue, isImplicit);
         }
 
-        private IVariableDeclaration CreateVariableDeclaration(BoundLocalDeclaration boundLocalDeclaration)
+        private IOperation CreateBoundLocalDeclarationOperation(BoundLocalDeclaration boundLocalDeclaration)
         {
-            // special case for variable decl due to BoundNode using same node for both statement and other construct
-            return _variableDeclarationCache.GetOrAdd(boundLocalDeclaration, d =>
+            if (boundLocalDeclaration.Syntax.Kind() == SyntaxKind.LocalDeclarationStatement)
             {
-                return OperationFactory.CreateVariableDeclaration(d.LocalSymbol, Create(d.InitializerOpt), _semanticModel, d.Syntax);
-            });
-        }
-
-        private IVariableDeclarationStatement CreateBoundLocalDeclarationOperation(BoundLocalDeclaration boundLocalDeclaration)
-        {
-            Lazy<ImmutableArray<IVariableDeclaration>> declarations = new Lazy<ImmutableArray<IVariableDeclaration>>(() => ImmutableArray.Create(CreateVariableDeclaration(boundLocalDeclaration)));
-            SyntaxNode syntax = boundLocalDeclaration.Syntax;
-            ITypeSymbol type = null;
-            Optional<object> constantValue = default(Optional<object>);
-            bool isImplicit = boundLocalDeclaration.WasCompilerGenerated;
-            return new LazyVariableDeclarationStatement(declarations, _semanticModel, syntax, type, constantValue, isImplicit);
+                Lazy<ImmutableArray<IVariableDeclaration>> declarations = new Lazy<ImmutableArray<IVariableDeclaration>>(() => ImmutableArray.Create(CreateVariableDeclaration(boundLocalDeclaration)));
+                SyntaxNode syntax = boundLocalDeclaration.Syntax;
+                ITypeSymbol type = null;
+                Optional<object> constantValue = default(Optional<object>);
+                bool isImplicit = boundLocalDeclaration.WasCompilerGenerated;
+                return new LazyVariableDeclarationStatement(declarations, _semanticModel, syntax, type, constantValue, isImplicit);
+            }
+            else
+            {
+                // we can get here if someone asked about 1 variable declarator on multi local declaration
+                Debug.Assert(boundLocalDeclaration.Syntax.Kind() == SyntaxKind.VariableDeclarator);
+                return CreateVariableDeclaration(boundLocalDeclaration);
+            }
         }
 
         private IVariableDeclarationStatement CreateBoundMultipleLocalDeclarationsOperation(BoundMultipleLocalDeclarations boundMultipleLocalDeclarations)
         {
             Lazy<ImmutableArray<IVariableDeclaration>> declarations = new Lazy<ImmutableArray<IVariableDeclaration>>(() =>
-                boundMultipleLocalDeclarations.LocalDeclarations.SelectAsArray(declaration => CreateVariableDeclaration(declaration)));
+                boundMultipleLocalDeclarations.LocalDeclarations.SelectAsArray(declaration => (IVariableDeclaration)Create(declaration)));
             SyntaxNode syntax = boundMultipleLocalDeclarations.Syntax;
             ITypeSymbol type = null;
             Optional<object> constantValue = default(Optional<object>);

src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory_Methods.cs

@@ -6,6 +6,7 @@
 using System.Linq;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.PooledObjects;
 
 namespace Microsoft.CodeAnalysis.Semantics
@@ -48,6 +49,37 @@ internal IArgument CreateArgumentOperation(ArgumentKind kind, IParameterSymbol p
                 isImplicit: expression.WasCompilerGenerated);
         }
 
+        private IVariableDeclaration CreateVariableDeclaration(BoundLocalDeclaration boundLocalDeclaration)
+        {
+            return OperationFactory.CreateVariableDeclaration(boundLocalDeclaration.LocalSymbol, Create(boundLocalDeclaration.InitializerOpt), _semanticModel, boundLocalDeclaration.Syntax);
+        }
+
+        private IOperation CreateBoundCallInstanceOperation(BoundCall boundCall)
+        {
+            if (boundCall.Method == null || boundCall.Method.IsStatic)
+            {
+                return null;
+            }
+
+            return Create(boundCall.ReceiverOpt);
+        }
+
+        private IEventReferenceExpression CreateBoundEventAccessOperation(BoundEventAssignmentOperator boundEventAssignmentOperator)
+        {
+            SyntaxNode syntax = boundEventAssignmentOperator.Syntax;
+            // BoundEventAssignmentOperator doesn't hold on to BoundEventAccess provided during binding.
+            // Based on the implementation of those two bound node types, the following data can be retrieved w/o changing BoundEventAssignmentOperator:
+            //  1. the type of BoundEventAccess is the type of the event symbol.
+            //  2. the constant value of BoundEventAccess is always null.
+            //  3. the syntax of the boundEventAssignmentOperator is always AssignmentExpressionSyntax, so the syntax for the event reference would be the LHS of the assignment.
+            IEventSymbol @event = boundEventAssignmentOperator.Event;
+            Lazy<IOperation> instance = new Lazy<IOperation>(() => Create(boundEventAssignmentOperator.Event.IsStatic ? null : boundEventAssignmentOperator.ReceiverOpt));
+            SyntaxNode eventAccessSyntax = ((AssignmentExpressionSyntax)syntax).Left;
+            bool isImplicit = boundEventAssignmentOperator.WasCompilerGenerated;
+
+            return new LazyEventReferenceExpression(@event, instance, @event, _semanticModel, eventAccessSyntax, @event.Type, ConvertToOptional(null), isImplicit);
+        }
+
         private ImmutableArray<IArgument> DeriveArguments(
             BoundNode boundNode,
             Binder binder,

src/Compilers/Core/Portable/Operations/Operation.cs

@@ -164,12 +164,12 @@ public static IOperation CreateOperationNone(SemanticModel semanticModel, Syntax
 
         private class NoneOperation : Operation
         {
-            private readonly Func<ImmutableArray<IOperation>> _getChildren;
+            private readonly Lazy<ImmutableArray<IOperation>> _lazyChildren;
 
             public NoneOperation(SemanticModel semanticModel, SyntaxNode node, Optional<object> constantValue, Func<ImmutableArray<IOperation>> getChildren, bool isImplicit) :
                 base(OperationKind.None, semanticModel, node, type: null, constantValue: constantValue, isImplicit: isImplicit)
             {
-                _getChildren = getChildren;
+                _lazyChildren = new Lazy<ImmutableArray<IOperation>>(getChildren);
             }
 
             public override void Accept(OperationVisitor visitor)
@@ -186,7 +186,7 @@ public override IEnumerable<IOperation> Children
             {
                 get
                 {
-                    foreach (var child in _getChildren().NullToEmpty())
+                    foreach (var child in _lazyChildren.Value)
                     {
                         if (child == null)
                         {

src/Compilers/VisualBasic/Portable/Binding/MemberSemanticModel.vb

@@ -794,8 +794,19 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
         End Function
 
         Friend Overrides Function GetOperationWorker(node As VisualBasicSyntaxNode, options As GetOperationOptions, cancellationToken As CancellationToken) As IOperation
+            Dim bindingRoot = DirectCast(GetBindingRoot(node), VisualBasicSyntaxNode)
+
+            Dim result As BoundNode = Nothing
+            Dim statementOrRootOperation = GetStatementOrRootOperation(bindingRoot, options, result, cancellationToken)
+
+            ' we might optimize it later
+            Return statementOrRootOperation.DescendantsAndSelf().FirstOrDefault(Function(o) o.Syntax Is node)
+        End Function
+
+        Private Function GetStatementOrRootOperation(node As VisualBasicSyntaxNode, options As GetOperationOptions, ByRef result As BoundNode, cancellationToken As CancellationToken) As IOperation
+            Debug.Assert(node Is GetBindingRoot(node))
+
             Dim summary = GetBoundNodeSummary(node)
-            Dim result As BoundNode
             Select Case options
                 Case GetOperationOptions.Highest
                     result = summary.HighestBoundNode

src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory.vb

@@ -34,41 +34,9 @@ Namespace Microsoft.CodeAnalysis.Semantics
                 Return _semanticModel.CloneOperation(CreateInternal(boundNode))
             End If
 
-            ' this should be removed once this issue is fixed
-            ' https://github.com/dotnet/roslyn/issues/21187
-            If IsIgnoredNode(boundNode) Then
-                ' due to how IOperation is set up, some of VB BoundNode must be ignored
-                ' while generating IOperation. otherwise, 2 different IOperation trees will be created
-                ' for nodes under same sub tree
-                Return Nothing
-            End If
-
             Return _cache.GetOrAdd(boundNode, Function(n) CreateInternal(n))
         End Function
 
-        Private Shared Function IsIgnoredNode(boundNode As BoundNode) As Boolean
-            ' since boundNode doesn't have parent pointer, it can't just look around using bound node
-            ' it needs to use syntax node.  we ignore these because this will return its own operation tree
-            ' that don't belong to its parent operation tree.
-            Select Case boundNode.Kind
-                Case BoundKind.LocalDeclaration
-                    Return boundNode.Syntax.Kind() = SyntaxKind.ModifiedIdentifier AndAlso
-                           If(boundNode.Syntax.Parent?.Kind() = SyntaxKind.VariableDeclarator, False)
-                Case BoundKind.ExpressionStatement
-                    Return boundNode.Syntax.Kind() = SyntaxKind.SelectStatement
-                Case BoundKind.CaseBlock
-                    Return True
-                Case BoundKind.CaseStatement
-                    Return True
-                Case BoundKind.EventAccess
-                    Return boundNode.Syntax.Parent.Kind() = SyntaxKind.AddHandlerStatement OrElse
-                           boundNode.Syntax.Parent.Kind() = SyntaxKind.RemoveHandlerStatement OrElse
-                           boundNode.Syntax.Parent.Kind() = SyntaxKind.RaiseEventAccessorStatement
-            End Select
-
-            Return False
-        End Function
-
         Private Function CreateInternal(boundNode As BoundNode) As IOperation
             Select Case boundNode.Kind
                 Case BoundKind.AssignmentOperator
@@ -161,6 +129,8 @@ Namespace Microsoft.CodeAnalysis.Semantics
                     Return CreateBoundIfStatementOperation(DirectCast(boundNode, BoundIfStatement))
                 Case BoundKind.SelectStatement
                     Return CreateBoundSelectStatementOperation(DirectCast(boundNode, BoundSelectStatement))
+                Case BoundKind.CaseBlock
+                    Return CreateBoundCaseBlockOperation(DirectCast(boundNode, BoundCaseBlock))
                 Case BoundKind.SimpleCaseClause
                     Return CreateBoundSimpleCaseClauseOperation(DirectCast(boundNode, BoundSimpleCaseClause))
                 Case BoundKind.RangeCaseClause
@@ -248,6 +218,10 @@ Namespace Microsoft.CodeAnalysis.Semantics
             Dim builder = ArrayBuilder(Of IOperation).GetInstance(boundNodeWithChildren.Children.Length)
             For Each childNode In boundNodeWithChildren.Children
                 Dim operation = Create(childNode)
+                If operation Is Nothing Then
+                    Continue For
+                End If
+
                 builder.Add(operation)
             Next
 
@@ -826,7 +800,7 @@ Namespace Microsoft.CodeAnalysis.Semantics
 
         Private Function CreateBoundSelectStatementOperation(boundSelectStatement As BoundSelectStatement) As ISwitchStatement
             Dim value As Lazy(Of IOperation) = New Lazy(Of IOperation)(Function() Create(boundSelectStatement.ExpressionStatement.Expression))
-            Dim cases As Lazy(Of ImmutableArray(Of ISwitchCase)) = New Lazy(Of ImmutableArray(Of ISwitchCase))(Function() GetSwitchStatementCases(boundSelectStatement.CaseBlocks))
+            Dim cases As Lazy(Of ImmutableArray(Of ISwitchCase)) = New Lazy(Of ImmutableArray(Of ISwitchCase))(Function() boundSelectStatement.CaseBlocks.SelectAsArray(Function(n) DirectCast(Create(n), ISwitchCase)))
             Dim syntax As SyntaxNode = boundSelectStatement.Syntax
             Dim type As ITypeSymbol = Nothing
             Dim constantValue As [Optional](Of Object) = New [Optional](Of Object)()
@@ -834,6 +808,32 @@ Namespace Microsoft.CodeAnalysis.Semantics
             Return New LazySwitchStatement(value, cases, _semanticModel, syntax, type, constantValue, isImplicit)
         End Function
 
+        Private Function CreateBoundCaseBlockOperation(boundCaseBlock As BoundCaseBlock) As ISwitchCase
+            Dim clauses As Lazy(Of ImmutableArray(Of ICaseClause)) = New Lazy(Of ImmutableArray(Of ICaseClause))(
+                Function()
+                    ' `CaseElseClauseSyntax` is bound to `BoundCaseStatement` with an empty list of case clauses,
+                    ' so we explicitly create an IOperation node for Case-Else clause to differentiate it from Case clause.
+                    Dim caseStatement = boundCaseBlock.CaseStatement
+                    If caseStatement.CaseClauses.IsEmpty AndAlso caseStatement.Syntax.Kind() = SyntaxKind.CaseElseStatement Then
+                        Return ImmutableArray.Create(Of ICaseClause)(
+                            New DefaultCaseClause(
+                                _semanticModel,
+                                caseStatement.Syntax,
+                                type:=Nothing,
+                                constantValue:=Nothing,
+                                isImplicit:=boundCaseBlock.WasCompilerGenerated))
+                    Else
+                        Return caseStatement.CaseClauses.SelectAsArray(Function(n) DirectCast(Create(n), ICaseClause))
+                    End If
+                End Function)
+            Dim body As Lazy(Of ImmutableArray(Of IOperation)) = New Lazy(Of ImmutableArray(Of IOperation))(Function() ImmutableArray.Create(Create(boundCaseBlock.Body)))
+            Dim syntax As SyntaxNode = boundCaseBlock.Syntax
+            Dim type As ITypeSymbol = Nothing
+            Dim constantValue As [Optional](Of Object) = New [Optional](Of Object)()
+            Dim isImplicit As Boolean = boundCaseBlock.WasCompilerGenerated
+            Return New LazySwitchCase(clauses, body, _semanticModel, syntax, type, constantValue, isImplicit)
+        End Function
+
         Private Function CreateBoundSimpleCaseClauseOperation(boundSimpleCaseClause As BoundSimpleCaseClause) As ISingleValueCaseClause
             Dim clauseValue = GetSingleValueCaseClauseValue(boundSimpleCaseClause)
             Dim value As Lazy(Of IOperation) = New Lazy(Of IOperation)(Function() Create(clauseValue))
@@ -945,7 +945,7 @@ Namespace Microsoft.CodeAnalysis.Semantics
         End Function
 
         Private Function CreateBoundForEachStatementOperation(boundForEachStatement As BoundForEachStatement) As IForEachLoopStatement
-            Dim iterationVariable As ILocalSymbol = Nothing ' Manual
+            Dim iterationVariable As ILocalSymbol = TryCast(boundForEachStatement.ControlVariable, BoundLocal)?.LocalSymbol
             Dim collection As Lazy(Of IOperation) = New Lazy(Of IOperation)(Function() Create(boundForEachStatement.Collection))
             Dim LoopKind As LoopKind = LoopKind.ForEach
             Dim body As Lazy(Of IOperation) = New Lazy(Of IOperation)(Function() Create(boundForEachStatement.Body))
@@ -969,7 +969,7 @@ Namespace Microsoft.CodeAnalysis.Semantics
 
         Private Function CreateBoundCatchBlockOperation(boundCatchBlock As BoundCatchBlock) As ICatchClause
             Dim handler As Lazy(Of IBlockStatement) = New Lazy(Of IBlockStatement)(Function() DirectCast(Create(boundCatchBlock.Body), IBlockStatement))
-            Dim caughtType As ITypeSymbol = Nothing ' Manual
+            Dim caughtType As ITypeSymbol = boundCatchBlock.ExceptionFilterOpt?.Type
             Dim filter As Lazy(Of IOperation) = New Lazy(Of IOperation)(Function() Create(boundCatchBlock.ExceptionFilterOpt))
             Dim exceptionLocal As ILocalSymbol = boundCatchBlock.LocalOpt
             Dim syntax As SyntaxNode = boundCatchBlock.Syntax

src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory_Methods.vb

@@ -186,32 +186,6 @@ Namespace Microsoft.CodeAnalysis.Semantics
             Return builder.ToImmutableAndFree()
         End Function
 
-        Private Function GetSwitchStatementCases(caseBlocks As ImmutableArray(Of BoundCaseBlock)) As ImmutableArray(Of ISwitchCase)
-            Return caseBlocks.SelectAsArray(
-                Function(boundCaseBlock)
-                    ' `CaseElseClauseSyntax` is bound to `BoundCaseStatement` with an empty list of case clauses,
-                    ' so we explicitly create an IOperation node for Case-Else clause to differentiate it from Case clause.
-                    Dim clauses As ImmutableArray(Of ICaseClause)
-                    Dim caseStatement = boundCaseBlock.CaseStatement
-                    Dim isImplicit As Boolean = boundCaseBlock.WasCompilerGenerated
-                    If caseStatement.CaseClauses.IsEmpty AndAlso caseStatement.Syntax.Kind() = SyntaxKind.CaseElseStatement Then
-                        clauses = ImmutableArray.Create(Of ICaseClause)(
-                                                                    New DefaultCaseClause(
-                                                                        _semanticModel,
-                                                                        caseStatement.Syntax,
-                                                                        type:=Nothing,
-                                                                        constantValue:=Nothing,
-                                                                        isImplicit:=isImplicit))
-                    Else
-                        clauses = caseStatement.CaseClauses.SelectAsArray(Function(n) DirectCast(Create(n), ICaseClause))
-                    End If
-
-                    Dim body = ImmutableArray.Create(Create(boundCaseBlock.Body))
-                    Dim syntax = boundCaseBlock.Syntax
-                    Return DirectCast(New SwitchCase(clauses, body, _semanticModel, syntax, type:=Nothing, constantValue:=Nothing, isImplicit:=isImplicit), ISwitchCase)
-                End Function)
-        End Function
-
         Private Shared Function GetSingleValueCaseClauseValue(clause As BoundSimpleCaseClause) As BoundExpression
             If clause.ValueOpt IsNot Nothing Then
                 Return clause.ValueOpt

src/Test/Utilities/Portable/CommonTestBase.cs

@@ -10,6 +10,8 @@
 using System.Reflection;
 using System.Reflection.Emit;
 using System.Reflection.PortableExecutable;
+using System.Text;
+using System.Threading;
 using System.Xml.Linq;
 using Microsoft.CodeAnalysis.CodeGen;
 using Microsoft.CodeAnalysis.Emit;
@@ -532,8 +534,6 @@ internal static SignatureDescription Signature(string fullyQualifiedTypeName, st
         #region Operation Test Helpers
         internal static void VerifyParentOperations(SemanticModel model)
         {
-            HashSet<string> output = new HashSet<string>();
-
             var parentMap = GetParentOperationsMap(model);
 
             // check parent for each child
@@ -545,10 +545,10 @@ internal static void VerifyParentOperations(SemanticModel model)
                 // check SearchparentOperation return same parent
                 Assert.Equal(((Operation)child).SearchParentOperation(), parent);
 
-                if (parent == null && child.Kind != OperationKind.None)
+                if (parent == null)
                 {
                     // this is root of operation tree
-                    VerifyOperationTreeContracts(child, output);
+                    VerifyOperationTreeContracts(child);
                 }
             }
         }
@@ -631,15 +631,15 @@ internal static void VerifyClone(SemanticModel model)
             }
         }
 
-        private static void VerifyOperationTreeContracts(IOperation root, HashSet<string> output)
+        private static void VerifyOperationTreeContracts(IOperation root)
         {
             var semanticModel = ((Operation)root).SemanticModel;
             var set = new HashSet<IOperation>(root.DescendantsAndSelf());
 
-            foreach (var child in GetOperationsUptoInvalidOperation(root))
+            foreach (var child in root.DescendantsAndSelf())
             {
-                // all spine of child.Syntax node must have IOperation up to the root
-                VerifyOperationTreeSpine(semanticModel, set, child.Syntax, output);
+                // all operations from spine should belong to the operation tree set
+                VerifyOperationTreeSpine(semanticModel, set, child.Syntax);
 
                 // operation tree's node must be part of root of semantic model which is 
                 // owner of operation's lifetime
@@ -647,129 +647,21 @@ private static void VerifyOperationTreeContracts(IOperation root, HashSet<string
             }
         }
 
-        private static IEnumerable<IOperation> GetOperationsUptoInvalidOperation(IOperation root)
-        {
-            foreach (var child in root.Children)
-            {
-                // don't go down invalid expression/statement until
-                // we decide what to do with such case.
-                // https://github.com/dotnet/roslyn/issues/21187
-                // in current implementation, below invalid expression/statements are all messed up
-                // and we can't gurantee it will return same operation tree
-                if (child == null ||
-                    child.Kind == OperationKind.InvalidExpression ||
-                    child.Kind == OperationKind.InvalidStatement)
-                {
-                    continue;
-                }
-
-                yield return child;
-
-                foreach (var nested in GetOperationsUptoInvalidOperation(child))
-                {
-                    yield return nested;
-                }
-            }
-        }
-
         private static void VerifyOperationTreeSpine(
-            SemanticModel semanticModel, HashSet<IOperation> set, SyntaxNode node, HashSet<string> output)
+            SemanticModel semanticModel, HashSet<IOperation> set, SyntaxNode node)
         {
             while (node != semanticModel.Root)
             {
-                if (!IsIgnoredNode(node))
+                var operation = semanticModel.GetOperationInternal(node);
+                if (operation != null)
                 {
-                    var operation = semanticModel.GetOperationInternal(node);
-                    Assert.NotNull(operation);
-
-                    // all operation from same sub tree must belong to same operation tree
-                    // except OperationKind.None and OperationKind.InvalidExpression and InvalidStatement
-                    // for those kinds, we can't guarantee it will share same tree
-                    if (operation != null &&
-                        operation.Kind != OperationKind.None &&
-                        operation.Kind != OperationKind.InvalidExpression &&
-                        operation.Kind != OperationKind.InvalidStatement &&
-                        operation.Kind != OperationKind.PlaceholderExpression)
-                    {
-                        Assert.True(set.Contains(operation));
-                    }
+                    Assert.True(set.Contains(operation));
                 }
 
                 node = node.Parent;
             }
         }
 
-        private static bool IsIgnoredNode(SyntaxNode node)
-        {
-            // this should be removed once this is fixed
-            // https://github.com/dotnet/roslyn/issues/21187
-            // basically, for these node. GetOpeartion will return null 
-            // even though GetOperation returns IOperation for its child (syntax node)
-            // violating our assumption that all spine once a node has IOperation should
-            // have an IOperation
-            if (node is CSharp.CSharpSyntaxNode csNode)
-            {
-                switch (csNode.Kind())
-                {
-                    case CSharp.SyntaxKind.VariableDeclarator:
-                        return csNode.Parent?.Kind() == CSharp.SyntaxKind.VariableDeclaration;
-                    case CSharp.SyntaxKind.EqualsValueClause:
-                        return csNode.Parent?.Kind() == CSharp.SyntaxKind.VariableDeclarator &&
-                               csNode.Parent?.Parent?.Kind() == CSharp.SyntaxKind.VariableDeclaration;
-                    case CSharp.SyntaxKind.IdentifierName when csNode.ToString() == "E":
-                        // related issue - https://github.com/dotnet/roslyn/pull/20960
-                        return csNode.Parent?.Kind() == CSharp.SyntaxKind.AddAssignmentExpression ||
-                               csNode.Parent?.Kind() == CSharp.SyntaxKind.SubtractAssignmentExpression;
-                    case CSharp.SyntaxKind.CheckedStatement:
-                    case CSharp.SyntaxKind.UncheckedStatement:
-                    case CSharp.SyntaxKind.UnsafeStatement:
-                        return true;
-
-                    default:
-                        return false;
-                }
-            }
-
-            var vbNode = (VisualBasic.VisualBasicSyntaxNode)node;
-            switch (vbNode.Kind())
-            {
-                case VisualBasic.SyntaxKind.SimpleArgument:
-                    return vbNode.Parent?.Kind() == VisualBasic.SyntaxKind.ArgumentList ||
-                           vbNode.Parent?.Kind() == VisualBasic.SyntaxKind.TupleExpression;
-                case VisualBasic.SyntaxKind.VariableDeclarator:
-                    return vbNode.Parent?.Kind() == VisualBasic.SyntaxKind.LocalDeclarationStatement ||
-                           vbNode.Parent?.Kind() == VisualBasic.SyntaxKind.FieldDeclaration;
-                case VisualBasic.SyntaxKind.EqualsValue:
-                    return vbNode.Parent?.Kind() == VisualBasic.SyntaxKind.VariableDeclarator;
-                case VisualBasic.SyntaxKind.NamedFieldInitializer:
-                    return vbNode.Parent?.Kind() == VisualBasic.SyntaxKind.ObjectMemberInitializer;
-                case VisualBasic.SyntaxKind.ObjectMemberInitializer:
-                    return vbNode.Parent?.Kind() == VisualBasic.SyntaxKind.AnonymousObjectCreationExpression;
-                case VisualBasic.SyntaxKind.SelectStatement:
-                    return vbNode.Parent?.Kind() == VisualBasic.SyntaxKind.SelectBlock;
-                case VisualBasic.SyntaxKind.CollectionInitializer:
-                case VisualBasic.SyntaxKind.ModifiedIdentifier:
-                case VisualBasic.SyntaxKind.CaseBlock:
-                case VisualBasic.SyntaxKind.CaseElseBlock:
-                case VisualBasic.SyntaxKind.CaseStatement:
-                case VisualBasic.SyntaxKind.CaseElseStatement:
-                case VisualBasic.SyntaxKind.WhileClause:
-                case VisualBasic.SyntaxKind.ArgumentList:
-                case VisualBasic.SyntaxKind.FromClause:
-                case VisualBasic.SyntaxKind.ExpressionRangeVariable:
-                case VisualBasic.SyntaxKind.LetClause:
-                case VisualBasic.SyntaxKind.JoinCondition:
-                case VisualBasic.SyntaxKind.AsNewClause:
-                case VisualBasic.SyntaxKind.ForStepClause:
-                case VisualBasic.SyntaxKind.UntilClause:
-                case VisualBasic.SyntaxKind.InterpolationAlignmentClause:
-                    return true;
-                default:
-                    return vbNode.Parent?.Kind() == VisualBasic.SyntaxKind.AddHandlerStatement ||
-                           vbNode.Parent?.Kind() == VisualBasic.SyntaxKind.RemoveHandlerStatement ||
-                           vbNode.Parent?.Kind() == VisualBasic.SyntaxKind.RaiseEventStatement;
-            }
-        }
         #endregion
     }
 }