diff --git a/Cargo.lock b/Cargo.lock index bbcf761..7edf5cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -277,6 +277,7 @@ dependencies = [ "anyhow", "chrono", "clap", + "colored", "fast-glob", "futures", "git2", diff --git a/README.md b/README.md index 024b4ac..91840d4 100644 --- a/README.md +++ b/README.md @@ -210,6 +210,7 @@ license agreements: Dual-licensed under [Apache 2.0][Apache2] or [MIT]. - [chrono](https://crates.io/crates/chrono): Dual-licensed under [Apache 2.0][Apache2] or [MIT]. +- [colored](https://crates.io/crates/colored): Licensed under [MPL-2.0] The python binding uses @@ -223,3 +224,4 @@ The node binding uses [MIT]: https://choosealicense.com/licenses/mit [Apache2]: https://choosealicense.com/licenses/apache-2.0/ +[MPL-2.0]: https://choosealicense.com/licenses/mpl-2.0 diff --git a/cpp-linter/Cargo.toml b/cpp-linter/Cargo.toml index 947eff8..176bddd 100644 --- a/cpp-linter/Cargo.toml +++ b/cpp-linter/Cargo.toml @@ -17,11 +17,12 @@ license.workspace = true anyhow = "1.0.89" chrono = "0.4.38" clap = "4.5.17" +colored = "2.1.0" fast-glob = "0.4.0" futures = "0.3.30" git2 = "0.19.0" lenient_semver = "0.4.2" -log = "0.4.22" +log = { version = "0.4.22", features = ["std"] } openssl = { version = "0.10", features = ["vendored"], optional = true } openssl-probe = { version = "0.1", optional = true } regex = "1.10.6" diff --git a/cpp-linter/src/clang_tools/mod.rs b/cpp-linter/src/clang_tools/mod.rs index 68063ae..1b1bdd6 100644 --- a/cpp-linter/src/clang_tools/mod.rs +++ b/cpp-linter/src/clang_tools/mod.rs @@ -21,8 +21,7 @@ use which::{which, which_in}; use super::common_fs::FileObj; use crate::{ cli::ClangParams, - logger::{end_log_group, start_log_group}, - rest_api::{COMMENT_MARKER, USER_OUTREACH}, + rest_api::{RestApiClient, COMMENT_MARKER, USER_OUTREACH}, }; pub mod clang_format; use clang_format::run_clang_format; @@ -144,6 +143,7 @@ pub async fn capture_clang_tools_output( files: &mut Vec>>, version: &str, clang_params: &mut ClangParams, + rest_api_client: &impl RestApiClient, ) -> Result<()> { // find the executable paths for clang-tidy and/or clang-format and show version // info as debugging output. @@ -192,11 +192,11 @@ pub async fn capture_clang_tools_output( while let Some(output) = executors.join_next().await { if let Ok(out) = output? { let (file_name, logs) = out; - start_log_group(format!("Analyzing {}", file_name.to_string_lossy())); + rest_api_client.start_log_group(format!("Analyzing {}", file_name.to_string_lossy())); for (level, msg) in logs { log::log!(level, "{}", msg); } - end_log_group(); + rest_api_client.end_log_group(); } } Ok(()) diff --git a/cpp-linter/src/logger.rs b/cpp-linter/src/logger.rs index 8426c27..f7fc004 100644 --- a/cpp-linter/src/logger.rs +++ b/cpp-linter/src/logger.rs @@ -1,58 +1,77 @@ //! A module to initialize and customize the logger object used in (most) stdout. -// non-std crates -use log::{Level, LevelFilter, Metadata, Record, SetLoggerError}; +use std::env; +use anyhow::{Error, Result}; +use colored::{control::set_override, Colorize}; +use log::{Level, LevelFilter, Metadata, Record}; + +#[derive(Default)] struct SimpleLogger; +impl SimpleLogger { + fn level_color(level: &Level) -> String { + let name = format!("{:>5}", level.as_str().to_uppercase()); + match level { + Level::Error => name.red().bold().to_string(), + Level::Warn => name.yellow().bold().to_string(), + Level::Info => name.green().bold().to_string(), + Level::Debug => name.blue().bold().to_string(), + Level::Trace => name.magenta().bold().to_string(), + } + } +} + impl log::Log for SimpleLogger { fn enabled(&self, metadata: &Metadata) -> bool { - metadata.level() <= Level::Debug + metadata.level() <= log::max_level() } fn log(&self, record: &Record) { - if self.enabled(record.metadata()) { - println!("{}: {}", record.level(), record.args()); + if record.target() == "CI_LOG_GROUPING" { + // this log is meant to manipulate a CI workflow's log grouping + println!("{}", record.args()); + } else if self.enabled(record.metadata()) { + println!( + "[{}]: {}", + Self::level_color(&record.level()), + record.args() + ); } } fn flush(&self) {} } -/// A private constant to manage the application's logger object. -static LOGGER: SimpleLogger = SimpleLogger; - /// A function to initialize the private `LOGGER`. /// /// The logging level defaults to [`LevelFilter::Info`]. /// Returns a [`SetLoggerError`] if the `LOGGER` is already initialized. -pub fn init() -> Result<(), SetLoggerError> { - log::set_logger(&LOGGER).map(|()| log::set_max_level(LevelFilter::Info)) -} - -/// This prints a line to indicate the beginning of a related group of log statements. -/// -/// This function may or may not get moved to [crate::rest_api::RestApiClient] trait -/// if/when platforms other than GitHub are supported. -pub fn start_log_group(name: String) { - println!("::group::{}", name); -} - -/// This prints a line to indicate the ending of a related group of log statements. -/// -/// This function may or may not get moved to [crate::rest_api::RestApiClient] trait -/// if/when platforms other than GitHub are supported. -pub fn end_log_group() { - println!("::endgroup::"); +pub fn init() -> Result<()> { + let logger: SimpleLogger = SimpleLogger; + if matches!( + env::var("CPP_LINTER_COLOR").as_deref(), + Ok("on" | "1" | "true") + ) { + set_override(true); + } + log::set_boxed_logger(Box::new(logger)) + .map(|()| log::set_max_level(LevelFilter::Info)) + .map_err(Error::from) } #[cfg(test)] -mod tests { - use super::{end_log_group, start_log_group}; +mod test { + use std::env; + + use super::{init, SimpleLogger}; #[test] - fn issue_log_grouping_stdout() { - start_log_group(String::from("a dumb test")); - end_log_group(); + fn trace_log() { + env::set_var("CPP_LINTER_COLOR", "true"); + init().unwrap_or(()); + assert!(SimpleLogger::level_color(&log::Level::Trace).contains("TRACE")); + log::set_max_level(log::LevelFilter::Trace); + log::trace!("A dummy log statement for code coverage"); } } diff --git a/cpp-linter/src/rest_api/github/mod.rs b/cpp-linter/src/rest_api/github/mod.rs index 0fa66c3..1d7c067 100644 --- a/cpp-linter/src/rest_api/github/mod.rs +++ b/cpp-linter/src/rest_api/github/mod.rs @@ -95,6 +95,16 @@ impl RestApiClient for GithubApiClient { checks_failed } + /// This prints a line to indicate the beginning of a related group of log statements. + fn start_log_group(&self, name: String) { + log::info!(target: "CI_LOG_GROUPING", "::group::{}", name); + } + + /// This prints a line to indicate the ending of a related group of log statements. + fn end_log_group(&self) { + log::info!(target: "CI_LOG_GROUPING", "::endgroup::"); + } + fn make_headers() -> Result> { let mut headers = HeaderMap::new(); headers.insert( diff --git a/cpp-linter/src/rest_api/mod.rs b/cpp-linter/src/rest_api/mod.rs index 4c9da48..589b52c 100644 --- a/cpp-linter/src/rest_api/mod.rs +++ b/cpp-linter/src/rest_api/mod.rs @@ -49,6 +49,12 @@ pub trait RestApiClient { tidy_checks_failed: Option, ) -> u64; + /// This prints a line to indicate the beginning of a related group of log statements. + fn start_log_group(&self, name: String); + + /// This prints a line to indicate the ending of a related group of log statements. + fn end_log_group(&self); + /// A convenience method to create the headers attached to all REST API calls. /// /// If an authentication token is provided (via environment variable), @@ -453,12 +459,21 @@ mod test { ) -> Result { Err(anyhow!("Not implemented")) } + + fn start_log_group(&self, name: String) { + log::info!(target: "CI_LOG_GROUPING", "start_log_group: {name}"); + } + + fn end_log_group(&self) { + log::info!(target: "CI_LOG_GROUPING", "end_log_group"); + } } #[tokio::test] async fn dummy_coverage() { assert!(TestClient::make_headers().is_err()); let dummy = TestClient::default(); + dummy.start_log_group("Dummy test".to_string()); assert_eq!(dummy.set_exit_code(1, None, None), 0); assert!(dummy .get_list_of_changed_files(&FileFilter::new(&[], vec![])) @@ -474,7 +489,8 @@ mod test { assert!(dummy .post_feedback(&[], FeedbackInput::default()) .await - .is_err()) + .is_err()); + dummy.end_log_group(); } // ************************************************* try_next_page() tests diff --git a/cpp-linter/src/run.rs b/cpp-linter/src/run.rs index 42cc050..a59b2af 100644 --- a/cpp-linter/src/run.rs +++ b/cpp-linter/src/run.rs @@ -17,7 +17,7 @@ use openssl_probe; use crate::clang_tools::capture_clang_tools_output; use crate::cli::{get_arg_parser, ClangParams, Cli, FeedbackInput, LinesChangedOnly}; use crate::common_fs::FileFilter; -use crate::logger::{self, end_log_group, start_log_group}; +use crate::logger; use crate::rest_api::{github::GithubApiClient, RestApiClient}; const VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -93,7 +93,7 @@ pub async fn run_main(args: Vec) -> Result<()> { } } - start_log_group(String::from("Get list of specified source files")); + rest_api_client.start_log_group(String::from("Get list of specified source files")); let files = if cli.lines_changed_only != LinesChangedOnly::Off || cli.files_changed_only { // parse_diff(github_rest_api_payload) rest_api_client @@ -124,16 +124,22 @@ pub async fn run_main(args: Vec) -> Result<()> { log::info!(" ./{}", file.name.to_string_lossy().replace('\\', "/")); arc_files.push(Arc::new(Mutex::new(file))); } - end_log_group(); + rest_api_client.end_log_group(); let mut clang_params = ClangParams::from(&cli); let user_inputs = FeedbackInput::from(&cli); - capture_clang_tools_output(&mut arc_files, cli.version.as_str(), &mut clang_params).await?; - start_log_group(String::from("Posting feedback")); + capture_clang_tools_output( + &mut arc_files, + cli.version.as_str(), + &mut clang_params, + &rest_api_client, + ) + .await?; + rest_api_client.start_log_group(String::from("Posting feedback")); let checks_failed = rest_api_client .post_feedback(&arc_files, user_inputs) .await?; - end_log_group(); + rest_api_client.end_log_group(); if env::var("PRE_COMMIT").is_ok_and(|v| v == "1") { if checks_failed > 1 { return Err(anyhow!("Some checks did not pass"));