WIP: use log crate

Author: ekump

.github/actions/clippy-annotation-reporter/src/analyzer.rs

@@ -7,6 +7,7 @@
 //! including getting changed files, comparing branches, and producing analysis results.
 
 use anyhow::{Context as _, Result};
+use log::{debug, error, info};
 use octocrab::params::pulls::MediaType::Raw;
 use octocrab::Octocrab;
 use regex::Regex;
@@ -70,7 +71,7 @@ fn analyze_annotations(
     head_branch: &str,
     rules: &[String],
 ) -> Result<AnalysisResult> {
-    println!("Analyzing clippy annotations in {} files...", files.len());
+    debug!("Analyzing clippy annotations in {} files...", files.len());
 
     // Create a regex for matching clippy allow annotations
     let rule_pattern = rules.join("|");
@@ -80,25 +81,22 @@ fn analyze_annotations(
     ))
     .context("Failed to compile annotation regex")?;
 
-    let mut base_annotations = Vec::with_capacity(files.len() * 3);
-    let mut head_annotations = Vec::with_capacity(files.len() * 3);
-    let mut changed_files = HashSet::with_capacity(files.len());
+    let mut base_annotations = Vec::new();
+    let mut head_annotations = Vec::new();
+    let mut changed_files = HashSet::new();
 
     // Cache for rule Rc instances to avoid duplicates
+    // TODO: EK - why?
     let mut rule_cache = HashMap::new();
 
-    // Process each file
     for file in files {
         changed_files.insert(file.clone());
 
         // Get file content from base branch
         let base_content = match get_file_content(file, base_branch) {
             Ok(content) => content,
             Err(e) => {
-                println!(
-                    "Warning: Failed to get {} content from {}: {}",
-                    file, base_branch, e
-                );
+                error!("Failed to get {} content from {}: {}", file, base_branch, e);
                 String::new()
             }
         };
@@ -107,10 +105,7 @@ fn analyze_annotations(
         let head_content = match get_file_content(file, head_branch) {
             Ok(content) => content,
             Err(e) => {
-                println!(
-                    "Warning: Failed to get {} content from {}: {}",
-                    file, head_branch, e
-                );
+                error!("Failed to get {} content from {}: {}", file, head_branch, e);
                 String::new()
             }
         };
@@ -142,7 +137,7 @@ fn analyze_annotations(
     let base_crate_counts = count_annotations_by_crate(&base_annotations);
     let head_crate_counts = count_annotations_by_crate(&head_annotations);
 
-    println!(
+    info!(
         "Analysis complete. Found {} annotations in base branch and {} in head branch",
         base_annotations.len(),
         head_annotations.len()
@@ -166,7 +161,7 @@ async fn get_changed_files(
     repo: &str,
     pr_number: u64,
 ) -> Result<Vec<String>> {
-    println!("Getting changed files from PR #{}...", pr_number);
+    info!("Getting changed files from PR #{}...", pr_number);
 
     let files = octocrab
         .pulls(owner, repo)
@@ -182,14 +177,14 @@ async fn get_changed_files(
         .map(|file| file.filename)
         .collect();
 
-    println!("Found {} changed Rust files", rust_files.len());
+    info!("Found {} changed Rust files", rust_files.len());
 
     Ok(rust_files)
 }
 
 /// Get file content from a specific branch
 fn get_file_content(file: &str, branch: &str) -> Result<String> {
-    println!("Getting content for {} from {}", file, branch);
+    debug!("Getting content for {} from {}", file, branch);
 
     let output = Command::new("git")
         .args(["show", &format!("{}:{}", branch, file)])
@@ -316,7 +311,7 @@ fn count_annotations_by_crate(annotations: &[ClippyAnnotation]) -> HashMap<Rc<St
 
 /// Get all Rust files in the repository
 fn get_all_rust_files() -> Result<Vec<String>> {
-    println!("Getting all Rust files in the repository...");
+    info!("Getting all Rust files in the repository...");
 
     // Use git ls-files to get all tracked Rust files
     let output = Command::new("git")
@@ -333,7 +328,7 @@ fn get_all_rust_files() -> Result<Vec<String>> {
 
     let rust_files: Vec<String> = files.lines().map(|line| line.to_owned()).collect();
 
-    println!("Found {} Rust files in total", rust_files.len());
+    info!("Found {} Rust files in total", rust_files.len());
 
     Ok(rust_files)
 }
@@ -345,7 +340,7 @@ fn analyze_all_files_for_crates(
     head_branch: &str,
     rules: &[String],
 ) -> Result<(HashMap<Rc<String>, usize>, HashMap<Rc<String>, usize>)> {
-    println!(
+    info!(
         "Analyzing all {} Rust files for crate-level statistics...",
         files.len()
     );
@@ -364,35 +359,8 @@ fn analyze_all_files_for_crates(
     let mut rule_cache = HashMap::new();
     // Process each file
     for file in files {
-        // Get file content from base branch
-        let base_content = match get_file_content(file, base_branch) {
-            Ok(content) => content,
-            Err(e) => {
-                // Skip errors for files that might not exist in one branch
-                if !e.to_string().contains("did not match any file") {
-                    println!(
-                        "Warning: Failed to get {} content from {}: {}",
-                        file, base_branch, e
-                    );
-                }
-                String::new()
-            }
-        };
-
-        // Get file content from head branch
-        let head_content = match get_file_content(file, head_branch) {
-            Ok(content) => content,
-            Err(e) => {
-                // Skip errors for files that might not exist in one branch
-                if !e.to_string().contains("did not match any file") {
-                    println!(
-                        "Warning: Failed to get {} content from {}: {}",
-                        file, head_branch, e
-                    );
-                }
-                String::new()
-            }
-        };
+        let base_content = get_branch_content(file, base_branch);
+        let head_content = get_branch_content(file, head_branch);
 
         // Find annotations in base branch
         find_annotations(
@@ -419,6 +387,20 @@ fn analyze_all_files_for_crates(
     Ok((base_crate_counts, head_crate_counts))
 }
 
+/// Get file content from a branch, handling common errors
+fn get_branch_content(file: &str, branch: &str) -> String {
+    match get_file_content(file, branch) {
+        Ok(content) => content,
+        Err(e) => {
+            // Skip errors for files that might not exist in one branch
+            if !e.to_string().contains("did not match any file") {
+                log::warn!("Failed to get {} content from {}: {}", file, branch, e);
+            }
+            String::new()
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

.github/actions/clippy-annotation-reporter/src/commenter.rs

@@ -7,6 +7,7 @@
 //! including finding existing comments and updating or creating comments.
 
 use anyhow::{Context as _, Result};
+use log::{debug, error, info};
 use octocrab::Octocrab;
 
 /// Post or update a comment on a PR with the given report
@@ -25,27 +26,25 @@ pub async fn post_comment(
     let report_with_signature = format!("{}\n\n{}", report, signature);
 
     // Search for existing comment by the bot
-    println!("Checking for existing comment on PR #{}", pr_number);
+    info!("Checking for existing comment on PR #{}", pr_number);
     let existing_comment =
         find_existing_comment(octocrab, owner, repo, pr_number, signature).await?;
 
     // Update existing comment or create a new one
     if let Some(comment_id) = existing_comment {
-        println!("Updating existing comment #{}", comment_id);
+        info!("Updating existing comment #{}", comment_id);
         octocrab
             .issues(owner, repo)
             .update_comment(comment_id.into(), report_with_signature)
             .await
             .context("Failed to update existing comment")?;
-        println!("Comment updated successfully!");
     } else {
-        println!("Creating new comment on PR #{}", pr_number);
+        info!("Creating new comment on PR #{}", pr_number);
         octocrab
             .issues(owner, repo)
             .create_comment(pr_number, report_with_signature)
             .await
             .context("Failed to post comment to PR")?;
-        println!("Comment created successfully!");
     }
 
     Ok(())
@@ -91,7 +90,7 @@ async fn find_existing_comment(
             }
             Err(e) => {
                 // TODO: EK - Handle error more gracefully
-                println!("Warning: Failed to fetch next page of comments: {}", e);
+                error!("Warning: Failed to fetch next page of comments: {}", e);
                 break;
             }
         }

.github/actions/clippy-annotation-reporter/src/config.rs

@@ -8,6 +8,7 @@
 
 use anyhow::{Context as _, Result};
 use clap::Parser;
+use log::{debug, warn};
 use serde_json::Value;
 use std::env;
 use std::fs;
@@ -81,9 +82,6 @@ impl GitHubContext {
         let event_path = env::var("GITHUB_EVENT_PATH")
             .context("GITHUB_EVENT_PATH environment variable not set")?;
 
-        println!("Event name: {}", event_name);
-        println!("Event path: {}", event_path);
-
         let event_data =
             fs::read_to_string(event_path).context("Failed to read GitHub event file")?;
 
@@ -99,31 +97,22 @@ impl GitHubContext {
 
                 // Direct access to base.ref
                 let base_ref = match event_json["pull_request"]["base"]["ref"].as_str() {
-                    Some(val) => {
-                        println!("Successfully found base.ref in event data: {}", val);
-                        val.to_owned()
-                    }
+                    Some(val) => val.to_owned(),
                     None => {
-                        println!(
-                            "Warning: Could not extract base.ref as string, raw value: {:?}",
-                            event_json["pull_request"]["base"]["ref"]
+                        warn!(
+                            "Could not extract base.ref as string, Falling back to main as base branch",
                         );
 
-                        // Fallback to main if we can't extract the value
-                        println!("Falling back to 'main' as base branch");
                         "main".to_owned()
                     }
                 };
 
                 // Direct access to head.ref
                 let head_ref = match event_json["pull_request"]["head"]["ref"].as_str() {
-                    Some(val) => {
-                        println!("Successfully found head.ref in event data: {}", val);
-                        val.to_owned()
-                    }
+                    Some(val) => val.to_owned(),
                     None => {
-                        println!("Warning: Could not extract head.ref as string");
-                        // We'll use the current branch as fallback
+                        warn!("Warning: Could not extract head.ref as string");
+
                         String::new()
                     }
                 };
@@ -136,9 +125,10 @@ impl GitHubContext {
             }
         };
 
-        println!("Extracted PR number: {}", pr_number);
-        println!("Extracted base branch: {}", base_ref);
-        println!("Extracted head branch: {}", head_ref);
+        debug!(
+            "Extracted PR: {}, base: {}, head: {}",
+            pr_number, base_ref, head_ref
+        );
 
         Ok(GitHubContext {
             repository,
@@ -214,7 +204,6 @@ impl Config {
         let owner = parts[0].to_owned();
         let repo = parts[1].to_owned();
 
-        // Parse rules list
         let rules = args.parse_rules();
 
         Ok(Config {

.github/actions/clippy-annotation-reporter/src/main.rs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use anyhow::{Context as _, Result};
+use log::info;
 use octocrab::Octocrab;
 
 mod analyzer;
@@ -14,14 +15,13 @@ use crate::report_generator::generate_report;
 
 #[tokio::main]
 async fn main() -> Result<()> {
-    // Initialize logger
-    // TODO: EK - Do we even want to use env_logger here?
     env_logger::init();
 
-    println!("Clippy Annotation Reporter starting...");
+    info!("Clippy Annotation Reporter starting...");
 
     let config = ConfigBuilder::new().build()?;
 
+    // TODO: EK - Should we use context here?
     let octocrab = Octocrab::builder()
         .personal_token(config.token.clone())
         .build()
@@ -41,7 +41,7 @@ async fn main() -> Result<()> {
         Ok(result) => result,
         Err(e) => {
             if e.to_string().contains("No Rust files changed") {
-                println!("No Rust files changed in this PR, nothing to analyze.");
+                info!("No Rust files changed in this PR, nothing to analyze.");
                 return Ok(());
             }
             return Err(e);
@@ -66,7 +66,7 @@ async fn main() -> Result<()> {
     )
     .await?;
 
-    println!("Process completed successfully!");
+    info!("Process completed successfully!");
 
     Ok(())
 }