Repository: ast-grep/ast-grep (language: rust)

Issue:
[refactor] move eprintn into tracing



Discussion / hints:

Fix patch:
diff --git a/crates/cli/src/run.rs b/crates/cli/src/run.rs
index 3110d399c9..1bb755805f 100644
--- a/crates/cli/src/run.rs
+++ b/crates/cli/src/run.rs
@@ -189,10 +189,7 @@ impl<P: Printer> Worker for RunWithInferredLang<P> {
       }
     }
     printer.after_print()?;
-    // TODO: better handle output format
-    if let Some(trace) = self.trace.print() {
-      eprintln!("{}", trace);
-    }
+    self.trace.print()?;
     Ok(())
   }
 }
@@ -207,9 +204,7 @@ impl<P: Printer> PathWorker for RunWithInferredLang<P> {
 
   fn produce_item(&self, path: &Path) -> Option<Vec<Self::Item>> {
     let lang = SgLang::from_path(path)?;
-    if let Some(n) = self.trace.print_file(path, lang) {
-      eprintln!("{}", n);
-    }
+    self.trace.print_file(path, lang).ok()?;
     let matcher = self.arg.build_pattern(lang).ok()?;
     // match sub region
     if let Some(sub_langs) = lang.injectable_sg_langs() {
@@ -267,9 +262,7 @@ impl<P: Printer> Worker for RunWithSpecificLang<P> {
       has_matches = true;
     }
     printer.after_print()?;
-    if let Some(stats) = self.stats.print() {
-      eprintln!("{}", stats);
-    }
+    self.stats.print()?;
     if !has_matches && self.pattern.has_error() {
       Err(anyhow::anyhow!(EC::PatternHasError))
     } else {
@@ -291,9 +284,7 @@ impl<P: Printer> PathWorker for RunWithSpecificLang<P> {
     let pattern = self.pattern.clone();
     let lang = arg.lang.expect("must present");
     let path_lang = SgLang::from_path(path)?;
-    if let Some(trace) = self.stats.print_file(path, path_lang) {
-      eprintln!("{trace}");
-    }
+    self.stats.print_file(path, path_lang).ok()?;
     let ret = if path_lang == lang {
       filter_file_pattern(path, lang, Some(pattern), std::iter::empty())?
     } else {
diff --git a/crates/cli/src/scan.rs b/crates/cli/src/scan.rs
index f8b840ecf8..1aaddf78ae 100644
--- a/crates/cli/src/scan.rs
+++ b/crates/cli/src/scan.rs
@@ -177,9 +177,7 @@ impl<P: Printer> Worker for ScanWithConfig<P> {
       )?;
     }
     self.printer.after_print()?;
-    if let Some(trace) = self.trace.print() {
-      eprintln!("{}", trace);
-    }
+    self.trace.print()?;
     if error_count > 0 {
       Err(anyhow::anyhow!(EC::DiagnosticError(error_count)))
     } else {
diff --git a/crates/cli/src/utils/inspect.rs b/crates/cli/src/utils/inspect.rs
index fe14bd067c..a2ba3f9d86 100644
--- a/crates/cli/src/utils/inspect.rs
+++ b/crates/cli/src/utils/inspect.rs
@@ -13,14 +13,15 @@
 use crate::lang::SgLang;
 use ast_grep_config::RuleConfig;
 
+use anyhow::Result;
 use clap::ValueEnum;
-use serde::{Deserialize, Serialize};
 
+use std::io::{Stderr, Write};
 use std::path::Path;
 use std::sync::atomic::{AtomicUsize, Ordering};
+use std::sync::Mutex;
 
-#[derive(Clone, Copy, ValueEnum, Serialize, Deserialize, Default, PartialEq, Debug)]
-#[serde(rename_all = "camelCase")]
+#[derive(Clone, Copy, ValueEnum, Default, PartialEq, Debug)]
 pub enum Granularity {
   /// Do not show any tracing information
   #[default]
@@ -34,25 +35,33 @@ pub enum Granularity {
 
 impl Granularity {
   pub fn run_trace(&self) -> RunTrace {
-    RunTrace {
+    self.run_trace_impl(std::io::stderr())
+  }
+  fn run_trace_impl<W: Write>(&self, w: W) -> TraceInfo<(), W> {
+    TraceInfo {
       level: *self,
       inner: (),
       file_trace: Default::default(),
+      output: Mutex::new(w),
     }
   }
+
   pub fn scan_trace(&self, rule_stats: RuleTrace) -> ScanTrace {
-    ScanTrace {
+    self.scan_trace_impl(rule_stats, std::io::stderr())
+  }
+  fn scan_trace_impl<W: Write>(&self, rule_stats: RuleTrace, w: W) -> TraceInfo<RuleTrace, W> {
+    TraceInfo {
       level: *self,
       inner: rule_stats,
       file_trace: Default::default(),
+      output: Mutex::new(w),
     }
   }
 }
 
 // total = scanned + skipped
 //       = (matched + unmatched) + skipped
-#[derive(Default, Serialize, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[derive(Default)]
 pub struct FileTrace {
   files_scanned: AtomicUsize,
   files_skipped: AtomicUsize,
@@ -65,79 +74,84 @@ impl FileTrace {
   pub fn add_skipped(&self) {
     self.files_skipped.fetch_add(1, Ordering::AcqRel);
   }
-  pub fn print(&self) -> String {
-    format!(
+  pub fn print<W: Write>(&self, w: &mut W) -> Result<()> {
+    write!(
+      w,
       "Files scanned: {}, Files skipped: {}",
       self.files_scanned.load(Ordering::Acquire),
       self.files_skipped.load(Ordering::Acquire)
-    )
+    )?;
+    Ok(())
   }
-  pub fn print_file(&self, path: &Path, lang: SgLang) -> String {
-    format!("Parse {} with {lang}", path.display())
+  pub fn print_file<W: Write>(&self, w: &mut W, path: &Path, lang: SgLang) -> Result<()> {
+    write!(w, "Parse {} with {lang}", path.display())?;
+    Ok(())
   }
 }
 
-#[derive(Serialize, Deserialize)]
-#[serde(rename_all = "camelCase")]
-pub struct TraceInfo<T> {
+pub struct TraceInfo<T, W: Write> {
   pub level: Granularity,
-  #[serde(flatten)]
   pub file_trace: FileTrace,
-  #[serde(flatten)]
   pub inner: T,
+  output: Mutex<W>,
 }
 
-impl TraceInfo<()> {
+impl<W: Write + Sync> TraceInfo<(), W> {
   // TODO: support more format?
-  pub fn print(&self) -> Option<String> {
+  pub fn print(&self) -> Result<()> {
     match self.level {
-      Granularity::Nothing => None,
-      Granularity::Summary | Granularity::Entity => Some(self.file_trace.print()),
+      Granularity::Nothing => Ok(()),
+      Granularity::Summary | Granularity::Entity => {
+        let mut w = self.output.lock().expect("lock should not be poisoned");
+        self.file_trace.print(&mut *w)?;
+        writeln!(&mut *w)?;
+        Ok(())
+      }
     }
   }
 
-  pub fn print_file(&self, path: &Path, lang: SgLang) -> Option<String> {
+  pub fn print_file(&self, path: &Path, lang: SgLang) -> Result<()> {
     match self.level {
-      Granularity::Nothing => None,
-      Granularity::Summary => None,
-      Granularity::Entity => Some(self.file_trace.print_file(path, lang)),
+      Granularity::Nothing | Granularity::Summary => Ok(()),
+      Granularity::Entity => {
+        let mut w = self.output.lock().expect("lock should not be poisoned");
+        self.file_trace.print_file(&mut *w, path, lang)?;
+        writeln!(&mut *w)?;
+        Ok(())
+      }
     }
   }
 }
 
-impl TraceInfo<RuleTrace> {
+impl<W: Write> TraceInfo<RuleTrace, W> {
   // TODO: support more format?
-  pub fn print(&self) -> Option<String> {
+  pub fn print(&self) -> Result<()> {
     match self.level {
-      Granularity::Nothing => None,
-      Granularity::Summary | Granularity::Entity => Some(format!(
-        "{}\n{}",
-        self.file_trace.print(),
-        self.inner.print()
-      )),
+      Granularity::Nothing => Ok(()),
+      Granularity::Summary | Granularity::Entity => {
+        let mut w = self.output.lock().expect("lock should not be poisoned");
+        self.file_trace.print(&mut *w)?;
+        writeln!(&mut *w, "\n{}", self.inner.print())?;
+        Ok(())
+      }
     }
   }
-  pub fn print_file(
-    &self,
-    path: &Path,
-    lang: SgLang,
-    rules: &[&RuleConfig<SgLang>],
-  ) -> Option<String> {
+  pub fn print_file(&self, path: &Path, lang: SgLang, rules: &[&RuleConfig<SgLang>]) -> Result<()> {
     let len = rules.len();
     match self.level {
-      Granularity::Nothing | Granularity::Summary => None,
-      Granularity::Entity => Some(format!(
-        "{}, applied {len} rule(s)",
-        self.file_trace.print_file(path, lang),
-      )),
+      Granularity::Nothing | Granularity::Summary => Ok(()),
+      Granularity::Entity => {
+        let mut w = self.output.lock().expect("lock should not be poisoned");
+        self.file_trace.print_file(&mut *w, path, lang)?;
+        writeln!(&mut *w, ", applied {len} rule(s)")?;
+        Ok(())
+      }
     }
   }
 }
 
-#[derive(Default, Serialize, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[derive(Default)]
 pub struct RuleTrace {
-  #[serde(skip_serializing)]
   pub effective_rule_count: usize,
   pub skipped_rule_count: usize,
 }
@@ -150,8 +164,8 @@ impl RuleTrace {
   }
 }
 
-pub type RunTrace = TraceInfo<()>;
-pub type ScanTrace = TraceInfo<RuleTrace>;
+pub type RunTrace = TraceInfo<(), Stderr>;
+pub type ScanTrace = TraceInfo<RuleTrace, Stderr>;
 
 #[cfg(test)]
 mod test {
@@ -160,7 +174,8 @@ mod test {
   #[test]
   fn test_tracing() {
     let tracing = Granularity::Summary;
-    let run_trace = tracing.run_trace();
+    let mut ret = String::new();
+    let run_trace = tracing.run_trace_impl(unsafe { ret.as_mut_vec() });
     assert_eq!(run_trace.level, Granularity::Summary);
     assert_eq!(
       run_trace.file_trace.files_scanned.load(Ordering::Relaxed),
@@ -170,14 +185,15 @@ mod test {
       run_trace.file_trace.files_skipped.load(Ordering::Relaxed),
       0
     );
-    let printed = run_trace.print().expect("should have output");
-    assert_eq!(printed, "Files scanned: 0, Files skipped: 0");
+    assert!(run_trace.print().is_ok());
+    assert_eq!(ret, "Files scanned: 0, Files skipped: 0\n");
 
+    let mut ret = String::new();
     let rule_stats = RuleTrace {
       effective_rule_count: 10,
       skipped_rule_count: 2,
     };
-    let scan_trace = tracing.scan_trace(rule_stats);
+    let scan_trace = tracing.scan_trace_impl(rule_stats, unsafe { ret.as_mut_vec() });
     assert_eq!(scan_trace.level, Granularity::Summary);
     assert_eq!(
       scan_trace.file_trace.files_scanned.load(Ordering::Relaxed),
@@ -189,19 +205,20 @@ mod test {
     );
     assert_eq!(scan_trace.inner.effective_rule_count, 10);
     assert_eq!(scan_trace.inner.skipped_rule_count, 2);
-    let printed = scan_trace.print().expect("should have output");
+    assert!(scan_trace.print().is_ok());
     assert_eq!(
-      printed,
-      "Files scanned: 0, Files skipped: 0\nEffective rules: 10, Skipped rules: 2"
+      ret,
+      "Files scanned: 0, Files skipped: 0\nEffective rules: 10, Skipped rules: 2\n"
     );
   }
 
   #[test]
   fn test_tracing_nothing() {
     let tracing = Granularity::Nothing;
-    let run_trace = tracing.run_trace();
+    let mut ret = String::new();
+    let run_trace = tracing.run_trace_impl(unsafe { ret.as_mut_vec() });
     assert_eq!(run_trace.level, Granularity::Nothing);
-    let printed = run_trace.print();
-    assert!(printed.is_none());
+    assert!(run_trace.print().is_ok());
+    assert!(ret.is_empty());
   }
 }
diff --git a/crates/cli/src/utils/mod.rs b/crates/cli/src/utils/mod.rs
index 0e97d85383..24617c3c89 100644
--- a/crates/cli/src/utils/mod.rs
+++ b/crates/cli/src/utils/mod.rs
@@ -112,9 +112,7 @@ fn filter(
   rule_stats: &ScanTrace,
 ) -> Option<PreScan> {
   let rules = configs.get_rule_from_lang(path, lang);
-  if let Some(rule_stats) = rule_stats.print_file(path, lang, &rules) {
-    eprintln!("{}", rule_stats);
-  }
+  rule_stats.print_file(path, lang, &rules).ok()?;
   let combined = CombinedScan::new(rules);
   let pre_scan = combined.find(grep);
   if pre_scan.hit_set.is_empty() {


Test patch:
diff --git a/crates/cli/tests/run_test.rs b/crates/cli/tests/run_test.rs
index b59b384a61..826724e0e4 100644
--- a/crates/cli/tests/run_test.rs
+++ b/crates/cli/tests/run_test.rs
@@ -47,3 +47,16 @@ fn test_js_in_html() -> Result<()> {
     .stdout(contains("alert(456)"));
   Ok(())
 }
+
+#[test]
+fn test_inspect() -> Result<()> {
+  let dir = create_test_files([("a.js", "alert(1)"), ("b.js", "alert(456)")])?;
+  Command::cargo_bin("sg")?
+    .current_dir(dir.path())
+    .args(["-p", "alert($A)", "-l", "js", "--inspect", "entity"])
+    .assert()
+    .success()
+    .stdout(contains("alert(1)"))
+    .stderr(contains("Files scanned: 2"));
+  Ok(())
+}