From 47e6a98989e2c756ebdb1bf1f6cdfa55a4a99cab Mon Sep 17 00:00:00 2001 From: Will Chandler Date: Wed, 17 Jul 2024 20:34:40 -0400 Subject: [PATCH 1/2] Ban use of print macros in non-test code With f472bce (Ignore EPIPE in CLI (#746), 2024-07-17) we added the `_nopipe` variants of the `(e)print(ln)!` macros to avoid panicking when piping to head(1). However, we do not systematically enforce their use, which will inevitably lead inconsistent usage within the project. Add `clippy` lints to ban use of the print macros in all non-test code. Either the `_nopipe` variants should be used when we don't care if the input is read, or `write!` when the information is crucial, such as an interactive session. --- cli/src/cmd_auth.rs | 32 +++++++++++++++++++++----------- cli/src/cmd_docs.rs | 4 ++-- cli/src/cmd_instance.rs | 3 ++- cli/src/main.rs | 1 + 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/cli/src/cmd_auth.rs b/cli/src/cmd_auth.rs index 3a216487..49992881 100644 --- a/cli/src/cmd_auth.rs +++ b/cli/src/cmd_auth.rs @@ -5,6 +5,7 @@ // Copyright 2024 Oxide Computer Company use std::fs::File; +use std::io::{self, Write}; use anyhow::{anyhow, bail, Result}; use async_trait::async_trait; @@ -237,12 +238,16 @@ impl CmdAuthLogin { }; if opened { - println!("Opened this URL in your browser:\n {}", uri); + writeln!(io::stdout(), "Opened this URL in your browser:\n {}", uri)?; } else { - println!("Open this URL in your browser:\n {}", uri); + writeln!(io::stdout(), "Open this URL in your browser:\n {}", uri)?; } - println!("\nEnter the code: {}\n", details.user_code().secret()); + writeln!( + io::stdout(), + "\nEnter the code: {}\n", + details.user_code().secret() + )?; let token = auth_client .exchange_device_access_token(&details) @@ -357,13 +362,17 @@ impl CmdAuthLogin { silo_name, } = &user; - println!("Login successful"); - println!(" silo: {} ({})", **silo_name, silo_id); - println!(" user: {} ({})", display_name, id); + writeln!(io::stdout(), "Login successful")?; + writeln!(io::stdout(), " silo: {} ({})", **silo_name, silo_id)?; + writeln!(io::stdout(), " user: {} ({})", display_name, id)?; if ctx.config_file().basics.default_profile.is_none() { - println!("Profile '{}' set as the default", profile_name); + writeln!( + io::stdout(), + "Profile '{}' set as the default", + profile_name + )?; } else { - println!("Use --profile '{}'", profile_name); + writeln!(io::stdout(), "Use --profile '{}'", profile_name)?; } Ok(()) @@ -396,7 +405,7 @@ impl CmdAuthLogout { if self.all { // Clear the entire file for users who want to reset their known hosts. let _ = File::create(credentials_path)?; - println!("Removed all authentication information"); + writeln!(io::stdout(), "Removed all authentication information")?; } else { let profile = ctx .client_config() @@ -424,10 +433,11 @@ impl CmdAuthLogout { }); std::fs::write(credentials_path, credentials.to_string()) .expect("unable to write credentials.toml"); - println!( + writeln!( + io::stdout(), "Removed authentication information for profile \"{}\"", profile_name, - ); + )?; } Ok(()) diff --git a/cli/src/cmd_docs.rs b/cli/src/cmd_docs.rs index 05cf06d6..1e4c3450 100644 --- a/cli/src/cmd_docs.rs +++ b/cli/src/cmd_docs.rs @@ -5,7 +5,7 @@ // Copyright 2024 Oxide Computer Company use crate::context::Context; -use crate::RunnableCmd; +use crate::{println_nopipe, RunnableCmd}; use super::cmd_version::built_info; use anyhow::Result; @@ -97,7 +97,7 @@ impl RunnableCmd for CmdDocs { app.build(); let json_doc = to_json(&app); let pretty_json = serde_json::to_string_pretty(&json_doc)?; - println!("{}", pretty_json); + println_nopipe!("{}", pretty_json); Ok(()) } } diff --git a/cli/src/cmd_instance.rs b/cli/src/cmd_instance.rs index 21fe018c..1b4e2595 100644 --- a/cli/src/cmd_instance.rs +++ b/cli/src/cmd_instance.rs @@ -14,6 +14,7 @@ use oxide::types::{ use oxide::ClientInstancesExt; use oxide::{Client, ClientImagesExt}; +use std::io::{self, Write}; use std::path::PathBuf; /// Connect to or retrieve data from the instance's serial console. @@ -196,7 +197,7 @@ impl CmdInstanceSerialHistory { let data = req.send().await.map_err(|e| e.into_untyped())?.into_inner(); if self.json { - println!("{}", serde_json::to_string(&data)?); + writeln!(io::stdout(), "{}", serde_json::to_string(&data)?)?; } else { let mut tty = thouart::Console::new_stdio(None).await?; tty.write_stdout(&data.data).await?; diff --git a/cli/src/main.rs b/cli/src/main.rs index 4acbedae..a5795b54 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -5,6 +5,7 @@ // Copyright 2024 Oxide Computer Company #![forbid(unsafe_code)] +#![cfg_attr(not(test), deny(clippy::print_stdout, clippy::print_stderr))] use std::io; use std::net::IpAddr; From d6b73ca6fed03e5185903928debd3600be5d83b0 Mon Sep 17 00:00:00 2001 From: "Adam H. Leventhal" Date: Thu, 8 Aug 2024 09:24:26 -0700 Subject: [PATCH 2/2] one more --- cli/src/cmd_instance.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cli/src/cmd_instance.rs b/cli/src/cmd_instance.rs index 1b4e2595..c31a7b2d 100644 --- a/cli/src/cmd_instance.rs +++ b/cli/src/cmd_instance.rs @@ -17,6 +17,8 @@ use oxide::{Client, ClientImagesExt}; use std::io::{self, Write}; use std::path::PathBuf; +use crate::println_nopipe; + /// Connect to or retrieve data from the instance's serial console. #[derive(Parser, Debug, Clone)] #[command(verbatim_doc_comment)] @@ -284,7 +286,7 @@ impl crate::AuthenticatedCmd for CmdInstanceFromImage { .send() .await?; - println!("instance {} created", instance.id); + println_nopipe!("instance {} created", instance.id); Ok(()) }