Merge pull request #31 from man-croft/feat/vyper-redundant-external-detector

feat(vyper): Add detector for redundant @external decorators

Author: mijinummi

apps/api/src/main.rs

@@ -1,7 +1,8 @@
+use anyhow::Result;
 use clap::{Parser, Subcommand};
+use colored::Colorize;
 use gasguard_engine::{ContractScanner, ScanAnalyzer};
 use std::path::PathBuf;
-use anyhow::Result;
 
 #[derive(Parser)]
 #[command(name = "gasguard")]
@@ -45,17 +46,17 @@ async fn main() -> Result<()> {
     match cli.command {
         Commands::Scan { file, format } => {
             println!("🔍 Scanning file: {:?}", file);
-            
+
             let result = scanner.scan_file(&file)?;
-            
+
             match format.as_str() {
                 "json" => {
                     println!("{}", result.to_json()?);
                 }
                 _ => {
                     println!("{}", ScanAnalyzer::format_violations(&result.violations));
                     println!("{}", ScanAnalyzer::generate_summary(&result.violations));
-                    
+
                     if !result.violations.is_empty() {
                         let savings = ScanAnalyzer::calculate_storage_savings(&result.violations);
                         println!("\n{}", savings);
@@ -65,16 +66,16 @@ async fn main() -> Result<()> {
         }
         Commands::ScanDir { directory, format } => {
             println!("🔍 Scanning directory: {:?}", directory);
-            
+
             let results = scanner.scan_directory(&directory)?;
-            
+
             if results.is_empty() {
                 println!("✅ No violations found in any files!");
                 return Ok(());
             }
-            
+
             let total_violations: usize = results.iter().map(|r| r.violations.len()).sum();
-            
+
             match format.as_str() {
                 "json" => {
                     println!("{}", serde_json::to_string_pretty(&results)?);
@@ -84,49 +85,62 @@ async fn main() -> Result<()> {
                         println!("\n📁 File: {}", result.source);
                         println!("{}", ScanAnalyzer::format_violations(&result.violations));
                     }
-                    
-                    println!("\n{}", format!("📊 Total violations across {} files: {}", results.len(), total_violations).bold());
-                    
-                    let all_violations: Vec<_> = results.iter().flat_map(|r| r.violations.clone()).collect();
+
+                    println!(
+                        "\n{}",
+                        format!(
+                            "📊 Total violations across {} files: {}",
+                            results.len(),
+                            total_violations
+                        )
+                        .bold()
+                    );
+
+                    let all_violations: Vec<_> =
+                        results.iter().flat_map(|r| r.violations.clone()).collect();
                     let savings = ScanAnalyzer::calculate_storage_savings(&all_violations);
                     println!("\n{}", savings);
                 }
             }
         }
         Commands::Analyze { path } => {
             println!("📊 Analyzing storage optimization potential: {:?}", path);
-            
+
             let results = if path.is_file() {
                 vec![scanner.scan_file(&path)?]
             } else {
                 scanner.scan_directory(&path)?
             };
-            
+
             if results.is_empty() {
                 println!("✅ No optimization opportunities found!");
                 return Ok(());
             }
-            
-            let all_violations: Vec<_> = results.iter().flat_map(|r| r.violations.clone()).collect();
+
+            let all_violations: Vec<_> =
+                results.iter().flat_map(|r| r.violations.clone()).collect();
             let savings = ScanAnalyzer::calculate_storage_savings(&all_violations);
-            
+
             println!("\n🎯 Storage Analysis Report");
             println!("========================");
             println!("Files analyzed: {}", results.len());
             println!("Total violations: {}", all_violations.len());
             println!("\n{}", savings);
-            
+
             // Group violations by type
             let mut unused_vars = 0;
             for violation in &all_violations {
                 if violation.rule_name == "unused-state-variables" {
                     unused_vars += 1;
                 }
             }
-            
+
             if unused_vars > 0 {
                 println!("\n🔧 Recommendations:");
-                println!("  • Remove {} unused state variables to reduce storage costs", unused_vars);
+                println!(
+                    "  • Remove {} unused state variables to reduce storage costs",
+                    unused_vars
+                );
                 println!("  • Consider using more efficient data types where possible");
                 println!("  • Implement lazy loading patterns for rarely accessed data");
             }

examples/redundant_external.vy

@@ -0,0 +1,71 @@
+# @version ^0.3.0
+
+# Example Vyper contract demonstrating redundant @external decorators
+# This file contains violations that GasGuard should detect
+
+# Storage variables
+owner: public(address)
+balance: public(uint256)
+fee_rate: uint256
+
+@external
+def __init__():
+    """Initialize the contract - legitimate @external"""
+    self.owner = msg.sender
+    self.balance = 0
+    self.fee_rate = 30  # 0.3%
+
+# VIOLATION 1: Internal naming convention with @external
+# Functions starting with _ should be @internal
+@external
+def _calculate_fee(amount: uint256) -> uint256:
+    """This function uses internal naming convention but is marked external"""
+    return amount * self.fee_rate / 10000
+
+# VIOLATION 2: Another internal-named function with @external
+@external
+def _validate_amount(amount: uint256) -> bool:
+    """Validation helper that should be internal"""
+    return amount > 0 and amount <= self.balance
+
+# CORRECT: Properly marked internal function
+@internal
+def _update_balance(new_balance: uint256):
+    """Internal helper - correctly marked"""
+    self.balance = new_balance
+
+# CORRECT: Legitimate external function
+@external
+def deposit():
+    """Public deposit function - legitimate @external"""
+    self.balance += msg.value
+
+# CORRECT: Legitimate external function
+@external
+def withdraw(amount: uint256):
+    """Public withdraw function - legitimate @external"""
+    assert self._validate_amount(amount), "Invalid amount"
+    fee: uint256 = self._calculate_fee(amount)
+    self._update_balance(self.balance - amount)
+    send(msg.sender, amount - fee)
+
+# VIOLATION 3: Helper function only called internally
+@external
+def calculate_withdrawal_fee(amount: uint256) -> uint256:
+    """This is only called internally but marked external"""
+    return self._calculate_fee(amount)
+
+# CORRECT: View function for external queries
+@external
+@view
+def get_balance() -> uint256:
+    """External view function - legitimate @external"""
+    return self.balance
+
+# CORRECT: External function with proper visibility
+@external
+def transfer(to: address, amount: uint256):
+    """Transfer function - legitimate @external"""
+    assert self._validate_amount(amount), "Invalid amount"
+    self._update_balance(self.balance - amount)
+    send(to, amount)

libs/engine/Cargo.toml

@@ -11,3 +11,5 @@ serde_json = "1.0"
 tokio = { version = "1.0", features = ["full"] }
 anyhow = "1.0"
 colored = "2.0"
+chrono = { version = "0.4", features = ["serde"] }
+walkdir = "2.0"

libs/engine/src/analyzer.rs

@@ -1,58 +1,67 @@
-use gasguard_rules::{RuleViolation, ViolationSeverity};
 use colored::*;
+use gasguard_rules::{RuleViolation, ViolationSeverity};
 use std::fmt;
 
 pub struct ScanAnalyzer;
 
 impl ScanAnalyzer {
     pub fn format_violations(violations: &[RuleViolation]) -> String {
         if violations.is_empty() {
-            return "✅ No violations found! Your contract is optimized.".green().to_string();
+            return "✅ No violations found! Your contract is optimized."
+                .green()
+                .to_string();
         }
-        
+
         let mut output = String::new();
         let (errors, warnings, info) = Self::categorize_violations(violations);
-        
+
         if !errors.is_empty() {
             output.push_str(&format!("🚨 {} Errors:\n", errors.len()).red().bold());
             for violation in errors {
                 output.push_str(&Self::format_single_violation(violation, "ERROR"));
             }
             output.push('\n');
         }
-        
+
         if !warnings.is_empty() {
-            output.push_str(&format!("⚠️  {} Warnings:\n", warnings.len()).yellow().bold());
+            output.push_str(
+                &format!("⚠️  {} Warnings:\n", warnings.len())
+                    .yellow()
+                    .bold(),
+            );
             for violation in warnings {
                 output.push_str(&Self::format_single_violation(violation, "WARNING"));
             }
             output.push('\n');
         }
-        
+
         if !info.is_empty() {
             output.push_str(&format!("ℹ️  {} Info:\n", info.len()).blue().bold());
             for violation in info {
                 output.push_str(&Self::format_single_violation(violation, "INFO"));
             }
         }
-        
+
         output
     }
-    
+
     pub fn generate_summary(violations: &[RuleViolation]) -> String {
         let total = violations.len();
         let (errors, warnings, info) = Self::categorize_violations(violations);
-        
+
         format!(
             "Scan Summary: {} total violations ({} errors, {} warnings, {} info)",
-            total, errors.len(), warnings.len(), info.len()
+            total,
+            errors.len(),
+            warnings.len(),
+            info.len()
         )
     }
-    
+
     pub fn calculate_storage_savings(violations: &[RuleViolation]) -> StorageSavings {
         let mut unused_vars = 0;
         let mut estimated_savings_kb = 0.0;
-        
+
         for violation in violations {
             if violation.rule_name == "unused-state-variables" {
                 unused_vars += 1;
@@ -61,38 +70,44 @@ impl ScanAnalyzer {
                 estimated_savings_kb += 2.5; // Average variable size in KB
             }
         }
-        
+
         StorageSavings {
             unused_variables: unused_vars,
             estimated_savings_kb,
             monthly_ledger_rent_savings: estimated_savings_kb * 0.001, // Rough estimate
         }
     }
-    
-    fn categorize_violations(violations: &[RuleViolation]) -> (Vec<&RuleViolation>, Vec<&RuleViolation>, Vec<&RuleViolation>) {
+
+    fn categorize_violations(
+        violations: &[RuleViolation],
+    ) -> (
+        Vec<&RuleViolation>,
+        Vec<&RuleViolation>,
+        Vec<&RuleViolation>,
+    ) {
         let mut errors = Vec::new();
         let mut warnings = Vec::new();
         let mut info = Vec::new();
-        
+
         for violation in violations {
             match violation.severity {
                 ViolationSeverity::Error => errors.push(violation),
                 ViolationSeverity::Warning => warnings.push(violation),
                 ViolationSeverity::Info => info.push(violation),
             }
         }
-        
+
         (errors, warnings, info)
     }
-    
+
     fn format_single_violation(violation: &RuleViolation, severity: &str) -> String {
         let severity_color = match severity {
             "ERROR" => colored::Color::Red,
             "WARNING" => colored::Color::Yellow,
             "INFO" => colored::Color::Blue,
             _ => colored::Color::White,
         };
-        
+
         format!(
             "{}\n  📍 Line {}: {}\n  📝 {}\n  💡 {}\n\n",
             format!("  [{}]", severity).color(severity_color).bold(),

libs/engine/src/lib.rs

@@ -1,5 +1,5 @@
-pub mod scanner;
 pub mod analyzer;
+pub mod scanner;
 
-pub use scanner::*;
 pub use analyzer::*;
+pub use scanner::*;