Fix class parsing (issue 3083)#3103
Conversation
Close PowerShell#3083 This problem only appears in the interactive session with manual input (or right click) and PSReadLine loaded so no tests.
|
What's the error message on the motivation example now? Essentially the smallest repro is class foo {
} |
|
The fix just allow blank lines. |
|
Oh, that's very interesting. From the diff it seems that it just changes extent, but keeps reporting the error. Let me check with a local build. |
|
Verified. Great change! |
|
Yes it is slightly a trick. :-) |
|
@iSazonov yeah, |
|
I found as apply |
|
The use of The important part is reporting where the error was detected, the second parameter - and that should be sufficient to fix the bug. |
I think it's more usable to report non-closed curly closer to the expected place. Also, the fix itself exploits the implementation of |
|
Yes, but it's somewhat difficult to infer where the closing brace should go. End of file is often the best we can do, and that's worse than pointing to the opening brace. |
|
Oh, the proposed fix moves the point where the error is reported by 1 character - the character after the curly - so it's not really much different, and it's not consistent with most other places we report the same error. |
|
The hash literal might be a bad example, I think the error should point to the Consider an example like: function foo
{
function bar
{
}
By reporting the error on the opening brace, assuming sane indentation, it's usually easy to figure out where the missing brace goes. So I'd say hash literals, class, and enum should all be modified to report the error after the opening brace, and make sure to pass the position the error is detected to ensure we correctly report an incomplete parse for interactive command line use. |
Fix for class, enum and hash expression
|
Fix for hash, class, and enum is ready. Now the error position is reported after the opening brace. |
| : (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); |
There was a problem hiding this comment.
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 =).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Must re-approve when new commits pushed.
|
@iSazonov Great commit description! It helps me quickly understand the problem and the fix. Thanks! |
Close #3083
This problem only appears in the interactive session with manual input
(or right click) and with PSReadLine loaded so no tests.