Separate jobs and errors in parse output
Registration callbacks now route Job results and ValidationErrors into
their own sinks instead of mixing both into a single Vec<Result>.
ParseOutput carries Vec<Job> and Vec<ValidationError> separately.

Assisted-by: GLM-5.1 via pi
change oqotxqlxmxmsrtpnpsmvrtrvywtrytnt
commit 261557aceac65f632e230e3c00c01a8f8652e646
author Alpha Chen <alpha@kejadlen.dev>
date
parent uzuowmtz
diff --git a/src/ci/lua.rs b/src/ci/lua.rs
index 4be8a92..c1ea6f9 100644
--- a/src/ci/lua.rs
+++ b/src/ci/lua.rs
@@ -21,7 +21,8 @@ use crate::secret::SecretString;
 /// Output of [`parse`]: registered jobs and the pipeline image (if
 /// declared).
 pub(super) struct ParseOutput {
-    pub(super) jobs: Vec<std::result::Result<Job, ValidationError>>,
+    pub(super) jobs: Vec<Job>,
+    pub(super) errors: Vec<ValidationError>,
     pub(super) image: Option<String>,
 }
 
@@ -30,15 +31,18 @@ pub(super) struct ParseOutput {
 /// Pre-graph rules run inside the callback, so a single bad job does
 /// not abort the rest of the script.
 pub(super) fn parse(fennel: &Fennel, source: &str, name: &str) -> Result<ParseOutput> {
-    let jobs = Rc::new(RefCell::new(Vec::new()));
+    let jobs: Rc<RefCell<Vec<Job>>> = Rc::new(RefCell::new(Vec::new()));
     let image = Rc::new(RefCell::new(None));
     let src = Rc::new(source.to_string());
 
+    let errors = Rc::new(RefCell::new(Vec::new()));
+
     fennel.eval_raw(source, name, |lua| {
         lua.register_module(
             "quire.ci",
             Registration {
                 jobs: jobs.clone(),
+                errors: errors.clone(),
                 image: image.clone(),
                 source: src.clone(),
             },
@@ -53,6 +57,7 @@ pub(super) fn parse(fennel: &Fennel, source: &str, name: &str) -> Result<ParseOu
     let image_name = image.borrow().as_ref().map(|i| i.name.clone());
     Ok(ParseOutput {
         jobs: jobs.take(),
+        errors: errors.take(),
         image: image_name,
     })
 }
@@ -73,7 +78,8 @@ pub(super) fn parse(fennel: &Fennel, source: &str, name: &str) -> Result<ParseOu
 ///     (sh ["echo" (secret :github_token)])))
 /// ```
 struct Registration {
-    jobs: Rc<RefCell<Vec<std::result::Result<Job, ValidationError>>>>,
+    jobs: Rc<RefCell<Vec<Job>>>,
+    errors: Rc<RefCell<Vec<ValidationError>>>,
     image: Rc<RefCell<Option<ImageRegistration>>>,
     source: Rc<String>,
 }
@@ -109,9 +115,9 @@ fn register_image(lua: &Lua, (name,): (String,)) -> mlua::Result<()> {
     match &*img {
         Some(_) => {
             let span = super::pipeline::span_for_line(&r.source, line);
-            r.jobs
+            r.errors
                 .borrow_mut()
-                .push(Err(ValidationError::DuplicateImage { span }));
+                .push(ValidationError::DuplicateImage { span });
         }
         None => {
             *img = Some(ImageRegistration { name, _line: line });
@@ -135,9 +141,10 @@ fn register_job(
         .flatten()
         .map(|l| l as u32)
         .unwrap_or(0);
-    r.jobs
-        .borrow_mut()
-        .push(Job::new(id, inputs, run_fn, line, &r.source));
+    match Job::new(id, inputs, run_fn, line, &r.source) {
+        Ok(job) => r.jobs.borrow_mut().push(job),
+        Err(e) => r.errors.borrow_mut().push(e),
+    }
     Ok(())
 }
 
diff --git a/src/ci/pipeline.rs b/src/ci/pipeline.rs
index 0015909..4580316 100644
--- a/src/ci/pipeline.rs
+++ b/src/ci/pipeline.rs
@@ -168,17 +168,11 @@ impl Pipeline {
     /// for miette to render with inline labels.
     pub(crate) fn load(source: &str, name: &str) -> Result<Pipeline> {
         let fennel = Fennel::new()?;
-        let output = lua::parse(&fennel, source, name)?;
-
-        let mut errors = Vec::new();
-        let mut jobs = Vec::new();
-        for r in output.jobs {
-            match r {
-                Ok(j) => jobs.push(j),
-                Err(e) => errors.push(e),
-            }
-        }
-        let image = output.image;
+        let parsed = lua::parse(&fennel, source, name)?;
+
+        let mut errors = parsed.errors;
+        let jobs = parsed.jobs;
+        let image = parsed.image;
 
         let (graph, node_index) = build_graph(&jobs);
 
@@ -435,24 +429,19 @@ mod tests {
         assert!(result.is_err(), "malformed Fennel should fail");
     }
 
-    /// Parse a Fennel source into per-job results. Pre-graph rules
-    /// run during parsing, so each entry is `Ok(Job)` or
-    /// `Err(ValidationError)`. The local Fennel is dropped on return,
-    /// but the returned `Job`s only need their non-VM fields here.
-    fn parse_results(source: &str) -> Vec<std::result::Result<Job, ValidationError>> {
+    /// Parse a Fennel source into jobs and errors. The local Fennel is
+    /// dropped on return, but the returned `Job`s only need their
+    /// non-VM fields here.
+    fn parse_jobs(source: &str) -> (Vec<Job>, Vec<ValidationError>) {
         let f = Fennel::new().expect("Fennel::new() should succeed");
-        lua::parse(&f, source, "ci.fnl")
-            .expect("parse should succeed")
-            .jobs
+        let output = lua::parse(&f, source, "ci.fnl").expect("parse should succeed");
+        (output.jobs, output.errors)
     }
 
     /// Discard parse errors and return only the successfully registered
     /// jobs — for tests that exercise post-graph rules.
     fn parsed_jobs(source: &str) -> Vec<Job> {
-        parse_results(source)
-            .into_iter()
-            .filter_map(|r| r.ok())
-            .collect()
+        parse_jobs(source).0
     }
 
     /// Run post-graph validation against `jobs`, building the dependency
@@ -537,29 +526,29 @@ mod tests {
 
     #[test]
     fn parse_rejects_empty_inputs() {
-        let results = parse_results(
+        let (_, errors) = parse_jobs(
             r#"(local ci (require :quire.ci))
 (ci.job :setup [] (fn [_] nil))"#,
         );
         assert!(
-            results.iter().any(
-                |r| matches!(r, Err(ValidationError::EmptyInputs { job_id, .. }) if job_id == "setup")
+            errors.iter().any(
+                |e| matches!(e, ValidationError::EmptyInputs { job_id, .. } if job_id == "setup")
             ),
-            "should report empty inputs for 'setup': {results:?}"
+            "should report empty inputs for 'setup': {errors:?}"
         );
     }
 
     #[test]
     fn parse_rejects_slash_in_job_id() {
-        let results = parse_results(
+        let (_, errors) = parse_jobs(
             r#"(local ci (require :quire.ci))
 (ci.job :foo/bar [:quire/push] (fn [_] nil))"#,
         );
         assert!(
-            results.iter().any(
-                |r| matches!(r, Err(ValidationError::ReservedSlash { job_id, .. }) if job_id == "foo/bar")
+            errors.iter().any(
+                |e| matches!(e, ValidationError::ReservedSlash { job_id, .. } if job_id == "foo/bar")
             ),
-            "should report slash in job id: {results:?}"
+            "should report slash in job id: {errors:?}"
         );
     }