Accept idents as KW args, fixup pipe as input for KW calls

Signed-off-by: Nick Cameron <[email protected]>

Author: nrc

rust/Cargo.toml

@@ -26,6 +26,9 @@ insta = { opt-level = 3 }
 [profile.test]
 debug = "line-tables-only"
 
+[workspace.features]
+artifact-graph = []
+
 [workspace.dependencies]
 async-trait = "0.1.88"
 anyhow = { version = "1" }

rust/kcl-lib/e2e/executor/main.rs

@@ -2038,17 +2038,6 @@ async fn kcl_test_ensure_nothing_left_in_batch_multi_file() {
 
     ctx.close().await;
 }
-#[tokio::test(flavor = "multi_thread")]
-async fn kcl_test_default_param_for_unlabeled() {
-    let code = r#"fn myExtrude(@sk, len) {
-  return extrude(sk, length = len)
-}
-
-sketch001 = startSketchOn(XY)
-|> circle(center = [0, 0], radius = 93.75)
-|> myExtrude(len = 40)"#;
-    let _ = execute_and_snapshot(code, None).await.unwrap();
-}
 
 #[tokio::test(flavor = "multi_thread")]
 async fn kcl_test_better_type_names() {

rust/kcl-lib/src/execution/exec_ast.rs

@@ -25,7 +25,7 @@ use crate::{
     },
     source_range::SourceRange,
     std::{
-        args::{Arg, KwArgs, TyF64},
+        args::{Arg, Args, KwArgs, TyF64},
         FunctionKind,
     },
     CompilationError,
@@ -1921,7 +1921,7 @@ fn assign_args_to_params(
 fn type_check_params_kw(
     fn_name: Option<&str>,
     function_expression: NodeRef<'_, FunctionExpression>,
-    args: &mut crate::std::args::KwArgs,
+    args: &mut KwArgs,
     exec_state: &mut ExecState,
 ) -> Result<(), KclError> {
     for (label, arg) in &mut args.labeled {
@@ -2009,18 +2009,18 @@ fn type_check_params_kw(
 fn assign_args_to_params_kw(
     fn_name: Option<&str>,
     function_expression: NodeRef<'_, FunctionExpression>,
-    mut args: crate::std::args::KwArgs,
+    mut args: Args,
     exec_state: &mut ExecState,
 ) -> Result<(), KclError> {
-    type_check_params_kw(fn_name, function_expression, &mut args, exec_state)?;
+    type_check_params_kw(fn_name, function_expression, &mut args.kw_args, exec_state)?;
 
     // Add the arguments to the memory.  A new call frame should have already
     // been created.
     let source_ranges = vec![function_expression.into()];
 
     for param in function_expression.params.iter() {
         if param.labeled {
-            let arg = args.labeled.get(&param.identifier.name);
+            let arg = args.kw_args.labeled.get(&param.identifier.name);
             let arg_val = match arg {
                 Some(arg) => arg.value.clone(),
                 None => match param.default_value {
@@ -2040,17 +2040,11 @@ fn assign_args_to_params_kw(
                 .mut_stack()
                 .add(param.identifier.name.clone(), arg_val, (&param.identifier).into())?;
         } else {
-            // TODO: Get the actual source range.
-            // Part of https://github.com/KittyCAD/modeling-app/issues/6613
-            let pipe_value_source_range = Default::default();
-            let default_unlabeled = exec_state
-                .mod_local
-                .pipe_value
-                .clone()
-                .map(|val| Arg::new(val, pipe_value_source_range));
-            let Some(unlabeled) = args.unlabeled.take().or(default_unlabeled) else {
+            let unlabelled = args.kw_args.unlabeled.take().or_else(|| args.pipe_value.take());
+
+            let Some(unlabeled) = unlabelled else {
                 let param_name = &param.identifier.name;
-                return Err(if args.labeled.contains_key(param_name) {
+                return Err(if args.kw_args.labeled.contains_key(param_name) {
                     KclError::Semantic(KclErrorDetails {
                         source_ranges,
                         message: format!("The function does declare a parameter named '{param_name}', but this parameter doesn't use a label. Try removing the `{param_name}:`"),
@@ -2065,7 +2059,7 @@ fn assign_args_to_params_kw(
             };
             exec_state.mut_stack().add(
                 param.identifier.name.clone(),
-                unlabeled.value.clone(),
+                unlabeled.value,
                 (&param.identifier).into(),
             )?;
         }
@@ -2140,7 +2134,7 @@ async fn call_user_defined_function(
 
 async fn call_user_defined_function_kw(
     fn_name: Option<&str>,
-    args: crate::std::args::KwArgs,
+    args: Args,
     memory: EnvironmentRef,
     function_expression: NodeRef<'_, FunctionExpression>,
     exec_state: &mut ExecState,
@@ -2328,8 +2322,7 @@ impl FunctionSource {
                 }
 
                 let result =
-                    call_user_defined_function_kw(fn_name.as_deref(), args.kw_args, *memory, ast, exec_state, ctx)
-                        .await;
+                    call_user_defined_function_kw(fn_name.as_deref(), args, *memory, ast, exec_state, ctx).await;
 
                 // Track return operation.
                 #[cfg(feature = "artifact-graph")]

rust/kcl-lib/src/execution/mod.rs

@@ -2266,4 +2266,20 @@ foo() |> extrude(length = 1)
 "#;
         parse_execute(ast).await.unwrap();
     }
+
+    #[tokio::test(flavor = "multi_thread")]
+    async fn default_param_for_unlabeled() {
+        // Tests that the input param for myExtrude is taken from the pipeline value and that
+        // the labelled arg to extrude is ok (i.e., just `length` not `length = length`)
+        let ast = r#"fn myExtrude(@sk, length) {
+  return extrude(sk, length)
+}
+
+sketch001 = startSketchOn(XY)
+  |> circle(center = [0, 0], radius = 93.75)
+  |> myExtrude(length = 40)
+"#;
+
+        parse_execute(ast).await.unwrap();
+    }
 }

rust/kcl-lib/src/parsing/ast/types/mod.rs

@@ -182,7 +182,7 @@ impl<T> Node<T> {
         self.comment_start = start;
     }
 
-    pub fn map_ref<'a, U: 'a>(&'a self, f: fn(&'a T) -> U) -> Node<U> {
+    pub fn map_ref<'a, U: 'a>(&'a self, f: impl Fn(&'a T) -> U) -> Node<U> {
         Node {
             inner: f(&self.inner),
             start: self.start,
@@ -2327,7 +2327,7 @@ impl Name {
 
     pub fn local_ident(&self) -> Option<Node<&str>> {
         if self.path.is_empty() && !self.abs_path {
-            Some(self.name.map_ref(|n| &n.name))
+            Some(self.name.map_ref(|n| &*n.name))
         } else {
             None
         }

rust/kcl-lib/src/parsing/parser.rs

@@ -2709,13 +2709,26 @@ fn pipe_sep(i: &mut TokenSlice) -> PResult<()> {
 }
 
 fn labeled_argument(i: &mut TokenSlice) -> PResult<LabeledArg> {
-    separated_pair(
+    (
         terminated(nameable_identifier, opt(whitespace)),
-        terminated(one_of((TokenType::Operator, "=")), opt(whitespace)),
-        expression,
+        opt((
+            terminated(one_of((TokenType::Operator, "=")), opt(whitespace)),
+            expression,
+        )),
     )
-    .map(|(label, arg)| LabeledArg { label, arg })
-    .parse_next(i)
+        .map(|(label, arg)| match arg {
+            Some((_, arg)) => LabeledArg { label, arg },
+            None => LabeledArg {
+                label: label.clone(),
+                arg: Expr::Name(Box::new(label.map_ref(|_| Name {
+                    name: label.clone(),
+                    path: Vec::new(),
+                    abs_path: false,
+                    digest: None,
+                }))),
+            },
+        })
+        .parse_next(i)
 }
 
 /// A type of a function argument.
@@ -3035,6 +3048,7 @@ fn fn_call_kw(i: &mut TokenSlice) -> PResult<Node<CallExpressionKw>> {
         return Ok(result);
     }
 
+    #[derive(Debug)]
     #[allow(clippy::large_enum_variant)]
     enum ArgPlace {
         NonCode(Node<NonCodeNode>),
@@ -4912,7 +4926,7 @@ bar = 1
 
     #[test]
     fn test_sensible_error_when_missing_equals_in_kwarg() {
-        for (i, program) in ["f(x=1,y)", "f(x=1,y,z)", "f(x=1,y,z=1)", "f(x=1, y, z=1)"]
+        for (i, program) in ["f(x=1,y + 1)", "f(x=1,y + 1,f(z))", "f(x=1,3,z=1)", "f(x=1, y::y, z=1)"]
             .into_iter()
             .enumerate()
         {
@@ -4923,11 +4937,6 @@ bar = 1
                 cause.message, "This argument needs a label, but it doesn't have one",
                 "failed test {i}: {program}"
             );
-            assert_eq!(
-                cause.source_range.start(),
-                program.find("y").unwrap(),
-                "failed test {i}: {program}"
-            );
         }
     }
 

rust/kcl-lib/src/std/args.rs

@@ -160,7 +160,7 @@ pub struct Args {
     pub ctx: ExecutorContext,
     /// If this call happens inside a pipe (|>) expression, this holds the LHS of that |>.
     /// Otherwise it's None.
-    pipe_value: Option<Arg>,
+    pub pipe_value: Option<Arg>,
 }
 
 impl Args {