Fix alias expansion ( issue #711)

examples/alias.ion

@@ -0,0 +1,15 @@
+alias piped = "sed s/A/B/ | sed s/B/C/"
+alias logical = "echo LOGICAL1 && echo B"
+alias piped_and_logical = "sed s/A/B/ | sed s/B/PIPED_AND_LOGICAL1/ && echo B"
+
+# Alias contains only pipes
+echo A | piped | sed s/C/PIPED/
+
+# Alias containes logical operator
+logical | sed s/B/LOGICAL2/
+
+# Alias breaks pipeline into half by logical operator
+echo A | piped_and_logical | sed s/B/PIPED_AND_LOGICAL2/
+
+# Expanding multiple aliases in a singel pipeline
+echo A | piped | sed s/C/PIPED_OK/ && logical | piped_and_logical | sed s/B/DONE/

examples/alias.out

@@ -0,0 +1,9 @@
+PIPED
+LOGICAL1
+LOGICAL2
+PIPED_AND_LOGICAL1
+PIPED_AND_LOGICAL2
+PIPED_OK
+LOGICAL1
+PIPED_AND_LOGICAL1
+DONE

src/lib/shell/flow.rs

@@ -3,8 +3,8 @@ use super::{
     job_control::JobControl, status::*, Shell,
 };
 use parser::{
-    assignments::is_array, expand_string, parse_and_validate, pipelines::Pipeline, ForExpression,
-    StatementSplitter,
+    assignments::is_array, expand_string, parse_and_validate, pipelines::{PipeItem, Pipeline},
+    ForExpression, StatementSplitter,
 };
 use shell::{assignments::VariableStore, variables::VariableType};
 use small;
@@ -209,13 +209,25 @@ impl FlowLogic for Shell {
                     Function::new(description, name.clone(), args, statements),
                 );
             }
-            Statement::Pipeline(mut pipeline) => {
-                self.run_pipeline(&mut pipeline);
-                if self.flags & ERR_EXIT != 0 && self.previous_status != SUCCESS {
-                    let status = self.previous_status;
-                    self.exit(status);
+            Statement::Pipeline(pipeline) => match expand_pipeline(&self, pipeline) {
+                Ok((mut pipeline, statements)) => {
+                    self.run_pipeline(&mut pipeline);
+                    if self.flags & ERR_EXIT != 0 && self.previous_status != SUCCESS {
+                        let status = self.previous_status;
+                        self.exit(status);
+                    }
+                    if !statements.is_empty() {
+                        self.execute_statements(statements);
+                    }
                 }
-            }
+                Err(e) => {
+                    eprintln!("ion: pipeline expansion error: {}", e);
+                    self.previous_status = FAILURE;
+                    self.variables.set("?", self.previous_status.to_string());
+                    self.flow_control.reset();
+                    return Condition::Break;
+                }
+            },
             Statement::Time(box_statement) => {
                 let time = ::std::time::Instant::now();
 
@@ -467,3 +479,91 @@ impl FlowLogic for Shell {
         }
     }
 }
+
+/// Expand a pipeline containing aliases. As aliases can split the pipeline by having logical
+/// operators in them, the function returns the first half of the pipeline and the rest of the
+/// statements, where the last statement has the other half of the pipeline merged.
+fn expand_pipeline(
+    shell: &Shell,
+    pipeline: Pipeline,
+) -> Result<(Pipeline, Vec<Statement>), String> {
+    let mut item_iter = pipeline.items.iter();
+    let mut items: Vec<PipeItem> = Vec::new();
+    let mut statements = Vec::new();
+
+    while let Some(item) = item_iter.next() {
+        let possible_alias = shell
+            .variables
+            .get::<types::Alias>(item.job.command.as_ref());
+        if let Some(alias) = possible_alias {
+            statements = StatementSplitter::new(alias.0.as_str())
+                .map(parse_and_validate)
+                .collect();
+
+            // First item in the alias should be a pipeline item, otherwise it cannot
+            // be placed into a pipeline!
+            let len = statements.len();
+            if let Some(Statement::Pipeline(ref mut pline)) = statements.first_mut() {
+                // Connect inputs and outputs of alias to pipeline
+                if let Some(first) = pline.items.first_mut() {
+                    first.inputs = item.inputs.clone();
+                }
+                if len == 1 {
+                    if let Some(last) = pline.items.last_mut() {
+                        last.outputs = item.outputs.clone();
+                        last.job.kind = item.job.kind;
+                    }
+                }
+                items.append(&mut pline.items);
+            } else {
+                return Err(format!(
+                    "unable to pipe inputs to alias: '{} = {}'",
+                    item.job.command.as_str(),
+                    alias.0.as_str()
+                ));
+            }
+            statements.remove(0);
+
+            // Handle pipeline being broken half by i.e.: '&&' or '||'
+            if statements.len() >= 1 {
+                let err = match statements.last_mut().unwrap() {
+                    Statement::And(ref mut boxed_stm)
+                    | Statement::Or(ref mut boxed_stm)
+                    | Statement::Not(ref mut boxed_stm)
+                    | Statement::Time(ref mut boxed_stm) => {
+                        let stm = &mut **boxed_stm;
+                        if let Statement::Pipeline(ref mut pline) = stm {
+                            // Set output of alias to be the output of last pipeline.
+                            if let Some(last) = pline.items.last_mut() {
+                                last.outputs = item.outputs.clone();
+                                last.job.kind = item.job.kind;
+                            }
+                            // Append rest of the pipeline to the last pipeline in the
+                            // alias.
+                            while let Some(item) = item_iter.next() {
+                                pline.items.push(item.clone());
+                            }
+                            // No error
+                            false
+                        } else {
+                            // Error in expansion
+                            true
+                        }
+                    }
+                    _ => true,
+                };
+                if err {
+                    return Err(format!(
+                        "unable to pipe outputs of alias: '{} = {}'",
+                        item.job.command.as_str(),
+                        alias.0.as_str()
+                    ));
+                }
+                break;
+            }
+        } else {
+            items.push(item.clone());
+        }
+    }
+    Ok((Pipeline { items }, statements))
+}

src/lib/shell/mod.rs

@@ -17,39 +17,25 @@ pub mod status;
 pub mod variables;
 
 pub use self::{
-    binary::Binary,
-    fork::{Capture, Fork, IonResult},
+    binary::Binary, fork::{Capture, Fork, IonResult},
 };
 pub(crate) use self::{
-    flow::FlowLogic,
-    history::{IgnoreSetting, ShellHistory},
-    job::{Job, JobKind},
+    flow::FlowLogic, history::{IgnoreSetting, ShellHistory}, job::{Job, JobKind},
     pipe_exec::{foreground, job_control},
 };
 
 use self::{
-    directory_stack::DirectoryStack,
-    flags::*,
-    flow_control::{FlowControl, Function, FunctionError},
-    foreground::ForegroundSignals,
-    job_control::{BackgroundProcess, JobControl},
-    pipe_exec::PipelineExecution,
-    status::*,
+    directory_stack::DirectoryStack, flags::*,
+    flow_control::{FlowControl, Function, FunctionError}, foreground::ForegroundSignals,
+    job_control::{BackgroundProcess, JobControl}, pipe_exec::PipelineExecution, status::*,
     variables::{VariableType, Variables},
 };
 use builtins::{BuiltinMap, BUILTINS};
-use lexers::ArgumentSplitter;
 use liner::Context;
 use parser::{pipelines::Pipeline, Expander, Select, Terminator};
 use std::{
-    fs::File,
-    io::{self, Read, Write},
-    iter::FromIterator,
-    ops::Deref,
-    path::Path,
-    process,
-    sync::{atomic::Ordering, Arc, Mutex},
-    time::SystemTime,
+    fs::File, io::{self, Read, Write}, iter::FromIterator, ops::Deref, path::Path, process,
+    sync::{atomic::Ordering, Arc, Mutex}, time::SystemTime,
 };
 use sys;
 use types::{self, Array};
@@ -251,35 +237,6 @@ impl Shell {
     pub(crate) fn run_pipeline(&mut self, pipeline: &mut Pipeline) -> Option<i32> {
         let command_start_time = SystemTime::now();
 
-        // Expand any aliases found
-        for item in &mut pipeline.items {
-            let mut last_command = String::with_capacity(32);
-            loop {
-                let possible_alias = {
-                    let key: &str = item.job.command.as_ref();
-                    if &last_command == key {
-                        break;
-                    }
-                    last_command.clear();
-                    last_command.push_str(key);
-                    self.variables.get::<types::Alias>(key)
-                };
-
-                if let Some(alias) = possible_alias {
-                    let new_args = ArgumentSplitter::new(&alias)
-                        .map(types::Str::from)
-                        .chain(item.job.args.drain().skip(1))
-                        .collect::<types::Array>();
-                    if let Some(builtin) = BUILTINS.get(&new_args[0]) {
-                        item.job.builtin = Some(builtin.main);
-                    } else {
-                        item.job.command = new_args[0].clone().into();
-                    }
-                    item.job.args = new_args;
-                }
-            }
-        }
-
         // Branch if -> input == shell command i.e. echo
         let exit_status = if let Some(main) = pipeline.items[0].job.builtin {
             pipeline.expand(self);