From 15d2fb214e9ce310d6fac8291a6de136d52c1215 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 25 Oct 2024 11:50:57 -0700 Subject: [PATCH 01/18] Recover better when a user uses commas in a for-statement instead of semicolons --- .../CSharp/Portable/Parser/LanguageParser.cs | 85 ++-- .../Syntax/Parsing/ForStatementParsingTest.cs | 377 ++++++++++++++++++ 2 files changed, 435 insertions(+), 27 deletions(-) create mode 100644 src/Compilers/CSharp/Test/Syntax/Parsing/ForStatementParsingTest.cs diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index f3db0e485c25..753c4243e005 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -5029,6 +5029,19 @@ private void ParseVariableDeclarators( } else if (this.CurrentToken.Kind == SyntaxKind.CommaToken) { + // If we see `for (int i = 0, j < ...` then we do not want to consume j as the next declarator. + // + // Legal forms here are `for (int i = 0, j; ...` or `for (int i = 0, j = ...` or `for (int i = 0, + // j). Anything else we'll treat as as more likely to be the following + if (flags.HasFlag(VariableFlags.ForStatement)) + { + if (!IsTrueIdentifier(this.PeekToken(1)) || + this.PeekToken(2).Kind is not (SyntaxKind.SemicolonToken or SyntaxKind.EqualsToken or SyntaxKind.CloseParenToken)) + { + break; + } + } + variables.AddSeparator(this.EatToken(SyntaxKind.CommaToken)); variables.Add( this.ParseVariableDeclarator( @@ -5062,9 +5075,11 @@ private PostSkipAction SkipBadVariableListTokens(SeparatedSyntaxListBuilder var saveTerm = _termState; _termState |= TerminatorState.IsEndOfFixedStatement; - var decl = ParseParenthesizedVariableDeclaration(); + var decl = ParseParenthesizedVariableDeclaration(VariableFlags.None); _termState = saveTerm; return _syntaxFactory.FixedStatement( @@ -9159,9 +9174,8 @@ private ForStatementSyntax ParseForStatement(SyntaxList att var saveTerm = _termState; _termState |= TerminatorState.IsEndOfForStatementArgument; - var resetPoint = this.GetResetPoint(); + using var resetPoint = this.GetDisposableResetPoint(resetOnDispose: false); var initializers = default(SeparatedSyntaxList); - var incrementors = default(SeparatedSyntaxList); try { // Here can be either a declaration or an expression statement list. Scan @@ -9180,7 +9194,7 @@ private ForStatementSyntax ParseForStatement(SyntaxList att { this.EatToken(); isDeclaration = ScanType() != ScanTypeFlags.NotType && this.CurrentToken.Kind == SyntaxKind.IdentifierToken; - this.Reset(ref resetPoint); + resetPoint.Reset(); } haveScopedKeyword = isDeclaration; @@ -9195,8 +9209,7 @@ private ForStatementSyntax ParseForStatement(SyntaxList att isDeclaration = !this.IsQueryExpression(mayBeVariableDeclaration: true, mayBeMemberDeclaration: false) && this.ScanType() != ScanTypeFlags.NotType && this.IsTrueIdentifier(); - - this.Reset(ref resetPoint); + resetPoint.Reset(); } if (isDeclaration) @@ -9208,7 +9221,7 @@ private ForStatementSyntax ParseForStatement(SyntaxList att scopedKeyword = EatContextualToken(SyntaxKind.ScopedKeyword); } - decl = ParseParenthesizedVariableDeclaration(); + decl = ParseParenthesizedVariableDeclaration(VariableFlags.ForStatement); var declType = decl.Type; @@ -9228,18 +9241,17 @@ private ForStatementSyntax ParseForStatement(SyntaxList att initializers = this.ParseForStatementExpressionList(ref openParen); } - var semi = this.EatToken(SyntaxKind.SemicolonToken); - ExpressionSyntax condition = null; - if (this.CurrentToken.Kind != SyntaxKind.SemicolonToken) - { - condition = this.ParseExpressionCore(); - } + var semi1 = eatCommaOrSemicolon(); - var semi2 = this.EatToken(SyntaxKind.SemicolonToken); - if (this.CurrentToken.Kind != SyntaxKind.CloseParenToken) - { - incrementors = this.ParseForStatementExpressionList(ref semi2); - } + var condition = this.CurrentToken.Kind is not SyntaxKind.SemicolonToken and not SyntaxKind.CommaToken + ? this.ParseExpressionCore() + : null; + + var semi2 = eatCommaOrSemicolon(); + + var incrementors = this.CurrentToken.Kind != SyntaxKind.CloseParenToken + ? this.ParseForStatementExpressionList(ref semi2) + : default; return _syntaxFactory.ForStatement( attributes, @@ -9247,7 +9259,7 @@ private ForStatementSyntax ParseForStatement(SyntaxList att openParen, decl, initializers, - semi, + semi1, condition, semi2, incrementors, @@ -9257,7 +9269,23 @@ private ForStatementSyntax ParseForStatement(SyntaxList att finally { _termState = saveTerm; - this.Release(ref resetPoint); + } + + SyntaxToken eatCommaOrSemicolon() + { + if (this.CurrentToken.Kind is SyntaxKind.CommaToken) + { + // Still parse out a semicolon so we give an appropriate error that the comma is unexpected, and so + // we have the correct token kind. + var semicolon = this.EatToken(SyntaxKind.SemicolonToken); + + // Now skip past the comma + return AddTrailingSkippedSyntax(semicolon, this.EatToken()); + } + else + { + return this.EatToken(SyntaxKind.SemicolonToken); + } } } @@ -9902,7 +9930,7 @@ private void ParseUsingExpression(ref VariableDeclarationSyntax declaration, ref if (scopedKeyword != null) { - declaration = ParseParenthesizedVariableDeclaration(); + declaration = ParseParenthesizedVariableDeclaration(VariableFlags.None); declaration = declaration.Update(_syntaxFactory.ScopedType(scopedKeyword, declaration.Type), declaration.Variables); return; } @@ -9936,14 +9964,14 @@ private void ParseUsingExpression(ref VariableDeclarationSyntax declaration, ref case SyntaxKind.CommaToken: case SyntaxKind.CloseParenToken: this.Reset(ref resetPoint); - declaration = ParseParenthesizedVariableDeclaration(); + declaration = ParseParenthesizedVariableDeclaration(VariableFlags.None); break; case SyntaxKind.EqualsToken: // Parse it as a decl. If the next token is a : and only one variable was parsed, // convert the whole thing to ?: expression. this.Reset(ref resetPoint); - declaration = ParseParenthesizedVariableDeclaration(); + declaration = ParseParenthesizedVariableDeclaration(VariableFlags.None); // We may have non-nullable types in error scenarios. if (this.CurrentToken.Kind == SyntaxKind.ColonToken && @@ -9964,7 +9992,7 @@ private void ParseUsingExpression(ref VariableDeclarationSyntax declaration, ref else if (IsUsingStatementVariableDeclaration(st)) { this.Reset(ref resetPoint); - declaration = ParseParenthesizedVariableDeclaration(); + declaration = ParseParenthesizedVariableDeclaration(VariableFlags.None); } else { @@ -10055,6 +10083,7 @@ private StatementSyntax ParseLocalDeclarationStatement(SyntaxList - private VariableDeclarationSyntax ParseParenthesizedVariableDeclaration() + private VariableDeclarationSyntax ParseParenthesizedVariableDeclaration(VariableFlags initialFlags) { var variables = _pool.AllocateSeparated(); ParseLocalDeclaration( @@ -10231,6 +10260,7 @@ private VariableDeclarationSyntax ParseParenthesizedVariableDeclaration() stopOnCloseParen: true, attributes: default, mods: default, + initialFlags, out var type, out var localFunction); Debug.Assert(localFunction == null); @@ -10245,12 +10275,13 @@ private void ParseLocalDeclaration( bool stopOnCloseParen, SyntaxList attributes, SyntaxList mods, + VariableFlags initialFlags, out TypeSyntax type, out LocalFunctionStatementSyntax localFunction) { type = allowLocalFunctions ? ParseReturnType() : this.ParseType(); - VariableFlags flags = VariableFlags.LocalOrField; + VariableFlags flags = initialFlags | VariableFlags.LocalOrField; if (mods.Any((int)SyntaxKind.ConstKeyword)) { flags |= VariableFlags.Const; diff --git a/src/Compilers/CSharp/Test/Syntax/Parsing/ForStatementParsingTest.cs b/src/Compilers/CSharp/Test/Syntax/Parsing/ForStatementParsingTest.cs new file mode 100644 index 000000000000..a2feede84454 --- /dev/null +++ b/src/Compilers/CSharp/Test/Syntax/Parsing/ForStatementParsingTest.cs @@ -0,0 +1,377 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Roslyn.Test.Utilities; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Parsing; + +public sealed class ForStatementParsingTest(ITestOutputHelper output) : ParsingTests(output) +{ + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66522")] + public void TestCommaSeparators1() + { + UsingStatement("for (int i = 0, j = 0; i < 10; i++) ;"); + + N(SyntaxKind.ForStatement); + { + N(SyntaxKind.ForKeyword); + N(SyntaxKind.OpenParenToken); + N(SyntaxKind.VariableDeclaration); + { + N(SyntaxKind.PredefinedType); + { + N(SyntaxKind.IntKeyword); + } + N(SyntaxKind.VariableDeclarator); + { + N(SyntaxKind.IdentifierToken, "i"); + N(SyntaxKind.EqualsValueClause); + { + N(SyntaxKind.EqualsToken); + N(SyntaxKind.NumericLiteralExpression); + { + N(SyntaxKind.NumericLiteralToken, "0"); + } + } + } + N(SyntaxKind.CommaToken); + N(SyntaxKind.VariableDeclarator); + { + N(SyntaxKind.IdentifierToken, "j"); + N(SyntaxKind.EqualsValueClause); + { + N(SyntaxKind.EqualsToken); + N(SyntaxKind.NumericLiteralExpression); + { + N(SyntaxKind.NumericLiteralToken, "0"); + } + } + } + } + N(SyntaxKind.SemicolonToken); + N(SyntaxKind.LessThanExpression); + { + N(SyntaxKind.IdentifierName); + { + N(SyntaxKind.IdentifierToken, "i"); + } + N(SyntaxKind.LessThanToken); + N(SyntaxKind.NumericLiteralExpression); + { + N(SyntaxKind.NumericLiteralToken, "10"); + } + } + N(SyntaxKind.SemicolonToken); + N(SyntaxKind.PostIncrementExpression); + { + N(SyntaxKind.IdentifierName); + { + N(SyntaxKind.IdentifierToken, "i"); + } + N(SyntaxKind.PlusPlusToken); + } + N(SyntaxKind.CloseParenToken); + N(SyntaxKind.EmptyStatement); + { + N(SyntaxKind.SemicolonToken); + } + } + EOF(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66522")] + public void TestCommaSeparators2() + { + UsingStatement("for (int i = 0, i < 10; i++) ;", + // (1,15): error CS1002: ; expected + // for (int i = 0, i < 10; i++) ; + Diagnostic(ErrorCode.ERR_SemicolonExpected, ",").WithLocation(1, 15)); + + N(SyntaxKind.ForStatement); + { + N(SyntaxKind.ForKeyword); + N(SyntaxKind.OpenParenToken); + N(SyntaxKind.VariableDeclaration); + { + N(SyntaxKind.PredefinedType); + { + N(SyntaxKind.IntKeyword); + } + N(SyntaxKind.VariableDeclarator); + { + N(SyntaxKind.IdentifierToken, "i"); + N(SyntaxKind.EqualsValueClause); + { + N(SyntaxKind.EqualsToken); + N(SyntaxKind.NumericLiteralExpression); + { + N(SyntaxKind.NumericLiteralToken, "0"); + } + } + } + } + M(SyntaxKind.SemicolonToken); + N(SyntaxKind.LessThanExpression); + { + N(SyntaxKind.IdentifierName); + { + N(SyntaxKind.IdentifierToken, "i"); + } + N(SyntaxKind.LessThanToken); + N(SyntaxKind.NumericLiteralExpression); + { + N(SyntaxKind.NumericLiteralToken, "10"); + } + } + N(SyntaxKind.SemicolonToken); + N(SyntaxKind.PostIncrementExpression); + { + N(SyntaxKind.IdentifierName); + { + N(SyntaxKind.IdentifierToken, "i"); + } + N(SyntaxKind.PlusPlusToken); + } + N(SyntaxKind.CloseParenToken); + N(SyntaxKind.EmptyStatement); + { + N(SyntaxKind.SemicolonToken); + } + } + EOF(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66522")] + public void TestCommaSeparators3() + { + UsingStatement("for (int i = 0, i < 10, i++) ;", + // (1,15): error CS1002: ; expected + // for (int i = 0, i < 10, i++) ; + Diagnostic(ErrorCode.ERR_SemicolonExpected, ",").WithLocation(1, 15), + // (1,23): error CS1002: ; expected + // for (int i = 0, i < 10, i++) ; + Diagnostic(ErrorCode.ERR_SemicolonExpected, ",").WithLocation(1, 23)); + + N(SyntaxKind.ForStatement); + { + N(SyntaxKind.ForKeyword); + N(SyntaxKind.OpenParenToken); + N(SyntaxKind.VariableDeclaration); + { + N(SyntaxKind.PredefinedType); + { + N(SyntaxKind.IntKeyword); + } + N(SyntaxKind.VariableDeclarator); + { + N(SyntaxKind.IdentifierToken, "i"); + N(SyntaxKind.EqualsValueClause); + { + N(SyntaxKind.EqualsToken); + N(SyntaxKind.NumericLiteralExpression); + { + N(SyntaxKind.NumericLiteralToken, "0"); + } + } + } + } + M(SyntaxKind.SemicolonToken); + N(SyntaxKind.LessThanExpression); + { + N(SyntaxKind.IdentifierName); + { + N(SyntaxKind.IdentifierToken, "i"); + } + N(SyntaxKind.LessThanToken); + N(SyntaxKind.NumericLiteralExpression); + { + N(SyntaxKind.NumericLiteralToken, "10"); + } + } + M(SyntaxKind.SemicolonToken); + N(SyntaxKind.PostIncrementExpression); + { + N(SyntaxKind.IdentifierName); + { + N(SyntaxKind.IdentifierToken, "i"); + } + N(SyntaxKind.PlusPlusToken); + } + N(SyntaxKind.CloseParenToken); + N(SyntaxKind.EmptyStatement); + { + N(SyntaxKind.SemicolonToken); + } + } + EOF(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66522")] + public void TestCommaSeparators4() + { + UsingStatement("for (int i = 0, i) ;", + // (1,18): error CS1002: ; expected + // for (int i = 0, i) ; + Diagnostic(ErrorCode.ERR_SemicolonExpected, ")").WithLocation(1, 18), + // (1,18): error CS1525: Invalid expression term ')' + // for (int i = 0, i) ; + Diagnostic(ErrorCode.ERR_InvalidExprTerm, ")").WithArguments(")").WithLocation(1, 18), + // (1,18): error CS1002: ; expected + // for (int i = 0, i) ; + Diagnostic(ErrorCode.ERR_SemicolonExpected, ")").WithLocation(1, 18)); + + N(SyntaxKind.ForStatement); + { + N(SyntaxKind.ForKeyword); + N(SyntaxKind.OpenParenToken); + N(SyntaxKind.VariableDeclaration); + { + N(SyntaxKind.PredefinedType); + { + N(SyntaxKind.IntKeyword); + } + N(SyntaxKind.VariableDeclarator); + { + N(SyntaxKind.IdentifierToken, "i"); + N(SyntaxKind.EqualsValueClause); + { + N(SyntaxKind.EqualsToken); + N(SyntaxKind.NumericLiteralExpression); + { + N(SyntaxKind.NumericLiteralToken, "0"); + } + } + } + N(SyntaxKind.CommaToken); + N(SyntaxKind.VariableDeclarator); + { + N(SyntaxKind.IdentifierToken, "i"); + } + } + M(SyntaxKind.SemicolonToken); + M(SyntaxKind.IdentifierName); + { + M(SyntaxKind.IdentifierToken); + } + M(SyntaxKind.SemicolonToken); + N(SyntaxKind.CloseParenToken); + N(SyntaxKind.EmptyStatement); + { + N(SyntaxKind.SemicolonToken); + } + } + EOF(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66522")] + public void TestCommaSeparators5() + { + UsingStatement("for (int i = 0,,) ;", + // (1,15): error CS1002: ; expected + // for (int i = 0,,) ; + Diagnostic(ErrorCode.ERR_SemicolonExpected, ",").WithLocation(1, 15), + // (1,16): error CS1002: ; expected + // for (int i = 0,,) ; + Diagnostic(ErrorCode.ERR_SemicolonExpected, ",").WithLocation(1, 16)); + + N(SyntaxKind.ForStatement); + { + N(SyntaxKind.ForKeyword); + N(SyntaxKind.OpenParenToken); + N(SyntaxKind.VariableDeclaration); + { + N(SyntaxKind.PredefinedType); + { + N(SyntaxKind.IntKeyword); + } + N(SyntaxKind.VariableDeclarator); + { + N(SyntaxKind.IdentifierToken, "i"); + N(SyntaxKind.EqualsValueClause); + { + N(SyntaxKind.EqualsToken); + N(SyntaxKind.NumericLiteralExpression); + { + N(SyntaxKind.NumericLiteralToken, "0"); + } + } + } + } + M(SyntaxKind.SemicolonToken); + M(SyntaxKind.SemicolonToken); + N(SyntaxKind.CloseParenToken); + N(SyntaxKind.EmptyStatement); + { + N(SyntaxKind.SemicolonToken); + } + } + EOF(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66522")] + public void TestCommaSeparators6() + { + UsingStatement("for (int i = 0, j; i < 10; i++) ;"); + + N(SyntaxKind.ForStatement); + { + N(SyntaxKind.ForKeyword); + N(SyntaxKind.OpenParenToken); + N(SyntaxKind.VariableDeclaration); + { + N(SyntaxKind.PredefinedType); + { + N(SyntaxKind.IntKeyword); + } + N(SyntaxKind.VariableDeclarator); + { + N(SyntaxKind.IdentifierToken, "i"); + N(SyntaxKind.EqualsValueClause); + { + N(SyntaxKind.EqualsToken); + N(SyntaxKind.NumericLiteralExpression); + { + N(SyntaxKind.NumericLiteralToken, "0"); + } + } + } + N(SyntaxKind.CommaToken); + N(SyntaxKind.VariableDeclarator); + { + N(SyntaxKind.IdentifierToken, "j"); + } + } + N(SyntaxKind.SemicolonToken); + N(SyntaxKind.LessThanExpression); + { + N(SyntaxKind.IdentifierName); + { + N(SyntaxKind.IdentifierToken, "i"); + } + N(SyntaxKind.LessThanToken); + N(SyntaxKind.NumericLiteralExpression); + { + N(SyntaxKind.NumericLiteralToken, "10"); + } + } + N(SyntaxKind.SemicolonToken); + N(SyntaxKind.PostIncrementExpression); + { + N(SyntaxKind.IdentifierName); + { + N(SyntaxKind.IdentifierToken, "i"); + } + N(SyntaxKind.PlusPlusToken); + } + N(SyntaxKind.CloseParenToken); + N(SyntaxKind.EmptyStatement); + { + N(SyntaxKind.SemicolonToken); + } + } + EOF(); + } +} From 2f1c5c9421c512c485f9059772e51bb4ff76d4a8 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 25 Oct 2024 11:57:50 -0700 Subject: [PATCH 02/18] Simplify --- .../CSharp/Portable/Parser/LanguageParser.cs | 77 ++++++++++--------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 91c4ba39698a..73b19e8aeb4d 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -9136,19 +9136,45 @@ private ForStatementSyntax ParseForStatement(SyntaxList att { Debug.Assert(this.CurrentToken.Kind == SyntaxKind.ForKeyword); + var saveTerm = _termState; + _termState |= TerminatorState.IsEndOfForStatementArgument; + var forToken = this.EatToken(SyntaxKind.ForKeyword); var openParen = this.EatToken(SyntaxKind.OpenParenToken); - var saveTerm = _termState; - _termState |= TerminatorState.IsEndOfForStatementArgument; + var (decl, initializers) = eatVariableDeclarationOrInitializers(); - using var resetPoint = this.GetDisposableResetPoint(resetOnDispose: false); - var initializers = default(SeparatedSyntaxList); - try + var firstSemicolonToken = eatCommaOrSemicolon(); + var condition = this.CurrentToken.Kind is not SyntaxKind.SemicolonToken and not SyntaxKind.CommaToken + ? this.ParseExpressionCore() + : null; + var secondSemicolonToken = eatCommaOrSemicolon(); + + var forStatement = _syntaxFactory.ForStatement( + attributes, + forToken, + openParen, + decl, + initializers, + firstSemicolonToken, + condition, + secondSemicolonToken, + incrementors: this.CurrentToken.Kind != SyntaxKind.CloseParenToken + ? this.ParseForStatementExpressionList(ref secondSemicolonToken) + : default, + this.EatToken(SyntaxKind.CloseParenToken), + ParseEmbeddedStatement()); + + _termState = saveTerm; + + return forStatement; + + (VariableDeclarationSyntax variableDeclaration, SeparatedSyntaxList) eatVariableDeclarationOrInitializers() { + using var resetPoint = this.GetDisposableResetPoint(resetOnDispose: false); + // Here can be either a declaration or an expression statement list. Scan // for a declaration first. - VariableDeclarationSyntax decl = null; bool isDeclaration = false; bool haveScopedKeyword = false; @@ -9189,7 +9215,7 @@ private ForStatementSyntax ParseForStatement(SyntaxList att scopedKeyword = EatContextualToken(SyntaxKind.ScopedKeyword); } - decl = ParseParenthesizedVariableDeclaration(VariableFlags.ForStatement); + var decl = ParseParenthesizedVariableDeclaration(VariableFlags.ForStatement); var declType = decl.Type; @@ -9202,41 +9228,18 @@ private ForStatementSyntax ParseForStatement(SyntaxList att { decl = decl.Update(declType, decl.Variables); } + + return (decl, default); } else if (this.CurrentToken.Kind != SyntaxKind.SemicolonToken) { // Not a type followed by an identifier, so it must be an expression list. - initializers = this.ParseForStatementExpressionList(ref openParen); + return (null, this.ParseForStatementExpressionList(ref openParen)); + } + else + { + return default; } - - var semi1 = eatCommaOrSemicolon(); - - var condition = this.CurrentToken.Kind is not SyntaxKind.SemicolonToken and not SyntaxKind.CommaToken - ? this.ParseExpressionCore() - : null; - - var semi2 = eatCommaOrSemicolon(); - - var incrementors = this.CurrentToken.Kind != SyntaxKind.CloseParenToken - ? this.ParseForStatementExpressionList(ref semi2) - : default; - - return _syntaxFactory.ForStatement( - attributes, - forToken, - openParen, - decl, - initializers, - semi1, - condition, - semi2, - incrementors, - this.EatToken(SyntaxKind.CloseParenToken), - ParseEmbeddedStatement()); - } - finally - { - _termState = saveTerm; } SyntaxToken eatCommaOrSemicolon() From d333f07bbe7e767df18ab9498491747ad20541b5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 25 Oct 2024 12:01:42 -0700 Subject: [PATCH 03/18] Simplify --- .../CSharp/Portable/Parser/LanguageParser.cs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 73b19e8aeb4d..d137c7f522e2 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -9141,24 +9141,22 @@ private ForStatementSyntax ParseForStatement(SyntaxList att var forToken = this.EatToken(SyntaxKind.ForKeyword); var openParen = this.EatToken(SyntaxKind.OpenParenToken); + var (variableDeclaration, initializers) = eatVariableDeclarationOrInitializers(); - var (decl, initializers) = eatVariableDeclarationOrInitializers(); - - var firstSemicolonToken = eatCommaOrSemicolon(); - var condition = this.CurrentToken.Kind is not SyntaxKind.SemicolonToken and not SyntaxKind.CommaToken - ? this.ParseExpressionCore() - : null; - var secondSemicolonToken = eatCommaOrSemicolon(); + // Pulled out as we need to track this when parsing incrementors to place skipped tokens. + SyntaxToken secondSemicolonToken; var forStatement = _syntaxFactory.ForStatement( attributes, forToken, openParen, - decl, + variableDeclaration, initializers, - firstSemicolonToken, - condition, - secondSemicolonToken, + firstSemicolonToken: eatCommaOrSemicolon(), + condition: this.CurrentToken.Kind is not SyntaxKind.SemicolonToken and not SyntaxKind.CommaToken + ? this.ParseExpressionCore() + : null, + secondSemicolonToken = eatCommaOrSemicolon(), incrementors: this.CurrentToken.Kind != SyntaxKind.CloseParenToken ? this.ParseForStatementExpressionList(ref secondSemicolonToken) : default, From b317e3136cde8afc4d27eef758d3c04076b79944 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 4 Nov 2024 13:14:48 -0800 Subject: [PATCH 04/18] Fixup tests --- .../CSharp/Portable/Parser/LanguageParser.cs | 15 +- .../CSharp/Portable/Parser/SyntaxParser.cs | 9 +- .../Semantics/PatternMatchingTestBase.cs | 9 + .../Semantics/PatternMatchingTests_Scope.cs | 163 ++++++++++++------ 4 files changed, 138 insertions(+), 58 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index d137c7f522e2..24e7222e612f 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -15,6 +15,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Syntax.InternalSyntax { + using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Syntax.InternalSyntax; internal sealed partial class LanguageParser : SyntaxParser @@ -9145,7 +9146,6 @@ private ForStatementSyntax ParseForStatement(SyntaxList att // Pulled out as we need to track this when parsing incrementors to place skipped tokens. SyntaxToken secondSemicolonToken; - var forStatement = _syntaxFactory.ForStatement( attributes, forToken, @@ -9160,7 +9160,7 @@ private ForStatementSyntax ParseForStatement(SyntaxList att incrementors: this.CurrentToken.Kind != SyntaxKind.CloseParenToken ? this.ParseForStatementExpressionList(ref secondSemicolonToken) : default, - this.EatToken(SyntaxKind.CloseParenToken), + eatUnexpectedTokensAndCloseParenToken(), ParseEmbeddedStatement()); _termState = saveTerm; @@ -9256,6 +9256,17 @@ SyntaxToken eatCommaOrSemicolon() return this.EatToken(SyntaxKind.SemicolonToken); } } + + SyntaxToken eatUnexpectedTokensAndCloseParenToken() + { + var skippedTokens = _pool.Allocate(); + + while (this.CurrentToken.Kind is SyntaxKind.SemicolonToken or SyntaxKind.CommaToken) + skippedTokens.Add(this.EatTokenWithPrejudice(SyntaxKind.CloseParenToken)); + + var result = this.EatToken(SyntaxKind.CloseParenToken); + return AddLeadingSkippedSyntax(result, _pool.ToTokenListAndFree(skippedTokens).Node); + } } private bool IsEndOfForStatementArgument() diff --git a/src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs b/src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs index 71b2a9c52bc3..4c9e5502030c 100644 --- a/src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs @@ -890,13 +890,20 @@ protected static SyntaxDiagnosticInfo MakeError(ErrorCode code, params object[] return new SyntaxDiagnosticInfo(code, args); } - protected TNode AddLeadingSkippedSyntax(TNode node, GreenNode skippedSyntax) where TNode : CSharpSyntaxNode +#nullable enable + + protected TNode AddLeadingSkippedSyntax(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); diff --git a/src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTestBase.cs b/src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTestBase.cs index 8d77f638ce69..d37a1bb88a5b 100644 --- a/src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTestBase.cs +++ b/src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTestBase.cs @@ -350,6 +350,9 @@ protected static void AssertContainedInDeclaratorArguments(SingleVariableDesigna Assert.True(decl.Ancestors().OfType().First().ArgumentList.Contains(decl)); } + protected static void AssertNotContainedInDeclaratorArguments(SingleVariableDesignationSyntax decl) + => Assert.Empty(decl.Ancestors().OfType()); + 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, diff --git a/src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTests_Scope.cs b/src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTests_Scope.cs index 3f5a3e860c02..7e5edc20234a 100644 --- a/src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTests_Scope.cs +++ b/src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTests_Scope.cs @@ -12394,48 +12394,81 @@ void Test14() } "; var compilation = CreateCompilation(source, options: TestOptions.DebugExe, parseOptions: TestOptions.Regular); - int[] exclude = new int[] { (int)ErrorCode.ERR_BadVarDecl, - (int)ErrorCode.ERR_SyntaxError, - (int)ErrorCode.ERR_UnexpectedToken, - (int)ErrorCode.WRN_UnreferencedVar, - (int)ErrorCode.ERR_UseDefViolation - }; + int[] exclude = [ + (int)ErrorCode.ERR_BadVarDecl, + (int)ErrorCode.ERR_SyntaxError, + (int)ErrorCode.ERR_UnexpectedToken, + (int)ErrorCode.WRN_UnreferencedVar, + (int)ErrorCode.ERR_UseDefViolation, + (int)ErrorCode.ERR_AbstractAndExtern, + (int)ErrorCode.ERR_SemicolonExpected, + (int)ErrorCode.ERR_CloseParenExpected, + ]; compilation.GetDiagnostics().Where(d => !exclude.Contains(d.Code)).Verify( - // (109,13): error CS1023: Embedded statement cannot be a declaration or labeled statement - // var y12 = 12; - Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "var y12 = 12;").WithLocation(109, 13), + // (12,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(12, 22), + // (22,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(22, 22), + // (33,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(33, 22), // (34,32): error CS0136: A local or parameter named 'x4' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(true is var x4 && x4) Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x4").WithArguments("x4").WithLocation(34, 32), + // (41,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(41, 22), // (42,20): error CS0841: Cannot use local variable 'x6' before it is declared // Dummy(x6 && true is var x6) Diagnostic(ErrorCode.ERR_VariableUsedBeforeDeclaration, "x6").WithArguments("x6").WithLocation(42, 20), + // (49,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(49, 22), // (53,17): error CS0136: A local or parameter named 'x7' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // var x7 = 12; Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x7").WithArguments("x7").WithLocation(53, 17), + // (60,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(60, 22), // (65,34): error CS0103: The name 'x8' does not exist in the current context // System.Console.WriteLine(x8); Diagnostic(ErrorCode.ERR_NameNotInContext, "x8").WithArguments("x8").WithLocation(65, 34), - // (65,9): warning CS0162: Unreachable code detected - // System.Console.WriteLine(x8); - Diagnostic(ErrorCode.WRN_UnreachableCode, "System").WithLocation(65, 9), + // (70,23): error CS0103: The name 'b1' does not exist in the current context + // for (bool a1, b1( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b1").WithArguments("b1").WithLocation(70, 23), + // (75,27): error CS0103: The name 'b2' does not exist in the current context + // for (bool a2, b2( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b2").WithArguments("b2").WithLocation(75, 27), // (76,36): error CS0136: A local or parameter named 'x9' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(true is var x9 && x9) // 2 Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x9").WithArguments("x9").WithLocation(76, 36), + // (84,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(84, 22), // (85,20): error CS0103: The name 'y10' does not exist in the current context // Dummy(y10 is var x10) Diagnostic(ErrorCode.ERR_NameNotInContext, "y10").WithArguments("y10").WithLocation(85, 20), + // (106,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(106, 22), // (107,20): error CS0103: The name 'y12' does not exist in the current context // Dummy(y12 is var x12) Diagnostic(ErrorCode.ERR_NameNotInContext, "y12").WithArguments("y12").WithLocation(107, 20), + // (109,13): error CS1023: Embedded statement cannot be a declaration or labeled statement + // var y12 = 12; + Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "var y12 = 12;").WithLocation(109, 13), // (109,17): warning CS0219: The variable 'y12' is assigned but its value is never used // var y12 = 12; Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "y12").WithArguments("y12").WithLocation(109, 17), + // (122,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(122, 22), // (124,29): error CS0128: A local variable or function named 'x14' is already defined in this scope // 2 is var x14, - Diagnostic(ErrorCode.ERR_LocalDuplicate, "x14").WithArguments("x14").WithLocation(124, 29) - ); + Diagnostic(ErrorCode.ERR_LocalDuplicate, "x14").WithArguments("x14").WithLocation(124, 29)); var tree = compilation.SyntaxTrees.Single(); var model = compilation.GetSemanticModel(tree); @@ -12443,39 +12476,39 @@ void Test14() var x1Decl = GetPatternDeclarations(tree, "x1").Single(); var x1Ref = GetReferences(tree, "x1").ToArray(); Assert.Equal(2, x1Ref.Length); - AssertContainedInDeclaratorArguments(x1Decl); + AssertNotContainedInDeclaratorArguments(x1Decl); VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x1Decl, x1Ref); var x2Decl = GetPatternDeclarations(tree, "x2").Single(); var x2Ref = GetReferences(tree, "x2").ToArray(); Assert.Equal(2, x2Ref.Length); - AssertContainedInDeclaratorArguments(x2Decl); + AssertNotContainedInDeclaratorArguments(x2Decl); VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x2Decl, x2Ref); var x4Decl = GetPatternDeclarations(tree, "x4").Single(); var x4Ref = GetReferences(tree, "x4").ToArray(); Assert.Equal(3, x4Ref.Length); - AssertContainedInDeclaratorArguments(x4Decl); + AssertNotContainedInDeclaratorArguments(x4Decl); VerifyNotAPatternLocal(model, x4Ref[0]); VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x4Decl, x4Ref[1], x4Ref[2]); var x6Decl = GetPatternDeclarations(tree, "x6").Single(); var x6Ref = GetReferences(tree, "x6").ToArray(); Assert.Equal(2, x6Ref.Length); - AssertContainedInDeclaratorArguments(x6Decl); + AssertNotContainedInDeclaratorArguments(x6Decl); VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x6Decl, x6Ref); var x7Decl = GetPatternDeclarations(tree, "x7").Single(); var x7Ref = GetReferences(tree, "x7").ToArray(); Assert.Equal(2, x7Ref.Length); - AssertContainedInDeclaratorArguments(x7Decl); + AssertNotContainedInDeclaratorArguments(x7Decl); VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x7Decl, x7Ref[0]); VerifyNotAPatternLocal(model, x7Ref[1]); var x8Decl = GetPatternDeclarations(tree, "x8").Single(); var x8Ref = GetReferences(tree, "x8").ToArray(); Assert.Equal(3, x8Ref.Length); - AssertContainedInDeclaratorArguments(x8Decl); + AssertNotContainedInDeclaratorArguments(x8Decl); VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x8Decl, x8Ref[0], x8Ref[1]); VerifyNotInScope(model, x8Ref[2]); @@ -12483,7 +12516,7 @@ void Test14() var x9Ref = GetReferences(tree, "x9").ToArray(); Assert.Equal(2, x9Decl.Length); Assert.Equal(4, x9Ref.Length); - AssertContainedInDeclaratorArguments(x9Decl); + AssertNotContainedInDeclaratorArguments(x9Decl); VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x9Decl[0], x9Ref[0], x9Ref[1]); VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x9Decl[1], x9Ref[2], x9Ref[3]); @@ -12499,7 +12532,7 @@ void Test14() var x14Ref = GetReferences(tree, "x14").ToArray(); Assert.Equal(2, x14Decl.Length); Assert.Equal(2, x14Ref.Length); - AssertContainedInDeclaratorArguments(x14Decl); + AssertNotContainedInDeclaratorArguments(x14Decl); VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x14Decl[0], x14Ref); VerifyModelForDeclarationOrVarPatternDuplicateInSameScope(model, x14Decl[1]); } @@ -12553,66 +12586,86 @@ void Test9() } "; var compilation = CreateCompilation(source, options: TestOptions.DebugExe, parseOptions: TestOptions.Regular); - int[] exclude = new int[] { (int)ErrorCode.ERR_BadVarDecl, - (int)ErrorCode.ERR_SyntaxError, - (int)ErrorCode.ERR_UnexpectedToken, - (int)ErrorCode.WRN_UnreferencedVar, - (int)ErrorCode.ERR_CloseParenExpected - }; + int[] exclude = [ + (int)ErrorCode.ERR_BadVarDecl, + (int)ErrorCode.ERR_SyntaxError, + (int)ErrorCode.ERR_UnexpectedToken, + (int)ErrorCode.WRN_UnreferencedVar, + (int)ErrorCode.ERR_CloseParenExpected, + (int)ErrorCode.ERR_SemicolonExpected, + ]; compilation.GetDiagnostics().Where(d => !exclude.Contains(d.Code)).Verify( - // (13,32): error CS0128: A local variable or function named 'x4' is already defined in this scope - // Dummy(true is var x4 && x4) - Diagnostic(ErrorCode.ERR_LocalDuplicate, "x4").WithArguments("x4").WithLocation(13, 32), - // (21,32): error CS0128: A local variable or function named 'x7' is already defined in this scope + // (31,41): error CS1513: } expected + // Dummy(true is var x8 && x8)) + Diagnostic(ErrorCode.ERR_RbraceExpected, ")").WithLocation(31, 41), + // (40,41): error CS1513: } expected + // Dummy(true is var x9 && x9)) + Diagnostic(ErrorCode.ERR_RbraceExpected, ")").WithLocation(40, 41), + // (12,22): error CS0841: Cannot use local variable 'x4' before it is declared + // for (bool d, x4( + Diagnostic(ErrorCode.ERR_VariableUsedBeforeDeclaration, "x4").WithArguments("x4").WithLocation(12, 22), + // (20,30): error CS0103: The name 'b' does not exist in the current context + // for (bool x7 = true, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(20, 30), + // (21,32): error CS0136: A local or parameter named 'x7' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(true is var x7 && x7) - Diagnostic(ErrorCode.ERR_LocalDuplicate, "x7").WithArguments("x7").WithLocation(21, 32), + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x7").WithArguments("x7").WithLocation(21, 32), // (20,19): warning CS0219: The variable 'x7' is assigned but its value is never used // for (bool x7 = true, b( Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x7").WithArguments("x7").WithLocation(20, 19), - // (29,37): error CS0128: A local variable or function named 'x8' is already defined in this scope + // (28,21): error CS0103: The name 'b1' does not exist in the current context + // for (bool d,b1(Dummy(true is var x8 && x8)], + Diagnostic(ErrorCode.ERR_NameNotInContext, "b1").WithArguments("b1").WithLocation(28, 21), + // (28,42): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // for (bool d,b1(Dummy(true is var x8 && x8)], + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(28, 42), + // (29,16): error CS0103: The name 'b2' does not exist in the current context // b2(Dummy(true is var x8 && x8)); - Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(29, 37), + Diagnostic(ErrorCode.ERR_NameNotInContext, "b2").WithArguments("b2").WithLocation(29, 16), + // (29,37): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // b2(Dummy(true is var x8 && x8)); + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(29, 37), // (30,32): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(true is var x8 && x8); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(30, 32), - // (31,32): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter - // Dummy(true is var x8 && x8)) - Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(31, 32), - // (37,23): error CS0841: Cannot use local variable 'x9' before it is declared + // (37,23): error CS0103: The name 'x9' does not exist in the current context // for (bool b = x9, - Diagnostic(ErrorCode.ERR_VariableUsedBeforeDeclaration, "x9").WithArguments("x9").WithLocation(37, 23), + Diagnostic(ErrorCode.ERR_NameNotInContext, "x9").WithArguments("x9").WithLocation(37, 23), + // (38,16): error CS0103: The name 'b2' does not exist in the current context + // b2(Dummy(true is var x9 && x9)); + Diagnostic(ErrorCode.ERR_NameNotInContext, "b2").WithArguments("b2").WithLocation(38, 16), // (39,32): error CS0136: A local or parameter named 'x9' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(true is var x9 && x9); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x9").WithArguments("x9").WithLocation(39, 32), // (40,32): error CS0136: A local or parameter named 'x9' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(true is var x9 && x9)) - Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x9").WithArguments("x9").WithLocation(40, 32) - ); + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x9").WithArguments("x9").WithLocation(40, 32)); var tree = compilation.SyntaxTrees.Single(); var model = compilation.GetSemanticModel(tree); var x4Decl = GetPatternDeclarations(tree, "x4").Single(); - var x4Ref = GetReferences(tree, "x4").Single(); - AssertContainedInDeclaratorArguments(x4Decl); - VerifyModelForDeclarationOrVarPatternDuplicateInSameScope(model, x4Decl); - VerifyNotAPatternLocal(model, x4Ref); + var x4Ref = GetReferences(tree, "x4").ToArray(); + Assert.Equal(2, x4Ref.Length); + AssertNotContainedInDeclaratorArguments(x4Decl); + VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x4Decl, x4Ref[0]); + VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x4Decl, x4Ref[1]); var x7Decl = GetPatternDeclarations(tree, "x7").Single(); var x7Ref = GetReferences(tree, "x7").Single(); - AssertContainedInDeclaratorArguments(x7Decl); - VerifyModelForDeclarationOrVarPatternDuplicateInSameScope(model, x7Decl); - VerifyNotAPatternLocal(model, x7Ref); + AssertNotContainedInDeclaratorArguments(x7Decl); + VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x7Decl); + VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x7Decl, x7Ref); var x8Decl = GetPatternDeclarations(tree, "x8").ToArray(); var x8Ref = GetReferences(tree, "x8").ToArray(); Assert.Equal(4, x8Decl.Length); Assert.Equal(4, x8Ref.Length); - AssertContainedInDeclaratorArguments(x8Decl[0]); - AssertContainedInDeclaratorArguments(x8Decl[1]); - VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x8Decl[0], x8Ref[0], x8Ref[1]); - VerifyModelForDeclarationOrVarPatternDuplicateInSameScope(model, x8Decl[1]); + AssertNotContainedInDeclaratorArguments(x8Decl[0]); + AssertNotContainedInDeclaratorArguments(x8Decl[1]); + VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x8Decl[0], x8Ref[0]); + VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x8Decl[1], x8Ref[1]); VerifyModelForDeclarationOrVarSimplePattern(model, x8Decl[2], x8Ref[2]); VerifyModelForDeclarationOrVarSimplePattern(model, x8Decl[3], x8Ref[3]); @@ -12620,8 +12673,8 @@ void Test9() var x9Ref = GetReferences(tree, "x9").ToArray(); Assert.Equal(3, x9Decl.Length); Assert.Equal(4, x9Ref.Length); - AssertContainedInDeclaratorArguments(x9Decl[0]); - VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x9Decl[0], x9Ref[0], x9Ref[1]); + AssertNotContainedInDeclaratorArguments(x9Decl[0]); + VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x9Decl[0], x9Ref[1]); VerifyModelForDeclarationOrVarSimplePattern(model, x9Decl[1], x9Ref[2]); VerifyModelForDeclarationOrVarSimplePattern(model, x9Decl[2], x9Ref[3]); } From 1a1addf95c2c50593126517247d847dcd0c9b252 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 4 Nov 2024 13:33:53 -0800 Subject: [PATCH 05/18] Fixup tests --- .../Test/Emit3/Semantics/OutVarTests.cs | 164 ++++++++++++------ 1 file changed, 108 insertions(+), 56 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit3/Semantics/OutVarTests.cs b/src/Compilers/CSharp/Test/Emit3/Semantics/OutVarTests.cs index 9ee322657292..7dd853f22111 100644 --- a/src/Compilers/CSharp/Test/Emit3/Semantics/OutVarTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/Semantics/OutVarTests.cs @@ -20954,6 +20954,9 @@ private static void AssertContainedInDeclaratorArguments(DeclarationExpressionSy Assert.True(decl.Ancestors().OfType().First().ArgumentList.Contains(decl)); } + private static void AssertNotContainedInDeclaratorArguments(DeclarationExpressionSyntax decl) + => Assert.Empty(decl.Ancestors().OfType()); + private static void AssertContainedInDeclaratorArguments(params DeclarationExpressionSyntax[] decls) { foreach (var decl in decls) @@ -21562,48 +21565,80 @@ static bool TakeOutParam(object y, out bool x) } "; var compilation = CreateCompilationWithMscorlib461(source, options: TestOptions.DebugExe, parseOptions: TestOptions.Regular); - int[] exclude = new int[] { (int)ErrorCode.ERR_BadVarDecl, - (int)ErrorCode.ERR_SyntaxError, - (int)ErrorCode.ERR_UnexpectedToken, - (int)ErrorCode.WRN_UnreferencedVar, - (int)ErrorCode.ERR_UseDefViolation - }; + int[] exclude = [ + (int)ErrorCode.ERR_BadVarDecl, + (int)ErrorCode.ERR_SyntaxError, + (int)ErrorCode.ERR_UnexpectedToken, + (int)ErrorCode.WRN_UnreferencedVar, + (int)ErrorCode.ERR_UseDefViolation, + (int)ErrorCode.ERR_SemicolonExpected, + (int)ErrorCode.ERR_CloseParenExpected, + ]; compilation.GetDiagnostics().Where(d => !exclude.Contains(d.Code)).Verify( - // (109,13): error CS1023: Embedded statement cannot be a declaration or labeled statement - // var y12 = 12; - Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "var y12 = 12;").WithLocation(109, 13), + // (12,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(12, 22), + // (22,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(22, 22), + // (33,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(33, 22), // (34,47): error CS0136: A local or parameter named 'x4' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(TakeOutParam(true, out var x4) && x4) Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x4").WithArguments("x4").WithLocation(34, 47), + // (41,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(41, 22), // (42,20): error CS0841: Cannot use local variable 'x6' before it is declared // Dummy(x6 && TakeOutParam(true, out var x6)) Diagnostic(ErrorCode.ERR_VariableUsedBeforeDeclaration, "x6").WithArguments("x6").WithLocation(42, 20), + // (49,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(49, 22), // (53,17): error CS0136: A local or parameter named 'x7' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // var x7 = 12; Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x7").WithArguments("x7").WithLocation(53, 17), + // (60,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(60, 22), // (65,34): error CS0103: The name 'x8' does not exist in the current context // System.Console.WriteLine(x8); Diagnostic(ErrorCode.ERR_NameNotInContext, "x8").WithArguments("x8").WithLocation(65, 34), - // (65,9): warning CS0162: Unreachable code detected - // System.Console.WriteLine(x8); - Diagnostic(ErrorCode.WRN_UnreachableCode, "System").WithLocation(65, 9), + // (70,23): error CS0103: The name 'b1' does not exist in the current context + // for (bool a1, b1( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b1").WithArguments("b1").WithLocation(70, 23), + // (75,27): error CS0103: The name 'b2' does not exist in the current context + // for (bool a2, b2( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b2").WithArguments("b2").WithLocation(75, 27), // (76,51): error CS0136: A local or parameter named 'x9' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(TakeOutParam(true, out var x9) && x9) // 2 Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x9").WithArguments("x9").WithLocation(76, 51), + // (84,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(84, 22), // (85,33): error CS0103: The name 'y10' does not exist in the current context // Dummy(TakeOutParam(y10, out var x10)) Diagnostic(ErrorCode.ERR_NameNotInContext, "y10").WithArguments("y10").WithLocation(85, 33), + // (106,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(106, 22), // (107,33): error CS0103: The name 'y12' does not exist in the current context // Dummy(TakeOutParam(y12, out var x12)) Diagnostic(ErrorCode.ERR_NameNotInContext, "y12").WithArguments("y12").WithLocation(107, 33), + // (109,13): error CS1023: Embedded statement cannot be a declaration or labeled statement + // var y12 = 12; + Diagnostic(ErrorCode.ERR_BadEmbeddedStmt, "var y12 = 12;").WithLocation(109, 13), // (109,17): warning CS0219: The variable 'y12' is assigned but its value is never used // var y12 = 12; Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "y12").WithArguments("y12").WithLocation(109, 17), - // (124,44): error CS0128: A local variable named 'x14' is already defined in this scope + // (122,22): error CS0103: The name 'b' does not exist in the current context + // for (bool a, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(122, 22), + // (124,44): error CS0128: A local variable or function named 'x14' is already defined in this scope // TakeOutParam(2, out var x14), - Diagnostic(ErrorCode.ERR_LocalDuplicate, "x14").WithArguments("x14").WithLocation(124, 44) - ); + Diagnostic(ErrorCode.ERR_LocalDuplicate, "x14").WithArguments("x14").WithLocation(124, 44)); var tree = compilation.SyntaxTrees.Single(); var model = compilation.GetSemanticModel(tree); @@ -21611,39 +21646,39 @@ static bool TakeOutParam(object y, out bool x) var x1Decl = GetOutVarDeclarations(tree, "x1").Single(); var x1Ref = GetReferences(tree, "x1").ToArray(); Assert.Equal(2, x1Ref.Length); - AssertContainedInDeclaratorArguments(x1Decl); + AssertNotContainedInDeclaratorArguments(x1Decl); VerifyModelForOutVarWithoutDataFlow(model, x1Decl, x1Ref); var x2Decl = GetOutVarDeclarations(tree, "x2").Single(); var x2Ref = GetReferences(tree, "x2").ToArray(); Assert.Equal(2, x2Ref.Length); - AssertContainedInDeclaratorArguments(x2Decl); + AssertNotContainedInDeclaratorArguments(x2Decl); VerifyModelForOutVarWithoutDataFlow(model, x2Decl, x2Ref); var x4Decl = GetOutVarDeclarations(tree, "x4").Single(); var x4Ref = GetReferences(tree, "x4").ToArray(); Assert.Equal(3, x4Ref.Length); - AssertContainedInDeclaratorArguments(x4Decl); + AssertNotContainedInDeclaratorArguments(x4Decl); VerifyNotAnOutLocal(model, x4Ref[0]); VerifyModelForOutVarWithoutDataFlow(model, x4Decl, x4Ref[1], x4Ref[2]); var x6Decl = GetOutVarDeclarations(tree, "x6").Single(); var x6Ref = GetReferences(tree, "x6").ToArray(); Assert.Equal(2, x6Ref.Length); - AssertContainedInDeclaratorArguments(x6Decl); + AssertNotContainedInDeclaratorArguments(x6Decl); VerifyModelForOutVarWithoutDataFlow(model, x6Decl, x6Ref); var x7Decl = GetOutVarDeclarations(tree, "x7").Single(); var x7Ref = GetReferences(tree, "x7").ToArray(); Assert.Equal(2, x7Ref.Length); - AssertContainedInDeclaratorArguments(x7Decl); + AssertNotContainedInDeclaratorArguments(x7Decl); VerifyModelForOutVarWithoutDataFlow(model, x7Decl, x7Ref[0]); VerifyNotAnOutLocal(model, x7Ref[1]); var x8Decl = GetOutVarDeclarations(tree, "x8").Single(); var x8Ref = GetReferences(tree, "x8").ToArray(); Assert.Equal(3, x8Ref.Length); - AssertContainedInDeclaratorArguments(x8Decl); + AssertNotContainedInDeclaratorArguments(x8Decl); VerifyModelForOutVarWithoutDataFlow(model, x8Decl, x8Ref[0], x8Ref[1]); VerifyNotInScope(model, x8Ref[2]); @@ -21651,7 +21686,7 @@ static bool TakeOutParam(object y, out bool x) var x9Ref = GetReferences(tree, "x9").ToArray(); Assert.Equal(2, x9Decl.Length); Assert.Equal(4, x9Ref.Length); - AssertContainedInDeclaratorArguments(x9Decl); + Assert.All(x9Decl, x9Decl => AssertNotContainedInDeclaratorArguments(x9Decl)); VerifyModelForOutVarWithoutDataFlow(model, x9Decl[0], x9Ref[0], x9Ref[1]); VerifyModelForOutVarWithoutDataFlow(model, x9Decl[1], x9Ref[2], x9Ref[3]); @@ -21667,7 +21702,7 @@ static bool TakeOutParam(object y, out bool x) var x14Ref = GetReferences(tree, "x14").ToArray(); Assert.Equal(2, x14Decl.Length); Assert.Equal(2, x14Ref.Length); - AssertContainedInDeclaratorArguments(x14Decl); + Assert.All(x14Decl, x14Decl => AssertNotContainedInDeclaratorArguments(x14Decl)); VerifyModelForOutVarWithoutDataFlow(model, x14Decl[0], x14Ref); VerifyModelForOutVarDuplicateInSameScope(model, x14Decl[1]); } @@ -21727,66 +21762,83 @@ static bool TakeOutParam(object y, out bool x) } "; var compilation = CreateCompilationWithMscorlib461(source, options: TestOptions.DebugExe, parseOptions: TestOptions.Regular); - int[] exclude = new int[] { (int)ErrorCode.ERR_BadVarDecl, - (int)ErrorCode.ERR_SyntaxError, - (int)ErrorCode.ERR_UnexpectedToken, - (int)ErrorCode.WRN_UnreferencedVar, - (int)ErrorCode.ERR_CloseParenExpected - }; + int[] exclude = [ + (int)ErrorCode.ERR_BadVarDecl, + (int)ErrorCode.ERR_SyntaxError, + (int)ErrorCode.ERR_UnexpectedToken, + (int)ErrorCode.WRN_UnreferencedVar, + (int)ErrorCode.ERR_CloseParenExpected, + (int)ErrorCode.ERR_SemicolonExpected, + ]; compilation.GetDiagnostics().Where(d => !exclude.Contains(d.Code)).Verify( - // (13,47): error CS0128: A local variable or function named 'x4' is already defined in this scope - // Dummy(TakeOutParam(true, out var x4) && x4) - Diagnostic(ErrorCode.ERR_LocalDuplicate, "x4").WithArguments("x4").WithLocation(13, 47), - // (21,47): error CS0128: A local variable or function named 'x7' is already defined in this scope + // (31,57): error CS1513: } expected + // Dummy(TakeOutParam(true, out var x8) && x8)) + Diagnostic(ErrorCode.ERR_RbraceExpected, ")").WithLocation(31, 57), + // (40,57): error CS1513: } expected + // Dummy(TakeOutParam(true, out var x9) && x9)) + Diagnostic(ErrorCode.ERR_RbraceExpected, ")").WithLocation(40, 57), + // (12,22): error CS0841: Cannot use local variable 'x4' before it is declared + // for (bool d, x4( + Diagnostic(ErrorCode.ERR_VariableUsedBeforeDeclaration, "x4").WithArguments("x4").WithLocation(12, 22), + // (20,30): error CS0103: The name 'b' does not exist in the current context + // for (bool x7 = true, b( + Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(20, 30), + // (21,47): error CS0136: A local or parameter named 'x7' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(TakeOutParam(true, out var x7) && x7) - Diagnostic(ErrorCode.ERR_LocalDuplicate, "x7").WithArguments("x7").WithLocation(21, 47), + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x7").WithArguments("x7").WithLocation(21, 47), // (20,19): warning CS0219: The variable 'x7' is assigned but its value is never used // for (bool x7 = true, b( Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x7").WithArguments("x7").WithLocation(20, 19), - // (29,52): error CS0128: A local variable or function named 'x8' is already defined in this scope + // (28,21): error CS0103: The name 'b1' does not exist in the current context + // for (bool d,b1(Dummy(TakeOutParam(true, out var x8) && x8)], + Diagnostic(ErrorCode.ERR_NameNotInContext, "b1").WithArguments("b1").WithLocation(28, 21), + // (28,57): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // for (bool d,b1(Dummy(TakeOutParam(true, out var x8) && x8)], + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(28, 57), + // (29,16): error CS0103: The name 'b2' does not exist in the current context // b2(Dummy(TakeOutParam(true, out var x8) && x8)); - Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(29, 52), + Diagnostic(ErrorCode.ERR_NameNotInContext, "b2").WithArguments("b2").WithLocation(29, 16), + // (29,52): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // b2(Dummy(TakeOutParam(true, out var x8) && x8)); + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(29, 52), // (30,47): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(TakeOutParam(true, out var x8) && x8); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(30, 47), - // (31,47): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter - // Dummy(TakeOutParam(true, out var x8) && x8)) - Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(31, 47), - // (37,23): error CS0841: Cannot use local variable 'x9' before it is declared + // (37,23): error CS0103: The name 'x9' does not exist in the current context // for (bool b = x9, - Diagnostic(ErrorCode.ERR_VariableUsedBeforeDeclaration, "x9").WithArguments("x9").WithLocation(37, 23), + Diagnostic(ErrorCode.ERR_NameNotInContext, "x9").WithArguments("x9").WithLocation(37, 23), + // (38,16): error CS0103: The name 'b2' does not exist in the current context + // b2(Dummy(TakeOutParam(true, out var x9) && x9)); + Diagnostic(ErrorCode.ERR_NameNotInContext, "b2").WithArguments("b2").WithLocation(38, 16), // (39,47): error CS0136: A local or parameter named 'x9' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(TakeOutParam(true, out var x9) && x9); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x9").WithArguments("x9").WithLocation(39, 47), // (40,47): error CS0136: A local or parameter named 'x9' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(TakeOutParam(true, out var x9) && x9)) - Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x9").WithArguments("x9").WithLocation(40, 47) - ); + Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x9").WithArguments("x9").WithLocation(40, 47)); var tree = compilation.SyntaxTrees.Single(); var model = compilation.GetSemanticModel(tree); var x4Decl = GetOutVarDeclarations(tree, "x4").Single(); - var x4Ref = GetReferences(tree, "x4").Single(); - AssertContainedInDeclaratorArguments(x4Decl); - VerifyModelForOutVarDuplicateInSameScope(model, x4Decl); - VerifyNotAnOutLocal(model, x4Ref); + var x4Ref = GetReferences(tree, "x4").ToArray(); + AssertNotContainedInDeclaratorArguments(x4Decl); + VerifyModelForOutVarWithoutDataFlow(model, x4Decl, x4Ref); var x7Decl = GetOutVarDeclarations(tree, "x7").Single(); var x7Ref = GetReferences(tree, "x7").Single(); - AssertContainedInDeclaratorArguments(x7Decl); - VerifyModelForOutVarDuplicateInSameScope(model, x7Decl); - VerifyNotAnOutLocal(model, x7Ref); + AssertNotContainedInDeclaratorArguments(x7Decl); + VerifyModelForOutVarWithoutDataFlow(model, x7Decl, x7Ref); var x8Decl = GetOutVarDeclarations(tree, "x8").ToArray(); var x8Ref = GetReferences(tree, "x8").ToArray(); Assert.Equal(4, x8Decl.Length); Assert.Equal(4, x8Ref.Length); - AssertContainedInDeclaratorArguments(x8Decl[0]); - AssertContainedInDeclaratorArguments(x8Decl[1]); - VerifyModelForOutVarWithoutDataFlow(model, x8Decl[0], x8Ref[0], x8Ref[1]); - VerifyModelForOutVarDuplicateInSameScope(model, x8Decl[1]); + AssertNotContainedInDeclaratorArguments(x8Decl[0]); + AssertNotContainedInDeclaratorArguments(x8Decl[1]); + VerifyModelForOutVar(model, x8Decl[0], x8Ref[0]); + VerifyModelForOutVar(model, x8Decl[1], x8Ref[1]); VerifyModelForOutVar(model, x8Decl[2], x8Ref[2]); VerifyModelForOutVar(model, x8Decl[3], x8Ref[3]); @@ -21794,8 +21846,8 @@ static bool TakeOutParam(object y, out bool x) var x9Ref = GetReferences(tree, "x9").ToArray(); Assert.Equal(3, x9Decl.Length); Assert.Equal(4, x9Ref.Length); - AssertContainedInDeclaratorArguments(x9Decl[0]); - VerifyModelForOutVarWithoutDataFlow(model, x9Decl[0], x9Ref[0], x9Ref[1]); + AssertNotContainedInDeclaratorArguments(x9Decl[0]); + VerifyModelForOutVar(model, x9Decl[0], x9Ref[1]); VerifyModelForOutVar(model, x9Decl[1], x9Ref[2]); VerifyModelForOutVar(model, x9Decl[2], x9Ref[3]); } From 961b913bea27b036c073b82155e64ca19c26a6e5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 4 Nov 2024 13:52:52 -0800 Subject: [PATCH 06/18] Improve recovery --- .../CSharp/Portable/Parser/LanguageParser.cs | 20 ++++++---- .../IOperationTests_IForLoopStatement.cs | 21 +++++++++- .../Semantic/Semantics/ForLoopErrorTests.cs | 38 ++++++++++--------- 3 files changed, 53 insertions(+), 26 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 24e7222e612f..704092ca0ee3 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -5032,9 +5032,14 @@ private void ParseVariableDeclarators( { // If we see `for (int i = 0, j < ...` then we do not want to consume j as the next declarator. // - // Legal forms here are `for (int i = 0, j; ...` or `for (int i = 0, j = ...` or `for (int i = 0, - // j). Anything else we'll treat as as more likely to be the following - if (flags.HasFlag(VariableFlags.ForStatement)) + // Legal forms here are `for (int i = 0, j; ...` or `for (int i = 0, j = ...` or `for (int i = 0, j)`. + // + // We also accept: `for (int i = 0, ;` as that's likely an intermediary state prior to writing the next + // variable. + // + // Anything else we'll treat as as more likely to be the following conditional. + + if (flags.HasFlag(VariableFlags.ForStatement) && this.PeekToken(1).Kind != SyntaxKind.SemicolonToken) { if (!IsTrueIdentifier(this.PeekToken(1)) || this.PeekToken(2).Kind is not (SyntaxKind.SemicolonToken or SyntaxKind.EqualsToken or SyntaxKind.CloseParenToken)) @@ -9158,7 +9163,7 @@ private ForStatementSyntax ParseForStatement(SyntaxList att : null, secondSemicolonToken = eatCommaOrSemicolon(), incrementors: this.CurrentToken.Kind != SyntaxKind.CloseParenToken - ? this.ParseForStatementExpressionList(ref secondSemicolonToken) + ? this.ParseForStatementExpressionList(ref secondSemicolonToken, allowSemicolonAsSeparator: true) : default, eatUnexpectedTokensAndCloseParenToken(), ParseEmbeddedStatement()); @@ -9232,7 +9237,7 @@ private ForStatementSyntax ParseForStatement(SyntaxList att else if (this.CurrentToken.Kind != SyntaxKind.SemicolonToken) { // Not a type followed by an identifier, so it must be an expression list. - return (null, this.ParseForStatementExpressionList(ref openParen)); + return (null, this.ParseForStatementExpressionList(ref openParen, allowSemicolonAsSeparator: false)); } else { @@ -9274,7 +9279,8 @@ private bool IsEndOfForStatementArgument() return this.CurrentToken.Kind is SyntaxKind.SemicolonToken or SyntaxKind.CloseParenToken or SyntaxKind.OpenBraceToken; } - private SeparatedSyntaxList ParseForStatementExpressionList(ref SyntaxToken startToken) + private SeparatedSyntaxList ParseForStatementExpressionList( + ref SyntaxToken startToken, bool allowSemicolonAsSeparator) { return ParseCommaSeparatedSyntaxList( ref startToken, @@ -9284,7 +9290,7 @@ private SeparatedSyntaxList ParseForStatementExpressionList(re skipBadForStatementExpressionListTokens, allowTrailingSeparator: false, requireOneElement: false, - allowSemicolonAsSeparator: false); + allowSemicolonAsSeparator); static PostSkipAction skipBadForStatementExpressionListTokens( LanguageParser @this, ref SyntaxToken startToken, SeparatedSyntaxListBuilder list, SyntaxKind expectedKind, SyntaxKind closeKind) diff --git a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IForLoopStatement.cs b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IForLoopStatement.cs index b6177d5b0449..542c76049941 100644 --- a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IForLoopStatement.cs +++ b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IForLoopStatement.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 @@ -2690,6 +2690,25 @@ static void Main(string[] args) } } } +"; + var tree = GetOperationTreeForTest(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) + { + /**/for (int k = 0, j = 0; k < 100, j > 5; k++) + { + }/**/ + } +} "; 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;') diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/ForLoopErrorTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/ForLoopErrorTests.cs index 19146c1c9979..43bf0dfdc598 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/ForLoopErrorTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/ForLoopErrorTests.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 = @" From fbd98e9e33de7aa5c5b97db0f819036695228b13 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 4 Nov 2024 13:57:05 -0800 Subject: [PATCH 07/18] Docs --- .../CSharp/Portable/Parser/LanguageParser.cs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 704092ca0ee3..38e6f19207a3 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -9162,6 +9162,8 @@ private ForStatementSyntax ParseForStatement(SyntaxList att ? this.ParseExpressionCore() : null, secondSemicolonToken = eatCommaOrSemicolon(), + // Do allow semicolons (with diagnostics) in the incrementors list. This allows us to consume + // accidental extra incrementors that should have been separated by commas. incrementors: this.CurrentToken.Kind != SyntaxKind.CloseParenToken ? this.ParseForStatementExpressionList(ref secondSemicolonToken, allowSemicolonAsSeparator: true) : default, @@ -9172,7 +9174,7 @@ private ForStatementSyntax ParseForStatement(SyntaxList att return forStatement; - (VariableDeclarationSyntax variableDeclaration, SeparatedSyntaxList) eatVariableDeclarationOrInitializers() + (VariableDeclarationSyntax variableDeclaration, SeparatedSyntaxList initializers) eatVariableDeclarationOrInitializers() { using var resetPoint = this.GetDisposableResetPoint(resetOnDispose: false); @@ -9232,12 +9234,15 @@ private ForStatementSyntax ParseForStatement(SyntaxList att decl = decl.Update(declType, decl.Variables); } - return (decl, default); + return (decl, initializers: default); } else if (this.CurrentToken.Kind != SyntaxKind.SemicolonToken) { - // Not a type followed by an identifier, so it must be an expression list. - return (null, this.ParseForStatementExpressionList(ref openParen, allowSemicolonAsSeparator: false)); + // Not a type followed by an identifier, so it must be the initializer expression list. + // + // Do not consume semicolons here as they are used to separate the initializers out from the + // condition of the for loop. + return (variableDeclaration: null, this.ParseForStatementExpressionList(ref openParen, allowSemicolonAsSeparator: false)); } else { @@ -9279,6 +9284,11 @@ private bool IsEndOfForStatementArgument() return this.CurrentToken.Kind is SyntaxKind.SemicolonToken or SyntaxKind.CloseParenToken or SyntaxKind.OpenBraceToken; } + /// + /// Parses out a sequence of expressions. Both for the initializer section (the `for (initializer1, + /// initializer2, ...` section), as well as the incrementor section (the `for (;; incrementor1, incrementor2, + /// ...` section). + /// private SeparatedSyntaxList ParseForStatementExpressionList( ref SyntaxToken startToken, bool allowSemicolonAsSeparator) { From f446d4e8416704d247bbcfaefb094e322e3cf6d8 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 4 Nov 2024 14:10:51 -0800 Subject: [PATCH 08/18] Update test --- .../IOperationTests_IForLoopStatement.cs | 79 ++++++++++--------- 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IForLoopStatement.cs b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IForLoopStatement.cs index 542c76049941..f0b3121c917d 100644 --- a/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IForLoopStatement.cs +++ b/src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IForLoopStatement.cs @@ -2710,45 +2710,46 @@ 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: ';') -"; + 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(source, expectedOperationTree); } From 448c735c397f2f5ace85a784ea3ffaf7f2ad3dcb Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 5 Nov 2024 08:43:52 -0800 Subject: [PATCH 09/18] Update test --- .../Test/Emit3/Semantics/OutVarTests.cs | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit3/Semantics/OutVarTests.cs b/src/Compilers/CSharp/Test/Emit3/Semantics/OutVarTests.cs index 7dd853f22111..5c1212d8e975 100644 --- a/src/Compilers/CSharp/Test/Emit3/Semantics/OutVarTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/Semantics/OutVarTests.cs @@ -21573,6 +21573,7 @@ static bool TakeOutParam(object y, out bool x) (int)ErrorCode.ERR_UseDefViolation, (int)ErrorCode.ERR_SemicolonExpected, (int)ErrorCode.ERR_CloseParenExpected, + (int)ErrorCode.ERR_InvalidExprTerm, ]; compilation.GetDiagnostics().Where(d => !exclude.Contains(d.Code)).Verify( @@ -21769,15 +21770,10 @@ static bool TakeOutParam(object y, out bool x) (int)ErrorCode.WRN_UnreferencedVar, (int)ErrorCode.ERR_CloseParenExpected, (int)ErrorCode.ERR_SemicolonExpected, + (int)ErrorCode.ERR_InvalidExprTerm, ]; compilation.GetDiagnostics().Where(d => !exclude.Contains(d.Code)).Verify( - // (31,57): error CS1513: } expected - // Dummy(TakeOutParam(true, out var x8) && x8)) - Diagnostic(ErrorCode.ERR_RbraceExpected, ")").WithLocation(31, 57), - // (40,57): error CS1513: } expected - // Dummy(TakeOutParam(true, out var x9) && x9)) - Diagnostic(ErrorCode.ERR_RbraceExpected, ")").WithLocation(40, 57), // (12,22): error CS0841: Cannot use local variable 'x4' before it is declared // for (bool d, x4( Diagnostic(ErrorCode.ERR_VariableUsedBeforeDeclaration, "x4").WithArguments("x4").WithLocation(12, 22), @@ -21793,18 +21789,18 @@ static bool TakeOutParam(object y, out bool x) // (28,21): error CS0103: The name 'b1' does not exist in the current context // for (bool d,b1(Dummy(TakeOutParam(true, out var x8) && x8)], Diagnostic(ErrorCode.ERR_NameNotInContext, "b1").WithArguments("b1").WithLocation(28, 21), - // (28,57): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter - // for (bool d,b1(Dummy(TakeOutParam(true, out var x8) && x8)], - Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(28, 57), // (29,16): error CS0103: The name 'b2' does not exist in the current context // b2(Dummy(TakeOutParam(true, out var x8) && x8)); Diagnostic(ErrorCode.ERR_NameNotInContext, "b2").WithArguments("b2").WithLocation(29, 16), // (29,52): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // b2(Dummy(TakeOutParam(true, out var x8) && x8)); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(29, 52), - // (30,47): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // (30,47): error CS0128: A local variable or function named 'x8' is already defined in this scope // Dummy(TakeOutParam(true, out var x8) && x8); - Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(30, 47), + Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(30, 47), + // (31,47): error CS0128: A local variable or function named 'x8' is already defined in this scope + // Dummy(TakeOutParam(true, out var x8) && x8)) + Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(31, 47), // (37,23): error CS0103: The name 'x9' does not exist in the current context // for (bool b = x9, Diagnostic(ErrorCode.ERR_NameNotInContext, "x9").WithArguments("x9").WithLocation(37, 23), @@ -21814,9 +21810,9 @@ static bool TakeOutParam(object y, out bool x) // (39,47): error CS0136: A local or parameter named 'x9' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(TakeOutParam(true, out var x9) && x9); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x9").WithArguments("x9").WithLocation(39, 47), - // (40,47): error CS0136: A local or parameter named 'x9' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // (40,47): error CS0128: A local variable or function named 'x9' is already defined in this scope // Dummy(TakeOutParam(true, out var x9) && x9)) - Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x9").WithArguments("x9").WithLocation(40, 47)); + Diagnostic(ErrorCode.ERR_LocalDuplicate, "x9").WithArguments("x9").WithLocation(40, 47)); var tree = compilation.SyntaxTrees.Single(); var model = compilation.GetSemanticModel(tree); @@ -21839,8 +21835,8 @@ static bool TakeOutParam(object y, out bool x) AssertNotContainedInDeclaratorArguments(x8Decl[1]); VerifyModelForOutVar(model, x8Decl[0], x8Ref[0]); VerifyModelForOutVar(model, x8Decl[1], x8Ref[1]); - VerifyModelForOutVar(model, x8Decl[2], x8Ref[2]); - VerifyModelForOutVar(model, x8Decl[3], x8Ref[3]); + VerifyModelForOutVarWithoutDataFlow(model, x8Decl[2], isShadowed: true); + VerifyModelForOutVarWithoutDataFlow(model, x8Decl[3], isShadowed: true); var x9Decl = GetOutVarDeclarations(tree, "x9").ToArray(); var x9Ref = GetReferences(tree, "x9").ToArray(); @@ -21849,7 +21845,7 @@ static bool TakeOutParam(object y, out bool x) AssertNotContainedInDeclaratorArguments(x9Decl[0]); VerifyModelForOutVar(model, x9Decl[0], x9Ref[1]); VerifyModelForOutVar(model, x9Decl[1], x9Ref[2]); - VerifyModelForOutVar(model, x9Decl[2], x9Ref[3]); + VerifyModelForOutVarWithoutDataFlow(model, x9Decl[2], isShadowed: true); } [Fact] From 7de7e64db894bcedec3fd0a1a5c73177e253ffe1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 5 Nov 2024 08:53:31 -0800 Subject: [PATCH 10/18] Update test --- .../Semantics/PatternMatchingTests_Scope.cs | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTests_Scope.cs b/src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTests_Scope.cs index 7e5edc20234a..cfb5fc42fed9 100644 --- a/src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTests_Scope.cs +++ b/src/Compilers/CSharp/Test/Emit3/Semantics/PatternMatchingTests_Scope.cs @@ -12403,6 +12403,7 @@ void Test14() (int)ErrorCode.ERR_AbstractAndExtern, (int)ErrorCode.ERR_SemicolonExpected, (int)ErrorCode.ERR_CloseParenExpected, + (int)ErrorCode.ERR_InvalidExprTerm, ]; compilation.GetDiagnostics().Where(d => !exclude.Contains(d.Code)).Verify( @@ -12593,15 +12594,10 @@ void Test9() (int)ErrorCode.WRN_UnreferencedVar, (int)ErrorCode.ERR_CloseParenExpected, (int)ErrorCode.ERR_SemicolonExpected, + (int)ErrorCode.ERR_InvalidExprTerm, ]; compilation.GetDiagnostics().Where(d => !exclude.Contains(d.Code)).Verify( - // (31,41): error CS1513: } expected - // Dummy(true is var x8 && x8)) - Diagnostic(ErrorCode.ERR_RbraceExpected, ")").WithLocation(31, 41), - // (40,41): error CS1513: } expected - // Dummy(true is var x9 && x9)) - Diagnostic(ErrorCode.ERR_RbraceExpected, ")").WithLocation(40, 41), // (12,22): error CS0841: Cannot use local variable 'x4' before it is declared // for (bool d, x4( Diagnostic(ErrorCode.ERR_VariableUsedBeforeDeclaration, "x4").WithArguments("x4").WithLocation(12, 22), @@ -12617,18 +12613,18 @@ void Test9() // (28,21): error CS0103: The name 'b1' does not exist in the current context // for (bool d,b1(Dummy(true is var x8 && x8)], Diagnostic(ErrorCode.ERR_NameNotInContext, "b1").WithArguments("b1").WithLocation(28, 21), - // (28,42): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter - // for (bool d,b1(Dummy(true is var x8 && x8)], - Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(28, 42), // (29,16): error CS0103: The name 'b2' does not exist in the current context // b2(Dummy(true is var x8 && x8)); Diagnostic(ErrorCode.ERR_NameNotInContext, "b2").WithArguments("b2").WithLocation(29, 16), // (29,37): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // b2(Dummy(true is var x8 && x8)); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(29, 37), - // (30,32): error CS0136: A local or parameter named 'x8' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // (30,32): error CS0128: A local variable or function named 'x8' is already defined in this scope // Dummy(true is var x8 && x8); - Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x8").WithArguments("x8").WithLocation(30, 32), + Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(30, 32), + // (31,32): error CS0128: A local variable or function named 'x8' is already defined in this scope + // Dummy(true is var x8 && x8)) + Diagnostic(ErrorCode.ERR_LocalDuplicate, "x8").WithArguments("x8").WithLocation(31, 32), // (37,23): error CS0103: The name 'x9' does not exist in the current context // for (bool b = x9, Diagnostic(ErrorCode.ERR_NameNotInContext, "x9").WithArguments("x9").WithLocation(37, 23), @@ -12638,9 +12634,9 @@ void Test9() // (39,32): error CS0136: A local or parameter named 'x9' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter // Dummy(true is var x9 && x9); Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x9").WithArguments("x9").WithLocation(39, 32), - // (40,32): error CS0136: A local or parameter named 'x9' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter + // (40,32): error CS0128: A local variable or function named 'x9' is already defined in this scope // Dummy(true is var x9 && x9)) - Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "x9").WithArguments("x9").WithLocation(40, 32)); + Diagnostic(ErrorCode.ERR_LocalDuplicate, "x9").WithArguments("x9").WithLocation(40, 32)); var tree = compilation.SyntaxTrees.Single(); var model = compilation.GetSemanticModel(tree); @@ -12666,8 +12662,8 @@ void Test9() AssertNotContainedInDeclaratorArguments(x8Decl[1]); VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x8Decl[0], x8Ref[0]); VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x8Decl[1], x8Ref[1]); - VerifyModelForDeclarationOrVarSimplePattern(model, x8Decl[2], x8Ref[2]); - VerifyModelForDeclarationOrVarSimplePattern(model, x8Decl[3], x8Ref[3]); + VerifyModelForDeclarationOrVarSimplePattern(model, x8Decl[2], isShadowed: true); + VerifyModelForDeclarationOrVarSimplePattern(model, x8Decl[3], isShadowed: true); var x9Decl = GetPatternDeclarations(tree, "x9").ToArray(); var x9Ref = GetReferences(tree, "x9").ToArray(); @@ -12676,7 +12672,7 @@ void Test9() AssertNotContainedInDeclaratorArguments(x9Decl[0]); VerifyModelForDeclarationOrVarSimplePatternWithoutDataFlow(model, x9Decl[0], x9Ref[1]); VerifyModelForDeclarationOrVarSimplePattern(model, x9Decl[1], x9Ref[2]); - VerifyModelForDeclarationOrVarSimplePattern(model, x9Decl[2], x9Ref[3]); + VerifyModelForDeclarationOrVarSimplePattern(model, x9Decl[2], isShadowed: true); } [Fact] From a68c5bc1c12cea8db171e7c57f9e75766bacc6ca Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 5 Nov 2024 09:02:03 -0800 Subject: [PATCH 11/18] inline --- .../CSharp/Portable/Parser/LanguageParser.cs | 45 +++++++++---------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 38e6f19207a3..e860418bf86b 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -9165,7 +9165,7 @@ private ForStatementSyntax ParseForStatement(SyntaxList att // Do allow semicolons (with diagnostics) in the incrementors list. This allows us to consume // accidental extra incrementors that should have been separated by commas. incrementors: this.CurrentToken.Kind != SyntaxKind.CloseParenToken - ? this.ParseForStatementExpressionList(ref secondSemicolonToken, allowSemicolonAsSeparator: true) + ? parseForStatementExpressionList(ref secondSemicolonToken, allowSemicolonAsSeparator: true) : default, eatUnexpectedTokensAndCloseParenToken(), ParseEmbeddedStatement()); @@ -9242,7 +9242,7 @@ private ForStatementSyntax ParseForStatement(SyntaxList att // // Do not consume semicolons here as they are used to separate the initializers out from the // condition of the for loop. - return (variableDeclaration: null, this.ParseForStatementExpressionList(ref openParen, allowSemicolonAsSeparator: false)); + return (variableDeclaration: null, parseForStatementExpressionList(ref openParen, allowSemicolonAsSeparator: false)); } else { @@ -9277,30 +9277,20 @@ SyntaxToken eatUnexpectedTokensAndCloseParenToken() var result = this.EatToken(SyntaxKind.CloseParenToken); return AddLeadingSkippedSyntax(result, _pool.ToTokenListAndFree(skippedTokens).Node); } - } - private bool IsEndOfForStatementArgument() - { - return this.CurrentToken.Kind is SyntaxKind.SemicolonToken or SyntaxKind.CloseParenToken or SyntaxKind.OpenBraceToken; - } - - /// - /// Parses out a sequence of expressions. Both for the initializer section (the `for (initializer1, - /// initializer2, ...` section), as well as the incrementor section (the `for (;; incrementor1, incrementor2, - /// ...` section). - /// - private SeparatedSyntaxList ParseForStatementExpressionList( - ref SyntaxToken startToken, bool allowSemicolonAsSeparator) - { - return ParseCommaSeparatedSyntaxList( - ref startToken, - SyntaxKind.CloseParenToken, - static @this => @this.IsPossibleExpression(), - static @this => @this.ParseExpressionCore(), - skipBadForStatementExpressionListTokens, - allowTrailingSeparator: false, - requireOneElement: false, - allowSemicolonAsSeparator); + // Parses out a sequence of expressions. Both for the initializer section (the `for (initializer1, + // initializer2, ...` section), as well as the incrementor section (the `for (;; incrementor1, incrementor2, + // ...` section). + SeparatedSyntaxList parseForStatementExpressionList(ref SyntaxToken startToken, bool allowSemicolonAsSeparator) + => ParseCommaSeparatedSyntaxList( + ref startToken, + SyntaxKind.CloseParenToken, + static @this => @this.IsPossibleExpression(), + static @this => @this.ParseExpressionCore(), + skipBadForStatementExpressionListTokens, + allowTrailingSeparator: false, + requireOneElement: false, + allowSemicolonAsSeparator); static PostSkipAction skipBadForStatementExpressionListTokens( LanguageParser @this, ref SyntaxToken startToken, SeparatedSyntaxListBuilder list, SyntaxKind expectedKind, SyntaxKind closeKind) @@ -9315,6 +9305,11 @@ static PostSkipAction skipBadForStatementExpressionListTokens( } } + private bool IsEndOfForStatementArgument() + { + return this.CurrentToken.Kind is SyntaxKind.SemicolonToken or SyntaxKind.CloseParenToken or SyntaxKind.OpenBraceToken; + } + private CommonForEachStatementSyntax ParseForEachStatement( SyntaxList attributes, SyntaxToken awaitTokenOpt) { From ffd6abfe753de866d470a3b178e8f110c79ee742 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 5 Nov 2024 09:09:16 -0800 Subject: [PATCH 12/18] IDE tests --- .../DeclarationNameCompletionProviderTests.cs | 2 +- .../ConvertLinqQueryToForEachTests.cs | 19 ++++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests.cs index 2c1975a5dbd0..e848f3223fa8 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests.cs @@ -1892,7 +1892,7 @@ void M() } } """; - await VerifyItemExistsAsync(markup, "streamReader"); + await VerifyItemIsAbsentAsync(markup, "streamReader"); } [Fact] diff --git a/src/Features/CSharpTest/ConvertLinq/ConvertLinqQueryToForEachTests.cs b/src/Features/CSharpTest/ConvertLinq/ConvertLinqQueryToForEachTests.cs index 0be23f6a32bd..d271b75cda11 100644 --- a/src/Features/CSharpTest/ConvertLinq/ConvertLinqQueryToForEachTests.cs +++ b/src/Features/CSharpTest/ConvertLinq/ConvertLinqQueryToForEachTests.cs @@ -3353,7 +3353,7 @@ void M(IEnumerable nums) { for(int i = ([|from int n1 in nums from int n2 in nums - select n1|]).Count(), i < 5; i++) + select n1|]).Count(); i < 5; i++) { Console.WriteLine(i); } @@ -3369,20 +3369,17 @@ class C { void M(IEnumerable nums) { - IEnumerable enumerable() + for(int i = 0; i < 5; i++) { - foreach (int n1 in nums) - { - foreach (int n2 in nums) - { - yield return n1; - } - } + Console.WriteLine(i); } - for (int i = enumerable().Count(), i < 5; i++) + foreach (int n1 in nums) { - Console.WriteLine(i); + foreach (int n2 in nums) + { + i++; + } } } } From 31aaa35bfbc333adfcbe9b78405f1d4afec40f7e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 5 Nov 2024 09:12:22 -0800 Subject: [PATCH 13/18] IDE tests --- ...etionProviderTests_NameDeclarationInfoTests.cs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests_NameDeclarationInfoTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests_NameDeclarationInfoTests.cs index 18fdf14d35f7..03185b0033a9 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests_NameDeclarationInfoTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests_NameDeclarationInfoTests.cs @@ -2,8 +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 System.Linq; using System.Threading; using System.Threading.Tasks; @@ -21,7 +19,7 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.DeclarationInfoTests; [Trait(Traits.Feature, Traits.Features.Completion)] public class DeclarationNameCompletion_ContextTests { - protected CSharpTestWorkspaceFixture fixture = new CSharpTestWorkspaceFixture(); + protected readonly CSharpTestWorkspaceFixture Fixture = new(); [Fact] public async Task AfterTypeInClass1() @@ -257,10 +255,9 @@ void M() } } """; - await VerifySymbolKinds(markup, - new SymbolKindOrTypeKind(SymbolKind.Local)); + await VerifySymbolKinds(markup); await VerifyModifiers(markup, new DeclarationModifiers()); - await VerifyTypeName(markup, "int"); + await VerifyTypeName(markup, null); await VerifyAccessibility(markup, null); } @@ -772,10 +769,10 @@ await VerifySymbolKinds(markup, new SymbolKindOrTypeKind(MethodKind.LocalFunction)); } - private async Task VerifyTypeName(string markup, string typeName) + private async Task VerifyTypeName(string markup, string? typeName) { var result = await GetResultsAsync(markup); - Assert.Equal(typeName, result.Type.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)); + Assert.Equal(typeName, result.Type?.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)); } private async Task VerifyNoModifiers(string markup) @@ -812,6 +809,6 @@ private async Task GetResultsAsync(string markup) private (Document, int) ApplyChangesToFixture(string markup) { MarkupTestFile.GetPosition(markup, out var text, out int position); - return (fixture.UpdateDocument(text, SourceCodeKind.Regular), position); + return (Fixture.UpdateDocument(text, SourceCodeKind.Regular), position); } } From 527468b58925f26e1841a0b41527f3fda416dd00 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 5 Nov 2024 09:12:44 -0800 Subject: [PATCH 14/18] Seal --- ...nNameCompletionProviderTests_NameDeclarationInfoTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests_NameDeclarationInfoTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests_NameDeclarationInfoTests.cs index 03185b0033a9..b0d85e386fdc 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests_NameDeclarationInfoTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests_NameDeclarationInfoTests.cs @@ -17,9 +17,9 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.DeclarationInfoTests; [UseExportProvider] [Trait(Traits.Feature, Traits.Features.Completion)] -public class DeclarationNameCompletion_ContextTests +public sealed class DeclarationNameCompletion_ContextTests { - protected readonly CSharpTestWorkspaceFixture Fixture = new(); + private readonly CSharpTestWorkspaceFixture _fixture = new(); [Fact] public async Task AfterTypeInClass1() @@ -809,6 +809,6 @@ private async Task GetResultsAsync(string markup) private (Document, int) ApplyChangesToFixture(string markup) { MarkupTestFile.GetPosition(markup, out var text, out int position); - return (Fixture.UpdateDocument(text, SourceCodeKind.Regular), position); + return (_fixture.UpdateDocument(text, SourceCodeKind.Regular), position); } } From 48734503f8e20783c0bf00f5b7b3a681c527bea8 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 5 Nov 2024 09:22:43 -0800 Subject: [PATCH 15/18] Simplify --- .../CSharp/Portable/Parser/LanguageParser.cs | 50 +++++++------------ 1 file changed, 18 insertions(+), 32 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index e860418bf86b..55ad424f7c64 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -8825,7 +8825,7 @@ private FixedStatementSyntax ParseFixedStatement(SyntaxList var saveTerm = _termState; _termState |= TerminatorState.IsEndOfFixedStatement; - var decl = ParseParenthesizedVariableDeclaration(VariableFlags.None); + var decl = ParseParenthesizedVariableDeclaration(VariableFlags.None, scopedKeyword: null); _termState = saveTerm; return _syntaxFactory.FixedStatement( @@ -9181,7 +9181,6 @@ private ForStatementSyntax ParseForStatement(SyntaxList att // Here can be either a declaration or an expression statement list. Scan // for a declaration first. bool isDeclaration = false; - bool haveScopedKeyword = false; if (this.CurrentToken.ContextualKind == SyntaxKind.ScopedKeyword) { @@ -9195,8 +9194,6 @@ private ForStatementSyntax ParseForStatement(SyntaxList att isDeclaration = ScanType() != ScanTypeFlags.NotType && this.CurrentToken.Kind == SyntaxKind.IdentifierToken; resetPoint.Reset(); } - - haveScopedKeyword = isDeclaration; } else if (this.CurrentToken.Kind == SyntaxKind.RefKeyword) { @@ -9213,28 +9210,7 @@ private ForStatementSyntax ParseForStatement(SyntaxList att if (isDeclaration) { - SyntaxToken scopedKeyword = null; - - if (haveScopedKeyword) - { - scopedKeyword = EatContextualToken(SyntaxKind.ScopedKeyword); - } - - var decl = ParseParenthesizedVariableDeclaration(VariableFlags.ForStatement); - - var declType = decl.Type; - - if (scopedKeyword != null) - { - declType = _syntaxFactory.ScopedType(scopedKeyword, declType); - } - - if (declType != decl.Type) - { - decl = decl.Update(declType, decl.Variables); - } - - return (decl, initializers: default); + return (ParseParenthesizedVariableDeclaration(VariableFlags.ForStatement, ParsePossibleScopedKeyword(isFunctionPointerParameter: false)), initializers: default); } else if (this.CurrentToken.Kind != SyntaxKind.SemicolonToken) { @@ -9921,8 +9897,7 @@ private void ParseUsingExpression(ref VariableDeclarationSyntax declaration, ref if (scopedKeyword != null) { - declaration = ParseParenthesizedVariableDeclaration(VariableFlags.None); - declaration = declaration.Update(_syntaxFactory.ScopedType(scopedKeyword, declaration.Type), declaration.Variables); + declaration = ParseParenthesizedVariableDeclaration(VariableFlags.None, scopedKeyword); return; } else @@ -9955,14 +9930,14 @@ private void ParseUsingExpression(ref VariableDeclarationSyntax declaration, ref case SyntaxKind.CommaToken: case SyntaxKind.CloseParenToken: this.Reset(ref resetPoint); - declaration = ParseParenthesizedVariableDeclaration(VariableFlags.None); + declaration = ParseParenthesizedVariableDeclaration(VariableFlags.None, scopedKeyword: null); break; case SyntaxKind.EqualsToken: // Parse it as a decl. If the next token is a : and only one variable was parsed, // convert the whole thing to ?: expression. this.Reset(ref resetPoint); - declaration = ParseParenthesizedVariableDeclaration(VariableFlags.None); + declaration = ParseParenthesizedVariableDeclaration(VariableFlags.None, scopedKeyword: null); // We may have non-nullable types in error scenarios. if (this.CurrentToken.Kind == SyntaxKind.ColonToken && @@ -9983,7 +9958,7 @@ private void ParseUsingExpression(ref VariableDeclarationSyntax declaration, ref else if (IsUsingStatementVariableDeclaration(st)) { this.Reset(ref resetPoint); - declaration = ParseParenthesizedVariableDeclaration(VariableFlags.None); + declaration = ParseParenthesizedVariableDeclaration(VariableFlags.None, scopedKeyword: null); } else { @@ -10074,6 +10049,7 @@ private StatementSyntax ParseLocalDeclarationStatement(SyntaxList /// Parse a local variable declaration for constructs where the variable declaration is enclosed in parentheses. /// Specifically, only for the `fixed (...)` `for(...)` or `using (...)` statements. /// - private VariableDeclarationSyntax ParseParenthesizedVariableDeclaration(VariableFlags initialFlags) + private VariableDeclarationSyntax ParseParenthesizedVariableDeclaration( + VariableFlags initialFlags, SyntaxToken? scopedKeyword) { var variables = _pool.AllocateSeparated(); ParseLocalDeclaration( @@ -10251,6 +10230,7 @@ private VariableDeclarationSyntax ParseParenthesizedVariableDeclaration(Variable stopOnCloseParen: true, attributes: default, mods: default, + scopedKeyword, initialFlags, out var type, out var localFunction); @@ -10266,12 +10246,16 @@ private void ParseLocalDeclaration( bool stopOnCloseParen, SyntaxList attributes, SyntaxList mods, + SyntaxToken? scopedKeyword, VariableFlags initialFlags, out TypeSyntax type, out LocalFunctionStatementSyntax localFunction) { type = allowLocalFunctions ? ParseReturnType() : this.ParseType(); + if (scopedKeyword != null) + type = _syntaxFactory.ScopedType(scopedKeyword, type); + VariableFlags flags = initialFlags | VariableFlags.LocalOrField; if (mods.Any((int)SyntaxKind.ConstKeyword)) { @@ -10298,6 +10282,8 @@ private void ParseLocalDeclaration( } } +#nullable disable + private bool IsEndOfDeclarationClause() { switch (this.CurrentToken.Kind) From b15605aa82d1933921259bf98db6d6461253b146 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 5 Nov 2024 09:27:12 -0800 Subject: [PATCH 16/18] Doc --- .../CSharp/Portable/Parser/LanguageParser.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 55ad424f7c64..9423cff2c394 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -5041,11 +5041,15 @@ private void ParseVariableDeclarators( if (flags.HasFlag(VariableFlags.ForStatement) && this.PeekToken(1).Kind != SyntaxKind.SemicolonToken) { - if (!IsTrueIdentifier(this.PeekToken(1)) || - this.PeekToken(2).Kind is not (SyntaxKind.SemicolonToken or SyntaxKind.EqualsToken or SyntaxKind.CloseParenToken)) - { + // `int i = 0, ...` where what follows is not an identifier. Don't treat this as the start of a + // second variable. + if (!IsTrueIdentifier(this.PeekToken(1))) + break; + + // `int i = 0, j ...` where what follows is not something that continues a variable declaration. + // In this case, treat that `j` as the start of the condition expression instead. + if (this.PeekToken(2).Kind is not (SyntaxKind.SemicolonToken or SyntaxKind.EqualsToken or SyntaxKind.CloseParenToken)) break; - } } variables.AddSeparator(this.EatToken(SyntaxKind.CommaToken)); From 949861ee9b8b2ac0f9f067d82f6f6ba32a6503f5 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 7 Nov 2024 22:21:23 -0800 Subject: [PATCH 17/18] Add test --- .../Syntax/Parsing/ForStatementParsingTest.cs | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/src/Compilers/CSharp/Test/Syntax/Parsing/ForStatementParsingTest.cs b/src/Compilers/CSharp/Test/Syntax/Parsing/ForStatementParsingTest.cs index a2feede84454..75dcb989303d 100644 --- a/src/Compilers/CSharp/Test/Syntax/Parsing/ForStatementParsingTest.cs +++ b/src/Compilers/CSharp/Test/Syntax/Parsing/ForStatementParsingTest.cs @@ -374,4 +374,78 @@ public void TestCommaSeparators6() } EOF(); } + + [Fact] + public void TestVariableDeclaratorVersusCondition1() + { + UsingStatement("for (int i = 0, i++; i < 10; i++) ;", + // (1,15): error CS1002: ; expected + // for (int i = 0, i++; i < 10; i++) ; + Diagnostic(ErrorCode.ERR_SemicolonExpected, ",").WithLocation(1, 15), + // (1,28): error CS1003: Syntax error, ',' expected + // for (int i = 0, i++; i < 10; i++) ; + Diagnostic(ErrorCode.ERR_SyntaxError, ";").WithArguments(",").WithLocation(1, 28)); + + N(SyntaxKind.ForStatement); + { + N(SyntaxKind.ForKeyword); + N(SyntaxKind.OpenParenToken); + N(SyntaxKind.VariableDeclaration); + { + N(SyntaxKind.PredefinedType); + { + N(SyntaxKind.IntKeyword); + } + N(SyntaxKind.VariableDeclarator); + { + N(SyntaxKind.IdentifierToken, "i"); + N(SyntaxKind.EqualsValueClause); + { + N(SyntaxKind.EqualsToken); + N(SyntaxKind.NumericLiteralExpression); + { + N(SyntaxKind.NumericLiteralToken, "0"); + } + } + } + } + M(SyntaxKind.SemicolonToken); + N(SyntaxKind.PostIncrementExpression); + { + N(SyntaxKind.IdentifierName); + { + N(SyntaxKind.IdentifierToken, "i"); + } + N(SyntaxKind.PlusPlusToken); + } + N(SyntaxKind.SemicolonToken); + N(SyntaxKind.LessThanExpression); + { + N(SyntaxKind.IdentifierName); + { + N(SyntaxKind.IdentifierToken, "i"); + } + N(SyntaxKind.LessThanToken); + N(SyntaxKind.NumericLiteralExpression); + { + N(SyntaxKind.NumericLiteralToken, "10"); + } + } + N(SyntaxKind.SemicolonToken); + N(SyntaxKind.PostIncrementExpression); + { + N(SyntaxKind.IdentifierName); + { + N(SyntaxKind.IdentifierToken, "i"); + } + N(SyntaxKind.PlusPlusToken); + } + N(SyntaxKind.CloseParenToken); + N(SyntaxKind.EmptyStatement); + { + N(SyntaxKind.SemicolonToken); + } + } + EOF(); + } } From 857ad2c64e47f0e6f803f70d103e0d757e6a941a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 13 Nov 2024 16:52:00 -0800 Subject: [PATCH 18/18] Simplify --- .../CSharp/Portable/Parser/LanguageParser.cs | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 9423cff2c394..37d5e9817022 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -9231,21 +9231,9 @@ private ForStatementSyntax ParseForStatement(SyntaxList att } SyntaxToken eatCommaOrSemicolon() - { - if (this.CurrentToken.Kind is SyntaxKind.CommaToken) - { - // Still parse out a semicolon so we give an appropriate error that the comma is unexpected, and so - // we have the correct token kind. - var semicolon = this.EatToken(SyntaxKind.SemicolonToken); - - // Now skip past the comma - return AddTrailingSkippedSyntax(semicolon, this.EatToken()); - } - else - { - return this.EatToken(SyntaxKind.SemicolonToken); - } - } + => this.CurrentToken.Kind is SyntaxKind.CommaToken + ? this.EatTokenAsKind(SyntaxKind.SemicolonToken) + : this.EatToken(SyntaxKind.SemicolonToken); SyntaxToken eatUnexpectedTokensAndCloseParenToken() {