Skip to content

Commit 6ccee4f

Browse files
authored
chore: remove dead code and clear all clippy warnings (#268)
Removed genuinely dead code: - `InfoTable.connection` — populated but never read; dropped the field and its two construction sites. - `LoadManagedTableResponse.{connection_id, arrow_schema_json}` — deserialize-only fields never read. - `ConnectorEntry.file_format` — deserialize-only field never read. - `raw_http::TOKEN_REDACT_KEYS` — a dead duplicate of `jwt`'s live copy, used only by its own tautological test (removed both). Tightened suppressions: - Dropped two over-broad module-level `#![allow(dead_code)]` (raw_http, ingest) now that the modules are clean. - `IndexScope::delete_path` was `#[cfg_attr(not(test), allow(dead_code))]` — dead in production but shipped in the binary. Gated with `#[cfg(test)]` so it stays a path-shape regression guard without shipping. Cleared every clippy warning (0 across all targets): - Rewrote two `match … => false / _ => true` into `!matches!(…)` (skill). - Collapsed a nested `if let` into a let-chain (test helper). - Moved a mid-file `#[cfg(test)] mod tests` to the end (jobs). - Documented `#[allow]`s for the unavoidable ones: `large_enum_variant` on the two clap subcommand enums, `too_many_arguments` on an internal auth helper. Verified: 0 compiler warnings and 0 clippy warnings across all targets, fmt clean, tests pass, no unused dependencies. Net -23 lines. Co-authored-by: Eddie A Tejeda <669988+eddietejeda@users.noreply.github.com>
1 parent da2fd0b commit 6ccee4f

10 files changed

Lines changed: 49 additions & 72 deletions

File tree

‎src/cli.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ use crate::commands::skill::SkillCommands;
88
use crate::commands::workspace::WorkspaceCommands;
99
use clap::Subcommand;
1010

11+
// Variant sizes differ, but a clap command tree is parsed once per invocation
12+
// and boxing subcommand variants would complicate the derive and match arms.
13+
#[allow(clippy::large_enum_variant)]
1114
#[derive(Subcommand)]
1215
pub enum Commands {
1316
/// Authenticate or manage auth settings

‎src/client/ingest.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
//!
3333
//! The result-reading endpoints are intentionally absent — once a run lands,
3434
//! that path is the core `query`/`databases`/`results` commands.
35-
#![allow(dead_code)] // Response structs are read only through serde/printing.
3635
3736
use crate::client::jwt;
3837
use crate::config;
@@ -967,9 +966,6 @@ pub struct ConnectorEntry {
967966
/// Kafka entries: `kafka` or `debezium`.
968967
#[serde(default)]
969968
pub connector_type: Option<String>,
970-
/// Filesystem entries: the file format the entry is named for.
971-
#[serde(default)]
972-
pub file_format: Option<String>,
973969
/// Which keys belong in this entry's deliberately free-form config map.
974970
#[serde(default)]
975971
pub options_hint: Option<OptionsHint>,

‎src/client/raw_http.rs‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414
1515
// Not every helper here is wired to a call site yet; the allow keeps the build
1616
// warning-free.
17-
#![allow(dead_code)]
18-
1917
use std::time::Duration;
2018

2119
/// Cap on any single (non-upload) HTTP request. Connection create + synchronous
@@ -29,9 +27,6 @@ const HTTP_REQUEST_TIMEOUT: Duration = Duration::from_secs(300);
2927
/// server-side.
3028
const TCP_KEEPALIVE_INTERVAL: Duration = Duration::from_secs(30);
3129

32-
/// JSON keys whose values are redacted in debug request/response logging.
33-
pub const TOKEN_REDACT_KEYS: &[&str] = &["access_token", "refresh_token", "api_token", "code"];
34-
3530
/// A timeout-bounded blocking client for ordinary raw requests (token mints,
3631
/// session mints, arbitrary GETs).
3732
pub fn build_http_client() -> reqwest::blocking::Client {
@@ -50,12 +45,4 @@ mod tests {
5045
fn http_client_builds() {
5146
let _ = build_http_client();
5247
}
53-
54-
#[test]
55-
fn redact_keys_cover_token_fields() {
56-
// Guards against silently dropping a sensitive key from debug logs.
57-
assert!(TOKEN_REDACT_KEYS.contains(&"access_token"));
58-
assert!(TOKEN_REDACT_KEYS.contains(&"refresh_token"));
59-
assert!(TOKEN_REDACT_KEYS.contains(&"api_token"));
60-
}
6148
}

‎src/commands/auth.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,7 @@ fn is_already_signed_in(profile_config: &config::ProfileConfig) -> bool {
244244
/// 3. Opens the browser and waits for the OAuth/registration callback.
245245
/// 4. Calls `exchange(code, code_verifier, port)` to mint a JWT session.
246246
/// 5. Saves the session, prints `success_print`, and displays the workspace.
247+
#[allow(clippy::too_many_arguments)]
247248
fn run_browser_auth(
248249
profile_config: &config::ProfileConfig,
249250
opening_msg: &str,

‎src/commands/databases.rs‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -395,8 +395,6 @@ struct DatabaseAttachment {
395395

396396
#[derive(Deserialize)]
397397
struct InfoTable {
398-
#[allow(dead_code)]
399-
connection: String,
400398
schema: String,
401399
table: String,
402400
synced: bool,
@@ -426,13 +424,9 @@ struct CreateDatabaseResponse {
426424

427425
#[derive(Deserialize)]
428426
struct LoadManagedTableResponse {
429-
#[allow(dead_code)]
430-
connection_id: String,
431427
schema_name: String,
432428
table_name: String,
433429
row_count: u64,
434-
#[allow(dead_code)]
435-
arrow_schema_json: String,
436430
}
437431

438432
impl From<hotdata::models::DatabaseDetailResponse> for Database {
@@ -955,7 +949,6 @@ fn collect_tables(
955949
.tables
956950
.into_iter()
957951
.map(|t| InfoTable {
958-
connection: t.connection,
959952
schema: t.schema,
960953
table: t.table,
961954
synced: t.synced,
@@ -979,7 +972,6 @@ fn collect_tables(
979972
))
980973
.unwrap_or_else(|e| e.exit());
981974
out.extend(resp.tables.into_iter().map(|t| InfoTable {
982-
connection: t.connection,
983975
schema: t.schema,
984976
table: t.table,
985977
synced: t.synced,
@@ -2258,7 +2250,6 @@ mod tests {
22582250
let rows = table_rows(
22592251
"default",
22602252
vec![InfoTable {
2261-
connection: "ignored".into(),
22622253
schema: "public".into(),
22632254
table: "orders".into(),
22642255
synced: true,

‎src/commands/indexes.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -407,9 +407,10 @@ impl IndexScope<'_> {
407407
}
408408
}
409409

410-
// Retained for path-shape regression tests; delete now routes through the
411-
// SDK `indexes()` handle by scope variant rather than a formatted path.
412-
#[cfg_attr(not(test), allow(dead_code))]
410+
// Retained only for path-shape regression tests; delete now routes through
411+
// the SDK `indexes()` handle by scope variant rather than a formatted path.
412+
// `#[cfg(test)]` keeps it as a test guard without shipping it in the binary.
413+
#[cfg(test)]
413414
fn delete_path(&self, index_name: &str) -> String {
414415
match self {
415416
IndexScope::Connection {

‎src/commands/ingest.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ use crate::util;
7171
/// Wire values for the ingest type; the CLI spells the first one `one-time`.
7272
const TYPES: [&str; 3] = ["one-time", "scheduled", "continuous"];
7373

74+
// Parsed once per invocation; boxing clap variants isn't worth the churn.
75+
#[allow(clippy::large_enum_variant)]
7476
#[derive(clap::Subcommand)]
7577
pub enum IngestCommands {
7678
/// Create a load definition

‎src/commands/jobs.rs‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -138,37 +138,6 @@ pub fn get(job_id: &str, workspace_id: &str, format: &str) {
138138
}
139139
}
140140

141-
#[cfg(test)]
142-
mod tests {
143-
use super::*;
144-
use hotdata::models::ListJobsResponse;
145-
146-
// Regression for #160: a `managed_load` row must deserialize, not blow up
147-
// the whole `jobs list` response. Proves SDK 0.6.0's JobType carries the
148-
// variant the server emits for `databases load`.
149-
#[test]
150-
fn list_response_with_managed_load_deserializes() {
151-
let body = r#"{
152-
"jobs": [
153-
{
154-
"id": "job_1",
155-
"job_type": "managed_load",
156-
"status": "succeeded",
157-
"attempts": 1,
158-
"created_at": "2026-06-18T06:00:00Z"
159-
}
160-
]
161-
}"#;
162-
let resp: ListJobsResponse = serde_json::from_str(body).expect("managed_load must parse");
163-
assert_eq!(resp.jobs[0].job_type, JobType::ManagedLoad);
164-
}
165-
166-
#[test]
167-
fn parse_job_type_accepts_managed_load() {
168-
assert_eq!(parse_job_type("managed_load"), Some(JobType::ManagedLoad));
169-
}
170-
}
171-
172141
fn fetch_jobs(
173142
api: &Api,
174143
job_type: Option<&str>,
@@ -242,3 +211,34 @@ pub fn list(
242211
_ => unreachable!(),
243212
}
244213
}
214+
215+
#[cfg(test)]
216+
mod tests {
217+
use super::*;
218+
use hotdata::models::ListJobsResponse;
219+
220+
// Regression for #160: a `managed_load` row must deserialize, not blow up
221+
// the whole `jobs list` response. Proves SDK 0.6.0's JobType carries the
222+
// variant the server emits for `databases load`.
223+
#[test]
224+
fn list_response_with_managed_load_deserializes() {
225+
let body = r#"{
226+
"jobs": [
227+
{
228+
"id": "job_1",
229+
"job_type": "managed_load",
230+
"status": "succeeded",
231+
"attempts": 1,
232+
"created_at": "2026-06-18T06:00:00Z"
233+
}
234+
]
235+
}"#;
236+
let resp: ListJobsResponse = serde_json::from_str(body).expect("managed_load must parse");
237+
assert_eq!(resp.jobs[0].job_type, JobType::ManagedLoad);
238+
}
239+
240+
#[test]
241+
fn parse_job_type_accepts_managed_load() {
242+
assert_eq!(parse_job_type("managed_load"), Some(JobType::ManagedLoad));
243+
}
244+
}

‎src/commands/skill.rs‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -146,10 +146,8 @@ pub fn maybe_auto_update_after_cli_upgrade() {
146146
}
147147

148148
let current = Version::parse(CURRENT_VERSION).expect("invalid package version");
149-
let needs_refresh = match read_installed_version() {
150-
Some(v) if v >= current && all_skill_stores_present() => false,
151-
_ => true,
152-
};
149+
let needs_refresh =
150+
!matches!(read_installed_version(), Some(v) if v >= current && all_skill_stores_present());
153151
if !needs_refresh {
154152
clear_skill_auto_update_suppression();
155153
return;
@@ -170,10 +168,8 @@ pub fn maybe_auto_update_after_cli_upgrade() {
170168
let _symlinks = ensure_symlinks();
171169
remove_retired_skills_global();
172170

173-
let still_needed = match read_installed_version() {
174-
Some(v) if v >= current && all_skill_stores_present() => false,
175-
_ => true,
176-
};
171+
let still_needed =
172+
!matches!(read_installed_version(), Some(v) if v >= current && all_skill_stores_present());
177173

178174
if still_needed {
179175
suppress_skill_auto_update_for_this_cli();

‎tests/common/mod.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,10 +204,10 @@ pub fn shared_database_id(cli: &Cli) -> String {
204204
let listing = cli.json(&["databases", "list", "-o", "json"]);
205205
if let Some(arr) = listing.as_array() {
206206
for db in arr {
207-
if db.get("name").and_then(|v| v.as_str()) == Some(SHARED_DATABASE_NAME) {
208-
if let Some(id) = db.get("id").and_then(|v| v.as_str()) {
209-
return id.to_string();
210-
}
207+
if db.get("name").and_then(|v| v.as_str()) == Some(SHARED_DATABASE_NAME)
208+
&& let Some(id) = db.get("id").and_then(|v| v.as_str())
209+
{
210+
return id.to_string();
211211
}
212212
}
213213
}

0 commit comments

Comments
 (0)