Address feedback from Heejae

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

@@ -278,9 +278,9 @@ private ImmutableArray<IOperation> GetIOperationChildren(BoundNode boundNode)
             return builder.ToImmutableAndFree();
         }
 
-        private IVariableDeclaratorOperation CreateVariableDeclaration(BoundLocalDeclaration boundNode)
+        private IVariableDeclaratorOperation CreateVariableDeclarator(BoundLocalDeclaration boundNode)
         {
-            return (IVariableDeclaratorOperation)_cache.GetOrAdd(boundNode, n => CreateVariableDeclarationInternal((BoundLocalDeclaration)n, n.Syntax));
+            return (IVariableDeclaratorOperation)_cache.GetOrAdd(boundNode, n => CreateVariableDeclaratorInternal((BoundLocalDeclaration)n, n.Syntax));
         }
 
         private IPlaceholderOperation CreateBoundDeconstructValuePlaceholderOperation(BoundDeconstructValuePlaceholder boundDeconstructValuePlaceholder)
@@ -894,7 +894,7 @@ private IArrayCreationOperation CreateBoundArrayCreationOperation(BoundArrayCrea
             SyntaxNode syntax = boundArrayCreation.Syntax;
             ITypeSymbol type = boundArrayCreation.Type;
             Optional<object> constantValue = ConvertToOptional(boundArrayCreation.ConstantValue);
-            bool isImplicit = boundArrayCreation.WasCompilerGenerated || 
+            bool isImplicit = boundArrayCreation.WasCompilerGenerated ||
                               (boundArrayCreation.InitializerOpt?.Syntax == syntax && !boundArrayCreation.InitializerOpt.WasCompilerGenerated);
             return new LazyArrayCreationExpression(dimensionSizes, initializer, _semanticModel, syntax, type, constantValue, isImplicit);
         }
@@ -1419,7 +1419,7 @@ private ITryOperation CreateBoundTryStatementOperation(BoundTryStatement boundTr
         private ICatchClauseOperation CreateBoundCatchBlockOperation(BoundCatchBlock boundCatchBlock)
         {
             var exceptionSourceOpt = (BoundLocal)boundCatchBlock.ExceptionSourceOpt;
-            Lazy<IOperation> expressionDeclarationOrExpression = new Lazy<IOperation>(() => exceptionSourceOpt != null ? CreateVariableDeclaration(exceptionSourceOpt) : null);
+            Lazy<IOperation> expressionDeclarationOrExpression = new Lazy<IOperation>(() => exceptionSourceOpt != null ? CreateVariableDeclarator(exceptionSourceOpt) : null);
             ITypeSymbol exceptionType = boundCatchBlock.ExceptionTypeOpt;
             ImmutableArray<ILocalSymbol> locals = boundCatchBlock.Locals.As<ILocalSymbol>();
             Lazy<IOperation> filter = new Lazy<IOperation>(() => Create(boundCatchBlock.ExceptionFilterOpt));
@@ -1518,42 +1518,42 @@ private IOperation CreateBoundLocalDeclarationOperation(BoundLocalDeclaration bo
             switch (kind)
             {
                 case SyntaxKind.LocalDeclarationStatement:
-                    {
-                        var statement = (LocalDeclarationStatementSyntax)node;
+                {
+                    var statement = (LocalDeclarationStatementSyntax)node;
 
-                        // this happen for simple int i = 0;
-                        // var statement points to LocalDeclarationStatementSyntax
-                        varStatement = statement;
+                    // this happen for simple int i = 0;
+                    // var statement points to LocalDeclarationStatementSyntax
+                    varStatement = statement;
 
-                        varDeclaration = statement.Declaration;
+                    varDeclaration = statement.Declaration;
 
-                        varDeclarator = statement.Declaration.Variables.First();
-                        break;
-                    }
+                    varDeclarator = statement.Declaration.Variables.First();
+                    break;
+                }
                 case SyntaxKind.VariableDeclarator:
-                    {
-                        // this happen for 'for loop' initializer
-                        // We generate a DeclarationGroup for this scenario to maintain tree shape consistency across IOperation.
-                        // var statement points to VariableDeclarationSyntax
-                        varStatement = node.Parent;
+                {
+                    // this happen for 'for loop' initializer
+                    // We generate a DeclarationGroup for this scenario to maintain tree shape consistency across IOperation.
+                    // var statement points to VariableDeclarationSyntax
+                    varStatement = node.Parent;
 
-                        varDeclaration = node.Parent;
+                    varDeclaration = node.Parent;
 
-                        // var declaration points to VariableDeclaratorSyntax
-                        varDeclarator = node;
-                        break;
-                    }
+                    // var declaration points to VariableDeclaratorSyntax
+                    varDeclarator = node;
+                    break;
+                }
                 default:
-                    {
-                        Debug.Fail($"Unexpected syntax: {kind}");
+                {
+                    Debug.Fail($"Unexpected syntax: {kind}");
 
-                        // otherwise, they points to whatever bound nodes are pointing to.
-                        varStatement = varDeclaration = varDeclarator = node;
-                        break;
-                    }
+                    // otherwise, they points to whatever bound nodes are pointing to.
+                    varStatement = varDeclaration = varDeclarator = node;
+                    break;
+                }
             }
 
-            Lazy<ImmutableArray<IVariableDeclaratorOperation>> declarations = new Lazy<ImmutableArray<IVariableDeclaratorOperation>>(() => ImmutableArray.Create(CreateVariableDeclarationInternal(boundLocalDeclaration, varDeclarator)));
+            Lazy<ImmutableArray<IVariableDeclaratorOperation>> declarations = new Lazy<ImmutableArray<IVariableDeclaratorOperation>>(() => ImmutableArray.Create(CreateVariableDeclaratorInternal(boundLocalDeclaration, varDeclarator)));
             bool multiVariableImplicit = boundLocalDeclaration.WasCompilerGenerated;
             // In C#, the MultiVariable initializer will always be null, but we can't pass null as the actual lazy. We assume that all lazy elements always exist
             Lazy<IVariableInitializerOperation> initializer = OperationFactory.EmptyInitializer;
@@ -1568,8 +1568,8 @@ private IOperation CreateBoundLocalDeclarationOperation(BoundLocalDeclaration bo
 
         private IVariableDeclarationGroupOperation CreateBoundMultipleLocalDeclarationsOperation(BoundMultipleLocalDeclarations boundMultipleLocalDeclarations)
         {
-            Lazy<ImmutableArray<IVariableDeclaratorOperation>> declarations = new Lazy<ImmutableArray<IVariableDeclaratorOperation>>(() =>
-                boundMultipleLocalDeclarations.LocalDeclarations.SelectAsArray(declaration => CreateVariableDeclaration(declaration)));
+            Lazy<ImmutableArray<IVariableDeclaratorOperation>> declarators = new Lazy<ImmutableArray<IVariableDeclaratorOperation>>(() =>
+                boundMultipleLocalDeclarations.LocalDeclarations.SelectAsArray(declaration => CreateVariableDeclarator(declaration)));
             // In C#, the MultiVariable initializer will always be null, but we can't pass null as the actual lazy. We assume that all lazy elements always exist
             Lazy<IVariableInitializerOperation> initializer = OperationFactory.EmptyInitializer;
 
@@ -1581,7 +1581,7 @@ private IVariableDeclarationGroupOperation CreateBoundMultipleLocalDeclarationsO
                     ((LocalDeclarationStatementSyntax)declarationGroupSyntax).Declaration :
                     declarationGroupSyntax;
             bool declarationIsImplicit = boundMultipleLocalDeclarations.WasCompilerGenerated;
-            IVariableDeclarationOperation multiVariableDeclaration = new LazyVariableDeclaration(declarations, initializer, _semanticModel, declarationSyntax, null, default, declarationIsImplicit);
+            IVariableDeclarationOperation multiVariableDeclaration = new LazyVariableDeclaration(declarators, initializer, _semanticModel, declarationSyntax, null, default, declarationIsImplicit);
 
             ITypeSymbol type = null;
             Optional<object> constantValue = default(Optional<object>);

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

@@ -51,7 +51,7 @@ internal IArgumentOperation CreateArgumentOperation(ArgumentKind kind, IParamete
                 isImplicit: expression.WasCompilerGenerated || argument == null);
         }
 
-        private IVariableDeclaratorOperation CreateVariableDeclarationInternal(BoundLocalDeclaration boundLocalDeclaration, SyntaxNode syntax)
+        private IVariableDeclaratorOperation CreateVariableDeclaratorInternal(BoundLocalDeclaration boundLocalDeclaration, SyntaxNode syntax)
         {
             IVariableInitializerOperation initializer = null;
             if (boundLocalDeclaration.InitializerOpt != null)
@@ -87,7 +87,7 @@ private IVariableDeclaratorOperation CreateVariableDeclarationInternal(BoundLoca
             return new VariableDeclarator(symbol, initializer, _semanticModel, syntax, type, constantValue, isImplicit);
         }
 
-        private IVariableDeclaratorOperation CreateVariableDeclaration(BoundLocal boundLocal)
+        private IVariableDeclaratorOperation CreateVariableDeclarator(BoundLocal boundLocal)
         {
             return new VariableDeclarator(boundLocal.LocalSymbol, initializer: null, semanticModel: _semanticModel, syntax: boundLocal.Syntax, type: null, constantValue: default, isImplicit: false);
         }

src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_IAnonymousFunctionExpression.cs

@@ -401,16 +401,16 @@ static void F()
             var variableDeclaration = syntaxTree.GetRoot().DescendantNodes().OfType<LocalDeclarationStatementSyntax>().Single();
             var lambdaSyntax = (LambdaExpressionSyntax)variableDeclaration.Declaration.Variables.Single().Initializer.Value;
 
-            var variableDeclarationOperation = (IVariableDeclarationGroupOperation)semanticModel.GetOperationInternal(variableDeclaration);
-            var variableTreeLambdaOperation = (IAnonymousFunctionOperation)variableDeclarationOperation.Declarations.Single().Declarators.Single().Initializer.Value;
+            var variableDeclarationGroupOperation = (IVariableDeclarationGroupOperation)semanticModel.GetOperationInternal(variableDeclaration);
+            var variableTreeLambdaOperation = (IAnonymousFunctionOperation)variableDeclarationGroupOperation.Declarations.Single().Declarators.Single().Initializer.Value;
             var lambdaOperation = (IAnonymousFunctionOperation)semanticModel.GetOperationInternal(lambdaSyntax);
 
             // Assert that both ways of getting to the lambda (requesting the lambda directly, and requesting via the lambda syntax)
             // return the same bound node.
             Assert.Same(variableTreeLambdaOperation, lambdaOperation);
 
-            var variableDeclarationOperationSecondRequest = (IVariableDeclarationGroupOperation)semanticModel.GetOperationInternal(variableDeclaration);
-            var variableTreeLambdaOperationSecondRequest = (IAnonymousFunctionOperation)variableDeclarationOperation.Declarations.Single().Declarators.Single(). Initializer.Value;
+            var variableDeclarationGroupOperationSecondRequest = (IVariableDeclarationGroupOperation)semanticModel.GetOperationInternal(variableDeclaration);
+            var variableTreeLambdaOperationSecondRequest = (IAnonymousFunctionOperation)variableDeclarationGroupOperation.Declarations.Single().Declarators.Single(). Initializer.Value;
             var lambdaOperationSecondRequest = (IAnonymousFunctionOperation)semanticModel.GetOperationInternal(lambdaSyntax);
 
             // Assert that, when request the variable declaration or the lambda for a second time, there is no rebinding of the

src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_IConversionExpression.cs

@@ -134,7 +134,7 @@ static void Main(string[] args)
                 additionalOperationTreeVerifier: new ExpectedSymbolVerifier().Verify);
         }
 
-        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/20175")]
+        [Fact]
         public void ConversionExpression_Implicit_NumericConversion_InvalidNoInitializer()
         {
             string source = @"
@@ -149,11 +149,11 @@ static void Main(string[] args)
 }
 ";
             string expectedOperationTree = @"
-IVariableDeclarationStatement (1 declarators) (OperationKind.VariableDeclarationStatement, IsInvalid) (Syntax: 'int /*<bind ... *</bind>*/;')
-  IVariableDeclaration (1 variables) (OperationKind.VariableDeclaration, IsInvalid) (Syntax: 'int /*<bind ... *</bind>*/;')
-    Variables: Local_1: System.Int32 i1
-    Initializer: IConversionExpression (ConversionKind.Invalid, Implicit) (OperationKind.ConversionExpression, Type: System.Int32, IsInvalid) (Syntax: '')
-        IInvalidExpression (OperationKind.InvalidExpression, Type: ?, IsInvalid) (Syntax: '')
+IVariableDeclaratorOperation (Symbol: System.Int32 i1) (OperationKind.VariableDeclarator, Type: null, IsInvalid) (Syntax: 'i1 =/*</bind>*/')
+  Initializer: 
+    IVariableInitializerOperation (OperationKind.VariableInitializer, Type: null, IsInvalid) (Syntax: '=/*</bind>*/')
+      IInvalidOperation (OperationKind.Invalid, Type: null, IsInvalid) (Syntax: '')
+        Children(0)
 ";
             var expectedDiagnostics = new DiagnosticDescription[] {
                 // CS1525: Invalid expression term ';'
@@ -162,7 +162,22 @@ static void Main(string[] args)
             };
 
             VerifyOperationTreeAndDiagnosticsForTest<VariableDeclaratorSyntax>(source, expectedOperationTree, expectedDiagnostics,
-                additionalOperationTreeVerifier: new ExpectedSymbolVerifier().Verify);
+                additionalOperationTreeVerifier: (operation, compilation, syntax) =>
+                {
+                    // This scenario, where the syntax has IsMissing set, is special cased. We remove the conversion, and leave
+                    // just an IInvalidOperation with null type. First assert that our assumptions are true, then test the actual
+                    // result
+                    var initializerSyntax = ((VariableDeclaratorSyntax)syntax).Initializer.Value;
+                    var typeInfo = compilation.GetSemanticModel(syntax.SyntaxTree).GetTypeInfo(initializerSyntax);
+                    Assert.Equal(SyntaxKind.IdentifierName, initializerSyntax.Kind());
+                    Assert.True(initializerSyntax.IsMissing);
+                    Assert.Null(typeInfo.Type);
+                    Assert.Null(typeInfo.ConvertedType);
+
+                    var initializerOperation = ((IVariableDeclaratorOperation)operation).Initializer.Value;
+                    Assert.Null(initializerOperation.Type);
+                    Assert.Equal(OperationKind.Invalid, initializerOperation.Kind);
+                });
         }
 
         [CompilerTrait(CompilerFeature.IOperation)]

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

@@ -265,10 +265,10 @@ Namespace Microsoft.CodeAnalysis.Operations
             Dim builder = ArrayBuilder(Of IVariableDeclarationOperation).GetInstance()
             For Each declarationGroup In groupedDeclarations
                 Dim first = declarationGroup.First()
-                Dim singleDeclarations As ImmutableArray(Of IVariableDeclaratorOperation) = Nothing
+                Dim declarators As ImmutableArray(Of IVariableDeclaratorOperation) = Nothing
                 Dim initializer As IVariableInitializerOperation = Nothing
                 If first.Kind = BoundKind.LocalDeclaration Then
-                    singleDeclarations = declarationGroup.Cast(Of BoundLocalDeclaration).SelectAsArray(AddressOf GetVariableDeclarator)
+                    declarators = declarationGroup.Cast(Of BoundLocalDeclaration).SelectAsArray(AddressOf GetVariableDeclarator)
 
                     ' The initializer we use for this group is the initializer attached to the last declaration in this declarator, as that's
                     ' where it will be parsed in an error case.
@@ -294,13 +294,13 @@ Namespace Microsoft.CodeAnalysis.Operations
                     End If
                 Else
                     Dim asNewDeclarations = DirectCast(first, BoundAsNewLocalDeclarations)
-                    singleDeclarations = asNewDeclarations.LocalDeclarations.SelectAsArray(AddressOf GetVariableDeclarator)
+                    declarators = asNewDeclarations.LocalDeclarations.SelectAsArray(AddressOf GetVariableDeclarator)
                     Dim initializerSyntax As AsClauseSyntax = DirectCast(asNewDeclarations.Syntax, VariableDeclaratorSyntax).AsClause
                     Dim initializerValue As IOperation = Create(asNewDeclarations.Initializer)
                     initializer = OperationFactory.CreateVariableInitializer(initializerSyntax, initializerValue, _semanticModel, isImplicit:=False)
                 End If
 
-                builder.Add(New VariableDeclaration(singleDeclarations,
+                builder.Add(New VariableDeclaration(declarators,
                                                          initializer,
                                                          _semanticModel,
                                                          declarationGroup.Key,