Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 22 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
214 changes: 129 additions & 85 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs

Large diffs are not rendered by default.

9 changes: 8 additions & 1 deletion src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs
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

Large diffs are not rendered by default.

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

Large diffs are not rendered by default.

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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no tree produced now since hte start/end of hte span is not validly containing the for-statement. it worked before because parsing was so thrown off it through the for loop was terminated.

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
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