Skip to content

derive: in complex conditions, wrap terms in as_bool() #203

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.

Sign up for GitHub

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

Merged
GuillaumeGomez merged 3 commits into from
Oct 20, 2024
Merged

derive: in complex conditions, wrap terms in as_bool() #203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a65a09b
derive: add `as_bool()` function
Kijewski Oct 16, 2024
ee08363
generator: add complex boolean condition test
Kijewski Oct 17, 2024
358165b
derive: in complex conditions, wrap terms in `as_bool()`
Kijewski Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions rinja/src/helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,3 +147,8 @@ impl FastWritable for Empty {
Ok(())
}
}

#[inline]
pub fn as_bool<T: PrimitiveType<Value = bool>>(value: T) -> bool {
value.get()
}
45 changes: 36 additions & 9 deletions rinja_derive/src/generator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ impl<'a> Generator<'a> {
this.visit_expr(ctx, &mut expr_buf, left)?;
this.visit_target(buf, true, true, target);
expr_buf.write(format_args!(" {op} "));
this.visit_expr(ctx, &mut expr_buf, right)?;
this.visit_condition(ctx, &mut expr_buf, right)?;
}
_ => {
this.visit_expr(ctx, &mut expr_buf, expr)?;
Expand All @@ -645,14 +645,8 @@ impl<'a> Generator<'a> {
}
buf.write(format_args!("= &{} {{", expr_buf.buf));
} else if cond_info.generate_condition {
// The following syntax `*(&(...) as &bool)` is used to
// trigger Rust's automatic dereferencing, to coerce
// e.g. `&&&&&bool` to `bool`. First `&(...) as &bool`
// coerces e.g. `&&&bool` to `&bool`. Then `*(&bool)`
// finally dereferences it to `bool`.
buf.write("*(&(");
buf.write(this.visit_expr_root(ctx, expr)?);
buf.write(") as &rinja::helpers::core::primitive::bool) {");
this.visit_condition(ctx, buf, expr)?;
buf.write('{');
}
} else if pos != 0 {
buf.write("} else {");
Expand Down Expand Up @@ -1496,6 +1490,39 @@ impl<'a> Generator<'a> {
})
}

fn visit_condition(
&mut self,
ctx: &Context<'_>,
buf: &mut Buffer,
expr: &WithSpan<'_, Expr<'_>>,
) -> Result<(), CompileError> {
match &**expr {
Expr::BoolLit(_) | Expr::IsDefined(_) | Expr::IsNotDefined(_) => {
self.visit_expr(ctx, buf, expr)?;
}
Expr::Unary("!", expr) => {
buf.write('!');
self.visit_condition(ctx, buf, expr)?;
}
Expr::BinOp(op @ ("&&" | "||"), left, right) => {
self.visit_condition(ctx, buf, left)?;
buf.write(format_args!(" {op} "));
self.visit_condition(ctx, buf, right)?;
}
Expr::Group(expr) => {
buf.write('(');
self.visit_condition(ctx, buf, expr)?;
buf.write(')');
}
_ => {
buf.write("rinja::helpers::as_bool(&(");
self.visit_expr(ctx, buf, expr)?;
buf.write("))");
}
}
Ok(())
}

fn visit_is_defined(
&mut self,
buf: &mut Buffer,
Expand Down
44 changes: 32 additions & 12 deletions rinja_derive/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ fn check_if_let() {
// In this test, we ensure that `query` never is `self.query`.
compare(
"{% if let Some(query) = s && !query.is_empty() %}{{query}}{% endif %}",
r"if let Some(query,) = &self.s && !query.is_empty() {
r"if let Some(query,) = &self.s && !rinja::helpers::as_bool(&(query.is_empty())) {
Copy link
Collaborator

@GuillaumeGomez GuillaumeGomez Oct 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In such cases, I'm not sure it's an improvement since the ! operator tells us that it's already a boolean.

match (
&((&&rinja::filters::AutoEscaper::new(&(query), rinja::filters::Text)).rinja_auto_escape()?),
) {
Expand Down Expand Up @@ -174,7 +174,7 @@ fn check_if_let() {
// condition.
compare(
"{% if let Some(s) = s && !s.is_empty() %}{{s}}{% endif %}",
r"if let Some(s,) = &self.s && !s.is_empty() {
r"if let Some(s,) = &self.s && !rinja::helpers::as_bool(&(s.is_empty())) {
match (
&((&&rinja::filters::AutoEscaper::new(&(s), rinja::filters::Text)).rinja_auto_escape()?),
) {
Expand Down Expand Up @@ -293,7 +293,7 @@ writer.write_str("bla")?;"#,
"{% if x == 12 %}bli
{%- else if x is defined %}12
{%- else %}nope{% endif %}",
r#"if *(&(self.x == 12) as &rinja::helpers::core::primitive::bool) {
r#"if rinja::helpers::as_bool(&(self.x == 12)) {
writer.write_str("bli")?;
} else {
writer.write_str("12")?;
Expand All @@ -306,7 +306,7 @@ writer.write_str("12")?;
// are present.
compare(
"{% if y is defined || x == 12 %}{{x}}{% endif %}",
r"if *(&(self.x == 12) as &rinja::helpers::core::primitive::bool) {
r"if rinja::helpers::as_bool(&(self.x == 12)) {
match (
&((&&rinja::filters::AutoEscaper::new(&(self.x), rinja::filters::Text)).rinja_auto_escape()?),
) {
Expand Down Expand Up @@ -347,7 +347,7 @@ writer.write_str("12")?;
compare(
"{% if y is defined && y == 12 %}{{y}}{% else %}bli{% endif %}",
r#"
if *(&(self.y == 12) as &rinja::helpers::core::primitive::bool) {
if rinja::helpers::as_bool(&(self.y == 12)) {
match (
&((&&rinja::filters::AutoEscaper::new(
&(self.y),
Expand Down Expand Up @@ -418,31 +418,31 @@ match (
// Ensure that the `!` is kept .
compare(
"{% if y is defined && !y %}bla{% endif %}",
r#"if *(&(!self.y) as &rinja::helpers::core::primitive::bool) {
r#"if !rinja::helpers::as_bool(&(self.y)) {
writer.write_str("bla")?;
}"#,
&[("y", "bool")],
3,
);
compare(
"{% if y is defined && !(y) %}bla{% endif %}",
r#"if *(&(!(self.y)) as &rinja::helpers::core::primitive::bool) {
r#"if !(rinja::helpers::as_bool(&(self.y))) {
writer.write_str("bla")?;
}"#,
&[("y", "bool")],
3,
);
compare(
"{% if y is not defined || !y %}bla{% endif %}",
r#"if *(&(!self.y) as &rinja::helpers::core::primitive::bool) {
r#"if !rinja::helpers::as_bool(&(self.y)) {
writer.write_str("bla")?;
}"#,
&[("y", "bool")],
3,
);
compare(
"{% if y is not defined || !(y) %}bla{% endif %}",
r#"if *(&(!(self.y)) as &rinja::helpers::core::primitive::bool) {
r#"if !(rinja::helpers::as_bool(&(self.y))) {
writer.write_str("bla")?;
}"#,
&[("y", "bool")],
Expand Down Expand Up @@ -507,7 +507,7 @@ fn check_bool_conditions() {
);
compare(
"{% if false || x == 12 %}{{x}}{% endif %}",
r"if *(&(self.x == 12) as &rinja::helpers::core::primitive::bool) {
r"if rinja::helpers::as_bool(&(self.x == 12)) {
match (
&((&&rinja::filters::AutoEscaper::new(
&(self.x),
Expand All @@ -531,7 +531,7 @@ fn check_bool_conditions() {
// condition.
compare(
"{% if y == 3 || (true || x == 12) %}{{x}}{% endif %}",
r"if *(&(self.y == 3 || (true)) as &rinja::helpers::core::primitive::bool) {
r"if rinja::helpers::as_bool(&(self.y == 3)) || (true) {
match (
&((&&rinja::filters::AutoEscaper::new(
&(self.x),
Expand Down Expand Up @@ -569,7 +569,10 @@ fn check_bool_conditions() {
);
compare(
"{% if y == 3 || (x == 12 || true) %}{{x}}{% endif %}",
r"if *(&(self.y == 3 || (self.x == 12 || true)) as &rinja::helpers::core::primitive::bool) {
r"
if rinja::helpers::as_bool(&(self.y == 3))
Copy link
Collaborator

@GuillaumeGomez GuillaumeGomez Oct 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same in this case, it's a comparison so we already know for sure it's a boolean.

|| (rinja::helpers::as_bool(&(self.x == 12)) || true)
{
match (
&((&&rinja::filters::AutoEscaper::new(
&(self.x),
Expand Down Expand Up @@ -600,6 +603,23 @@ fn check_bool_conditions() {
&[],
3,
);

// Complex condition
compare(
"{% if (a || !b) && !(c || !d) %}x{% endif %}",
r#"
if (
rinja::helpers::as_bool(&(self.a))
|| !rinja::helpers::as_bool(&(self.b))
) && !(
rinja::helpers::as_bool(&(self.c))
|| !rinja::helpers::as_bool(&(self.d))
) {
writer.write_str("x")?;
}"#,
&[("a", "i32"), ("b", "i32"), ("c", "i32"), ("d", "i32")],
1,
);
}

#[test]
Expand Down
Loading