begin refactoring job/key split (#15)

src/finding/locate.rs

@@ -3,7 +3,7 @@
 
 use anyhow::Result;
 
-use super::{ConcreteLocation, Feature, WorkflowLocation};
+use super::{ConcreteLocation, Feature, JobOrKey, StepOrKey, WorkflowLocation};
 use crate::models::Workflow;
 
 pub(crate) struct Locator {}
@@ -20,22 +20,22 @@ impl Locator {
     ) -> Result<Feature<'w>> {
         let mut builder = yamlpath::QueryBuilder::new();
 
-        if let Some(job) = &location.job {
-            builder = builder.key("jobs").key(job.id);
+        builder = match &location.job_or_key {
+            Some(JobOrKey::Job(job)) => {
+                builder = builder.key("jobs").key(job.id);
 
-            if let Some(step) = &job.step {
-                builder = builder.key("steps").index(step.index);
-            } else if let Some(key) = &job.key {
-                builder = builder.key(*key);
+                match &job.step_or_key {
+                    Some(StepOrKey::Step(step)) => builder.key("steps").index(step.index),
+                    Some(StepOrKey::Key(key)) => builder.key(*key),
+                    None => builder,
+                }
             }
-        } else {
-            // Non-job top-level key.
-            builder = builder.key(
-                location
-                    .key
-                    .expect("API misuse: must provide key if job is not specified"),
-            );
-        }
+            Some(JobOrKey::Key(key)) => {
+                // Non-job top-level key.
+                builder.key(*key)
+            }
+            None => panic!("API misuse: workflow location must specify a top-level key or job"),
+        };
 
         let query = builder.build();
         let feature = workflow.document.query(&query)?;

src/finding/mod.rs

@@ -44,19 +44,23 @@ impl<'w> From<&Step<'w>> for StepLocation<'w> {
     }
 }
 
+/// Represents a job-level key or step location.
+#[derive(Serialize, Clone, Debug)]
+pub(crate) enum StepOrKey<'w> {
+    Key(&'w str),
+    Step(StepLocation<'w>),
+}
+
 #[derive(Serialize, Clone, Debug)]
 pub(crate) struct JobLocation<'w> {
     /// The job's unique ID within its parent workflow.
     pub(crate) id: &'w str,
 
-    /// A non-step job-level key, like [`WorkflowLocation::key`].
-    pub(crate) key: Option<&'w str>,
-
     /// The job's name, if present.
     pub(crate) name: Option<&'w str>,
 
-    /// The location of a step within the job, if present.
-    pub(crate) step: Option<StepLocation<'w>>,
+    /// The step or non-step key within this workflow.
+    pub(crate) step_or_key: Option<StepOrKey<'w>>,
 }
 
 impl<'w> JobLocation<'w> {
@@ -66,9 +70,8 @@ impl<'w> JobLocation<'w> {
     pub(crate) fn with_key(&self, key: &'w str) -> JobLocation<'w> {
         JobLocation {
             id: self.id,
-            key: Some(key),
             name: self.name,
-            step: None,
+            step_or_key: Some(StepOrKey::Key(key)),
         }
     }
 
@@ -78,26 +81,30 @@ impl<'w> JobLocation<'w> {
     fn with_step(&self, step: &Step<'w>) -> JobLocation<'w> {
         JobLocation {
             id: self.id,
-            key: None,
             name: self.name,
-            step: Some(step.into()),
+            step_or_key: Some(StepOrKey::Step(step.into())),
         }
     }
 }
 
+/// Represents a workflow-level key or job location.
+#[derive(Serialize, Clone, Debug)]
+pub(crate) enum JobOrKey<'w> {
+    Key(&'w str),
+    Job(JobLocation<'w>),
+}
+
 /// Represents a symbolic workflow location.
 #[derive(Serialize, Clone, Debug)]
 pub(crate) struct WorkflowLocation<'w> {
+    /// The name of the workflow.
     pub(crate) name: &'w str,
 
-    /// A top-level workflow key to isolate, if present.
-    pub(crate) key: Option<&'w str>,
-
-    /// The job location within this workflow, if present.
-    pub(crate) job: Option<JobLocation<'w>>,
-
     /// An optional annotation for this location.
     pub(crate) annotation: Option<String>,
+
+    /// The job or non-job key within this workflow.
+    pub(crate) job_or_key: Option<JobOrKey<'w>>,
 }
 
 impl<'w> WorkflowLocation<'w> {
@@ -106,8 +113,7 @@ impl<'w> WorkflowLocation<'w> {
     pub(crate) fn with_key(&self, key: &'w str) -> WorkflowLocation<'w> {
         WorkflowLocation {
             name: self.name,
-            key: Some(key),
-            job: None,
+            job_or_key: Some(JobOrKey::Key(key)),
             annotation: self.annotation.clone(),
         }
     }
@@ -116,13 +122,11 @@ impl<'w> WorkflowLocation<'w> {
     pub(crate) fn with_job(&self, job: &Job<'w>) -> WorkflowLocation<'w> {
         WorkflowLocation {
             name: self.name,
-            key: None,
-            job: Some(JobLocation {
+            job_or_key: Some(JobOrKey::Job(JobLocation {
                 id: job.id,
-                key: None,
                 name: job.inner.name(),
-                step: None,
-            }),
+                step_or_key: None,
+            })),
             annotation: self.annotation.clone(),
         }
     }
@@ -132,14 +136,13 @@ impl<'w> WorkflowLocation<'w> {
     /// This can only be called after the `WorkflowLocation` already has a job,
     /// since steps belong to jobs.
     pub(crate) fn with_step(&self, step: &Step<'w>) -> WorkflowLocation<'w> {
-        match &self.job {
-            None => panic!("API misuse: can't set step without parent job"),
-            Some(job) => WorkflowLocation {
+        match &self.job_or_key {
+            Some(JobOrKey::Job(job)) => WorkflowLocation {
                 name: self.name,
-                key: None,
-                job: Some(job.with_step(step)),
+                job_or_key: Some(JobOrKey::Job(job.with_step(step))),
                 annotation: self.annotation.clone(),
             },
+            _ => panic!("API misuse: can't set step without parent job"),
         }
     }
 

src/models.rs

@@ -3,7 +3,7 @@ use std::{collections::hash_map, iter::Enumerate, ops::Deref, path::Path};
 use anyhow::{Context, Result};
 use github_actions_models::workflow;
 
-use crate::finding::WorkflowLocation;
+use crate::finding::{JobOrKey, WorkflowLocation};
 
 pub(crate) struct Workflow {
     pub(crate) filename: String,
@@ -45,8 +45,7 @@ impl Workflow {
     pub(crate) fn location(&self) -> WorkflowLocation {
         WorkflowLocation {
             name: &self.filename,
-            key: None,
-            job: None,
+            job_or_key: None,
             annotation: None,
         }
     }
@@ -85,8 +84,12 @@ impl<'w> Job<'w> {
 
     pub(crate) fn key_location(&self, key: &'w str) -> WorkflowLocation<'w> {
         let mut location = self.parent.with_job(self);
-        // NOTE: Infallible unwrap due to job always being supplied above.
-        location.job = Some(location.job.unwrap().with_key(key));
+        let Some(JobOrKey::Job(job)) = location.job_or_key else {
+            panic!("unreachable")
+        };
+        let job = job.with_key(key);
+
+        location.job_or_key = Some(JobOrKey::Job(job));
 
         location
     }