feat: accept comparison between boolean expression and integer constant

Expressions like `pe.is_signed == 1` are now accepted. This kind of expressions are valid in legacy YARA, and very common. So we are now accepting them, but raising a warning.

lib/src/compiler/ir/ast2ir.rs

@@ -438,7 +438,52 @@ pub(in crate::compiler) fn expr_from_ast(
         ast::Expr::BitwiseXor(expr) => bitwise_xor_expr_from_ast(ctx, expr),
 
         // Comparison operations
-        ast::Expr::Eq(expr) => eq_expr_from_ast(ctx, expr),
+        ast::Expr::Eq(expr) => {
+            let span = expr.span();
+
+            let lhs_span = expr.lhs.span();
+            let rhs_span = expr.rhs.span();
+
+            let lhs = expr_from_ast(ctx, &expr.lhs)?;
+            let rhs = expr_from_ast(ctx, &expr.rhs)?;
+
+            // Detect cases in which the equal operator is comparing a boolean
+            // expression with an integer constant (e.g: `pe.is_signed == 0`).
+            // This is quite common in YARA rules, it is accepted without
+            // errors, but a warning is raised.
+            let replacement = match (lhs.type_value(), rhs.type_value()) {
+                (TypeValue::Bool(_), TypeValue::Integer(Value::Const(0))) => {
+                    Some((
+                        Expr::Not { operand: Box::new(lhs) },
+                        format!("not {}", ctx.report_builder.get_snippet(&lhs_span.into()))
+                    ))
+                }
+                (TypeValue::Integer(Value::Const(0)), TypeValue::Bool(_)) => {
+                    Some((
+                        Expr::Not { operand: Box::new(rhs) },
+                        format!("not {}", ctx.report_builder.get_snippet(&rhs_span.into()))
+                    ))
+                }
+                (TypeValue::Bool(_), TypeValue::Integer(Value::Const(1))) => {
+                    Some((lhs, ctx.report_builder.get_snippet(&lhs_span.into())))
+                }
+                (TypeValue::Integer(Value::Const(1)), TypeValue::Bool(_)) => {
+                    Some((rhs, ctx.report_builder.get_snippet(&rhs_span.into())))
+                }
+                _ => None
+            };
+
+            if let Some((expr, msg)) = replacement {
+                ctx.warnings.add(|| Warning::boolean_integer_comparison(
+                    ctx.report_builder,
+                    span.into(),
+                    msg,
+                ));
+                Ok(expr)
+            } else {
+                eq_expr_from_ast(ctx, expr)
+            }
+        },
         ast::Expr::Ne(expr) => ne_expr_from_ast(ctx, expr),
         ast::Expr::Gt(expr) => gt_expr_from_ast(ctx, expr),
         ast::Expr::Ge(expr) => ge_expr_from_ast(ctx, expr),

lib/src/compiler/report.rs

@@ -137,6 +137,20 @@ impl ReportBuilder {
         self
     }
 
+    /// Returns the fragment of source code indicated by `source_ref`.
+    pub fn get_snippet(&self, source_ref: &SourceRef) -> String {
+        let source_id = source_ref
+            .source_id
+            .or_else(|| self.current_source_id())
+            .expect("create_report without registering any source code");
+
+        let cache = self.cache.borrow();
+        let cache_entry = cache.data.get(&source_id).unwrap();
+        let src = cache_entry.code.as_str();
+
+        src[source_ref.span.range()].to_string()
+    }
+
     /// Creates a new error or warning report.
     pub fn create_report(
         &self,

lib/src/compiler/tests/testdata/warnings/27.in

@@ -0,0 +1,5 @@
+import "test_proto2"
+
+rule test {
+	condition: test_proto2.array_bool[0] == 1
+}

lib/src/compiler/tests/testdata/warnings/27.out

@@ -0,0 +1,6 @@
+warning[bool_int_comparison]: comparison between boolean and integer
+ --> line:4:13
+  |
+4 |  condition: test_proto2.array_bool[0] == 1
+  |             ------------------------------ this comparison can be replaced with: `test_proto2.array_bool[0]`
+  |

lib/src/compiler/tests/testdata/warnings/28.in

@@ -0,0 +1,5 @@
+import "test_proto2"
+
+rule test {
+	condition: test_proto2.array_bool[0] == 0
+}

lib/src/compiler/tests/testdata/warnings/28.out

@@ -0,0 +1,6 @@
+warning[bool_int_comparison]: comparison between boolean and integer
+ --> line:4:13
+  |
+4 |  condition: test_proto2.array_bool[0] == 0
+  |             ------------------------------ this comparison can be replaced with: `not test_proto2.array_bool[0]`
+  |

lib/src/compiler/tests/testdata/warnings/29.in

@@ -0,0 +1,5 @@
+import "test_proto2"
+
+rule test {
+	condition: 1 == test_proto2.array_bool[0]
+}

lib/src/compiler/tests/testdata/warnings/29.out

@@ -0,0 +1,6 @@
+warning[bool_int_comparison]: comparison between boolean and integer
+ --> line:4:13
+  |
+4 |  condition: 1 == test_proto2.array_bool[0]
+  |             ------------------------------ this comparison can be replaced with: `test_proto2.array_bool[0]`
+  |

lib/src/compiler/tests/testdata/warnings/30.in

@@ -0,0 +1,5 @@
+import "test_proto2"
+
+rule test {
+	condition: 0 == test_proto2.array_bool[0]
+}

lib/src/compiler/tests/testdata/warnings/30.out

@@ -0,0 +1,6 @@
+warning[bool_int_comparison]: comparison between boolean and integer
+ --> line:4:13
+  |
+4 |  condition: 0 == test_proto2.array_bool[0]
+  |             ------------------------------ this comparison can be replaced with: `not test_proto2.array_bool[0]`
+  |

lib/src/compiler/warnings.rs

@@ -51,6 +51,14 @@ pub enum Warning {
         note: Option<String>,
     },
 
+    #[warning("bool_int_comparison", "comparison between boolean and integer")]
+    #[label("this comparison can be replaced with: `{replacement}`", span)]
+    BooleanIntegerComparison {
+        detailed_report: String,
+        span: SourceRef,
+        replacement: String,
+    },
+
     #[warning("duplicate_import", "duplicate import statement")]
     #[label(
       "duplicate import",

lib/src/modules/test_proto2/tests/mod.rs

@@ -75,6 +75,11 @@ fn test_proto2_module() {
     condition_false!(r#"test_proto2.array_bool[0]"#);
     condition_true!(r#"test_proto2.array_bool[1]"#);
 
+    condition_false!(r#"test_proto2.array_bool[0] == 1"#);
+    condition_true!(r#"test_proto2.array_bool[0] == 0"#);
+    condition_false!(r#"1 == test_proto2.array_bool[0]"#);
+    condition_true!(r#"0 == test_proto2.array_bool[0]"#);
+
     // array_int64[3] is undefined, so both conditions are false.
     condition_false!(r#"test_proto2.array_int64[3] == 0"#);
     condition_false!(r#"test_proto2.array_int64[3] != 0"#);