Skip to content

Commit 2472474

Browse files
drewdrew
authored
ci(rust): enforce -D warnings on clippy (#1008)
1 parent 2adddaa commit 2472474

129 files changed

Lines changed: 2007 additions & 1906 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎crates/openshell-bootstrap/src/docker.rs‎

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const REGISTRY_NAMESPACE_DEFAULT: &str = "openshell";
2828
/// `connect_with_local_defaults()` ceiling is 120s, which is far too short for
2929
/// multi-GB image exports — a 7 GB image on a laptop SSD takes ~4–5 minutes.
3030
/// One hour is a safe upper bound; override with `OPENSHELL_DOCKER_TIMEOUT_SECS`.
31-
pub(crate) const DEFAULT_LARGE_TRANSFER_TIMEOUT_SECS: u64 = 3600;
31+
pub const DEFAULT_LARGE_TRANSFER_TIMEOUT_SECS: u64 = 3600;
3232

3333
/// Build a local-Docker client suitable for large streaming transfers.
3434
/// Respects `OPENSHELL_DOCKER_TIMEOUT_SECS` (in seconds); falls back to
@@ -50,7 +50,7 @@ pub fn connect_local_for_large_transfers() -> std::result::Result<Docker, Bollar
5050
/// | `["legacy"]` | `["legacy"]` — pass through to the non-CDI fallback path |
5151
/// | `["auto"]` | `["nvidia.com/gpu=all"]` if CDI enabled, else `["legacy"]` |
5252
/// | `[cdi-ids…]` | unchanged |
53-
pub(crate) fn resolve_gpu_device_ids(gpu: &[String], cdi_enabled: bool) -> Vec<String> {
53+
pub fn resolve_gpu_device_ids(gpu: &[String], cdi_enabled: bool) -> Vec<String> {
5454
match gpu {
5555
[] => vec![],
5656
[v] if v == "auto" => {
@@ -346,22 +346,25 @@ pub async fn find_gateway_container(docker: &Docker, port: Option<u16>) -> Resul
346346

347347
let matches: Vec<String> = containers
348348
.iter()
349-
.filter(|c| is_gateway_image(c) && port.map_or(true, |p| has_port(c, p)))
349+
.filter(|c| is_gateway_image(c) && port.is_none_or(|p| has_port(c, p)))
350350
.filter_map(container_name)
351351
.collect();
352352

353353
match matches.len() {
354354
0 => {
355-
let hint = if let Some(p) = port {
356-
format!(
357-
"No openshell gateway container found listening on port {p}.\n\
355+
let hint = port.map_or_else(
356+
|| {
357+
"No openshell gateway container found.\n\
358358
Is the gateway running? Check with: docker ps"
359-
)
360-
} else {
361-
"No openshell gateway container found.\n\
362-
Is the gateway running? Check with: docker ps"
363-
.to_string()
364-
};
359+
.to_string()
360+
},
361+
|p| {
362+
format!(
363+
"No openshell gateway container found listening on port {p}.\n\
364+
Is the gateway running? Check with: docker ps"
365+
)
366+
},
367+
);
365368
Err(miette::miette!("{hint}"))
366369
}
367370
1 => Ok(matches.into_iter().next().unwrap()),
@@ -488,6 +491,8 @@ pub async fn ensure_image(
488491
/// Returns the actual host port the container is using. When an existing
489492
/// container is reused (same image), this may differ from `gateway_port`
490493
/// because the container was originally created with a different port.
494+
// Refactoring this signature would touch many call sites across the workspace.
495+
#[allow(clippy::too_many_arguments)]
491496
pub async fn ensure_container(
492497
docker: &Docker,
493498
name: &str,
@@ -744,10 +749,7 @@ pub async fn ensure_container(
744749
// When OPENSHELL_PUSH_IMAGES is set the entrypoint overrides the baked-in
745750
// HelmChart manifest so k3s uses the locally-pushed images with
746751
// IfNotPresent pull policy instead of pulling from the remote registry.
747-
let push_mode = std::env::var("OPENSHELL_PUSH_IMAGES")
748-
.ok()
749-
.filter(|v| !v.trim().is_empty())
750-
.is_some();
752+
let push_mode = std::env::var("OPENSHELL_PUSH_IMAGES").is_ok_and(|v| !v.trim().is_empty());
751753
let effective_tag = std::env::var("IMAGE_TAG")
752754
.ok()
753755
.filter(|v| !v.trim().is_empty())
@@ -863,22 +865,22 @@ pub async fn check_port_conflicts(
863865

864866
let ports = container.ports.as_deref().unwrap_or_default();
865867
for port in ports {
866-
if let Some(public) = port.public_port {
867-
if needed_ports.contains(&public) {
868-
let cname = names
869-
.first()
870-
.map(|n| n.trim_start_matches('/').to_string())
871-
.unwrap_or_else(|| {
872-
container
873-
.id
874-
.clone()
875-
.unwrap_or_else(|| "<unknown>".to_string())
876-
});
877-
conflicts.push(PortConflict {
878-
container_name: cname,
879-
host_port: public,
880-
});
881-
}
868+
if let Some(public) = port.public_port
869+
&& needed_ports.contains(&public)
870+
{
871+
let cname = names.first().map_or_else(
872+
|| {
873+
container
874+
.id
875+
.clone()
876+
.unwrap_or_else(|| "<unknown>".to_string())
877+
},
878+
|n| n.trim_start_matches('/').to_string(),
879+
);
880+
conflicts.push(PortConflict {
881+
container_name: cname,
882+
host_port: public,
883+
});
882884
}
883885
}
884886
}
@@ -1371,6 +1373,9 @@ mod tests {
13711373
);
13721374
}
13731375

1376+
// Test-only: mutates DOCKER_HOST env var via std::env::set_var/remove_var,
1377+
// which require unsafe in the 2024 edition.
1378+
#[allow(unsafe_code)]
13741379
#[test]
13751380
fn docker_not_reachable_error_with_docker_host() {
13761381
// Simulate: DOCKER_HOST is set but daemon unresponsive.

‎crates/openshell-bootstrap/src/lib.rs‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ pub struct DeployOptions {
117117
/// - `[]` — no GPU passthrough (default)
118118
/// - `["legacy"]` — internal non-CDI fallback path (`driver="nvidia"`, `count=-1`)
119119
/// - `["auto"]` — resolved at deploy time: CDI if enabled on the daemon, else the non-CDI fallback
120-
/// - `[cdi-ids…]` — CDI DeviceRequest with the given device IDs
120+
/// - `[cdi-ids…]` — CDI `DeviceRequest` with the given device IDs
121121
pub gpu: Vec<String>,
122122
/// When true, destroy any existing gateway resources before deploying.
123123
/// When false, an existing gateway is left as-is and deployment is
@@ -340,8 +340,8 @@ where
340340
// the image to recreate the container, so the pull must happen.
341341
let need_image = !resume || !resume_container_exists;
342342
if need_image {
343+
log("[status] Downloading gateway".to_string());
343344
if remote_opts.is_some() {
344-
log("[status] Downloading gateway".to_string());
345345
let on_log_clone = Arc::clone(&on_log);
346346
let progress_cb = move |msg: String| {
347347
if let Ok(mut f) = on_log_clone.lock() {
@@ -358,7 +358,6 @@ where
358358
.await?;
359359
} else {
360360
// Local deployment: ensure image exists (pull if needed)
361-
log("[status] Downloading gateway".to_string());
362361
ensure_image(
363362
&target_docker,
364363
&image_ref,
@@ -732,16 +731,16 @@ pub async fn gateway_container_logs<W: std::io::Write>(
732731
Ok(())
733732
}
734733

735-
/// Fetch the last `n` lines of container logs for a local gateway as a
736-
/// `String`. This is a convenience wrapper for diagnostic call sites (e.g.
737-
/// failure diagnosis in the CLI) that do not hold a Docker client handle.
734+
/// Fetch the last `n` lines of container logs for a local gateway as a `String`.
735+
///
736+
/// This is a convenience wrapper for diagnostic call sites (e.g. failure
737+
/// diagnosis in the CLI) that do not hold a Docker client handle.
738738
///
739739
/// Returns an empty string on any Docker/connection error so callers don't
740740
/// need to worry about error handling.
741741
pub async fn fetch_gateway_logs(name: &str, n: usize) -> String {
742-
let docker = match Docker::connect_with_local_defaults() {
743-
Ok(d) => d,
744-
Err(_) => return String::new(),
742+
let Ok(docker) = Docker::connect_with_local_defaults() else {
743+
return String::new();
745744
};
746745
let container = container_name(name);
747746
fetch_recent_logs(&docker, &container, n).await

‎crates/openshell-bootstrap/src/metadata.rs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -305,12 +305,11 @@ pub fn load_last_sandbox(gateway: &str) -> Option<String> {
305305
/// This should be called after a sandbox is deleted so that subsequent commands
306306
/// don't try to connect to a sandbox that no longer exists.
307307
pub fn clear_last_sandbox_if_matches(gateway: &str, sandbox: &str) {
308-
if let Some(current) = load_last_sandbox(gateway) {
309-
if current == sandbox {
310-
if let Ok(path) = last_sandbox_path(gateway) {
311-
let _ = std::fs::remove_file(path);
312-
}
313-
}
308+
if let Some(current) = load_last_sandbox(gateway)
309+
&& current == sandbox
310+
&& let Ok(path) = last_sandbox_path(gateway)
311+
{
312+
let _ = std::fs::remove_file(path);
314313
}
315314
}
316315

‎crates/openshell-cli/src/auth.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ fn generate_confirmation_code() -> String {
7272
let hash_b = hasher_b.finish();
7373

7474
prev_hash = hash_b;
75+
// hash_b is `u64`; truncation to `usize` is acceptable here since we mod
76+
// by charset.len() (small) and only use it as an index.
77+
#[allow(clippy::cast_possible_truncation)]
7578
let idx = (hash_b as usize) % charset.len();
7679
code.push(charset[idx] as char);
7780
}
@@ -91,8 +94,7 @@ pub async fn browser_auth_flow(gateway_endpoint: &str) -> Result<String> {
9194
// listener, spawns a callback server, and waits the full AUTH_TIMEOUT
9295
// (120 s) for a POST that will never arrive.
9396
let no_browser = std::env::var("OPENSHELL_NO_BROWSER")
94-
.map(|v| v == "1" || v.eq_ignore_ascii_case("true"))
95-
.unwrap_or(false);
97+
.is_ok_and(|v| v == "1" || v.eq_ignore_ascii_case("true"));
9698
if no_browser {
9799
return Err(miette::miette!(
98100
"authentication skipped (OPENSHELL_NO_BROWSER is set).\n\
@@ -129,8 +131,9 @@ pub async fn browser_auth_flow(gateway_endpoint: &str) -> Result<String> {
129131

130132
eprint!("Press Enter to open the browser for authentication...");
131133
std::io::stderr().flush().ok();
132-
let mut _input = String::new();
133-
std::io::stdin().read_line(&mut _input).ok();
134+
let mut input = String::new();
135+
std::io::stdin().read_line(&mut input).ok();
136+
drop(input);
134137

135138
if let Err(e) = open_browser(&auth_url) {
136139
debug!(error = %e, "failed to open browser");

‎crates/openshell-cli/src/bootstrap.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ fn is_connectivity_error(error: &miette::Report) -> bool {
101101
/// `false` to skip bootstrap. Otherwise returns `true` — a gateway is created
102102
/// automatically without prompting the user.
103103
pub fn confirm_bootstrap(override_value: Option<bool>) -> Result<bool> {
104-
if let Some(false) = override_value {
104+
if override_value == Some(false) {
105105
return Ok(false);
106106
}
107107
Ok(true)

‎crates/openshell-cli/src/completers.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -188,11 +188,11 @@ mod tests {
188188
.unwrap();
189189

190190
let result = complete_gateway_names(OsStr::new("a"));
191-
let names: Vec<String> = result
192-
.iter()
193-
.map(|candidate| candidate.get_value().to_string_lossy().into_owned())
194-
.collect();
195-
assert!(names.contains(&"alpha".to_string()));
191+
assert!(
192+
result
193+
.iter()
194+
.any(|candidate| candidate.get_value().to_string_lossy() == "alpha")
195+
);
196196
});
197197
}
198198
}

‎crates/openshell-cli/src/main.rs‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ fn resolve_sandbox_name(name: Option<String>, gateway: &str) -> Result<String> {
151151
Specify a sandbox name or connect to one first: nav sandbox connect <name>"
152152
)
153153
})?;
154-
eprintln!("{} Using sandbox '{}' (last used)", "→".bold(), last.bold(),);
154+
eprintln!("{} Using sandbox '{}' (last used)", "→".bold(), last.bold());
155155
Ok(last)
156156
}
157157

@@ -816,7 +816,7 @@ enum GatewayCommands {
816816
/// `nvidia.com/gpu` resources. Requires NVIDIA drivers and the
817817
/// NVIDIA Container Toolkit on the host.
818818
///
819-
/// When enabled, OpenShell auto-selects CDI when the Docker daemon has
819+
/// When enabled, `OpenShell` auto-selects CDI when the Docker daemon has
820820
/// CDI enabled and falls back to Docker's NVIDIA GPU request path
821821
/// (`--gpus all`) otherwise.
822822
#[arg(long)]
@@ -1163,7 +1163,7 @@ enum SandboxCommands {
11631163
policy: Option<String>,
11641164

11651165
/// Forward a local port to the sandbox before the initial command or shell starts.
1166-
/// Accepts [bind_address:]port (e.g. 8080, 0.0.0.0:8080). Keeps the sandbox alive.
1166+
/// Accepts [`bind_address`:]port (e.g. 8080, 0.0.0.0:8080). Keeps the sandbox alive.
11671167
#[arg(long, conflicts_with = "no_keep")]
11681168
forward: Option<String>,
11691169

@@ -1472,11 +1472,11 @@ enum PolicyCommands {
14721472
#[arg(long = "remove-endpoint")]
14731473
remove_endpoints: Vec<String>,
14741474

1475-
/// Add a REST allow rule: host:port:METHOD:path_glob.
1475+
/// Add a REST allow rule: `host:port:METHOD:path_glob`.
14761476
#[arg(long = "add-allow")]
14771477
add_allow: Vec<String>,
14781478

1479-
/// Add a REST deny rule: host:port:METHOD:path_glob.
1479+
/// Add a REST deny rule: `host:port:METHOD:path_glob`.
14801480
#[arg(long = "add-deny")]
14811481
add_deny: Vec<String>,
14821482

@@ -1556,7 +1556,7 @@ enum PolicyCommands {
15561556
/// Prove properties of a sandbox policy — or find counterexamples.
15571557
#[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")]
15581558
Prove {
1559-
/// Path to OpenShell sandbox policy YAML.
1559+
/// Path to `OpenShell` sandbox policy YAML.
15601560
#[arg(long, value_hint = ValueHint::FilePath)]
15611561
policy: String,
15621562

@@ -1646,7 +1646,7 @@ enum ForwardCommands {
16461646
/// Start forwarding a local port to a sandbox.
16471647
#[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")]
16481648
Start {
1649-
/// Port to forward: [bind_address:]port (e.g. 8080, 0.0.0.0:8080).
1649+
/// Port to forward: [`bind_address`:]port (e.g. 8080, 0.0.0.0:8080).
16501650
port: String,
16511651

16521652
/// Sandbox name (defaults to last-used sandbox).
@@ -1675,6 +1675,7 @@ enum ForwardCommands {
16751675
}
16761676

16771677
#[tokio::main]
1678+
#[allow(clippy::large_stack_frames)] // CLI dispatch holds many futures; OK at top level.
16781679
async fn main() -> Result<()> {
16791680
// Install the rustls crypto provider before completion runs — completers may
16801681
// establish TLS connections to the gateway.
@@ -1744,7 +1745,7 @@ async fn main() -> Result<()> {
17441745
} else {
17451746
vec![]
17461747
};
1747-
run::gateway_admin_deploy(
1748+
Box::pin(run::gateway_admin_deploy(
17481749
&name,
17491750
remote.as_deref(),
17501751
ssh_key.as_deref(),
@@ -1756,7 +1757,7 @@ async fn main() -> Result<()> {
17561757
registry_username.as_deref(),
17571758
registry_token.as_deref(),
17581759
gpu,
1759-
)
1760+
))
17601761
.await?;
17611762
}
17621763
GatewayCommands::Stop {
@@ -1873,7 +1874,7 @@ async fn main() -> Result<()> {
18731874
} else {
18741875
println!("{}", "Gateway Status".cyan().bold());
18751876
println!();
1876-
println!(" {} No gateway configured.", "Status:".dimmed(),);
1877+
println!(" {} No gateway configured.", "Status:".dimmed());
18771878
println!();
18781879
println!(
18791880
"Deploy a gateway with: {}",
@@ -1891,16 +1892,15 @@ async fn main() -> Result<()> {
18911892
ForwardCommands::Stop { port, name } => {
18921893
let name = match name {
18931894
Some(n) => n,
1894-
None => match run::find_forward_by_port(port)? {
1895-
Some(n) => {
1895+
None => {
1896+
if let Some(n) = run::find_forward_by_port(port)? {
18961897
eprintln!("→ Found forward on sandbox '{n}'");
18971898
n
1898-
}
1899-
None => {
1900-
eprintln!("{} No active forward found for port {port}", "!".yellow(),);
1899+
} else {
1900+
eprintln!("{} No active forward found for port {port}", "!".yellow());
19011901
return Ok(());
19021902
}
1903-
},
1903+
}
19041904
};
19051905
if run::stop_forward(&name, port)? {
19061906
eprintln!(
@@ -3280,7 +3280,7 @@ mod tests {
32803280
});
32813281
}
32823282

3283-
/// Verify the flag names the TUI uses to build its ProxyCommand are
3283+
/// Verify the flag names the TUI uses to build its `ProxyCommand` are
32843284
/// accepted by the `SshProxy` subcommand and land in the right fields.
32853285
/// This catches drift when CLI flags are renamed or restructured.
32863286
#[test]

0 commit comments

Comments
 (0)