Merge pull request #81 from wycats/better-parser

Improve parser

src/cli.rs

@@ -10,7 +10,8 @@ use crate::evaluate::Scope;
 crate use crate::format::{EntriesListView, GenericView};
 use crate::git::current_branch;
 use crate::object::Value;
-use crate::parser::{ParsedCommand, Pipeline};
+use crate::parser::ast::{Expression, Leaf};
+use crate::parser::{Args, ParsedCommand, Pipeline};
 use crate::stream::empty_stream;
 
 use log::debug;
@@ -208,6 +209,18 @@ async fn process_line(readline: Result<String, ReadlineError>, ctx: &mut Context
                 input = match (item, next) {
                     (None, _) => break,
 
+                    (Some(ClassifiedCommand::Expr(_)), _) => {
+                        return LineResult::Error(ShellError::unimplemented(
+                            "Expression-only commands",
+                        ))
+                    }
+
+                    (_, Some(ClassifiedCommand::Expr(_))) => {
+                        return LineResult::Error(ShellError::unimplemented(
+                            "Expression-only commands",
+                        ))
+                    }
+
                     (
                         Some(ClassifiedCommand::Internal(left)),
                         Some(ClassifiedCommand::Internal(_)),
@@ -305,20 +318,25 @@ fn classify_pipeline(
 }
 
 fn classify_command(
-    command: &ParsedCommand,
+    command: &Expression,
     context: &Context,
 ) -> Result<ClassifiedCommand, ShellError> {
-    let command_name = &command.name[..];
+    // let command_name = &command.name[..];
-    let args = &command.args;
+    // let args = &command.args;
 
-    match command_name {
+    if let Expression::Call(call) = command {
-        other => match context.has_command(command_name) {
+        match (&call.name, &call.args) {
+            (Expression::Leaf(Leaf::Bare(name)), args) => {
+                match context.has_command(&name.to_string()) {
                     true => {
-                let command = context.get_command(command_name);
+                        let command = context.get_command(&name.to_string());
                         let config = command.config();
                         let scope = Scope::empty();
 
-                let args = config.evaluate_args(args.iter(), &scope)?;
+                        let args = match args {
+                            Some(args) => config.evaluate_args(args.iter(), &scope)?,
+                            None => Args::default(),
+                        };
 
                         Ok(ClassifiedCommand::Internal(InternalCommand {
                             command,
@@ -326,15 +344,29 @@ fn classify_command(
                         }))
                     }
                     false => {
-                let arg_list_strings: Vec<String> =
+                        let arg_list_strings: Vec<String> = match args {
-                    args.iter().map(|i| i.as_external_arg()).collect();
+                            Some(args) => args.iter().map(|i| i.as_external_arg()).collect(),
+                            None => vec![],
+                        };
 
                         Ok(ClassifiedCommand::External(ExternalCommand {
-                    name: other.to_string(),
+                            name: name.to_string(),
                             args: arg_list_strings,
                         }))
                     }
-        },
+                }
+            }
+
+            (_, None) => Err(ShellError::string(
+                "Unimplemented command that is just an expression (1)",
+            )),
+            (_, Some(args)) => Err(ShellError::string("Unimplemented dynamic command")),
+        }
+    } else {
+        Err(ShellError::string(&format!(
+            "Unimplemented command that is just an expression (2) -- {:?}",
+            command
+        )))
     }
 }
 

src/commands/classified.rs

@@ -1,3 +1,4 @@
+use crate::parser::ast::Expression;
 use crate::parser::registry::Args;
 use crate::prelude::*;
 use bytes::{BufMut, BytesMut};
@@ -75,6 +76,7 @@ crate struct ClassifiedPipeline {
 }
 
 crate enum ClassifiedCommand {
+    Expr(Expression),
     Internal(InternalCommand),
     External(ExternalCommand),
 }

src/errors.rs

@@ -48,6 +48,10 @@ impl ShellError {
         ShellError::String(StringError::new(title.into(), Value::nothing()))
     }
 
+    crate fn unimplemented(title: impl Into<String>) -> ShellError {
+        ShellError::string(&format!("Unimplemented: {}", title.into()))
+    }
+
     crate fn copy_error(&self) -> ShellError {
         self.clone()
     }
@@ -58,6 +62,23 @@ pub struct ShellDiagnostic {
     crate diagnostic: Diagnostic<Span>,
 }
 
+impl ShellDiagnostic {
+    crate fn simple_diagnostic(
+        span: impl Into<Span>,
+        source: impl Into<String>,
+    ) -> ShellDiagnostic {
+        use language_reporting::*;
+
+        let span = span.into();
+        let source = source.into();
+
+        let diagnostic =
+            Diagnostic::new(Severity::Error, "Parse error").with_label(Label::new_primary(span));
+
+        ShellDiagnostic { diagnostic }
+    }
+}
+
 impl PartialEq for ShellDiagnostic {
     fn eq(&self, _other: &ShellDiagnostic) -> bool {
         false

src/evaluate/evaluator.rs

@@ -23,6 +23,7 @@ impl Scope {
 crate fn evaluate_expr(expr: &ast::Expression, scope: &Scope) -> Result<Value, ShellError> {
     use ast::*;
     match expr {
+        Expression::Call(c) => Err(ShellError::unimplemented("Evaluating call expression")),
         Expression::Leaf(l) => Ok(evaluate_leaf(l)),
         Expression::Parenthesized(p) => evaluate_expr(&p.expr, scope),
         Expression::Flag(f) => Ok(Value::Primitive(Primitive::String(f.print()))),

src/format/table.rs

@@ -1,5 +1,5 @@
 use crate::format::RenderView;
-use crate::object::Value;
+use crate::object::{DataDescriptor, Value};
 use crate::prelude::*;
 use derive_new::new;
 use prettytable::{color, Attr, Cell, Row, Table};
@@ -11,7 +11,7 @@ use prettytable::{color, Attr, Cell, Row, Table};
 // another_name : ...
 #[derive(new)]
 pub struct TableView {
-    headers: Vec<String>,
+    headers: Vec<DataDescriptor>,
     entries: Vec<Vec<String>>,
 }
 
@@ -22,18 +22,16 @@ impl TableView {
         }
 
         let item = &values[0];
-        let descs = item.data_descriptors();
+        let headers = item.data_descriptors();
 
-        if descs.len() == 0 {
+        if headers.len() == 0 {
             return None;
         }
 
-        let headers: Vec<String> = descs.iter().map(|d| d.name.display().to_string()).collect();
-
         let mut entries = vec![];
 
         for value in values {
-            let row = descs
+            let row = headers
                 .iter()
                 .enumerate()
                 .map(|(i, d)| value.get_data(d).borrow().format_leaf(Some(&headers[i])))
@@ -60,7 +58,7 @@ impl RenderView for TableView {
             .headers
             .iter()
             .map(|h| {
-                Cell::new(h)
+                Cell::new(h.display_header())
                     .with_style(Attr::ForegroundColor(color::GREEN))
                     .with_style(Attr::Bold)
             })

src/object/base.rs

@@ -75,7 +75,7 @@ impl Primitive {
         .to_string()
     }
 
-    crate fn format(&self, field_name: Option<&str>) -> String {
+    crate fn format(&self, field_name: Option<&DataDescriptor>) -> String {
         match self {
             Primitive::Nothing => format!("{}", Color::Black.bold().paint("-")),
             Primitive::Bytes(b) => {
@@ -98,8 +98,10 @@ impl Primitive {
             Primitive::Boolean(b) => match (b, field_name) {
                 (true, None) => format!("Yes"),
                 (false, None) => format!("No"),
-                (true, Some(s)) => format!("{}", s),
+                (true, Some(s)) if s.is_string_name() => format!("{}", s.display_header()),
-                (false, Some(_)) => format!(""),
+                (false, Some(s)) if s.is_string_name() => format!(""),
+                (true, Some(_)) => format!("Yes"),
+                (false, Some(_)) => format!("No"),
             },
             Primitive::Date(d) => format!("{}", d.humanize()),
         }
@@ -207,9 +209,9 @@ impl Value {
         }
     }
 
-    crate fn format_leaf(&self, field_name: Option<&str>) -> String {
+    crate fn format_leaf(&self, desc: Option<&DataDescriptor>) -> String {
         match self {
-            Value::Primitive(p) => p.format(field_name),
+            Value::Primitive(p) => p.format(desc),
             Value::Block(b) => b.expression.print(),
             Value::Object(_) => format!("[object Object]"),
             Value::List(_) => format!("[list List]"),

src/object/desc.rs

@@ -1,7 +1,7 @@
 use crate::object::types::Type;
 use derive_new::new;
-use serde_derive::{Deserialize, Serialize};
 use serde::{Serialize, Serializer};
+use serde_derive::{Deserialize, Serialize};
 
 #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Serialize, Deserialize, Hash)]
 pub enum DescriptorName {
@@ -39,18 +39,27 @@ pub struct DataDescriptor {
     crate ty: Type,
 }
 
+impl DataDescriptor {
+    crate fn display_header(&self) -> &str {
+        self.name.display()
+    }
+
+    crate fn is_string_name(&self) -> bool {
+        match self.name {
+            DescriptorName::String(_) => true,
+            DescriptorName::ValueOf => false,
+        }
+    }
+}
+
 impl Serialize for DataDescriptor {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: Serializer,
     {
         match self.name {
-            DescriptorName::String(ref s) => {
+            DescriptorName::String(ref s) => serializer.serialize_str(s),
-                serializer.serialize_str(s)
+            DescriptorName::ValueOf => serializer.serialize_str("value"),
-            }
-            DescriptorName::ValueOf => {
-                serializer.serialize_str("value")
-            }
         }
     }
 }

src/parser.rs

@@ -10,12 +10,20 @@ crate use registry::{Args, CommandConfig};
 
 use crate::errors::ShellError;
 use lexer::Lexer;
+use log::trace;
 use parser::PipelineParser;
 
 pub fn parse(input: &str) -> Result<Pipeline, ShellError> {
+    let _ = pretty_env_logger::try_init();
+
     let parser = PipelineParser::new();
     let tokens = Lexer::new(input, false);
 
+    trace!(
+        "Tokens: {:?}",
+        tokens.clone().collect::<Result<Vec<_>, _>>()
+    );
+
     match parser.parse(tokens) {
         Ok(val) => Ok(val),
         Err(err) => Err(ShellError::parse_error(err, input.to_string())),
@@ -25,12 +33,33 @@ pub fn parse(input: &str) -> Result<Pipeline, ShellError> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::parser::ast::{bare, binary, flag, short, unit, var, Pipeline};
+    use crate::parser::ast::{bare, flag, short, unit, var, Expression, Operator, Pipeline};
     use pretty_assertions::assert_eq;
 
     fn assert_parse(source: &str, expected: Pipeline) {
-        let parsed = parse(source).unwrap();
+        let parsed = match parse(source) {
+            Ok(p) => p,
+            Err(ShellError::Diagnostic(diag, source)) => {
+                use language_reporting::termcolor;
+
+                let writer = termcolor::StandardStream::stdout(termcolor::ColorChoice::Auto);
+                let files = crate::parser::span::Files::new(source);
+
+                language_reporting::emit(
+                    &mut writer.lock(),
+                    &files,
+                    &diag.diagnostic,
+                    &language_reporting::DefaultConfig,
+                )
+                .unwrap();
+
+                panic!("Test failed")
+            }
+            Err(err) => panic!("Something went wrong during parse: {:#?}", err),
+        };
+
         let printed = parsed.print();
+
         assert_eq!(parsed, expected);
         assert_eq!(source, printed);
     }
@@ -47,15 +76,15 @@ mod tests {
 
     macro_rules! command {
         ($name:ident $( $command:expr )*) => {
-            ParsedCommand::new(stringify!($name).into(), vec![ $($command.into()),* ])
+            Expression::call(Expression::bare(stringify!($name)), vec![ $($command.into()),* ])
         };
 
         ($name:ident $( $command:expr )*) => {
-            ParsedCommand::new(stringify!($name).into(), vec![ $($command.into()),* ])
+            Expression::call(Expression::bare(stringify!($name)), vec![ $($command.into()),* ])
         };
 
         ($name:tt $( $command:expr )*) => {
-            ParsedCommand::new($name.into(), vec![ $($command.into()),* ])
+            Expression::call(Expression::bare($name), vec![ $($command.into()),* ])
         };
     }
 
@@ -164,4 +193,12 @@ mod tests {
             ],
         );
     }
+
+    fn binary(
+        left: impl Into<Expression>,
+        op: impl Into<Operator>,
+        right: impl Into<Expression>,
+    ) -> Expression {
+        Expression::binary(left, op, right)
+    }
 }

src/parser/ast.rs

@@ -59,6 +59,7 @@ pub enum Expression {
     Block(Box<Block>),
     Binary(Box<Binary>),
     Path(Box<Path>),
+    Call(Box<ParsedCommand>),
     VariableReference(Variable),
 }
 
@@ -68,6 +69,12 @@ impl From<&str> for Expression {
     }
 }
 
+impl From<String> for Expression {
+    fn from(input: String) -> Expression {
+        Expression::Leaf(Leaf::String(input.into()))
+    }
+}
+
 impl From<i64> for Expression {
     fn from(input: i64) -> Expression {
         Expression::Leaf(Leaf::Int(input.into()))
@@ -99,8 +106,41 @@ impl From<Binary> for Expression {
 }
 
 impl Expression {
+    crate fn leaf(leaf: impl Into<Leaf>) -> Expression {
+        Expression::Leaf(leaf.into())
+    }
+
+    crate fn flag(flag: impl Into<Flag>) -> Expression {
+        Expression::Flag(flag.into())
+    }
+
+    crate fn call(head: Expression, tail: Vec<Expression>) -> Expression {
+        if tail.len() == 0 {
+            Expression::Call(Box::new(ParsedCommand::new(head.into(), None)))
+        } else {
+            Expression::Call(Box::new(ParsedCommand::new(head.into(), Some(tail))))
+        }
+    }
+
+    crate fn binary(
+        left: impl Into<Expression>,
+        operator: impl Into<Operator>,
+        right: impl Into<Expression>,
+    ) -> Expression {
+        Expression::Binary(Box::new(Binary {
+            left: left.into(),
+            operator: operator.into(),
+            right: right.into(),
+        }))
+    }
+
+    crate fn block(expr: impl Into<Expression>) -> Expression {
+        Expression::Block(Box::new(Block::new(expr.into())))
+    }
+
     crate fn print(&self) -> String {
         match self {
+            Expression::Call(c) => c.print(),
             Expression::Leaf(l) => l.print(),
             Expression::Flag(f) => f.print(),
             Expression::Parenthesized(p) => p.print(),
@@ -113,6 +153,7 @@ impl Expression {
 
     crate fn as_external_arg(&self) -> String {
         match self {
+            Expression::Call(c) => c.as_external_arg(),
             Expression::Leaf(l) => l.as_external_arg(),
             Expression::Flag(f) => f.as_external_arg(),
             Expression::Parenthesized(p) => p.as_external_arg(),
@@ -123,6 +164,10 @@ impl Expression {
         }
     }
 
+    crate fn bare(path: impl Into<BarePath>) -> Expression {
+        Expression::Leaf(Leaf::Bare(path.into()))
+    }
+
     crate fn as_string(&self) -> Option<String> {
         match self {
             Expression::Leaf(Leaf::String(s)) => Some(s.to_string()),
@@ -131,6 +176,13 @@ impl Expression {
         }
     }
 
+    crate fn as_bare(&self) -> Option<String> {
+        match self {
+            Expression::Leaf(Leaf::Bare(p)) => Some(p.to_string()),
+            _ => None,
+        }
+    }
+
     crate fn is_flag(&self, value: &str) -> bool {
         match self {
             Expression::Flag(Flag::Longhand(f)) if value == f => true,
@@ -218,8 +270,8 @@ impl Variable {
     crate fn from_str(input: &str) -> Expression {
         match input {
             "it" => Expression::VariableReference(Variable::It),
-            "true" => Expression::Leaf(Leaf::Boolean(true)),
+            "yes" => Expression::Leaf(Leaf::Boolean(true)),
-            "false" => Expression::Leaf(Leaf::Boolean(false)),
+            "no" => Expression::Leaf(Leaf::Boolean(false)),
             other => Expression::VariableReference(Variable::Other(other.to_string())),
         }
     }
@@ -236,8 +288,7 @@ impl Variable {
     }
 }
 
-#[cfg(test)]
+pub fn bare(s: impl Into<String>) -> BarePath {
-pub fn bare(s: &str) -> BarePath {
     BarePath {
         head: s.into(),
         tail: vec![],
@@ -250,7 +301,23 @@ pub struct BarePath {
     tail: Vec<String>,
 }
 
+impl<T: Into<String>> From<T> for BarePath {
+    fn from(input: T) -> BarePath {
+        BarePath {
+            head: input.into(),
+            tail: vec![],
+        }
+    }
+}
+
 impl BarePath {
+    crate fn from_token(head: SpannedToken) -> BarePath {
+        BarePath {
+            head: head.to_string(),
+            tail: vec![],
+        }
+    }
+
     crate fn from_tokens(head: SpannedToken, tail: Vec<SpannedToken>) -> BarePath {
         BarePath {
             head: head.to_string(),
@@ -363,19 +430,6 @@ impl Binary {
     }
 }
 
-#[cfg(test)]
-crate fn binary(
-    left: impl Into<Expression>,
-    operator: impl Into<Operator>,
-    right: impl Into<Expression>,
-) -> Binary {
-    Binary {
-        left: left.into(),
-        operator: operator.into(),
-        right: right.into(),
-    }
-}
-
 impl Binary {
     fn print(&self) -> String {
         format!(
@@ -427,22 +481,37 @@ impl Flag {
     }
 }
 
-#[derive(new, Debug, Clone, Eq, PartialEq)]
+#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq, new)]
 pub struct ParsedCommand {
-    crate name: String,
+    crate name: Expression,
-    crate args: Vec<Expression>,
+    crate args: Option<Vec<Expression>>,
 }
 
 impl ParsedCommand {
-    #[allow(unused)]
+    fn as_external_arg(&self) -> String {
+        let mut out = vec![];
+
+        write!(out, "{}", self.name.as_external_arg()).unwrap();
+
+        if let Some(args) = &self.args {
+            for arg in args.iter() {
+                write!(out, " {}", arg.as_external_arg()).unwrap();
+            }
+        }
+
+        String::from_utf8_lossy(&out).into_owned()
+    }
+
     fn print(&self) -> String {
         let mut out = vec![];
 
-        write!(out, "{}", self.name).unwrap();
+        write!(out, "{}", self.name.print()).unwrap();
 
-        for arg in self.args.iter() {
+        if let Some(args) = &self.args {
+            for arg in args.iter() {
                 write!(out, " {}", arg.print()).unwrap();
             }
+        }
 
         String::from_utf8_lossy(&out).into_owned()
     }
@@ -451,8 +520,8 @@ impl ParsedCommand {
 impl From<&str> for ParsedCommand {
     fn from(input: &str) -> ParsedCommand {
         ParsedCommand {
-            name: input.to_string(),
+            name: Expression::Leaf(Leaf::Bare(bare(input))),
-            args: vec![],
+            args: None,
         }
     }
 }
@@ -460,19 +529,19 @@ impl From<&str> for ParsedCommand {
 impl From<(&str, Vec<Expression>)> for ParsedCommand {
     fn from(input: (&str, Vec<Expression>)) -> ParsedCommand {
         ParsedCommand {
-            name: input.0.to_string(),
+            name: Expression::bare(input.0),
-            args: input.1,
+            args: Some(input.1),
         }
     }
 }
 
 #[derive(new, Debug, Eq, PartialEq)]
 pub struct Pipeline {
-    crate commands: Vec<ParsedCommand>,
+    crate commands: Vec<Expression>,
 }
 
 impl Pipeline {
-    crate fn from_parts(command: ParsedCommand, rest: Vec<ParsedCommand>) -> Pipeline {
+    crate fn from_parts(command: Expression, rest: Vec<Expression>) -> Pipeline {
         let mut commands = vec![command];
         commands.extend(rest);
 

src/parser/lexer.rs

@@ -23,7 +23,7 @@ crate enum TopToken {
     #[regex = r#""([^"]|\\")*""#]
     DQString,
 
-    #[regex = r"\$"]
+    #[token = "$"]
     #[callback = "start_variable"]
     Dollar,
 
@@ -257,6 +257,12 @@ crate enum AfterMemberDot {
     #[callback = "finish_member"]
     Member,
 
+    #[regex = r#"'([^']|\\')*'"#]
+    SQString,
+
+    #[regex = r#""([^"]|\\")*""#]
+    DQString,
+
     #[regex = r"\s"]
     Whitespace,
 }
@@ -268,6 +274,9 @@ impl AfterMemberDot {
         let result = match self {
             END => return None,
             Member => Token::Member,
+            SQString => Token::SQMember,
+            DQString => Token::DQMember,
+
             Whitespace => Token::Whitespace,
             Error => unreachable!("Don't call to_token with the error variant"),
         };
@@ -387,6 +396,8 @@ pub enum Token {
     Variable,
     PathDot,
     Member,
+    SQMember,
+    DQMember,
     Num,
     SQString,
     DQString,
@@ -418,6 +429,7 @@ pub enum Token {
 //     Whitespace(SpannedToken<'source, &'source str>),
 // }
 
+#[derive(Clone)]
 crate struct Lexer<'source> {
     lexer: logos::Lexer<TopToken, &'source str>,
     first: bool,

src/parser/parser.lalrpop

@@ -6,72 +6,117 @@ use crate::prelude::*;
 use crate::parser::lexer::{SpannedToken, Token};
 use byte_unit::Byte;
 
+// nu's grammar is a little bit different from a lot of other languages, to better match
+// the idioms and constraints of a shell environment. A lot of the constraints are
+// the same as PowerShell, but mostly derived from the same first principles.
+//
+// - Other than at the beginning of a command, bare words are virtually always parsed as
+//   strings. This means that, in general, bare words cannot be used as keywords or
+//   variables.
+// - Variable names begin with `$`, and so do keywords
+// - Functions are invoked without `()` and without comma separation
+// - In general, because of the lack of comma-separation, expressions must be grouped:
+//   - a single token
+//   - a path ($variable followed by any number of `"." member`)
+//   - parenthesized expression
+// - This means that more elaborate expressions, like binary expressions, must usually
+//   be parenthesized
+// - There is a special case for a command that takes a single expression, which can
+//   omit the parens
+
 grammar<'input>;
 
 pub Pipeline: Pipeline = {
-  <first:Command> => Pipeline::new(vec![first]),
+  <first:PipelineElement> <rest: ( "|" <PipelineElement> )*> => Pipeline::from_parts(first, rest),
-  <first:Command> <rest: ( "|" <Command> )+> => Pipeline::from_parts(first, rest),
 }
 
-Command: ParsedCommand = {
+PipelineElement: Expression = {
-    <command:BarePath> => ParsedCommand::new(command.to_string(), vec![]),
+    <Bare> => Expression::call(Expression::bare(<>), vec![]),
-    <command:BarePath> <expr:Expr+> => ParsedCommand::new(command.to_string(), expr),
+    <SingleExpression> => <>,
-    <command:BarePath> <expr:BinaryExpression> => ParsedCommand::new(command.to_string(), vec![expr]),
 }
 
-Leaf: Expression = {
+// A leaf expression is a single logical token that directly represents an expression
-    <String> => Expression::Leaf(Leaf::String(<>)),
+LeafExpression: Expression = {
-    <Int> => Expression::Leaf(Leaf::Int(<>)),
+    <String> => <>,
-    <UnitsNum> => Expression::Leaf(<>),
+    <Int> => Expression::leaf(Leaf::Int(<>)),
+    <UnitsNum> => <>,
     <Var> => <>,
 }
 
-BinaryExpression: Expression = {
+pub Call: Expression = {
-    <left:Expr> <op:Operator> <right:Expr> => Expression::Binary(Box::new(Binary::new(left, op, right))),
+    <expr:Expression> <rest:SingleCallArgument> => Expression::call(expr, vec![rest]),
+    <expr:Expression> <first:CallArgument> <rest:( <CallArgument> )+> => Expression::call(expr, { let mut rest = rest; let mut v = vec![first]; v.append(&mut rest); v }),
+    <expr:Bare> <rest:SingleCallArgument> => Expression::call(Expression::bare(expr), vec![rest]),
+    <expr:Bare> <first:CallArgument> <rest:( <CallArgument> )+> => Expression::call(Expression::bare(expr), { let mut v = vec![first]; let mut rest = rest; v.append(&mut rest); v }),
 }
 
-Parenthesized: Expression = {
+Binary: Expression = {
-    "(" <Leaf> ")" => Expression::Parenthesized(Box::new(Parenthesized::new(<>))),
+    <left:ArgumentExpression> <op:Operator> <right:ArgumentExpression> => Expression::binary(left, op, right),
-    "(" <BinaryExpression> ")" => Expression::Parenthesized(Box::new(Parenthesized::new(<>))),
-}
-
-AtomicExpression: Expression = {
-    <Parenthesized>,
-    <Leaf>,
 }
 
+// In a block, a single bare word is interpreted as a call:
+//
+// foreach { ls }
 Block: Expression = {
-    "{" <AtomicExpression> "}" => Expression::Block(Box::new(Block::new(<>))),
+    "{" <SingleExpression> "}" => Expression::block(<>),
-    "{" <BinaryExpression> "}" => Expression::Block(Box::new(Block::new(<>))),
+    "{" <Bare> "}" => Expression::block(Expression::call(Expression::bare(<>), vec![])),
 }
 
-WholeExpression: Expression = {
+// An `Expression` is the most general kind of expression. It can go anywhere, even right next to another expression, and
-    <AtomicExpression>,
+// even as the first part of a call.
-    <Block>,
+Expression: Expression = {
+    <LeafExpression> => <>,
+    <Block> => <>,
+    "(" <Call> ")" => <>,
+    "(" <Bare> ")" => Expression::call(Expression::bare(<>), vec![]),
+    "(" <Binary> ")" => <>,
 }
 
-PathHead: Expression = {
+// An `ArgumentExpression` is an expression that appears in an argument list. It includes all of `Expression`, and
-    <WholeExpression>,
+// bare words are interpreted as strings.
-    <BarePath> => Expression::Leaf(Leaf::Bare(<>)),
+ArgumentExpression: Expression = {
-    <Flag> => Expression::Flag(<>),
+    <Expression>,
+    <Bare> => Expression::bare(<>),
 }
 
-PathExpression: Expression = {
+CallArgument: Expression = {
-    <head:WholeExpression> <tail: ( "???." <Member> )+> => Expression::Path(Box::new(Path::new(head, tail)))
+    <ArgumentExpression> => <>,
+    <Flag> => Expression::flag(<>),
 }
 
-Expr: Expression = {
+SingleCallArgument: Expression = {
-    <PathExpression>,
+    <CallArgument>,
-    <PathHead>
+    <Binary>,
 }
 
-Var: Expression = {
+// A `SingleExpression` is a special-case of `Expression` for situations where expressions do not appear side-by-side.
-    "$" <"variable"> => Variable::from_str(<>.as_slice()),
+// Because expression lists in nu are not comma-separated, composite expressions (like binary expressions) must be
+// parenthesized in lists. If only a single expression appears alone, the parentheses may be left out.
+//
+// `SingleExpression` does not include `Bare`, because expressions that include `SingleExpression` must decide how
+// to interpret a single bare word (`foreach { ls }` vs `cd ls`).
+SingleExpression: Expression = {
+    <Expression>,
+    <Call>,
+    <Binary>,
 }
 
+// === LOGICAL TOKENS === //
+
+// A logical token may be composed of more than one raw token, but the tokens must be emitted
+// from the stream in exactly one sequence. This allows us to use parser infrastructure to
+// compose tokens without the risk that these logical tokens will introduce ambiguities.
+
+Bare: BarePath = {
+    <head: "bare"> => BarePath::from_token(head)
+}
+
+// A member is a special token that represents bare words or string literals immediate
+// following a dot.
 Member: String = {
     <"member"> => <>.to_string(),
-    <String>
+    <"dqmember"> => <>.to_string(),
+    <"sqmember"> => <>.to_string(),
 }
 
 Operator: Operator = {
@@ -83,26 +128,26 @@ Operator: Operator = {
     ">=" => Operator::GreaterThanOrEqual
 }
 
-Flag: Flag = {
-    "-" <BarePath> => Flag::Shorthand(<>.to_string()),
-    "--" <BarePath> => Flag::Longhand(<>.to_string()),
-}
-
-String: String = {
-    <"sqstring"> => <>.as_slice()[1..(<>.as_slice().len() - 1)].to_string(),
-    <"dqstring"> => <>.as_slice()[1..(<>.as_slice().len() - 1)].to_string()
-}
-
-BarePath: BarePath = {
-    <head: "bare"> <tail: ( "???." <"member"> )*> => BarePath::from_tokens(head, tail)
-}
-
 Int: i64 = {
     <"num"> => i64::from_str(<>.as_slice()).unwrap()
 }
 
-UnitsNum: Leaf = {
+UnitsNum: Expression = {
-    <num: Int> <unit: "unit"> => Leaf::Unit(num, Unit::from_str(unit.as_slice()).unwrap())
+    <num: Int> <unit: "unit"> => Expression::leaf(Leaf::Unit(num, Unit::from_str(unit.as_slice()).unwrap()))
+}
+
+String: Expression = {
+    <"sqstring"> => <>.as_slice()[1..(<>.as_slice().len() - 1)].to_string().into(),
+    <"dqstring"> => <>.as_slice()[1..(<>.as_slice().len() - 1)].to_string().into()
+}
+
+Flag: Flag = {
+    "-" <Bare> => Flag::Shorthand(<>.to_string()),
+    "--" <Bare> => Flag::Longhand(<>.to_string()),
+}
+
+Var: Expression = {
+    "$" <"variable"> => Variable::from_str(<>.as_slice()).into(),
 }
 
 extern {
@@ -127,6 +172,8 @@ extern {
         "???." => SpannedToken { token: Token::PathDot, .. },
         "num" => SpannedToken { token: Token::Num, .. },
         "member" => SpannedToken { token: Token::Member, .. },
+        "sqmember" => SpannedToken { token: Token::SQMember, .. },
+        "dqmember" => SpannedToken { token: Token::SQMember, .. },
         "variable" => SpannedToken { token: Token::Variable, .. },
         "bare" => SpannedToken { token: Token::Bare, .. },
         "dqstring" => SpannedToken { token: Token::DQString, .. },

src/parser/registry.rs

@@ -73,6 +73,7 @@ pub struct CommandConfig {
     crate named: IndexMap<String, NamedType>,
 }
 
+#[derive(Debug, Default)]
 pub struct Args {
     pub positional: Vec<Value>,
     pub named: IndexMap<String, Value>,