Skip to content

Commit ba3b2e5

Browse files
committed
cli: Enable workspace linting and fix clippy violations
Add [workspace.lints] configuration to enforce clippy::all, clippy::pedantic, and deny all warnings. Fix resulting violations across the codebase
1 parent 7333eeb commit ba3b2e5

25 files changed

+328
-404
lines changed

cli/Cargo.toml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,13 @@ regex = "1"
3939

4040
[target.'cfg(unix)'.dev-dependencies]
4141
libc = "0.2"
42+
43+
[lints]
44+
workspace = true
45+
46+
[workspace.lints.clippy]
47+
all = { level = "deny" }
48+
pedantic = { level = "deny" }
49+
50+
[workspace.lints.rust]
51+
warnings = "deny"

cli/build.rs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use std::{
2-
env, fs,
3-
io::{self, Write},
2+
env,
3+
fmt::Write,
4+
fs,
5+
io::{self, Write as IoWrite},
46
path::{Path, PathBuf},
57
};
68

@@ -35,12 +37,12 @@ fn main() {
3537
}
3638

3739
fn generate_embedded_asset_manifest() -> io::Result<()> {
38-
let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").map_err(invalid_data)?);
40+
let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").map_err(|e| invalid_data(&e))?);
3941
let repository_root = manifest_dir
4042
.parent()
41-
.ok_or_else(|| invalid_data("CARGO_MANIFEST_DIR does not have a parent"))?
43+
.ok_or_else(|| invalid_data(&"CARGO_MANIFEST_DIR does not have a parent"))?
4244
.to_path_buf();
43-
let out_dir = PathBuf::from(env::var("OUT_DIR").map_err(invalid_data)?);
45+
let out_dir = PathBuf::from(env::var("OUT_DIR").map_err(|e| invalid_data(&e))?);
4446
let destination_path = out_dir.join("setup_embedded_assets.rs");
4547

4648
let mut output = String::new();
@@ -53,19 +55,21 @@ fn generate_embedded_asset_manifest() -> io::Result<()> {
5355
collect_files(&source_root, &source_root, &mut files)?;
5456
files.sort_unstable_by(|a, b| a.relative_path.cmp(&b.relative_path));
5557

56-
output.push_str(&format!(
57-
"pub static {}: &[EmbeddedAsset] = &[\n",
58+
let _ = writeln!(
59+
output,
60+
"pub static {}: &[EmbeddedAsset] = &[",
5861
target.const_name
59-
));
62+
);
6063

6164
for file in &files {
6265
println!("cargo:rerun-if-changed={}", file.absolute_path.display());
63-
output.push_str(&format!(
64-
" EmbeddedAsset {{ relative_path: \"{}\", bytes: include_bytes!(concat!(env!(\"CARGO_MANIFEST_DIR\"), \"{}{}\")) }},\n",
66+
let _ = writeln!(
67+
output,
68+
" EmbeddedAsset {{ relative_path: \"{}\", bytes: include_bytes!(concat!(env!(\"CARGO_MANIFEST_DIR\"), \"{}{}\")) }},",
6569
escape_for_rust_string(&file.relative_path),
6670
target.include_prefix,
6771
escape_for_rust_string(&file.relative_path),
68-
));
72+
);
6973
}
7074

7175
output.push_str("];\n\n");
@@ -97,7 +101,7 @@ fn collect_files(
97101

98102
let relative_path = path
99103
.strip_prefix(base_root)
100-
.map_err(|_| invalid_data("failed to strip source root from file path"))?;
104+
.map_err(|_| invalid_data(&"failed to strip source root from file path"))?;
101105

102106
let relative_path = normalize_relative_path(relative_path)?;
103107

@@ -113,15 +117,15 @@ fn collect_files(
113117
fn normalize_relative_path(path: &Path) -> io::Result<String> {
114118
let normalized = path
115119
.to_str()
116-
.ok_or_else(|| invalid_data("non-UTF-8 config paths are not supported"))?
120+
.ok_or_else(|| invalid_data(&"non-UTF-8 config paths are not supported"))?
117121
.replace('\\', "/");
118122

119123
if normalized.is_empty() {
120-
return Err(invalid_data("relative path cannot be empty"));
124+
return Err(invalid_data(&"relative path cannot be empty"));
121125
}
122126

123127
if normalized.starts_with('/') {
124-
return Err(invalid_data("relative path must not start with '/'"));
128+
return Err(invalid_data(&"relative path must not start with '/'"));
125129
}
126130

127131
Ok(normalized)
@@ -131,6 +135,6 @@ fn escape_for_rust_string(value: &str) -> String {
131135
value.replace('\\', "\\\\").replace('"', "\\\"")
132136
}
133137

134-
fn invalid_data<E: ToString>(error: E) -> io::Error {
138+
fn invalid_data<E: ToString>(error: &E) -> io::Error {
135139
io::Error::new(io::ErrorKind::InvalidData, error.to_string())
136140
}

cli/src/app.rs

Lines changed: 20 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ where
310310
format: services::version::VersionFormat::Text,
311311
}));
312312
}
313-
return Err(classify_clap_error(error));
313+
return Err(classify_clap_error(&error));
314314
}
315315
};
316316

@@ -323,42 +323,28 @@ where
323323
convert_clap_command(command)
324324
}
325325

326-
/// Classify a clap error into our ClassifiedError taxonomy.
327-
fn classify_clap_error(error: clap::Error) -> ClassifiedError {
326+
/// Classify a clap error into our `ClassifiedError` taxonomy.
327+
fn classify_clap_error(error: &clap::Error) -> ClassifiedError {
328328
use clap::error::ErrorKind;
329329

330330
let message = error.to_string();
331331

332332
// Determine error class based on clap error kind
333+
// Note: Many clap error kinds map to Parse failures
333334
let class = match error.kind() {
334-
// Parse errors: unknown commands, unknown arguments, missing arguments
335-
ErrorKind::InvalidSubcommand
336-
| ErrorKind::UnknownArgument
337-
| ErrorKind::InvalidValue
338-
| ErrorKind::ValueValidation
339-
| ErrorKind::TooFewValues
340-
| ErrorKind::TooManyValues
341-
| ErrorKind::WrongNumberOfValues => FailureClass::Parse,
342-
343335
// Validation errors: missing required arguments, argument conflicts
344336
ErrorKind::MissingRequiredArgument | ErrorKind::ArgumentConflict | ErrorKind::NoEquals => {
345337
FailureClass::Validation
346338
}
347339

348-
// Display errors (help, version) - treat as parse since user asked for something invalid
349-
ErrorKind::DisplayHelp
350-
| ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand
351-
| ErrorKind::DisplayVersion => FailureClass::Parse,
352-
353-
// Default to parse for any other error types
340+
// All other errors (parse errors, display errors, etc.) map to Parse
354341
_ => FailureClass::Parse,
355342
};
356343

357344
// Clean up clap's error message to match our style
358345
let cleaned_message = clean_clap_error_message(&message, error.kind());
359346

360347
match class {
361-
FailureClass::Parse => ClassifiedError::parse(cleaned_message),
362348
FailureClass::Validation => ClassifiedError::validation(cleaned_message),
363349
_ => ClassifiedError::parse(cleaned_message),
364350
}
@@ -377,55 +363,49 @@ fn clean_clap_error_message(message: &str, kind: clap::error::ErrorKind) -> Stri
377363
if let Some(subcommand) = extract_quoted_value(message) {
378364
if command_surface::is_known_command(&subcommand) {
379365
format!(
380-
"Command '{}' is currently unavailable in this build. Try: run 'sce --help' to see available commands in this build.",
381-
subcommand
366+
"Command '{subcommand}' is currently unavailable in this build. Try: run 'sce --help' to see available commands in this build."
382367
)
383368
} else {
384369
format!(
385-
"Unknown command '{}'. Try: run 'sce --help' to list valid commands, then rerun with a valid command such as 'sce version' or 'sce setup --help'.",
386-
subcommand
370+
"Unknown command '{subcommand}'. Try: run 'sce --help' to list valid commands, then rerun with a valid command such as 'sce version' or 'sce setup --help'."
387371
)
388372
}
389373
} else {
390-
format!("{}. Try: run 'sce --help' to see valid usage.", message)
374+
format!("{message}. Try: run 'sce --help' to see valid usage.")
391375
}
392376
}
393377
ErrorKind::UnknownArgument => {
394378
// Extract the unknown argument and provide helpful guidance
395379
if let Some(arg) = extract_quoted_value(message) {
396380
format!(
397-
"Unknown option '{}'. Try: run 'sce --help' to see top-level usage, or use 'sce <command> --help' for command-specific options.",
398-
arg
381+
"Unknown option '{arg}'. Try: run 'sce --help' to see top-level usage, or use 'sce <command> --help' for command-specific options."
399382
)
400383
} else {
401-
format!("{}. Try: run 'sce --help' to see valid usage.", message)
384+
format!("{message}. Try: run 'sce --help' to see valid usage.")
402385
}
403386
}
404387
ErrorKind::MissingRequiredArgument => {
405388
// Clean up clap's message for missing required arguments
406389
if message.contains("required") {
407-
format!(
408-
"{}. Try: run 'sce --help' to see required arguments.",
409-
message
410-
)
390+
format!("{message}. Try: run 'sce --help' to see required arguments.")
411391
} else {
412-
format!("{}. Try: run 'sce --help' to see valid usage.", message)
392+
format!("{message}. Try: run 'sce --help' to see valid usage.")
413393
}
414394
}
415395
ErrorKind::ArgumentConflict => {
416396
// Handle mutually exclusive arguments
417397
if message.contains("cannot be used with") || message.contains("conflicts with") {
418-
format!("{}. Try: use only one of the conflicting options.", message)
398+
format!("{message}. Try: use only one of the conflicting options.")
419399
} else {
420-
format!("{}. Try: run 'sce --help' to see valid usage.", message)
400+
format!("{message}. Try: run 'sce --help' to see valid usage.")
421401
}
422402
}
423403
_ => {
424404
// Default cleanup: ensure message ends with guidance
425405
if message.contains("Try:") {
426406
message.to_string()
427407
} else {
428-
format!("{}. Try: run 'sce --help' to see valid usage.", message)
408+
format!("{message}. Try: run 'sce --help' to see valid usage.")
429409
}
430410
}
431411
}
@@ -477,6 +457,7 @@ fn convert_clap_command(command: cli_schema::Commands) -> Result<Command, Classi
477457
}
478458
}
479459

460+
#[allow(clippy::unnecessary_wraps, clippy::needless_pass_by_value)]
480461
fn convert_auth_subcommand(
481462
subcommand: cli_schema::AuthSubcommand,
482463
) -> Result<Command, ClassifiedError> {
@@ -525,6 +506,7 @@ fn convert_completion_shell(
525506
}
526507

527508
/// Convert clap config subcommand to service config subcommand.
509+
#[allow(clippy::unnecessary_wraps)]
528510
fn convert_config_subcommand(
529511
subcommand: cli_schema::ConfigSubcommand,
530512
) -> Result<Command, ClassifiedError> {
@@ -568,7 +550,8 @@ fn convert_log_level(level: cli_schema::LogLevel) -> services::config::LogLevel
568550
}
569551
}
570552

571-
/// Convert setup command flags to SetupRequest.
553+
/// Convert setup command flags to `SetupRequest`.
554+
#[allow(clippy::fn_params_excessive_bools)]
572555
fn convert_setup_command(
573556
opencode: bool,
574557
claude: bool,
@@ -595,6 +578,7 @@ fn convert_setup_command(
595578
}
596579

597580
/// Convert clap hooks subcommand to service hooks subcommand.
581+
#[allow(clippy::unnecessary_wraps)]
598582
fn convert_hooks_subcommand(
599583
subcommand: cli_schema::HooksSubcommand,
600584
) -> Result<Command, ClassifiedError> {

cli/src/cli_schema.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ pub struct Cli {
1616

1717
impl Cli {
1818
/// Parse arguments from an iterator of strings
19+
#[allow(dead_code)]
1920
pub fn parse_from<I, T>(args: I) -> Self
2021
where
2122
I: IntoIterator<Item = T>,
@@ -36,7 +37,7 @@ impl Cli {
3637

3738
#[derive(Subcommand, Debug, Clone, PartialEq, Eq)]
3839
pub enum Commands {
39-
/// Authenticate with WorkOS device authorization flow
40+
/// Authenticate with `WorkOS` device authorization flow
4041
Auth {
4142
#[command(subcommand)]
4243
subcommand: AuthSubcommand,
@@ -51,15 +52,15 @@ pub enum Commands {
5152
/// Prepare local repository/workspace prerequisites
5253
#[command(about = "Prepare local repository/workspace prerequisites")]
5354
Setup {
54-
/// Install OpenCode configuration
55+
/// Install `OpenCode` configuration
5556
#[arg(long, conflicts_with_all = ["claude", "both"])]
5657
opencode: bool,
5758

5859
/// Install Claude configuration
5960
#[arg(long, conflicts_with_all = ["opencode", "both"])]
6061
claude: bool,
6162

62-
/// Install both OpenCode and Claude configuration
63+
/// Install both `OpenCode` and Claude configuration
6364
#[arg(long, conflicts_with_all = ["opencode", "claude"])]
6465
both: bool,
6566

@@ -373,7 +374,7 @@ mod tests {
373374
ConfigSubcommand::Show { format, .. } => {
374375
assert_eq!(format, OutputFormat::Text);
375376
}
376-
_ => panic!("Expected Show subcommand"),
377+
ConfigSubcommand::Validate { .. } => panic!("Expected Show subcommand"),
377378
},
378379
_ => panic!("Expected Config command"),
379380
}
@@ -388,7 +389,7 @@ mod tests {
388389
ConfigSubcommand::Validate { format, .. } => {
389390
assert_eq!(format, OutputFormat::Json);
390391
}
391-
_ => panic!("Expected Validate subcommand"),
392+
ConfigSubcommand::Show { .. } => panic!("Expected Validate subcommand"),
392393
},
393394
_ => panic!("Expected Config command"),
394395
}
@@ -420,7 +421,7 @@ mod tests {
420421
assert_eq!(log_level, Some(LogLevel::Debug));
421422
assert_eq!(timeout_ms, Some(60000));
422423
}
423-
_ => panic!("Expected Show subcommand"),
424+
ConfigSubcommand::Validate { .. } => panic!("Expected Show subcommand"),
424425
},
425426
_ => panic!("Expected Config command"),
426427
}

cli/src/command_surface.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,10 @@ Auth usage:\n sce auth <login|logout|status> [--format <text|json>]\n\n\
9393
Completion usage:\n sce completion --shell <bash|zsh|fish>\n\n\
9494
Output format contract:\n Supported commands accept --format <text|json>\n\n\
9595
Examples:\n sce setup\n sce setup --opencode --non-interactive --hooks\n sce setup --hooks --repo ../demo-repo\n sce auth status\n sce auth login --format json\n sce doctor --format json\n sce version --format json\n\n\
96-
Commands:\n{}\n\n\
96+
Commands:\n{command_rows}\n\n\
9797
Setup defaults to interactive target selection when no setup target flag is passed, and installs hooks in the same run.\n\
9898
Use '--hooks' to install required git hooks for the current repository or '--repo <path>' for a specific repository.\n\
99-
`setup`, `doctor`, `auth`, `hooks`, `version`, and `completion` are implemented; `mcp` and `sync` remain placeholder-oriented.\n",
100-
command_rows
99+
`setup`, `doctor`, `auth`, `hooks`, `version`, and `completion` are implemented; `mcp` and `sync` remain placeholder-oriented.\n"
101100
)
102101
}
103102

0 commit comments

Comments
 (0)