From 3d9ca8bb6984a6b91aed04d54524b80e33471a48 Mon Sep 17 00:00:00 2001 From: Harkaran Brar Date: Sun, 17 May 2026 14:02:46 -0700 Subject: [PATCH 1/2] fix: resolve critical bugs in file scanning, classification, and language detection - Fix files command ignoring user-provided CLI paths (was always scanning cwd) - Fix CStyleClassifier misclassifying inline /* */ comments as pure Comment - Fix PythonClassifier misclassifying triple-quotes with trailing code as Docstring - Add Rust (.rs), YAML (.yml/.yaml), TOML (.toml) language detection - Map Dockerfile/Makefile to Shell language classification - Remove unimplemented History command stub from CLI --- src/classifier.rs | 41 ++++++++++++++++++++++++++++++++-------- src/files.rs | 48 +++++++++++++++++++++++------------------------ src/language.rs | 22 ++++++++++++++++++---- 3 files changed, 75 insertions(+), 36 deletions(-) diff --git a/src/classifier.rs b/src/classifier.rs index 285e995..c2a53d1 100644 --- a/src/classifier.rs +++ b/src/classifier.rs @@ -68,9 +68,14 @@ impl Classifier for PythonClassifier { return LineType::Comment; } - if trimmed.starts_with("\"\"\"") { - let count = line.matches("\"\"\"").count(); - if count >= 2 { + if let Some(idx) = trimmed.find("\"\"\"") { + if trimmed.matches("\"\"\"").count() >= 2 { + // Single-line docstring. Check for code before or after. + let before = &trimmed[..idx]; + let after = &trimmed[idx + 3..]; + if !before.trim().is_empty() || !after.trim().is_empty() { + return LineType::Pure; + } return LineType::Docstring; } else { self.in_triple_double = true; @@ -78,9 +83,14 @@ impl Classifier for PythonClassifier { } } - if trimmed.starts_with("'''") { - let count = line.matches("'''").count(); - if count >= 2 { + if let Some(idx) = trimmed.find("'''") { + if trimmed.matches("'''").count() >= 2 { + // Single-line docstring. Check for code before or after. + let before = &trimmed[..idx]; + let after = &trimmed[idx + 3..]; + if !before.trim().is_empty() || !after.trim().is_empty() { + return LineType::Pure; + } return LineType::Docstring; } else { self.in_triple_single = true; @@ -133,9 +143,21 @@ impl Classifier for CStyleClassifier { if let Some(start_idx) = trimmed.find("/*") { if let Some(end_idx) = trimmed.find("*/") { if end_idx > start_idx { + // Single-line block comment. Check for code before or after. + let before = &trimmed[..start_idx]; + let after = &trimmed[end_idx + 2..]; + if !before.trim().is_empty() || !after.trim().is_empty() { + return LineType::Pure; + } return LineType::Comment; } } + // Starts but doesn't end on same line. Check for code before /*. + let before = &trimmed[..start_idx]; + if !before.trim().is_empty() { + self.in_block = true; + return LineType::Pure; + } self.in_block = true; return LineType::Comment; } @@ -287,8 +309,11 @@ pub fn get_classifier(lang: Language) -> Box { | Language::Swift | Language::Kotlin | Language::Scala - | Language::Css => Box::new(CStyleClassifier::new()), - Language::Shell | Language::PowerShell => Box::new(ShellClassifier), + | Language::Css + | Language::Rust => Box::new(CStyleClassifier::new()), + Language::Shell | Language::PowerShell | Language::Yaml | Language::Toml => { + Box::new(ShellClassifier) + } Language::Ruby => Box::new(RubyClassifier::new()), Language::Html | Language::Vue => Box::new(HtmlClassifier::new()), Language::Other => Box::new(DefaultClassifier), diff --git a/src/files.rs b/src/files.rs index 85b271e..1e765ee 100644 --- a/src/files.rs +++ b/src/files.rs @@ -8,9 +8,10 @@ use std::path::Path; use walkdir::WalkDir; pub fn analyze_files( + paths: &[String], include: &[String], exclude: &[String], - reader: Option>, // For stdin support + reader: Option>, ) -> Result, std::io::Error> { let mut stats = Vec::new(); @@ -40,32 +41,31 @@ pub fn analyze_files( .filter_map(|p| Pattern::new(p).ok()) .collect(); - for entry in WalkDir::new(".").into_iter().flatten() { - let path = entry.path(); - if path.is_dir() { - continue; - } + for root in paths { + for entry in WalkDir::new(root).into_iter().flatten() { + let path = entry.path(); + if path.is_dir() { + continue; + } - // Convert path to relative string for matching - let path_str = path.to_string_lossy(); - let clean_path = if let Some(stripped) = path_str.strip_prefix("./") { - stripped - } else { - &path_str - }; - - // Check excludes - if exclude_patterns.iter().any(|p| p.matches(clean_path)) { - continue; - } + let path_str = path.to_string_lossy(); + let clean_path = if let Some(stripped) = path_str.strip_prefix("./") { + stripped + } else { + &path_str + }; - // Check includes (at least one must match if we are strict, or default include is "**/*") - if !include_patterns.iter().any(|p| p.matches(clean_path)) { - continue; - } + if exclude_patterns.iter().any(|p| p.matches(clean_path)) { + continue; + } + + if !include_patterns.iter().any(|p| p.matches(clean_path)) { + continue; + } - if let Ok(fs) = process_file(path) { - stats.push(fs); + if let Ok(fs) = process_file(path) { + stats.push(fs); + } } } diff --git a/src/language.rs b/src/language.rs index 4ed4382..b1faaf9 100644 --- a/src/language.rs +++ b/src/language.rs @@ -21,6 +21,9 @@ pub enum Language { Shell, PowerShell, Vue, + Rust, + Yaml, + Toml, Other, } @@ -45,12 +48,20 @@ impl Language { Some("sh") | Some("bash") | Some("zsh") => Language::Shell, Some("ps1") | Some("psm1") => Language::PowerShell, Some("vue") => Language::Vue, + Some("rs") => Language::Rust, + Some("yml") | Some("yaml") => Language::Yaml, + Some("toml") => Language::Toml, _ => { // Check filename for special cases - match path.file_name().and_then(|n| n.to_str()) { - Some("Dockerfile") => Language::Other, // Or maybe shell-like? keeping Other for now - Some("Makefile") => Language::Other, - _ => Language::Other, + let name = path.file_name().and_then(|n| n.to_str()).unwrap_or(""); + if name == "Dockerfile" + || name.starts_with("Dockerfile.") + || name == "Makefile" + || name.ends_with(".mk") + { + Language::Shell + } else { + Language::Other } } } @@ -78,6 +89,9 @@ impl fmt::Display for Language { Language::Shell => "Shell", Language::PowerShell => "PowerShell", Language::Vue => "Vue", + Language::Rust => "Rust", + Language::Yaml => "YAML", + Language::Toml => "TOML", Language::Other => "Other", }; write!(f, "{}", s) From d1747e9ecad95209767bc1ec4c7d1ba7e3f5a122 Mon Sep 17 00:00:00 2001 From: Harkaran Brar Date: Sun, 17 May 2026 14:02:58 -0700 Subject: [PATCH 2/2] refactor: improve error handling, extract shared aggregation, and apply code quality fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace std::process::exit() with Result pattern in main() - Extract aggregate_stats() to stats.rs for shared use by report and threshold checks - Fix threshold aggregation missing removed/comment/docstring/blank fields - Fix JSON output silently swallowing serialization errors - Fix CI mode appending summary line after JSON (breaking parsers) - Apply clippy fixes: uninlined format args, #[must_use], const fn constructors - Replace wildcard import colored::* with explicit Colorize - Fix usizeā†’i64 casts with TryFrom to prevent wrap on 64-bit targets - Use let-else patterns, nested or-patterns, and move use statements to top --- src/main.rs | 130 ++++++++++++++++++-------------------------------- src/report.rs | 78 +++++++++++++----------------- src/stats.rs | 24 ++++++++++ 3 files changed, 104 insertions(+), 128 deletions(-) diff --git a/src/main.rs b/src/main.rs index cecab76..16f8194 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,10 +1,10 @@ use clap::{Parser, Subcommand, ValueEnum}; use purecode::{ config, diff, files, parser, report, - stats::{FileStats, LangStats, ThresholdError}, + stats::{self, FileStats, ThresholdError}, }; use std::io::BufReader; -use std::process::exit; +use std::process::ExitCode; #[derive(Parser, Debug)] #[command(name = "purecode")] @@ -16,7 +16,6 @@ struct Cli { #[command(subcommand)] command: Option, - // Fallback flags for root command (diff mode) /// Base ref for git diff #[arg(long)] base: Option, @@ -140,8 +139,6 @@ enum Commands { #[arg(long)] ci: bool, }, - /// History analysis (Scaffolding) - History, } #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum, Debug)] @@ -162,27 +159,28 @@ impl From for report::OutputFormat { } fn resolve_format(cli_format: Option, config_format: &str) -> Format { - if let Some(f) = cli_format { - f - } else { - match config_format { - "json" => Format::Json, - "plain" => Format::Plain, - _ => Format::Human, - } - } + cli_format.unwrap_or(match config_format { + "json" => Format::Json, + "plain" => Format::Plain, + _ => Format::Human, + }) } -fn main() { +struct FilesConfig { + format: Format, + per_file: bool, + max_noise_ratio: Option, + min_pure_lines: Option, + fail_on_decrease: bool, + warn_only: bool, + ci: bool, +} + +fn run() -> Result> { let cli = Cli::parse(); let config = config::load_config(); - // Determine mode and arguments let (stats, mode, active_config) = match cli.command { - Some(Commands::History) => { - println!("History analysis not implemented yet."); - exit(0); - } Some(Commands::Files { paths, stdin, @@ -195,13 +193,6 @@ fn main() { ci, }) => { let final_format = resolve_format(format, &config.format); - - let _files_to_scan = if paths == vec!["."] { - vec![".".to_string()] - } else { - paths - }; - let include = if config.include.is_empty() { vec!["**/*".to_string()] } else { @@ -215,13 +206,8 @@ fn main() { None }; - let stats = match files::analyze_files(&include, &exclude, reader) { - Ok(s) => s, - Err(e) => { - eprintln!("Error analyzing files: {}", e); - exit(1); - } - }; + let stats = files::analyze_files(&paths, &include, &exclude, reader) + .map_err(|e| format!("Error analyzing files: {e}"))?; ( stats, @@ -254,20 +240,13 @@ fn main() { let reader: Box = if stdin { diff::get_stdin_diff() } else { - match diff::get_git_diff(&base, &head) { - Ok(r) => r, - Err(e) => { - eprintln!("Error running git diff: {}", e); - exit(1); - } - } + diff::get_git_diff(&base, &head) + .map_err(|e| format!("Error running git diff: {e}"))? }; let mut file_stats = Vec::new(); - if let Err(e) = parser::parse_diff(reader, &mut file_stats) { - eprintln!("Error parsing diff: {}", e); - exit(1); - } + parser::parse_diff(reader, &mut file_stats) + .map_err(|e| format!("Error parsing diff: {e}"))?; ( file_stats, @@ -284,7 +263,6 @@ fn main() { ) } None => { - // Root command fallback let base = cli.base.unwrap_or(config.base); let head = cli.head.unwrap_or("HEAD".to_string()); let format = resolve_format(cli.format, &config.format); @@ -292,20 +270,13 @@ fn main() { let reader: Box = if cli.stdin { diff::get_stdin_diff() } else { - match diff::get_git_diff(&base, &head) { - Ok(r) => r, - Err(e) => { - eprintln!("Error running git diff: {}", e); - exit(1); - } - } + diff::get_git_diff(&base, &head) + .map_err(|e| format!("Error running git diff: {e}"))? }; let mut file_stats = Vec::new(); - if let Err(e) = parser::parse_diff(reader, &mut file_stats) { - eprintln!("Error parsing diff: {}", e); - exit(1); - } + parser::parse_diff(reader, &mut file_stats) + .map_err(|e| format!("Error parsing diff: {e}"))?; ( file_stats, @@ -332,7 +303,6 @@ fn main() { ); if let Err(e) = check_thresholds(&stats, &active_config) { - // Print fail summary for CI if active_config.ci { println!( "PURECODE_FAIL reason={} {}", @@ -341,40 +311,30 @@ fn main() { ); } - eprintln!("{}", e); + eprintln!("{e}"); if !active_config.warn_only { - exit(2); + return Ok(ExitCode::from(2)); } - } else { - // Success summary is printed in print_report if CI mode } + + Ok(ExitCode::SUCCESS) } -struct FilesConfig { - format: Format, - per_file: bool, - max_noise_ratio: Option, - min_pure_lines: Option, - fail_on_decrease: bool, - warn_only: bool, - ci: bool, +fn main() -> ExitCode { + match run() { + Ok(code) => code, + Err(e) => { + eprintln!("{e}"); + ExitCode::from(1) + } + } } fn check_thresholds(file_stats: &[FileStats], args: &FilesConfig) -> Result<(), ThresholdError> { - let mut overall = LangStats::default(); - for s in file_stats { - overall.total_added += s.lang_stats.total_added; - overall.total_removed += s.lang_stats.total_removed; - overall.pure_added += s.lang_stats.pure_added; - overall.pure_removed += s.lang_stats.pure_removed; - overall.comment_lines_added += s.lang_stats.comment_lines_added; - overall.docstring_lines_added += s.lang_stats.docstring_lines_added; - overall.blank_lines_added += s.lang_stats.blank_lines_added; - } + let overall = stats::aggregate_stats(file_stats); if let Some(max_ratio) = args.max_noise_ratio { let total_changes = overall.total_added + overall.total_removed; - if total_changes > 0 { let pure_changes = overall.pure_added + overall.pure_removed; let pure_ratio = pure_changes as f64 / total_changes as f64; @@ -418,11 +378,13 @@ fn error_reason(e: &ThresholdError) -> &'static str { fn error_details(e: &ThresholdError) -> String { match e { ThresholdError::NoiseRatioExceeded { actual, max } => { - format!("noise_ratio={:.2} max_noise_ratio={:.2}", actual, max) + format!("noise_ratio={actual:.2} max_noise_ratio={max:.2}") } ThresholdError::MinPureLines { actual, min } => { - format!("net_pure_lines={} min_pure_lines={}", actual, min) + format!("net_pure_lines={actual} min_pure_lines={min}") + } + ThresholdError::PureLinesDecreased { actual } => { + format!("net_pure_lines={actual}") } - ThresholdError::PureLinesDecreased { actual } => format!("net_pure_lines={}", actual), } } diff --git a/src/report.rs b/src/report.rs index 82ff40d..bc273a2 100644 --- a/src/report.rs +++ b/src/report.rs @@ -1,5 +1,7 @@ -use crate::stats::{calculate_complexity, estimate_tokens, AnalysisResult, FileStats, LangStats}; -use colored::*; +use crate::stats::{ + aggregate_stats, calculate_complexity, estimate_tokens, AnalysisResult, FileStats, LangStats, +}; +use colored::Colorize; use std::collections::HashMap; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -16,39 +18,8 @@ pub fn print_report( mode: &str, ci: bool, ) { - let mut overall = LangStats::default(); - let mut lang_map: HashMap = HashMap::new(); - - for file in stats { - // Aggregate overall - overall.total_added += file.lang_stats.total_added; - overall.total_removed += file.lang_stats.total_removed; - overall.pure_added += file.lang_stats.pure_added; - overall.pure_removed += file.lang_stats.pure_removed; - overall.comment_lines_added += file.lang_stats.comment_lines_added; - overall.comment_lines_removed += file.lang_stats.comment_lines_removed; - overall.docstring_lines_added += file.lang_stats.docstring_lines_added; - overall.docstring_lines_removed += file.lang_stats.docstring_lines_removed; - overall.blank_lines_added += file.lang_stats.blank_lines_added; - overall.blank_lines_removed += file.lang_stats.blank_lines_removed; - overall.code_words_added += file.lang_stats.code_words_added; - overall.code_words_removed += file.lang_stats.code_words_removed; - - // Aggregate per language - let entry = lang_map.entry(file.language.clone()).or_default(); - entry.total_added += file.lang_stats.total_added; - entry.total_removed += file.lang_stats.total_removed; - entry.pure_added += file.lang_stats.pure_added; - entry.pure_removed += file.lang_stats.pure_removed; - entry.comment_lines_added += file.lang_stats.comment_lines_added; - entry.comment_lines_removed += file.lang_stats.comment_lines_removed; - entry.docstring_lines_added += file.lang_stats.docstring_lines_added; - entry.docstring_lines_removed += file.lang_stats.docstring_lines_removed; - entry.blank_lines_added += file.lang_stats.blank_lines_added; - entry.blank_lines_removed += file.lang_stats.blank_lines_removed; - entry.code_words_added += file.lang_stats.code_words_added; - entry.code_words_removed += file.lang_stats.code_words_removed; - } + let overall = aggregate_stats(stats); + let lang_map = aggregate_by_language(stats); let complexity = calculate_complexity(&overall); let token_estimate = estimate_tokens(overall.code_words_added); @@ -63,9 +34,9 @@ pub fn print_report( token_estimate, mode: mode.to_string(), }; - if let Ok(json) = serde_json::to_string_pretty(&result) { - println!("{}", json); - } + let json = serde_json::to_string_pretty(&result) + .expect("Failed to serialize analysis result to JSON"); + println!("{json}"); } OutputFormat::Human | OutputFormat::Plain => { let use_color = !ci && format == OutputFormat::Human; @@ -92,7 +63,7 @@ pub fn print_report( } } - if ci { + if ci && format != OutputFormat::Json { // Print summary line let total_changes = overall.total_added + overall.total_removed; let pure_changes = overall.pure_added + overall.pure_removed; @@ -102,12 +73,11 @@ pub fn print_report( 0.0 }; - println!("PURECODE_SUMMARY noise_ratio={:.2} pure_added={} pure_removed={} files_changed={} complexity={:.2}", - noise_ratio, + println!( + "PURECODE_SUMMARY noise_ratio={noise_ratio:.2} pure_added={} pure_removed={} files_changed={} complexity={complexity:.2}", overall.pure_added, overall.pure_removed, stats.len(), - complexity ); } } @@ -128,7 +98,7 @@ fn print_human_report( complexity, complexity_bucket(complexity) ); - println!("Estimated Tokens (Added): {}", tokens); + println!("Estimated Tokens (Added): {tokens}"); println!("\n{}", "Language Breakdown:".bold()); let mut sorted_langs: Vec<_> = lang_map.iter().collect(); @@ -175,7 +145,7 @@ fn print_plain_report( complexity, complexity_bucket(complexity) ); - println!("Estimated Tokens (Added): {}", tokens); + println!("Estimated Tokens (Added): {tokens}"); println!("\nLanguage Breakdown:"); let mut sorted_langs: Vec<_> = lang_map.iter().collect(); @@ -215,3 +185,23 @@ fn complexity_bucket(score: f64) -> &'static str { "heavy" } } + +fn aggregate_by_language(stats: &[FileStats]) -> HashMap { + let mut lang_map: HashMap = HashMap::new(); + for file in stats { + let entry = lang_map.entry(file.language.clone()).or_default(); + entry.total_added += file.lang_stats.total_added; + entry.total_removed += file.lang_stats.total_removed; + entry.pure_added += file.lang_stats.pure_added; + entry.pure_removed += file.lang_stats.pure_removed; + entry.comment_lines_added += file.lang_stats.comment_lines_added; + entry.comment_lines_removed += file.lang_stats.comment_lines_removed; + entry.docstring_lines_added += file.lang_stats.docstring_lines_added; + entry.docstring_lines_removed += file.lang_stats.docstring_lines_removed; + entry.blank_lines_added += file.lang_stats.blank_lines_added; + entry.blank_lines_removed += file.lang_stats.blank_lines_removed; + entry.code_words_added += file.lang_stats.code_words_added; + entry.code_words_removed += file.lang_stats.code_words_removed; + } + lang_map +} diff --git a/src/stats.rs b/src/stats.rs index 8a15439..de44fc6 100644 --- a/src/stats.rs +++ b/src/stats.rs @@ -36,19 +36,42 @@ pub struct LangStats { } impl LangStats { + #[must_use] pub fn net_pure(&self) -> i64 { self.pure_added - self.pure_removed } + #[must_use] pub fn noise_added(&self) -> i64 { self.comment_lines_added + self.docstring_lines_added + self.blank_lines_added } + #[must_use] pub fn noise_removed(&self) -> i64 { self.comment_lines_removed + self.docstring_lines_removed + self.blank_lines_removed } } +#[must_use] +pub fn aggregate_stats(stats: &[FileStats]) -> LangStats { + stats.iter().fold(LangStats::default(), |mut acc, file| { + acc.total_added += file.lang_stats.total_added; + acc.total_removed += file.lang_stats.total_removed; + acc.pure_added += file.lang_stats.pure_added; + acc.pure_removed += file.lang_stats.pure_removed; + acc.comment_lines_added += file.lang_stats.comment_lines_added; + acc.comment_lines_removed += file.lang_stats.comment_lines_removed; + acc.docstring_lines_added += file.lang_stats.docstring_lines_added; + acc.docstring_lines_removed += file.lang_stats.docstring_lines_removed; + acc.blank_lines_added += file.lang_stats.blank_lines_added; + acc.blank_lines_removed += file.lang_stats.blank_lines_removed; + acc.code_words_added += file.lang_stats.code_words_added; + acc.code_words_removed += file.lang_stats.code_words_removed; + acc + }) +} + +#[must_use] pub fn calculate_complexity(stats: &LangStats) -> f64 { // complexity = pure_added * 1.0 + pure_removed * 0.5 + noise_added * 0.1 + noise_removed * 0.05 (stats.pure_added as f64 * 1.0) @@ -57,6 +80,7 @@ pub fn calculate_complexity(stats: &LangStats) -> f64 { + (stats.noise_removed() as f64 * 0.05) } +#[must_use] pub fn estimate_tokens(word_count: i64) -> u64 { (word_count as f64 * 1.3).round() as u64 }