Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Parse incomplete code differently to better reflect user intent. #15885

Merged
merged 11 commits into from
Dec 19, 2016
3 changes: 1 addition & 2 deletions src/Compilers/CSharp/Portable/CSharpExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,6 @@ public static SymbolInfo GetSpeculativeSymbolInfo(this SemanticModel semanticMod
}
}


public static TypeInfo GetTypeInfo(this SemanticModel semanticModel, SelectOrGroupClauseSyntax node, CancellationToken cancellationToken = default(CancellationToken))
{
var csmodel = semanticModel as CSharpSemanticModel;
Expand Down Expand Up @@ -1339,4 +1338,4 @@ public static Conversion ClassifyConversion(this SemanticModel semanticModel, in
}
#endregion
}
}
}
70 changes: 70 additions & 0 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7010,6 +7010,56 @@ 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))
Copy link
Member

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?

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

Choose a reason for hiding this comment

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

There is also the case z[ which is an erroneous local array declaration.

Copy link
Member Author

Choose a reason for hiding this comment

The 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 :-)

Copy link
Member

Choose a reason for hiding this comment

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

Could you please add a test case showing that?

Copy link
Member Author

Choose a reason for hiding this comment

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

sure!

Copy link
Member Author

Choose a reason for hiding this comment

The 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;
if (token4Kind != SyntaxKind.SemicolonToken &&
token4Kind != SyntaxKind.EqualsToken &&
token4Kind != SyntaxKind.CommaToken &&
token4Kind != SyntaxKind.OpenParenToken &&
token4Kind != SyntaxKind.LessThanToken)
{
return false;
}
}
Copy link
Contributor

@AlekseyTs AlekseyTs Dec 17, 2016

Choose a reason for hiding this comment

The 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
{
Expand Down Expand Up @@ -9631,6 +9681,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;

Expand Down
Loading