From db1c5394356009315576677ef07b0f178328c9af Mon Sep 17 00:00:00 2001 From: grouvie <20428462+grouvie@users.noreply.github.com> Date: Fri, 24 Apr 2026 13:14:28 +0200 Subject: [PATCH 01/54] chore(cargo): add crate metadata and strict rust/clippy lint baseline --- Cargo.toml | 250 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 250 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 75e5675..8b079ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,12 @@ version = "0.1.0" edition = "2024" license = "MIT" +description = "A simple STDIO forwarder over TCP that also offers a health check port." +repository = "https://github.com/ninjaneers-team/stdioxide" +readme = "README.md" +keywords = ["tcp", "stdio", "tcp-proxy", "language-server", "health-check"] +categories = ["command-line-utilities", "network-programming", "development-tools"] + [dependencies] anyhow = "1.0.102" clap = { version = "4.6.1", features = ["derive", "env"] } @@ -12,3 +18,247 @@ subprocess = "1.0.3" [dev-dependencies] lsp-types = "0.95" serde_json = "1.0" + +[lints.rust] +###################################################################################################################### +# Broad rustc lint groups +###################################################################################################################### + +# Future-proofing and edition migration. +future_incompatible = { level = "warn", priority = -1 } +rust_2018_idioms = { level = "warn", priority = -1 } +rust_2021_compatibility = { level = "warn", priority = -1 } +rust_2024_compatibility = { level = "warn", priority = -1 } + +# Useful rustc lint groups. +keyword_idents = { level = "warn", priority = -1 } +let_underscore = { level = "warn", priority = -1 } +nonstandard_style = { level = "warn", priority = -1 } +refining_impl_trait = { level = "warn", priority = -1 } +unknown_or_malformed_diagnostic_attributes = { level = "warn", priority = -1 } +unused = { level = "warn", priority = -1 } + +###################################################################################################################### +# Lints commonly overridden depending on project state or type +###################################################################################################################### + +# These can be noisy during early development, but are useful before release. +missing_docs = "warn" +dead_code = "warn" + +# Disable if this reports false positives for build-dependencies or target-specific deps. +unused_crate_dependencies = "warn" + +###################################################################################################################### +# Rust 2024 / compatibility lints worth keeping explicit +###################################################################################################################### + +boxed_slice_into_iter = "warn" +dependency_on_unit_never_type_fallback = "warn" +deprecated_safe_2024 = "warn" +edition_2024_expr_fragment_specifier = "warn" +if_let_rescope = "warn" +impl_trait_overcaptures = "warn" +keyword_idents_2024 = "warn" +missing_unsafe_on_extern = "warn" +never_type_fallback_flowing_into_unsafe = "warn" +rust_2024_guarded_string_incompatible_syntax = "warn" +rust_2024_incompatible_pat = "warn" +rust_2024_prelude_collisions = "warn" +static_mut_refs = "warn" +tail_expr_drop_order = "warn" +unsafe_attr_outside_unsafe = "warn" + +###################################################################################################################### +# Lints that should only be overridden for a small scope with a comment explaining the exception +###################################################################################################################### + +absolute_paths_not_starting_with_crate = "warn" +elided_lifetimes_in_paths = "warn" +explicit_outlives_requirements = "warn" +ffi_unwind_calls = "warn" +let_underscore_drop = "warn" +let_underscore_lock = "warn" +macro_use_extern_crate = "warn" +meta_variable_misuse = "warn" +missing_abi = "warn" +non_ascii_idents = "warn" +rust_2021_incompatible_closure_captures = "warn" +rust_2021_incompatible_or_patterns = "warn" +rust_2021_prefixes_incompatible_syntax = "warn" +rust_2021_prelude_collisions = "warn" +single_use_lifetimes = "warn" +trivial_casts = "warn" +trivial_numeric_casts = "warn" +unit_bindings = "warn" +unreachable_pub = "warn" +unsafe_code = "forbid" +unsafe_op_in_unsafe_fn = "warn" +unstable_features = "warn" +unused_extern_crates = "warn" +unused_import_braces = "warn" +unused_lifetimes = "warn" +unused_macro_rules = "warn" +unused_qualifications = "warn" +variant_size_differences = "warn" + +# Unstable lints available on nightly only: +# fuzzy_provenance_casts = "warn" +# lossy_provenance_casts = "warn" +# multiple_supertrait_upcastable = "warn" +# must_not_suspend = "warn" +# non_exhaustive_omitted_patterns = "warn" +# unnameable_types = "warn" + +[lints.clippy] +###################################################################################################################### +# Broad Clippy groups +###################################################################################################################### + +all = { level = "warn", priority = -1 } +pedantic = { level = "warn", priority = -1 } +nursery = { level = "warn", priority = -1 } + +# checks Cargo.toml quality, feature naming, duplicate crate versions, wildcard deps, etc. +cargo = { level = "warn", priority = -1 } + +###################################################################################################################### +# Lints commonly overridden depending on project state or type +###################################################################################################################### + +# These are noisy during early development, but useful before release. +missing_panics_doc = "warn" +missing_errors_doc = "warn" + +# Disable or locally allow until proper error handling has been implemented. +unwrap_used = "warn" +panic = "warn" +todo = "warn" +expect_used = "warn" +missing_assert_message = "warn" +unwrap_in_result = "warn" +indexing_slicing = "warn" +panic_in_result_fn = "warn" + +# Libraries should generally not write to stdout/stderr. +# For CLI binaries, locally allow these in the presentation / command layer with a reason. +print_stderr = "warn" +print_stdout = "warn" + +# These two lints can interfere. Keep one allowed globally and handle the other locally. +pattern_type_mismatch = "allow" +needless_borrowed_reference = "warn" + +###################################################################################################################### +# Cargo / manifest lints +###################################################################################################################### + +cargo_common_metadata = "warn" +multiple_crate_versions = "warn" +negative_feature_names = "warn" +redundant_feature_names = "warn" +wildcard_dependencies = "warn" + +###################################################################################################################### +# Extra strict restriction lints worth considering +###################################################################################################################### + +arithmetic_side_effects = "warn" +as_conversions = "warn" +as_pointer_underscore = "warn" +big_endian_bytes = "warn" +default_numeric_fallback = "warn" +else_if_without_else = "warn" +exhaustive_enums = "warn" +exhaustive_structs = "warn" +field_scoped_visibility_modifiers = "warn" +float_arithmetic = "warn" +impl_trait_in_params = "warn" +integer_division = "warn" +little_endian_bytes = "warn" +missing_docs_in_private_items = "warn" +pub_use = "warn" +ref_patterns = "warn" +same_name_method = "warn" +self_named_module_files = "warn" +separated_literal_suffix = "warn" +std_instead_of_alloc = "warn" +std_instead_of_core = "warn" +str_to_string = "warn" +string_add = "warn" +string_to_string = "allow" # Deprecated; keep allowed if older toolchains still mention it. +unnecessary_self_imports = "warn" +wildcard_enum_match_arm = "warn" + +###################################################################################################################### +# Maintenance lints that are generally overridden, but useful for cleanup +###################################################################################################################### + +# Noisy, but useful for periodic cleanup. +shadow_unrelated = "warn" + +###################################################################################################################### +# Lints that should only be overridden for a small scope with a comment explaining the exception +###################################################################################################################### + +absolute_paths = "warn" +alloc_instead_of_core = "warn" +allow_attributes = "warn" +allow_attributes_without_reason = "warn" +assertions_on_result_states = "warn" +clone_on_ref_ptr = "warn" +create_dir = "warn" +dbg_macro = "warn" +decimal_literal_representation = "warn" +default_union_representation = "warn" +deref_by_slicing = "warn" +disallowed_script_idents = "warn" +empty_drop = "warn" +empty_enum_variants_with_brackets = "warn" +empty_structs_with_brackets = "warn" +error_impl_error = "warn" +exit = "warn" +filetype_is_file = "warn" +float_cmp_const = "warn" +fn_to_numeric_cast_any = "warn" +format_push_string = "warn" +get_unwrap = "warn" +host_endian_bytes = "warn" +if_then_some_else_none = "warn" +infinite_loop = "warn" +inline_asm_x86_att_syntax = "warn" +inline_asm_x86_intel_syntax = "warn" +large_include_file = "warn" +let_underscore_must_use = "warn" +let_underscore_untyped = "warn" +lossy_float_literal = "warn" +map_err_ignore = "warn" +mem_forget = "warn" +min_ident_chars = "warn" +mixed_read_write_in_expression = "warn" +mod_module_files = "warn" +modulo_arithmetic = "warn" +module_name_repetitions = "warn" +multiple_inherent_impl = "allow" +multiple_unsafe_ops_per_block = "warn" +mutex_atomic = "warn" +needless_raw_strings = "warn" +pub_without_shorthand = "warn" +rc_buffer = "warn" +rc_mutex = "warn" +redundant_type_annotations = "warn" +rest_pat_in_fully_bound_structs = "warn" +semicolon_inside_block = "warn" +shadow_same = "warn" +single_char_lifetime_names = "warn" +string_lit_chars_any = "warn" +string_slice = "warn" +suspicious_xor_used_as_pow = "warn" +tests_outside_test_module = "warn" +try_err = "warn" +undocumented_unsafe_blocks = "warn" +unimplemented = "warn" +unnecessary_safety_comment = "warn" +unnecessary_safety_doc = "warn" +unseparated_literal_suffix = "warn" +verbose_file_reads = "warn" From db21297db2077f332f0ff48faaeb3ecd7fc551a3 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 09:25:00 +0200 Subject: [PATCH 02/54] fix: use explicit drop() for intentionally ignored Result values Replace let _ = ... patterns with drop(...) for Result return values in spawned thread closures. This makes the intent explicit and addresses all let-underscore-drop warnings. --- src/app.rs | 15 ++++++++++----- src/control.rs | 4 ++-- src/servers/protocol.rs | 8 ++++---- src/servers/stderr.rs | 8 ++++---- tests/integration_test.rs | 16 ++++++++-------- tests/lsp_client.rs | 4 ++-- 6 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/app.rs b/src/app.rs index 0fd5e09..f16a17a 100644 --- a/src/app.rs +++ b/src/app.rs @@ -27,14 +27,14 @@ pub fn run(args: Args) -> Result<(), anyhow::Error> { { let stdout_state = Arc::clone(&stdout_state); thread::spawn(move || { - let _ = pump_output_to_state(child.stdout, stdout_state, "stdout"); + drop(pump_output_to_state(child.stdout, stdout_state, "stdout")); }); } { let stderr_state = Arc::clone(&stderr_state); thread::spawn(move || { - let _ = pump_output_to_state(child.stderr, stderr_state, "stderr"); + drop(pump_output_to_state(child.stderr, stderr_state, "stderr")); }); } @@ -42,7 +42,12 @@ pub fn run(args: Args) -> Result<(), anyhow::Error> { let stdout_state = Arc::clone(&stdout_state); let control_tx = control_tx.clone(); thread::spawn(move || { - let _ = protocol_server(protocol_listener, stdout_state, child.stdin, control_tx); + drop(protocol_server( + protocol_listener, + stdout_state, + child.stdin, + control_tx, + )); }); } @@ -50,7 +55,7 @@ pub fn run(args: Args) -> Result<(), anyhow::Error> { let stderr_state = Arc::clone(&stderr_state); let control_tx = control_tx.clone(); thread::spawn(move || { - let _ = stderr_server(stderr_listener, stderr_state, control_tx); + drop(stderr_server(stderr_listener, stderr_state, control_tx)); }); } @@ -60,7 +65,7 @@ pub fn run(args: Args) -> Result<(), anyhow::Error> { { thread::spawn(move || { - let _ = health_server(health_listener); + drop(health_server(health_listener)); }); } diff --git a/src/control.rs b/src/control.rs index fe0d4f9..fe2935d 100644 --- a/src/control.rs +++ b/src/control.rs @@ -19,7 +19,7 @@ pub fn run_child_coordinator( match control_rx.recv_timeout(std::time::Duration::from_millis(100)) { Ok(ControlMessage::KillChild) => { - let _ = job.kill(); + drop(job.kill()); let status = job.wait()?; eprintln!("Child process killed; exit status: {status}"); return Ok(()); @@ -30,7 +30,7 @@ pub fn run_child_coordinator( Err(mpsc::RecvTimeoutError::Disconnected) => { // All senders are gone; we terminate the child process and exit. eprintln!("Control channel disconnected; terminating child process"); - let _ = job.kill(); + drop(job.kill()); let status = job.wait()?; eprintln!("Child process killed; exit status: {status}"); return Ok(()); diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index 6e989c2..c825cc7 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -60,20 +60,20 @@ pub fn protocol_server( let cloned_control_tx = control_tx.clone(); ( thread::spawn(move || { - let _ = forward_stream_data_to_child_process( + drop(forward_stream_data_to_child_process( cloned_stream, child_stdin, cloned_control_tx, - ); + )); }), thread::spawn(move || { - let _ = serve_output_on_stream( + drop(serve_output_on_stream( stream, Arc::clone(&stdout_state), control_tx, ServingBehavior::KillChildOnDisconnect, "protocol", - ); + )); }), ) } diff --git a/src/servers/stderr.rs b/src/servers/stderr.rs index 379ac61..e63847d 100644 --- a/src/servers/stderr.rs +++ b/src/servers/stderr.rs @@ -82,15 +82,15 @@ pub fn stderr_server( // Spawn read monitoring thread to detect disconnection proactively. thread::spawn(move || { - let _ = monitor_stderr_client_connection( + drop(monitor_stderr_client_connection( connection_monitoring_stream, has_active_connection_monitor, - ); + )); }); // Spawn write thread to serve stderr output. thread::spawn(move || { - let _ = serve_output_on_stream( + drop(serve_output_on_stream( stream, stderr_state, control_tx, @@ -98,7 +98,7 @@ pub fn stderr_server( &has_active_connection_write, )), "stderr", - ); + )); // When the write thread finishes, also clear the connection flag // (idempotent if the read thread already did this). has_active_connection_clone.store(false, Ordering::Release); diff --git a/tests/integration_test.rs b/tests/integration_test.rs index a7a2c75..5292bfd 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -295,8 +295,8 @@ impl TestForwarder { impl Drop for TestForwarder { fn drop(&mut self) { // Clean up: kill the process if it’s still running. - let _ = self.process.kill(); - let _ = self.process.wait(); + drop(self.process.kill()); + drop(self.process.wait()); } } @@ -447,7 +447,7 @@ fn test_default_port_values() { let error_message = format!( "TEST SETUP FAILED: Default port {port} is not available. This is an environment issue, not a test failure." ); - let _ = TcpListener::bind(format!("127.0.0.1:{port}")).expect(&error_message); + drop(TcpListener::bind(format!("127.0.0.1:{port}")).expect(&error_message)); // Port is immediately released here. } @@ -495,8 +495,8 @@ fn test_default_port_values() { } // Clean up. - let _ = process.kill(); - let _ = process.wait(); + drop(process.kill()); + drop(process.wait()); } #[test] @@ -562,8 +562,8 @@ fn test_port_override_via_environment_variables() { ); // Clean up. - let _ = process.kill(); - let _ = process.wait(); + drop(process.kill()); + drop(process.wait()); } #[test] @@ -847,7 +847,7 @@ fn test_stderr_port_reconnect_continues_from_current_state() { assert!(output_str.contains("before_connection")); assert!(output_str.contains("during_first_connection")); // Disconnect now (at ~t=1.0s), before "trigger_disconnect" (at t=1.5s) - let _ = stream.shutdown(std::net::Shutdown::Both); + drop(stream.shutdown(std::net::Shutdown::Both)); drop(stream); assert!( !output_str.contains("trigger_disconnect"), diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index 6a81b3a..df6ec9d 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -193,9 +193,9 @@ impl LspClient { impl Drop for LspClient { fn drop(&mut self) { // Automatically send exit notification when the client is dropped. - let _ = self.send_message(&serde_json::json!({ + drop(self.send_message(&serde_json::json!({ "jsonrpc": "2.0", "method": "exit" - })); + }))); } } From dba8816a4ca855375b8011cc54eccf479c1a760c Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 09:39:04 +0200 Subject: [PATCH 03/54] fix: resolve unused subprocess dependency warning Make child, control, output, and servers modules private since they are only used internally by the app module. Suppress the unused-crate warning in main.rs with a reason attribute explaining the transitive dependency through the stdioxide library. --- src/lib.rs | 8 ++++---- src/main.rs | 9 +++++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 5be80d2..eb4f0d1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,6 @@ pub mod app; pub mod args; -pub mod child; -pub mod control; -pub mod output; -pub mod servers; +mod child; +mod control; +mod output; +mod servers; diff --git a/src/main.rs b/src/main.rs index 8b925df..f29af89 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,3 +1,12 @@ +//! stdioxide binary entry point. +//! +//! Launches the TCP forwarder that exposes a child process’s standard streams over the network. + +#![allow( + unused_crate_dependencies, + reason = "The binary depends on subprocess transitively through the stdioxide library" +)] + use clap::Parser; use stdioxide::{app, args::Args}; From e9260c185abc935603b4e064ee3eb15893ecace2 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 09:51:48 +0200 Subject: [PATCH 04/54] fix: switch from `pub` to `pub(crate)` where possible --- src/child.rs | 4 ++-- src/control.rs | 4 ++-- src/output.rs | 12 ++++++------ src/servers/health.rs | 2 +- src/servers/mod.rs | 6 +++--- src/servers/protocol.rs | 2 +- src/servers/stderr.rs | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/child.rs b/src/child.rs index f1170f9..4983c9f 100644 --- a/src/child.rs +++ b/src/child.rs @@ -1,6 +1,6 @@ use subprocess::{Exec, Job, Redirection}; -pub struct StartedChild { +pub(crate) struct StartedChild { pub job: Job, pub stdin: std::fs::File, pub stdout: std::fs::File, @@ -8,7 +8,7 @@ pub struct StartedChild { } impl StartedChild { - pub fn start(command: &str, args: &[String]) -> Result { + pub(crate) fn start(command: &str, args: &[String]) -> Result { let mut process = Exec::cmd(command); for arg in args { process = process.arg(arg); diff --git a/src/control.rs b/src/control.rs index fe2935d..9d8e6d9 100644 --- a/src/control.rs +++ b/src/control.rs @@ -3,11 +3,11 @@ use std::sync::mpsc; use subprocess::Job; #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ControlMessage { +pub(crate) enum ControlMessage { KillChild, } -pub fn run_child_coordinator( +pub(crate) fn run_child_coordinator( job: Job, control_rx: mpsc::Receiver, ) -> Result<(), anyhow::Error> { diff --git a/src/output.rs b/src/output.rs index 5aab68b..508d1fb 100644 --- a/src/output.rs +++ b/src/output.rs @@ -11,23 +11,23 @@ use std::{ use crate::control::ControlMessage; #[derive(Debug, Clone)] -pub enum ServingBehavior { +pub(crate) enum ServingBehavior { KillChildOnDisconnect, DoNotKillChildOnDisconnect(Arc), } -pub struct OutputState { +pub(crate) struct OutputState { pub buffer: Vec, pub eof: bool, } -pub struct NotifyableOutputState { +pub(crate) struct NotifyableOutputState { pub state: Mutex, pub condition_variable: Condvar, } impl NotifyableOutputState { - pub fn new() -> Self { + pub(crate) fn new() -> Self { Self::default() } } @@ -45,7 +45,7 @@ impl Default for NotifyableOutputState { } /// Pumps data from the given `source` (either `stdout` or `stderr` of the child process) into the shared `state`. -pub fn pump_output_to_state( +pub(crate) fn pump_output_to_state( mut source: impl Read, output_state: Arc, label: &'static str, @@ -73,7 +73,7 @@ pub fn pump_output_to_state( Ok(()) } -pub fn serve_output_on_stream( +pub(crate) fn serve_output_on_stream( mut stream: TcpStream, output_state: Arc, control_tx: mpsc::Sender, diff --git a/src/servers/health.rs b/src/servers/health.rs index 3fba5d3..dea820f 100644 --- a/src/servers/health.rs +++ b/src/servers/health.rs @@ -2,7 +2,7 @@ use std::net::TcpListener; /// Waits for clients to connect on the `health` port, and immediately drops any connections. The existence /// of a successful connection is used by the client as a health check for whether the process is alive. -pub fn health_server(listener: TcpListener) -> Result<(), anyhow::Error> { +pub(crate) fn health_server(listener: TcpListener) -> Result<(), anyhow::Error> { for stream in listener.incoming() { match stream { Ok(_stream) => { diff --git a/src/servers/mod.rs b/src/servers/mod.rs index 732f3e0..afc1be7 100644 --- a/src/servers/mod.rs +++ b/src/servers/mod.rs @@ -1,3 +1,3 @@ -pub mod health; -pub mod protocol; -pub mod stderr; +pub(crate) mod health; +pub(crate) mod protocol; +pub(crate) mod stderr; diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index c825cc7..ccf7e2f 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -45,7 +45,7 @@ fn forward_stream_data_to_child_process( /// client and the child process. This function spawns two threads: one for forwarding data from /// the client to the child process’s `stdin`, and another for forwarding data from the child /// process’s `stdout` to the client. -pub fn protocol_server( +pub(crate) fn protocol_server( listener: TcpListener, stdout_state: Arc, child_stdin: std::fs::File, diff --git a/src/servers/stderr.rs b/src/servers/stderr.rs index e63847d..e219917 100644 --- a/src/servers/stderr.rs +++ b/src/servers/stderr.rs @@ -47,7 +47,7 @@ fn monitor_stderr_client_connection( /// to the first client that connects. If that client disconnects, we wait for the next client to connect /// and serve the current `stderr` output to them instead, and so on. The function spawns two threads /// for each client connection: one for monitoring disconnection and one for writing output. -pub fn stderr_server( +pub(crate) fn stderr_server( listener: TcpListener, stderr_state: Arc, control_tx: mpsc::Sender, From 5ce40d4bb6f0f314f5d8cccc1566c310621a84b7 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 09:52:02 +0200 Subject: [PATCH 05/54] docs: add documentation to public items --- src/app.rs | 32 +++++++++++++++++++++++++++++ src/args.rs | 10 ++++++++++ src/child.rs | 16 ++++++++++++--- src/control.rs | 8 ++++++++ src/lib.rs | 32 +++++++++++++++++++++++++++++ src/output.rs | 20 +++++++++++++++++++ src/servers/health.rs | 2 ++ src/servers/protocol.rs | 6 ++++++ src/servers/stderr.rs | 2 ++ tests/integration_test.rs | 2 ++ tests/lsp_client.rs | 2 ++ tests/test_utils.rs | 42 ++++++++++++++++++++++++++++++++++++++- 12 files changed, 170 insertions(+), 4 deletions(-) diff --git a/src/app.rs b/src/app.rs index f16a17a..fa3af16 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,3 +1,8 @@ +//! Application entry point that orchestrates the TCP servers and child process lifecycle. +//! +//! Spawns all the server threads (protocol, stderr, health) and manages the child process +//! coordination loop. + use std::{ net::TcpListener, sync::{Arc, mpsc}, @@ -12,6 +17,33 @@ use crate::{ servers::{health::health_server, protocol::protocol_server, stderr::stderr_server}, }; +/// Run the stdioxide forwarder with the given configuration. +/// +/// This function: +/// 1. Starts the child process with the specified command and arguments +/// 2. Binds TCP listeners on the configured ports (protocol, stderr, health) +/// 3. Spawns threads to pump child process output into shared buffered state +/// 4. Spawns server threads to handle client connections on each port +/// 5. Coordinates child process lifecycle and graceful shutdown +/// +/// The function blocks until the child process exits or is terminated. +/// +/// # Errors +/// +/// Returns an error if: +/// - Port binding fails (port already in use or insufficient permissions) +/// - Child process fails to start +/// - Child process coordinator thread panics +/// +/// # Example +/// +/// ```no_run +/// use stdioxide::{app, args::Args}; +/// use clap::Parser; +/// +/// let args = Args::parse(); +/// app::run(args).expect("Failed to run stdioxide"); +/// ``` pub fn run(args: Args) -> Result<(), anyhow::Error> { let protocol_listener = TcpListener::bind(("0.0.0.0", args.protocol_port))?; let stderr_listener = TcpListener::bind(("0.0.0.0", args.stderr_port))?; diff --git a/src/args.rs b/src/args.rs index 44b1cea..8098054 100644 --- a/src/args.rs +++ b/src/args.rs @@ -1,5 +1,15 @@ use clap::Parser; +/// Command-line arguments for stdioxide. +/// +/// Configures the TCP ports and child process to launch. Ports can be specified +/// via command-line flags or environment variables (flags take precedence). +/// +/// # Example +/// +/// ```bash +/// stdioxide --protocol-port 7000 --stderr-port 7001 --health-port 7002 python script.py --arg1 --arg2 +/// ``` #[derive(Parser, Debug)] #[command(version, about, long_about = None)] pub struct Args { diff --git a/src/child.rs b/src/child.rs index 4983c9f..3e7a4e0 100644 --- a/src/child.rs +++ b/src/child.rs @@ -1,13 +1,23 @@ +//! Child process spawning and stream capture. + use subprocess::{Exec, Job, Redirection}; +/// A spawned child process with captured `stdin`, `stdout`, and `stderr` streams. pub(crate) struct StartedChild { + /// The subprocess `Job` handle for process lifecycle management. pub job: Job, + /// File handle for writing to the child’s `stdin`. pub stdin: std::fs::File, + /// File handle for reading from the child’s `stdout`. pub stdout: std::fs::File, + /// File handle for reading from the child’s `stderr`. pub stderr: std::fs::File, } impl StartedChild { + /// Spawns a child process with the given command and arguments. + /// + /// All three standard streams (`stdin`, `stdout`, `stderr`) are captured as pipes. pub(crate) fn start(command: &str, args: &[String]) -> Result { let mut process = Exec::cmd(command); for arg in args { @@ -21,15 +31,15 @@ impl StartedChild { let child_stdin = job .stdin .take() - .ok_or_else(|| anyhow::anyhow!("Failed to capture child stdin"))?; + .ok_or_else(|| anyhow::anyhow!("Failed to capture child `stdin`"))?; let child_stdout = job .stdout .take() - .ok_or_else(|| anyhow::anyhow!("Failed to capture child stdout"))?; + .ok_or_else(|| anyhow::anyhow!("Failed to capture child `stdout`"))?; let child_stderr = job .stderr .take() - .ok_or_else(|| anyhow::anyhow!("Failed to capture child stderr"))?; + .ok_or_else(|| anyhow::anyhow!("Failed to capture child `stderr`"))?; Ok(Self { job, stdin: child_stdin, diff --git a/src/control.rs b/src/control.rs index 9d8e6d9..38e377d 100644 --- a/src/control.rs +++ b/src/control.rs @@ -1,12 +1,20 @@ +//! Child process lifecycle coordination and control messages. + use std::sync::mpsc; use subprocess::Job; +/// Messages sent to the child process coordinator to control lifecycle. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum ControlMessage { + /// Terminate the child process immediately. KillChild, } +/// Monitors the child process and handles termination requests. +/// +/// Polls the child process for exit and listens for control messages to kill it. +/// Returns when the child process exits or is explicitly terminated. pub(crate) fn run_child_coordinator( job: Job, control_rx: mpsc::Receiver, diff --git a/src/lib.rs b/src/lib.rs index eb4f0d1..0adb93b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,37 @@ +//! A TCP forwarder that exposes a child process’s stdin, stdout, and stderr streams over the network. +//! +//! stdioxide launches an arbitrary child process and forwards its standard streams over two TCP ports, +//! allowing remote interaction with any command-line application. Output is buffered to prevent data +//! loss when no clients are connected. A third TCP port provides health check functionality for +//! container orchestrators. +//! +//! # Architecture +//! +//! - **Protocol Port**: Bidirectional communication for stdin/stdout (single client, kills child on disconnect) +//! - **Stderr Port**: Reconnectable stderr streaming with buffering (single client, child continues on disconnect) +//! - **Health Port**: Simple readiness check endpoint +//! +//! # Example +//! +//! ```no_run +//! use stdioxide::{app, args::Args}; +//! use clap::Parser; +//! +//! let args = Args::parse(); +//! app::run(args).expect("Failed to run stdioxide"); +//! ``` + +/// Application entry point and main event loop. +/// +/// Contains the `run()` function that orchestrates TCP listeners, child process management, +/// and output streaming threads. pub mod app; + +/// Command-line argument parsing and configuration. +/// +/// Defines the `Args` struct with port configurations and child process command. pub mod args; + mod child; mod control; mod output; diff --git a/src/output.rs b/src/output.rs index 508d1fb..08b56c5 100644 --- a/src/output.rs +++ b/src/output.rs @@ -1,3 +1,5 @@ +//! Output buffering and stream serving logic. + use std::{ io::{Read, Write}, net::TcpStream, @@ -10,23 +12,35 @@ use std::{ use crate::control::ControlMessage; +/// Defines how output serving should handle client disconnection. #[derive(Debug, Clone)] pub(crate) enum ServingBehavior { + /// Kill the child process when the client disconnects (protocol port behavior). KillChildOnDisconnect, + /// Keep child alive on disconnect, allow reconnection (`stderr` port behavior). + /// + /// The `Arc` tracks whether a connection is currently active. DoNotKillChildOnDisconnect(Arc), } +/// Buffered output state from a child process stream. pub(crate) struct OutputState { + /// Accumulated output bytes not yet sent to clients. pub buffer: Vec, + /// Whether EOF has been reached on the source stream. pub eof: bool, } +/// Thread-safe output state with condition variable for synchronization. pub(crate) struct NotifyableOutputState { + /// Protected output buffer and EOF flag. pub state: Mutex, + /// Condition variable to notify waiters when new output arrives. pub condition_variable: Condvar, } impl NotifyableOutputState { + /// Creates a new empty output state. pub(crate) fn new() -> Self { Self::default() } @@ -45,6 +59,8 @@ impl Default for NotifyableOutputState { } /// Pumps data from the given `source` (either `stdout` or `stderr` of the child process) into the shared `state`. +/// +/// Continuously reads from the source and appends to the buffer, notifying waiters on each read. pub(crate) fn pump_output_to_state( mut source: impl Read, output_state: Arc, @@ -73,6 +89,10 @@ pub(crate) fn pump_output_to_state( Ok(()) } +/// Serves output from the shared `state` to the given `stream`. +/// +/// Waits for output to become available, then writes it to the TCP stream. +/// Handles disconnection according to the specified `serving_behavior`. pub(crate) fn serve_output_on_stream( mut stream: TcpStream, output_state: Arc, diff --git a/src/servers/health.rs b/src/servers/health.rs index dea820f..0ae9200 100644 --- a/src/servers/health.rs +++ b/src/servers/health.rs @@ -1,3 +1,5 @@ +//! Health check TCP server that accepts and immediately drops connections. + use std::net::TcpListener; /// Waits for clients to connect on the `health` port, and immediately drops any connections. The existence diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index ccf7e2f..72b3437 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -1,3 +1,5 @@ +//! Protocol TCP server for bidirectional `stdin`/`stdout` forwarding. + use std::{ io::{Read, Write}, net::{TcpListener, TcpStream}, @@ -10,6 +12,10 @@ use crate::{ output::{NotifyableOutputState, ServingBehavior, serve_output_on_stream}, }; +/// Forwards data from a TCP client stream to the child process’s `stdin`. +/// +/// Reads from the client and writes to child `stdin` until the client disconnects +/// or an error occurs. Sends a kill signal on disconnection. fn forward_stream_data_to_child_process( mut stream: TcpStream, mut child_stdin: std::fs::File, diff --git a/src/servers/stderr.rs b/src/servers/stderr.rs index e219917..a27943a 100644 --- a/src/servers/stderr.rs +++ b/src/servers/stderr.rs @@ -1,3 +1,5 @@ +//! `stderr` TCP server with reconnection support and output buffering. + use std::{ io::Read, net::{TcpListener, TcpStream}, diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 5292bfd..246e64e 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -1,3 +1,5 @@ +//! Integration tests for stdioxide TCP forwarder functionality. + use std::{ collections::HashSet, io::{Read, Write}, diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index df6ec9d..84ed844 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -1,3 +1,5 @@ +//! Test utilities for LSP client communication. + use std::{ io::{Read, Write}, net::TcpStream, diff --git a/tests/test_utils.rs b/tests/test_utils.rs index 35eb397..00b916c 100644 --- a/tests/test_utils.rs +++ b/tests/test_utils.rs @@ -7,6 +7,17 @@ //! The goal is to make integration tests work on all platforms without #[cfg(not(windows))] //! guards scattered throughout the test code. +// Acknowledge available dev-dependencies not used in this test file. +use anyhow as _; +use clap as _; +use lsp_types as _; +use serde_json as _; +use stdioxide as _; +use subprocess as _; +use tracing as _; +use tracing_subscriber as _; + +/// Returns a command that sleeps for the specified number of seconds. #[cfg(windows)] pub fn sleep_cmd(seconds: u32) -> (&'static str, Vec) { // Use ping as a sleep alternative on Windows @@ -18,11 +29,13 @@ pub fn sleep_cmd(seconds: u32) -> (&'static str, Vec) { ) } +/// Returns a command that sleeps for the specified number of seconds. #[cfg(not(windows))] pub fn sleep_cmd(seconds: u32) -> (&'static str, Vec) { ("sleep", vec![seconds.to_string()]) } +/// Returns a command that echoes text to `stdout`, then sleeps. #[cfg(windows)] pub fn echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Vec) { let pings = seconds + 1; @@ -35,6 +48,7 @@ pub fn echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Vec (&'static str, Vec) { ( @@ -46,6 +60,7 @@ pub fn echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Vec (&'static str, Vec) { let pings = seconds + 1; @@ -58,6 +73,7 @@ pub fn stderr_echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Ve ) } +/// Returns a command that echoes text to `stderr`, then sleeps. #[cfg(not(windows))] pub fn stderr_echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Vec) { ( @@ -69,6 +85,7 @@ pub fn stderr_echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Ve ) } +/// Returns a command that echoes to `stderr`, sleeps, echoes again, then sleeps more. #[cfg(windows)] pub fn multi_echo_stderr_cmd( buffered: &str, @@ -91,6 +108,7 @@ pub fn multi_echo_stderr_cmd( ) } +/// Returns a command that echoes to `stderr`, sleeps, echoes again, then sleeps more. #[cfg(not(windows))] pub fn multi_echo_stderr_cmd( buffered: &str, @@ -110,6 +128,7 @@ pub fn multi_echo_stderr_cmd( ) } +/// Returns a command that echoes to `stdout`, sleeps, echoes again, then sleeps more. #[cfg(windows)] pub fn multi_echo_stdout_cmd( buffered: &str, @@ -132,6 +151,7 @@ pub fn multi_echo_stdout_cmd( ) } +/// Returns a command that echoes to `stdout`, sleeps, echoes again, then sleeps more. #[cfg(not(windows))] pub fn multi_echo_stdout_cmd( buffered: &str, @@ -151,6 +171,7 @@ pub fn multi_echo_stdout_cmd( ) } +/// Returns a command that reads from `stdin` and echoes to `stdout` (like `cat`). #[cfg(windows)] pub fn cat_cmd() -> (&'static str, Vec) { // Use Python for reliable line-by-line I/O on Windows. @@ -165,15 +186,17 @@ pub fn cat_cmd() -> (&'static str, Vec) { ) } +/// Returns a command that reads from `stdin` and echoes to `stdout` (like `cat`). #[cfg(not(windows))] pub fn cat_cmd() -> (&'static str, Vec) { ("cat", vec![]) } +/// Returns a command that continuously reads from `stdin` and writes "response" to `stdout`. #[cfg(windows)] pub fn loop_stdin_to_stdout_cmd() -> (&'static str, Vec) { // PowerShell script that reads line by line and echoes - // Use [Console]::In to read from stdin and [Console]::WriteLine() for immediate flushing + // Use [Console]::In to read from `stdin` and [Console]::WriteLine() for immediate flushing ( "powershell", vec![ @@ -185,6 +208,7 @@ pub fn loop_stdin_to_stdout_cmd() -> (&'static str, Vec) { ) } +/// Returns a command that continuously reads from `stdin` and writes "response" to `stdout`. #[cfg(not(windows))] pub fn loop_stdin_to_stdout_cmd() -> (&'static str, Vec) { ( @@ -196,6 +220,7 @@ pub fn loop_stdin_to_stdout_cmd() -> (&'static str, Vec) { ) } +/// Returns a command that continuously writes "error" to `stderr` in a loop. #[cfg(windows)] pub fn continuous_stderr_loop_cmd() -> (&'static str, Vec) { ( @@ -209,6 +234,7 @@ pub fn continuous_stderr_loop_cmd() -> (&'static str, Vec) { ) } +/// Returns a command that continuously writes "error" to `stderr` in a loop. #[cfg(not(windows))] pub fn continuous_stderr_loop_cmd() -> (&'static str, Vec) { ( @@ -220,6 +246,7 @@ pub fn continuous_stderr_loop_cmd() -> (&'static str, Vec) { ) } +/// Returns a command that generates a large block of output (repeated 'A' characters). #[cfg(windows)] pub fn generate_large_output_cmd(size: usize) -> (&'static str, Vec) { // Generate large output using PowerShell @@ -233,6 +260,7 @@ pub fn generate_large_output_cmd(size: usize) -> (&'static str, Vec) { ) } +/// Returns a command that generates a large block of output (repeated 'A' characters). #[cfg(not(windows))] pub fn generate_large_output_cmd(size: usize) -> (&'static str, Vec) { ( @@ -244,6 +272,7 @@ pub fn generate_large_output_cmd(size: usize) -> (&'static str, Vec) { ) } +/// Returns a command that outputs numbered lines to both `stdout` and `stderr` with delays. #[cfg(windows)] pub fn numbered_output_loop_cmd(count: u32, interval_ms: u32) -> (&'static str, Vec) { ( @@ -259,6 +288,7 @@ pub fn numbered_output_loop_cmd(count: u32, interval_ms: u32) -> (&'static str, ) } +/// Returns a command that outputs numbered lines to both `stdout` and `stderr` with delays. #[cfg(not(windows))] pub fn numbered_output_loop_cmd(count: u32, interval_ms: u32) -> (&'static str, Vec) { let interval_sec = interval_ms as f32 / 1000.0; @@ -273,6 +303,7 @@ pub fn numbered_output_loop_cmd(count: u32, interval_ms: u32) -> (&'static str, ) } +/// Returns a command that emits timed `stderr` output for testing reconnection scenarios. #[cfg(windows)] pub fn complex_stderr_reconnect_cmd() -> (&'static str, Vec) { ( @@ -291,6 +322,7 @@ pub fn complex_stderr_reconnect_cmd() -> (&'static str, Vec) { ) } +/// Returns a command that emits timed `stderr` output for testing reconnection scenarios. #[cfg(not(windows))] pub fn complex_stderr_reconnect_cmd() -> (&'static str, Vec) { ( @@ -309,6 +341,7 @@ pub fn complex_stderr_reconnect_cmd() -> (&'static str, Vec) { ) } +/// Returns a command that outputs to both `stdout` and `stderr`, then sleeps. #[cfg(windows)] pub fn combined_output_cmd( stdout_msg: &str, @@ -329,6 +362,7 @@ pub fn combined_output_cmd( ) } +/// Returns a command that outputs to both `stdout` and `stderr`, then sleeps. #[cfg(not(windows))] pub fn combined_output_cmd( stdout_msg: &str, @@ -347,6 +381,7 @@ pub fn combined_output_cmd( ) } +/// Returns a command that echoes all provided arguments to `stdout`. #[cfg(windows)] pub fn echo_args_cmd(args: &[&str]) -> (&'static str, Vec) { let mut cmd_args = vec!["/C".to_string()]; @@ -357,6 +392,7 @@ pub fn echo_args_cmd(args: &[&str]) -> (&'static str, Vec) { ("cmd", cmd_args) } +/// Returns a command that echoes all provided arguments to `stdout`. #[cfg(not(windows))] pub fn echo_args_cmd(args: &[&str]) -> (&'static str, Vec) { let mut script_args = vec![ @@ -368,6 +404,7 @@ pub fn echo_args_cmd(args: &[&str]) -> (&'static str, Vec) { ("bash", script_args) } +/// Returns a command that outputs a message and exits quickly after a brief delay. #[cfg(windows)] pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) { ( @@ -383,6 +420,7 @@ pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) ) } +/// Returns a command that outputs a message and exits quickly after a brief delay. #[cfg(not(windows))] pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) { let sleep_sec = sleep_ms as f32 / 1000.0; @@ -395,11 +433,13 @@ pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) ) } +/// Returns the platform-specific Python command name. #[cfg(windows)] pub fn python_cmd() -> &'static str { "python" } +/// Returns the platform-specific Python command name. #[cfg(not(windows))] pub fn python_cmd() -> &'static str { "python3" From aa3daf694b3be55dd3a00eac39c83194c8eb5863 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 10:21:13 +0200 Subject: [PATCH 06/54] fix: remove unused import --- src/main.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index f29af89..43fba67 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7,7 +7,6 @@ reason = "The binary depends on subprocess transitively through the stdioxide library" )] -use clap::Parser; use stdioxide::{app, args::Args}; fn main() -> Result<(), anyhow::Error> { From c0c44ba3ced490a8c52484e15c457d55909edd35 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 10:42:22 +0200 Subject: [PATCH 07/54] refactor: eliminate `unsafe` env var mutation in tests Co-authored-by: Copilot --- src/args.rs | 293 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 219 insertions(+), 74 deletions(-) diff --git a/src/args.rs b/src/args.rs index 8098054..7ecb4f0 100644 --- a/src/args.rs +++ b/src/args.rs @@ -1,5 +1,30 @@ +use std::{ + env::{self, args_os}, + ffi::OsString, +}; + use clap::Parser; +/// Trait for abstracting environment variable access. +/// +/// This allows dependency injection of environment providers in tests, +/// avoiding the need to mutate process-wide environment variables. +pub trait Env { + /// Get an environment variable value. + fn var(&self, key: &str) -> Option; +} + +/// Production environment provider that reads from the process environment. +#[derive(Debug, Default, Clone, Copy)] +#[non_exhaustive] +pub struct ProcessEnv; + +impl Env for ProcessEnv { + fn var(&self, key: &str) -> Option { + env::var(key).ok() + } +} + /// Command-line arguments for stdioxide. /// /// Configures the TCP ports and child process to launch. Ports can be specified @@ -14,15 +39,15 @@ use clap::Parser; #[command(version, about, long_about = None)] pub struct Args { /// The port to use for forwarding stdin and stdout. - #[arg(long, env = "STDIOXIDE_PROTOCOL_PORT", default_value_t = 7000)] + #[arg(long, default_value_t = 7000)] pub protocol_port: u16, /// The port to use for forwarding stderr. - #[arg(long, env = "STDIOXIDE_STDERR_PORT", default_value_t = 7001)] + #[arg(long, default_value_t = 7001)] pub stderr_port: u16, /// The port to use for health checks. - #[arg(long, env = "STDIOXIDE_HEALTH_PORT", default_value_t = 7002)] + #[arg(long, default_value_t = 7002)] pub health_port: u16, /// The command to run as a subprocess. @@ -34,52 +59,159 @@ pub struct Args { pub args: Vec, } +impl Args { + /// Parse arguments from the process environment and command line. + /// + /// This is the main entry point for production use. + #[expect( + clippy::same_name_method, + reason = "We want to mimic the API of clap’s `parse()` method while adding environment variable support" + )] + #[must_use] + pub fn parse() -> Self { + Self::parse_from_env(&ProcessEnv) + } + + /// Parse arguments using a custom environment provider. + /// + /// This method allows dependency injection of environment variables, + /// which is useful for testing without mutating global state. + pub fn parse_from_env(env: &E) -> Self { + Self::parse_from_env_and_args(env, args_os()) + } + + /// Parse arguments from a custom environment and argument iterator. + /// + /// This is the most flexible parsing method, used internally and in tests. + pub fn parse_from_env_and_args, A: IntoIterator>( + env: &E, + args: A, + ) -> Self { + // Build argument list with environment variable defaults injected. + let mut arg_vec: Vec = args.into_iter().map(Into::into).collect(); + + // Check if ports are provided via CLI; if not, inject from environment. + let has_protocol_port = arg_vec + .iter() + .any(|arg| arg.to_str() == Some("--protocol-port")); + let has_stderr_port = arg_vec + .iter() + .any(|arg| arg.to_str() == Some("--stderr-port")); + let has_health_port = arg_vec + .iter() + .any(|arg| arg.to_str() == Some("--health-port")); + + // Inject environment variables as CLI args if not already present. + let mut insertions = Vec::new(); + + if !has_protocol_port && let Some(port) = env.var("STDIOXIDE_PROTOCOL_PORT") { + insertions.push("--protocol-port".to_owned()); + insertions.push(port); + } + if !has_stderr_port && let Some(port) = env.var("STDIOXIDE_STDERR_PORT") { + insertions.push("--stderr-port".to_owned()); + insertions.push(port); + } + if !has_health_port && let Some(port) = env.var("STDIOXIDE_HEALTH_PORT") { + insertions.push("--health-port".to_owned()); + insertions.push(port); + } + // Insert environment-derived args after the program name but before other args. + if !insertions.is_empty() && !arg_vec.is_empty() { + let rest = arg_vec.split_off(1); + arg_vec.extend(insertions.into_iter().map(Into::into)); + arg_vec.extend(rest); + } + + Self::try_parse_from(arg_vec).unwrap_or_else(|error| error.exit()) + } +} + +#[cfg(test)] +impl Args { + /// Parse arguments from a custom environment and argument iterator for testing. + /// + /// Returns a Result instead of exiting on error, allowing tests to verify failure cases. + fn try_parse_from_env_and_args, A: IntoIterator>( + env: &E, + args: A, + ) -> Result { + // Build argument list with environment variable defaults injected + let mut arg_vec: Vec = args.into_iter().map(Into::into).collect(); + + // Check if ports are provided via CLI; if not, inject from environment + let has_protocol_port = arg_vec + .iter() + .any(|arg| arg.to_str() == Some("--protocol-port")); + let has_stderr_port = arg_vec + .iter() + .any(|arg| arg.to_str() == Some("--stderr-port")); + let has_health_port = arg_vec + .iter() + .any(|arg| arg.to_str() == Some("--health-port")); + + // Inject environment variables as CLI args if not already present + let mut insertions = Vec::new(); + + if !has_protocol_port && let Some(port) = env.var("STDIOXIDE_PROTOCOL_PORT") { + insertions.push("--protocol-port".to_owned()); + insertions.push(port); + } + if !has_stderr_port && let Some(port) = env.var("STDIOXIDE_STDERR_PORT") { + insertions.push("--stderr-port".to_owned()); + insertions.push(port); + } + if !has_health_port && let Some(port) = env.var("STDIOXIDE_HEALTH_PORT") { + insertions.push("--health-port".to_owned()); + insertions.push(port); + } + + // Insert environment-derived args after the program name but before other args. + if !insertions.is_empty() && !arg_vec.is_empty() { + let rest = arg_vec.split_off(1); + arg_vec.extend(insertions.into_iter().map(Into::into)); + arg_vec.extend(rest); + } + + Self::try_parse_from(arg_vec) + } +} + #[cfg(test)] mod tests { use super::*; - use std::sync::Mutex; - - // Mutex to serialize tests that modify environment variables. - // This prevents interference between parallel test execution. - static ENV_MUTEX: Mutex<()> = Mutex::new(()); - - /// RAII helper for temporarily setting environment variables in tests. - /// Automatically restores the original state when dropped. - struct EnvVar { - key: String, - original_value: Option, - } - - impl EnvVar { - /// Set an environment variable temporarily. - fn set(key: impl Into, value: impl AsRef) -> Self { - let key = key.into(); - let original_value = std::env::var(&key).ok(); - unsafe { - std::env::set_var(&key, value.as_ref()); - } + use std::collections::HashMap; + + /// Test environment provider backed by an in-memory hashmap. + /// + /// This allows testing environment variable behavior without mutating + /// the process-wide environment state. + #[derive(Debug, Default)] + struct TestEnv { + values: HashMap, + } + + impl TestEnv { + fn new(values: impl IntoIterator) -> Self { Self { - key, - original_value, + values: values + .into_iter() + .map(|(key, value)| (key.to_owned(), value.to_owned())) + .collect(), } } } - impl Drop for EnvVar { - fn drop(&mut self) { - unsafe { - match &self.original_value { - Some(value) => std::env::set_var(&self.key, value), - None => std::env::remove_var(&self.key), - } - } + impl Env for TestEnv { + fn var(&self, key: &str) -> Option { + self.values.get(key).cloned() } } #[test] fn test_default_port_values() { - let _lock = ENV_MUTEX.lock().unwrap(); - let args = Args::try_parse_from(["stdioxide", "echo"]).unwrap(); + let env = TestEnv::default(); + let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"]).unwrap(); assert_eq!(args.protocol_port, 7000); assert_eq!(args.stderr_port, 7001); assert_eq!(args.health_port, 7002); @@ -87,16 +219,20 @@ mod tests { #[test] fn test_custom_port_values_via_args() { - let args = Args::try_parse_from([ - "stdioxide", - "--protocol-port", - "8000", - "--stderr-port", - "8001", - "--health-port", - "8002", - "echo", - ]) + let env = TestEnv::default(); + let args = Args::try_parse_from_env_and_args( + &env, + [ + "stdioxide", + "--protocol-port", + "8000", + "--stderr-port", + "8001", + "--health-port", + "8002", + "echo", + ], + ) .unwrap(); assert_eq!(args.protocol_port, 8000); assert_eq!(args.stderr_port, 8001); @@ -105,72 +241,81 @@ mod tests { #[test] fn test_command_and_args() { - let args = Args::try_parse_from(["stdioxide", "python", "-m", "http.server"]).unwrap(); + let env = TestEnv::default(); + let args = + Args::try_parse_from_env_and_args(&env, ["stdioxide", "python", "-m", "http.server"]) + .unwrap(); assert_eq!(args.command, "python"); assert_eq!(args.args, vec!["-m", "http.server"]); } #[test] fn test_args_with_hyphen_values() { - let args = Args::try_parse_from(["stdioxide", "myapp", "--flag", "-value"]).unwrap(); + let env = TestEnv::default(); + let args = + Args::try_parse_from_env_and_args(&env, ["stdioxide", "myapp", "--flag", "-value"]) + .unwrap(); assert_eq!(args.command, "myapp"); assert_eq!(args.args, vec!["--flag", "-value"]); } #[test] fn test_empty_args() { - let args = Args::try_parse_from(["stdioxide", "echo"]).unwrap(); + let env = TestEnv::default(); + let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"]).unwrap(); assert_eq!(args.command, "echo"); assert!(args.args.is_empty()); } #[test] fn test_missing_command_fails() { - let result = Args::try_parse_from(["stdioxide"]); + let env = TestEnv::default(); + let result = Args::try_parse_from_env_and_args(&env, ["stdioxide"]); assert!(result.is_err()); } #[test] fn test_env_var_protocol_port() { - let _lock = ENV_MUTEX.lock().unwrap(); - let _env = EnvVar::set("STDIOXIDE_PROTOCOL_PORT", "9000"); - let args = Args::try_parse_from(["stdioxide", "echo"]).unwrap(); + let env = TestEnv::new([("STDIOXIDE_PROTOCOL_PORT", "9000")]); + let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"]).unwrap(); assert_eq!(args.protocol_port, 9000); } #[test] fn test_env_var_stderr_port() { - let _lock = ENV_MUTEX.lock().unwrap(); - let _env = EnvVar::set("STDIOXIDE_STDERR_PORT", "9001"); - let args = Args::try_parse_from(["stdioxide", "echo"]).unwrap(); + let env = TestEnv::new([("STDIOXIDE_STDERR_PORT", "9001")]); + let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"]).unwrap(); assert_eq!(args.stderr_port, 9001); } #[test] fn test_env_var_health_port() { - let _lock = ENV_MUTEX.lock().unwrap(); - let _env = EnvVar::set("STDIOXIDE_HEALTH_PORT", "9002"); - let args = Args::try_parse_from(["stdioxide", "echo"]).unwrap(); + let env = TestEnv::new([("STDIOXIDE_HEALTH_PORT", "9002")]); + let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"]).unwrap(); assert_eq!(args.health_port, 9002); } #[test] fn test_cli_args_override_env_vars() { - let _lock = ENV_MUTEX.lock().unwrap(); - let _env1 = EnvVar::set("STDIOXIDE_PROTOCOL_PORT", "9000"); - let _env2 = EnvVar::set("STDIOXIDE_STDERR_PORT", "9001"); - let _env3 = EnvVar::set("STDIOXIDE_HEALTH_PORT", "9002"); - - let args = Args::try_parse_from([ - "stdioxide", - "--protocol-port", - "8000", - "--stderr-port", - "8001", - "--health-port", - "8002", - "echo", - ]) + let env = TestEnv::new([ + ("STDIOXIDE_PROTOCOL_PORT", "9000"), + ("STDIOXIDE_STDERR_PORT", "9001"), + ("STDIOXIDE_HEALTH_PORT", "9002"), + ]); + + let args = Args::try_parse_from_env_and_args( + &env, + [ + "stdioxide", + "--protocol-port", + "8000", + "--stderr-port", + "8001", + "--health-port", + "8002", + "echo", + ], + ) .unwrap(); assert_eq!(args.protocol_port, 8000); From f1b0614137408cbd8611c7d0741f2430beb8947f Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 11:08:41 +0200 Subject: [PATCH 08/54] fix: acknowledge unused dev-dependencies in test files Add explicit 'use crate_name as _;' imports in each test file to acknowledge dev-dependencies that are available but not used by that specific test target. This resolves unused-crate-dependencies warnings. --- src/lib.rs | 9 +++++---- tests/integration_test.rs | 7 +++++++ tests/lsp_client.rs | 7 +++++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 0adb93b..43c1c05 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -21,10 +21,11 @@ //! app::run(args).expect("Failed to run stdioxide"); //! ``` -/// Application entry point and main event loop. -/// -/// Contains the `run()` function that orchestrates TCP listeners, child process management, -/// and output streaming threads. +#![allow( + unused_crate_dependencies, + reason = "dev-dependencies available to lib tests" +)] + pub mod app; /// Command-line argument parsing and configuration. diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 246e64e..a9612f5 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -10,6 +10,13 @@ use std::{ time::Duration, }; +// Acknowledge available dev-dependencies not used in this test file. +use anyhow as _; +use clap as _; +use lsp_types as _; +use stdioxide as _; +use subprocess as _; + use crate::lsp_client::LspClient; mod test_utils; diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index 84ed844..b393495 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -7,6 +7,13 @@ use std::{ time::Duration, }; +// Acknowledge available dev-dependencies not used in this test file. +use anyhow as _; +use clap as _; +use lsp_types as _; +use stdioxide as _; +use subprocess as _; + /// RAII wrapper for LSP communication over a TCP stream. /// Automatically sends the exit notification when dropped. pub struct LspClient { From 8060e07442887bfe72d2c3f9e3b3b69e5e11aebc Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 11:15:23 +0200 Subject: [PATCH 09/54] feat: replace eprintln! with structured tracing Co-authored-by: Copilot --- Cargo.lock | 161 ++++++++++++++++++++++++++++++++++++++ Cargo.toml | 2 + src/control.rs | 9 ++- src/main.rs | 7 ++ src/output.rs | 12 +-- src/servers/health.rs | 4 +- src/servers/protocol.rs | 6 +- src/servers/stderr.rs | 14 ++-- tests/integration_test.rs | 2 + tests/lsp_client.rs | 2 + 10 files changed, 200 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c2cd1a8..42645a9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,15 @@ # It is not intended for manual editing. version = 4 +[[package]] +name = "aho-corasick" +version = "1.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ddd31a130427c27518df266943a5308ed92d4b226cc639f5a8f1002816174301" +dependencies = [ + "memchr", +] + [[package]] name = "anstream" version = "1.0.0" @@ -64,6 +73,12 @@ version = "1.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" +[[package]] +name = "cfg-if" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" + [[package]] name = "clap" version = "4.6.1" @@ -251,6 +266,12 @@ version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" +[[package]] +name = "lazy_static" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" + [[package]] name = "libc" version = "0.2.185" @@ -263,6 +284,12 @@ version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "92daf443525c4cce67b150400bc2316076100ce0b3686209eb8cf3c31612e6f0" +[[package]] +name = "log" +version = "0.4.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e5032e24019045c762d3c0f28f5b6b8bbf38563a65908389bf7978758920897" + [[package]] name = "lsp-types" version = "0.95.1" @@ -276,12 +303,36 @@ dependencies = [ "url", ] +[[package]] +name = "matchers" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d1525a2a28c7f4fa0fc98bb91ae755d1e2d1505079e05539e35bc876b5d65ae9" +dependencies = [ + "regex-automata", +] + [[package]] name = "memchr" version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" +[[package]] +name = "nu-ansi-term" +version = "0.50.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" +dependencies = [ + "windows-sys", +] + +[[package]] +name = "once_cell" +version = "1.21.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" + [[package]] name = "once_cell_polyfill" version = "1.70.2" @@ -294,6 +345,12 @@ version = "2.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" +[[package]] +name = "pin-project-lite" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" + [[package]] name = "potential_utf" version = "0.1.5" @@ -321,6 +378,23 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "regex-automata" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e1dd4122fc1595e8162618945476892eefca7b88c52820e74af6262213cae8f" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + +[[package]] +name = "regex-syntax" +version = "0.8.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a" + [[package]] name = "serde" version = "1.0.228" @@ -375,6 +449,15 @@ dependencies = [ "syn", ] +[[package]] +name = "sharded-slab" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f40ca3c46823713e0d4209592e8d6e826aa57e928f09752619fc696c499637f6" +dependencies = [ + "lazy_static", +] + [[package]] name = "smallvec" version = "1.15.1" @@ -396,6 +479,8 @@ dependencies = [ "lsp-types", "serde_json", "subprocess", + "tracing", + "tracing-subscriber", ] [[package]] @@ -436,6 +521,15 @@ dependencies = [ "syn", ] +[[package]] +name = "thread_local" +version = "1.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f60246a4944f24f6e018aa17cdeffb7818b76356965d03b07d6a9886e8962185" +dependencies = [ + "cfg-if", +] + [[package]] name = "tinystr" version = "0.8.3" @@ -446,6 +540,67 @@ dependencies = [ "zerovec", ] +[[package]] +name = "tracing" +version = "0.1.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63e71662fa4b2a2c3a26f570f037eb95bb1f85397f3cd8076caed2f026a6d100" +dependencies = [ + "pin-project-lite", + "tracing-attributes", + "tracing-core", +] + +[[package]] +name = "tracing-attributes" +version = "0.1.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "tracing-core" +version = "0.1.36" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "db97caf9d906fbde555dd62fa95ddba9eecfd14cb388e4f491a66d74cd5fb79a" +dependencies = [ + "once_cell", + "valuable", +] + +[[package]] +name = "tracing-log" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee855f1f400bd0e5c02d150ae5de3840039a3f54b025156404e34c23c03f47c3" +dependencies = [ + "log", + "once_cell", + "tracing-core", +] + +[[package]] +name = "tracing-subscriber" +version = "0.3.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb7f578e5945fb242538965c2d0b04418d38ec25c79d160cd279bf0731c8d319" +dependencies = [ + "matchers", + "nu-ansi-term", + "once_cell", + "regex-automata", + "sharded-slab", + "smallvec", + "thread_local", + "tracing", + "tracing-core", + "tracing-log", +] + [[package]] name = "unicode-ident" version = "1.0.24" @@ -477,6 +632,12 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" +[[package]] +name = "valuable" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" + [[package]] name = "winapi" version = "0.3.9" diff --git a/Cargo.toml b/Cargo.toml index 8b079ee..fe40e58 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,8 @@ categories = ["command-line-utilities", "network-programming", "development-tool anyhow = "1.0.102" clap = { version = "4.6.1", features = ["derive", "env"] } subprocess = "1.0.3" +tracing = "0.1" +tracing-subscriber = { version = "0.3", features = ["env-filter"] } [dev-dependencies] lsp-types = "0.95" diff --git a/src/control.rs b/src/control.rs index 38e377d..ca81bdf 100644 --- a/src/control.rs +++ b/src/control.rs @@ -3,6 +3,7 @@ use std::sync::mpsc; use subprocess::Job; +use tracing::info; /// Messages sent to the child process coordinator to control lifecycle. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -21,7 +22,7 @@ pub(crate) fn run_child_coordinator( ) -> Result<(), anyhow::Error> { loop { if let Some(status) = job.poll() { - eprintln!("Child process exited with status: {status}"); + info!("Child process exited with status: {status}"); return Ok(()); } @@ -29,7 +30,7 @@ pub(crate) fn run_child_coordinator( Ok(ControlMessage::KillChild) => { drop(job.kill()); let status = job.wait()?; - eprintln!("Child process killed; exit status: {status}"); + info!("Child process killed; exit status: {status}"); return Ok(()); } Err(mpsc::RecvTimeoutError::Timeout) => { @@ -37,10 +38,10 @@ pub(crate) fn run_child_coordinator( } Err(mpsc::RecvTimeoutError::Disconnected) => { // All senders are gone; we terminate the child process and exit. - eprintln!("Control channel disconnected; terminating child process"); + info!("Control channel disconnected; terminating child process"); drop(job.kill()); let status = job.wait()?; - eprintln!("Child process killed; exit status: {status}"); + info!("Child process killed; exit status: {status}"); return Ok(()); } } diff --git a/src/main.rs b/src/main.rs index 43fba67..e020e51 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7,9 +7,16 @@ reason = "The binary depends on subprocess transitively through the stdioxide library" )] +use std::io::stderr; + use stdioxide::{app, args::Args}; fn main() -> Result<(), anyhow::Error> { + tracing_subscriber::fmt() + .with_writer(stderr) + .with_target(false) + .init(); + let args = Args::parse(); app::run(args) } diff --git a/src/output.rs b/src/output.rs index 08b56c5..649cb45 100644 --- a/src/output.rs +++ b/src/output.rs @@ -10,6 +10,8 @@ use std::{ }, }; +use tracing::{debug, info}; + use crate::control::ControlMessage; /// Defines how output serving should handle client disconnection. @@ -75,7 +77,7 @@ pub(crate) fn pump_output_to_state( .expect("Failed to lock output state"); if num_bytes_read == 0 { - eprintln!("[{label}] EOF reached"); + debug!("[{label}] EOF reached"); guard.eof = true; output_state.condition_variable.notify_all(); break; @@ -116,9 +118,7 @@ pub(crate) fn serve_output_on_stream( } if guard.buffer.is_empty() && guard.eof { - eprintln!( - "[{label}] EOF reached and no buffered output; closing client connection" - ); + debug!("[{label}] EOF reached and no buffered output; closing client connection"); return Ok(()); } @@ -154,7 +154,7 @@ pub(crate) fn serve_output_on_stream( if let ServingBehavior::DoNotKillChildOnDisconnect(ref active) = serving_behavior && !active.load(Ordering::Acquire) { - eprintln!( + info!( "[{label}] Connection no longer active (detected by monitoring thread); exiting without draining buffer to prevent data loss" ); return Ok(()); @@ -181,7 +181,7 @@ pub(crate) fn serve_output_on_stream( } if guard.eof && guard.buffer.is_empty() { - eprintln!("[{label}] EOF reached; closing client connection"); + debug!("[{label}] EOF reached; closing client connection"); return Ok(()); } } diff --git a/src/servers/health.rs b/src/servers/health.rs index 0ae9200..9a56911 100644 --- a/src/servers/health.rs +++ b/src/servers/health.rs @@ -2,6 +2,8 @@ use std::net::TcpListener; +use tracing::warn; + /// Waits for clients to connect on the `health` port, and immediately drops any connections. The existence /// of a successful connection is used by the client as a health check for whether the process is alive. pub(crate) fn health_server(listener: TcpListener) -> Result<(), anyhow::Error> { @@ -11,7 +13,7 @@ pub(crate) fn health_server(listener: TcpListener) -> Result<(), anyhow::Error> // Immediately drop it; successful connect is enough. } Err(e) => { - eprintln!("[health] accept failed: {e}"); + warn!("[health] accept failed: {e}"); } } } diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index 72b3437..51f3c2e 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -7,6 +7,8 @@ use std::{ thread, }; +use tracing::info; + use crate::{ control::ControlMessage, output::{NotifyableOutputState, ServingBehavior, serve_output_on_stream}, @@ -25,7 +27,7 @@ fn forward_stream_data_to_child_process( loop { let num_bytes_read = match stream.read(&mut read_buffer) { Ok(0) => { - eprintln!("[protocol] client disconnected; terminating child process"); + info!("[protocol] client disconnected; terminating child process"); let _ = control_tx.send(ControlMessage::KillChild); return Ok(()); } @@ -61,7 +63,7 @@ pub(crate) fn protocol_server( // When the client disconnects, we terminate the child process and exit the server. let (stdin_thread, stdout_thread) = match listener.accept() { Ok((stream, address)) => { - eprintln!("[protocol] client connected from {address}"); + info!("[protocol] client connected from {address}"); let cloned_stream = stream.try_clone()?; let cloned_control_tx = control_tx.clone(); ( diff --git a/src/servers/stderr.rs b/src/servers/stderr.rs index a27943a..01e0b35 100644 --- a/src/servers/stderr.rs +++ b/src/servers/stderr.rs @@ -11,6 +11,8 @@ use std::{ thread, }; +use tracing::{debug, info, warn}; + use crate::{ control::ControlMessage, output::{NotifyableOutputState, ServingBehavior, serve_output_on_stream}, @@ -28,7 +30,7 @@ fn monitor_stderr_client_connection( match stream.read(&mut read_buffer) { Ok(0) => { // EOF; client disconnected gracefully. - eprintln!("[stderr] client disconnect detected"); + debug!("[stderr] client disconnect detected"); has_active_connection.store(false, Ordering::Release); return Ok(()); } @@ -37,7 +39,7 @@ fn monitor_stderr_client_connection( } Err(e) => { // Error reading; treat as disconnection. - eprintln!("[stderr] read error (client likely disconnected): {e}"); + debug!("[stderr] read error (client likely disconnected): {e}"); has_active_connection.store(false, Ordering::Release); return Err(anyhow::anyhow!("Failed to read from stderr client: {e}")); } @@ -65,12 +67,12 @@ pub(crate) fn stderr_server( .is_ok() { // Atomic value has been successfully changed from `false` to `true`. - eprintln!("[stderr] client connected from {}", stream.peer_addr()?); + info!("[stderr] client connected from {}", stream.peer_addr()?); let connection_monitoring_stream = match stream.try_clone() { Ok(s) => s, Err(e) => { - eprintln!("[stderr] failed to clone stream: {e}"); + warn!("[stderr] failed to clone stream: {e}"); has_active_connection.store(false, Ordering::Release); continue; } @@ -107,14 +109,14 @@ pub(crate) fn stderr_server( }); } else { // Atomic value was already `true`, so there is already an active connection. - eprintln!( + info!( "[stderr] client connected from {}, but another client is already connected; rejecting connection", stream.peer_addr()? ); } } Err(e) => { - eprintln!("[stderr] accept failed: {e}"); + warn!("[stderr] accept failed: {e}"); } } } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index a9612f5..98f523d 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -16,6 +16,8 @@ use clap as _; use lsp_types as _; use stdioxide as _; use subprocess as _; +use tracing as _; +use tracing_subscriber as _; use crate::lsp_client::LspClient; diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index b393495..1790ab9 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -13,6 +13,8 @@ use clap as _; use lsp_types as _; use stdioxide as _; use subprocess as _; +use tracing as _; +use tracing_subscriber as _; /// RAII wrapper for LSP communication over a TCP stream. /// Automatically sends the exit notification when dropped. From 8b213839074dca8be94c45f1cd306650a3ee0cc3 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 11:22:39 +0200 Subject: [PATCH 10/54] fix: remove redundant pub(crate) in private modules Change pub(crate) to pub for items in private modules (child, control, output, servers). Since the modules themselves are private, pub(crate) is redundant - items are already scoped to the crate. --- src/child.rs | 4 ++-- src/servers/health.rs | 2 +- src/servers/mod.rs | 6 +++--- src/servers/protocol.rs | 2 +- src/servers/stderr.rs | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/child.rs b/src/child.rs index 3e7a4e0..542e554 100644 --- a/src/child.rs +++ b/src/child.rs @@ -3,7 +3,7 @@ use subprocess::{Exec, Job, Redirection}; /// A spawned child process with captured `stdin`, `stdout`, and `stderr` streams. -pub(crate) struct StartedChild { +pub struct StartedChild { /// The subprocess `Job` handle for process lifecycle management. pub job: Job, /// File handle for writing to the child’s `stdin`. @@ -18,7 +18,7 @@ impl StartedChild { /// Spawns a child process with the given command and arguments. /// /// All three standard streams (`stdin`, `stdout`, `stderr`) are captured as pipes. - pub(crate) fn start(command: &str, args: &[String]) -> Result { + pub fn start(command: &str, args: &[String]) -> Result { let mut process = Exec::cmd(command); for arg in args { process = process.arg(arg); diff --git a/src/servers/health.rs b/src/servers/health.rs index 9a56911..9f671b0 100644 --- a/src/servers/health.rs +++ b/src/servers/health.rs @@ -6,7 +6,7 @@ use tracing::warn; /// Waits for clients to connect on the `health` port, and immediately drops any connections. The existence /// of a successful connection is used by the client as a health check for whether the process is alive. -pub(crate) fn health_server(listener: TcpListener) -> Result<(), anyhow::Error> { +pub fn health_server(listener: TcpListener) -> Result<(), anyhow::Error> { for stream in listener.incoming() { match stream { Ok(_stream) => { diff --git a/src/servers/mod.rs b/src/servers/mod.rs index afc1be7..732f3e0 100644 --- a/src/servers/mod.rs +++ b/src/servers/mod.rs @@ -1,3 +1,3 @@ -pub(crate) mod health; -pub(crate) mod protocol; -pub(crate) mod stderr; +pub mod health; +pub mod protocol; +pub mod stderr; diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index 51f3c2e..8fa05ff 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -53,7 +53,7 @@ fn forward_stream_data_to_child_process( /// client and the child process. This function spawns two threads: one for forwarding data from /// the client to the child process’s `stdin`, and another for forwarding data from the child /// process’s `stdout` to the client. -pub(crate) fn protocol_server( +pub fn protocol_server( listener: TcpListener, stdout_state: Arc, child_stdin: std::fs::File, diff --git a/src/servers/stderr.rs b/src/servers/stderr.rs index 01e0b35..1fe16d4 100644 --- a/src/servers/stderr.rs +++ b/src/servers/stderr.rs @@ -51,7 +51,7 @@ fn monitor_stderr_client_connection( /// to the first client that connects. If that client disconnects, we wait for the next client to connect /// and serve the current `stderr` output to them instead, and so on. The function spawns two threads /// for each client connection: one for monitoring disconnection and one for writing output. -pub(crate) fn stderr_server( +pub fn stderr_server( listener: TcpListener, stderr_state: Arc, control_tx: mpsc::Sender, From 9e85f4cf103bb1556a0b475307a08d36cb5614dd Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 11:25:51 +0200 Subject: [PATCH 11/54] refaactor: pass `Args` by reference in `app::run()` Co-authored-by: Copilot --- src/app.rs | 4 ++-- src/lib.rs | 2 +- src/main.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app.rs b/src/app.rs index fa3af16..cdbd18f 100644 --- a/src/app.rs +++ b/src/app.rs @@ -42,9 +42,9 @@ use crate::{ /// use clap::Parser; /// /// let args = Args::parse(); -/// app::run(args).expect("Failed to run stdioxide"); +/// app::run(&args).expect("Failed to run stdioxide"); /// ``` -pub fn run(args: Args) -> Result<(), anyhow::Error> { +pub fn run(args: &Args) -> Result<(), anyhow::Error> { let protocol_listener = TcpListener::bind(("0.0.0.0", args.protocol_port))?; let stderr_listener = TcpListener::bind(("0.0.0.0", args.stderr_port))?; let health_listener = TcpListener::bind(("0.0.0.0", args.health_port))?; diff --git a/src/lib.rs b/src/lib.rs index 43c1c05..60f5f81 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,7 +18,7 @@ //! use clap::Parser; //! //! let args = Args::parse(); -//! app::run(args).expect("Failed to run stdioxide"); +//! app::run(&args).expect("Failed to run stdioxide"); //! ``` #![allow( diff --git a/src/main.rs b/src/main.rs index e020e51..87b950b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,5 +18,5 @@ fn main() -> Result<(), anyhow::Error> { .init(); let args = Args::parse(); - app::run(args) + app::run(&args) } From 59ba6f4768c8908f8c51029e9b304239f5ae5412 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 12:41:14 +0200 Subject: [PATCH 12/54] fix: remov `expect()`s and `panic!()`s Co-authored-by: Copilot --- src/app.rs | 2 +- src/args.rs | 51 ++--- src/output.rs | 46 +++-- src/servers/protocol.rs | 8 +- tests/integration_test.rs | 425 ++++++++++++++++++++++---------------- tests/lsp_client.rs | 72 ++++--- 6 files changed, 347 insertions(+), 257 deletions(-) diff --git a/src/app.rs b/src/app.rs index cdbd18f..392757b 100644 --- a/src/app.rs +++ b/src/app.rs @@ -106,7 +106,7 @@ pub fn run(args: &Args) -> Result<(), anyhow::Error> { coordinator_thread .join() - .expect("Failed to join coordinator thread")?; + .map_err(|error| anyhow::anyhow!("Child coordinator thread panicked: {error:?}"))??; Ok(()) } diff --git a/src/args.rs b/src/args.rs index 7ecb4f0..75f9f66 100644 --- a/src/args.rs +++ b/src/args.rs @@ -209,16 +209,17 @@ mod tests { } #[test] - fn test_default_port_values() { + fn test_default_port_values() -> Result<(), anyhow::Error> { let env = TestEnv::default(); - let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"]).unwrap(); + let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"])?; assert_eq!(args.protocol_port, 7000); assert_eq!(args.stderr_port, 7001); assert_eq!(args.health_port, 7002); + Ok(()) } #[test] - fn test_custom_port_values_via_args() { + fn test_custom_port_values_via_args() -> Result<(), anyhow::Error> { let env = TestEnv::default(); let args = Args::try_parse_from_env_and_args( &env, @@ -232,71 +233,75 @@ mod tests { "8002", "echo", ], - ) - .unwrap(); + )?; assert_eq!(args.protocol_port, 8000); assert_eq!(args.stderr_port, 8001); assert_eq!(args.health_port, 8002); + Ok(()) } #[test] - fn test_command_and_args() { + fn test_command_and_args() -> Result<(), anyhow::Error> { let env = TestEnv::default(); let args = - Args::try_parse_from_env_and_args(&env, ["stdioxide", "python", "-m", "http.server"]) - .unwrap(); + Args::try_parse_from_env_and_args(&env, ["stdioxide", "python", "-m", "http.server"])?; assert_eq!(args.command, "python"); assert_eq!(args.args, vec!["-m", "http.server"]); + Ok(()) } #[test] - fn test_args_with_hyphen_values() { + fn test_args_with_hyphen_values() -> Result<(), anyhow::Error> { let env = TestEnv::default(); let args = - Args::try_parse_from_env_and_args(&env, ["stdioxide", "myapp", "--flag", "-value"]) - .unwrap(); + Args::try_parse_from_env_and_args(&env, ["stdioxide", "myapp", "--flag", "-value"])?; assert_eq!(args.command, "myapp"); assert_eq!(args.args, vec!["--flag", "-value"]); + Ok(()) } #[test] - fn test_empty_args() { + fn test_empty_args() -> Result<(), anyhow::Error> { let env = TestEnv::default(); - let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"]).unwrap(); + let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"])?; assert_eq!(args.command, "echo"); assert!(args.args.is_empty()); + Ok(()) } #[test] fn test_missing_command_fails() { let env = TestEnv::default(); let result = Args::try_parse_from_env_and_args(&env, ["stdioxide"]); - assert!(result.is_err()); + assert!(result.is_err(), "Expected parsing to fail when command is missing"); } #[test] - fn test_env_var_protocol_port() { + fn test_env_var_protocol_port() -> Result<(), anyhow::Error> { let env = TestEnv::new([("STDIOXIDE_PROTOCOL_PORT", "9000")]); - let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"]).unwrap(); + let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"])?; assert_eq!(args.protocol_port, 9000); + Ok(()) } #[test] - fn test_env_var_stderr_port() { + fn test_env_var_stderr_port() -> Result<(), anyhow::Error> { let env = TestEnv::new([("STDIOXIDE_STDERR_PORT", "9001")]); - let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"]).unwrap(); + let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"])?; assert_eq!(args.stderr_port, 9001); + Ok(()) } #[test] - fn test_env_var_health_port() { + fn test_env_var_health_port() -> Result<(), anyhow::Error> { let env = TestEnv::new([("STDIOXIDE_HEALTH_PORT", "9002")]); - let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"]).unwrap(); + let args = Args::try_parse_from_env_and_args(&env, ["stdioxide", "echo"])?; assert_eq!(args.health_port, 9002); + Ok(()) } #[test] - fn test_cli_args_override_env_vars() { + fn test_cli_args_override_env_vars() -> Result<(), anyhow::Error> { let env = TestEnv::new([ ("STDIOXIDE_PROTOCOL_PORT", "9000"), ("STDIOXIDE_STDERR_PORT", "9001"), @@ -315,11 +320,11 @@ mod tests { "8002", "echo", ], - ) - .unwrap(); + )?; assert_eq!(args.protocol_port, 8000); assert_eq!(args.stderr_port, 8001); assert_eq!(args.health_port, 8002); + Ok(()) } } diff --git a/src/output.rs b/src/output.rs index 649cb45..bfb8c73 100644 --- a/src/output.rs +++ b/src/output.rs @@ -71,20 +71,21 @@ pub(crate) fn pump_output_to_state( loop { let mut buffer = [0u8; 8192]; let num_bytes_read = source.read(&mut buffer)?; - let mut guard = output_state - .state - .lock() - .expect("Failed to lock output state"); + { + let mut guard = output_state.state.lock().map_err(|error| { + anyhow::anyhow!("Failed to lock output state for {label}: {error}") + })?; + + if num_bytes_read == 0 { + debug!("[{label}] EOF reached"); + guard.eof = true; + output_state.condition_variable.notify_all(); + break; + } - if num_bytes_read == 0 { - debug!("[{label}] EOF reached"); - guard.eof = true; - output_state.condition_variable.notify_all(); - break; + let chunk = &buffer[..num_bytes_read]; + guard.buffer.extend_from_slice(chunk); } - - let chunk = &buffer[..num_bytes_read]; - guard.buffer.extend_from_slice(chunk); output_state.condition_variable.notify_all(); } @@ -104,17 +105,18 @@ pub(crate) fn serve_output_on_stream( ) -> Result<(), anyhow::Error> { loop { let buffered_data = { - let mut guard = output_state - .state - .lock() - .expect("Failed to lock stdout state"); + let mut guard = output_state.state.lock().map_err(|error| { + anyhow::anyhow!("Failed to lock stdout state for {label}: {error}") + })?; while guard.buffer.is_empty() && !guard.eof { // Wait until there’s either new output to send or we’ve reached EOF. guard = output_state .condition_variable .wait(guard) - .expect("Failed to wait on condition variable"); + .map_err(|error| { + anyhow::anyhow!("Failed to wait on condition variable for {label}: {error}") + })?; } if guard.buffer.is_empty() && guard.eof { @@ -163,7 +165,7 @@ pub(crate) fn serve_output_on_stream( let mut guard = output_state .state .lock() - .expect("Failed to lock stdout state"); + .map_err(|error| anyhow::anyhow!("Failed to lock stdout state for {label}: {error}"))?; // Since we copied the buffer, there may have been new output produced while we were writing to the stream. We // only remove the number of bytes that we successfully wrote, so that any new output will still be in the buffer @@ -220,7 +222,7 @@ mod tests { } #[test] - fn test_pump_output_to_state_multiple_chunks() { + fn test_pump_output_to_state_multiple_chunks() -> Result<(), anyhow::Error> { let state = Arc::new(NotifyableOutputState::new()); let data = vec![0u8; 16384]; // Larger than buffer size (8192). let input = Cursor::new(data.clone()); @@ -228,8 +230,12 @@ mod tests { let result = pump_output_to_state(input, Arc::clone(&state), "test"); assert!(result.is_ok()); - let guard = state.state.lock().unwrap(); + let guard = state + .state + .lock() + .map_err(|error| anyhow::anyhow!("Failed to lock output state for test: {error}"))?; assert_eq!(guard.buffer, data); assert!(guard.eof); + Ok(()) } } diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index 8fa05ff..9bf9ac7 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -90,8 +90,12 @@ pub fn protocol_server( } }; - stdin_thread.join().expect("Failed to join stdin thread"); - stdout_thread.join().expect("Failed to join stdout thread"); + stdin_thread + .join() + .map_err(|error| anyhow::anyhow!("Stdin forwarding thread panicked: {error:?}"))?; + stdout_thread + .join() + .map_err(|error| anyhow::anyhow!("Stdout forwarding thread panicked: {error:?}"))?; Ok(()) } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 98f523d..e42fdb8 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -1,4 +1,9 @@ //! Integration tests for stdioxide TCP forwarder functionality. +#![cfg(test)] +#![allow( + clippy::panic_in_result_fn, + reason = "Using `assert!()`s is idiomatic, but we need to return `Result`s to be able to return I/O-related errors." +)] use std::{ collections::HashSet, @@ -11,7 +16,6 @@ use std::{ }; // Acknowledge available dev-dependencies not used in this test file. -use anyhow as _; use clap as _; use lsp_types as _; use stdioxide as _; @@ -32,7 +36,7 @@ static ALLOCATED_PORTS_REGISTRY: LazyLock>> = /// Find an available port by binding to port 0 and letting the OS assign one. /// Returns the port number that was assigned. /// Never returns default ports (7000, 7001, 7002) to avoid conflicts with `test_default_port_values()`. -fn find_available_port() -> u16 { +fn find_available_port() -> Result { loop { // Note: This function binds to a port to find out whether it’s available. But this exposes a race // condition if actions happen in the following order: @@ -48,14 +52,14 @@ fn find_available_port() -> u16 { // spawning the forwarder, so if it fails to bind to the port, it will try again with a different // shortly after. let port = TcpListener::bind("127.0.0.1:0") - .expect("Failed to bind to find available port") + .map_err(|error| anyhow::anyhow!("Failed to bind to find available port: {error}"))? .local_addr() - .expect("Failed to get local address") + .map_err(|error| anyhow::anyhow!("Failed to get local address: {error}"))? .port(); // Skip default ports to avoid conflicts with `test_default_port_values()`. if !(7000..=7002).contains(&port) { - return port; + return Ok(port); } } } @@ -67,6 +71,10 @@ struct PortGuard { impl Drop for PortGuard { fn drop(&mut self) { + #[expect( + clippy::unwrap_used, + reason = "This is the destructor and we cannot really do something else if locking fails, so it’s acceptable to unwrap here." + )] let mut registry = ALLOCATED_PORTS_REGISTRY.lock().unwrap(); for port in &self.ports { registry.remove(port); @@ -89,12 +97,12 @@ impl AllocatedPorts { /// Allocate three available ports and register them globally. /// Retries if the OS-assigned ports are already allocated by another test. /// Multiple tests can allocate ports concurrently without blocking each other. - fn new() -> Self { + fn new() -> Result { loop { // Find candidate ports from the OS - let p1 = find_available_port(); - let p2 = find_available_port(); - let p3 = find_available_port(); + let p1 = find_available_port()?; + let p2 = find_available_port()?; + let p3 = find_available_port()?; // `find_available_port()` could return a port value that was previously found out to // be free in the context of a different test, but has not been bound yet. However, it would @@ -102,20 +110,22 @@ impl AllocatedPorts { // that such a port is *really* available. // Try to reserve them in the global registry. - let mut registry = ALLOCATED_PORTS_REGISTRY.lock().unwrap(); + let mut registry = ALLOCATED_PORTS_REGISTRY.lock().map_err(|error| { + anyhow::anyhow!("Failed to lock allocated ports registry: {error}") + })?; if !registry.contains(&p1) && !registry.contains(&p2) && !registry.contains(&p3) { registry.insert(p1); registry.insert(p2); registry.insert(p3); - return Self { + return Ok(Self { protocol_port: p1, stderr_port: p2, health_port: p3, _guard: PortGuard { ports: [p1, p2, p3], }, - }; + }); } // If any port is already allocated, try again } @@ -147,17 +157,17 @@ impl TestForwarder { /// Start a new `stdioxide` forwarder with the given command and arguments. /// Automatically allocates unique available ports for this test. /// Retries if port binding fails (e.g., if a port was grabbed between discovery and binding). - fn start(command: &str, args: &[&str]) -> Self { + fn start(command: &str, args: &[&str]) -> Result { const MAX_RETRIES: usize = 3; let mut last_error = None; for attempt in 0..MAX_RETRIES { // Allocate ports--registered in global registry. - let ports = AllocatedPorts::new(); + let ports = AllocatedPorts::new()?; // Try to start with these ports. match Self::try_start_with_ports(command, args, ports) { - Ok(forwarder) => return forwarder, + Ok(forwarder) => return Ok(forwarder), Err(e) => { last_error = Some(e); if attempt < MAX_RETRIES - 1 { @@ -169,11 +179,10 @@ impl TestForwarder { } } - panic!( - "Failed to start forwarder after {} attempts. Last error: {}", - MAX_RETRIES, - last_error.unwrap() - ); + let last_error = last_error.unwrap_or_else(|| "Unknown error".to_owned()); + Err(anyhow::anyhow!( + "Failed to start forwarder after {MAX_RETRIES} attempts. Last error: {last_error}", + )) } /// Try to start a new `stdioxide` forwarder with pre-allocated ports. @@ -230,7 +239,7 @@ impl TestForwarder { /// Try to wait for the forwarder to be ready by attempting to connect to the health port. /// Returns an error if the forwarder doesn’t become ready in time. - fn try_wait_for_ready(&self) -> Result<(), String> { + fn try_wait_for_ready(&self) -> Result<(), anyhow::Error> { const NUM_ATTEMPTS: usize = 30; let mut last_error = None; @@ -238,7 +247,9 @@ impl TestForwarder { match TcpStream::connect_timeout( &format!("127.0.0.1:{}", self.ports.health_port()) .parse() - .unwrap(), + .map_err(|error| { + anyhow::anyhow!("Failed to parse health port address: {error}") + })?, Duration::from_millis(100), ) { Ok(_) => return Ok(()), @@ -251,36 +262,39 @@ impl TestForwarder { } } - Err(format!( - "Forwarder did not become ready in time on port {}. Last error: {}", + let last_error = + last_error.map_or_else(|| "Unknown error".to_owned(), |error| format!("{error}")); + Err(anyhow::anyhow!( + "Forwarder did not become ready in time on port {}. Last error: {last_error}", self.ports.health_port(), - last_error.unwrap() )) } /// Connect to the protocol port. - fn connect_protocol(&self) -> TcpStream { + fn connect_protocol(&self) -> Result { self.connect_with_retry(self.ports.protocol_port(), "protocol") } /// Connect to the `stderr` port. - fn connect_stderr(&self) -> TcpStream { + fn connect_stderr(&self) -> Result { self.connect_with_retry(self.ports.stderr_port(), "stderr") } /// Connect to the health port. - fn connect_health(&self) -> TcpStream { + fn connect_health(&self) -> Result { self.connect_with_retry(self.ports.health_port(), "health") } /// Connect to a port with retries. - fn connect_with_retry(&self, port: u16, label: &str) -> TcpStream { + fn connect_with_retry(&self, port: u16, label: &str) -> Result { const NUM_ATTEMPTS: usize = 20; for attempt in 0..NUM_ATTEMPTS { match TcpStream::connect(("127.0.0.1", port)) { - Ok(stream) => return stream, + Ok(stream) => return Ok(stream), Err(e) if attempt == NUM_ATTEMPTS - 1 => { - panic!("Failed to connect to {} port {}: {}", label, port, e); + return Err(anyhow::anyhow!( + "Failed to connect to {label} port {port} after {NUM_ATTEMPTS} attempts: {e}" + )); } Err(_) => { thread::sleep(Duration::from_millis(50)); @@ -318,12 +332,12 @@ fn read_with_timeout(stream: &mut TcpStream, buffer: &mut [u8]) -> std::io::Resu } /// Helper function to read all available data from a stream up to a timeout. -fn read_all_available(stream: &mut TcpStream, timeout: Duration) -> Vec { +fn read_all_available(stream: &mut TcpStream, timeout: Duration) -> Result, anyhow::Error> { let mut result = Vec::new(); let mut buffer = [0u8; 8192]; stream .set_read_timeout(Some(timeout)) - .expect("Failed to set read timeout"); + .map_err(|error| anyhow::anyhow!("Failed to set read timeout: {error}"))?; loop { match stream.read(&mut buffer) { @@ -335,7 +349,7 @@ fn read_all_available(stream: &mut TcpStream, timeout: Duration) -> Vec { } } - result + Ok(result) } // ============================================================================ @@ -343,65 +357,68 @@ fn read_all_available(stream: &mut TcpStream, timeout: Duration) -> Vec { // ============================================================================ #[test] -fn test_forwarder_starts_arbitrary_child_process() { +fn test_forwarder_starts_arbitrary_child_process() -> Result<(), anyhow::Error> { // * [x] A standalone forwarder executable can be started that launches an arbitrary child process. let (cmd, args) = sleep_cmd(5); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; // If we got here, the forwarder started successfully. // The forwarder should be ready (health port should be accessible). - assert!(forwarder.connect_health().peer_addr().is_ok()); + assert!(forwarder.connect_health()?.peer_addr().is_ok()); + Ok(()) } #[test] -fn test_forwarder_passes_arguments_unchanged() { +fn test_forwarder_passes_arguments_unchanged() -> Result<(), anyhow::Error> { // * [x] The forwarder passes command-line arguments through to the child process unchanged. // Use a command that outputs arguments and then waits, so we have time to connect. let (cmd, args) = echo_args_cmd(&["-n", "test", "with spaces", "--flag"]); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; - let mut stream = forwarder.connect_protocol(); - let output = read_all_available(&mut stream, Duration::from_millis(500)); + let mut stream = forwarder.connect_protocol()?; + let output = read_all_available(&mut stream, Duration::from_millis(500))?; // `bash` should output all the arguments. let output_str = String::from_utf8_lossy(&output); assert!(output_str.contains("test")); assert!(output_str.contains("with spaces")); assert!(output_str.contains("--flag")); + Ok(()) } #[test] -fn test_child_process_configurable_externally() { +fn test_child_process_configurable_externally() -> Result<(), anyhow::Error> { // * [x] The child process to launch can be configured externally. // Test with different commands to verify external configuration works. let (cmd1, args1) = echo_with_sleep_cmd("first", 5); let args1_refs: Vec<&str> = args1.iter().map(|s| s.as_str()).collect(); - let forwarder1 = TestForwarder::start(cmd1, &args1_refs); - let mut stream1 = forwarder1.connect_protocol(); - let output1 = read_all_available(&mut stream1, Duration::from_millis(500)); + let forwarder1 = TestForwarder::start(cmd1, &args1_refs)?; + let mut stream1 = forwarder1.connect_protocol()?; + let output1 = read_all_available(&mut stream1, Duration::from_millis(500))?; assert!(String::from_utf8_lossy(&output1).contains("first")); let (cmd2, args2) = echo_with_sleep_cmd("second", 5); let args2_refs: Vec<&str> = args2.iter().map(|s| s.as_str()).collect(); - let forwarder2 = TestForwarder::start(cmd2, &args2_refs); - let mut stream2 = forwarder2.connect_protocol(); - let output2 = read_all_available(&mut stream2, Duration::from_millis(500)); + let forwarder2 = TestForwarder::start(cmd2, &args2_refs)?; + let mut stream2 = forwarder2.connect_protocol()?; + let output2 = read_all_available(&mut stream2, Duration::from_millis(500))?; assert!(String::from_utf8_lossy(&output2).contains("second")); + Ok(()) } #[test] -fn test_forwarder_exits_when_child_exits() { +fn test_forwarder_exits_when_child_exits() -> Result<(), anyhow::Error> { // * [x] When the child process exits for any reason, the forwarder also terminates. // Use a command that runs briefly and then exits. let (cmd, args) = short_lived_cmd("test", 0); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let mut forwarder = TestForwarder::start(cmd, &args_refs); + let mut forwarder = TestForwarder::start(cmd, &args_refs)?; // Connect to protocol port to ensure we’re monitoring the forwarder. let _stream = forwarder.connect_protocol(); @@ -414,10 +431,11 @@ fn test_forwarder_exits_when_child_exits() { forwarder.wait_for_exit(), "Forwarder should exit when child exits" ); + Ok(()) } #[test] -fn test_forwarder_exposes_three_tcp_ports() { +fn test_forwarder_exposes_three_tcp_ports() -> Result<(), anyhow::Error> { // * [x] The forwarder exposes three TCP ports: // * [x] a **protocol port** // * [x] an **stderr port** @@ -425,28 +443,29 @@ fn test_forwarder_exposes_three_tcp_ports() { let (cmd, args) = sleep_cmd(10); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; // Verify all three ports are accessible. - // Note: The `connect_*()` methods already `panic!()` if connection fails, so the real - // accessibility check happens *there*. The `.peer_addr().is_ok()` is redundant but - // serves as documentation. + // Note: The `connect_*()` methods already return an `Err` if connection fails (which + // gets propagated), so the real accessibility check happens *there*. The + // `.peer_addr().is_ok()` is redundant but serves as documentation. assert!( - forwarder.connect_protocol().peer_addr().is_ok(), + forwarder.connect_protocol()?.peer_addr().is_ok(), "Protocol port should be accessible" ); assert!( - forwarder.connect_stderr().peer_addr().is_ok(), + forwarder.connect_stderr()?.peer_addr().is_ok(), "Stderr port should be accessible" ); assert!( - forwarder.connect_health().peer_addr().is_ok(), + forwarder.connect_health()?.peer_addr().is_ok(), "Health port should be accessible" ); + Ok(()) } #[test] -fn test_default_port_values() { +fn test_default_port_values() -> Result<(), anyhow::Error> { // * [x] The default port values are: // * [x] `7000` for the protocol port // * [x] `7001` for the stderr port @@ -458,7 +477,10 @@ fn test_default_port_values() { let error_message = format!( "TEST SETUP FAILED: Default port {port} is not available. This is an environment issue, not a test failure." ); - drop(TcpListener::bind(format!("127.0.0.1:{port}")).expect(&error_message)); + drop( + TcpListener::bind(format!("127.0.0.1:{port}")) + .map_err(|error| anyhow::anyhow!("{error}"))?, + ); // Port is immediately released here. } @@ -475,17 +497,16 @@ fn test_default_port_values() { cmd.stderr(Stdio::piped()); cmd.stdout(Stdio::piped()); - let mut process = cmd.spawn().expect("Failed to start stdioxide"); + let mut process = cmd + .spawn() + .map_err(|error| anyhow::anyhow!("Failed to start stdioxide: {error}"))?; // Wait for the forwarder to be ready by connecting to the default health port. let mut connected = false; const NUM_ATTEMPTS: usize = 30; for _ in 0..NUM_ATTEMPTS { - if TcpStream::connect_timeout( - &"127.0.0.1:7002".parse().unwrap(), - Duration::from_millis(100), - ) - .is_ok() + if TcpStream::connect_timeout(&"127.0.0.1:7002".parse()?, Duration::from_millis(100)) + .is_ok() { connected = true; break; @@ -508,14 +529,15 @@ fn test_default_port_values() { // Clean up. drop(process.kill()); drop(process.wait()); + Ok(()) } #[test] -fn test_port_override_via_environment_variables() { +fn test_port_override_via_environment_variables() -> Result<(), anyhow::Error> { // * [x] All three port numbers can be overridden via environment variables. // Allocate unique ports to avoid conflicts. - let ports = AllocatedPorts::new(); + let ports = AllocatedPorts::new()?; let custom_protocol = ports.protocol_port(); let custom_stderr = ports.stderr_port(); let custom_health = ports.health_port(); @@ -535,7 +557,9 @@ fn test_port_override_via_environment_variables() { } cmd.stderr(Stdio::piped()).stdout(Stdio::piped()); - let mut process = cmd.spawn().expect("Failed to start stdioxide"); + let mut process = cmd + .spawn() + .map_err(|error| anyhow::anyhow!("Failed to start stdioxide: {error}"))?; // Wait for the forwarder to be ready by connecting to the custom health port. let mut connected = false; @@ -575,97 +599,108 @@ fn test_port_override_via_environment_variables() { // Clean up. drop(process.kill()); drop(process.wait()); + Ok(()) } #[test] -fn test_stdout_sent_over_protocol_port() { +fn test_stdout_sent_over_protocol_port() -> Result<(), anyhow::Error> { // * [x] The forwarder sends the child process’s `stdout` stream over the protocol port. let (cmd, args) = echo_with_sleep_cmd("Hello from stdout", 5); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; - let mut stream = forwarder.connect_protocol(); - let output = read_all_available(&mut stream, Duration::from_millis(500)); + let mut stream = forwarder.connect_protocol()?; + let output = read_all_available(&mut stream, Duration::from_millis(500))?; let output_str = String::from_utf8_lossy(&output); assert!(output_str.contains("Hello from stdout")); + Ok(()) } #[test] -fn test_stdin_received_on_protocol_port() { +fn test_stdin_received_on_protocol_port() -> Result<(), anyhow::Error> { // * [x] The forwarder receives input for the child process’s `stdin` stream on the protocol port. // * [x] Data received on the protocol port is forwarded to the child process’s `stdin` while the connection is active. let (cmd, args) = cat_cmd(); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; - let mut stream = forwarder.connect_protocol(); + let mut stream = forwarder.connect_protocol()?; // Send data to `stdin` via the protocol port. stream .write_all(b"test input\n") - .expect("Failed to write to protocol port"); - stream.flush().expect("Failed to flush"); + .map_err(|error| anyhow::anyhow!("Failed to write to protocol port: {error}"))?; + stream + .flush() + .map_err(|error| anyhow::anyhow!("Failed to flush protocol port: {error}"))?; // Read back the echoed output from `stdout`. // Increased timeout to account for PowerShell startup on Windows - let output = read_all_available(&mut stream, Duration::from_millis(1500)); + let output = read_all_available(&mut stream, Duration::from_millis(1500))?; let output_str = String::from_utf8_lossy(&output); assert!(output_str.contains("test input")); + Ok(()) } #[test] -fn test_stderr_sent_over_stderr_port() { +fn test_stderr_sent_over_stderr_port() -> Result<(), anyhow::Error> { // * [x] The forwarder sends the child process’s `stderr` stream over the `stderr` port. // Use a `bash` command that writes to `stderr` and then waits. let (cmd, args) = stderr_echo_with_sleep_cmd("error message", 5); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); - - let mut stream = forwarder.connect_stderr(); - let output = read_all_available(&mut stream, Duration::from_millis(500)); + let forwarder = TestForwarder::start(cmd, &args_refs)?; + let mut stream = forwarder.connect_stderr()?; + let output = read_all_available(&mut stream, Duration::from_millis(500))?; let output_str = String::from_utf8_lossy(&output); assert!(output_str.contains("error message")); + Ok(()) } #[test] -fn test_protocol_port_single_client_only() { +fn test_protocol_port_single_client_only() -> Result<(), anyhow::Error> { // * [x] The protocol port allows at most one active client connection at a time. let (cmd, args) = loop_stdin_to_stdout_cmd(); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; // First client connects successfully. - let mut stream1 = forwarder.connect_protocol(); + let mut stream1 = forwarder.connect_protocol()?; // Verify first client works. - stream1.write_all(b"test\n").expect("Failed to write"); - stream1.flush().expect("Failed to flush"); + stream1 + .write_all(b"test\n") + .map_err(|error| anyhow::anyhow!("Failed to write to protocol port: {error}"))?; + stream1 + .flush() + .map_err(|error| anyhow::anyhow!("Failed to flush protocol port: {error}"))?; // Increased timeout to account for PowerShell startup and buffering on Windows - let output = read_all_available(&mut stream1, Duration::from_millis(1500)); + let output = read_all_available(&mut stream1, Duration::from_millis(1500))?; assert!(String::from_utf8_lossy(&output).contains("response")); // Second client connection attempt. // The protocol server only calls `accept()` once, so the second connection // succeeds at the TCP level (queued in backlog) but is never accepted/served. let mut stream2 = TcpStream::connect_timeout( - &format!("127.0.0.1:{}", forwarder.ports.protocol_port()) - .parse() - .unwrap(), + &format!("127.0.0.1:{}", forwarder.ports.protocol_port()).parse()?, Duration::from_millis(200), ) - .expect("Second client should connect successfully (TCP handshake completes)"); + .map_err(|error| { + anyhow::anyhow!( + "Second client should connect successfully (TCP handshake completes): {error}" + ) + })?; // The connection is established but never served--reading should timeout. stream2 .set_read_timeout(Some(Duration::from_millis(200))) - .expect("Should set read timeout"); + .map_err(|error| anyhow::anyhow!("Should set read timeout: {error}"))?; let mut buf = [0u8; 100]; let result = stream2.read(&mut buf); @@ -678,29 +713,32 @@ fn test_protocol_port_single_client_only() { ), "Second client should timeout reading (connection never served by protocol server)" ); + Ok(()) } #[test] -fn test_stderr_port_single_client_only() { +fn test_stderr_port_single_client_only() -> Result<(), anyhow::Error> { // * [x] The `stderr` port allows at most one active client connection at a time. let (cmd, args) = continuous_stderr_loop_cmd(); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; // First client connects successfully. - let _stream1 = forwarder.connect_stderr(); + let _stream1 = forwarder.connect_stderr()?; // Second client should connect but be rejected. // According to the `stderr_server` implementation, it rejects additional connections. - let stream2 = forwarder.connect_stderr(); + let stream2 = forwarder.connect_stderr()?; // The second connection is made but immediately closed/rejected. // Try to read--should get no data or connection closed. let output = read_all_available( - &mut stream2.try_clone().unwrap(), + &mut stream2 + .try_clone() + .map_err(|error| anyhow::anyhow!("Failed to clone stream: {error}"))?, Duration::from_millis(500), - ); + )?; // The second client should not receive meaningful data since the first is still active. // In practice, the connection is accepted but dropped, so we should see minimal/no data. @@ -709,20 +747,21 @@ fn test_stderr_port_single_client_only() { output.is_empty() || output.len() < 100, "Second stderr client should not receive full stream data while first client is active" ); + Ok(()) } #[test] -fn test_health_port_multiple_clients() { +fn test_health_port_multiple_clients() -> Result<(), anyhow::Error> { // * [x] The health port allows multiple simultaneous client connections. let (cmd, args) = sleep_cmd(10); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; // Connect multiple clients to the health port. - let _stream1 = forwarder.connect_health(); - let _stream2 = forwarder.connect_health(); - let _stream3 = forwarder.connect_health(); + let _stream1 = forwarder.connect_health()?; + let _stream2 = forwarder.connect_health()?; + let _stream3 = forwarder.connect_health()?; // All connections should succeed. assert!( @@ -730,10 +769,11 @@ fn test_health_port_multiple_clients() { && _stream2.peer_addr().is_ok() && _stream3.peer_addr().is_ok() ); + Ok(()) } #[test] -fn test_protocol_port_buffered_stdout_replay() { +fn test_protocol_port_buffered_stdout_replay() -> Result<(), anyhow::Error> { // * [x] When a client connects to the protocol port for the first time, it first receives all buffered `stdout` data // produced before the connection was established. // * [x] After the buffered `stdout` data has been sent, the client continues to receive newly produced `stdout` data @@ -742,30 +782,31 @@ fn test_protocol_port_buffered_stdout_replay() { // Use a script that produces output immediately and then waits. let (cmd, args) = multi_echo_stdout_cmd("buffered output", 1.0, "realtime output", 10); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; // Now connect - buffering ensures we receive output produced before connection. thread::sleep(Duration::from_millis(100)); // Connect and we should receive the buffered output first. - let mut stream = forwarder.connect_protocol(); + let mut stream = forwarder.connect_protocol()?; // Read the output. - let output = read_all_available(&mut stream, Duration::from_secs(3)); + let output = read_all_available(&mut stream, Duration::from_secs(3))?; let output_str = String::from_utf8_lossy(&output); // Verify we got both buffered and realtime output. assert!(output_str.contains("buffered output")); assert!(output_str.contains("realtime output")); + Ok(()) } #[test] -fn test_protocol_disconnect_kills_child() { +fn test_protocol_disconnect_kills_child() -> Result<(), anyhow::Error> { // * [x] When a client disconnects from the protocol port, the child process is killed and the forwarder terminates. let (cmd, args) = sleep_cmd(100); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let mut forwarder = TestForwarder::start(cmd, &args_refs); + let mut forwarder = TestForwarder::start(cmd, &args_refs)?; { let _stream = forwarder.connect_protocol(); @@ -777,10 +818,11 @@ fn test_protocol_disconnect_kills_child() { forwarder.wait_for_exit(), "Forwarder should exit when protocol client disconnects" ); + Ok(()) } #[test] -fn test_stderr_port_buffered_stderr_replay() { +fn test_stderr_port_buffered_stderr_replay() -> Result<(), anyhow::Error> { // * [x] When a client connects to the `stderr` port, it first receives all buffered `stderr` data // produced before the connection was established. // * [x] After the buffered `stderr` data has been sent, the client continues to receive newly produced @@ -789,32 +831,32 @@ fn test_stderr_port_buffered_stderr_replay() { // Use a script that produces `stderr` immediately and then waits. let (cmd, args) = multi_echo_stderr_cmd("buffered error", 1.0, "realtime error", 10); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; // Now connect to `stderr`--buffering ensures we receive output produced before connection. thread::sleep(Duration::from_millis(100)); // Connect and we should receive the buffered output first. - let mut stream = forwarder.connect_stderr(); + let mut stream = forwarder.connect_stderr()?; // Read the output. - let output = read_all_available(&mut stream, Duration::from_secs(2)); + let output = read_all_available(&mut stream, Duration::from_secs(2))?; let output_str = String::from_utf8_lossy(&output); // Verify we got both buffered and realtime output. assert!(output_str.contains("buffered error")); assert!(output_str.contains("realtime error")); + Ok(()) } #[test] -fn test_stderr_disconnect_does_not_kill_child() { +fn test_stderr_disconnect_does_not_kill_child() -> Result<(), anyhow::Error> { // * [x] When a client disconnects from the `stderr` port, neither the forwarder nor the child // process terminate because of that. let (cmd, args) = sleep_cmd(10); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); - + let forwarder = TestForwarder::start(cmd, &args_refs)?; { let _stderr_stream = forwarder.connect_stderr(); // Disconnect by dropping the stream. @@ -825,13 +867,14 @@ fn test_stderr_disconnect_does_not_kill_child() { // Forwarder should still be running--we can connect to health port. assert!( - forwarder.connect_health().peer_addr().is_ok(), + forwarder.connect_health()?.peer_addr().is_ok(), "Forwarder should still be running after stderr disconnect" ); + Ok(()) } #[test] -fn test_stderr_port_reconnect_continues_from_current_state() { +fn test_stderr_port_reconnect_continues_from_current_state() -> Result<(), anyhow::Error> { // * [x] When a client connects to the `stderr` port, it first receives all buffered `stderr` data // produced before the connection was established and after a previous connection was active // (i.e., no logging data is lost). @@ -844,16 +887,16 @@ fn test_stderr_port_reconnect_continues_from_current_state() { // - "during_second_connection" after 5 seconds (sent to second client) let (cmd, args) = complex_stderr_reconnect_cmd(); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; // Wait to ensure "before_connection" is buffered. thread::sleep(Duration::from_millis(100)); // First connection--connect, read initial data, then disconnect BEFORE "trigger_disconnect". { - let mut stream = forwarder.connect_stderr(); + let mut stream = forwarder.connect_stderr()?; // Read for 800ms to get "before_connection" (immediate) and "during_first_connection" (at t=0.5s) - let output = read_all_available(&mut stream, Duration::from_millis(800)); + let output = read_all_available(&mut stream, Duration::from_millis(800))?; let output_str = String::from_utf8_lossy(&output); assert!(output_str.contains("before_connection")); assert!(output_str.contains("during_first_connection")); @@ -875,9 +918,9 @@ fn test_stderr_port_reconnect_continues_from_current_state() { // Second connection--should receive all buffered data (trigger_disconnect, while_disconnected) // and realtime data (during_second_connection). No logging data must be lost. { - let mut stream = forwarder.connect_stderr(); + let mut stream = forwarder.connect_stderr()?; // Read for 2.5s to get buffered and realtime data - let output = read_all_available(&mut stream, Duration::from_millis(2500)); + let output = read_all_available(&mut stream, Duration::from_millis(2500))?; let output_str = String::from_utf8_lossy(&output); assert!( @@ -898,59 +941,62 @@ fn test_stderr_port_reconnect_continues_from_current_state() { output_str ); } + Ok(()) } #[test] -fn test_output_buffering_prevents_data_loss() { +fn test_output_buffering_prevents_data_loss() -> Result<(), anyhow::Error> { // * [x] Output buffering must prevent loss of `stdout` and `stderr` data when no client is connected yet. // Start a process that produces output immediately. let (cmd, args) = combined_output_cmd("stdout message", "stderr message", 10); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; // Buffering ensures output is captured even if we connect immediately. thread::sleep(Duration::from_millis(100)); // Now connect--we should receive the buffered output. - let mut stdout_stream = forwarder.connect_protocol(); - let stdout_data = read_all_available(&mut stdout_stream, Duration::from_millis(500)); + let mut stdout_stream = forwarder.connect_protocol()?; + let stdout_data = read_all_available(&mut stdout_stream, Duration::from_millis(500))?; let stdout_str = String::from_utf8_lossy(&stdout_data); - let mut stderr_stream = forwarder.connect_stderr(); - let stderr_data = read_all_available(&mut stderr_stream, Duration::from_millis(500)); + let mut stderr_stream = forwarder.connect_stderr()?; + let stderr_data = read_all_available(&mut stderr_stream, Duration::from_millis(500))?; let stderr_str = String::from_utf8_lossy(&stderr_data); assert!(stdout_str.contains("stdout message")); assert!(stderr_str.contains("stderr message")); + Ok(()) } #[test] -fn test_health_port_indicates_readiness() { +fn test_health_port_indicates_readiness() -> Result<(), anyhow::Error> { // * [x] A successful TCP connection to the health port indicates that the forwarder is ready to accept connections and operate normally. let (cmd, args) = sleep_cmd(10); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; // If we can connect to health port, the forwarder is ready. - let health_stream = forwarder.connect_health(); + let health_stream = forwarder.connect_health()?; assert!(health_stream.peer_addr().is_ok()); // And the other ports should also be accessible. - assert!(forwarder.connect_protocol().peer_addr().is_ok()); - assert!(forwarder.connect_stderr().peer_addr().is_ok()); + assert!(forwarder.connect_protocol()?.peer_addr().is_ok()); + assert!(forwarder.connect_stderr()?.peer_addr().is_ok()); + Ok(()) } #[test] -fn test_health_checks_do_not_interfere() { +fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { // * [x] Health checks on the health port must not interfere with the behavior of the protocol port or the `stderr` port. // Use a process that produces high-volume output on both `stdout` and `stderr`. // Output a unique numbered line every 10ms for 3 seconds (300 lines on each stream). let (cmd, args) = numbered_output_loop_cmd(300, 10); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; // Spawn a thread to continuously perform health checks for 3.5 seconds. let health_port = forwarder.ports.health_port(); @@ -967,7 +1013,7 @@ fn test_health_checks_do_not_interfere() { }); // Spawn a thread to read from the protocol port (`stdout`). - let mut protocol_stream = forwarder.connect_protocol(); + let mut protocol_stream = forwarder.connect_protocol()?; let protocol_handle = thread::spawn(move || { let mut all_output = Vec::new(); let mut buffer = [0u8; 8192]; @@ -988,7 +1034,7 @@ fn test_health_checks_do_not_interfere() { }); // Spawn a thread to read from the stderr port (`stderr`). - let mut stderr_stream = forwarder.connect_stderr(); + let mut stderr_stream = forwarder.connect_stderr()?; let stderr_handle = thread::spawn(move || { let mut all_output = Vec::new(); let mut buffer = [0u8; 8192]; @@ -1011,9 +1057,13 @@ fn test_health_checks_do_not_interfere() { // Wait for all threads to complete. let health_check_count = health_check_handle .join() - .expect("Health check thread panicked"); - let protocol_output = protocol_handle.join().expect("Protocol thread panicked"); - let stderr_output = stderr_handle.join().expect("Stderr thread panicked"); + .map_err(|error| anyhow::anyhow!("Health check thread panicked: {error:?}"))?; + let protocol_output = protocol_handle + .join() + .map_err(|error| anyhow::anyhow!("Protocol thread panicked: {error:?}"))?; + let stderr_output = stderr_handle + .join() + .map_err(|error| anyhow::anyhow!("Stderr thread panicked: {error:?}"))?; // Verify that health checks were performed successfully. // Note: The count varies significantly by platform (Linux/Windows: ~150+, macOS: ~35-40) @@ -1050,10 +1100,12 @@ fn test_health_checks_do_not_interfere() { assert!(protocol_str.contains("stdout_line_100")); assert!(stderr_str.contains("stderr_line_1")); assert!(stderr_str.contains("stderr_line_100")); + + Ok(()) } #[test] -fn test_works_with_various_executables() { +fn test_works_with_various_executables() -> Result<(), anyhow::Error> { // * [x] The forwarder must work not only for Python applications, but for any executable child process. // Test with various common executables. @@ -1062,9 +1114,9 @@ fn test_works_with_various_executables() { { let (cmd, args) = echo_with_sleep_cmd("test1", 2); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); - let mut stream = forwarder.connect_protocol(); - let output = read_all_available(&mut stream, Duration::from_millis(500)); + let forwarder = TestForwarder::start(cmd, &args_refs)?; + let mut stream = forwarder.connect_protocol()?; + let output = read_all_available(&mut stream, Duration::from_millis(500))?; assert!(String::from_utf8_lossy(&output).contains("test1")); } @@ -1072,9 +1124,9 @@ fn test_works_with_various_executables() { { let (cmd, args) = echo_with_sleep_cmd("test2", 2); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); - let mut stream = forwarder.connect_protocol(); - let output = read_all_available(&mut stream, Duration::from_millis(500)); + let forwarder = TestForwarder::start(cmd, &args_refs)?; + let mut stream = forwarder.connect_protocol()?; + let output = read_all_available(&mut stream, Duration::from_millis(500))?; assert!(String::from_utf8_lossy(&output).contains("test2")); } @@ -1082,11 +1134,15 @@ fn test_works_with_various_executables() { { let (cmd, args) = cat_cmd(); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); - let mut stream = forwarder.connect_protocol(); - stream.write_all(b"test3\n").expect("Failed to write"); - stream.flush().expect("Failed to flush"); - let output = read_all_available(&mut stream, Duration::from_millis(500)); + let forwarder = TestForwarder::start(cmd, &args_refs)?; + let mut stream = forwarder.connect_protocol()?; + stream + .write_all(b"test3\n") + .map_err(|error| anyhow::anyhow!("Failed to write to protocol port: {error}"))?; + stream + .flush() + .map_err(|error| anyhow::anyhow!("Failed to flush protocol port: {error}"))?; + let output = read_all_available(&mut stream, Duration::from_millis(500))?; assert!(String::from_utf8_lossy(&output).contains("test3")); } @@ -1097,35 +1153,37 @@ fn test_works_with_various_executables() { let forwarder = TestForwarder::start( python, &["-u", "-c", "import time; print('test4'); time.sleep(2)"], - ); - let mut stream = forwarder.connect_protocol(); + )?; + let mut stream = forwarder.connect_protocol()?; // Delay to ensure Python has started and produced output. thread::sleep(Duration::from_millis(300)); // Increased timeout to account for Python interpreter startup (especially on Windows). - let output = read_all_available(&mut stream, Duration::from_millis(3000)); + let output = read_all_available(&mut stream, Duration::from_millis(3000))?; assert!( String::from_utf8_lossy(&output).contains("test4"), "Expected 'test4' in output, got: {:?}", String::from_utf8_lossy(&output) ); } + + Ok(()) } #[test] -fn test_large_output_buffering() { +fn test_large_output_buffering() -> Result<(), anyhow::Error> { // Additional test: verify that large outputs are buffered correctly. // Generate a large output. let large_size = 100_000; let (cmd, args) = generate_large_output_cmd(large_size); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); + let forwarder = TestForwarder::start(cmd, &args_refs)?; // Wait for output to be generated. thread::sleep(Duration::from_millis(200)); // Connect and read the buffered output. - let mut stream = forwarder.connect_protocol(); + let mut stream = forwarder.connect_protocol()?; let mut total_read = 0; let mut buffer = [0u8; 8192]; @@ -1143,35 +1201,39 @@ fn test_large_output_buffering() { large_size, total_read ); + Ok(()) } #[test] -fn test_concurrent_stdin_stdout_bidirectional() { +fn test_concurrent_stdin_stdout_bidirectional() -> Result<(), anyhow::Error> { // Additional test: verify bidirectional communication works correctly. // Use `cat` which echoes `stdin` to `stdout`. let (cmd, args) = cat_cmd(); let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - let forwarder = TestForwarder::start(cmd, &args_refs); - let mut stream = forwarder.connect_protocol(); + let forwarder = TestForwarder::start(cmd, &args_refs)?; + let mut stream = forwarder.connect_protocol()?; // Send multiple lines and verify echo. for i in 0..5 { let message = format!("line {i}\n"); stream .write_all(message.as_bytes()) - .expect("Failed to write"); - stream.flush().expect("Failed to flush"); + .map_err(|error| anyhow::anyhow!("Failed to write to protocol port: {error}"))?; + stream + .flush() + .map_err(|error| anyhow::anyhow!("Failed to flush protocol port: {error}"))?; // Increased timeout for Python startup on Windows. - let output = read_all_available(&mut stream, Duration::from_millis(1500)); + let output = read_all_available(&mut stream, Duration::from_millis(1500))?; let output_str = String::from_utf8_lossy(&output); assert!( output_str.contains(&format!("line {i}")), - "Expected 'line {i}' in output, got: {:?}", - output_str + "Expected 'line {i}' in output, got: {output_str:?}" ); } + + Ok(()) } // ============================================================================ @@ -1181,23 +1243,23 @@ fn test_concurrent_stdin_stdout_bidirectional() { mod lsp_client; #[test] -fn test_lsp_rust_analyzer_integration() { +fn test_lsp_rust_analyzer_integration() -> Result<(), anyhow::Error> { // Test that stdioxide can successfully tunnel LSP communication with `rust-analyzer`. // This validates the real-world use case of running a language server through the forwarder. - let mut forwarder = TestForwarder::start("rust-analyzer", &[]); - let stream = forwarder.connect_protocol(); + let mut forwarder = TestForwarder::start("rust-analyzer", &[])?; + let stream = forwarder.connect_protocol()?; - let mut lsp = LspClient::new(stream); + let mut lsp = LspClient::new(stream)?; // Initialize the LSP server. let workspace_path = std::env::current_dir() - .expect("Failed to get current directory") + .map_err(|error| anyhow::anyhow!("Failed to get current directory: {error}"))? .to_string_lossy() .to_string(); let root_uri = format!("file://{}", workspace_path); - let init_response = lsp.initialize(&root_uri); + let init_response = lsp.initialize(&root_uri)?; assert_eq!(init_response["jsonrpc"], "2.0"); assert!( @@ -1211,20 +1273,20 @@ fn test_lsp_rust_analyzer_integration() { // Open a document (src/main.rs). let main_rs_path = format!("{workspace_path}/src/main.rs"); let main_rs_uri = format!("file://{main_rs_path}"); - let main_rs_content = - std::fs::read_to_string(&main_rs_path).expect("Failed to read src/main.rs"); + let main_rs_content = std::fs::read_to_string(&main_rs_path) + .map_err(|error| anyhow::anyhow!("Failed to read src/main.rs: {error}"))?; lsp.did_open(&main_rs_uri, "rust", main_rs_content); // Request document symbols. - let symbols_response = lsp.document_symbol(&main_rs_uri); + let symbols_response = lsp.document_symbol(&main_rs_uri)?; assert_eq!(symbols_response["jsonrpc"], "2.0"); // Verify we got some symbols (src/main.rs should have at least the main function). let symbols = symbols_response["result"] .as_array() - .expect("Expected array of symbols"); + .ok_or_else(|| anyhow::anyhow!("Expected array of symbols"))?; assert!( !symbols.is_empty(), @@ -1239,7 +1301,7 @@ fn test_lsp_rust_analyzer_integration() { ); // Shutdown the LSP server. - let shutdown_response = lsp.shutdown(); + let shutdown_response = lsp.shutdown()?; assert_eq!(shutdown_response["result"], serde_json::Value::Null); // Exit notification is sent automatically when lsp is dropped. @@ -1251,4 +1313,5 @@ fn test_lsp_rust_analyzer_integration() { forwarder.wait_for_exit(), "Forwarder should exit after LSP client disconnects from protocol port" ); + Ok(()) } diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index 1790ab9..c0d31c2 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -25,19 +25,19 @@ pub struct LspClient { impl LspClient { /// Create a new LSP client from a TCP stream. - pub fn new(stream: TcpStream) -> Self { + pub fn new(stream: TcpStream) -> Result { // Set reasonable timeouts for LSP communication. stream .set_read_timeout(Some(Duration::from_secs(10))) - .expect("Failed to set read timeout"); + .map_err(|error| anyhow::anyhow!("Failed to set read timeout: {error}"))?; stream .set_write_timeout(Some(Duration::from_secs(5))) - .expect("Failed to set write timeout"); + .map_err(|error| anyhow::anyhow!("Failed to set write timeout: {error}"))?; - Self { + Ok(Self { stream, next_request_id: 1, - } + }) } /// Send an LSP message over the stream. @@ -52,7 +52,7 @@ impl LspClient { /// Read an LSP message from the stream. /// Returns the parsed JSON value. - fn read_message(&mut self) -> std::io::Result { + fn read_message(&mut self) -> anyhow::Result { // Read the Content-Length header. let mut header = String::new(); let mut buffer = [0u8; 1]; @@ -66,10 +66,7 @@ impl LspClient { } // Prevent infinite loops on malformed headers if header.len() > 1000 { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidData, - "Header too long", - )); + return Err(anyhow::anyhow!("Header too long")); } } @@ -79,9 +76,7 @@ impl LspClient { .find(|line| line.starts_with("Content-Length:")) .and_then(|line| line.strip_prefix("Content-Length:")) .and_then(|len_str| len_str.trim().parse::().ok()) - .ok_or_else(|| { - std::io::Error::new(std::io::ErrorKind::InvalidData, "Missing Content-Length") - })?; + .ok_or_else(|| anyhow::anyhow!("Missing Content-Length"))?; // Read the JSON content. let mut content = vec![0u8; content_length]; @@ -93,7 +88,11 @@ impl LspClient { } /// Send an LSP request and return the next request ID to use. - fn send_request(&mut self, method: &str, params: serde_json::Value) -> i32 { + fn send_request( + &mut self, + method: &str, + params: serde_json::Value, + ) -> Result { let request_id = self.next_request_id; self.next_request_id += 1; @@ -105,12 +104,16 @@ impl LspClient { }); self.send_message(&request) - .expect("Failed to send LSP request"); - request_id + .map_err(|error| anyhow::anyhow!("Failed to send LSP request: {error}"))?; + Ok(request_id) } /// Send an LSP notification (no response expected). - fn send_notification(&mut self, method: &str, params: serde_json::Value) { + fn send_notification( + &mut self, + method: &str, + params: serde_json::Value, + ) -> Result<(), anyhow::Error> { let notification = serde_json::json!({ "jsonrpc": "2.0", "method": method, @@ -118,35 +121,44 @@ impl LspClient { }); self.send_message(¬ification) - .expect("Failed to send LSP notification"); + .map_err(|error| anyhow::anyhow!("Failed to send LSP notification: {error}"))?; + Ok(()) } /// Read responses until we get a response with the specified ID. /// Skips notifications that may arrive in between. - fn read_response(&mut self, expected_id: i32) -> serde_json::Value { + fn read_response(&mut self, expected_id: i32) -> anyhow::Result { for _ in 0..20 { match self.read_message() { Ok(msg) => { // Check if this is our response. if msg.get("id") == Some(&serde_json::json!(expected_id)) { - return msg; + return Ok(msg); } // Otherwise, it's a notification, keep reading. } - Err(e) if e.kind() == std::io::ErrorKind::TimedOut => { + Err(error) + if error + .downcast_ref::() + .is_some_and(|io_error| { + io_error.kind() == std::io::ErrorKind::TimedOut + }) => + { thread::sleep(Duration::from_millis(100)); - continue; } Err(e) => { - panic!("Failed to read LSP response: {}", e); + return Err(anyhow::anyhow!("Failed to read LSP response: {}", e)); } } } - panic!("Did not receive response with id {}", expected_id); + Err(anyhow::anyhow!( + "Did not receive response with id {}", + expected_id + )) } /// Initialize the LSP server with the given workspace root. - pub fn initialize(&mut self, root_uri: &str) -> serde_json::Value { + pub fn initialize(&mut self, root_uri: &str) -> anyhow::Result { let params = serde_json::json!({ "processId": null, "rootUri": root_uri, @@ -159,7 +171,7 @@ impl LspClient { } }); - let request_id = self.send_request("initialize", params); + let request_id = self.send_request("initialize", params)?; self.read_response(request_id) } @@ -183,20 +195,20 @@ impl LspClient { } /// Request document symbols for a file. - pub fn document_symbol(&mut self, uri: &str) -> serde_json::Value { + pub fn document_symbol(&mut self, uri: &str) -> anyhow::Result { let params = serde_json::json!({ "textDocument": { "uri": uri } }); - let request_id = self.send_request("textDocument/documentSymbol", params); + let request_id = self.send_request("textDocument/documentSymbol", params)?; self.read_response(request_id) } /// Shutdown the LSP server. - pub fn shutdown(&mut self) -> serde_json::Value { - let request_id = self.send_request("shutdown", serde_json::json!(null)); + pub fn shutdown(&mut self) -> anyhow::Result { + let request_id = self.send_request("shutdown", serde_json::json!(null))?; self.read_response(request_id) } } From 8f22d7478755425abdfdd95f21530fe052223464 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 13:30:22 +0200 Subject: [PATCH 13/54] fix: fix and/or silence visibility warnings Co-authored-by: Copilot --- src/child.rs | 8 ++++++-- src/control.rs | 8 ++++++++ src/lib.rs | 8 ++++---- src/output.rs | 20 ++++++++++++++++++++ src/servers/health.rs | 2 +- src/servers/mod.rs | 22 +++++++++++++++++++--- src/servers/protocol.rs | 2 +- src/servers/stderr.rs | 2 +- tests/lsp_client.rs | 5 +++++ tests/test_utils.rs | 5 +++++ 10 files changed, 70 insertions(+), 12 deletions(-) diff --git a/src/child.rs b/src/child.rs index 542e554..934dd62 100644 --- a/src/child.rs +++ b/src/child.rs @@ -3,7 +3,11 @@ use subprocess::{Exec, Job, Redirection}; /// A spawned child process with captured `stdin`, `stdout`, and `stderr` streams. -pub struct StartedChild { +#[expect( + clippy::redundant_pub_crate, + reason = "Linting conflict with `rustc::unreachable_pub`." +)] +pub(crate) struct StartedChild { /// The subprocess `Job` handle for process lifecycle management. pub job: Job, /// File handle for writing to the child’s `stdin`. @@ -18,7 +22,7 @@ impl StartedChild { /// Spawns a child process with the given command and arguments. /// /// All three standard streams (`stdin`, `stdout`, `stderr`) are captured as pipes. - pub fn start(command: &str, args: &[String]) -> Result { + pub(crate) fn start(command: &str, args: &[String]) -> Result { let mut process = Exec::cmd(command); for arg in args { process = process.arg(arg); diff --git a/src/control.rs b/src/control.rs index ca81bdf..b35958e 100644 --- a/src/control.rs +++ b/src/control.rs @@ -7,6 +7,10 @@ use tracing::info; /// Messages sent to the child process coordinator to control lifecycle. #[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[expect( + clippy::redundant_pub_crate, + reason = "Linting conflict with `rustc::unreachable_pub`." +)] pub(crate) enum ControlMessage { /// Terminate the child process immediately. KillChild, @@ -16,6 +20,10 @@ pub(crate) enum ControlMessage { /// /// Polls the child process for exit and listens for control messages to kill it. /// Returns when the child process exits or is explicitly terminated. +#[expect( + clippy::redundant_pub_crate, + reason = "Linting conflict with `rustc::unreachable_pub`." +)] pub(crate) fn run_child_coordinator( job: Job, control_rx: mpsc::Receiver, diff --git a/src/lib.rs b/src/lib.rs index 60f5f81..c3aec17 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -33,7 +33,7 @@ pub mod app; /// Defines the `Args` struct with port configurations and child process command. pub mod args; -mod child; -mod control; -mod output; -mod servers; +pub(crate) mod child; +pub(crate) mod control; +pub(crate) mod output; +pub(crate) mod servers; diff --git a/src/output.rs b/src/output.rs index bfb8c73..71d2d5f 100644 --- a/src/output.rs +++ b/src/output.rs @@ -16,6 +16,10 @@ use crate::control::ControlMessage; /// Defines how output serving should handle client disconnection. #[derive(Debug, Clone)] +#[expect( + clippy::redundant_pub_crate, + reason = "Linting conflict with `rustc::unreachable_pub`." +)] pub(crate) enum ServingBehavior { /// Kill the child process when the client disconnects (protocol port behavior). KillChildOnDisconnect, @@ -26,6 +30,10 @@ pub(crate) enum ServingBehavior { } /// Buffered output state from a child process stream. +#[expect( + clippy::redundant_pub_crate, + reason = "Linting conflict with `rustc::unreachable_pub`." +)] pub(crate) struct OutputState { /// Accumulated output bytes not yet sent to clients. pub buffer: Vec, @@ -34,6 +42,10 @@ pub(crate) struct OutputState { } /// Thread-safe output state with condition variable for synchronization. +#[expect( + clippy::redundant_pub_crate, + reason = "Linting conflict with `rustc::unreachable_pub`." +)] pub(crate) struct NotifyableOutputState { /// Protected output buffer and EOF flag. pub state: Mutex, @@ -63,6 +75,10 @@ impl Default for NotifyableOutputState { /// Pumps data from the given `source` (either `stdout` or `stderr` of the child process) into the shared `state`. /// /// Continuously reads from the source and appends to the buffer, notifying waiters on each read. +#[expect( + clippy::redundant_pub_crate, + reason = "Linting conflict with `rustc::unreachable_pub`." +)] pub(crate) fn pump_output_to_state( mut source: impl Read, output_state: Arc, @@ -96,6 +112,10 @@ pub(crate) fn pump_output_to_state( /// /// Waits for output to become available, then writes it to the TCP stream. /// Handles disconnection according to the specified `serving_behavior`. +#[expect( + clippy::redundant_pub_crate, + reason = "Linting conflict with `rustc::unreachable_pub`." +)] pub(crate) fn serve_output_on_stream( mut stream: TcpStream, output_state: Arc, diff --git a/src/servers/health.rs b/src/servers/health.rs index 9f671b0..9a56911 100644 --- a/src/servers/health.rs +++ b/src/servers/health.rs @@ -6,7 +6,7 @@ use tracing::warn; /// Waits for clients to connect on the `health` port, and immediately drops any connections. The existence /// of a successful connection is used by the client as a health check for whether the process is alive. -pub fn health_server(listener: TcpListener) -> Result<(), anyhow::Error> { +pub(crate) fn health_server(listener: TcpListener) -> Result<(), anyhow::Error> { for stream in listener.incoming() { match stream { Ok(_stream) => { diff --git a/src/servers/mod.rs b/src/servers/mod.rs index 732f3e0..9da26aa 100644 --- a/src/servers/mod.rs +++ b/src/servers/mod.rs @@ -1,3 +1,19 @@ -pub mod health; -pub mod protocol; -pub mod stderr; +//! TCP server implementations for protocol, stderr, and health endpoints. + +#[expect( + clippy::redundant_pub_crate, + reason = "Linting conflict with `rustc::unreachable_pub`." +)] +pub(crate) mod health; + +#[expect( + clippy::redundant_pub_crate, + reason = "Linting conflict with `rustc::unreachable_pub`." +)] +pub(crate) mod protocol; + +#[expect( + clippy::redundant_pub_crate, + reason = "Linting conflict with `rustc::unreachable_pub`." +)] +pub(crate) mod stderr; diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index 9bf9ac7..268f796 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -53,7 +53,7 @@ fn forward_stream_data_to_child_process( /// client and the child process. This function spawns two threads: one for forwarding data from /// the client to the child process’s `stdin`, and another for forwarding data from the child /// process’s `stdout` to the client. -pub fn protocol_server( +pub(crate) fn protocol_server( listener: TcpListener, stdout_state: Arc, child_stdin: std::fs::File, diff --git a/src/servers/stderr.rs b/src/servers/stderr.rs index 1fe16d4..01e0b35 100644 --- a/src/servers/stderr.rs +++ b/src/servers/stderr.rs @@ -51,7 +51,7 @@ fn monitor_stderr_client_connection( /// to the first client that connects. If that client disconnects, we wait for the next client to connect /// and serve the current `stderr` output to them instead, and so on. The function spawns two threads /// for each client connection: one for monitoring disconnection and one for writing output. -pub fn stderr_server( +pub(crate) fn stderr_server( listener: TcpListener, stderr_state: Arc, control_tx: mpsc::Sender, diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index c0d31c2..f430ad1 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -1,5 +1,10 @@ //! Test utilities for LSP client communication. +#![allow( + unreachable_pub, + reason = "Private test module, but items need `pub` for parent access" +)] + use std::{ io::{Read, Write}, net::TcpStream, diff --git a/tests/test_utils.rs b/tests/test_utils.rs index 00b916c..fae7893 100644 --- a/tests/test_utils.rs +++ b/tests/test_utils.rs @@ -7,6 +7,11 @@ //! The goal is to make integration tests work on all platforms without #[cfg(not(windows))] //! guards scattered throughout the test code. +#![allow( + unreachable_pub, + reason = "Private test module, but items need `pub` for parent access" +)] + // Acknowledge available dev-dependencies not used in this test file. use anyhow as _; use clap as _; From 657a66723f9e11d3d8fee20129b096603e7e771a Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 13:35:00 +0200 Subject: [PATCH 14/54] fix: use unused variable --- tests/integration_test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_test.rs b/tests/integration_test.rs index e42fdb8..ceb5304 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -479,7 +479,7 @@ fn test_default_port_values() -> Result<(), anyhow::Error> { ); drop( TcpListener::bind(format!("127.0.0.1:{port}")) - .map_err(|error| anyhow::anyhow!("{error}"))?, + .map_err(|error| anyhow::anyhow!("{error_message}: {error}"))?, ); // Port is immediately released here. } From f09f21438f77a84f6b0017fb1f79c0df7a987445 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 13:35:24 +0200 Subject: [PATCH 15/54] fix: add `#[must_use]` and explicitly `drop()` unused results Co-authored-by: Copilot --- tests/lsp_client.rs | 4 ++-- tests/test_utils.rs | 31 ++++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index f430ad1..efd18df 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -182,7 +182,7 @@ impl LspClient { /// Send the initialized notification. pub fn initialized(&mut self) { - self.send_notification("initialized", serde_json::json!({})); + drop(self.send_notification("initialized", serde_json::json!({}))); } /// Open a document. @@ -196,7 +196,7 @@ impl LspClient { } }); - self.send_notification("textDocument/didOpen", params); + drop(self.send_notification("textDocument/didOpen", params)); } /// Request document symbols for a file. diff --git a/tests/test_utils.rs b/tests/test_utils.rs index fae7893..7dd8fd7 100644 --- a/tests/test_utils.rs +++ b/tests/test_utils.rs @@ -24,6 +24,7 @@ use tracing_subscriber as _; /// Returns a command that sleeps for the specified number of seconds. #[cfg(windows)] +#[must_use] pub fn sleep_cmd(seconds: u32) -> (&'static str, Vec) { // Use ping as a sleep alternative on Windows // Pings localhost N+1 times with 1-second intervals (approximately N seconds total) @@ -36,6 +37,7 @@ pub fn sleep_cmd(seconds: u32) -> (&'static str, Vec) { /// Returns a command that sleeps for the specified number of seconds. #[cfg(not(windows))] +#[must_use] pub fn sleep_cmd(seconds: u32) -> (&'static str, Vec) { ("sleep", vec![seconds.to_string()]) } @@ -55,11 +57,12 @@ pub fn echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Vec (&'static str, Vec) { ( "bash", vec![ - "-c".to_string(), + "-c".to_owned(), format!("echo '{}' && sleep {}", text, seconds), ], ) @@ -67,6 +70,7 @@ pub fn echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Vec (&'static str, Vec) { let pings = seconds + 1; ( @@ -80,6 +84,7 @@ pub fn stderr_echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Ve /// Returns a command that echoes text to `stderr`, then sleeps. #[cfg(not(windows))] +#[must_use] pub fn stderr_echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Vec) { ( "bash", @@ -92,6 +97,7 @@ pub fn stderr_echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Ve /// Returns a command that echoes to `stderr`, sleeps, echoes again, then sleeps more. #[cfg(windows)] +#[must_use] pub fn multi_echo_stderr_cmd( buffered: &str, sleep1: f32, @@ -115,6 +121,7 @@ pub fn multi_echo_stderr_cmd( /// Returns a command that echoes to `stderr`, sleeps, echoes again, then sleeps more. #[cfg(not(windows))] +#[must_use] pub fn multi_echo_stderr_cmd( buffered: &str, sleep1: f32, @@ -135,6 +142,7 @@ pub fn multi_echo_stderr_cmd( /// Returns a command that echoes to `stdout`, sleeps, echoes again, then sleeps more. #[cfg(windows)] +#[must_use] pub fn multi_echo_stdout_cmd( buffered: &str, sleep1: f32, @@ -158,6 +166,7 @@ pub fn multi_echo_stdout_cmd( /// Returns a command that echoes to `stdout`, sleeps, echoes again, then sleeps more. #[cfg(not(windows))] +#[must_use] pub fn multi_echo_stdout_cmd( buffered: &str, sleep1: f32, @@ -178,6 +187,7 @@ pub fn multi_echo_stdout_cmd( /// Returns a command that reads from `stdin` and echoes to `stdout` (like `cat`). #[cfg(windows)] +#[must_use] pub fn cat_cmd() -> (&'static str, Vec) { // Use Python for reliable line-by-line I/O on Windows. // `-u` flag disables buffering for immediate output. @@ -193,12 +203,14 @@ pub fn cat_cmd() -> (&'static str, Vec) { /// Returns a command that reads from `stdin` and echoes to `stdout` (like `cat`). #[cfg(not(windows))] +#[must_use] pub fn cat_cmd() -> (&'static str, Vec) { ("cat", vec![]) } /// Returns a command that continuously reads from `stdin` and writes "response" to `stdout`. #[cfg(windows)] +#[must_use] pub fn loop_stdin_to_stdout_cmd() -> (&'static str, Vec) { // PowerShell script that reads line by line and echoes // Use [Console]::In to read from `stdin` and [Console]::WriteLine() for immediate flushing @@ -215,6 +227,7 @@ pub fn loop_stdin_to_stdout_cmd() -> (&'static str, Vec) { /// Returns a command that continuously reads from `stdin` and writes "response" to `stdout`. #[cfg(not(windows))] +#[must_use] pub fn loop_stdin_to_stdout_cmd() -> (&'static str, Vec) { ( "bash", @@ -227,6 +240,7 @@ pub fn loop_stdin_to_stdout_cmd() -> (&'static str, Vec) { /// Returns a command that continuously writes "error" to `stderr` in a loop. #[cfg(windows)] +#[must_use] pub fn continuous_stderr_loop_cmd() -> (&'static str, Vec) { ( "powershell", @@ -241,6 +255,7 @@ pub fn continuous_stderr_loop_cmd() -> (&'static str, Vec) { /// Returns a command that continuously writes "error" to `stderr` in a loop. #[cfg(not(windows))] +#[must_use] pub fn continuous_stderr_loop_cmd() -> (&'static str, Vec) { ( "bash", @@ -253,6 +268,7 @@ pub fn continuous_stderr_loop_cmd() -> (&'static str, Vec) { /// Returns a command that generates a large block of output (repeated 'A' characters). #[cfg(windows)] +#[must_use] pub fn generate_large_output_cmd(size: usize) -> (&'static str, Vec) { // Generate large output using PowerShell ( @@ -267,6 +283,7 @@ pub fn generate_large_output_cmd(size: usize) -> (&'static str, Vec) { /// Returns a command that generates a large block of output (repeated 'A' characters). #[cfg(not(windows))] +#[must_use] pub fn generate_large_output_cmd(size: usize) -> (&'static str, Vec) { ( "bash", @@ -279,6 +296,7 @@ pub fn generate_large_output_cmd(size: usize) -> (&'static str, Vec) { /// Returns a command that outputs numbered lines to both `stdout` and `stderr` with delays. #[cfg(windows)] +#[must_use] pub fn numbered_output_loop_cmd(count: u32, interval_ms: u32) -> (&'static str, Vec) { ( "powershell", @@ -295,6 +313,7 @@ pub fn numbered_output_loop_cmd(count: u32, interval_ms: u32) -> (&'static str, /// Returns a command that outputs numbered lines to both `stdout` and `stderr` with delays. #[cfg(not(windows))] +#[must_use] pub fn numbered_output_loop_cmd(count: u32, interval_ms: u32) -> (&'static str, Vec) { let interval_sec = interval_ms as f32 / 1000.0; ( @@ -310,6 +329,7 @@ pub fn numbered_output_loop_cmd(count: u32, interval_ms: u32) -> (&'static str, /// Returns a command that emits timed `stderr` output for testing reconnection scenarios. #[cfg(windows)] +#[must_use] pub fn complex_stderr_reconnect_cmd() -> (&'static str, Vec) { ( "powershell", @@ -329,6 +349,7 @@ pub fn complex_stderr_reconnect_cmd() -> (&'static str, Vec) { /// Returns a command that emits timed `stderr` output for testing reconnection scenarios. #[cfg(not(windows))] +#[must_use] pub fn complex_stderr_reconnect_cmd() -> (&'static str, Vec) { ( "bash", @@ -348,6 +369,7 @@ pub fn complex_stderr_reconnect_cmd() -> (&'static str, Vec) { /// Returns a command that outputs to both `stdout` and `stderr`, then sleeps. #[cfg(windows)] +#[must_use] pub fn combined_output_cmd( stdout_msg: &str, stderr_msg: &str, @@ -369,6 +391,7 @@ pub fn combined_output_cmd( /// Returns a command that outputs to both `stdout` and `stderr`, then sleeps. #[cfg(not(windows))] +#[must_use] pub fn combined_output_cmd( stdout_msg: &str, stderr_msg: &str, @@ -388,6 +411,7 @@ pub fn combined_output_cmd( /// Returns a command that echoes all provided arguments to `stdout`. #[cfg(windows)] +#[must_use] pub fn echo_args_cmd(args: &[&str]) -> (&'static str, Vec) { let mut cmd_args = vec!["/C".to_string()]; // Use echo %* to print all arguments on Windows (requires a batch context) @@ -399,6 +423,7 @@ pub fn echo_args_cmd(args: &[&str]) -> (&'static str, Vec) { /// Returns a command that echoes all provided arguments to `stdout`. #[cfg(not(windows))] +#[must_use] pub fn echo_args_cmd(args: &[&str]) -> (&'static str, Vec) { let mut script_args = vec![ "-c".to_string(), @@ -411,6 +436,7 @@ pub fn echo_args_cmd(args: &[&str]) -> (&'static str, Vec) { /// Returns a command that outputs a message and exits quickly after a brief delay. #[cfg(windows)] +#[must_use] pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) { ( "powershell", @@ -427,6 +453,7 @@ pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) /// Returns a command that outputs a message and exits quickly after a brief delay. #[cfg(not(windows))] +#[must_use] pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) { let sleep_sec = sleep_ms as f32 / 1000.0; ( @@ -440,12 +467,14 @@ pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) /// Returns the platform-specific Python command name. #[cfg(windows)] +#[must_use] pub fn python_cmd() -> &'static str { "python" } /// Returns the platform-specific Python command name. #[cfg(not(windows))] +#[must_use] pub fn python_cmd() -> &'static str { "python3" } From e4477d462d6dccb8a73c1ec200916db152834665 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 13:38:22 +0200 Subject: [PATCH 16/54] fix: allow `assert`s in tests returning a `Result` --- src/lib.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index c3aec17..f3ed91d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,6 +25,13 @@ unused_crate_dependencies, reason = "dev-dependencies available to lib tests" )] +#![cfg_attr( + test, + allow( + clippy::panic_in_result_fn, + reason = "Using `assert!()`s is idiomatic, but we need to return `Result`s to be able to return I/O-related errors." + ) +)] pub mod app; From b861841c83e2baa11440ba05e1f8ee90007c124b Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 13:53:54 +0200 Subject: [PATCH 17/54] fix: avoid `panic` on index access Co-authored-by: Copilot --- tests/integration_test.rs | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/tests/integration_test.rs b/tests/integration_test.rs index ceb5304..fae60fb 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -1261,9 +1261,17 @@ fn test_lsp_rust_analyzer_integration() -> Result<(), anyhow::Error> { let init_response = lsp.initialize(&root_uri)?; - assert_eq!(init_response["jsonrpc"], "2.0"); + assert_eq!( + init_response + .get("jsonrpc") + .ok_or_else(|| anyhow::anyhow!("Expected 'jsonrpc' field in initialize response"))?, + "2.0" + ); assert!( - init_response["result"]["capabilities"].is_object(), + init_response + .get("result") + .and_then(|result| result.get("capabilities")) + .is_some_and(serde_json::Value::is_object), "Should receive server capabilities" ); @@ -1281,11 +1289,19 @@ fn test_lsp_rust_analyzer_integration() -> Result<(), anyhow::Error> { // Request document symbols. let symbols_response = lsp.document_symbol(&main_rs_uri)?; - assert_eq!(symbols_response["jsonrpc"], "2.0"); + assert_eq!( + symbols_response + .get("jsonrpc") + .ok_or_else(|| anyhow::anyhow!( + "Expected 'jsonrpc' field in document symbol response" + ))?, + "2.0" + ); // Verify we got some symbols (src/main.rs should have at least the main function). - let symbols = symbols_response["result"] - .as_array() + let symbols = symbols_response + .get("result") + .and_then(|result| result.as_array()) .ok_or_else(|| anyhow::anyhow!("Expected array of symbols"))?; assert!( @@ -1302,7 +1318,12 @@ fn test_lsp_rust_analyzer_integration() -> Result<(), anyhow::Error> { // Shutdown the LSP server. let shutdown_response = lsp.shutdown()?; - assert_eq!(shutdown_response["result"], serde_json::Value::Null); + assert_eq!( + shutdown_response + .get("result") + .ok_or_else(|| anyhow::anyhow!("Expected 'result' field in shutdown response"))?, + &serde_json::Value::Null + ); // Exit notification is sent automatically when lsp is dropped. drop(lsp); From 39717dcdecdf573a525c6553a1f02bf4db9106a0 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 13:56:34 +0200 Subject: [PATCH 18/54] docs: add returned errors to doc comments Co-authored-by: Copilot --- tests/lsp_client.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index efd18df..581f549 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -30,6 +30,10 @@ pub struct LspClient { impl LspClient { /// Create a new LSP client from a TCP stream. + /// + /// # Errors + /// + /// Returns an error if setting read or write timeouts on the stream fails. pub fn new(stream: TcpStream) -> Result { // Set reasonable timeouts for LSP communication. stream @@ -163,6 +167,10 @@ impl LspClient { } /// Initialize the LSP server with the given workspace root. + /// + /// # Errors + /// + /// Returns an error if the initialize request fails to send or the response cannot be read. pub fn initialize(&mut self, root_uri: &str) -> anyhow::Result { let params = serde_json::json!({ "processId": null, @@ -200,6 +208,10 @@ impl LspClient { } /// Request document symbols for a file. + /// + /// # Errors + /// + /// Returns an error if the document symbol request fails to send or the response cannot be read. pub fn document_symbol(&mut self, uri: &str) -> anyhow::Result { let params = serde_json::json!({ "textDocument": { @@ -212,6 +224,10 @@ impl LspClient { } /// Shutdown the LSP server. + /// + /// # Errors + /// + /// Returns an error if the shutdown request fails to send or the response cannot be read. pub fn shutdown(&mut self) -> anyhow::Result { let request_id = self.send_request("shutdown", serde_json::json!(null))?; self.read_response(request_id) From 127b0a64330f889a3c56b7abb70daea350fac8db Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 14:08:12 +0200 Subject: [PATCH 19/54] fix: avoid more `panic`s and satisfy significant `drop` warning by Clippy Co-authored-by: Copilot --- src/output.rs | 28 ++++++++++++++++++---------- tests/integration_test.rs | 8 ++++---- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/output.rs b/src/output.rs index 71d2d5f..744c01b 100644 --- a/src/output.rs +++ b/src/output.rs @@ -215,30 +215,38 @@ mod tests { use std::io::Cursor; #[test] - fn test_pump_output_to_state_empty_input() { + fn test_pump_output_to_state_empty_input() -> Result<(), anyhow::Error> { let state = Arc::new(NotifyableOutputState::new()); let input = Cursor::new(Vec::::new()); - let result = pump_output_to_state(input, Arc::clone(&state), "test"); - assert!(result.is_ok()); + pump_output_to_state(input, Arc::clone(&state), "test")?; - let guard = state.state.lock().unwrap(); + let guard = state + .state + .lock() + .map_err(|error| anyhow::anyhow!("Failed to lock output state for test: {error}"))?; assert!(guard.buffer.is_empty()); assert!(guard.eof); + drop(guard); // Only needed to satisfy Clippy ¯\_(ツ)_/¯ + Ok(()) } #[test] - fn test_pump_output_to_state_single_chunk() { + fn test_pump_output_to_state_single_chunk() -> Result<(), anyhow::Error> { let state = Arc::new(NotifyableOutputState::new()); let data = b"Hello, World!"; let input = Cursor::new(data.to_vec()); - let result = pump_output_to_state(input, Arc::clone(&state), "test"); - assert!(result.is_ok()); + pump_output_to_state(input, Arc::clone(&state), "test")?; - let guard = state.state.lock().unwrap(); + let guard = state + .state + .lock() + .map_err(|error| anyhow::anyhow!("Failed to lock output state for test: {error}"))?; assert_eq!(guard.buffer, data); assert!(guard.eof); + drop(guard); // Only needed to satisfy Clippy ¯\_(ツ)_/¯ + Ok(()) } #[test] @@ -247,8 +255,7 @@ mod tests { let data = vec![0u8; 16384]; // Larger than buffer size (8192). let input = Cursor::new(data.clone()); - let result = pump_output_to_state(input, Arc::clone(&state), "test"); - assert!(result.is_ok()); + pump_output_to_state(input, Arc::clone(&state), "test")?; let guard = state .state @@ -256,6 +263,7 @@ mod tests { .map_err(|error| anyhow::anyhow!("Failed to lock output state for test: {error}"))?; assert_eq!(guard.buffer, data); assert!(guard.eof); + drop(guard); // Only needed to satisfy Clippy ¯\_(ツ)_/¯ Ok(()) } } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index fae60fb..be861c8 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -366,7 +366,7 @@ fn test_forwarder_starts_arbitrary_child_process() -> Result<(), anyhow::Error> // If we got here, the forwarder started successfully. // The forwarder should be ready (health port should be accessible). - assert!(forwarder.connect_health()?.peer_addr().is_ok()); + forwarder.connect_health()?.peer_addr()?; Ok(()) } @@ -980,11 +980,11 @@ fn test_health_port_indicates_readiness() -> Result<(), anyhow::Error> { // If we can connect to health port, the forwarder is ready. let health_stream = forwarder.connect_health()?; - assert!(health_stream.peer_addr().is_ok()); + health_stream.peer_addr()?; // And the other ports should also be accessible. - assert!(forwarder.connect_protocol()?.peer_addr().is_ok()); - assert!(forwarder.connect_stderr()?.peer_addr().is_ok()); + forwarder.connect_protocol()?.peer_addr()?; + forwarder.connect_stderr()?.peer_addr()?; Ok(()) } From 9081f7ba4495cacdb81c4b26317ddfa2cdafaea1 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 14:27:46 +0200 Subject: [PATCH 20/54] fix: fix warnings caused by too long qualified names --- src/child.rs | 8 +++++--- src/control.rs | 4 ++-- src/output.rs | 4 ++-- src/servers/protocol.rs | 9 +++------ tests/integration_test.rs | 40 +++++++++++++++++---------------------- tests/lsp_client.rs | 8 ++++---- 6 files changed, 33 insertions(+), 40 deletions(-) diff --git a/src/child.rs b/src/child.rs index 934dd62..2daeb6b 100644 --- a/src/child.rs +++ b/src/child.rs @@ -1,5 +1,7 @@ //! Child process spawning and stream capture. +use std::fs; + use subprocess::{Exec, Job, Redirection}; /// A spawned child process with captured `stdin`, `stdout`, and `stderr` streams. @@ -11,11 +13,11 @@ pub(crate) struct StartedChild { /// The subprocess `Job` handle for process lifecycle management. pub job: Job, /// File handle for writing to the child’s `stdin`. - pub stdin: std::fs::File, + pub stdin: fs::File, /// File handle for reading from the child’s `stdout`. - pub stdout: std::fs::File, + pub stdout: fs::File, /// File handle for reading from the child’s `stderr`. - pub stderr: std::fs::File, + pub stderr: fs::File, } impl StartedChild { diff --git a/src/control.rs b/src/control.rs index b35958e..4c4268a 100644 --- a/src/control.rs +++ b/src/control.rs @@ -1,6 +1,6 @@ //! Child process lifecycle coordination and control messages. -use std::sync::mpsc; +use std::{sync::mpsc, time::Duration}; use subprocess::Job; use tracing::info; @@ -34,7 +34,7 @@ pub(crate) fn run_child_coordinator( return Ok(()); } - match control_rx.recv_timeout(std::time::Duration::from_millis(100)) { + match control_rx.recv_timeout(Duration::from_millis(100)) { Ok(ControlMessage::KillChild) => { drop(job.kill()); let status = job.wait()?; diff --git a/src/output.rs b/src/output.rs index 744c01b..166feb7 100644 --- a/src/output.rs +++ b/src/output.rs @@ -1,7 +1,7 @@ //! Output buffering and stream serving logic. use std::{ - io::{Read, Write}, + io::{self, Read, Write}, net::TcpStream, sync::{ Arc, Condvar, Mutex, @@ -160,7 +160,7 @@ pub(crate) fn serve_output_on_stream( Ok(n) => { num_bytes_written += n; } - Err(e) if e.kind() == std::io::ErrorKind::Interrupted => { + Err(e) if e.kind() == io::ErrorKind::Interrupted => { // Interrupted by a signal, just retry. continue; } diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index 268f796..2683ac7 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -1,10 +1,7 @@ //! Protocol TCP server for bidirectional `stdin`/`stdout` forwarding. use std::{ - io::{Read, Write}, - net::{TcpListener, TcpStream}, - sync::{Arc, mpsc}, - thread, + fs, io::{Read, Write}, net::{TcpListener, TcpStream}, sync::{Arc, mpsc}, thread }; use tracing::info; @@ -20,7 +17,7 @@ use crate::{ /// or an error occurs. Sends a kill signal on disconnection. fn forward_stream_data_to_child_process( mut stream: TcpStream, - mut child_stdin: std::fs::File, + mut child_stdin: fs::File, control_tx: mpsc::Sender, ) -> Result<(), anyhow::Error> { let mut read_buffer = [0u8; 8192]; @@ -56,7 +53,7 @@ fn forward_stream_data_to_child_process( pub(crate) fn protocol_server( listener: TcpListener, stdout_state: Arc, - child_stdin: std::fs::File, + child_stdin: fs::File, control_tx: mpsc::Sender, ) -> Result<(), anyhow::Error> { // We only accept a single (i.e., the first) client connection on the protocol port. diff --git a/tests/integration_test.rs b/tests/integration_test.rs index be861c8..68de191 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -6,13 +6,7 @@ )] use std::{ - collections::HashSet, - io::{Read, Write}, - net::{TcpListener, TcpStream}, - process::{Child, Command, Stdio}, - sync::{LazyLock, Mutex}, - thread, - time::Duration, + collections::HashSet, env, fs::read_to_string, io::{self, Read, Write}, net::{Shutdown, TcpListener, TcpStream}, process::{Child, Command, Stdio}, sync::{LazyLock, Mutex}, thread, time::{Duration, Instant} }; // Acknowledge available dev-dependencies not used in this test file. @@ -212,10 +206,10 @@ impl TestForwarder { command: &str, args: &[&str], ports: &AllocatedPorts, - ) -> std::io::Result { + ) -> io::Result { // Get the path to the `stdioxide` binary. // In integration tests, we need to use the binary from the target directory. - let bin_path = std::env::var("CARGO_BIN_EXE_stdioxide") + let bin_path = env::var("CARGO_BIN_EXE_stdioxide") .unwrap_or_else(|_| "target/debug/stdioxide".to_string()); let mut cmd = Command::new(&bin_path); @@ -326,7 +320,7 @@ impl Drop for TestForwarder { } /// Helper function to read from a stream with a timeout. -fn read_with_timeout(stream: &mut TcpStream, buffer: &mut [u8]) -> std::io::Result { +fn read_with_timeout(stream: &mut TcpStream, buffer: &mut [u8]) -> io::Result { stream.set_read_timeout(Some(Duration::from_secs(5)))?; stream.read(buffer) } @@ -343,8 +337,8 @@ fn read_all_available(stream: &mut TcpStream, timeout: Duration) -> Result break, Ok(n) => result.extend_from_slice(&buffer[..n]), - Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => break, - Err(e) if e.kind() == std::io::ErrorKind::TimedOut => break, + Err(e) if e.kind() == io::ErrorKind::WouldBlock => break, + Err(e) if e.kind() == io::ErrorKind::TimedOut => break, Err(_) => break, } } @@ -485,7 +479,7 @@ fn test_default_port_values() -> Result<(), anyhow::Error> { } // Launch `stdioxide` *without* specifying ports to verify it uses the defaults. - let bin_path = std::env::var("CARGO_BIN_EXE_stdioxide") + let bin_path = env::var("CARGO_BIN_EXE_stdioxide") .unwrap_or_else(|_| "target/debug/stdioxide".to_string()); let (sleep_command, sleep_args) = sleep_cmd(10); @@ -543,7 +537,7 @@ fn test_port_override_via_environment_variables() -> Result<(), anyhow::Error> { let custom_health = ports.health_port(); // Launch `stdioxide` with environment variables (NOT command-line args) to test env var override. - let bin_path = std::env::var("CARGO_BIN_EXE_stdioxide") + let bin_path = env::var("CARGO_BIN_EXE_stdioxide") .unwrap_or_else(|_| "target/debug/stdioxide".to_string()); let (sleep_command, sleep_args) = sleep_cmd(10); @@ -709,7 +703,7 @@ fn test_protocol_port_single_client_only() -> Result<(), anyhow::Error> { result.is_err() && matches!( result.as_ref().unwrap_err().kind(), - std::io::ErrorKind::WouldBlock | std::io::ErrorKind::TimedOut + io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut ), "Second client should timeout reading (connection never served by protocol server)" ); @@ -901,7 +895,7 @@ fn test_stderr_port_reconnect_continues_from_current_state() -> Result<(), anyho assert!(output_str.contains("before_connection")); assert!(output_str.contains("during_first_connection")); // Disconnect now (at ~t=1.0s), before "trigger_disconnect" (at t=1.5s) - drop(stream.shutdown(std::net::Shutdown::Both)); + drop(stream.shutdown(Shutdown::Both)); drop(stream); assert!( !output_str.contains("trigger_disconnect"), @@ -1001,7 +995,7 @@ fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { // Spawn a thread to continuously perform health checks for 3.5 seconds. let health_port = forwarder.ports.health_port(); let health_check_handle = thread::spawn(move || { - let start = std::time::Instant::now(); + let start = Instant::now(); let mut check_count = 0; while start.elapsed() < Duration::from_millis(3500) { if TcpStream::connect(("127.0.0.1", health_port)).is_ok() { @@ -1025,8 +1019,8 @@ fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { match protocol_stream.read(&mut buffer) { Ok(0) => break, Ok(n) => all_output.extend_from_slice(&buffer[..n]), - Err(e) if e.kind() == std::io::ErrorKind::TimedOut => break, - Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => break, + Err(e) if e.kind() == io::ErrorKind::TimedOut => break, + Err(e) if e.kind() == io::ErrorKind::WouldBlock => break, Err(_) => break, } } @@ -1046,8 +1040,8 @@ fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { match stderr_stream.read(&mut buffer) { Ok(0) => break, Ok(n) => all_output.extend_from_slice(&buffer[..n]), - Err(e) if e.kind() == std::io::ErrorKind::TimedOut => break, - Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => break, + Err(e) if e.kind() == io::ErrorKind::TimedOut => break, + Err(e) if e.kind() == io::ErrorKind::WouldBlock => break, Err(_) => break, } } @@ -1253,7 +1247,7 @@ fn test_lsp_rust_analyzer_integration() -> Result<(), anyhow::Error> { let mut lsp = LspClient::new(stream)?; // Initialize the LSP server. - let workspace_path = std::env::current_dir() + let workspace_path = env::current_dir() .map_err(|error| anyhow::anyhow!("Failed to get current directory: {error}"))? .to_string_lossy() .to_string(); @@ -1281,7 +1275,7 @@ fn test_lsp_rust_analyzer_integration() -> Result<(), anyhow::Error> { // Open a document (src/main.rs). let main_rs_path = format!("{workspace_path}/src/main.rs"); let main_rs_uri = format!("file://{main_rs_path}"); - let main_rs_content = std::fs::read_to_string(&main_rs_path) + let main_rs_content = read_to_string(&main_rs_path) .map_err(|error| anyhow::anyhow!("Failed to read src/main.rs: {error}"))?; lsp.did_open(&main_rs_uri, "rust", main_rs_content); diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index 581f549..e09bbca 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -6,7 +6,7 @@ )] use std::{ - io::{Read, Write}, + io::{self, Read, Write}, net::TcpStream, thread, time::Duration, @@ -51,7 +51,7 @@ impl LspClient { /// Send an LSP message over the stream. /// LSP uses JSON-RPC 2.0 with a Content-Length header. - fn send_message(&mut self, message: &serde_json::Value) -> std::io::Result<()> { + fn send_message(&mut self, message: &serde_json::Value) -> io::Result<()> { let json_str = serde_json::to_string(message)?; let content = format!("Content-Length: {}\r\n\r\n{}", json_str.len(), json_str); self.stream.write_all(content.as_bytes())?; @@ -148,9 +148,9 @@ impl LspClient { } Err(error) if error - .downcast_ref::() + .downcast_ref::() .is_some_and(|io_error| { - io_error.kind() == std::io::ErrorKind::TimedOut + io_error.kind() == io::ErrorKind::TimedOut }) => { thread::sleep(Duration::from_millis(100)); From 3c7a478182fe67a548b11e74ce527bce4965d2fb Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 14:30:23 +0200 Subject: [PATCH 21/54] fix: use short form of format strings Co-authored-by: Copilot --- src/servers/protocol.rs | 6 ++++- tests/integration_test.rs | 49 ++++++++++++++++++--------------------- tests/lsp_client.rs | 11 ++++----- 3 files changed, 31 insertions(+), 35 deletions(-) diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index 2683ac7..ba9c8dd 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -1,7 +1,11 @@ //! Protocol TCP server for bidirectional `stdin`/`stdout` forwarding. use std::{ - fs, io::{Read, Write}, net::{TcpListener, TcpStream}, sync::{Arc, mpsc}, thread + fs, + io::{Read, Write}, + net::{TcpListener, TcpStream}, + sync::{Arc, mpsc}, + thread, }; use tracing::info; diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 68de191..69d6f5c 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -6,7 +6,15 @@ )] use std::{ - collections::HashSet, env, fs::read_to_string, io::{self, Read, Write}, net::{Shutdown, TcpListener, TcpStream}, process::{Child, Command, Stdio}, sync::{LazyLock, Mutex}, thread, time::{Duration, Instant} + collections::HashSet, + env, + fs::read_to_string, + io::{self, Read, Write}, + net::{Shutdown, TcpListener, TcpStream}, + process::{Child, Command, Stdio}, + sync::{LazyLock, Mutex}, + thread, + time::{Duration, Instant}, }; // Acknowledge available dev-dependencies not used in this test file. @@ -187,14 +195,14 @@ impl TestForwarder { ports: AllocatedPorts, ) -> Result { let process = Self::spawn_process(command, args, &ports) - .map_err(|e| format!("Failed to spawn process: {}", e))?; + .map_err(|error| format!("Failed to spawn process: {error}"))?; let forwarder = Self { process, ports }; // Wait for the forwarder to bind to the ports forwarder .try_wait_for_ready() - .map_err(|e| format!("Failed to become ready: {}", e))?; + .map_err(|error| format!("Failed to become ready: {error}"))?; // Ports remain in the forwarder and will be released when it’s dropped. Ok(forwarder) @@ -202,11 +210,7 @@ impl TestForwarder { /// Internal helper to spawn the forwarder process. /// Returns the spawned Child process or an error if spawning fails (e.g., due to port conflicts). - fn spawn_process( - command: &str, - args: &[&str], - ports: &AllocatedPorts, - ) -> io::Result { + fn spawn_process(command: &str, args: &[&str], ports: &AllocatedPorts) -> io::Result { // Get the path to the `stdioxide` binary. // In integration tests, we need to use the binary from the target directory. let bin_path = env::var("CARGO_BIN_EXE_stdioxide") @@ -560,7 +564,7 @@ fn test_port_override_via_environment_variables() -> Result<(), anyhow::Error> { const NUM_ATTEMPTS: usize = 30; for _ in 0..NUM_ATTEMPTS { if TcpStream::connect_timeout( - &format!("127.0.0.1:{}", custom_health).parse().unwrap(), + &format!("127.0.0.1:{custom_health}").parse()?, Duration::from_millis(100), ) .is_ok() @@ -572,8 +576,7 @@ fn test_port_override_via_environment_variables() -> Result<(), anyhow::Error> { } assert!( connected, - "Forwarder should be ready on custom health port {}", - custom_health + "Forwarder should be ready on custom health port {custom_health}", ); // Verify we can connect to all three custom ports. @@ -919,20 +922,17 @@ fn test_stderr_port_reconnect_continues_from_current_state() -> Result<(), anyho assert!( output_str.contains("trigger_disconnect"), - "Second client should receive 'trigger_disconnect' (buffered during disconnect), got: {}", - output_str + "Second client should receive 'trigger_disconnect' (buffered during disconnect), got: {output_str}", ); assert!( output_str.contains("while_disconnected"), - "Second client should receive 'while_disconnected' (buffered during disconnect), got: {}", - output_str + "Second client should receive 'while_disconnected' (buffered during disconnect), got: {output_str}", ); assert!( output_str.contains("during_second_connection"), - "Second client should receive realtime data, got: {}", - output_str + "Second client should receive realtime data, got: {output_str}", ); } Ok(()) @@ -1065,8 +1065,7 @@ fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { // a reasonable number of health checks occurred without interfering with data transfer. assert!( health_check_count > 20, - "Should have performed multiple health checks (got {})", - health_check_count + "Should have performed multiple health checks (got {health_check_count})", ); // Verify that we received substantial data on both ports despite constant health checks. @@ -1079,14 +1078,12 @@ fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { assert!( protocol_line_count >= 250, - "Should have received most stdout lines despite health checks (got {})", - protocol_line_count + "Should have received most stdout lines despite health checks (got {protocol_line_count})", ); assert!( stderr_line_count >= 250, - "Should have received most stderr lines despite health checks (got {})", - stderr_line_count + "Should have received most stderr lines despite health checks (got {stderr_line_count})", ); // Verify data integrity: check for a few specific lines. @@ -1191,9 +1188,7 @@ fn test_large_output_buffering() -> Result<(), anyhow::Error> { assert!( total_read >= large_size, - "Should have read at least {} bytes, got {}", - large_size, - total_read + "Should have read at least {large_size} bytes, got {total_read}", ); Ok(()) } @@ -1251,7 +1246,7 @@ fn test_lsp_rust_analyzer_integration() -> Result<(), anyhow::Error> { .map_err(|error| anyhow::anyhow!("Failed to get current directory: {error}"))? .to_string_lossy() .to_string(); - let root_uri = format!("file://{}", workspace_path); + let root_uri = format!("file://{workspace_path}"); let init_response = lsp.initialize(&root_uri)?; diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index e09bbca..ea35d29 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -149,20 +149,17 @@ impl LspClient { Err(error) if error .downcast_ref::() - .is_some_and(|io_error| { - io_error.kind() == io::ErrorKind::TimedOut - }) => + .is_some_and(|io_error| io_error.kind() == io::ErrorKind::TimedOut) => { thread::sleep(Duration::from_millis(100)); } - Err(e) => { - return Err(anyhow::anyhow!("Failed to read LSP response: {}", e)); + Err(error) => { + return Err(anyhow::anyhow!("Failed to read LSP response: {error}")); } } } Err(anyhow::anyhow!( - "Did not receive response with id {}", - expected_id + "Did not receive response with id {expected_id}" )) } From e92805998c8e65efafd1b5ba36d88c7f934cd7cf Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 14:41:14 +0200 Subject: [PATCH 22/54] fix: use and fix integer literal suffixes Co-authored-by: Copilot --- Cargo.toml | 3 +-- src/output.rs | 4 ++-- src/servers/protocol.rs | 2 +- src/servers/stderr.rs | 2 +- tests/integration_test.rs | 22 +++++++++++----------- tests/lsp_client.rs | 10 +++++----- 6 files changed, 21 insertions(+), 22 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fe40e58..4774ec7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -183,13 +183,13 @@ pub_use = "warn" ref_patterns = "warn" same_name_method = "warn" self_named_module_files = "warn" -separated_literal_suffix = "warn" std_instead_of_alloc = "warn" std_instead_of_core = "warn" str_to_string = "warn" string_add = "warn" string_to_string = "allow" # Deprecated; keep allowed if older toolchains still mention it. unnecessary_self_imports = "warn" +unseparated_literal_suffix = "warn" wildcard_enum_match_arm = "warn" ###################################################################################################################### @@ -262,5 +262,4 @@ undocumented_unsafe_blocks = "warn" unimplemented = "warn" unnecessary_safety_comment = "warn" unnecessary_safety_doc = "warn" -unseparated_literal_suffix = "warn" verbose_file_reads = "warn" diff --git a/src/output.rs b/src/output.rs index 166feb7..881e808 100644 --- a/src/output.rs +++ b/src/output.rs @@ -85,7 +85,7 @@ pub(crate) fn pump_output_to_state( label: &'static str, ) -> Result<(), anyhow::Error> { loop { - let mut buffer = [0u8; 8192]; + let mut buffer = [0_u8; 8192]; let num_bytes_read = source.read(&mut buffer)?; { let mut guard = output_state.state.lock().map_err(|error| { @@ -252,7 +252,7 @@ mod tests { #[test] fn test_pump_output_to_state_multiple_chunks() -> Result<(), anyhow::Error> { let state = Arc::new(NotifyableOutputState::new()); - let data = vec![0u8; 16384]; // Larger than buffer size (8192). + let data = vec![0_u8; 16384]; // Larger than buffer size (8192). let input = Cursor::new(data.clone()); pump_output_to_state(input, Arc::clone(&state), "test")?; diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index ba9c8dd..59715d1 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -24,7 +24,7 @@ fn forward_stream_data_to_child_process( mut child_stdin: fs::File, control_tx: mpsc::Sender, ) -> Result<(), anyhow::Error> { - let mut read_buffer = [0u8; 8192]; + let mut read_buffer = [0_u8; 8192]; loop { let num_bytes_read = match stream.read(&mut read_buffer) { Ok(0) => { diff --git a/src/servers/stderr.rs b/src/servers/stderr.rs index 01e0b35..2e38676 100644 --- a/src/servers/stderr.rs +++ b/src/servers/stderr.rs @@ -25,7 +25,7 @@ fn monitor_stderr_client_connection( mut stream: TcpStream, has_active_connection: Arc, ) -> Result<(), anyhow::Error> { - let mut read_buffer = [0u8; 1]; + let mut read_buffer = [0_u8; 1]; loop { match stream.read(&mut read_buffer) { Ok(0) => { diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 69d6f5c..6036c11 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -332,7 +332,7 @@ fn read_with_timeout(stream: &mut TcpStream, buffer: &mut [u8]) -> io::Result Result, anyhow::Error> { let mut result = Vec::new(); - let mut buffer = [0u8; 8192]; + let mut buffer = [0_u8; 8192]; stream .set_read_timeout(Some(timeout)) .map_err(|error| anyhow::anyhow!("Failed to set read timeout: {error}"))?; @@ -471,7 +471,7 @@ fn test_default_port_values() -> Result<(), anyhow::Error> { // Test setup: Ensure default ports are available. // If they’re not, this is an environment issue, not a test failure. - for port in 7000..=7002 { + for port in 7000_u16..=7002_u16 { let error_message = format!( "TEST SETUP FAILED: Default port {port} is not available. This is an environment issue, not a test failure." ); @@ -517,7 +517,7 @@ fn test_default_port_values() -> Result<(), anyhow::Error> { ); // Verify we can connect to all three default ports. - for port in 7000..=7002 { + for port in 7000_u16..=7002_u16 { assert!( TcpStream::connect(format!("127.0.0.1:{port}")).is_ok(), "Should connect to default port {port}" @@ -699,7 +699,7 @@ fn test_protocol_port_single_client_only() -> Result<(), anyhow::Error> { .set_read_timeout(Some(Duration::from_millis(200))) .map_err(|error| anyhow::anyhow!("Should set read timeout: {error}"))?; - let mut buf = [0u8; 100]; + let mut buf = [0_u8; 100]; let result = stream2.read(&mut buf); assert!( @@ -996,10 +996,10 @@ fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { let health_port = forwarder.ports.health_port(); let health_check_handle = thread::spawn(move || { let start = Instant::now(); - let mut check_count = 0; + let mut check_count = 0_usize; while start.elapsed() < Duration::from_millis(3500) { if TcpStream::connect(("127.0.0.1", health_port)).is_ok() { - check_count += 1; + check_count += 1_usize; } thread::sleep(Duration::from_millis(20)); } @@ -1010,7 +1010,7 @@ fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { let mut protocol_stream = forwarder.connect_protocol()?; let protocol_handle = thread::spawn(move || { let mut all_output = Vec::new(); - let mut buffer = [0u8; 8192]; + let mut buffer = [0_u8; 8192]; protocol_stream .set_read_timeout(Some(Duration::from_secs(4))) .ok(); @@ -1031,7 +1031,7 @@ fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { let mut stderr_stream = forwarder.connect_stderr()?; let stderr_handle = thread::spawn(move || { let mut all_output = Vec::new(); - let mut buffer = [0u8; 8192]; + let mut buffer = [0_u8; 8192]; stderr_stream .set_read_timeout(Some(Duration::from_secs(4))) .ok(); @@ -1175,8 +1175,8 @@ fn test_large_output_buffering() -> Result<(), anyhow::Error> { // Connect and read the buffered output. let mut stream = forwarder.connect_protocol()?; - let mut total_read = 0; - let mut buffer = [0u8; 8192]; + let mut total_read = 0_usize; + let mut buffer = [0_u8; 8192]; while total_read < large_size { match read_with_timeout(&mut stream, &mut buffer) { @@ -1204,7 +1204,7 @@ fn test_concurrent_stdin_stdout_bidirectional() -> Result<(), anyhow::Error> { let mut stream = forwarder.connect_protocol()?; // Send multiple lines and verify echo. - for i in 0..5 { + for i in 0_usize..5_usize { let message = format!("line {i}\n"); stream .write_all(message.as_bytes()) diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index ea35d29..79d45ae 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -64,7 +64,7 @@ impl LspClient { fn read_message(&mut self) -> anyhow::Result { // Read the Content-Length header. let mut header = String::new(); - let mut buffer = [0u8; 1]; + let mut buffer = [0_u8; 1]; // Read until we find "\r\n\r\n" loop { @@ -88,7 +88,7 @@ impl LspClient { .ok_or_else(|| anyhow::anyhow!("Missing Content-Length"))?; // Read the JSON content. - let mut content = vec![0u8; content_length]; + let mut content = vec![0_u8; content_length]; self.stream.read_exact(&mut content)?; // Parse JSON. @@ -103,7 +103,7 @@ impl LspClient { params: serde_json::Value, ) -> Result { let request_id = self.next_request_id; - self.next_request_id += 1; + self.next_request_id += 1_i32; let request = serde_json::json!({ "jsonrpc": "2.0", @@ -137,7 +137,7 @@ impl LspClient { /// Read responses until we get a response with the specified ID. /// Skips notifications that may arrive in between. fn read_response(&mut self, expected_id: i32) -> anyhow::Result { - for _ in 0..20 { + for _ in 0_usize..20_usize { match self.read_message() { Ok(msg) => { // Check if this is our response. @@ -196,7 +196,7 @@ impl LspClient { "textDocument": { "uri": uri, "languageId": language_id, - "version": 1, + "version": 1_i32, "text": text } }); From 892fbc9ed480be05a81fba41815b2f9c955ff355 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 14:50:50 +0200 Subject: [PATCH 23/54] fix: remove silent `as` casts Co-authored-by: Copilot --- tests/lsp_client.rs | 2 +- tests/test_utils.rs | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index 79d45ae..87998c0 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -69,7 +69,7 @@ impl LspClient { // Read until we find "\r\n\r\n" loop { self.stream.read_exact(&mut buffer)?; - header.push(buffer[0] as char); + header.push(buffer[0].into()); if header.ends_with("\r\n\r\n") { break; } diff --git a/tests/test_utils.rs b/tests/test_utils.rs index 7dd8fd7..f0e1176 100644 --- a/tests/test_utils.rs +++ b/tests/test_utils.rs @@ -315,7 +315,13 @@ pub fn numbered_output_loop_cmd(count: u32, interval_ms: u32) -> (&'static str, #[cfg(not(windows))] #[must_use] pub fn numbered_output_loop_cmd(count: u32, interval_ms: u32) -> (&'static str, Vec) { - let interval_sec = interval_ms as f32 / 1000.0; + #[expect( + clippy::integer_division, + reason = "Intentional conversion of milliseconds to seconds with fractional part" + )] + let seconds = interval_ms / 1000; + let millis = interval_ms % 1000; + let interval_sec = format!("{seconds}.{millis:03}"); ( "bash", vec![ @@ -455,12 +461,18 @@ pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) #[cfg(not(windows))] #[must_use] pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) { - let sleep_sec = sleep_ms as f32 / 1000.0; + #[expect( + clippy::integer_division, + reason = "Intentional conversion of milliseconds to seconds with fractional part" + )] + let seconds = sleep_ms / 1000; + let millis = sleep_ms % 1000; + let sleep_arg = format!("{seconds}.{millis:03}"); ( "bash", vec![ "-c".to_string(), - format!("echo {} && sleep {}", msg, sleep_sec), + format!("echo {} && sleep {}", msg, sleep_arg), ], ) } From 7d337cfa92d5e95893a6cbb15b35b9a9573fdad4 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 14:55:03 +0200 Subject: [PATCH 24/54] fix: import from `core` instead of `std` Co-authored-by: Copilot --- src/control.rs | 4 ++-- src/output.rs | 7 ++----- src/servers/stderr.rs | 7 ++----- tests/integration_test.rs | 3 ++- tests/lsp_client.rs | 2 +- 5 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/control.rs b/src/control.rs index 4c4268a..e266fe3 100644 --- a/src/control.rs +++ b/src/control.rs @@ -1,7 +1,7 @@ //! Child process lifecycle coordination and control messages. -use std::{sync::mpsc, time::Duration}; - +use core::time::Duration; +use std::sync::mpsc; use subprocess::Job; use tracing::info; diff --git a/src/output.rs b/src/output.rs index 881e808..c8eecda 100644 --- a/src/output.rs +++ b/src/output.rs @@ -1,13 +1,10 @@ //! Output buffering and stream serving logic. +use core::sync::atomic::{AtomicBool, Ordering}; use std::{ io::{self, Read, Write}, net::TcpStream, - sync::{ - Arc, Condvar, Mutex, - atomic::{AtomicBool, Ordering}, - mpsc, - }, + sync::{Arc, Condvar, Mutex, mpsc}, }; use tracing::{debug, info}; diff --git a/src/servers/stderr.rs b/src/servers/stderr.rs index 2e38676..fdbd2c3 100644 --- a/src/servers/stderr.rs +++ b/src/servers/stderr.rs @@ -1,13 +1,10 @@ //! `stderr` TCP server with reconnection support and output buffering. +use core::sync::atomic::{AtomicBool, Ordering}; use std::{ io::Read, net::{TcpListener, TcpStream}, - sync::{ - Arc, - atomic::{AtomicBool, Ordering}, - mpsc, - }, + sync::{Arc, mpsc}, thread, }; diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 6036c11..300b6cd 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -5,6 +5,7 @@ reason = "Using `assert!()`s is idiomatic, but we need to return `Result`s to be able to return I/O-related errors." )] +use core::time::Duration; use std::{ collections::HashSet, env, @@ -14,7 +15,7 @@ use std::{ process::{Child, Command, Stdio}, sync::{LazyLock, Mutex}, thread, - time::{Duration, Instant}, + time::Instant, }; // Acknowledge available dev-dependencies not used in this test file. diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index 87998c0..0575caa 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -5,11 +5,11 @@ reason = "Private test module, but items need `pub` for parent access" )] +use core::time::Duration; use std::{ io::{self, Read, Write}, net::TcpStream, thread, - time::Duration, }; // Acknowledge available dev-dependencies not used in this test file. From d82a6017b8a16ae287f27ccef323152c60a30b4d Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 14:57:39 +0200 Subject: [PATCH 25/54] fix: avoid passing by-value unnecessarily --- src/app.rs | 2 +- src/servers/stderr.rs | 6 +++--- tests/integration_test.rs | 2 +- tests/lsp_client.rs | 8 ++++---- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/app.rs b/src/app.rs index 392757b..5c18c9a 100644 --- a/src/app.rs +++ b/src/app.rs @@ -87,7 +87,7 @@ pub fn run(args: &Args) -> Result<(), anyhow::Error> { let stderr_state = Arc::clone(&stderr_state); let control_tx = control_tx.clone(); thread::spawn(move || { - drop(stderr_server(stderr_listener, stderr_state, control_tx)); + drop(stderr_server(&stderr_listener, &stderr_state, &control_tx)); }); } diff --git a/src/servers/stderr.rs b/src/servers/stderr.rs index fdbd2c3..9f9cfab 100644 --- a/src/servers/stderr.rs +++ b/src/servers/stderr.rs @@ -49,9 +49,9 @@ fn monitor_stderr_client_connection( /// and serve the current `stderr` output to them instead, and so on. The function spawns two threads /// for each client connection: one for monitoring disconnection and one for writing output. pub(crate) fn stderr_server( - listener: TcpListener, - stderr_state: Arc, - control_tx: mpsc::Sender, + listener: &TcpListener, + stderr_state: &Arc, + control_tx: &mpsc::Sender, ) -> Result<(), anyhow::Error> { // We allow reconnects on the `stderr` port, but only one client at a time. When a client disconnects, // we simply wait for the next one to connect. diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 300b6cd..2817277 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -1274,7 +1274,7 @@ fn test_lsp_rust_analyzer_integration() -> Result<(), anyhow::Error> { let main_rs_content = read_to_string(&main_rs_path) .map_err(|error| anyhow::anyhow!("Failed to read src/main.rs: {error}"))?; - lsp.did_open(&main_rs_uri, "rust", main_rs_content); + lsp.did_open(&main_rs_uri, "rust", &main_rs_content); // Request document symbols. let symbols_response = lsp.document_symbol(&main_rs_uri)?; diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index 0575caa..e1c10c5 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -121,7 +121,7 @@ impl LspClient { fn send_notification( &mut self, method: &str, - params: serde_json::Value, + params: &serde_json::Value, ) -> Result<(), anyhow::Error> { let notification = serde_json::json!({ "jsonrpc": "2.0", @@ -187,11 +187,11 @@ impl LspClient { /// Send the initialized notification. pub fn initialized(&mut self) { - drop(self.send_notification("initialized", serde_json::json!({}))); + drop(self.send_notification("initialized", &serde_json::json!({}))); } /// Open a document. - pub fn did_open(&mut self, uri: &str, language_id: &str, text: String) { + pub fn did_open(&mut self, uri: &str, language_id: &str, text: &str) { let params = serde_json::json!({ "textDocument": { "uri": uri, @@ -201,7 +201,7 @@ impl LspClient { } }); - drop(self.send_notification("textDocument/didOpen", params)); + drop(self.send_notification("textDocument/didOpen", ¶ms)); } /// Request document symbols for a file. From b93aa447bf35d541422add399ee190b284ce4079 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 15:32:20 +0200 Subject: [PATCH 26/54] fix: avoid one-digit identifiers Co-authored-by: Copilot --- src/servers/health.rs | 4 +-- src/servers/protocol.rs | 18 +++++----- src/servers/stderr.rs | 18 +++++----- tests/integration_test.rs | 75 ++++++++++++++++++++------------------- tests/test_utils.rs | 2 +- 5 files changed, 61 insertions(+), 56 deletions(-) diff --git a/src/servers/health.rs b/src/servers/health.rs index 9a56911..d13d99a 100644 --- a/src/servers/health.rs +++ b/src/servers/health.rs @@ -12,8 +12,8 @@ pub(crate) fn health_server(listener: TcpListener) -> Result<(), anyhow::Error> Ok(_stream) => { // Immediately drop it; successful connect is enough. } - Err(e) => { - warn!("[health] accept failed: {e}"); + Err(error) => { + warn!("[health] accept failed: {error}"); } } } diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index 59715d1..0b9dc10 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -33,19 +33,19 @@ fn forward_stream_data_to_child_process( return Ok(()); } Ok(n) => n, - Err(e) => { + Err(error) => { let _ = control_tx.send(ControlMessage::KillChild); - return Err(anyhow::anyhow!("Failed to read from protocol client: {e}")); + return Err(anyhow::anyhow!("Failed to read from protocol client: {error}")); } }; - if let Err(e) = child_stdin.write_all(&read_buffer[..num_bytes_read]) { + if let Err(error) = child_stdin.write_all(&read_buffer[..num_bytes_read]) { let _ = control_tx.send(ControlMessage::KillChild); - return Err(anyhow::anyhow!("Failed to write to child stdin: {e}")); + return Err(anyhow::anyhow!("Failed to write to child stdin: {error}")); } - if let Err(e) = child_stdin.flush() { + if let Err(error) = child_stdin.flush() { let _ = control_tx.send(ControlMessage::KillChild); - return Err(anyhow::anyhow!("Failed to flush child stdin: {e}")); + return Err(anyhow::anyhow!("Failed to flush child stdin: {error}")); } } } @@ -86,8 +86,10 @@ pub(crate) fn protocol_server( }), ) } - Err(e) => { - return Err(anyhow::anyhow!("Failed to accept client connection: {e}")); + Err(error) => { + return Err(anyhow::anyhow!( + "Failed to accept client connection: {error}" + )); } }; diff --git a/src/servers/stderr.rs b/src/servers/stderr.rs index 9f9cfab..751d73d 100644 --- a/src/servers/stderr.rs +++ b/src/servers/stderr.rs @@ -34,11 +34,13 @@ fn monitor_stderr_client_connection( Ok(_) => { // Ignore any data sent by client (unexpected but harmless). } - Err(e) => { + Err(error) => { // Error reading; treat as disconnection. - debug!("[stderr] read error (client likely disconnected): {e}"); + debug!("[stderr] read error (client likely disconnected): {error}"); has_active_connection.store(false, Ordering::Release); - return Err(anyhow::anyhow!("Failed to read from stderr client: {e}")); + return Err(anyhow::anyhow!( + "Failed to read from stderr client: {error}" + )); } } } @@ -67,9 +69,9 @@ pub(crate) fn stderr_server( info!("[stderr] client connected from {}", stream.peer_addr()?); let connection_monitoring_stream = match stream.try_clone() { - Ok(s) => s, - Err(e) => { - warn!("[stderr] failed to clone stream: {e}"); + Ok(stream) => stream, + Err(error) => { + warn!("[stderr] failed to clone stream: {error}"); has_active_connection.store(false, Ordering::Release); continue; } @@ -112,8 +114,8 @@ pub(crate) fn stderr_server( ); } } - Err(e) => { - warn!("[stderr] accept failed: {e}"); + Err(error) => { + warn!("[stderr] accept failed: {error}"); } } } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 2817277..b2f3ea5 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -13,6 +13,7 @@ use std::{ io::{self, Read, Write}, net::{Shutdown, TcpListener, TcpStream}, process::{Child, Command, Stdio}, + string::String, sync::{LazyLock, Mutex}, thread, time::Instant, @@ -171,8 +172,8 @@ impl TestForwarder { // Try to start with these ports. match Self::try_start_with_ports(command, args, ports) { Ok(forwarder) => return Ok(forwarder), - Err(e) => { - last_error = Some(e); + Err(error) => { + last_error = Some(error); if attempt < MAX_RETRIES - 1 { // Retry with new ports thread::sleep(Duration::from_millis(100)); @@ -252,8 +253,8 @@ impl TestForwarder { Duration::from_millis(100), ) { Ok(_) => return Ok(()), - Err(e) => { - last_error = Some(e); + Err(error) => { + last_error = Some(error); if attempt < NUM_ATTEMPTS - 1 { thread::sleep(Duration::from_millis(50)); } @@ -290,9 +291,9 @@ impl TestForwarder { for attempt in 0..NUM_ATTEMPTS { match TcpStream::connect(("127.0.0.1", port)) { Ok(stream) => return Ok(stream), - Err(e) if attempt == NUM_ATTEMPTS - 1 => { + Err(error) if attempt == NUM_ATTEMPTS - 1 => { return Err(anyhow::anyhow!( - "Failed to connect to {label} port {port} after {NUM_ATTEMPTS} attempts: {e}" + "Failed to connect to {label} port {port} after {NUM_ATTEMPTS} attempts: {error}" )); } Err(_) => { @@ -342,8 +343,8 @@ fn read_all_available(stream: &mut TcpStream, timeout: Duration) -> Result break, Ok(n) => result.extend_from_slice(&buffer[..n]), - Err(e) if e.kind() == io::ErrorKind::WouldBlock => break, - Err(e) if e.kind() == io::ErrorKind::TimedOut => break, + Err(error) if error.kind() == io::ErrorKind::WouldBlock => break, + Err(error) if error.kind() == io::ErrorKind::TimedOut => break, Err(_) => break, } } @@ -360,7 +361,7 @@ fn test_forwarder_starts_arbitrary_child_process() -> Result<(), anyhow::Error> // * [x] A standalone forwarder executable can be started that launches an arbitrary child process. let (cmd, args) = sleep_cmd(5); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; // If we got here, the forwarder started successfully. @@ -375,7 +376,7 @@ fn test_forwarder_passes_arguments_unchanged() -> Result<(), anyhow::Error> { // Use a command that outputs arguments and then waits, so we have time to connect. let (cmd, args) = echo_args_cmd(&["-n", "test", "with spaces", "--flag"]); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; let mut stream = forwarder.connect_protocol()?; @@ -395,14 +396,14 @@ fn test_child_process_configurable_externally() -> Result<(), anyhow::Error> { // Test with different commands to verify external configuration works. let (cmd1, args1) = echo_with_sleep_cmd("first", 5); - let args1_refs: Vec<&str> = args1.iter().map(|s| s.as_str()).collect(); + let args1_refs: Vec<&str> = args1.iter().map(String::as_str).collect(); let forwarder1 = TestForwarder::start(cmd1, &args1_refs)?; let mut stream1 = forwarder1.connect_protocol()?; let output1 = read_all_available(&mut stream1, Duration::from_millis(500))?; assert!(String::from_utf8_lossy(&output1).contains("first")); let (cmd2, args2) = echo_with_sleep_cmd("second", 5); - let args2_refs: Vec<&str> = args2.iter().map(|s| s.as_str()).collect(); + let args2_refs: Vec<&str> = args2.iter().map(String::as_str).collect(); let forwarder2 = TestForwarder::start(cmd2, &args2_refs)?; let mut stream2 = forwarder2.connect_protocol()?; let output2 = read_all_available(&mut stream2, Duration::from_millis(500))?; @@ -416,7 +417,7 @@ fn test_forwarder_exits_when_child_exits() -> Result<(), anyhow::Error> { // Use a command that runs briefly and then exits. let (cmd, args) = short_lived_cmd("test", 0); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let mut forwarder = TestForwarder::start(cmd, &args_refs)?; // Connect to protocol port to ensure we’re monitoring the forwarder. @@ -441,7 +442,7 @@ fn test_forwarder_exposes_three_tcp_ports() -> Result<(), anyhow::Error> { // * [x] a **health port** let (cmd, args) = sleep_cmd(10); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; // Verify all three ports are accessible. @@ -605,7 +606,7 @@ fn test_stdout_sent_over_protocol_port() -> Result<(), anyhow::Error> { // * [x] The forwarder sends the child process’s `stdout` stream over the protocol port. let (cmd, args) = echo_with_sleep_cmd("Hello from stdout", 5); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; let mut stream = forwarder.connect_protocol()?; @@ -622,7 +623,7 @@ fn test_stdin_received_on_protocol_port() -> Result<(), anyhow::Error> { // * [x] Data received on the protocol port is forwarded to the child process’s `stdin` while the connection is active. let (cmd, args) = cat_cmd(); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; let mut stream = forwarder.connect_protocol()?; @@ -650,7 +651,7 @@ fn test_stderr_sent_over_stderr_port() -> Result<(), anyhow::Error> { // Use a `bash` command that writes to `stderr` and then waits. let (cmd, args) = stderr_echo_with_sleep_cmd("error message", 5); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; let mut stream = forwarder.connect_stderr()?; let output = read_all_available(&mut stream, Duration::from_millis(500))?; @@ -665,7 +666,7 @@ fn test_protocol_port_single_client_only() -> Result<(), anyhow::Error> { // * [x] The protocol port allows at most one active client connection at a time. let (cmd, args) = loop_stdin_to_stdout_cmd(); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; // First client connects successfully. @@ -719,7 +720,7 @@ fn test_stderr_port_single_client_only() -> Result<(), anyhow::Error> { // * [x] The `stderr` port allows at most one active client connection at a time. let (cmd, args) = continuous_stderr_loop_cmd(); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; // First client connects successfully. @@ -753,7 +754,7 @@ fn test_health_port_multiple_clients() -> Result<(), anyhow::Error> { // * [x] The health port allows multiple simultaneous client connections. let (cmd, args) = sleep_cmd(10); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; // Connect multiple clients to the health port. @@ -779,7 +780,7 @@ fn test_protocol_port_buffered_stdout_replay() -> Result<(), anyhow::Error> { // Use a script that produces output immediately and then waits. let (cmd, args) = multi_echo_stdout_cmd("buffered output", 1.0, "realtime output", 10); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; // Now connect - buffering ensures we receive output produced before connection. @@ -803,7 +804,7 @@ fn test_protocol_disconnect_kills_child() -> Result<(), anyhow::Error> { // * [x] When a client disconnects from the protocol port, the child process is killed and the forwarder terminates. let (cmd, args) = sleep_cmd(100); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let mut forwarder = TestForwarder::start(cmd, &args_refs)?; { @@ -828,7 +829,7 @@ fn test_stderr_port_buffered_stderr_replay() -> Result<(), anyhow::Error> { // Use a script that produces `stderr` immediately and then waits. let (cmd, args) = multi_echo_stderr_cmd("buffered error", 1.0, "realtime error", 10); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; // Now connect to `stderr`--buffering ensures we receive output produced before connection. @@ -853,7 +854,7 @@ fn test_stderr_disconnect_does_not_kill_child() -> Result<(), anyhow::Error> { // process terminate because of that. let (cmd, args) = sleep_cmd(10); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; { let _stderr_stream = forwarder.connect_stderr(); @@ -884,7 +885,7 @@ fn test_stderr_port_reconnect_continues_from_current_state() -> Result<(), anyho // - "while_disconnected" after 3 seconds (buffered while no client connected) // - "during_second_connection" after 5 seconds (sent to second client) let (cmd, args) = complex_stderr_reconnect_cmd(); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; // Wait to ensure "before_connection" is buffered. @@ -945,7 +946,7 @@ fn test_output_buffering_prevents_data_loss() -> Result<(), anyhow::Error> { // Start a process that produces output immediately. let (cmd, args) = combined_output_cmd("stdout message", "stderr message", 10); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; // Buffering ensures output is captured even if we connect immediately. @@ -970,7 +971,7 @@ fn test_health_port_indicates_readiness() -> Result<(), anyhow::Error> { // * [x] A successful TCP connection to the health port indicates that the forwarder is ready to accept connections and operate normally. let (cmd, args) = sleep_cmd(10); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; // If we can connect to health port, the forwarder is ready. @@ -990,7 +991,7 @@ fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { // Use a process that produces high-volume output on both `stdout` and `stderr`. // Output a unique numbered line every 10ms for 3 seconds (300 lines on each stream). let (cmd, args) = numbered_output_loop_cmd(300, 10); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; // Spawn a thread to continuously perform health checks for 3.5 seconds. @@ -1020,8 +1021,8 @@ fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { match protocol_stream.read(&mut buffer) { Ok(0) => break, Ok(n) => all_output.extend_from_slice(&buffer[..n]), - Err(e) if e.kind() == io::ErrorKind::TimedOut => break, - Err(e) if e.kind() == io::ErrorKind::WouldBlock => break, + Err(error) if error.kind() == io::ErrorKind::TimedOut => break, + Err(error) if error.kind() == io::ErrorKind::WouldBlock => break, Err(_) => break, } } @@ -1041,8 +1042,8 @@ fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { match stderr_stream.read(&mut buffer) { Ok(0) => break, Ok(n) => all_output.extend_from_slice(&buffer[..n]), - Err(e) if e.kind() == io::ErrorKind::TimedOut => break, - Err(e) if e.kind() == io::ErrorKind::WouldBlock => break, + Err(error) if error.kind() == io::ErrorKind::TimedOut => break, + Err(error) if error.kind() == io::ErrorKind::WouldBlock => break, Err(_) => break, } } @@ -1105,7 +1106,7 @@ fn test_works_with_various_executables() -> Result<(), anyhow::Error> { // Test with `echo` via shell command. { let (cmd, args) = echo_with_sleep_cmd("test1", 2); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; let mut stream = forwarder.connect_protocol()?; let output = read_all_available(&mut stream, Duration::from_millis(500))?; @@ -1115,7 +1116,7 @@ fn test_works_with_various_executables() -> Result<(), anyhow::Error> { // Test with another echo. { let (cmd, args) = echo_with_sleep_cmd("test2", 2); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; let mut stream = forwarder.connect_protocol()?; let output = read_all_available(&mut stream, Duration::from_millis(500))?; @@ -1125,7 +1126,7 @@ fn test_works_with_various_executables() -> Result<(), anyhow::Error> { // Test with `cat` (interactive). { let (cmd, args) = cat_cmd(); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; let mut stream = forwarder.connect_protocol()?; stream @@ -1168,7 +1169,7 @@ fn test_large_output_buffering() -> Result<(), anyhow::Error> { // Generate a large output. let large_size = 100_000; let (cmd, args) = generate_large_output_cmd(large_size); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; // Wait for output to be generated. @@ -1200,7 +1201,7 @@ fn test_concurrent_stdin_stdout_bidirectional() -> Result<(), anyhow::Error> { // Use `cat` which echoes `stdin` to `stdout`. let (cmd, args) = cat_cmd(); - let args_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + let args_refs: Vec<&str> = args.iter().map(String::as_str).collect(); let forwarder = TestForwarder::start(cmd, &args_refs)?; let mut stream = forwarder.connect_protocol()?; diff --git a/tests/test_utils.rs b/tests/test_utils.rs index f0e1176..b57b954 100644 --- a/tests/test_utils.rs +++ b/tests/test_utils.rs @@ -436,7 +436,7 @@ pub fn echo_args_cmd(args: &[&str]) -> (&'static str, Vec) { "echo $@ && sleep 5".to_string(), "--".to_string(), ]; - script_args.extend(args.iter().map(|s| s.to_string())); + script_args.extend(args.iter().map(ToString::to_string)); ("bash", script_args) } From 4b08e5cefe7fdf05bfda4e05b19404d0ff1cfc0f Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 15:39:22 +0200 Subject: [PATCH 27/54] fix: merge identical `match` arms Co-authored-by: Copilot --- tests/integration_test.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/integration_test.rs b/tests/integration_test.rs index b2f3ea5..e4d5a42 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -1182,9 +1182,8 @@ fn test_large_output_buffering() -> Result<(), anyhow::Error> { while total_read < large_size { match read_with_timeout(&mut stream, &mut buffer) { - Ok(0) => break, + Ok(0) | Err(_) => break, Ok(n) => total_read += n, - Err(_) => break, } } From 5a6d4f9670670d789de8f3817deb4bab36c9cd05 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 15:44:24 +0200 Subject: [PATCH 28/54] fix: use `get()` instead of slicing to avoid panic Co-authored-by: Copilot --- src/output.rs | 4 ++-- src/servers/protocol.rs | 2 +- tests/integration_test.rs | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/output.rs b/src/output.rs index c8eecda..50836a3 100644 --- a/src/output.rs +++ b/src/output.rs @@ -96,7 +96,7 @@ pub(crate) fn pump_output_to_state( break; } - let chunk = &buffer[..num_bytes_read]; + let chunk = buffer.get(..num_bytes_read).unwrap_or_default(); guard.buffer.extend_from_slice(chunk); } output_state.condition_variable.notify_all(); @@ -149,7 +149,7 @@ pub(crate) fn serve_output_on_stream( let mut num_bytes_written = 0; while num_bytes_written < buffered_data.len() { - match stream.write(&buffered_data[num_bytes_written..]) { + match stream.write(buffered_data.get(num_bytes_written..).unwrap_or_default()) { Ok(0) => { // Treat as connection no longer writable. break; diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index 0b9dc10..24b1f97 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -39,7 +39,7 @@ fn forward_stream_data_to_child_process( } }; - if let Err(error) = child_stdin.write_all(&read_buffer[..num_bytes_read]) { + if let Err(error) = child_stdin.write_all(read_buffer.get(..num_bytes_read).unwrap_or_default()) { let _ = control_tx.send(ControlMessage::KillChild); return Err(anyhow::anyhow!("Failed to write to child stdin: {error}")); } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index e4d5a42..4711e66 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -342,7 +342,7 @@ fn read_all_available(stream: &mut TcpStream, timeout: Duration) -> Result break, - Ok(n) => result.extend_from_slice(&buffer[..n]), + Ok(n) => result.extend_from_slice(buffer.get(..n).unwrap_or_default()), Err(error) if error.kind() == io::ErrorKind::WouldBlock => break, Err(error) if error.kind() == io::ErrorKind::TimedOut => break, Err(_) => break, @@ -1020,7 +1020,7 @@ fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { loop { match protocol_stream.read(&mut buffer) { Ok(0) => break, - Ok(n) => all_output.extend_from_slice(&buffer[..n]), + Ok(n) => all_output.extend_from_slice(buffer.get(..n).unwrap_or_default()), Err(error) if error.kind() == io::ErrorKind::TimedOut => break, Err(error) if error.kind() == io::ErrorKind::WouldBlock => break, Err(_) => break, @@ -1041,7 +1041,7 @@ fn test_health_checks_do_not_interfere() -> Result<(), anyhow::Error> { loop { match stderr_stream.read(&mut buffer) { Ok(0) => break, - Ok(n) => all_output.extend_from_slice(&buffer[..n]), + Ok(n) => all_output.extend_from_slice(buffer.get(..n).unwrap_or_default()), Err(error) if error.kind() == io::ErrorKind::TimedOut => break, Err(error) if error.kind() == io::ErrorKind::WouldBlock => break, Err(_) => break, From ef78eaa5c143de68a0c5a3ca40ba64391b5bd277 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 15:46:42 +0200 Subject: [PATCH 29/54] style: reformat code --- src/servers/protocol.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index 24b1f97..6d0a5cf 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -35,11 +35,15 @@ fn forward_stream_data_to_child_process( Ok(n) => n, Err(error) => { let _ = control_tx.send(ControlMessage::KillChild); - return Err(anyhow::anyhow!("Failed to read from protocol client: {error}")); + return Err(anyhow::anyhow!( + "Failed to read from protocol client: {error}" + )); } }; - if let Err(error) = child_stdin.write_all(read_buffer.get(..num_bytes_read).unwrap_or_default()) { + if let Err(error) = + child_stdin.write_all(read_buffer.get(..num_bytes_read).unwrap_or_default()) + { let _ = control_tx.send(ControlMessage::KillChild); return Err(anyhow::anyhow!("Failed to write to child stdin: {error}")); } From bb3d52a1769624361146eec862094eed8723b3f3 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 15:51:40 +0200 Subject: [PATCH 30/54] style: apply formatter --- src/args.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/args.rs b/src/args.rs index 75f9f66..e26ab86 100644 --- a/src/args.rs +++ b/src/args.rs @@ -273,7 +273,10 @@ mod tests { fn test_missing_command_fails() { let env = TestEnv::default(); let result = Args::try_parse_from_env_and_args(&env, ["stdioxide"]); - assert!(result.is_err(), "Expected parsing to fail when command is missing"); + assert!( + result.is_err(), + "Expected parsing to fail when command is missing" + ); } #[test] From c6e594390cad4569f5e0e959834412563436e379 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 15:51:49 +0200 Subject: [PATCH 31/54] chore: add pre-commit hooks --- .githooks/pre-commit | 12 ++++++++++++ README.md | 10 ++++++++++ 2 files changed, 22 insertions(+) create mode 100755 .githooks/pre-commit diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 0000000..dbb21a8 --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,12 @@ +#!/usr/bin/env bash +set -euo pipefail + +cargo fmt --all + +if ! git diff --quiet; then + echo "cargo fmt changed files. Please review and stage the formatting changes." + exit 1 +fi + +cargo check --all-targets --all-features +cargo test --all-targets --all-features diff --git a/README.md b/README.md index 2feaeea..485a5dc 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,16 @@ cargo test This project was developed with AI assistance using GitHub Copilot and Claude Sonnet 4.5. +### Pre-commit Hooks + +To enable pre-commit hooks that run tests and linting before commits, run: + +```bash +git config core.hooksPath .githooks +``` + +This will ensure code quality checks run automatically before each commit. + ## License See LICENSE file for details. From 5a9709006eca10659b934b583511e6e782933b13 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 15:52:48 +0200 Subject: [PATCH 32/54] =?UTF-8?q?fix:=20don=E2=80=99t=20prefix=20used=20va?= =?UTF-8?q?riables=20with=20underscore?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/integration_test.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 4711e66..a4f0b4f 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -758,15 +758,13 @@ fn test_health_port_multiple_clients() -> Result<(), anyhow::Error> { let forwarder = TestForwarder::start(cmd, &args_refs)?; // Connect multiple clients to the health port. - let _stream1 = forwarder.connect_health()?; - let _stream2 = forwarder.connect_health()?; - let _stream3 = forwarder.connect_health()?; + let stream1 = forwarder.connect_health()?; + let stream2 = forwarder.connect_health()?; + let stream3 = forwarder.connect_health()?; // All connections should succeed. assert!( - _stream1.peer_addr().is_ok() - && _stream2.peer_addr().is_ok() - && _stream3.peer_addr().is_ok() + stream1.peer_addr().is_ok() && stream2.peer_addr().is_ok() && stream3.peer_addr().is_ok() ); Ok(()) } From f7e01d9e29520d945e5a25e91d6c03922673a203 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 15:58:04 +0200 Subject: [PATCH 33/54] chore: disable linting decimal representation-related integer literals --- Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 4774ec7..8fbe2d1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -211,7 +211,6 @@ assertions_on_result_states = "warn" clone_on_ref_ptr = "warn" create_dir = "warn" dbg_macro = "warn" -decimal_literal_representation = "warn" default_union_representation = "warn" deref_by_slicing = "warn" disallowed_script_idents = "warn" From d651d49a5dff3425b8ca7de93f3e5ced173e9d4d Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 15:58:28 +0200 Subject: [PATCH 34/54] fix: specify wrapping behavior for integer addition Co-authored-by: Copilot --- src/output.rs | 2 +- tests/lsp_client.rs | 2 +- tests/test_utils.rs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/output.rs b/src/output.rs index 50836a3..70816da 100644 --- a/src/output.rs +++ b/src/output.rs @@ -155,7 +155,7 @@ pub(crate) fn serve_output_on_stream( break; } Ok(n) => { - num_bytes_written += n; + num_bytes_written = num_bytes_written.saturating_add(n); } Err(e) if e.kind() == io::ErrorKind::Interrupted => { // Interrupted by a signal, just retry. diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index e1c10c5..af2478d 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -103,7 +103,7 @@ impl LspClient { params: serde_json::Value, ) -> Result { let request_id = self.next_request_id; - self.next_request_id += 1_i32; + self.next_request_id = self.next_request_id.wrapping_add(1_i32); let request = serde_json::json!({ "jsonrpc": "2.0", diff --git a/tests/test_utils.rs b/tests/test_utils.rs index b57b954..ea9b714 100644 --- a/tests/test_utils.rs +++ b/tests/test_utils.rs @@ -204,7 +204,7 @@ pub fn cat_cmd() -> (&'static str, Vec) { /// Returns a command that reads from `stdin` and echoes to `stdout` (like `cat`). #[cfg(not(windows))] #[must_use] -pub fn cat_cmd() -> (&'static str, Vec) { +pub const fn cat_cmd() -> (&'static str, Vec) { ("cat", vec![]) } @@ -480,13 +480,13 @@ pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) /// Returns the platform-specific Python command name. #[cfg(windows)] #[must_use] -pub fn python_cmd() -> &'static str { +pub const fn python_cmd() -> &'static str { "python" } /// Returns the platform-specific Python command name. #[cfg(not(windows))] #[must_use] -pub fn python_cmd() -> &'static str { +pub const fn python_cmd() -> &'static str { "python3" } From 6e933880e210a0eef76fccb33921379d325854c2 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Mon, 27 Apr 2026 15:59:51 +0200 Subject: [PATCH 35/54] =?UTF-8?q?fix:=20don=E2=80=99t=20use=20`unwrap=5Fer?= =?UTF-8?q?r()`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Copilot --- tests/integration_test.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/integration_test.rs b/tests/integration_test.rs index a4f0b4f..abd4e7f 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -704,15 +704,17 @@ fn test_protocol_port_single_client_only() -> Result<(), anyhow::Error> { let mut buf = [0_u8; 100]; let result = stream2.read(&mut buf); - assert!( - result.is_err() - && matches!( - result.as_ref().unwrap_err().kind(), - io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut - ), - "Second client should timeout reading (connection never served by protocol server)" - ); - Ok(()) + if let Err(error) = &result { + assert!( + error.kind() == io::ErrorKind::WouldBlock || error.kind() == io::ErrorKind::TimedOut, + "Second client should timeout reading (connection never served by protocol server)" + ); + Ok(()) + } else { + anyhow::bail!( + "Second client should not receive data (connection never served by protocol server)" + ); + } } #[test] From 2dcc362b55a01755fa6e83e75e159fb41f5c2d82 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:03:15 +0200 Subject: [PATCH 36/54] fix: use `to_owned()` instead of `to_string()` on `str` values Co-authored-by: Copilot --- tests/integration_test.rs | 10 +++++----- tests/test_utils.rs | 32 ++++++++++++++++---------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/tests/integration_test.rs b/tests/integration_test.rs index abd4e7f..5bfde83 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -216,7 +216,7 @@ impl TestForwarder { // Get the path to the `stdioxide` binary. // In integration tests, we need to use the binary from the target directory. let bin_path = env::var("CARGO_BIN_EXE_stdioxide") - .unwrap_or_else(|_| "target/debug/stdioxide".to_string()); + .unwrap_or_else(|_| "target/debug/stdioxide".to_owned()); let mut cmd = Command::new(&bin_path); cmd.arg("--protocol-port") @@ -485,8 +485,8 @@ fn test_default_port_values() -> Result<(), anyhow::Error> { } // Launch `stdioxide` *without* specifying ports to verify it uses the defaults. - let bin_path = env::var("CARGO_BIN_EXE_stdioxide") - .unwrap_or_else(|_| "target/debug/stdioxide".to_string()); + let bin_path = + env::var("CARGO_BIN_EXE_stdioxide").unwrap_or_else(|_| "target/debug/stdioxide".to_owned()); let (sleep_command, sleep_args) = sleep_cmd(10); let mut cmd = Command::new(&bin_path); @@ -543,8 +543,8 @@ fn test_port_override_via_environment_variables() -> Result<(), anyhow::Error> { let custom_health = ports.health_port(); // Launch `stdioxide` with environment variables (NOT command-line args) to test env var override. - let bin_path = env::var("CARGO_BIN_EXE_stdioxide") - .unwrap_or_else(|_| "target/debug/stdioxide".to_string()); + let bin_path = + env::var("CARGO_BIN_EXE_stdioxide").unwrap_or_else(|_| "target/debug/stdioxide".to_owned()); let (sleep_command, sleep_args) = sleep_cmd(10); let mut cmd = Command::new(&bin_path); diff --git a/tests/test_utils.rs b/tests/test_utils.rs index ea9b714..ab9fe93 100644 --- a/tests/test_utils.rs +++ b/tests/test_utils.rs @@ -89,7 +89,7 @@ pub fn stderr_echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Ve ( "bash", vec![ - "-c".to_string(), + "-c".to_owned(), format!("echo '{}' >&2 && sleep {}", text, seconds), ], ) @@ -131,7 +131,7 @@ pub fn multi_echo_stderr_cmd( ( "bash", vec![ - "-c".to_string(), + "-c".to_owned(), format!( "echo '{}' >&2; sleep {}; echo '{}' >&2; sleep {}", buffered, sleep1, realtime, sleep2 @@ -176,7 +176,7 @@ pub fn multi_echo_stdout_cmd( ( "bash", vec![ - "-c".to_string(), + "-c".to_owned(), format!( "echo '{}'; sleep {}; echo '{}'; sleep {}", buffered, sleep1, realtime, sleep2 @@ -232,8 +232,8 @@ pub fn loop_stdin_to_stdout_cmd() -> (&'static str, Vec) { ( "bash", vec![ - "-c".to_string(), - "while true; do read line; echo response; done".to_string(), + "-c".to_owned(), + "while true; do read line; echo response; done".to_owned(), ], ) } @@ -260,8 +260,8 @@ pub fn continuous_stderr_loop_cmd() -> (&'static str, Vec) { ( "bash", vec![ - "-c".to_string(), - "while true; do echo error >&2; sleep 0.1; done".to_string(), + "-c".to_owned(), + "while true; do echo error >&2; sleep 0.1; done".to_owned(), ], ) } @@ -288,7 +288,7 @@ pub fn generate_large_output_cmd(size: usize) -> (&'static str, Vec) { ( "bash", vec![ - "-c".to_string(), + "-c".to_owned(), format!("head -c {} /dev/zero | tr '\\0' 'A'; sleep 10", size), ], ) @@ -325,7 +325,7 @@ pub fn numbered_output_loop_cmd(count: u32, interval_ms: u32) -> (&'static str, ( "bash", vec![ - "-c".to_string(), + "-c".to_owned(), format!( "for i in {{1..{count}}}; do echo \"stdout_line_$i\"; echo \"stderr_line_$i\" >&2; sleep {interval_sec}; done" ), @@ -360,7 +360,7 @@ pub fn complex_stderr_reconnect_cmd() -> (&'static str, Vec) { ( "bash", vec![ - "-c".to_string(), + "-c".to_owned(), concat!( "echo 'before_connection' >&2; sleep 0.5; ", "echo 'during_first_connection' >&2; sleep 1; ", @@ -368,7 +368,7 @@ pub fn complex_stderr_reconnect_cmd() -> (&'static str, Vec) { "echo 'while_disconnected' >&2; sleep 2; ", "echo 'during_second_connection' >&2; sleep 10", ) - .to_string(), + .to_owned(), ], ) } @@ -406,7 +406,7 @@ pub fn combined_output_cmd( ( "bash", vec![ - "-c".to_string(), + "-c".to_owned(), format!( "echo '{}'; echo '{}' >&2; sleep {}", stdout_msg, stderr_msg, sleep_sec @@ -432,9 +432,9 @@ pub fn echo_args_cmd(args: &[&str]) -> (&'static str, Vec) { #[must_use] pub fn echo_args_cmd(args: &[&str]) -> (&'static str, Vec) { let mut script_args = vec![ - "-c".to_string(), - "echo $@ && sleep 5".to_string(), - "--".to_string(), + "-c".to_owned(), + "echo $@ && sleep 5".to_owned(), + "--".to_owned(), ]; script_args.extend(args.iter().map(ToString::to_string)); ("bash", script_args) @@ -471,7 +471,7 @@ pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) ( "bash", vec![ - "-c".to_string(), + "-c".to_owned(), format!("echo {} && sleep {}", msg, sleep_arg), ], ) From cfa1918d19f7f52c3a435427498c4364e3e4e4a8 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:04:30 +0200 Subject: [PATCH 37/54] =?UTF-8?q?fix:=20don=E2=80=99t=20add=20items=20afte?= =?UTF-8?q?r=20statements?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/integration_test.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 5bfde83..7301357 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -470,6 +470,7 @@ fn test_default_port_values() -> Result<(), anyhow::Error> { // * [x] `7000` for the protocol port // * [x] `7001` for the stderr port // * [x] `7002` for the health port + const NUM_ATTEMPTS: usize = 30; // Test setup: Ensure default ports are available. // If they’re not, this is an environment issue, not a test failure. @@ -503,7 +504,6 @@ fn test_default_port_values() -> Result<(), anyhow::Error> { // Wait for the forwarder to be ready by connecting to the default health port. let mut connected = false; - const NUM_ATTEMPTS: usize = 30; for _ in 0..NUM_ATTEMPTS { if TcpStream::connect_timeout(&"127.0.0.1:7002".parse()?, Duration::from_millis(100)) .is_ok() @@ -535,6 +535,7 @@ fn test_default_port_values() -> Result<(), anyhow::Error> { #[test] fn test_port_override_via_environment_variables() -> Result<(), anyhow::Error> { // * [x] All three port numbers can be overridden via environment variables. + const NUM_ATTEMPTS: usize = 30; // Allocate unique ports to avoid conflicts. let ports = AllocatedPorts::new()?; @@ -563,7 +564,6 @@ fn test_port_override_via_environment_variables() -> Result<(), anyhow::Error> { // Wait for the forwarder to be ready by connecting to the custom health port. let mut connected = false; - const NUM_ATTEMPTS: usize = 30; for _ in 0..NUM_ATTEMPTS { if TcpStream::connect_timeout( &format!("127.0.0.1:{custom_health}").parse()?, From 7d0689376ffd519d71c83ea62432c05b094379ce Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:05:25 +0200 Subject: [PATCH 38/54] fix: remove redundant `self` parameter Co-authored-by: Copilot --- tests/integration_test.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 7301357..fd7e96d 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -272,21 +272,21 @@ impl TestForwarder { /// Connect to the protocol port. fn connect_protocol(&self) -> Result { - self.connect_with_retry(self.ports.protocol_port(), "protocol") + Self::connect_with_retry(self.ports.protocol_port(), "protocol") } /// Connect to the `stderr` port. fn connect_stderr(&self) -> Result { - self.connect_with_retry(self.ports.stderr_port(), "stderr") + Self::connect_with_retry(self.ports.stderr_port(), "stderr") } /// Connect to the health port. fn connect_health(&self) -> Result { - self.connect_with_retry(self.ports.health_port(), "health") + Self::connect_with_retry(self.ports.health_port(), "health") } /// Connect to a port with retries. - fn connect_with_retry(&self, port: u16, label: &str) -> Result { + fn connect_with_retry(port: u16, label: &str) -> Result { const NUM_ATTEMPTS: usize = 20; for attempt in 0..NUM_ATTEMPTS { match TcpStream::connect(("127.0.0.1", port)) { From 75cf0334d7295f6f567725b8fde33347856677d3 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:06:28 +0200 Subject: [PATCH 39/54] =?UTF-8?q?fix:=20don=E2=80=99t=20redundantly=20pass?= =?UTF-8?q?=20parameter=20by=20value?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Copilot --- tests/lsp_client.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index af2478d..b7daf37 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -100,7 +100,7 @@ impl LspClient { fn send_request( &mut self, method: &str, - params: serde_json::Value, + params: &serde_json::Value, ) -> Result { let request_id = self.next_request_id; self.next_request_id = self.next_request_id.wrapping_add(1_i32); @@ -181,7 +181,7 @@ impl LspClient { } }); - let request_id = self.send_request("initialize", params)?; + let request_id = self.send_request("initialize", ¶ms)?; self.read_response(request_id) } @@ -216,7 +216,7 @@ impl LspClient { } }); - let request_id = self.send_request("textDocument/documentSymbol", params)?; + let request_id = self.send_request("textDocument/documentSymbol", ¶ms)?; self.read_response(request_id) } @@ -226,7 +226,7 @@ impl LspClient { /// /// Returns an error if the shutdown request fails to send or the response cannot be read. pub fn shutdown(&mut self) -> anyhow::Result { - let request_id = self.send_request("shutdown", serde_json::json!(null))?; + let request_id = self.send_request("shutdown", &serde_json::json!(null))?; self.read_response(request_id) } } From 599e11200025031571f1f6f6a5fe0678da8b8077 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:09:42 +0200 Subject: [PATCH 40/54] fix: remove redundant `continue` statements --- src/output.rs | 3 +-- tests/integration_test.rs | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/output.rs b/src/output.rs index 70816da..8c5ec53 100644 --- a/src/output.rs +++ b/src/output.rs @@ -157,9 +157,8 @@ pub(crate) fn serve_output_on_stream( Ok(n) => { num_bytes_written = num_bytes_written.saturating_add(n); } - Err(e) if e.kind() == io::ErrorKind::Interrupted => { + Err(error) if error.kind() == io::ErrorKind::Interrupted => { // Interrupted by a signal, just retry. - continue; } Err(_) => { // Any other error is treated as the connection being no longer writable. diff --git a/tests/integration_test.rs b/tests/integration_test.rs index fd7e96d..0930730 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -177,7 +177,6 @@ impl TestForwarder { if attempt < MAX_RETRIES - 1 { // Retry with new ports thread::sleep(Duration::from_millis(100)); - continue; } } } From 5c4d2de5d5705c909b74b8d7b356db01ee71b21d Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:16:01 +0200 Subject: [PATCH 41/54] fix: drop lock earlier Co-authored-by: Copilot --- tests/integration_test.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 0930730..002381e 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -117,10 +117,12 @@ impl AllocatedPorts { let mut registry = ALLOCATED_PORTS_REGISTRY.lock().map_err(|error| { anyhow::anyhow!("Failed to lock allocated ports registry: {error}") })?; - if !registry.contains(&p1) && !registry.contains(&p2) && !registry.contains(&p3) { + let ports_are_available = [p1, p2, p3].iter().all(|port| !registry.contains(port)); + if ports_are_available { registry.insert(p1); registry.insert(p2); registry.insert(p3); + drop(registry); // Drop “early” to satisfy Clippy. return Ok(Self { protocol_port: p1, From 1526c459d0d2e9d0cb70e647335dcf7d1a76d779 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:20:09 +0200 Subject: [PATCH 42/54] fix: allow imports via `std` instead of `core` or `alloc` --- Cargo.toml | 2 -- src/control.rs | 3 +-- src/output.rs | 7 +++++-- src/servers/stderr.rs | 7 +++++-- tests/integration_test.rs | 5 ++--- tests/lsp_client.rs | 2 +- 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8fbe2d1..27c69d9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -183,8 +183,6 @@ pub_use = "warn" ref_patterns = "warn" same_name_method = "warn" self_named_module_files = "warn" -std_instead_of_alloc = "warn" -std_instead_of_core = "warn" str_to_string = "warn" string_add = "warn" string_to_string = "allow" # Deprecated; keep allowed if older toolchains still mention it. diff --git a/src/control.rs b/src/control.rs index e266fe3..210e947 100644 --- a/src/control.rs +++ b/src/control.rs @@ -1,7 +1,6 @@ //! Child process lifecycle coordination and control messages. -use core::time::Duration; -use std::sync::mpsc; +use std::{sync::mpsc, time::Duration}; use subprocess::Job; use tracing::info; diff --git a/src/output.rs b/src/output.rs index 8c5ec53..e0f5971 100644 --- a/src/output.rs +++ b/src/output.rs @@ -1,10 +1,13 @@ //! Output buffering and stream serving logic. -use core::sync::atomic::{AtomicBool, Ordering}; use std::{ io::{self, Read, Write}, net::TcpStream, - sync::{Arc, Condvar, Mutex, mpsc}, + sync::{ + Arc, Condvar, Mutex, + atomic::{AtomicBool, Ordering}, + mpsc, + }, }; use tracing::{debug, info}; diff --git a/src/servers/stderr.rs b/src/servers/stderr.rs index 751d73d..dee5174 100644 --- a/src/servers/stderr.rs +++ b/src/servers/stderr.rs @@ -1,10 +1,13 @@ //! `stderr` TCP server with reconnection support and output buffering. -use core::sync::atomic::{AtomicBool, Ordering}; use std::{ io::Read, net::{TcpListener, TcpStream}, - sync::{Arc, mpsc}, + sync::{ + Arc, + atomic::{AtomicBool, Ordering}, + mpsc, + }, thread, }; diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 002381e..13dc082 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -5,7 +5,6 @@ reason = "Using `assert!()`s is idiomatic, but we need to return `Result`s to be able to return I/O-related errors." )] -use core::time::Duration; use std::{ collections::HashSet, env, @@ -13,10 +12,10 @@ use std::{ io::{self, Read, Write}, net::{Shutdown, TcpListener, TcpStream}, process::{Child, Command, Stdio}, - string::String, + //string::String, sync::{LazyLock, Mutex}, thread, - time::Instant, + time::{Duration, Instant}, }; // Acknowledge available dev-dependencies not used in this test file. diff --git a/tests/lsp_client.rs b/tests/lsp_client.rs index b7daf37..24beede 100644 --- a/tests/lsp_client.rs +++ b/tests/lsp_client.rs @@ -5,11 +5,11 @@ reason = "Private test module, but items need `pub` for parent access" )] -use core::time::Duration; use std::{ io::{self, Read, Write}, net::TcpStream, thread, + time::Duration, }; // Acknowledge available dev-dependencies not used in this test file. From 091624b65460ad320ea58ed7e0b674b371ea36d0 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:21:02 +0200 Subject: [PATCH 43/54] fix: remove redundant reference --- src/servers/stderr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/servers/stderr.rs b/src/servers/stderr.rs index dee5174..d683513 100644 --- a/src/servers/stderr.rs +++ b/src/servers/stderr.rs @@ -83,7 +83,7 @@ pub(crate) fn stderr_server( let has_active_connection_clone = Arc::clone(&has_active_connection); let has_active_connection_monitor = Arc::clone(&has_active_connection); let has_active_connection_write = Arc::clone(&has_active_connection); - let stderr_state = Arc::clone(&stderr_state); + let stderr_state = Arc::clone(stderr_state); let control_tx = control_tx.clone(); // Spawn read monitoring thread to detect disconnection proactively. From 8f6da0c8830598c0626f6e219bc6ccb58c842a4d Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:25:00 +0200 Subject: [PATCH 44/54] fix: don't unnecessarily pass arguments by value Co-authored-by: Copilot --- src/app.rs | 10 +++++----- src/control.rs | 2 +- src/output.rs | 16 ++++++++-------- src/servers/health.rs | 2 +- src/servers/protocol.rs | 12 ++++++------ src/servers/stderr.rs | 10 +++++----- 6 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/app.rs b/src/app.rs index 5c18c9a..2dc9ef7 100644 --- a/src/app.rs +++ b/src/app.rs @@ -59,14 +59,14 @@ pub fn run(args: &Args) -> Result<(), anyhow::Error> { { let stdout_state = Arc::clone(&stdout_state); thread::spawn(move || { - drop(pump_output_to_state(child.stdout, stdout_state, "stdout")); + drop(pump_output_to_state(child.stdout, &stdout_state, "stdout")); }); } { let stderr_state = Arc::clone(&stderr_state); thread::spawn(move || { - drop(pump_output_to_state(child.stderr, stderr_state, "stderr")); + drop(pump_output_to_state(child.stderr, &stderr_state, "stderr")); }); } @@ -75,7 +75,7 @@ pub fn run(args: &Args) -> Result<(), anyhow::Error> { let control_tx = control_tx.clone(); thread::spawn(move || { drop(protocol_server( - protocol_listener, + &protocol_listener, stdout_state, child.stdin, control_tx, @@ -97,12 +97,12 @@ pub fn run(args: &Args) -> Result<(), anyhow::Error> { { thread::spawn(move || { - drop(health_server(health_listener)); + drop(health_server(&health_listener)); }); } let coordinator_thread: JoinHandle> = - thread::spawn(move || run_child_coordinator(child.job, control_rx)); + thread::spawn(move || run_child_coordinator(child.job, &control_rx)); coordinator_thread .join() diff --git a/src/control.rs b/src/control.rs index 210e947..1ed1e9d 100644 --- a/src/control.rs +++ b/src/control.rs @@ -25,7 +25,7 @@ pub(crate) enum ControlMessage { )] pub(crate) fn run_child_coordinator( job: Job, - control_rx: mpsc::Receiver, + control_rx: &mpsc::Receiver, ) -> Result<(), anyhow::Error> { loop { if let Some(status) = job.poll() { diff --git a/src/output.rs b/src/output.rs index e0f5971..c76aa55 100644 --- a/src/output.rs +++ b/src/output.rs @@ -81,7 +81,7 @@ impl Default for NotifyableOutputState { )] pub(crate) fn pump_output_to_state( mut source: impl Read, - output_state: Arc, + output_state: &Arc, label: &'static str, ) -> Result<(), anyhow::Error> { loop { @@ -118,9 +118,9 @@ pub(crate) fn pump_output_to_state( )] pub(crate) fn serve_output_on_stream( mut stream: TcpStream, - output_state: Arc, - control_tx: mpsc::Sender, - serving_behavior: ServingBehavior, + output_state: &Arc, + control_tx: &mpsc::Sender, + serving_behavior: &ServingBehavior, label: &'static str, ) -> Result<(), anyhow::Error> { loop { @@ -172,7 +172,7 @@ pub(crate) fn serve_output_on_stream( // Before draining the buffer, check if the connection is still active (for `stderr` reconnect support). // If the read monitoring thread detected a disconnect, we should NOT drain the buffer to prevent data loss. - if let ServingBehavior::DoNotKillChildOnDisconnect(ref active) = serving_behavior + if let ServingBehavior::DoNotKillChildOnDisconnect(active) = serving_behavior && !active.load(Ordering::Acquire) { info!( @@ -218,7 +218,7 @@ mod tests { let state = Arc::new(NotifyableOutputState::new()); let input = Cursor::new(Vec::::new()); - pump_output_to_state(input, Arc::clone(&state), "test")?; + pump_output_to_state(input, &state, "test")?; let guard = state .state @@ -236,7 +236,7 @@ mod tests { let data = b"Hello, World!"; let input = Cursor::new(data.to_vec()); - pump_output_to_state(input, Arc::clone(&state), "test")?; + pump_output_to_state(input, &state, "test")?; let guard = state .state @@ -254,7 +254,7 @@ mod tests { let data = vec![0_u8; 16384]; // Larger than buffer size (8192). let input = Cursor::new(data.clone()); - pump_output_to_state(input, Arc::clone(&state), "test")?; + pump_output_to_state(input, &state, "test")?; let guard = state .state diff --git a/src/servers/health.rs b/src/servers/health.rs index d13d99a..0e590c9 100644 --- a/src/servers/health.rs +++ b/src/servers/health.rs @@ -6,7 +6,7 @@ use tracing::warn; /// Waits for clients to connect on the `health` port, and immediately drops any connections. The existence /// of a successful connection is used by the client as a health check for whether the process is alive. -pub(crate) fn health_server(listener: TcpListener) -> Result<(), anyhow::Error> { +pub(crate) fn health_server(listener: &TcpListener) -> Result<(), anyhow::Error> { for stream in listener.incoming() { match stream { Ok(_stream) => { diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index 6d0a5cf..2622a5f 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -22,7 +22,7 @@ use crate::{ fn forward_stream_data_to_child_process( mut stream: TcpStream, mut child_stdin: fs::File, - control_tx: mpsc::Sender, + control_tx: &mpsc::Sender, ) -> Result<(), anyhow::Error> { let mut read_buffer = [0_u8; 8192]; loop { @@ -59,7 +59,7 @@ fn forward_stream_data_to_child_process( /// the client to the child process’s `stdin`, and another for forwarding data from the child /// process’s `stdout` to the client. pub(crate) fn protocol_server( - listener: TcpListener, + listener: &TcpListener, stdout_state: Arc, child_stdin: fs::File, control_tx: mpsc::Sender, @@ -76,15 +76,15 @@ pub(crate) fn protocol_server( drop(forward_stream_data_to_child_process( cloned_stream, child_stdin, - cloned_control_tx, + &cloned_control_tx, )); }), thread::spawn(move || { drop(serve_output_on_stream( stream, - Arc::clone(&stdout_state), - control_tx, - ServingBehavior::KillChildOnDisconnect, + &stdout_state, + &control_tx, + &ServingBehavior::KillChildOnDisconnect, "protocol", )); }), diff --git a/src/servers/stderr.rs b/src/servers/stderr.rs index d683513..29d4803 100644 --- a/src/servers/stderr.rs +++ b/src/servers/stderr.rs @@ -23,7 +23,7 @@ use crate::{ /// When disconnection is detected, the atomic flag is cleared to allow new connections. fn monitor_stderr_client_connection( mut stream: TcpStream, - has_active_connection: Arc, + has_active_connection: &Arc, ) -> Result<(), anyhow::Error> { let mut read_buffer = [0_u8; 1]; loop { @@ -90,7 +90,7 @@ pub(crate) fn stderr_server( thread::spawn(move || { drop(monitor_stderr_client_connection( connection_monitoring_stream, - has_active_connection_monitor, + &has_active_connection_monitor, )); }); @@ -98,9 +98,9 @@ pub(crate) fn stderr_server( thread::spawn(move || { drop(serve_output_on_stream( stream, - stderr_state, - control_tx, - ServingBehavior::DoNotKillChildOnDisconnect(Arc::clone( + &stderr_state, + &control_tx, + &ServingBehavior::DoNotKillChildOnDisconnect(Arc::clone( &has_active_connection_write, )), "stderr", From 9991f9d594459d07d019eee5fb604b8740a286b4 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:28:05 +0200 Subject: [PATCH 45/54] fix: remove non-binding `let`s without type annotations Co-authored-by: Copilot --- src/output.rs | 2 +- src/servers/protocol.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/output.rs b/src/output.rs index c76aa55..7602ba1 100644 --- a/src/output.rs +++ b/src/output.rs @@ -196,7 +196,7 @@ pub(crate) fn serve_output_on_stream( // We treat this as the connection being no longer writable and exit the loop (and potentially kill the // child process, depending on the serving behavior). if matches!(serving_behavior, ServingBehavior::KillChildOnDisconnect) { - let _ = control_tx.send(ControlMessage::KillChild); + let _result = control_tx.send(ControlMessage::KillChild); } return Ok(()); } diff --git a/src/servers/protocol.rs b/src/servers/protocol.rs index 2622a5f..0d7b24d 100644 --- a/src/servers/protocol.rs +++ b/src/servers/protocol.rs @@ -29,12 +29,12 @@ fn forward_stream_data_to_child_process( let num_bytes_read = match stream.read(&mut read_buffer) { Ok(0) => { info!("[protocol] client disconnected; terminating child process"); - let _ = control_tx.send(ControlMessage::KillChild); + let _result = control_tx.send(ControlMessage::KillChild); return Ok(()); } Ok(n) => n, Err(error) => { - let _ = control_tx.send(ControlMessage::KillChild); + let _result = control_tx.send(ControlMessage::KillChild); return Err(anyhow::anyhow!( "Failed to read from protocol client: {error}" )); @@ -44,11 +44,11 @@ fn forward_stream_data_to_child_process( if let Err(error) = child_stdin.write_all(read_buffer.get(..num_bytes_read).unwrap_or_default()) { - let _ = control_tx.send(ControlMessage::KillChild); + let _result = control_tx.send(ControlMessage::KillChild); return Err(anyhow::anyhow!("Failed to write to child stdin: {error}")); } if let Err(error) = child_stdin.flush() { - let _ = control_tx.send(ControlMessage::KillChild); + let _result = control_tx.send(ControlMessage::KillChild); return Err(anyhow::anyhow!("Failed to flush child stdin: {error}")); } } From 4753a9c762fa3e5fa42d2b36ffdf7f1cdf1a196c Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:35:05 +0200 Subject: [PATCH 46/54] fix: remove redundant return value --- src/app.rs | 2 +- src/servers/health.rs | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/app.rs b/src/app.rs index 2dc9ef7..130716b 100644 --- a/src/app.rs +++ b/src/app.rs @@ -97,7 +97,7 @@ pub fn run(args: &Args) -> Result<(), anyhow::Error> { { thread::spawn(move || { - drop(health_server(&health_listener)); + health_server(&health_listener); }); } diff --git a/src/servers/health.rs b/src/servers/health.rs index 0e590c9..ecdbfa9 100644 --- a/src/servers/health.rs +++ b/src/servers/health.rs @@ -6,7 +6,7 @@ use tracing::warn; /// Waits for clients to connect on the `health` port, and immediately drops any connections. The existence /// of a successful connection is used by the client as a health check for whether the process is alive. -pub(crate) fn health_server(listener: &TcpListener) -> Result<(), anyhow::Error> { +pub(crate) fn health_server(listener: &TcpListener) { for stream in listener.incoming() { match stream { Ok(_stream) => { @@ -17,6 +17,4 @@ pub(crate) fn health_server(listener: &TcpListener) -> Result<(), anyhow::Error> } } } - - Ok(()) } From 99402082b3aed35dc2608571637f9a731149ba28 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:35:56 +0200 Subject: [PATCH 47/54] fixup! fix: don't unnecessarily pass arguments by value --- src/app.rs | 2 +- src/control.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app.rs b/src/app.rs index 130716b..3e29c84 100644 --- a/src/app.rs +++ b/src/app.rs @@ -102,7 +102,7 @@ pub fn run(args: &Args) -> Result<(), anyhow::Error> { } let coordinator_thread: JoinHandle> = - thread::spawn(move || run_child_coordinator(child.job, &control_rx)); + thread::spawn(move || run_child_coordinator(&child.job, &control_rx)); coordinator_thread .join() diff --git a/src/control.rs b/src/control.rs index 1ed1e9d..dcab1cb 100644 --- a/src/control.rs +++ b/src/control.rs @@ -24,7 +24,7 @@ pub(crate) enum ControlMessage { reason = "Linting conflict with `rustc::unreachable_pub`." )] pub(crate) fn run_child_coordinator( - job: Job, + job: &Job, control_rx: &mpsc::Receiver, ) -> Result<(), anyhow::Error> { loop { From 874009a69e101b071ad9c9ee2f7219e7966f724d Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:36:54 +0200 Subject: [PATCH 48/54] fix: mark exported struct as non-exhaustive --- src/args.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/args.rs b/src/args.rs index e26ab86..08625fd 100644 --- a/src/args.rs +++ b/src/args.rs @@ -37,6 +37,7 @@ impl Env for ProcessEnv { /// ``` #[derive(Parser, Debug)] #[command(version, about, long_about = None)] +#[non_exhaustive] pub struct Args { /// The port to use for forwarding stdin and stdout. #[arg(long, default_value_t = 7000)] From de5290a6b38aa25d33f461819b9fcb852cab73cc Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:38:11 +0200 Subject: [PATCH 49/54] chore: remove deprecated clippy setting --- Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 27c69d9..bbe4793 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -185,7 +185,6 @@ same_name_method = "warn" self_named_module_files = "warn" str_to_string = "warn" string_add = "warn" -string_to_string = "allow" # Deprecated; keep allowed if older toolchains still mention it. unnecessary_self_imports = "warn" unseparated_literal_suffix = "warn" wildcard_enum_match_arm = "warn" From 9668c3f42adc7ddd69860165e8a05a918de3083c Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 07:41:14 +0200 Subject: [PATCH 50/54] refactor: switch to new modules layout --- Cargo.toml | 1 - src/{servers/mod.rs => servers.rs} | 0 2 files changed, 1 deletion(-) rename src/{servers/mod.rs => servers.rs} (100%) diff --git a/Cargo.toml b/Cargo.toml index bbe4793..3fb36c7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -182,7 +182,6 @@ missing_docs_in_private_items = "warn" pub_use = "warn" ref_patterns = "warn" same_name_method = "warn" -self_named_module_files = "warn" str_to_string = "warn" string_add = "warn" unnecessary_self_imports = "warn" diff --git a/src/servers/mod.rs b/src/servers.rs similarity index 100% rename from src/servers/mod.rs rename to src/servers.rs From ea20c2fce639c38b4208d9ab6bb750ae6a55b0f9 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 08:06:18 +0200 Subject: [PATCH 51/54] refactor: simplify construction `Duration` --- tests/integration_test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 13dc082..39a1bfd 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -1151,7 +1151,7 @@ fn test_works_with_various_executables() -> Result<(), anyhow::Error> { // Delay to ensure Python has started and produced output. thread::sleep(Duration::from_millis(300)); // Increased timeout to account for Python interpreter startup (especially on Windows). - let output = read_all_available(&mut stream, Duration::from_millis(3000))?; + let output = read_all_available(&mut stream, Duration::from_secs(3))?; assert!( String::from_utf8_lossy(&output).contains("test4"), "Expected 'test4' in output, got: {:?}", From 0933088836c94d89be939c44da92de0b3e7cc1d5 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 08:24:28 +0200 Subject: [PATCH 52/54] fix: correct windows-specific warnings Co-authored-by: Copilot --- tests/test_utils.rs | 125 ++++++++++++++++++-------------------------- 1 file changed, 51 insertions(+), 74 deletions(-) diff --git a/tests/test_utils.rs b/tests/test_utils.rs index ab9fe93..0eca61f 100644 --- a/tests/test_utils.rs +++ b/tests/test_utils.rs @@ -28,10 +28,10 @@ use tracing_subscriber as _; pub fn sleep_cmd(seconds: u32) -> (&'static str, Vec) { // Use ping as a sleep alternative on Windows // Pings localhost N+1 times with 1-second intervals (approximately N seconds total) - let pings = seconds + 1; + let pings = seconds.saturating_add(1); ( "ping", - vec!["-n".to_string(), pings.to_string(), "127.0.0.1".to_string()], + vec!["-n".to_owned(), pings.to_string(), "127.0.0.1".to_owned()], ) } @@ -44,13 +44,14 @@ pub fn sleep_cmd(seconds: u32) -> (&'static str, Vec) { /// Returns a command that echoes text to `stdout`, then sleeps. #[cfg(windows)] +#[must_use] pub fn echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Vec) { - let pings = seconds + 1; + let pings = seconds.saturating_add(1); ( "cmd", vec![ - "/C".to_string(), - format!("echo {} && ping -n {} 127.0.0.1 >nul", text, pings), + "/C".to_owned(), + format!("echo {text} && ping -n {pings} 127.0.0.1 >nul"), ], ) } @@ -61,10 +62,7 @@ pub fn echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Vec (&'static str, Vec) { ( "bash", - vec![ - "-c".to_owned(), - format!("echo '{}' && sleep {}", text, seconds), - ], + vec!["-c".to_owned(), format!("echo '{text}' && sleep {seconds}")], ) } @@ -72,12 +70,12 @@ pub fn echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Vec (&'static str, Vec) { - let pings = seconds + 1; + let pings = seconds.saturating_add(1); ( "cmd", vec![ - "/C".to_string(), - format!("echo {} 1>&2 && ping -n {} 127.0.0.1 >nul", text, pings), + "/C".to_owned(), + format!("echo {text} 1>&2 && ping -n {pings} 127.0.0.1 >nul"), ], ) } @@ -90,7 +88,7 @@ pub fn stderr_echo_with_sleep_cmd(text: &str, seconds: u32) -> (&'static str, Ve "bash", vec![ "-c".to_owned(), - format!("echo '{}' >&2 && sleep {}", text, seconds), + format!("echo '{text}' >&2 && sleep {seconds}"), ], ) } @@ -104,16 +102,14 @@ pub fn multi_echo_stderr_cmd( realtime: &str, sleep2: u32, ) -> (&'static str, Vec) { - let sleep1_ms = (sleep1 * 1000.0) as u32; - let pings = sleep2 + 1; + let pings = sleep2.saturating_add(1); ( "powershell", vec![ - "-NoProfile".to_string(), - "-Command".to_string(), + "-NoProfile".to_owned(), + "-Command".to_owned(), format!( - "[Console]::Error.WriteLine('{}'); Start-Sleep -Milliseconds {}; [Console]::Error.WriteLine('{}'); ping -n {} 127.0.0.1 >$null", - buffered, sleep1_ms, realtime, pings + "[Console]::Error.WriteLine('{buffered}'); Start-Sleep -Seconds {sleep1}; [Console]::Error.WriteLine('{realtime}'); ping -n {pings} 127.0.0.1 >$null" ), ], ) @@ -132,10 +128,7 @@ pub fn multi_echo_stderr_cmd( "bash", vec![ "-c".to_owned(), - format!( - "echo '{}' >&2; sleep {}; echo '{}' >&2; sleep {}", - buffered, sleep1, realtime, sleep2 - ), + format!("echo '{buffered}' >&2; sleep {sleep1}; echo '{realtime}' >&2; sleep {sleep2}"), ], ) } @@ -149,16 +142,14 @@ pub fn multi_echo_stdout_cmd( realtime: &str, sleep2: u32, ) -> (&'static str, Vec) { - let sleep1_ms = (sleep1 * 1000.0) as u32; - let pings = sleep2 + 1; + let pings = sleep2.saturating_add(1); ( "powershell", vec![ - "-NoProfile".to_string(), - "-Command".to_string(), + "-NoProfile".to_owned(), + "-Command".to_owned(), format!( - "Write-Output '{}'; Start-Sleep -Milliseconds {}; Write-Output '{}'; ping -n {} 127.0.0.1 >$null", - buffered, sleep1_ms, realtime, pings + "Write-Output '{buffered}'; Start-Sleep -Seconds {sleep1}; Write-Output '{realtime}'; ping -n {pings} 127.0.0.1 >$null" ), ], ) @@ -177,10 +168,7 @@ pub fn multi_echo_stdout_cmd( "bash", vec![ "-c".to_owned(), - format!( - "echo '{}'; sleep {}; echo '{}'; sleep {}", - buffered, sleep1, realtime, sleep2 - ), + format!("echo '{buffered}'; sleep {sleep1}; echo '{realtime}'; sleep {sleep2}",), ], ) } @@ -194,9 +182,9 @@ pub fn cat_cmd() -> (&'static str, Vec) { ( python_cmd(), vec![ - "-u".to_string(), - "-c".to_string(), - "import sys; [print(line.rstrip()) for line in sys.stdin]".to_string(), + "-u".to_owned(), + "-c".to_owned(), + "import sys; [print(line.rstrip()) for line in sys.stdin]".to_owned(), ], ) } @@ -217,10 +205,10 @@ pub fn loop_stdin_to_stdout_cmd() -> (&'static str, Vec) { ( "powershell", vec![ - "-NoProfile".to_string(), - "-Command".to_string(), + "-NoProfile".to_owned(), + "-Command".to_owned(), "while($line = [Console]::In.ReadLine()) { [Console]::WriteLine('response') }" - .to_string(), + .to_owned(), ], ) } @@ -245,10 +233,10 @@ pub fn continuous_stderr_loop_cmd() -> (&'static str, Vec) { ( "powershell", vec![ - "-NoProfile".to_string(), - "-Command".to_string(), + "-NoProfile".to_owned(), + "-Command".to_owned(), "while($true) { [Console]::Error.WriteLine('error'); Start-Sleep -Milliseconds 100 }" - .to_string(), + .to_owned(), ], ) } @@ -274,9 +262,9 @@ pub fn generate_large_output_cmd(size: usize) -> (&'static str, Vec) { ( "powershell", vec![ - "-NoProfile".to_string(), - "-Command".to_string(), - format!("'A' * {}; ping -n 11 127.0.0.1 >$null", size), + "-NoProfile".to_owned(), + "-Command".to_owned(), + format!("'A' * {size}; ping -n 11 127.0.0.1 >$null"), ], ) } @@ -289,7 +277,7 @@ pub fn generate_large_output_cmd(size: usize) -> (&'static str, Vec) { "bash", vec![ "-c".to_owned(), - format!("head -c {} /dev/zero | tr '\\0' 'A'; sleep 10", size), + format!("head -c {size} /dev/zero | tr '\\0' 'A'; sleep 10"), ], ) } @@ -301,11 +289,10 @@ pub fn numbered_output_loop_cmd(count: u32, interval_ms: u32) -> (&'static str, ( "powershell", vec![ - "-NoProfile".to_string(), - "-Command".to_string(), + "-NoProfile".to_owned(), + "-Command".to_owned(), format!( - "1..{} | ForEach-Object {{ Write-Output \"stdout_line_$_\"; [Console]::Error.WriteLine(\"stderr_line_$_\"); Start-Sleep -Milliseconds {} }}", - count, interval_ms + "1..{count} | ForEach-Object {{ Write-Output \"stdout_line_$_\"; [Console]::Error.WriteLine(\"stderr_line_$_\"); Start-Sleep -Milliseconds {interval_ms} }}" ), ], ) @@ -340,15 +327,15 @@ pub fn complex_stderr_reconnect_cmd() -> (&'static str, Vec) { ( "powershell", vec![ - "-NoProfile".to_string(), - "-Command".to_string(), + "-NoProfile".to_owned(), + "-Command".to_owned(), concat!( "[Console]::Error.WriteLine('before_connection'); Start-Sleep -Milliseconds 500; ", "[Console]::Error.WriteLine('during_first_connection'); Start-Sleep -Milliseconds 1000; ", "[Console]::Error.WriteLine('trigger_disconnect'); Start-Sleep -Milliseconds 1500; ", "[Console]::Error.WriteLine('while_disconnected'); Start-Sleep -Milliseconds 2000; ", "[Console]::Error.WriteLine('during_second_connection'); Start-Sleep -Seconds 10" - ).to_string(), + ).to_owned(), ], ) } @@ -381,15 +368,14 @@ pub fn combined_output_cmd( stderr_msg: &str, sleep_sec: u32, ) -> (&'static str, Vec) { - let pings = sleep_sec + 1; + let pings = sleep_sec.saturating_add(1); ( "powershell", vec![ - "-NoProfile".to_string(), - "-Command".to_string(), + "-NoProfile".to_owned(), + "-Command".to_owned(), format!( - "Write-Output '{}'; [Console]::Error.WriteLine('{}'); ping -n {} 127.0.0.1 >$null", - stdout_msg, stderr_msg, pings + "Write-Output '{stdout_msg}'; [Console]::Error.WriteLine('{stderr_msg}'); ping -n {pings} 127.0.0.1 >$null" ), ], ) @@ -407,10 +393,7 @@ pub fn combined_output_cmd( "bash", vec![ "-c".to_owned(), - format!( - "echo '{}'; echo '{}' >&2; sleep {}", - stdout_msg, stderr_msg, sleep_sec - ), + format!("echo '{stdout_msg}'; echo '{stderr_msg}' >&2; sleep {sleep_sec}"), ], ) } @@ -419,11 +402,11 @@ pub fn combined_output_cmd( #[cfg(windows)] #[must_use] pub fn echo_args_cmd(args: &[&str]) -> (&'static str, Vec) { - let mut cmd_args = vec!["/C".to_string()]; + let mut cmd_args = vec!["/C".to_owned()]; // Use echo %* to print all arguments on Windows (requires a batch context) // Alternative: build the echo command with all args let echo_str = args.join(" "); - cmd_args.push(format!("echo {} && ping -n 6 127.0.0.1 >nul", echo_str)); + cmd_args.push(format!("echo {echo_str} && ping -n 6 127.0.0.1 >nul")); ("cmd", cmd_args) } @@ -447,12 +430,9 @@ pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) ( "powershell", vec![ - "-NoProfile".to_string(), - "-Command".to_string(), - format!( - "Write-Output '{}'; Start-Sleep -Milliseconds {}", - msg, sleep_ms - ), + "-NoProfile".to_owned(), + "-Command".to_owned(), + format!("Write-Output '{msg}'; Start-Sleep -Milliseconds {sleep_ms}"), ], ) } @@ -470,10 +450,7 @@ pub fn short_lived_cmd(msg: &str, sleep_ms: u32) -> (&'static str, Vec) let sleep_arg = format!("{seconds}.{millis:03}"); ( "bash", - vec![ - "-c".to_owned(), - format!("echo {} && sleep {}", msg, sleep_arg), - ], + vec!["-c".to_owned(), format!("echo {msg} && sleep {sleep_arg}")], ) } From f89bd5f2bda847e2ca3266f2c2da5f16dd3fffb9 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 09:33:33 +0200 Subject: [PATCH 53/54] test: simplify test timing coordination Co-authored-by: Copilot --- tests/integration_test.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 39a1bfd..61a790e 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -1148,10 +1148,9 @@ fn test_works_with_various_executables() -> Result<(), anyhow::Error> { &["-u", "-c", "import time; print('test4'); time.sleep(2)"], )?; let mut stream = forwarder.connect_protocol()?; - // Delay to ensure Python has started and produced output. - thread::sleep(Duration::from_millis(300)); - // Increased timeout to account for Python interpreter startup (especially on Windows). - let output = read_all_available(&mut stream, Duration::from_secs(3))?; + + let read_timeout = Duration::from_secs(3); // Python startup (especially slow on Windows). + let output = read_all_available(&mut stream, read_timeout)?; assert!( String::from_utf8_lossy(&output).contains("test4"), "Expected 'test4' in output, got: {:?}", From 5d72aef5f811959933a08793a740abbf8efdcd52 Mon Sep 17 00:00:00 2001 From: Michael Gerhold Date: Tue, 28 Apr 2026 09:48:35 +0200 Subject: [PATCH 54/54] test: increase timeouts to make tests more stable in CI Co-authored-by: Copilot --- tests/integration_test.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 61a790e..ce73f38 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -790,7 +790,8 @@ fn test_protocol_port_buffered_stdout_replay() -> Result<(), anyhow::Error> { let mut stream = forwarder.connect_protocol()?; // Read the output. - let output = read_all_available(&mut stream, Duration::from_secs(3))?; + let read_timeout = Duration::from_secs(5); + let output = read_all_available(&mut stream, read_timeout)?; let output_str = String::from_utf8_lossy(&output); // Verify we got both buffered and realtime output. @@ -839,7 +840,8 @@ fn test_stderr_port_buffered_stderr_replay() -> Result<(), anyhow::Error> { let mut stream = forwarder.connect_stderr()?; // Read the output. - let output = read_all_available(&mut stream, Duration::from_secs(2))?; + let read_timeout = Duration::from_secs(5); + let output = read_all_available(&mut stream, read_timeout)?; let output_str = String::from_utf8_lossy(&output); // Verify we got both buffered and realtime output. @@ -1149,7 +1151,7 @@ fn test_works_with_various_executables() -> Result<(), anyhow::Error> { )?; let mut stream = forwarder.connect_protocol()?; - let read_timeout = Duration::from_secs(3); // Python startup (especially slow on Windows). + let read_timeout = Duration::from_secs(5); // Python startup (especially slow on Windows). let output = read_all_available(&mut stream, read_timeout)?; assert!( String::from_utf8_lossy(&output).contains("test4"), @@ -1214,7 +1216,8 @@ fn test_concurrent_stdin_stdout_bidirectional() -> Result<(), anyhow::Error> { .map_err(|error| anyhow::anyhow!("Failed to flush protocol port: {error}"))?; // Increased timeout for Python startup on Windows. - let output = read_all_available(&mut stream, Duration::from_millis(1500))?; + let read_timeout = Duration::from_secs(5); + let output = read_all_available(&mut stream, read_timeout)?; let output_str = String::from_utf8_lossy(&output); assert!( output_str.contains(&format!("line {i}")),