Skip to content

feat: Configure Test Harness & Coverage #2

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

Open
JerryIdoko wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: Configure Test Harness & Coverage #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,4 @@ cache/
artifacts/
out/
.soroban/
tarpaulin-report.html
25 changes: 0 additions & 25 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,3 @@ edition = "2021"
authors = ["GasGuard Team"]
license = "MIT"
description = "Automated Optimization Suite for Stellar Soroban Contracts"

[workspace.dependencies]
# Async runtime
tokio = { version = "1.0", features = ["full"] }

# CLI and argument parsing
clap = { version = "4.0", features = ["derive"] }

# Serialization
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"

# Error handling
anyhow = "1.0"
thiserror = "1.0"

# Parsing and AST
syn = { version = "2.0", features = ["full", "extra-traits"] }
quote = "1.0"
proc-macro2 = "1.0"

# Utilities
colored = "2.0"
walkdir = "2.0"
chrono = { version = "0.4", features = ["serde"] }
4 changes: 4 additions & 0 deletions libs/engine/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@ anyhow = "1.0"
colored = "2.0"
chrono = { version = "0.4", features = ["serde"] }
walkdir = "2.0"

[dev-dependencies]
mockall = "0.14.0"
rstest = "0.26.1"
7 changes: 5 additions & 2 deletions libs/engine/src/analyzer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl ScanAnalyzer {
let mut estimated_savings_kb = 0.0;

for violation in violations {
if violation.rule_name == "unused-state-variables" {
if violation.rule_name == "unused-state-variable" {
unused_vars += 1;
// Estimate average storage cost per unused variable
// This is a rough estimate - actual costs vary by type
Expand Down Expand Up @@ -92,6 +92,9 @@ impl ScanAnalyzer {
for violation in violations {
match violation.severity {
ViolationSeverity::Error => errors.push(violation),
// Map High and Medium to Warnings for now
ViolationSeverity::High => warnings.push(violation),
ViolationSeverity::Medium => warnings.push(violation),
ViolationSeverity::Warning => warnings.push(violation),
ViolationSeverity::Info => info.push(violation),
}
Expand Down Expand Up @@ -136,4 +139,4 @@ impl fmt::Display for StorageSavings {
self.monthly_ledger_rent_savings
)
}
}
}
4 changes: 4 additions & 0 deletions packages/rules/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
thiserror = "1.0"
regex = "1.10"

[dev-dependencies]
mockall = "0.14.0"
rstest = "0.26.1"
24 changes: 20 additions & 4 deletions packages/rules/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,23 @@ pub mod unused_state_variables;
pub mod vyper;
pub mod soroban;

pub use rule_engine::*;
pub use unused_state_variables::*;
pub use vyper::*;
pub use soroban::*;
// Explicitly export core types to avoid ambiguity
pub use rule_engine::{Rule, RuleEngine, RuleViolation, ViolationSeverity, extract_struct_fields, find_variable_usage};
pub use unused_state_variables::UnusedStateVariablesRule;

// Export Soroban types specifically
pub use soroban::{
SorobanAnalyzer,
SorobanContract,
SorobanParser,
SorobanResult,
SorobanRuleEngine,
SorobanStruct,
SorobanImpl,
SorobanFunction,
SorobanField,
SorobanParam
};

// Export Vyper types (keeping glob here is fine if Vyper module is clean, but let's be safe)
pub use vyper::*;
82 changes: 18 additions & 64 deletions packages/rules/src/rule_engine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
//! Rule Engine Core
//!
//! Provides the fundamental traits and AST traversal logic for the rules engine.

use serde::{Deserialize, Serialize};
use std::collections::HashSet;
use syn::{Expr, Item, ItemImpl, ItemStruct, Member, Pat};

// Re-export from soroban module
pub use crate::soroban::{SorobanRuleEngine, SorobanContract, SorobanParser, SorobanResult};

#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct RuleViolation {
pub rule_name: String,
Expand All @@ -19,6 +20,8 @@ pub struct RuleViolation {
#[derive(Debug, Clone, Serialize, Deserialize)]
pub enum ViolationSeverity {
Error,
High,
Medium,
Warning,
Info,
}
Expand Down Expand Up @@ -100,7 +103,6 @@ fn extract_variables_from_expr(expr: &Expr, used_vars: &mut HashSet<String>) {
Expr::Path(path) => {
if let Some(segment) = path.path.segments.last() {
let ident = segment.ident.to_string();
// Skip common Rust keywords and types
if !is_rust_keyword(&ident) {
used_vars.insert(ident);
}
Expand All @@ -112,6 +114,10 @@ fn extract_variables_from_expr(expr: &Expr, used_vars: &mut HashSet<String>) {
used_vars.insert(ident.to_string());
}
}
Expr::Assign(assign) => {
extract_variables_from_expr(&assign.left, used_vars);
extract_variables_from_expr(&assign.right, used_vars);
}
Expr::MethodCall(method_call) => {
extract_variables_from_expr(&method_call.receiver, used_vars);
for arg in &method_call.args {
Expand Down Expand Up @@ -211,64 +217,12 @@ fn extract_variables_from_pat(pat: &Pat, used_vars: &mut HashSet<String>) {
fn is_rust_keyword(ident: &str) -> bool {
matches!(
ident,
"self"
| "Self"
| "super"
| "crate"
| "mod"
| "use"
| "pub"
| "const"
| "static"
| "let"
| "fn"
| "struct"
| "enum"
| "impl"
| "trait"
| "where"
| "for"
| "while"
| "loop"
| "if"
| "else"
| "match"
| "break"
| "continue"
| "return"
| "async"
| "await"
| "move"
| "ref"
| "mut"
| "unsafe"
| "extern"
| "type"
| "union"
| "macro"
| "Some"
| "None"
| "Ok"
| "Err"
| "Result"
| "Option"
| "Vec"
| "String"
| "str"
| "bool"
| "u8"
| "u16"
| "u32"
| "u64"
| "u128"
| "i8"
| "i16"
| "i32"
| "i64"
| "i128"
| "f32"
| "f64"
| "usize"
| "isize"
"self" | "Self" | "super" | "crate" | "mod" | "use" | "pub" | "const" | "static" | "let"
| "fn" | "struct" | "enum" | "impl" | "trait" | "where" | "for" | "while" | "loop"
| "if" | "else" | "match" | "break" | "continue" | "return" | "async" | "await"
| "move" | "ref" | "mut" | "unsafe" | "extern" | "type" | "union" | "macro" | "Some"
| "None" | "Ok" | "Err" | "Result" | "Option" | "Vec" | "String" | "str" | "bool"
| "u8" | "u16" | "u32" | "u64" | "u128" | "i8" | "i16" | "i32" | "i64" | "i128"
| "f32" | "f64" | "usize" | "isize"
)
}
}
47 changes: 30 additions & 17 deletions packages/rules/src/soroban/analyzer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ impl SorobanAnalyzer {
description: "Contract should have a constructor function for initialization".to_string(),
suggestion: "Add a 'new' function that initializes the contract state".to_string(),
line_number: 1,
column_number: 0,
variable_name: contract.name.clone(),
severity: ViolationSeverity::Warning,
});
Expand All @@ -112,6 +113,7 @@ impl SorobanAnalyzer {
description: "Consider adding an admin/owner field for access control".to_string(),
suggestion: "Add an 'admin: Address' field to your contract state".to_string(),
line_number: 1,
column_number: 0,
variable_name: contract.name.clone(),
severity: ViolationSeverity::Info,
});
Expand All @@ -128,14 +130,15 @@ impl SorobanAnalyzer {
// Count occurrences of field name in the source (excluding struct definition)
let field_usage_count = source.matches(&field.name).count();

// If field is only mentioned in its own declaration, it's likely unused
// (this is a heuristic - a more sophisticated analysis would be needed for production)
if field_usage_count <= 1 {
// Heuristic: Definition + Initialization = 2 occurrences.
// If it's <= 2, it's likely defined and initialized but never accessed again.
if field_usage_count <= 2 {
violations.push(RuleViolation {
rule_name: "unused-state-variable".to_string(),
description: format!("State variable '{}' appears to be unused", field.name),
suggestion: format!("Remove unused state variable '{}' to save ledger storage", field.name),
line_number: field.line_number,
column_number: 0,
variable_name: field.name.clone(),
severity: ViolationSeverity::Warning,
});
Expand All @@ -155,8 +158,9 @@ impl SorobanAnalyzer {
violations.push(RuleViolation {
rule_name: "inefficient-integer-type".to_string(),
description: format!("Field '{}' uses {} which may be unnecessarily large", field.name, field.type_name),
suggestion: format!("Consider using a smaller integer type like u64 or u32 if the range permits", field.name),
suggestion: "Consider using a smaller integer type like u64 or u32 if the range permits".to_string(),
line_number: field.line_number,
column_number: 0,
variable_name: field.name.clone(),
severity: ViolationSeverity::Info,
});
Expand All @@ -169,6 +173,7 @@ impl SorobanAnalyzer {
description: format!("Field '{}' uses String type", field.name),
suggestion: "Consider using Symbol for fixed string values to save storage costs".to_string(),
line_number: field.line_number,
column_number: 0,
variable_name: field.name.clone(),
severity: ViolationSeverity::Info,
});
Expand All @@ -189,6 +194,7 @@ impl SorobanAnalyzer {
description: format!("Field '{}' is private but contract fields should typically be public", field.name),
suggestion: format!("Change '{}' to 'pub {}' to make it accessible", field.name, field.name),
line_number: field.line_number,
column_number: 0,
variable_name: field.name.clone(),
severity: ViolationSeverity::Warning,
});
Expand All @@ -199,17 +205,18 @@ impl SorobanAnalyzer {
}

/// Check for expensive operations in functions
fn check_expensive_operations(function: &SorobanFunction, source: &str) -> Vec<RuleViolation> {
fn check_expensive_operations(function: &SorobanFunction, _source: &str) -> Vec<RuleViolation> {
let mut violations = Vec::new();
let function_source = &function.raw_definition;

// Check for string operations
if function_source.contains(".to_string()") || function_source.contains("String::from(") {
violations.push(RuleViolation {
rule_name: "expensive-string-operation".to_string(),
description: "String operations can be expensive in terms of gas/storage",
suggestion: "Consider using Symbol or Bytes for fixed data, or minimize string operations",
description: "String operations can be expensive in terms of gas/storage".to_string(),
suggestion: "Consider using Symbol or Bytes for fixed data, or minimize string operations".to_string(),
line_number: function.line_number,
column_number: 0,
variable_name: function.name.clone(),
severity: ViolationSeverity::Medium,
});
Expand All @@ -219,9 +226,10 @@ impl SorobanAnalyzer {
if function_source.contains("Vec::new()") && !function_source.contains("with_capacity") {
violations.push(RuleViolation {
rule_name: "vec-without-capacity".to_string(),
description: "Vec::new() without capacity can cause multiple reallocations",
suggestion: "Use Vec::with_capacity() to pre-allocate memory when size is known",
description: "Vec::new() without capacity can cause multiple reallocations".to_string(),
suggestion: "Use Vec::with_capacity() to pre-allocate memory when size is known".to_string(),
line_number: function.line_number,
column_number: 0,
variable_name: function.name.clone(),
severity: ViolationSeverity::Medium,
});
Expand All @@ -231,9 +239,10 @@ impl SorobanAnalyzer {
if function_source.contains(".clone()") {
violations.push(RuleViolation {
rule_name: "unnecessary-clone".to_string(),
description: "Clone operations increase resource usage and gas costs",
suggestion: "Avoid unnecessary cloning, use references where possible",
description: "Clone operations increase resource usage and gas costs".to_string(),
suggestion: "Avoid unnecessary cloning, use references where possible".to_string(),
line_number: function.line_number,
column_number: 0,
variable_name: function.name.clone(),
severity: ViolationSeverity::Medium,
});
Expand All @@ -254,8 +263,9 @@ impl SorobanAnalyzer {
violations.push(RuleViolation {
rule_name: "missing-address-validation".to_string(),
description: format!("Function '{}' takes Address parameter but may lack validation", function.name),
suggestion: "Validate Address parameters to prevent invalid addresses",
suggestion: "Validate Address parameters to prevent invalid addresses".to_string(),
line_number: function.line_number,
column_number: 0,
variable_name: function.name.clone(),
severity: ViolationSeverity::Medium,
});
Expand All @@ -279,8 +289,9 @@ impl SorobanAnalyzer {
violations.push(RuleViolation {
rule_name: "missing-error-handling".to_string(),
description: format!("Function '{}' should return Result for error handling", function.name),
suggestion: "Return Result<(), Error> to properly handle operation failures",
suggestion: "Return Result<(), Error> to properly handle operation failures".to_string(),
line_number: function.line_number,
column_number: 0,
variable_name: function.name.clone(),
severity: ViolationSeverity::Medium,
});
Expand All @@ -291,7 +302,7 @@ impl SorobanAnalyzer {
}

/// Check for unbounded loops
fn check_unbounded_loops(implementation: &SorobanImpl, source: &str) -> Vec<RuleViolation> {
fn check_unbounded_loops(implementation: &SorobanImpl, _source: &str) -> Vec<RuleViolation> {
let mut violations = Vec::new();

for function in &implementation.functions {
Expand All @@ -304,8 +315,9 @@ impl SorobanAnalyzer {
violations.push(RuleViolation {
rule_name: "unbounded-loop".to_string(),
description: format!("Function '{}' contains potentially unbounded loop", function.name),
suggestion: "Ensure loops have clear termination conditions to prevent CPU limit exhaustion",
suggestion: "Ensure loops have clear termination conditions to prevent CPU limit exhaustion".to_string(),
line_number: function.line_number,
column_number: 0,
variable_name: function.name.clone(),
severity: ViolationSeverity::High,
});
Expand All @@ -316,7 +328,7 @@ impl SorobanAnalyzer {
}

/// Check for inefficient storage patterns
fn check_storage_patterns(implementation: &SorobanImpl, source: &str) -> Vec<RuleViolation> {
fn check_storage_patterns(implementation: &SorobanImpl, _source: &str) -> Vec<RuleViolation> {
let mut violations = Vec::new();

// Check for multiple storage reads of the same key
Expand All @@ -339,8 +351,9 @@ impl SorobanAnalyzer {
violations.push(RuleViolation {
rule_name: "inefficient-storage-access".to_string(),
description: format!("Function '{}' performs {} storage reads - consider caching", function.name, read_count),
suggestion: "Cache frequently accessed storage values in local variables",
suggestion: "Cache frequently accessed storage values in local variables".to_string(),
line_number: function.line_number,
column_number: 0,
variable_name: function.name.clone(),
severity: ViolationSeverity::Medium,
});
Expand Down
Loading