Skip to content

Fix class parsing (issue 3083) #3103

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.

Sign up for GitHub

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

Merged
lzybkr merged 4 commits into from
Feb 14, 2017
Merged

Fix class parsing (issue 3083) #3103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b77dcae
Fix class parsing (issue 3083)
iSazonov Feb 6, 2017
4e3a28d
Correct error position
iSazonov Feb 6, 2017
c2990e6
Fix error position reporting
iSazonov Feb 7, 2017
68ab509
Fix hash expression parsing
iSazonov Feb 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/System.Management.Automation/engine/parser/Parser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3909,7 +3909,7 @@ private StatementAst ClassDefinitionRule(List<AttributeBaseAst> customAttributes
if (rCurly.Kind != TokenKind.RCurly)
{
UngetToken(rCurly);
ReportIncompleteInput(lastExtent, () => ParserStrings.MissingEndCurlyBrace);
ReportIncompleteInput(After(lCurly), rCurly.Extent, () => ParserStrings.MissingEndCurlyBrace);
}
else
{
Expand Down Expand Up @@ -4269,7 +4269,7 @@ private StatementAst EnumDefinitionRule(List<AttributeBaseAst> customAttributes,
if (rCurly.Kind != TokenKind.RCurly)
{
UngetToken(rCurly);
ReportIncompleteInput(After(lastExtent), () => ParserStrings.MissingEndCurlyBrace);
ReportIncompleteInput(After(lCurly), rCurly.Extent, () => ParserStrings.MissingEndCurlyBrace);
}

var startExtent = customAttributes != null && customAttributes.Count > 0
Expand Down Expand Up @@ -6344,8 +6344,8 @@ private ExpressionAst HashExpressionRule(Token atCurlyToken, bool parsingSchemaE
// Note - the error handling function inspects the error message body to extra the ParserStrings property name. It uses this value as the errorid.
var errorMessageExpression = parsingSchemaElement ?
(Expression<Func<string>>)(() => ParserStrings.IncompletePropertyAssignmentBlock)
: (Expression<Func<string>>)(() => ParserStrings.IncompleteHashLiteral);
ReportIncompleteInput(keyValuePairs.Any() ? After(keyValuePairs.Last().Item2) : After(atCurlyToken), rCurly.Extent, errorMessageExpression);
: (Expression<Func<string>>)(() => ParserStrings.MissingEndCurlyBrace);
ReportIncompleteInput(After(atCurlyToken), rCurly.Extent, errorMessageExpression);
Copy link
Copy Markdown
Contributor

@lzybkr lzybkr Feb 8, 2017

Choose a reason for hiding this comment

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

This looks like it might degrade the quality of the error on:

@{
a =
}

or something like that - please try variants of this (for example: with and without the closing curly, with and without the =).

Copy link
Copy Markdown
Collaborator Author

@iSazonov iSazonov Feb 9, 2017

Choose a reason for hiding this comment

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

I see. I tried to fix.

After that there is another question: Are NewLine and Semi terminators having the same rights? Compare:

@{
a = 
b =1
c =2
}


@{
a = ;
b =1;
c =2;
}

If we will fix this (require that the expression begin immediately after the equal on the same line) of cause it will be a breaking change.

Copy link
Copy Markdown
Contributor

@lzybkr lzybkr Feb 14, 2017

Choose a reason for hiding this comment

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

Interesting, I don't recall explicitly supporting multiple statement terminators (you can have multiple semicolons as well) - so we shouldn't break anybody even though that's kinda broken.

Copy link
Copy Markdown
Collaborator Author

@iSazonov iSazonov Feb 16, 2017

Choose a reason for hiding this comment

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

Clear. Thanks!

endExtent = Before(rCurly);
}
else
Expand Down Expand Up @@ -6394,7 +6394,7 @@ private KeyValuePair GetKeyValuePair(bool parsingSchemaElement)
? (() => (ParserStrings.MissingEqualsInPropertyAssignmentBlock))
: (Expression<Func<string>>)(() => ParserStrings.MissingEqualsInHashLiteral);
ReportError(errorExtent, errorMessageExpression);
SyncOnError(true, TokenKind.RCurly, TokenKind.Semi, TokenKind.NewLine);
SyncOnError(false, TokenKind.RCurly, TokenKind.Semi, TokenKind.NewLine);
return new KeyValuePair(key, new ErrorStatementAst(errorExtent));
}

Expand Down
3 changes: 0 additions & 3 deletions src/System.Management.Automation/resources/ParserStrings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,6 @@ Possible matches are</value>
<data name="MissingExpressionInNamedArgument" xml:space="preserve">
<value>Missing statement after '=' in named argument.</value>
</data>
<data name="IncompleteHashLiteral" xml:space="preserve">
<value>The hash literal was incomplete.</value>
</data>
<data name="MissingPropertyTerminator" xml:space="preserve">
<value>Missing ';' or end-of-line in property definition.</value>
</data>
Expand Down
4 changes: 3 additions & 1 deletion test/powershell/Language/Classes/Scripting.Classes.BasicParsing.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ Describe 'Positive Parse Properties Tests' -Tags "CI" {
Describe 'Negative Parsing Tests' -Tags "CI" {
ShouldBeParseError 'class' MissingNameAfterKeyword 5
ShouldBeParseError 'class foo' MissingTypeBody 9
ShouldBeParseError 'class foo {' MissingEndCurlyBrace 10
ShouldBeParseError 'class foo {' MissingEndCurlyBrace 11
ShouldBeParseError 'class foo { [int] }' IncompleteMemberDefinition 17
ShouldBeParseError 'class foo { $private: }' InvalidVariableReference 12
ShouldBeParseError 'class foo { [int]$global: }' InvalidVariableReference 17
Expand Down Expand Up @@ -307,6 +307,8 @@ Describe 'Negative Parsing Tests' -Tags "CI" {
ShouldBeParseError 'class C { static [int]$i; [void] foo() {$i = 10} }' MissingTypeInStaticPropertyAssignment 40

ShouldBeParseError 'class C : B' MissingTypeBody 11

ShouldBeParseError 'Class foo { q(){} w(){}' MissingEndCurlyBrace 11
}

Describe 'Negative methods Tests' -Tags "CI" {
Expand Down
2 changes: 1 addition & 1 deletion test/powershell/Language/Classes/scripting.Classes.using.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ New-Object Foo
It "report an error on incomplete using input" {
$err = Get-ParseResults "using module @{ModuleName = 'FooWithManifest'; FooWithManifest = 1." # missing closing bracket
$err.Count | Should Be 2
$err[0].ErrorId | Should Be 'IncompleteHashLiteral'
$err[0].ErrorId | Should Be 'MissingEndCurlyBrace'
$err[1].ErrorId | Should Be 'RequiresModuleInvalid'
}

Expand Down
3 changes: 2 additions & 1 deletion test/powershell/Language/Classes/scripting.enums.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ Describe 'Basic enum errors' -Tags "CI" {
ShouldBeParseError 'enum foo' MissingTypeBody 8
ShouldBeParseError 'enum foo {' MissingEndCurlyBrace 10
ShouldBeParseError 'enum foo { x = }' ExpectedValueExpression 14
ShouldBeParseError 'enum foo { x =' ExpectedValueExpression,MissingEndCurlyBrace 14,14
ShouldBeParseError 'enum foo { x =' ExpectedValueExpression,MissingEndCurlyBrace 14,10
ShouldBeParseError 'enum foo {} enum foo {}' MemberAlreadyDefined 12
ShouldBeParseError 'enum foo { x; x }' MemberAlreadyDefined 14 -SkipAndCheckRuntimeError
ShouldBeParseError 'enum foo { X; x }' MemberAlreadyDefined 14 -SkipAndCheckRuntimeError
Expand All @@ -94,4 +94,5 @@ Describe 'Basic enum errors' -Tags "CI" {
ShouldBeParseError 'enum foo { e = [int]::MaxValue + 1 }' EnumeratorValueTooLarge 15 -SkipAndCheckRuntimeError
ShouldBeParseError 'enum foo { e = $foo }' EnumeratorValueMustBeConstant 15 -SkipAndCheckRuntimeError
ShouldBeParseError 'enum foo { e = "hello" }' CannotConvertValue 15 -SkipAndCheckRuntimeError
ShouldBeParseError 'enum foo { a;b;c;' MissingEndCurlyBrace 10
}
6 changes: 5 additions & 1 deletion test/powershell/Language/Parser/Parsing.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,4 +299,8 @@ Describe 'expressions parsing' -Tags "CI" {
ShouldBeParseError '[ref][ref]$x' ReferenceNeedsToBeByItselfInTypeSequence 5
ShouldBeParseError '[int][ref]$x' ReferenceNeedsToBeLastTypeInTypeConversion 5
ShouldBeParseError '[int][ref]$x = 42' ReferenceNeedsToBeByItselfInTypeConstraint 5
}
}

Describe 'Hash Expression parsing' -Tags "CI" {
ShouldBeParseError '@{ a=1;b=2;c=3;' MissingEndCurlyBrace 2
}