Recover better when a user uses commas in a for-statement instead of semicolons (#75632)

Author: CyrusNajmabadi

src/Compilers/CSharp/Portable/Parser/LanguageParser.cs

@@ -890,13 +890,20 @@ protected static SyntaxDiagnosticInfo MakeError(ErrorCode code, params object[]
             return new SyntaxDiagnosticInfo(code, args);
         }
 
-        protected TNode AddLeadingSkippedSyntax<TNode>(TNode node, GreenNode skippedSyntax) where TNode : CSharpSyntaxNode
+#nullable enable
+
+        protected TNode AddLeadingSkippedSyntax<TNode>(TNode node, GreenNode? skippedSyntax) where TNode : CSharpSyntaxNode
         {
+            if (skippedSyntax is null)
+                return node;
+
             var oldToken = node as SyntaxToken ?? node.GetFirstToken();
             var newToken = AddSkippedSyntax(oldToken, skippedSyntax, trailing: false);
             return SyntaxFirstTokenReplacer.Replace(node, oldToken, newToken, skippedSyntax.FullWidth);
         }
 
+#nullable disable
+
         protected void AddTrailingSkippedSyntax(SyntaxListBuilder list, GreenNode skippedSyntax)
         {
             list[list.Count - 1] = AddTrailingSkippedSyntax((CSharpSyntaxNode)list[list.Count - 1], skippedSyntax);

src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs

@@ -350,6 +350,9 @@ protected static void AssertContainedInDeclaratorArguments(SingleVariableDesigna
             Assert.True(decl.Ancestors().OfType<VariableDeclaratorSyntax>().First().ArgumentList.Contains(decl));
         }
 
+        protected static void AssertNotContainedInDeclaratorArguments(SingleVariableDesignationSyntax decl)
+            => Assert.Empty(decl.Ancestors().OfType<VariableDeclaratorSyntax>());
+
         protected static void AssertContainedInDeclaratorArguments(params SingleVariableDesignationSyntax[] decls)
         {
             foreach (var decl in decls)
@@ -358,6 +361,12 @@ protected static void AssertContainedInDeclaratorArguments(params SingleVariable
             }
         }
 
+        protected static void AssertNotContainedInDeclaratorArguments(params SingleVariableDesignationSyntax[] decls)
+        {
+            foreach (var decl in decls)
+                AssertNotContainedInDeclaratorArguments(decl);
+        }
+
         protected static void VerifyModelNotSupported(
             SemanticModel model,
             SingleVariableDesignationSyntax designation,

src/Compilers/CSharp/Test/Emit3/Semantics/OutVarTests.cs

@@ -2678,7 +2678,7 @@ static void Main(string[] args)
 
         [CompilerTrait(CompilerFeature.IOperation)]
         [Fact, WorkItem(17602, "https://github.com/dotnet/roslyn/issues/17602")]
-        public void IForLoopStatement_InvalidExpression()
+        public void IForLoopStatement_InvalidExpression1()
         {
             string source = @"
 class C
@@ -2691,45 +2691,65 @@ static void Main(string[] args)
     }
 }
 ";
-            string expectedOperationTree = @"
-IForLoopOperation (LoopKind.For, Continue Label Id: 0, Exit Label Id: 1) (OperationKind.Loop, Type: null, IsInvalid) (Syntax: 'for (int k  ... 100, j > 5;')
-  Locals: Local_1: System.Int32 k
-    Local_2: System.Int32 j
-  Condition: 
-    IBinaryOperation (BinaryOperatorKind.LessThan) (OperationKind.Binary, Type: System.Boolean, IsInvalid) (Syntax: 'k < 100')
-      Left: 
-        ILocalReferenceOperation: k (OperationKind.LocalReference, Type: System.Int32) (Syntax: 'k')
-      Right: 
-        ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 100, IsInvalid) (Syntax: '100')
-  Before:
-      IVariableDeclarationGroupOperation (1 declarations) (OperationKind.VariableDeclarationGroup, Type: null, IsImplicit) (Syntax: 'int k = 0, j = 0')
-        IVariableDeclarationOperation (2 declarators) (OperationKind.VariableDeclaration, Type: null) (Syntax: 'int k = 0, j = 0')
-          Declarators:
-              IVariableDeclaratorOperation (Symbol: System.Int32 k) (OperationKind.VariableDeclarator, Type: null) (Syntax: 'k = 0')
-                Initializer: 
-                  IVariableInitializerOperation (OperationKind.VariableInitializer, Type: null) (Syntax: '= 0')
-                    ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 0) (Syntax: '0')
-              IVariableDeclaratorOperation (Symbol: System.Int32 j) (OperationKind.VariableDeclarator, Type: null) (Syntax: 'j = 0')
-                Initializer: 
-                  IVariableInitializerOperation (OperationKind.VariableInitializer, Type: null) (Syntax: '= 0')
-                    ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 0) (Syntax: '0')
-          Initializer: 
-            null
-  AtLoopBottom:
-      IExpressionStatementOperation (OperationKind.ExpressionStatement, Type: null, IsInvalid, IsImplicit) (Syntax: '')
-        Expression: 
-          IInvalidOperation (OperationKind.Invalid, Type: null, IsInvalid) (Syntax: '')
-            Children(0)
-      IExpressionStatementOperation (OperationKind.ExpressionStatement, Type: null, IsInvalid, IsImplicit) (Syntax: 'j > 5')
-        Expression: 
-          IBinaryOperation (BinaryOperatorKind.GreaterThan) (OperationKind.Binary, Type: System.Boolean, IsInvalid) (Syntax: 'j > 5')
-            Left: 
-              ILocalReferenceOperation: j (OperationKind.LocalReference, Type: System.Int32, IsInvalid) (Syntax: 'j')
-            Right: 
-              ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 5, IsInvalid) (Syntax: '5')
-  Body: 
-    IEmptyOperation (OperationKind.Empty, Type: null, IsInvalid) (Syntax: ';')
+            var tree = GetOperationTreeForTest<ForStatementSyntax>(source);
+            Assert.Null(tree);
+        }
+
+        [CompilerTrait(CompilerFeature.IOperation)]
+        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/17602")]
+        public void IForLoopStatement_InvalidExpression2()
+        {
+            string source = @"
+class C
+{
+    static void Main(string[] args)
+    {
+        /*<bind>*/for (int k = 0, j = 0; k < 100, j > 5; k++)
+        {
+        }/*</bind>*/
+    }
+}
 ";
+            string expectedOperationTree = """
+                IForLoopOperation (LoopKind.For, Continue Label Id: 0, Exit Label Id: 1) (OperationKind.Loop, Type: null, IsInvalid) (Syntax: 'for (int k  ... }')
+                      Locals: Local_1: System.Int32 k
+                        Local_2: System.Int32 j
+                      Condition:
+                        IBinaryOperation (BinaryOperatorKind.LessThan) (OperationKind.Binary, Type: System.Boolean, IsInvalid) (Syntax: 'k < 100')
+                          Left:
+                            ILocalReferenceOperation: k (OperationKind.LocalReference, Type: System.Int32) (Syntax: 'k')
+                          Right:
+                            ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 100, IsInvalid) (Syntax: '100')
+                      Before:
+                          IVariableDeclarationGroupOperation (1 declarations) (OperationKind.VariableDeclarationGroup, Type: null, IsImplicit) (Syntax: 'int k = 0, j = 0')
+                            IVariableDeclarationOperation (2 declarators) (OperationKind.VariableDeclaration, Type: null) (Syntax: 'int k = 0, j = 0')
+                              Declarators:
+                                  IVariableDeclaratorOperation (Symbol: System.Int32 k) (OperationKind.VariableDeclarator, Type: null) (Syntax: 'k = 0')
+                                    Initializer:
+                                      IVariableInitializerOperation (OperationKind.VariableInitializer, Type: null) (Syntax: '= 0')
+                                        ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 0) (Syntax: '0')
+                                  IVariableDeclaratorOperation (Symbol: System.Int32 j) (OperationKind.VariableDeclarator, Type: null) (Syntax: 'j = 0')
+                                    Initializer:
+                                      IVariableInitializerOperation (OperationKind.VariableInitializer, Type: null) (Syntax: '= 0')
+                                        ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 0) (Syntax: '0')
+                              Initializer:
+                                null
+                      AtLoopBottom:
+                          IExpressionStatementOperation (OperationKind.ExpressionStatement, Type: null, IsInvalid, IsImplicit) (Syntax: 'j > 5')
+                            Expression:
+                              IBinaryOperation (BinaryOperatorKind.GreaterThan) (OperationKind.Binary, Type: System.Boolean, IsInvalid) (Syntax: 'j > 5')
+                                Left:
+                                  ILocalReferenceOperation: j (OperationKind.LocalReference, Type: System.Int32, IsInvalid) (Syntax: 'j')
+                                Right:
+                                  ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 5, IsInvalid) (Syntax: '5')
+                          IExpressionStatementOperation (OperationKind.ExpressionStatement, Type: null, IsImplicit) (Syntax: 'k++')
+                            Expression:
+                              IIncrementOrDecrementOperation (Postfix) (OperationKind.Increment, Type: System.Int32) (Syntax: 'k++')
+                                Target:
+                                  ILocalReferenceOperation: k (OperationKind.LocalReference, Type: System.Int32) (Syntax: 'k')
+                      Body:
+                        IBlockOperation (0 statements) (OperationKind.Block, Type: null) (Syntax: '{ ... }')
+                """;
             VerifyOperationTreeForTest<ForStatementSyntax>(source, expectedOperationTree);
         }
 

src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTestBase.cs

@@ -2,9 +2,6 @@
 // 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 Microsoft.CodeAnalysis.Test.Utilities;
 using Xunit;
 
 namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Semantics
@@ -27,16 +24,16 @@ static void Main(string[] args)
     }
 }
 ";
-            CreateCompilation(text).
-                VerifyDiagnostics(
-                    Diagnostic(ErrorCode.ERR_SemicolonExpected, ","),
-                    Diagnostic(ErrorCode.ERR_InvalidExprTerm, ",").WithArguments(","),
-                    Diagnostic(ErrorCode.ERR_CloseParenExpected, ";"),
-                    Diagnostic(ErrorCode.ERR_SemicolonExpected, ")"),
-                    Diagnostic(ErrorCode.ERR_RbraceExpected, ")"),
-                    Diagnostic(ErrorCode.ERR_IllegalStatement, "j > 5"),
-                    Diagnostic(ErrorCode.ERR_NameNotInContext, "k").WithArguments("k")
-                );
+            CreateCompilation(text).VerifyDiagnostics(
+                // (6,39): error CS1002: ; expected
+                //         for (int k = 0, j = 0; k < 100, j > 5; k++)
+                Diagnostic(ErrorCode.ERR_SemicolonExpected, ",").WithLocation(6, 39),
+                // (6,46): error CS1003: Syntax error, ',' expected
+                //         for (int k = 0, j = 0; k < 100, j > 5; k++)
+                Diagnostic(ErrorCode.ERR_SyntaxError, ";").WithArguments(",").WithLocation(6, 46),
+                // (6,41): error CS0201: Only assignment, call, increment, decrement, await, and new object expressions can be used as a statement
+                //         for (int k = 0, j = 0; k < 100, j > 5; k++)
+                Diagnostic(ErrorCode.ERR_IllegalStatement, "j > 5").WithLocation(6, 41));
         }
 
         // Condition expression must be bool type
@@ -94,11 +91,16 @@ static void Main(string[] args)
     }
 }
 ";
-            CreateCompilation(text).
-                VerifyDiagnostics(
-                    Diagnostic(ErrorCode.ERR_CloseParenExpected, ";"),
-                    Diagnostic(ErrorCode.ERR_RbraceExpected, ")")
-                );
+            CreateCompilation(text).VerifyDiagnostics(
+                // (6,34): error CS1525: Invalid expression term ';'
+                //         for (int i = 10; i < 100;;);
+                Diagnostic(ErrorCode.ERR_InvalidExprTerm, ";").WithArguments(";").WithLocation(6, 34),
+                // (6,34): error CS1003: Syntax error, ',' expected
+                //         for (int i = 10; i < 100;;);
+                Diagnostic(ErrorCode.ERR_SyntaxError, ";").WithArguments(",").WithLocation(6, 34),
+                // (6,35): error CS1525: Invalid expression term ')'
+                //         for (int i = 10; i < 100;;);
+                Diagnostic(ErrorCode.ERR_InvalidExprTerm, ")").WithArguments(")").WithLocation(6, 35));
 
             text =
 @"