Expressions not being run if indented by 1 more space than current indentation level

[radeusgd]

I've just encountered this in our tests and it was a great confusion for me: e4bebc7

Minimal example:

from Standard.Base import all
from Standard.Test import all

add_specs suite_builder =
    suite_builder.group "My Test" group_builder->
        group_builder.specify "this will run" <|
            IO.println "this will be printed"

         # one more indent than should be here
         group_builder.specify "and this test will not even show up on the list!" <|
             IO.println "but this will not!"

main filter=Nothing =
    suite = Test.build suite_builder->
        add_specs suite_builder
    suite.run_with_filter filter

Actual behaviour

this will be printed
My Test: [1/1, 27ms]
    - this will run [27ms]
1 tests succeeded.
0 tests failed.
0 tests skipped.
0 groups skipped.

The expression with one more indent level seems to be completely ignored and not evaluated.

Expected behaviour

I guess this should be a syntax error? Definitely we shouldn't just silently swallow an expression and not run it.

[radeusgd]

This is related to #8858 - if that was implemented we would at least see that there is a problem (the indentation there is not a multiple of 4), so we could easier spot the issue.

However, while in some cases the non-divisible-by-4 indentation is just a matter of style and it is enough to get a warning, in the case shown in this ticket it seems more severe - as the block is completely ignored. So IMO this particular case should be a hard error.

Unless this is 'expected behaviour' due to some weird constructs - then we could close this and just warning (#8858) would be enough.

[radeusgd]

Curiously

from Standard.Base import all

main =
    IO.println "A"
     IO.println "B"
    IO.println "C"
     IO.println "D"

fails with

A
Execution finished with an error: Type error: expected a function, but got Nothing.
        at <enso> test-ignored.main(test-ignored.enso:4:5-18)

[kazcw]

• Indent levels are every 4 spaces
• Bad indentation is rounded up to the next level
• A line's parent is the nearest line before it that was at a lower indent level

So the first case parses the same as this:

add_specs suite_builder =
    suite_builder.group "My Test" group_builder->
        group_builder.specify "this will run" <|
            IO.println "this will be printed"

            group_builder.specify "and this test will not even show up on the list!" <|
                IO.println "but this will not!"

The second case parses like this:

main =
    IO.println "A"
        IO.println "B"
    IO.println "C"
        IO.println "D"

[JaroslavTulach]

Radek, haven't you already reported this problem once?

• Warn about invalid indentation levels #8858

[JaroslavTulach]

• Bad indentation is rounded up to the next level

Wouldn't it be better to just report an error or at least print a warning as #8858 suggests?

Especially this case:

        group_builder.specify "this will run" <|
            IO.println "this will be printed"

         group_builder.specify "and this test will not even show up on the list!"

is pretty dangerous! Indenting 4 spaces on IO.println and then indenting 1 space signals an error, in my opinion. Treating the second group_builder. line as being as indented as the IO.println isn't helping any developer.

[kazcw]

https://github.com/enso-org/design/blob/wip/wd/enso-spec/epics/enso-spec-1.0/03.%20Code%20format%20and%20layout.md#code-blocks

GitHub

Build software better, together

GitHub is where people build software. More than 100 million people use GitHub to discover, fork, and contribute to over 420 million projects.

[radeusgd]

Radek, haven't you already reported this problem once?

• Warn about invalid indentation levels #8858

I have written in my comment that these 2 issues are related. Maybe one fix can fix both, however I thought that this is another issue as explained in my comment above.

[enso-bot]

Jaroslav Tulach reports a new STANDUP for yesterday (2024-05-06):

Progress: - merging WithWarnings.isException: #9840

• 1 space parser changes: 3e8e6c0
• syntax error & TCK fix: Syntax error shall contain location of the error #9864
• polyglot-api fix: 27163b3
• NI can execute Standard.Base: Importing (some of) Standard.Base works from NI runner #9866
• standups & meetings It should be finished by 2024-05-14.