Shell: Allow commands in variables, and properly substitute them on use

This allows the below interaction to work:
```
$ silence=(2>&1 >/dev/null)
$ do_noisy_thing with these args $silence
<nothing here lol>
```

Shell/AST.cpp

@@ -52,6 +52,31 @@ static inline void print_indented(const String& str, int indent)
     dbgprintf("%.*c%s\n", indent * 2, ' ', str.characters());
 }
 
+static inline Vector<Command> join_commands(Vector<Command> left, Vector<Command> right)
+{
+    Command command;
+
+    auto last_in_left = left.take_last();
+    auto first_in_right = right.take_first();
+
+    command.argv.append(last_in_left.argv);
+    command.argv.append(first_in_right.argv);
+
+    command.redirections.append(last_in_left.redirections);
+    command.redirections.append(first_in_right.redirections);
+
+    command.should_wait = first_in_right.should_wait && last_in_left.should_wait;
+    command.is_pipe_source = first_in_right.is_pipe_source;
+    command.should_notify_if_in_background = first_in_right.should_wait && last_in_left.should_notify_if_in_background;
+
+    Vector<Command> commands;
+    commands.append(left);
+    commands.append(command);
+    commands.append(right);
+
+    return commands;
+}
+
 void Node::dump(int level) const
 {
     print_indented(String::format("%s at %d:%d", class_name().characters(), m_position.start_offset, m_position.end_offset), level);
@@ -164,8 +189,16 @@ void ListConcatenate::dump(int level) const
 
 RefPtr<Value> ListConcatenate::run(TheExecutionInputType input_value)
 {
-    auto list = m_list->run(input_value);
+    auto list = m_list->run(input_value)->resolve_without_cast(input_value);
-    auto element = m_element->run(input_value);
+    auto element = m_element->run(input_value)->resolve_without_cast(input_value);
+
+    if (list->is_command() || element->is_command()) {
+        auto joined_commands = join_commands(element->resolve_as_commands(input_value), list->resolve_as_commands(input_value));
+
+        if (joined_commands.size() == 1)
+            return create<CommandValue>(joined_commands[0]);
+        return create<CommandSequenceValue>(move(joined_commands));
+    }
 
     return create<ListValue>({ move(element), move(list) });
 }
@@ -304,8 +337,11 @@ RefPtr<Value> CastToCommand::run(TheExecutionInputType input_value)
         return m_inner->run(input_value);
 
     auto shell = input_value;
-    auto argv = m_inner->run(input_value)->resolve_as_list(input_value);
+    auto value = m_inner->run(input_value)->resolve_without_cast(input_value);
+    if (value->is_command())
+        return value;
 
+    auto argv = value->resolve_as_list(input_value);
     return create<CommandValue>(move(argv));
 }
 
@@ -353,13 +389,24 @@ CastToCommand::~CastToCommand()
 void CastToList::dump(int level) const
 {
     Node::dump(level);
-    m_inner->dump(level + 1);
+    if (m_inner)
+        m_inner->dump(level + 1);
+    else
+        print_indented("(empty)", level + 1);
 }
 
 RefPtr<Value> CastToList::run(TheExecutionInputType input_value)
 {
+    if (!m_inner)
+        return create<ListValue>({});
+
     auto shell = input_value;
-    auto values = m_inner->run(input_value)->resolve_as_list(input_value);
+    auto inner_value = m_inner->run(input_value);
+
+    if (inner_value->is_command())
+        return inner_value;
+
+    auto values = inner_value->resolve_as_list(input_value);
     Vector<RefPtr<Value>> cast_values;
     for (auto& value : values)
         cast_values.append(create<StringValue>(value));
@@ -369,7 +416,8 @@ RefPtr<Value> CastToList::run(TheExecutionInputType input_value)
 
 void CastToList::highlight_in_editor(Line::Editor& editor, Shell& shell, HighlightMetadata metadata)
 {
-    m_inner->highlight_in_editor(editor, shell, metadata);
+    if (m_inner)
+        m_inner->highlight_in_editor(editor, shell, metadata);
 }
 
 HitTestResult CastToList::hit_test_position(size_t offset)
@@ -377,6 +425,9 @@ HitTestResult CastToList::hit_test_position(size_t offset)
     if (!position().contains(offset))
         return {};
 
+    if (!m_inner)
+        return {};
+
     return m_inner->hit_test_position(offset);
 }
 
@@ -520,7 +571,7 @@ void DynamicEvaluate::dump(int level) const
 
 RefPtr<Value> DynamicEvaluate::run(TheExecutionInputType input_value)
 {
-    auto result = m_inner->run(input_value);
+    auto result = m_inner->run(input_value)->resolve_without_cast(input_value);
     // Dynamic Evaluation behaves differently between strings and lists.
     // Strings are treated as variables, and Lists are treated as commands.
     if (result->is_string()) {
@@ -815,28 +866,10 @@ void Join::dump(int level) const
 
 RefPtr<Value> Join::run(TheExecutionInputType input_value)
 {
-    Command command;
-
     auto left = m_left->run(input_value)->resolve_as_commands(input_value);
     auto right = m_right->run(input_value)->resolve_as_commands(input_value);
 
-    auto last_in_left = left.take_last();
+    return create<CommandSequenceValue>(join_commands(move(left), move(right)));
-    auto first_in_right = right.take_first();
-
-    command.argv.append(last_in_left.argv);
-    command.argv.append(first_in_right.argv);
-
-    command.redirections.append(last_in_left.redirections);
-    command.redirections.append(first_in_right.redirections);
-
-    command.should_wait = first_in_right.should_wait && last_in_left.should_wait;
-
-    Vector<Command> commands;
-    commands.append(left);
-    commands.append(command);
-    commands.append(right);
-
-    return create<CommandSequenceValue>(move(commands));
 }
 
 void Join::highlight_in_editor(Line::Editor& editor, Shell& shell, HighlightMetadata metadata)
@@ -935,22 +968,18 @@ void Pipe::dump(int level) const
 
 RefPtr<Value> Pipe::run(TheExecutionInputType input_value)
 {
-    int pipefd[2];
-    int rc = pipe(pipefd);
-    if (rc < 0) {
-        dbg() << "Error: cannot pipe(): " << strerror(errno);
-        return create<StringValue>("");
-    }
     auto left = m_left->run(input_value)->resolve_as_commands(input_value);
     auto right = m_right->run(input_value)->resolve_as_commands(input_value);
 
     auto last_in_left = left.take_last();
     auto first_in_right = right.take_first();
 
-    last_in_left.redirections.append(*new FdRedirection(STDOUT_FILENO, pipefd[1], Rewiring::Close::Destination));
+    auto pipe_write_end = new FdRedirection(STDIN_FILENO, -1, Rewiring::Close::Destination);
+    auto pipe_read_end = new FdRedirection(STDOUT_FILENO, -1, pipe_write_end, Rewiring::Close::RefreshDestination);
+    first_in_right.redirections.append(*pipe_write_end);
+    last_in_left.redirections.append(*pipe_read_end);
     last_in_left.should_wait = false;
     last_in_left.is_pipe_source = true;
-    first_in_right.redirections.append(*new FdRedirection(STDIN_FILENO, pipefd[0], Rewiring::Close::Destination));
 
     Vector<Command> commands;
     commands.append(left);
@@ -1247,8 +1276,8 @@ void Juxtaposition::dump(int level) const
 
 RefPtr<Value> Juxtaposition::run(TheExecutionInputType input_value)
 {
-    auto left_value = m_left->run(input_value);
+    auto left_value = m_left->run(input_value)->resolve_without_cast(input_value);
-    auto right_value = m_right->run(input_value);
+    auto right_value = m_right->run(input_value)->resolve_without_cast(input_value);
 
     auto left = left_value->resolve_as_list(input_value);
     auto right = right_value->resolve_as_list(input_value);
@@ -1539,6 +1568,8 @@ RefPtr<Value> VariableDeclarations::run(TheExecutionInputType input_value)
         if (value->is_list()) {
             auto parts = value->resolve_as_list(input_value);
             shell->set_local_variable(name, adopt(*new ListValue(move(parts))));
+        } else if (value->is_command()) {
+            shell->set_local_variable(name, value);
         } else {
             auto part = value->resolve_as_list(input_value);
             shell->set_local_variable(name, adopt(*new StringValue(part[0])));
@@ -1678,9 +1709,7 @@ SimpleVariableValue::~SimpleVariableValue()
 }
 Vector<String> SimpleVariableValue::resolve_as_list(TheExecutionInputType input_value)
 {
-    auto shell = input_value;
+    if (auto value = resolve_without_cast(input_value); value != this)
-
-    if (auto value = shell->lookup_local_variable(m_name))
         return value->resolve_as_list(input_value);
 
     char* env_value = getenv(m_name.characters());
@@ -1695,6 +1724,16 @@ Vector<String> SimpleVariableValue::resolve_as_list(TheExecutionInputType input_
     return res;
 }
 
+RefPtr<Value> SimpleVariableValue::resolve_without_cast(TheExecutionInputType input_value)
+{
+    auto shell = input_value;
+
+    if (auto value = shell->lookup_local_variable(m_name))
+        return value;
+
+    return this;
+}
+
 SpecialVariableValue::~SpecialVariableValue()
 {
 }
@@ -1723,28 +1762,24 @@ Vector<String> TildeValue::resolve_as_list(TheExecutionInputType input_value)
     return { shell->expand_tilde(builder.to_string()) };
 }
 
-Result<Rewiring, String> CloseRedirection::apply() const
+Result<RefPtr<Rewiring>, String> CloseRedirection::apply()
 {
-    auto rc = close(fd);
+    return static_cast<RefPtr<Rewiring>>((adopt(*new Rewiring(fd, fd, Rewiring::Close::ImmediatelyCloseDestination))));
-    if (rc < 0)
-        return String { strerror(errno) };
-
-    return String {};
 }
 
 CloseRedirection::~CloseRedirection()
 {
 }
 
-Result<Rewiring, String> PathRedirection::apply() const
+Result<RefPtr<Rewiring>, String> PathRedirection::apply()
 {
-    auto check_fd_and_return = [my_fd = this->fd](int fd, const String& path) -> Result<Rewiring, String> {
+    auto check_fd_and_return = [my_fd = this->fd](int fd, const String& path) -> Result<RefPtr<Rewiring>, String> {
         if (fd < 0) {
             String error = strerror(errno);
             dbg() << "open() failed for '" << path << "' with " << error;
             return error;
         }
-        return Rewiring { my_fd, fd };
+        return static_cast<RefPtr<Rewiring>>((adopt(*new Rewiring(my_fd, fd, Rewiring::Close::Destination))));
     };
     switch (direction) {
     case AST::PathRedirection::WriteAppend:

Shell/AST.h

@@ -50,18 +50,36 @@ struct Position {
     bool contains(size_t offset) const { return start_offset <= offset && offset <= end_offset; }
 };
 
-struct Rewiring {
+struct Rewiring : public RefCounted<Rewiring> {
     int source_fd { -1 };
     int dest_fd { -1 };
+    Rewiring* other_pipe_end { nullptr };
     enum class Close {
         None,
         Source,
         Destination,
-    } must_be_closed { Close::None };
+        RefreshDestination,
+        ImmediatelyCloseDestination,
+    } fd_action { Close::None };
+
+    Rewiring(int source, int dest, Close close = Close::None)
+        : source_fd(source)
+        , dest_fd(dest)
+        , fd_action(close)
+    {
+    }
+
+    Rewiring(int source, int dest, Rewiring* other_end, Close close)
+        : source_fd(source)
+        , dest_fd(dest)
+        , other_pipe_end(other_end)
+        , fd_action(close)
+    {
+    }
 };
 
 struct Redirection : public RefCounted<Redirection> {
-    virtual Result<Rewiring, String> apply() const = 0;
+    virtual Result<RefPtr<Rewiring>, String> apply() = 0;
     virtual ~Redirection();
     virtual bool is_path_redirection() const { return false; }
     virtual bool is_fd_redirection() const { return false; }
@@ -71,7 +89,7 @@ struct Redirection : public RefCounted<Redirection> {
 struct CloseRedirection : public Redirection {
     int fd { -1 };
 
-    virtual Result<Rewiring, String> apply() const override;
+    virtual Result<RefPtr<Rewiring>, String> apply() override;
     virtual ~CloseRedirection();
     CloseRedirection(int fd)
         : fd(fd)
@@ -92,7 +110,7 @@ struct PathRedirection : public Redirection {
         ReadWrite,
     } direction { Read };
 
-    virtual Result<Rewiring, String> apply() const override;
+    virtual Result<RefPtr<Rewiring>, String> apply() override;
     virtual ~PathRedirection();
     PathRedirection(String path, int fd, decltype(direction) direction)
         : path(move(path))
@@ -108,10 +126,14 @@ private:
 struct FdRedirection : public Redirection
     , public Rewiring {
 
-    virtual Result<Rewiring, String> apply() const override { return *this; }
+    virtual Result<RefPtr<Rewiring>, String> apply() override { return static_cast<RefPtr<Rewiring>>(this); }
     virtual ~FdRedirection();
     FdRedirection(int source, int dest, Rewiring::Close close)
-        : Rewiring({ source, dest, close })
+        : Rewiring(source, dest, close)
+    {
+    }
+    FdRedirection(int source, int dest, Rewiring* pipe_end, Rewiring::Close close)
+        : Rewiring(source, dest, pipe_end, close)
     {
     }
 
@@ -136,6 +158,7 @@ class Value : public RefCounted<Value> {
 public:
     virtual Vector<String> resolve_as_list(TheExecutionInputType) = 0;
     virtual Vector<Command> resolve_as_commands(TheExecutionInputType);
+    virtual RefPtr<Value> resolve_without_cast(TheExecutionInputType) { return this; }
     virtual ~Value();
     virtual bool is_command() const { return false; }
     virtual bool is_glob() const { return false; }
@@ -245,6 +268,7 @@ private:
 class SimpleVariableValue final : public Value {
 public:
     virtual Vector<String> resolve_as_list(TheExecutionInputType) override;
+    RefPtr<Value> resolve_without_cast(TheExecutionInputType) override;
     virtual ~SimpleVariableValue();
     SimpleVariableValue(String name)
         : m_name(name)
@@ -305,6 +329,7 @@ public:
     virtual bool is_glob() const { return false; }
     virtual bool is_tilde() const { return false; }
     virtual bool is_variable_decls() const { return false; }
+    virtual bool is_syntax_error() const { return false; }
 
     virtual bool is_list() const { return false; }
 
@@ -745,6 +770,7 @@ private:
     virtual void highlight_in_editor(Line::Editor&, Shell&, HighlightMetadata = {}) override;
     virtual HitTestResult hit_test_position(size_t) override { return { nullptr, nullptr }; }
     virtual String class_name() const override { return "SyntaxError"; }
+    virtual bool is_syntax_error() const override { return true; }
 };
 
 class Tilde final : public Node {

Shell/Parser.cpp

@@ -200,7 +200,19 @@ RefPtr<AST::Node> Parser::parse_variable_decls()
 
     auto name_expr = create<AST::BarewordLiteral>(move(var_name));
 
+    auto start = push_start();
     auto expression = parse_expression();
+    if (!expression || expression->is_syntax_error()) {
+        m_offset = start->offset;
+        if (peek() == '(') {
+            consume();
+            auto command = parse_pipe_sequence();
+            expect(')');
+            if (!command)
+                m_offset = start->offset;
+            expression = command;
+        }
+    }
     if (!expression) {
         if (is_whitespace(peek())) {
             auto string_start = push_start();
@@ -432,10 +444,10 @@ RefPtr<AST::Node> Parser::parse_expression()
     if (starting_char == '(') {
         consume();
         auto list = parse_list_expression();
-        if (!list)
+        if (!expect(')')) {
-            list = create<AST::SyntaxError>();
+            m_offset = rule_start->offset;
-        if (!expect(')'))
+            return nullptr;
-            return read_concat(create<AST::SyntaxError>());
+        }
         return read_concat(create<AST::CastToList>(move(list))); // Cast To List
     }
 

Shell/Parser.h

@@ -107,6 +107,7 @@ sequence :: variable_decls? pipe_sequence ';' sequence
           | variable_decls? pipe_sequence
 
 variable_decls :: identifier '=' expression (' '+ variable_decls)? ' '*
+                | identifier '=' '(' pipe_sequence ')' (' '+ variable_decls)? ' '*
 
 pipe_sequence :: command '|' pipe_sequence
                | command

Shell/Shell.cpp

@@ -324,7 +324,7 @@ int Shell::run_command(const StringView& cmd)
     if (!command)
         return 0;
 
-#ifndef SH_DEBUG
+#ifdef SH_DEBUG
     dbg() << "Command follows";
     command->dump(0);
 #endif
@@ -349,7 +349,7 @@ RefPtr<Job> Shell::run_command(AST::Command& command)
     FileDescriptionCollector fds;
 
     // Resolve redirections.
-    Vector<AST::Rewiring> rewirings;
+    NonnullRefPtrVector<AST::Rewiring> rewirings;
     for (auto& redirection : command.redirections) {
         auto rewiring_result = redirection->apply();
         if (rewiring_result.is_error()) {
@@ -358,12 +358,29 @@ RefPtr<Job> Shell::run_command(AST::Command& command)
             continue;
         }
         auto& rewiring = rewiring_result.value();
-        rewirings.append(rewiring);
 
-        if (rewiring.must_be_closed == AST::Rewiring::Close::Source) {
+        if (rewiring->fd_action != AST::Rewiring::Close::ImmediatelyCloseDestination)
-            fds.add(rewiring.source_fd);
+            rewirings.append(*rewiring);
-        } else if (rewiring.must_be_closed == AST::Rewiring::Close::Destination) {
+
-            fds.add(rewiring.dest_fd);
+        if (rewiring->fd_action == AST::Rewiring::Close::Source) {
+            fds.add(rewiring->source_fd);
+        } else if (rewiring->fd_action == AST::Rewiring::Close::Destination) {
+            if (rewiring->dest_fd != -1)
+                fds.add(rewiring->dest_fd);
+        } else if (rewiring->fd_action == AST::Rewiring::Close::ImmediatelyCloseDestination) {
+            fds.add(rewiring->dest_fd);
+        } else if (rewiring->fd_action == AST::Rewiring::Close::RefreshDestination) {
+            ASSERT(rewiring->other_pipe_end);
+
+            int pipe_fd[2];
+            int rc = pipe(pipe_fd);
+            if (rc < 0) {
+                perror("pipe(RedirRefresh)");
+                return nullptr;
+            }
+            rewiring->dest_fd = pipe_fd[1];
+            rewiring->other_pipe_end->dest_fd = pipe_fd[0]; // This fd will be added to the collection on one of the next iterations.
+            fds.add(pipe_fd[1]);
         }
     }
 
@@ -377,7 +394,7 @@ RefPtr<Job> Shell::run_command(AST::Command& command)
 #endif
             int rc = dup2(rewiring.dest_fd, rewiring.source_fd);
             if (rc < 0) {
-                perror("dup2");
+                perror("dup2(run)");
                 return nullptr;
             }
         }
@@ -410,7 +427,7 @@ RefPtr<Job> Shell::run_command(AST::Command& command)
 #endif
             int rc = dup2(rewiring.dest_fd, rewiring.source_fd);
             if (rc < 0) {
-                perror("dup2");
+                perror("dup2(run)");
                 return nullptr;
             }
         }
@@ -457,6 +474,8 @@ RefPtr<Job> Shell::run_command(AST::Command& command)
         job->disown();
     };
 
+    fds.collect();
+
     return *job;
 }