-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Parse incomplete code differently to better reflect user intent. #15885
Changes from 5 commits
0d05f0a
426677b
0851ac8
ec74124
24f4563
fb3f230
3b21ccb
32a294e
77cc0c6
3bf28e5
6c6a6b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7010,6 +7010,53 @@ private bool IsPossibleLocalDeclarationStatement(bool allowAnyExpression) | |
return typedIdentifier.Value; | ||
} | ||
|
||
// It's common to have code like the following: | ||
// | ||
// Task. | ||
// await Task.Delay() | ||
// | ||
// In this case we don't want to parse this as as a local declaration like: | ||
// | ||
// Task.await Task | ||
// | ||
// This does not represent user intent, and it causes all sorts of problems to higher | ||
// layers. This is because both the parse tree is strage, and the symbol tables have | ||
// entries that throw things off (like a bogus 'Task' local). | ||
// | ||
// Note that we explicitly do this check when we see that the code spreads over multiple | ||
// lines. We don't want this if the user has actually written "X.Y z" | ||
if (tk == SyntaxKind.IdentifierToken) | ||
{ | ||
var token1 = PeekToken(1); | ||
if (token1.Kind == SyntaxKind.DotToken && | ||
token1.TrailingTrivia.Any((int)SyntaxKind.EndOfLineTrivia)) | ||
{ | ||
if (PeekToken(2).Kind == SyntaxKind.IdentifierToken && | ||
PeekToken(3).Kind == SyntaxKind.IdentifierToken) | ||
{ | ||
// We have something like: | ||
// | ||
// X. | ||
// Y z | ||
// | ||
// This is only a local declaration if we have: | ||
// | ||
// X.Y z; | ||
// X.Y z = ... | ||
// X.Y z, ... | ||
// X.Y z( ... (local function) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is also the case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fort that case, I think the new error handling heuristic will actually produce a better result :-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please add a test case showing that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test added. |
||
// X.Y z<W... (local function) | ||
// | ||
var token4Kind = PeekToken(4).Kind; | ||
return token4Kind == SyntaxKind.SemicolonToken || | ||
token4Kind == SyntaxKind.EqualsToken || | ||
token4Kind == SyntaxKind.CommaToken || | ||
token4Kind == SyntaxKind.OpenParenToken || | ||
token4Kind == SyntaxKind.LessThanToken; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we simply continue if this condition is true? #Closed |
||
} | ||
} | ||
|
||
var resetPoint = this.GetResetPoint(); | ||
try | ||
{ | ||
|
@@ -9631,6 +9678,26 @@ private ExpressionSyntax ParsePostFixExpression(ExpressionSyntax expr) | |
expr = _syntaxFactory.MemberAccessExpression(SyntaxKind.PointerMemberAccessExpression, expr, this.EatToken(), this.ParseSimpleName(NameOptions.InExpression)); | ||
break; | ||
case SyntaxKind.DotToken: | ||
// if we have the error situation: | ||
// | ||
// expr. | ||
// X Y | ||
// | ||
// Then we don't want to parse this out as "Expr.X" | ||
// | ||
// It's far more likely the member access expression is simply incomplete and | ||
// there is a new declaration on the next line. | ||
if (this.CurrentToken.TrailingTrivia.Any((int)SyntaxKind.EndOfLineTrivia) && | ||
this.PeekToken(1).Kind == SyntaxKind.IdentifierToken && | ||
this.PeekToken(2).Kind == SyntaxKind.IdentifierToken) | ||
{ | ||
expr = _syntaxFactory.MemberAccessExpression( | ||
SyntaxKind.SimpleMemberAccessExpression, expr, this.EatToken(), | ||
this.AddError(this.CreateMissingIdentifierName(), ErrorCode.ERR_IdentifierExpected)); | ||
|
||
return expr; | ||
} | ||
|
||
expr = _syntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, expr, this.EatToken(), this.ParseSimpleName(NameOptions.InExpression)); | ||
break; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this check sensitive to spaces?