Merge pull request #2 from ThomasLaPiana/make-things-more-rustic

Make things more Rustic

roxfile.yml

@@ -2,10 +2,6 @@ version_requirements:
   - command: "cargo nextest --version"
     minimum_version: "0.9.0"
     split: true
-
-  - command: "rox --version"
-    minimum_version: "0.4.2"
-    split: true
 
 file_requirements:
   - path: "Cargo.toml"

src/cli.rs

@@ -1,23 +1,17 @@
-use crate::model_injection::inject_pipeline_metadata;
 use crate::models::{Pipeline, Task};
 use clap::{crate_version, Arg, ArgAction, Command};
 
 /// Dyanmically construct the CLI from the Roxfile
-pub fn construct_cli(
-    tasks: Vec<Task>,
-    pipelines: Option<Vec<Pipeline>>,
-    file_path: &str,
-) -> clap::Command {
+pub fn construct_cli(tasks: &[Task], pipelines: &Option<Vec<Pipeline>>) -> clap::Command {
     let mut cli = cli_builder();
 
     // Tasks
-    let task_subcommands = build_task_subcommands(&tasks);
+    let task_subcommands = build_task_subcommands(tasks);
     cli = cli.subcommands(vec![task_subcommands]);
 
     // Pipelines
     if let Some(pipelines) = pipelines {
-        let sorted_pipelines = inject_pipeline_metadata(pipelines, file_path);
-        let pipeline_subcommands = build_pipeline_subcommands(&sorted_pipelines);
+        let pipeline_subcommands = build_pipeline_subcommands(pipelines);
         cli = cli.subcommands(vec![pipeline_subcommands]);
     }
     cli
@@ -54,7 +48,7 @@ pub fn build_task_subcommands(tasks: &[Task]) -> Command {
     let subcommands: Vec<Command> = tasks
         .iter()
         .filter(|target| !target.hide.unwrap_or_default())
-        .map(|task| Command::new(&task.name).about(task.description.clone().unwrap_or_default()))
+        .map(|task| Command::new(&task.name).about(task.description.to_owned().unwrap_or_default()))
         .collect();
 
     Command::new("task")

src/execution.rs

@@ -55,7 +55,7 @@ pub fn run_task(task: &Task) -> TaskResult {
         name: task.name.to_string(),
         result: get_result_passfail(command_results),
         elapsed_time: start.elapsed().as_secs(),
-        file_path: task.file_path.clone().unwrap(),
+        file_path: task.file_path.to_owned().unwrap(),
     }
 }
 
@@ -92,7 +92,7 @@ pub fn execute_tasks(
 
 /// Execute a vector of Stages
 pub fn execute_stages(
-    stages: Vec<Vec<String>>,
+    stages: &[Vec<String>],
     task_map: &HashMap<String, Task>,
     parallel: bool,
 ) -> Vec<Vec<TaskResult>> {

src/lib.rs

@@ -9,23 +9,27 @@ mod version_requirements;
 
 use crate::cli::{cli_builder, construct_cli};
 use crate::execution::{execute_stages, execute_tasks, PassFail, TaskResult};
-use crate::model_injection::{inject_task_metadata, inject_template_values};
+use crate::model_injection::{
+    inject_pipeline_metadata, inject_task_metadata, inject_template_values,
+};
 use crate::models::Validate;
 use std::collections::HashMap;
 use std::error::Error;
 
 type RoxResult<T> = Result<T, Box<dyn Error>>;
 
-/// Get the filepath from the CLI
-fn get_filepath() -> String {
-    // TODO: This is kind of a code smell. No inputs but potentially mutating state via the CLI
+/// Get the filepath argument from the CLI
+///
+/// This is required because we might need to
+/// dynamically populate the CLI based on this arg
+fn get_filepath_arg_value() -> String {
     let cli = cli_builder();
     // Get the file arg from the CLI if set
     let cli_matches = cli.clone().arg_required_else_help(false).get_matches();
     cli_matches.get_one::<String>("roxfile").unwrap().to_owned()
 }
 
-// Entrypoint for the Crate CLI
+/// Entrypoint for the Crate CLI
 pub fn rox() -> RoxResult<()> {
     let start = std::time::Instant::now();
 
@@ -34,15 +38,15 @@ pub fn rox() -> RoxResult<()> {
     // the filename arg and once to actually build the CLI.
 
     // Get the file arg from the CLI if set
-    let file_path = get_filepath();
+    let file_path = get_filepath_arg_value();
     let roxfile = utils::parse_file_contents(utils::load_file(&file_path));
     roxfile.validate()?;
     utils::print_horizontal_rule();
 
     // Build & Generate the CLI based on the loaded Roxfile
     let tasks = inject_task_metadata(roxfile.tasks, &file_path);
-    let pipelines = roxfile.pipelines;
-    let cli = construct_cli(tasks.clone(), pipelines.clone(), &file_path);
+    let pipelines = inject_pipeline_metadata(roxfile.pipelines);
+    let cli = construct_cli(&tasks, &pipelines);
     let cli_matches = cli.get_matches();
 
     // Run File and Version checks
@@ -68,24 +72,24 @@ pub fn rox() -> RoxResult<()> {
             .templates
             .into_iter()
             .flatten()
-            .map(|template| (template.name.clone(), template)),
+            .map(|template| (template.name.to_owned(), template)),
     );
     let task_map: HashMap<String, models::Task> = std::collections::HashMap::from_iter(
         tasks
             .into_iter()
-            .map(|task| match task.uses.clone() {
+            .map(|task| match task.uses.to_owned() {
                 Some(task_use) => {
                     inject_template_values(task, template_map.get(&task_use).unwrap())
                 }
                 None => task,
             })
-            .map(|task| (task.name.clone(), task)),
+            .map(|task| (task.name.to_owned(), task)),
     );
     let pipeline_map: HashMap<String, models::Pipeline> = std::collections::HashMap::from_iter(
         pipelines
             .into_iter()
             .flatten()
-            .map(|pipeline| (pipeline.name.clone(), pipeline)),
+            .map(|pipeline| (pipeline.name.to_owned(), pipeline)),
     );
 
     // Execute the Task(s)
@@ -95,35 +99,37 @@ pub fn rox() -> RoxResult<()> {
             let (_, args) = cli_matches.subcommand().unwrap();
             let pipeline_name = args.subcommand_name().unwrap();
             let parallel = args.get_flag("parallel");
-            execute_stages(
-                pipeline_map.get(pipeline_name).unwrap().stages.clone(),
+            let execution_results = execute_stages(
+                &pipeline_map.get(pipeline_name).unwrap().stages,
                 &task_map,
                 parallel,
-            )
+            );
+            execution_results
         }
         "task" => {
             let (_, args) = cli_matches.subcommand().unwrap();
             let task_name = args.subcommand_name().unwrap().to_owned();
-            vec![execute_tasks(vec![task_name], &task_map, false)]
+            let execution_results = vec![execute_tasks(vec![task_name], &task_map, false)];
+            execution_results
         }
         command => {
             println!("'{}' is not a valid subcommand!", command);
             std::process::exit(2);
         }
     };
-    output::display_execution_results(results.clone());
+    output::display_execution_results(&results);
     println!(
         "> Total elapsed time: {}s | {}ms",
         start.elapsed().as_secs(),
         start.elapsed().as_millis(),
     );
-    nonzero_exit_if_failure(results);
+    nonzero_exit_if_failure(&results);
 
     Ok(())
 }
 
 /// Throw a non-zero exit if any Task(s) had a failing result
-pub fn nonzero_exit_if_failure(results: Vec<Vec<TaskResult>>) {
+pub fn nonzero_exit_if_failure(results: &[Vec<TaskResult>]) {
     // TODO: Figure out a way to get this info without looping again
     for result in results.iter().flatten() {
         if result.result == PassFail::Fail {

src/model_injection.rs

@@ -2,26 +2,21 @@ use crate::models;
 
 /// Inject additional metadata into each Pipeline and sort based on name.
 pub fn inject_pipeline_metadata(
-    pipelines: Vec<models::Pipeline>,
-    file_path: &str,
-) -> Vec<models::Pipeline> {
-    let mut sorted_pipelines: Vec<models::Pipeline> = pipelines
-        .into_iter()
-        .map(|mut pipeline| {
-            pipeline.file_path = Some(file_path.to_owned());
-            pipeline
-        })
-        .collect();
-    sorted_pipelines.sort_by(|x, y| x.name.to_lowercase().cmp(&y.name.to_lowercase()));
-    sorted_pipelines
+    pipelines: Option<Vec<models::Pipeline>>,
+) -> Option<Vec<models::Pipeline>> {
+    if let Some(mut some_pipelines) = pipelines {
+        some_pipelines.sort_by(|x, y| x.name.to_lowercase().cmp(&y.name.to_lowercase()));
+        return Some(some_pipelines);
+    }
+    pipelines
 }
 
 /// Get used Template's information and inject set values
 pub fn inject_template_values(mut task: models::Task, template: &models::Template) -> models::Task {
     task.command = {
-        let mut template_command = template.command.clone();
-        let template_symbols = template.symbols.clone();
-        let task_values = task.values.clone().unwrap();
+        let mut template_command = template.command.to_owned();
+        let template_symbols = template.symbols.to_owned();
+        let task_values = task.values.as_ref().unwrap();
 
         for i in 0..task_values.len() {
             template_command = template_command.replace(
@@ -63,7 +58,7 @@ pub fn inject_task_metadata(tasks: Vec<models::Task>, file_path: &str) -> Vec<mo
             task.file_path = Some(file_path.to_owned());
 
             if task.description.is_none() {
-                task.description = task.command.clone()
+                task.description = task.command.to_owned()
             }
             task
         })

src/models.rs

@@ -57,7 +57,7 @@ impl Validate for VersionRequirement {
             .into_iter()
             .flatten()
             .collect();
-        for version in versions.clone() {
+        for version in versions.iter() {
             if Version::from_str(version).is_err() {
                 color_print(vec![failure_message], ColorEnum::Red);
                 return Err(ValidationError {
@@ -181,8 +181,8 @@ impl Validate for Template {
         let failure_message = format!("> Template '{}' failed validation!", self.name);
 
         // All of the 'Symbol' items must exist within the 'Command'
-        for symbol in self.symbols.clone() {
-            let exists = self.command.contains(&symbol);
+        for symbol in &self.symbols {
+            let exists = self.command.contains(symbol);
             if !exists {
                 color_print(vec![failure_message], ColorEnum::Red);
                 return Err(ValidationError {
@@ -205,7 +205,6 @@ pub struct Pipeline {
     pub name: String,
     pub description: Option<String>,
     pub stages: Vec<Vec<String>>,
-    pub file_path: Option<String>,
 }
 
 /// The top-level structure of the Roxfile
@@ -222,22 +221,20 @@ pub struct RoxFile {
 
 impl Validate for RoxFile {
     fn validate(&self) -> Result<(), ValidationError> {
-        let roxfile = self.clone();
-
         // Task Validation
-        for task in roxfile.tasks {
+        for task in &self.tasks {
             task.validate()?
         }
 
         // Template Validation
-        if let Some(templates) = roxfile.templates {
+        if let Some(templates) = &self.templates {
             for template in templates {
                 template.validate()?
             }
         }
 
         // Version Requirement Validation
-        if let Some(version_requirements) = roxfile.version_requirements {
+        if let Some(version_requirements) = &self.version_requirements {
             for requirement in version_requirements {
                 requirement.validate()?
             }

src/output.rs

@@ -2,13 +2,13 @@ use crate::execution::{PassFail, TaskResult};
 use cli_table::{format::Justify, print_stdout, Cell, Style, Table};
 use colored::Colorize;
 
-pub fn display_execution_results(results: Vec<Vec<TaskResult>>) {
+pub fn display_execution_results(results: &[Vec<TaskResult>]) {
     let mut table = Vec::new();
 
-    for (i, stage_results) in results.into_iter().enumerate() {
+    for (i, stage_results) in results.iter().enumerate() {
         for result in stage_results {
             table.push(vec![
-                result.name.cell(),
+                result.name.to_owned().cell(),
                 format!("{}", i + 1).cell().justify(Justify::Center),
                 match result.result {
                     PassFail::Pass => result
@@ -25,7 +25,7 @@ pub fn display_execution_results(results: Vec<Vec<TaskResult>>) {
                         .justify(Justify::Center),
                 },
                 result.elapsed_time.cell().justify(Justify::Center),
-                result.file_path.cell().justify(Justify::Right),
+                result.file_path.to_owned().cell().justify(Justify::Right),
             ])
         }
     }