Skip to content

Commit

Permalink
Recover better when a user uses commas in a for-statement instead of …
Browse files Browse the repository at this point in the history 
…semicolons (#75632)
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi authored Nov 14, 2024
2 parents 29e09e0 + 857ad2c commit feb1350
Show file tree
Hide file tree
Showing 11 changed files with 900 additions and 288 deletions.
206 changes: 119 additions & 87 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
View file

Large diffs are not rendered by default.

9 changes: 8 additions & 1 deletion src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
170 changes: 109 additions & 61 deletions src/Compilers/CSharp/Test/Emit3/Semantics/OutVarTests.cs
View file

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down
169 changes: 109 additions & 60 deletions src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTests_Scope.cs
View file

Large diffs are not rendered by default.

98 changes: 59 additions & 39 deletions src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IForLoopStatement.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
}

Expand Down
38 changes: 20 additions & 18 deletions src/Compilers/CSharp/Test/Semantic/Semantics/ForLoopErrorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 =
@"
Expand Down
Loading

0 comments on commit feb1350

Please sign in to comment.