Skip to content

Commit

Permalink
logger: add log_debugf, rework logger to accept Display (#111)
Browse files Browse the repository at this point in the history
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<String>`.
  • Loading branch information
Adjective-Object authored Dec 11, 2024
1 parent 4941ac9 commit e43aa64
Show file tree
Hide file tree
Showing 11 changed files with 121 additions and 60 deletions.
7 changes: 0 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -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"
}
3 changes: 2 additions & 1 deletion crates/import_resolver/src/swc_resolver/node_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)
}))
Expand Down
38 changes: 27 additions & 11 deletions crates/logger/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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<String>);
fn warn(&self, message: impl Into<String>) {
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<String>) {
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<T: Logger> Logger for &T {
fn log(&self, message: impl Into<String>) {
fn log(&self, message: impl Display) {
(*self).log(message);
}
}
Expand All @@ -22,9 +38,9 @@ pub struct StdioLogger {
zero_time: std::time::Instant,
}
impl Logger for &StdioLogger {
fn log(&self, message: impl Into<String>) {
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 {
Expand All @@ -45,11 +61,11 @@ pub struct VecLogger {
}

impl Logger for &VecLogger {
fn log(&self, message: impl Into<String>) {
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 {
Expand Down
6 changes: 3 additions & 3 deletions crates/logger_console/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::sync::Arc;
use std::{fmt::Display, sync::Arc};

use napi::{
threadsafe_function::{
Expand Down Expand Up @@ -33,8 +33,8 @@ impl ConsoleLogger {
}

impl Logger for ConsoleLogger {
fn log(&self, message: impl Into<String>) {
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);
Expand Down
36 changes: 13 additions & 23 deletions crates/logger_srcfile/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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<String>) {
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<String>) {
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,
));
}
}
Expand All @@ -27,24 +23,18 @@ pub trait HasSourceMap {
}

pub trait SrcFileLogger: Logger + HasSourceMap {
fn src_warn(&self, location: &Span, message: impl Into<String>) {
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<String>) {
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,
));
}
}
Expand All @@ -63,13 +53,13 @@ impl<'a, TLogger: Logger> WrapFileLogger<'a, TLogger> {
}
}
impl<TLogger: Logger> Logger for WrapFileLogger<'_, TLogger> {
fn log(&self, message: impl Into<String>) {
fn log(&self, message: impl Display) {
self.inner_logger.log(message);
}
fn error(&self, message: impl Into<String>) {
fn error(&self, message: impl Display) {
self.inner_logger.error(message);
}
fn warn(&self, message: impl Into<String>) {
fn warn(&self, message: impl Display) {
self.inner_logger.warn(message);
}
}
Expand Down
1 change: 0 additions & 1 deletion crates/unused_finder/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }

Expand Down
58 changes: 56 additions & 2 deletions crates/unused_finder/src/graph.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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)]
Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -137,6 +147,38 @@ impl Graph {
initial_frontier_symbols: Vec<(&Path, Vec<ExportedSymbol>)>,
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::<Vec<_>>()
.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::<Vec<_>>()
.join(", ")
)))
.collect::<Vec<_>>()
.join("\n ")
);

let initial_file_edges = initial_frontier_files
.into_iter()
.filter_map(|path| match self.path_to_id.get(path) {
Expand Down Expand Up @@ -184,6 +226,18 @@ impl Graph {
.chain(initial_symbol_edges)
.collect::<Vec<_>>();

debug_logf!(
logger,
"initial frontier ({}:{}): {}",
tag,
frontier.len(),
frontier
.iter()
.map(|p| format!("{}:{}", self.files[p.file_id].file_path.display(), p.symbol))
.collect::<Vec<_>>()
.join("\n ")
);

// Traverse the graph until we exhaust the frontier
const MAX_ITERATIONS: usize = 1_000_000;
for _ in 0..MAX_ITERATIONS {
Expand Down
8 changes: 0 additions & 8 deletions crates/unused_finder/src/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
15 changes: 12 additions & 3 deletions crates/unused_finder/src/unused_finder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -104,7 +104,7 @@ impl SourceFiles {
"Multiple errors occurred during resolution:\n{}",
errors
.into_iter()
.map(|x| format!("{:#?}", x))
.map(|x| format!("{:#}", x))
.collect::<Vec<_>>()
.join("\n")
));
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,4 @@
"."
],
"packageManager": "yarn@4.4.0+sha512.91d93b445d9284e7ed52931369bc89a663414e5582d00eea45c67ddc459a2582919eece27c412d6ffd1bd0793ff35399381cb229326b961798ce4f4cc60ddfdb"
}
}

0 comments on commit e43aa64

Please sign in to comment.