diff --git a/src/finding/locate.rs b/src/finding/locate.rs index a6c14d7a..992d551d 100644 --- a/src/finding/locate.rs +++ b/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> { 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)?; diff --git a/src/finding/mod.rs b/src/finding/mod.rs index 4a1d8860..24b37d8b 100644 --- a/src/finding/mod.rs +++ b/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>, + /// The step or non-step key within this workflow. + pub(crate) step_or_key: Option>, } 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>, - /// An optional annotation for this location. pub(crate) annotation: Option, + + /// The job or non-job key within this workflow. + pub(crate) job_or_key: Option>, } 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"), } } diff --git a/src/models.rs b/src/models.rs index 63118aed..199aecce 100644 --- a/src/models.rs +++ b/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 }