Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions libs/engine/src/scanner.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use anyhow::{Context, Result};
use gasguard_rule_engine::{RuleEngine, RuleViolation};
use gasguard_rule_engine::{RedundantBooleanComparisonsRule, RuleEngine, RuleViolation};
use gasguard_parser_rust::RustParser;
use gasguard_parser_solidity::SolidityParser;
use gasguard_parser_vyper::VyperParser;
Expand All @@ -20,8 +20,11 @@ pub struct ContractScanner {

impl ContractScanner {
pub fn new() -> Self {
let rule_engine = RuleEngine::new();
// Rules will be added here or via plugins
let mut rule_engine = RuleEngine::new();

// Default built-in rules
rule_engine.add_rule(Box::new(RedundantBooleanComparisonsRule::default()));

Self { rule_engine }
}

Expand Down
5 changes: 4 additions & 1 deletion libs/rule-engine/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use gasguard_ast::UnifiedAST;
use serde::{Deserialize, Serialize};
use serde_json::json;
use std::collections::{HashMap, HashSet, VecDeque};
use std::collections::{HashMap, HashSet};

pub mod rules;
pub use rules::optimization::style::RedundantBooleanComparisonsRule;

#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct RuleViolation {
Expand Down
2 changes: 2 additions & 0 deletions libs/rule-engine/src/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pub mod optimization;

2 changes: 2 additions & 0 deletions libs/rule-engine/src/rules/optimization/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pub mod style;

4 changes: 4 additions & 0 deletions libs/rule-engine/src/rules/optimization/style/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
mod redundant_boolean_comparisons;

pub use redundant_boolean_comparisons::RedundantBooleanComparisonsRule;

Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
use crate::{Rule, RuleViolation, ViolationSeverity};
use gasguard_ast::{Language, UnifiedAST};

#[derive(Default)]
pub struct RedundantBooleanComparisonsRule;

impl RedundantBooleanComparisonsRule {
fn scan_line(line: &str) -> Vec<(usize, String, String)> {
// Returns: (column_number_1_based, matched_comparison, suggested_replacement)
let bytes = line.as_bytes();
let mut findings = Vec::new();

let mut i = 0;
while i + 1 < bytes.len() {
let op = match (bytes[i], bytes[i + 1]) {
(b'=', b'=') => "==",
(b'!', b'=') => "!=",
_ => {
i += 1;
continue;
}
};

// Parse RHS boolean literal: skip whitespace after operator.
let mut rhs_start = i + 2;
while rhs_start < bytes.len() && bytes[rhs_start].is_ascii_whitespace() {
rhs_start += 1;
}

let rhs_is_true = bytes.get(rhs_start..rhs_start + 4) == Some(b"true");
let rhs_is_false = bytes.get(rhs_start..rhs_start + 5) == Some(b"false");

// Parse LHS token: skip whitespace before operator, then grab a "token-ish" span.
let mut lhs_end = i;
while lhs_end > 0 && bytes[lhs_end - 1].is_ascii_whitespace() {
lhs_end -= 1;
}
let mut lhs_start = lhs_end;
while lhs_start > 0 {
let c = bytes[lhs_start - 1];
let ok = c.is_ascii_alphanumeric()
|| matches!(c, b'_' | b'.' | b'!' | b')' | b'(' | b']' | b'[');
if !ok {
break;
}
lhs_start -= 1;
}
let lhs_token_raw = line.get(lhs_start..lhs_end).unwrap_or("").trim();
let lhs_token = lhs_token_raw.trim_matches(|c| c == '(' || c == ')');

// Also support "true == x" / "false != x" (literal on LHS).
let lhs_is_true = lhs_token == "true";
let lhs_is_false = lhs_token == "false";

// Parse RHS token if boolean literal is on LHS.
let rhs_token = if lhs_is_true || lhs_is_false {
// Skip whitespace after operator.
let mut tok_start = rhs_start;
while tok_start < bytes.len() && bytes[tok_start].is_ascii_whitespace() {
tok_start += 1;
}
let mut tok_end = tok_start;
while tok_end < bytes.len() {
let c = bytes[tok_end];
let ok = c.is_ascii_alphanumeric()
|| matches!(c, b'_' | b'.' | b'!' | b')' | b'(' | b']' | b'[');
if !ok {
break;
}
tok_end += 1;
}
let tok = line.get(tok_start..tok_end).unwrap_or("").trim();
tok.trim_matches(|c| c == '(' || c == ')')
} else {
""
};

let (expr, literal, literal_on_rhs) = if rhs_is_true || rhs_is_false {
(lhs_token, if rhs_is_true { "true" } else { "false" }, true)
} else if lhs_is_true || lhs_is_false {
(rhs_token, if lhs_is_true { "true" } else { "false" }, false)
} else {
i += 2;
continue;
};

// Avoid empty expr (malformed token capture).
if expr.is_empty() {
i += 2;
continue;
}

let simplified = simplify(expr, op, literal);
let comparison = if literal_on_rhs {
format!("{expr} {op} {literal}")
} else {
format!("{literal} {op} {expr}")
};

// Column is position of operator start (1-based).
findings.push((i + 1, comparison, simplified));

i += 2;
}

findings
}
}

impl Rule for RedundantBooleanComparisonsRule {
fn id(&self) -> &str {
"solidity-redundant-boolean-comparisons"
}

fn name(&self) -> &str {
"Redundant Boolean Comparisons"
}

fn description(&self) -> &str {
"Detects unnecessary boolean comparisons like `== true` or `!= false` and suggests simplified expressions"
}

fn dependencies(&self) -> Vec<String> {
Vec::new()
}

fn check(&self, ast: &UnifiedAST) -> Vec<RuleViolation> {
if ast.language != Language::Solidity {
return Vec::new();
}

let mut violations = Vec::new();

for (line_idx, line) in ast.source.lines().enumerate() {
let line_number = line_idx + 1;
for (column_number, comparison, simplified) in Self::scan_line(line) {
violations.push(RuleViolation {
rule_name: self.id().to_string(),
description: format!(
"Redundant boolean comparison `{}` can be simplified",
comparison
),
severity: ViolationSeverity::Info,
line_number,
column_number,
variable_name: comparison.clone(),
suggestion: format!("Replace `{}` with `{}`.", comparison, simplified),
});
}
}

violations
}
}

fn simplify(expr: &str, op: &str, literal: &str) -> String {
let want_truthy = match (op, literal) {
("==", "true") => true,
("==", "false") => false,
("!=", "true") => false,
("!=", "false") => true,
_ => true,
};

if want_truthy {
// Prefer `x` over `!!x` if we can avoid it.
expr.trim().to_string()
} else {
let trimmed = expr.trim();
if let Some(rest) = trimmed.strip_prefix('!') {
rest.trim().to_string()
} else {
format!("!{trimmed}")
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use gasguard_ast::{ContractNode, FunctionNode, Visibility};

#[test]
fn detects_common_boolean_comparisons() {
let source = r#"
contract C {
function f(bool x) public {
if (x == true) { }
if (x != false) { }
if (true == x) { }
if (false != x) { }
if (x == false) { }
if (x != true) { }
}
}
"#;

let ast = UnifiedAST {
language: Language::Solidity,
source: source.to_string(),
file_path: "C.sol".to_string(),
contracts: vec![ContractNode {
name: "C".to_string(),
functions: vec![FunctionNode {
name: "f".to_string(),
params: vec![],
return_type: None,
visibility: Visibility::Public,
decorators: vec![],
is_constructor: false,
is_external: false,
is_payable: false,
line_number: 3,
body_raw: String::new(),
}],
state_variables: vec![],
line_number: 2,
}],
structs: vec![],
enums: vec![],
};

let rule = RedundantBooleanComparisonsRule::default();
let violations = rule.check(&ast);

assert_eq!(violations.len(), 6);
assert!(violations.iter().any(|v| v.suggestion.contains("Replace `x == true` with `x`")));
assert!(violations.iter().any(|v| v.suggestion.contains("Replace `x != false` with `x`")));
assert!(violations.iter().any(|v| v.suggestion.contains("Replace `x == false` with `!x`")));
assert!(violations.iter().any(|v| v.suggestion.contains("Replace `x != true` with `!x`")));
}
}
Loading