Error on indents

[JaroslavTulach]

Pull Request Description

Modify the parser to report an error on wrong indentation fixes #9419.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

changes, a screencast is preferred.

• All code follows the Rust and Java style guides.
• Unit tests have been written where possible.

[JaroslavTulach]

The code is:

group =
    space4
        space8
     space5

e.g. block with indentation 4, followed by nested block with indentation 8 and then block with indentation 5 - a typical sign of error.

Right now the test succeeds - the goal is to make it fail!

[JaroslavTulach]

With the 350c2c3 change I am getting a failure in following tests:

code_block_bad_indents1
code_block_bad_indents2
fail_on_bad_indents

all the tests seem to resemble to problem reported in #9419 (comment) - e.g. it seems good that they are now failing. However I am not sure if the failure is the right one? I tried to insert Variant::invalid() - is that the right way to mark an indentation problem, @kazcw? Thanks in advance for your guidance.

[JaroslavTulach]

Expecting invalid node since 3e8e6c0

[kazcw]

token::Variant::invalid() is for an illegal token (e.g. an interpolation-quote outside of a text literal)--in this case the error it produces wouldn't be associated with the right span (which should be the rest of the tokens in the line). The marker token will also need to be located at the end of the preceding newline, not the beginning--token code references must cover the source code in order.

If this indentation-handling change is to be made, it could be done like this:

• In the lexer, construct a token of a new type InvalidRestOfLine { error: Cow<'static, str> }. Use the marker_token function to attach the correct location information for a zero-length marker.
• In the parser, we'll use a macro to handle it. The macro resolver isn't token-type aware, so we'll need to add the ability to use a zero-length marker token of a specific type as a macro segment header.
  • Define a macro that matches the header <InvalidRestOfLine> (this is the string we'll use to name the marker token; it would never match any real single token), followed by everything(). Its implementation parses its tokens and wraps them in an Invalid¹.
  • In the macro resolver, in process_token, before looking up the token code in any macro maps, check if it is zero-length; if it is, match against the token's variant, and if it is InvalidRestOfLine look up <InvalidRestOfLine> instead of the actual (empty) token code.

Let me know if you have any questions!

Footnotes

1. In the macro implementation, one tricky case will be if precedence.resolve returns None. This shouldn't be reachable now, but it might be possible with future uses of InvalidRestOfLine, or in case of a bug (like the one I noted in another comment). The difficulty is that a Tree must still be created, and its spans (though 0-length) must still be appropriately-positioned within the document. You can handle this by applying into_ident to the header token, and putting the result in a dummy Tree::Ident node, to have something to wrap with Invalid. ↩

[JaroslavTulach]

We have few failures:

• AvoidIdInstrumentationTagTest.avoidIdInstrumentationInLambdaMapFunctionWithFloor - Compilation aborted due to errors.
• Cannot compile Standard/Database.
• lexer::tests::test_case_block_bad_indents

[radeusgd]

What is the error message?
Is it possible to have the message mention 'invalid indentation level'? That would be much more helpful than just 'unexpected expression'.

[JaroslavTulach]

I guess I need to get that information from the Rust side. How do you want me to do it, @kazcw?

[radeusgd]

Cool.

It would be great to have a situation-specific syntax error message for this, to make the errors more user-friendly.

[kazcw]

When end_blocks is called when the input is at EOF, an error should not be emitted for a partial-indent in a whitespace-only final line. This test should pass:

fn partial_indent_at_eof_is_not_an_error() {
    test("main =\n    42\n   ", block![(Function /* ... */)]);
}

[kazcw]

token::Variant::invalid() is for an illegal token (e.g. an interpolation-quote outside of a text literal)--in this case the error it produces wouldn't be associated with the right span (which should be the rest of the tokens in the line). The marker token will also need to be located at the end of the preceding newline, not the beginning--token code references must cover the source code in order.

If this indentation-handling change is to be made, it could be done like this:

• In the lexer, construct a token of a new type InvalidRestOfLine { error: Cow<'static, str> }. Use the marker_token function to attach the correct location information for a zero-length marker.
• In the parser, we'll use a macro to handle it. The macro resolver isn't token-type aware, so we'll need to add the ability to use a zero-length marker token of a specific type as a macro segment header.
  • Define a macro that matches the header <InvalidRestOfLine> (this is the string we'll use to name the marker token; it would never match any real single token), followed by everything(). Its implementation parses its tokens and wraps them in an Invalid¹.
  • In the macro resolver, in process_token, before looking up the token code in any macro maps, check if it is zero-length; if it is, match against the token's variant, and if it is InvalidRestOfLine look up <InvalidRestOfLine> instead of the actual (empty) token code.

Let me know if you have any questions!

Footnotes

1. In the macro implementation, one tricky case will be if precedence.resolve returns None. This shouldn't be reachable now, but it might be possible with future uses of InvalidRestOfLine, or in case of a bug (like the one I noted in another comment). The difficulty is that a Tree must still be created, and its spans (though 0-length) must still be appropriately-positioned within the document. You can handle this by applying into_ident to the header token, and putting the result in a dummy Tree::Ident node, to have something to wrap with Invalid. ↩