Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't evaluate if conditions properly with an undefined variable #157

Closed
Mido-sys opened this issue Jun 8, 2022 · 7 comments · Fixed by #164, #166 or #168
Closed

Can't evaluate if conditions properly with an undefined variable #157

Mido-sys opened this issue Jun 8, 2022 · 7 comments · Fixed by #164, #166 or #168
Labels
bug Something isn't working s: fixed was fixed or solution offered
Milestone

Comments

@Mido-sys
Copy link
Contributor

Mido-sys commented Jun 8, 2022

Plush evaluates undefined variables in if conditions without throwing an error; however, it becomes an issue with this example.

	r := require.New(t)
	type page struct {
		PageTitle string
	}
	g := &page{"cafe"}
	ctx := NewContext()
	ctx.Set("pages", g)
	ctx.Set("path", "home")

	input := `<%= if ( path != "pagePath" || (page && page.PageTitle != "cafe") ) { %>hi<%} %>`

	s, err := Render(input, ctx)
	r.NoError(err)
	r.Equal("hi", s)

Currently, this should return "hi" as one of the conditions path != "pagePath" is true; however, it returns an empty string because the if condition values on the right of the or return an error, so it bubbles back up and results in evaluating the condition as false.

This behaviour is inconsistent because this condition evaluates as true <%= if ( path != "pagePath" || !page) { %>hi<%} %>

@Mido-sys Mido-sys changed the title Can't evaluated if conditions properly with an undefined variable is Can't evaluated if conditions properly with an undefined variable Jun 8, 2022
@Mido-sys
Copy link
Contributor Author

Mido-sys commented Jun 8, 2022

A quick fix will be adding this :

if node.Operator == "||" {
	if c.isTruthy(lres) {
		return true, nil
	}
}

Here but it will still fail if the "true" condition is on the right of the operator and not left like this one:

	ctx.Set("path", "cart")
	ctx.Set("paths", "cart")
	input := `<%= if ( path == "pagePath" || (page && page.PageTitle != "cafe") || paths == "cart") { %>hi<%} %>`

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment. Otherwise, this will be closed in 7 days.

@github-actions github-actions bot added the stale label Sep 27, 2022
@mattbnz
Copy link

mattbnz commented Sep 27, 2022

I've hit this issue too so I'd like this to remain open rather than be auto-closed as stale. It's definitely a pain-point with plush.

@sio4
Copy link
Member

sio4 commented Sep 27, 2022

I've hit this issue too so I'd like this to remain open rather than be auto-closed as stale. It's definitely a pain-point with plush.

Sure, absolutely! I configured auto close action across all modules to make it easy to maintain, but if any issue is still meaningful, it should remain as open. The main purpose of this autoclose action is actually not to close issues but make it possible to get an insight into the issue. Also, the more information gathered, the better the solution will be.

@sio4 sio4 added s: triage and removed stale labels Sep 27, 2022
@Mido-sys Mido-sys changed the title Can't evaluated if conditions properly with an undefined variable Can't evaluate if conditions properly with an undefined variable Sep 27, 2022
@Mido-sys
Copy link
Contributor Author

@sio4, I pushed a fix for this.

Thanks, @mattbnz, for the reminder.

@sio4
Copy link
Member

sio4 commented Sep 27, 2022

@sio4, I pushed a fix for this.

Thanks, @mattbnz, for the reminder.

Thank! Will check!

@Mido-sys
Copy link
Contributor Author

I pushed an improved version. I realized my first commit would have ignored syntax errors and returned true if one of the nodes was true. I also added more tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment