fix: parenthesis now appear in printed quote objects

r_derive/src/lib.rs

@@ -77,7 +77,11 @@ pub fn builtin(attr: TokenStream, item: TokenStream) -> TokenStream {
             }
 
             #[automatically_derived]
-            impl Builtin for #what {}
+            impl Builtin for #what {
+                fn kind(&self) -> SymKind {
+                    SymKind::#kind
+                }
+            }
         },
         Builtin::Keyword => quote! {
             #item
@@ -97,6 +101,10 @@ pub fn builtin(attr: TokenStream, item: TokenStream) -> TokenStream {
                 fn is_transparent(&self) -> bool {
                     true
                 }
+
+                fn kind(&self) -> SymKind {
+                    SymKind::Keyword
+                }
             }
         },
     };

src/callable/core.rs

@@ -194,14 +194,24 @@ pub trait Builtin: Callable + CallableClone + Format + DynCompare + Sync {
     fn is_transparent(&self) -> bool {
         false
     }
+
+    fn is_infix(&self) -> bool {
+        self.kind() == SymKind::Infix
+    }
+
+    fn kind(&self) -> SymKind {
+        SymKind::Function
+    }
 }
 
 pub trait Sym {
     const SYM: &'static str;
     const KIND: &'static SymKind;
 }
 
+#[derive(PartialEq)]
 pub enum SymKind {
+    Keyword,
     Function,
     Infix,
     Prefix,
@@ -233,6 +243,7 @@ where
                 let rest = args.collect::<ExprList>();
                 format!("{}{l}{}{r}", first.1, rest)
             }
+            Keyword => sym.to_string(), // keywords generally implement their own formatter
         }
     }
 

src/callable/keywords.rs

@@ -209,6 +209,27 @@ impl Callable for KeywordRepeat {
     }
 }
 
+#[derive(Debug, Clone, PartialEq)]
+#[builtin]
+pub struct KeywordParen;
+
+impl Format for KeywordParen {
+    fn rfmt_call_with(&self, _state: FormatState, args: &ExprList) -> String {
+        format!("({})", args.values.first().unwrap())
+    }
+
+    fn rfmt_with(&self, _: FormatState) -> String {
+        "(".to_string()
+    }
+}
+
+impl Callable for KeywordParen {
+    fn call(&self, args: ExprList, stack: &mut CallStack) -> EvalResult {
+        let expr = args.values.into_iter().next().unwrap();
+        stack.eval_and_finalize(expr)
+    }
+}
+
 #[derive(Debug, Clone, PartialEq)]
 #[builtin]
 pub struct KeywordBlock;
@@ -245,6 +266,7 @@ impl Callable for KeywordBlock {
         Ok(value)
     }
 }
+
 #[cfg(test)]
 mod test {
     use crate::r;

src/callable/primitive/substitute.rs

@@ -2,6 +2,7 @@ use lazy_static::lazy_static;
 use r_derive::*;
 
 use crate::callable::core::*;
+use crate::callable::keywords::KeywordParen;
 use crate::context::Context;
 use crate::internal_err;
 use crate::lang::*;
@@ -41,33 +42,62 @@ impl Callable for PrimitiveSubstitute {
             return internal_err!();
         };
 
-        fn recurse(exprs: ExprList, env: &Environment) -> ExprList {
+        fn recurse(exprs: ExprList, env: &Environment, paren: bool) -> ExprList {
             exprs
                 .into_iter()
-                .map(|(key, expr)| (key, substitute(expr, env)))
+                .map(|(key, expr)| (key, substitute(expr, env, paren)))
                 .collect()
         }
 
-        fn substitute(expr: Expr, env: &Environment) -> Expr {
+        // add parenthesis around ambigous expressions, namely anonymous functions and infix calls
+        fn paren_if_infix(expr: Expr) -> Expr {
+            match expr {
+                Function(..) => Expr::new_primitive_call(KeywordParen, ExprList::from(vec![expr])),
+                Call(what, exprs) => match *what {
+                    Primitive(p) if p.is_infix() => {
+                        let expr = Call(Box::new(Primitive(p)), exprs);
+                        Expr::new_primitive_call(KeywordParen, ExprList::from(vec![expr]))
+                    }
+                    _ => Call(what, exprs),
+                },
+                _ => expr,
+            }
+        }
+
+        fn substitute(expr: Expr, env: &Environment, paren: bool) -> Expr {
             match expr {
                 Symbol(s) => {
-                    // promises are replaced
+                    // promise expressions (ie arguments) are replaced with their unevaluated expressions
                     match env.values.borrow().get(&s) {
-                        Some(Obj::Promise(_, expr, _)) => expr.clone(),
-                        Some(Obj::Expr(e)) => e.clone(),
+                        Some(Obj::Expr(expr)) | Some(Obj::Promise(_, expr, _)) => {
+                            if paren {
+                                paren_if_infix(expr.clone())
+                            } else {
+                                expr.clone()
+                            }
+                        }
                         _ => Symbol(s),
                     }
                 }
-                List(exprs) => List(recurse(exprs, env)),
-                Function(params, body) => {
-                    Function(recurse(params, env), Box::new(substitute(*body, env)))
-                }
-                Call(what, exprs) => Call(Box::new(substitute(*what, env)), recurse(exprs, env)),
+                List(exprs) => List(recurse(exprs, env, false)),
+                Function(params, body) => Function(
+                    recurse(params, env, false),
+                    Box::new(substitute(*body, env, false)),
+                ),
+                Call(what, exprs) => match *what {
+                    Primitive(p) if p.is_infix() => {
+                        Call(Box::new(Primitive(p)), recurse(exprs, env, true))
+                    }
+                    _ => Call(
+                        Box::new(substitute(*what, env, true)),
+                        recurse(exprs, env, false),
+                    ),
+                },
                 other => other,
             }
         }
 
-        match substitute(expr, env.as_ref()) {
+        match substitute(expr, env.as_ref(), false) {
             e @ (Symbol(_) | List(..) | Function(..) | Call(..) | Primitive(..)) => {
                 Ok(Obj::Expr(e))
             }
@@ -91,8 +121,32 @@ mod test {
     #[test]
     fn quoted_expressions() {
         assert_eq!(
-            r! { x <- quote(1 + 2); substitute(0 + x) },
-            r! { quote(0 + (1 + 2)) } // note non-default associativity
+            r! { x <- quote(a(b, c)); substitute(0 + x) },
+            r! { quote(0 + a(b, c)) }
+        );
+    }
+
+    #[test]
+    fn substituted_minimally_parenthesizes() {
+        assert_eq!(
+            r! { x <- quote(1 + 2); substitute(x(a, b, x)) },
+            r! { quote((1 + 2)(a, b, 1 + 2)) }
+        );
+    }
+
+    #[test]
+    fn substituted_infix_op_calls_get_parenthesized() {
+        assert_eq!(
+            r! { x <- quote(1 + 2); substitute(0 * x) },
+            r! { quote(0 * (1 + 2)) } // note non-default associativity
+        );
+    }
+
+    #[test]
+    fn substituted_functions_gets_parenthesized() {
+        assert_eq!(
+            r! { x <- quote(function(a, b) a + b); substitute(0 + x) },
+            r! { quote(0 + (function(a, b) a + b)) }
         );
     }
 

src/grammar/grammar.pest

@@ -121,7 +121,7 @@
             block_exprs = { WS* ~ expr? ~ ( block_sep+ ~ expr? )* ~ WS* }
             block_sep = _{ WS_NO_NL* ~ ( ";" | comment? ~ NEWLINE ) ~ WS* }
 
-        paren_expr = _{ "(" ~ WS* ~ expr ~ WS* ~ ")" }
+        paren_expr = { "(" ~ WS* ~ expr ~ WS* ~ ")" }
 
         atom = _{
               block

src/object/ast.rs

@@ -85,7 +85,7 @@ impl fmt::Display for Expr {
             Expr::Call(what, args) => match &**what {
                 Expr::Primitive(p) => write!(f, "{}", p.rfmt_call(args)),
                 Expr::String(s) | Expr::Symbol(s) => write!(f, "{}({})", s, args),
-                rexpr => write!(f, "({})({})", rexpr, args),
+                rexpr => write!(f, "{}({})", rexpr, args),
             },
             Expr::Function(head, body) => write!(f, "function({}) {}", head, body),
             Expr::Primitive(p) => write!(f, "Primitive(\"{}\")", p.rfmt()),

src/parser/core.rs

@@ -85,6 +85,7 @@ where
 
         // bracketed expression block
         en::Rule::expr => parse_expr(config, parser, pratt, pair.into_inner()),
+        en::Rule::paren_expr => parse_paren(config, parser, pratt, pair),
         en::Rule::block_exprs => parse_block(config, parser, pratt, pair),
 
         // keyworded composite expressions
@@ -147,6 +148,26 @@ where
     }
 }
 
+fn parse_paren<P, R>(
+    config: &SessionParserConfig,
+    parser: &P,
+    pratt: &PrattParser<R>,
+    pair: Pair<R>,
+) -> ParseResult
+where
+    P: Parser<R> + LocalizedParser,
+    R: RuleType + Into<en::Rule>,
+{
+    // extract each inline expression, and treat as unnamed list
+    let exprs: ExprList = pair
+        .into_inner()
+        .map(|i| parse_expr(config, parser, pratt, i.into_inner()))
+        .collect::<Result<_, _>>()?;
+
+    // build call from symbol and list
+    Ok(Expr::new_primitive_call(KeywordParen, exprs))
+}
+
 fn parse_block<P, R>(
     config: &SessionParserConfig,
     parser: &P,