Fix invalid if condition parameter

[Mido-sys]

Fixes issue 115

The new pull requests confirm the validity of the If condition parameter during parsing and compiling. It checks for syntax error and if the evaluated If condition is of a type Bool or nil

During Parsing:

We check if the expression. Condition implements ast.Comparable. If not, then a syntax error is returned. If the type of expression. Condition is of type ast.InflixExpression then we also check the Left and Right expressions do implement ast.Comparable recursively.

During Compilation:

We confirm the evaluated type return of the if condition to be either bool or nil.

[paganotoni]

Thanks for this one!

[Mido-sys]

@paganotoni , I'm glad I was able to help :)

[paganotoni]

I think I found an issue with this PR, it seems it broke the example in here:

https://github.com/gobuffalo/plush#full-example

if(names) {

Should return true if the names variable is set.

[paganotoni]

I think there is a test for the example in there but this is a different test case, added it here.

func Test_Render_If_Value(t *testing.T) {
	r := require.New(t)
	input := `<%= if (names) { %>hi<%} %>`
	ctx := NewContext()
	ctx.Set("names", "123")
	s, err := Render(input, ctx)
	r.NoError(err)
	r.Equal("", s)
}

[Mido-sys]

The provided test should return "hi"?

[paganotoni]

I think so, the issue is then if (x + 1) test case will print hi as well. But given we're saying <%= if (names && len(names) > 0) { %> then we should accept <%= if (names) { %>. Don't you think so ?

[Mido-sys]

@paganotoni , it makes sense to accept it. I will push a fix soon to check if names is set. I found another test, Test_If_String_Truthy, that should have failed, but it's not failing. According to the current code if (username && username != "") should return an error.

[Mido-sys]

@paganotoni ,

ctx.Set("username", "foo")
<p><%= if (username + 1) { return "hi" } else { return "bye" } %></p>

I was wondering if we shouldn't accept username + 1. This code doesn't evaluate to true/false. It concatenates the value of username with 1. The result of the if condition evaluation is foo1. So I think we should only allow statements that will evaluate to boolean at the end. Please let me know what you think.

[paganotoni]

I'm thinking we should go back to the concept of truth in plush, from the examples it seems to me that any variable/value set in the context should be considered true except nil and false. That takes me to think that plush should accept the if (x + 1), we would be evaluating the result of x+1 and given that's a non-nil value in the context we should accept it.

[Mido-sys]

@paganotoni ,

Sounds good. I will remove the check from the compiler. However, the parser will still throw an error for these conditions:

1. <% if ( y == 1 && x = 1) { x } %>
2. if (!(x = 1)) { x } %>

Sent a pull request pull#131