From e43aa6412d9dfe002f5ff7ef5c2838b43c840c47 Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 11 Dec 2024 10:52:48 -0800 Subject: [PATCH] logger: add log_debugf, rework logger to accept `Display` (#111) Adds a new log_debug macro for logging debug messages. This will be stripped out of production builds. Also reworks the `logger` interface to accept `Display` rather than `Into`. --- Cargo.lock | 7 --- ...-030587f5-d1cd-41ae-b155-14710a5fe812.json | 7 +++ .../src/swc_resolver/node_resolver.rs | 3 +- crates/logger/src/lib.rs | 38 ++++++++---- crates/logger_console/src/lib.rs | 6 +- crates/logger_srcfile/src/lib.rs | 36 +++++------- crates/unused_finder/Cargo.toml | 1 - crates/unused_finder/src/graph.rs | 58 ++++++++++++++++++- crates/unused_finder/src/report.rs | 8 --- crates/unused_finder/src/unused_finder.rs | 15 ++++- package.json | 2 +- 11 files changed, 121 insertions(+), 60 deletions(-) create mode 100644 change/@good-fences-api-030587f5-d1cd-41ae-b155-14710a5fe812.json diff --git a/Cargo.lock b/Cargo.lock index 4b05bc2d..3f4e290a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -530,12 +530,6 @@ dependencies = [ "hashbrown", ] -[[package]] -name = "debug_print" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f215f9b7224f49fb73256115331f677d868b34d18b65dbe4db392e6021eea90" - [[package]] name = "debugid" version = "0.8.0" @@ -2769,7 +2763,6 @@ dependencies = [ "anyhow", "bitflags", "const_format", - "debug_print", "glob", "ignore", "import_resolver", diff --git a/change/@good-fences-api-030587f5-d1cd-41ae-b155-14710a5fe812.json b/change/@good-fences-api-030587f5-d1cd-41ae-b155-14710a5fe812.json new file mode 100644 index 00000000..ac99b7a2 --- /dev/null +++ b/change/@good-fences-api-030587f5-d1cd-41ae-b155-14710a5fe812.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "logger: add debug_logf macro for debug logger", + "packageName": "@good-fences/api", + "email": "mhuan13@gmail.com", + "dependentChangeType": "patch" +} diff --git a/crates/import_resolver/src/swc_resolver/node_resolver.rs b/crates/import_resolver/src/swc_resolver/node_resolver.rs index 971ba892..066a3fe8 100644 --- a/crates/import_resolver/src/swc_resolver/node_resolver.rs +++ b/crates/import_resolver/src/swc_resolver/node_resolver.rs @@ -612,7 +612,8 @@ impl<'caches> CachingNodeModulesResolver<'caches> { .or_else(|_| self.resolve_as_directory(&path, true)) .and_then(|p| self.wrap(p).with_context(|| { format!( - "failed to resolve non-absolute target path {}", + "failed to resolve non-absolute target path {} (resolved as {})", + target, path.display(), ) })) diff --git a/crates/logger/src/lib.rs b/crates/logger/src/lib.rs index c78fc924..1fdc54a4 100644 --- a/crates/logger/src/lib.rs +++ b/crates/logger/src/lib.rs @@ -1,19 +1,35 @@ -use std::sync::Mutex; +use std::{fmt::Display, sync::Mutex}; use anyhow::anyhow; pub trait Logger: Clone { - fn log(&self, message: impl Into); - fn warn(&self, message: impl Into) { - self.log(format!("WARN: {}", message.into())); + fn log(&self, message: impl Display); + fn warn(&self, message: impl Display) { + self.log(format!("WARN: {}", message)); } - fn error(&self, message: impl Into) { - self.log(format!("ERROR: {}", message.into())); + fn error(&self, message: impl Display) { + self.log(format!("ERROR: {}", message)); } } +#[macro_export] +macro_rules! cfg { + ($($cfg:tt)*) => { + /* compiler built-in */ + }; +} + +#[macro_export] +macro_rules! debug_logf { + ($logger:expr, $fmt:expr $(, $arg:expr)*) => { + if cfg!(debug_assertions) { + $logger.log(format!($fmt $(, $arg)*)); + } + }; +} + impl Logger for &T { - fn log(&self, message: impl Into) { + fn log(&self, message: impl Display) { (*self).log(message); } } @@ -22,9 +38,9 @@ pub struct StdioLogger { zero_time: std::time::Instant, } impl Logger for &StdioLogger { - fn log(&self, message: impl Into) { + fn log(&self, message: impl Display) { let delta_time = std::time::Instant::now().duration_since(self.zero_time); - println!("[{:.04}] {}", delta_time.as_secs_f64(), message.into()); + println!("[{:.04}] {}", delta_time.as_secs_f64(), message); } } impl StdioLogger { @@ -45,11 +61,11 @@ pub struct VecLogger { } impl Logger for &VecLogger { - fn log(&self, message: impl Into) { + fn log(&self, message: impl Display) { self.logs .lock() .expect("locking the logger array should not fail!") - .push(message.into()); + .push(format!("{}", message)); } } impl VecLogger { diff --git a/crates/logger_console/src/lib.rs b/crates/logger_console/src/lib.rs index 9cf2b643..9d8d3b11 100644 --- a/crates/logger_console/src/lib.rs +++ b/crates/logger_console/src/lib.rs @@ -1,4 +1,4 @@ -use std::sync::Arc; +use std::{fmt::Display, sync::Arc}; use napi::{ threadsafe_function::{ @@ -33,8 +33,8 @@ impl ConsoleLogger { } impl Logger for ConsoleLogger { - fn log(&self, message: impl Into) { - let message_string: String = message.into(); + fn log(&self, message: impl Display) { + let message_string: String = format!("{}", message); let status = self .logfn .call(Ok(message_string), ThreadsafeFunctionCallMode::Blocking); diff --git a/crates/logger_srcfile/src/lib.rs b/crates/logger_srcfile/src/lib.rs index 8bff0bf1..f7d1d2aa 100644 --- a/crates/logger_srcfile/src/lib.rs +++ b/crates/logger_srcfile/src/lib.rs @@ -1,23 +1,19 @@ +use std::fmt::Display; + use logger::Logger; use swc_common::{Loc, SourceMap, Span}; pub trait SrcLogger: Logger { - fn src_warn(&self, loc: &Loc, message: impl Into) { + fn src_warn(&self, loc: &Loc, message: impl Display) { self.warn(format!( "{}:{}:{} :: {}", - loc.file.name, - loc.line, - loc.col_display, - message.into() + loc.file.name, loc.line, loc.col_display, message, )); } - fn src_error(&self, loc: &Loc, message: impl Into) { + fn src_error(&self, loc: &Loc, message: impl Display) { self.error(format!( "{}:{}:{} :: {}", - loc.file.name, - loc.line, - loc.col_display, - message.into() + loc.file.name, loc.line, loc.col_display, message, )); } } @@ -27,24 +23,18 @@ pub trait HasSourceMap { } pub trait SrcFileLogger: Logger + HasSourceMap { - fn src_warn(&self, location: &Span, message: impl Into) { + fn src_warn(&self, location: &Span, message: impl Display) { let loc = self.source_map().lookup_char_pos(location.lo); self.warn(format!( "{}:{}:{} :: {}", - loc.file.name, - loc.line, - loc.col_display, - message.into() + loc.file.name, loc.line, loc.col_display, message, )); } - fn src_error(&self, location: &Span, message: impl Into) { + fn src_error(&self, location: &Span, message: impl Display) { let loc = self.source_map().lookup_char_pos(location.lo); self.error(format!( "{}:{}:{} :: {}", - loc.file.name, - loc.line, - loc.col_display, - message.into() + loc.file.name, loc.line, loc.col_display, message, )); } } @@ -63,13 +53,13 @@ impl<'a, TLogger: Logger> WrapFileLogger<'a, TLogger> { } } impl Logger for WrapFileLogger<'_, TLogger> { - fn log(&self, message: impl Into) { + fn log(&self, message: impl Display) { self.inner_logger.log(message); } - fn error(&self, message: impl Into) { + fn error(&self, message: impl Display) { self.inner_logger.error(message); } - fn warn(&self, message: impl Into) { + fn warn(&self, message: impl Display) { self.inner_logger.warn(message); } } diff --git a/crates/unused_finder/Cargo.toml b/crates/unused_finder/Cargo.toml index 478056b4..fc029cbc 100644 --- a/crates/unused_finder/Cargo.toml +++ b/crates/unused_finder/Cargo.toml @@ -36,7 +36,6 @@ bitflags = "2.6.0" ignore = "0.4.23" abspath = { version = "0.2.0", path = "../abspath" } itertools = "0.13.0" -debug_print = "1.0.0" schemars.workspace = true logger = { version = "0.2.0", path = "../logger" } diff --git a/crates/unused_finder/src/graph.rs b/crates/unused_finder/src/graph.rs index 3d05f43a..dd2413ee 100644 --- a/crates/unused_finder/src/graph.rs +++ b/crates/unused_finder/src/graph.rs @@ -1,5 +1,9 @@ use core::option::Option::None; -use std::{collections::HashSet, path::Path, path::PathBuf}; +use std::{ + collections::HashSet, + fmt::Display, + path::{Path, PathBuf}, +}; use ahashmap::{AHashMap, AHashSet}; use anyhow::Result; @@ -10,7 +14,7 @@ use crate::{ tag::UsedTag, walked_file::ResolvedSourceFile, }; -use logger::Logger; +use logger::{debug_logf, Logger}; // graph node used to represent a file during the "used file" walk #[derive(Debug, Clone, Default)] @@ -89,6 +93,12 @@ pub struct Edge { pub symbol: ExportedSymbol, } +impl Display for Edge { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}:{:?}", self.file_id, self.symbol) + } +} + impl Edge { pub fn new(file_id: usize, symbol: ExportedSymbol) -> Self { Self { file_id, symbol } @@ -137,6 +147,38 @@ impl Graph { initial_frontier_symbols: Vec<(&Path, Vec)>, tag: UsedTag, ) -> Result<()> { + debug_logf!( + logger, + "initial_frontier_files ({}:{}):\n {}", + tag, + initial_frontier_files.len(), + initial_frontier_files + .iter() + .map(|path| path.display().to_string()) + .collect::>() + .join("\n ") + ); + + debug_logf!( + logger, + "initial_frontier_symbols ({}:{}):\n {}", + tag, + initial_frontier_symbols.len(), + initial_frontier_symbols + .iter() + .map(|(path, symbols)| (format!( + "{}:{}", + path.display(), + symbols + .iter() + .map(|x| x.to_string()) + .collect::>() + .join(", ") + ))) + .collect::>() + .join("\n ") + ); + let initial_file_edges = initial_frontier_files .into_iter() .filter_map(|path| match self.path_to_id.get(path) { @@ -184,6 +226,18 @@ impl Graph { .chain(initial_symbol_edges) .collect::>(); + debug_logf!( + logger, + "initial frontier ({}:{}): {}", + tag, + frontier.len(), + frontier + .iter() + .map(|p| format!("{}:{}", self.files[p.file_id].file_path.display(), p.symbol)) + .collect::>() + .join("\n ") + ); + // Traverse the graph until we exhaust the frontier const MAX_ITERATIONS: usize = 1_000_000; for _ in 0..MAX_ITERATIONS { diff --git a/crates/unused_finder/src/report.rs b/crates/unused_finder/src/report.rs index 670a016f..c06ebe11 100644 --- a/crates/unused_finder/src/report.rs +++ b/crates/unused_finder/src/report.rs @@ -5,7 +5,6 @@ use core::{ use std::fmt::Display; use ahashmap::AHashMap; -use debug_print::debug_println; use rayon::prelude::*; use serde::{Deserialize, Serialize}; use swc_common::source_map::SmallPos; @@ -194,13 +193,6 @@ impl From<&UnusedFinderResult> for UnusedFinderReport { let default: UsedTag = Default::default(); let symbol_bitflags: &UsedTag = file.symbol_tags.get(symbol_name).unwrap_or(&default); - debug_println!( - "visit symbol {}:{} ({})", - file.file_path.display(), - symbol_name, - symbol_bitflags - ); - if !include_extra(symbol_bitflags) { // don't return symbols that are used or symbols that are truly unused return None; diff --git a/crates/unused_finder/src/unused_finder.rs b/crates/unused_finder/src/unused_finder.rs index b4af9182..9bf8e785 100644 --- a/crates/unused_finder/src/unused_finder.rs +++ b/crates/unused_finder/src/unused_finder.rs @@ -20,7 +20,7 @@ use import_resolver::swc_resolver::{ MonorepoResolver, }; use js_err::JsErr; -use logger::Logger; +use logger::{debug_logf, Logger}; use rayon::{iter::Either, prelude::*}; use swc_ecma_loader::{resolve::Resolve, TargetEnv}; @@ -104,7 +104,7 @@ impl SourceFiles { "Multiple errors occurred during resolution:\n{}", errors .into_iter() - .map(|x| format!("{:#?}", x)) + .map(|x| format!("{:#}", x)) .collect::>() .join("\n") )); @@ -329,6 +329,10 @@ impl UnusedFinder { // Create a new graph with all entries marked as "unused". let mut graph = Graph::from_source_files(self.last_walk_result.source_files.values()); + + // print the entry packages config + debug_logf!(logger, "Entry packages: {:#?}", self.config.entry_packages); + // Get the walk roots and perform the graph traversal let entrypoints = self.get_entrypoints(&logger); logger.log(format!( @@ -449,11 +453,16 @@ impl UnusedFinder { } }; + let relative_package_path = owning_package + .package_path + .strip_prefix(&self.config.repo_root) + .expect("absolue paths of packages within the repo should be relative"); + // only "entry packages" may export scripts if !self .config .entry_packages - .matches(&owning_package.package_path, owning_package_name) + .matches(relative_package_path, owning_package_name) { return false; } diff --git a/package.json b/package.json index 87182e8a..6bf7f309 100644 --- a/package.json +++ b/package.json @@ -53,4 +53,4 @@ "." ], "packageManager": "yarn@4.4.0+sha512.91d93b445d9284e7ed52931369bc89a663414e5582d00eea45c67ddc459a2582919eece27c412d6ffd1bd0793ff35399381cb229326b961798ce4f4cc60ddfdb" -} +} \ No newline at end of file