after_each may be executed before the test (if it contains only a statement that doesn't end with a semicolon)

[JohanLorenzo]

Hello,

First and foremost, thank you for this framework! It really helps in writing readable tests 😃

I came across an edge case, today: if you don't have statements that end with a semicolon, the generated code doesn't get ordered properly. For example:

#![feature(const_fn, plugin)]
#![plugin(stainless)]

describe! thread {
    before_each {
        let mut after_each_done = false;
    }

    it "should be executed in the middle" {
        // I concede, tests shouldn't have contain ifs. But in this case, it helps showing 
        // the failure, without looking at the expanded macros.
        if !after_each_done {
            assert!(true);
        } else {
            panic!();
        }
    }

    after_each {
        after_each_done = true;
    }
}

Here's a part of the code when macros are expanded:

    pub fn should_be_executed_in_the_middle() 
     {
        let mut after_each_done = false;

        after_each_done = true;
        if !after_each_done {
        // test code below

[reem]

Wow this is a weird case!

I think I know why this happens - in this code https://github.com/reem/stainless/blob/master/src/generate.rs#L58, which deals with combining the three blocks, all attributes other than the statements in the block are copied from the main test block, including the attribute for "final expression". This causes the final expression of the test block, if it has one, to be placed after all statements in all three blocks!

For now I would just make sure to stick a semicolon in every test, but I will try to get a fix for this up soon.

[hfiguiere]

Also if the last statement doesn't end with a ';'

Like a if { } else { } block. Or a match { } block.

[hfiguiere]

And what it does in that case is that it put the content for after_each before the block in question. (if or match). Adding a ';' after the closing } of said block works around the problem.

[cramertj]

To solve both this and #34, do you want to make it an error to have a block have a top-level expression (with an error message suggesting ending with a ;), or do you think stainless should handle the case and just convert each into a syntax::ast::Stmt_::StmtExpr?

[reem]

I would make it an error - implicitly converting to a statement would probably have unintended consequences.

[cramertj]

Okay. I'm just concerned about the weirdness of having to do things like putting semicolons after for loops :).

[reem]

Hmm, that's a good point. It is a bit awkward to have to semicolon while, loop, and for expressions. Perhaps transforming the final expression into a statement is nicer.