feat(common): output channel foundation (#14727)

Author: stevencartavia

CONTRIBUTING.md

@@ -135,6 +135,17 @@ If you would like to test the binaries built from your change, see [foundryup](h
 
 If you would like to use a debugger with breakpoints to debug a patch you might be working on, keep in mind we currently strip debug info for faster builds, which is _not_ the default. Therefore, to use a debugger, you need to enable it on the workspace [`Cargo.toml`'s `dev` profile](https://github.com/foundry-rs/foundry/tree/HEAD/Cargo.toml#L15-L18).
 
+#### Output channels (stdout vs. stderr)
+
+Foundry CLIs follow a strict output-channel contract: **stdout is the command's
+machine-readable result; stderr is everything else** (warnings, errors,
+progress, status prose, prompts). When adding or modifying user-facing output,
+read [`docs/dev/output-channels.md`](docs/dev/output-channels.md) and use the
+`sh_*` macros from `foundry_common::io` (`sh_println!`, `sh_status!`,
+`sh_progress!`, `sh_warn!`, `sh_err!`). A workspace-wide clippy
+`disallowed-macros` lint (see [`clippy.toml`](clippy.toml)) forbids
+`std::print*` and `std::eprint*`; use the `sh_*` macros instead.
+
 #### Adding tests
 
 If the change being proposed alters code, it is either adding new functionality to Foundry, or fixing existing, broken functionality.

crates/common/src/compile.rs

@@ -487,7 +487,8 @@ pub struct ContractInfo {
 
 /// Compiles target file path.
 ///
-/// If `quiet` no solc related output will be emitted to stdout.
+/// If `quiet` is set, the compilation reporter's progress/status output is suppressed.
+/// (When not suppressed, that output is emitted to stderr; see `with_compilation_reporter`.)
 ///
 /// **Note:** this expects the `target_path` to be absolute
 pub fn compile_target<C: Compiler<CompilerContract = Contract>>(
@@ -554,6 +555,11 @@ pub fn etherscan_project(metadata: &Metadata, target_path: &Path) -> Result<Proj
 }
 
 /// Configures the reporter and runs the given closure.
+///
+/// In TTY mode, [`SpinnerReporter`] paints the progress to stderr. The non-TTY fallback
+/// still writes to stdout via `BasicStdoutReporter`; migrating that path to stderr is
+/// part of the per-command stdout migration tracked in `docs/dev/output-channels.md`
+/// (it would shift many existing snapshot tests at once).
 pub fn with_compilation_reporter<O>(
     quiet: bool,
     project_root: Option<PathBuf>,
@@ -563,7 +569,7 @@ pub fn with_compilation_reporter<O>(
     let reporter = if quiet || shell::is_json() {
         Report::new(NoReporter::default())
     } else {
-        if std::io::stdout().is_terminal() {
+        if std::io::stderr().is_terminal() {
             Report::new(SpinnerReporter::spawn(project_root))
         } else {
             Report::new(BasicStdoutReporter::default())

crates/common/src/io/macros.rs

@@ -1,4 +1,4 @@
-/// Prints a message to [`stdout`][std::io::stdout] and reads a line from stdin into a String.
+/// Prints a message to [`stderr`][std::io::stderr] and reads a line from stdin into a String.
 ///
 /// Returns `Result<T>`, so sometimes `T` must be explicitly specified, like in `str::parse`.
 ///
@@ -20,10 +20,10 @@ macro_rules! prompt {
     };
 
     ($($tt:tt)+) => {{
-        let _ = $crate::sh_print!($($tt)+);
-        match ::std::io::Write::flush(&mut ::std::io::stdout()) {
+        let _ = $crate::sh_eprint!($($tt)+);
+        match ::std::io::Write::flush(&mut ::std::io::stderr()) {
             ::core::result::Result::Ok(()) => $crate::prompt!(),
-            ::core::result::Result::Err(e) => ::core::result::Result::Err(::eyre::eyre!("Could not flush stdout: {e}"))
+            ::core::result::Result::Err(e) => ::core::result::Result::Err(::eyre::eyre!("Could not flush stderr: {e}"))
         }
     }};
 }
@@ -102,6 +102,36 @@ macro_rules! sh_println {
     };
 }
 
+/// Prints a status message to stderr with a trailing newline.
+///
+/// Use for human-facing diagnostic prose ("Compiling…", "Deploying contract…")
+/// that is not the command's primary machine-readable result.
+#[macro_export]
+macro_rules! sh_status {
+    ($($args:tt)*) => {
+        $crate::sh_eprintln!($($args)*)
+    };
+}
+
+/// Prints a progress message to stderr with a trailing newline.
+///
+/// Use for transient progress updates outside the spinner.
+///
+/// Suppressed when:
+/// - `--quiet` is set, or
+/// - stderr is not a tty (e.g. CI logs, piped consumers).
+///
+/// Always returns `Ok(())`; progress is best-effort and never fails the caller.
+#[macro_export]
+macro_rules! sh_progress {
+    ($($args:tt)*) => {{
+        if $crate::shell::is_err_tty() && !$crate::shell::is_quiet() {
+            let _ = $crate::sh_eprintln!($($args)*);
+        }
+        ::core::result::Result::<(), ::eyre::Report>::Ok(())
+    }};
+}
+
 /// Prints a raw formatted message to stderr, with a trailing newline.
 ///
 /// **Note**: if `verbosity` is set to `Quiet`, this is a no-op.
@@ -176,6 +206,12 @@ mod tests {
         sh_eprintln!("eprintln")?;
         sh_eprintln!("eprintln {}", "arg")?;
 
+        sh_status!("status")?;
+        sh_status!("status {}", "arg")?;
+
+        sh_progress!("progress")?;
+        sh_progress!("progress {}", "arg")?;
+
         sh_println!("{:?}", {
             sh_println!("hi")?;
             solar::data_structures::fmt::from_fn(|f| {
@@ -198,4 +234,64 @@ mod tests {
 
         Ok(())
     }
+
+    /// Asserts that every macro routes to the channel documented in
+    /// `docs/dev/output-channels.md`.
+    #[test]
+    fn routing_contract() -> eyre::Result<()> {
+        let mut shell = crate::Shell::captured();
+
+        // stdout: machine-readable result
+        sh_print!(&mut shell, "out-print")?;
+        sh_println!(&mut shell, "out-println")?;
+
+        // stderr: diagnostics + raw stderr
+        sh_eprint!(&mut shell, "err-print")?;
+        sh_eprintln!(&mut shell, "err-println")?;
+        crate::Shell::warn(&mut shell, "warn-msg")?;
+        crate::Shell::error(&mut shell, "err-msg")?;
+
+        let stdout = std::str::from_utf8(shell.captured_stdout().unwrap()).unwrap();
+        let stderr = std::str::from_utf8(shell.captured_stderr().unwrap()).unwrap();
+
+        // stdout only contains what `sh_print!`/`sh_println!` produced.
+        assert_eq!(stdout, "out-printout-println\n");
+
+        // stderr received the eprint/warn/error output and no stdout content.
+        assert!(stderr.contains("err-print"), "stderr missing eprint: {stderr:?}");
+        assert!(stderr.contains("err-println"), "stderr missing eprintln: {stderr:?}");
+        assert!(stderr.contains("warn-msg"), "stderr missing warn: {stderr:?}");
+        assert!(stderr.contains("err-msg"), "stderr missing error: {stderr:?}");
+        assert!(!stderr.contains("out-print"), "stdout content leaked to stderr: {stderr:?}");
+        assert!(!stderr.contains("out-println"), "stdout content leaked to stderr: {stderr:?}");
+
+        Ok(())
+    }
+
+    /// `--quiet` currently suppresses both stdout and stderr diagnostics, but `sh_err!` must
+    /// always be visible. The stdout half of this is intentional for now; it will be flipped
+    /// to "stdout is never suppressed" once the prose `sh_println!` call sites in forge/script
+    /// are migrated to `sh_status!` (see `docs/dev/output-channels.md`).
+    #[test]
+    fn quiet_contract() -> eyre::Result<()> {
+        let mut shell = crate::Shell::captured();
+        shell.set_output_mode(crate::shell::OutputMode::Quiet);
+
+        sh_println!(&mut shell, "result")?;
+        sh_eprintln!(&mut shell, "diag")?;
+        crate::Shell::warn(&mut shell, "warned")?;
+        crate::Shell::error(&mut shell, "boom")?;
+
+        let stdout = std::str::from_utf8(shell.captured_stdout().unwrap()).unwrap();
+        let stderr = std::str::from_utf8(shell.captured_stderr().unwrap()).unwrap();
+
+        // Today's behavior: stdout is suppressed by --quiet. Pinned here so the future
+        // migration that flips this bypass has to deliberately update the test.
+        assert!(stdout.is_empty(), "stdout leaked through --quiet: {stdout:?}");
+        assert!(!stderr.contains("diag"), "eprintln leaked through --quiet: {stderr:?}");
+        assert!(!stderr.contains("warned"), "warn leaked through --quiet: {stderr:?}");
+        assert!(stderr.contains("boom"), "sh_err was suppressed by --quiet: {stderr:?}");
+
+        Ok(())
+    }
 }

crates/common/src/io/shell.rs

@@ -38,6 +38,13 @@ pub fn is_quiet() -> bool {
     Shell::get().output_mode().is_quiet()
 }
 
+/// Returns whether stderr is a terminal (tty).
+///
+/// Used to gate progress/spinner output that only makes sense for interactive use.
+pub fn is_err_tty() -> bool {
+    Shell::get().is_err_tty()
+}
+
 /// Returns whether the output format is [`OutputFormat::Json`].
 pub fn is_json() -> bool {
     Shell::get().is_json()
@@ -150,6 +157,8 @@ enum ShellOut {
     },
     /// A write object that ignores all output.
     Empty(std::io::Empty),
+    /// Captures stdout and stderr into in-memory buffers. Intended for tests.
+    Captured { stdout: Vec<u8>, stderr: Vec<u8> },
 }
 
 /// Whether messages should use color output.
@@ -214,6 +223,37 @@ impl Shell {
         }
     }
 
+    /// Creates a shell that captures stdout and stderr into in-memory buffers.
+    ///
+    /// Intended for tests that want to assert how a piece of code routes output
+    /// between stdout and stderr. Use [`Shell::captured_stdout`] and
+    /// [`Shell::captured_stderr`] to read the buffers back.
+    pub const fn captured() -> Self {
+        Self {
+            output: ShellOut::Captured { stdout: Vec::new(), stderr: Vec::new() },
+            output_format: OutputFormat::Text,
+            output_mode: OutputMode::Normal,
+            verbosity: 0,
+            needs_clear: AtomicBool::new(false),
+        }
+    }
+
+    /// Returns the captured stdout buffer, if this shell was created via [`Shell::captured`].
+    pub fn captured_stdout(&self) -> Option<&[u8]> {
+        match &self.output {
+            ShellOut::Captured { stdout, .. } => Some(stdout),
+            _ => None,
+        }
+    }
+
+    /// Returns the captured stderr buffer, if this shell was created via [`Shell::captured`].
+    pub fn captured_stderr(&self) -> Option<&[u8]> {
+        match &self.output {
+            ShellOut::Captured { stderr, .. } => Some(stderr),
+            _ => None,
+        }
+    }
+
     /// Acquire a lock to the global shell.
     ///
     /// Initializes it with the default values if it has not been set yet.
@@ -283,38 +323,43 @@ impl Shell {
         self.verbosity = verbosity;
     }
 
+    /// Sets the output mode.
+    pub const fn set_output_mode(&mut self, output_mode: OutputMode) {
+        self.output_mode = output_mode;
+    }
+
     /// Gets the current color choice.
     ///
     /// If we are not using a color stream, this will always return `Never`, even if the color
     /// choice has been set to something else.
     pub const fn color_choice(&self) -> ColorChoice {
         match self.output {
             ShellOut::Stream { color_choice, .. } => color_choice,
-            ShellOut::Empty(_) => ColorChoice::Never,
+            ShellOut::Empty(_) | ShellOut::Captured { .. } => ColorChoice::Never,
         }
     }
 
     /// Returns `true` if stderr is a tty.
     pub const fn is_err_tty(&self) -> bool {
         match self.output {
             ShellOut::Stream { stderr_tty, .. } => stderr_tty,
-            ShellOut::Empty(_) => false,
+            ShellOut::Empty(_) | ShellOut::Captured { .. } => false,
         }
     }
 
     /// Whether `stderr` supports color.
     pub fn err_supports_color(&self) -> bool {
         match &self.output {
             ShellOut::Stream { stderr, .. } => supports_color(stderr.current_choice()),
-            ShellOut::Empty(_) => false,
+            ShellOut::Empty(_) | ShellOut::Captured { .. } => false,
         }
     }
 
     /// Whether `stdout` supports color.
     pub fn out_supports_color(&self) -> bool {
         match &self.output {
             ShellOut::Stream { stdout, .. } => supports_color(stdout.current_choice()),
-            ShellOut::Empty(_) => false,
+            ShellOut::Empty(_) | ShellOut::Captured { .. } => false,
         }
     }
 
@@ -370,6 +415,10 @@ impl Shell {
     /// Write a styled fragment with the default color. Use the [`sh_print!`] macro instead.
     ///
     /// **Note**: if `verbosity` is set to `Quiet`, this is a no-op.
+    //
+    // TODO: stdout is the canonical machine-readable result of a command and should NOT be
+    // suppressed by `--quiet` (see `docs/dev/output-channels.md`). Flip this once the major
+    // prose `sh_println!` call sites in forge/script have been migrated to `sh_status!`.
     pub fn print_out(&mut self, fragment: impl fmt::Display) -> Result<()> {
         match self.output_mode {
             OutputMode::Quiet => Ok(()),
@@ -450,6 +499,7 @@ impl ShellOut {
         match self {
             Self::Stream { stdout, .. } => stdout,
             Self::Empty(e) => e,
+            Self::Captured { stdout, .. } => stdout,
         }
     }
 
@@ -458,6 +508,7 @@ impl ShellOut {
         match self {
             Self::Stream { stderr, .. } => stderr,
             Self::Empty(e) => e,
+            Self::Captured { stderr, .. } => stderr,
         }
     }
 

crates/common/src/term.rs

@@ -39,8 +39,11 @@ pub struct TermSettings {
 
 impl TermSettings {
     /// Returns a new [`TermSettings`], configured from the current environment.
+    ///
+    /// Progress is written to stderr (see [`Spinner::tick`]), so it is enabled only
+    /// when stderr is a terminal.
     pub fn from_env() -> Self {
-        Self { indicate_progress: std::io::stdout().is_terminal() }
+        Self { indicate_progress: std::io::stderr().is_terminal() }
     }
 }
 
@@ -74,8 +77,10 @@ impl Spinner {
 
         let indicator = self.indicator[self.idx % self.indicator.len()].green();
         let indicator = Paint::new(format!("[{indicator}]")).bold();
-        let _ = sh_print!("\r\x1B[2K\r{indicator} {}", self.message);
-        io::stdout().flush().unwrap();
+        // Progress is a diagnostic, not data: write to stderr so stdout stays clean
+        // for machine-readable output.
+        let _ = sh_eprint!("\r\x1B[2K\r{indicator} {}", self.message);
+        io::stderr().flush().unwrap();
 
         self.idx = self.idx.wrapping_add(1);
     }
@@ -110,17 +115,27 @@ impl SpinnerReporter {
             .name("spinner".into())
             .spawn(move || {
                 let mut spinner = Spinner::new("Compiling...");
+                // Only emit the trailing newline (so past messages aren't overwritten by
+                // future ticks) when the spinner is actually painting to stderr. When
+                // `no_progress` is set the spinner is a no-op, so we shouldn't pollute
+                // stderr with blank lines either.
+                let emits_progress = !spinner.no_progress;
                 loop {
                     spinner.tick();
                     match rx.try_recv() {
                         Ok(SpinnerMsg::Msg(msg)) => {
                             spinner.message(msg);
-                            // new line so past messages are not overwritten
-                            let _ = sh_println!();
+                            if emits_progress {
+                                // new line so past messages are not overwritten
+                                // (matches the spinner channel: stderr)
+                                let _ = sh_eprintln!();
+                            }
                         }
                         Ok(SpinnerMsg::Shutdown(ack)) => {
-                            // end with a newline
-                            let _ = sh_println!();
+                            if emits_progress {
+                                // end with a newline (matches the spinner channel: stderr)
+                                let _ = sh_eprintln!();
+                            }
                             let _ = ack.send(());
                             break;
                         }