From 216024622de73f9dbd1afe6a85282c4a56e6e70c Mon Sep 17 00:00:00 2001
From: jdx <216188+jdx@users.noreply.github.com>
Date: Sun, 18 Jan 2026 20:33:21 -0600
Subject: [PATCH] refactor: improve code quality with safety and lint fixes

- Replace unsafe .unwrap() on Path::parent() with proper error handling
  in include directive parsing
- Add #[must_use] attributes to builder build() methods and parse functions
  to prevent accidentally discarding important return values
- Add doc comments to parse functions explaining their purpose

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 lib/src/parse.rs        |  8 ++++++++
 lib/src/spec/builder.rs |  3 +++
 lib/src/spec/mod.rs     | 15 ++++++++++++++-
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/lib/src/parse.rs b/lib/src/parse.rs
index 6a7903e0..4c17897c 100644
--- a/lib/src/parse.rs
+++ b/lib/src/parse.rs
@@ -46,6 +46,10 @@ pub enum ParseValue {
     MultiString(Vec<String>),
 }
 
+/// Parse command-line arguments according to a spec.
+///
+/// Returns the parsed arguments and flags, with defaults and env vars applied.
+#[must_use = "parsing result should be used"]
 pub fn parse(spec: &Spec, input: &[String]) -> Result<ParseOutput, miette::Error> {
     let mut out = parse_partial(spec, input)?;
     trace!("{out:?}");
@@ -156,6 +160,10 @@ pub fn parse(spec: &Spec, input: &[String]) -> Result<ParseOutput, miette::Error
     Ok(out)
 }
 
+/// Parse command-line arguments without applying defaults.
+///
+/// Use this for help text generation or when you need the raw parsed values.
+#[must_use = "parsing result should be used"]
 pub fn parse_partial(spec: &Spec, input: &[String]) -> Result<ParseOutput, miette::Error> {
     trace!("parse_partial: {input:?}");
     let mut input = input.iter().cloned().collect::<VecDeque<_>>();
diff --git a/lib/src/spec/builder.rs b/lib/src/spec/builder.rs
index 1f395159..8ec50c02 100644
--- a/lib/src/spec/builder.rs
+++ b/lib/src/spec/builder.rs
@@ -185,6 +185,7 @@ impl SpecFlagBuilder {
     }
 
     /// Build the final SpecFlag
+    #[must_use]
     pub fn build(mut self) -> SpecFlag {
         self.inner.usage = self.inner.usage();
         if self.inner.name.is_empty() {
@@ -300,6 +301,7 @@ impl SpecArgBuilder {
     }
 
     /// Build the final SpecArg
+    #[must_use]
     pub fn build(mut self) -> SpecArg {
         self.inner.usage = self.inner.usage();
         self.inner
@@ -428,6 +430,7 @@ impl SpecCommandBuilder {
     }
 
     /// Build the final SpecCommand
+    #[must_use]
     pub fn build(mut self) -> SpecCommand {
         self.inner.usage = self.inner.usage();
         self.inner
diff --git a/lib/src/spec/mod.rs b/lib/src/spec/mod.rs
index 44d5e09e..57ff574d 100644
--- a/lib/src/spec/mod.rs
+++ b/lib/src/spec/mod.rs
@@ -61,6 +61,8 @@ pub struct Spec {
 }
 
 impl Spec {
+    /// Parse a spec from a file (either a .usage.kdl file or a script with embedded spec).
+    #[must_use = "parsing result should be used"]
     pub fn parse_file(file: &Path) -> Result<(Spec, String), UsageErr> {
         let (spec, body) = split_script(file)?;
         let ctx = ParsingContext::new(file, &spec);
@@ -77,6 +79,8 @@ impl Spec {
         }
         Ok((schema, body))
     }
+    /// Parse a spec from a script file's USAGE comments.
+    #[must_use = "parsing result should be used"]
     pub fn parse_script(file: &Path) -> Result<Spec, UsageErr> {
         let raw = extract_usage_from_comments(&file::read_to_string(file)?);
         let ctx = ParsingContext::new(file, &raw);
@@ -177,7 +181,16 @@ impl Spec {
                         .ok_or_else(|| ctx.build_err("missing file".into(), node.span()))?;
                     let file = Path::new(&file);
                     let file = match file.is_relative() {
-                        true => ctx.file.parent().unwrap().join(file),
+                        true => ctx
+                            .file
+                            .parent()
+                            .ok_or_else(|| {
+                                ctx.build_err(
+                                    format!("cannot get parent of {}", ctx.file.display()),
+                                    node.span(),
+                                )
+                            })?
+                            .join(file),
                         false => file.to_path_buf(),
                     };
                     info!("include: {}", file.display());