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

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

Author: nrc

docs/kcl-src/modules.md

@@ -12,7 +12,7 @@ to other modules.
 
 ```kcl
 // util.kcl
-export fn increment(x) {
+export fn increment(@x) {
   return x + 1
 }
 ```
@@ -37,11 +37,11 @@ Multiple functions can be exported in a file.
 
 ```kcl
 // util.kcl
-export fn increment(x) {
+export fn increment(@x) {
   return x + 1
 }
 
-export fn decrement(x) {
+export fn decrement(@x) {
   return x - 1
 }
 ```
@@ -81,7 +81,7 @@ fn cube(center) {
     |> extrude(length = 10)
 }
 
-myCube = cube([0, 0])
+myCube = cube(center = [0, 0])
 ```
 
 *Pros*

docs/kcl-src/types.md

@@ -249,8 +249,8 @@ fn rect(origin) {
     |> close()
 }
 
-rect([0, 0])
-rect([20, 0])
+rect(origin = [0, 0])
+rect(origin = [20, 0])
 ``` 
 
 Those tags would only be available in the `rect` function and not globally.
@@ -279,8 +279,8 @@ fn rect(origin) {
     |> close()
 }
 
-rect([0, 0])
-myRect = rect([20, 0])
+rect(origin = [0, 0])
+myRect = rect(origin = [20, 0])
 
 myRect
   |> extrude(length = 10)

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,
@@ -2068,7 +2068,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 {
@@ -2156,18 +2156,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 {
@@ -2187,17 +2187,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}:`"),
@@ -2212,7 +2206,7 @@ fn assign_args_to_params_kw(
             };
             exec_state.mut_stack().add(
                 param.identifier.name.clone(),
-                unlabeled.value.clone(),
+                unlabeled.value,
                 (&param.identifier).into(),
             )?;
         }
@@ -2287,7 +2281,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,
@@ -2474,7 +2468,7 @@ impl FunctionSource {
                     });
                 }
 
-                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
             }
             FunctionSource::None => unreachable!(),
         }

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

@@ -2188,4 +2188,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

@@ -176,7 +176,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,
@@ -2390,7 +2390,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

@@ -2030,8 +2030,8 @@ fn expr_allowed_in_pipe_expr(i: &mut TokenSlice) -> PResult<Expr> {
         bool_value.map(Expr::Literal),
         tag.map(Box::new).map(Expr::TagDeclarator),
         literal.map(Expr::Literal),
-        fn_call.map(Box::new).map(Expr::CallExpression),
         fn_call_kw.map(Box::new).map(Expr::CallExpressionKw),
+        fn_call.map(Box::new).map(Expr::CallExpression),
         name.map(Box::new).map(Expr::Name),
         array,
         object.map(Box::new).map(Expr::ObjectExpression),
@@ -2050,8 +2050,8 @@ fn possible_operands(i: &mut TokenSlice) -> PResult<Expr> {
         bool_value.map(Expr::Literal),
         member_expression.map(Box::new).map(Expr::MemberExpression),
         literal.map(Expr::Literal),
-        fn_call.map(Box::new).map(Expr::CallExpression),
         fn_call_kw.map(Box::new).map(Expr::CallExpressionKw),
+        fn_call.map(Box::new).map(Expr::CallExpression),
         name.map(Box::new).map(Expr::Name),
         binary_expr_in_parens.map(Box::new).map(Expr::BinaryExpression),
         unnecessarily_bracketed,
@@ -2719,13 +2719,26 @@ fn arguments(i: &mut TokenSlice) -> PResult<Vec<Expr>> {
 }
 
 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.
@@ -2987,8 +3000,8 @@ fn binding_name(i: &mut TokenSlice) -> PResult<Node<Identifier>> {
 /// Either a positional or keyword function call.
 fn fn_call_pos_or_kw(i: &mut TokenSlice) -> PResult<Expr> {
     alt((
-        fn_call.map(Box::new).map(Expr::CallExpression),
         fn_call_kw.map(Box::new).map(Expr::CallExpressionKw),
+        fn_call.map(Box::new).map(Expr::CallExpression),
     ))
     .parse_next(i)
 }
@@ -3086,6 +3099,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>),
@@ -4975,7 +4989,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()
         {
@@ -4986,11 +5000,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/parsing/snapshots/kcl_lib__parsing__parser__snapshot_tests__a.snap

@@ -19,24 +19,6 @@ expression: actual
         "init": {
           "body": [
             {
-              "arguments": [
-                {
-                  "abs_path": false,
-                  "commentStart": 26,
-                  "end": 28,
-                  "name": {
-                    "commentStart": 26,
-                    "end": 28,
-                    "name": "XY",
-                    "start": 26,
-                    "type": "Identifier"
-                  },
-                  "path": [],
-                  "start": 26,
-                  "type": "Name",
-                  "type": "Name"
-                }
-              ],
               "callee": {
                 "abs_path": false,
                 "commentStart": 12,
@@ -55,8 +37,24 @@ expression: actual
               "commentStart": 12,
               "end": 29,
               "start": 12,
-              "type": "CallExpression",
-              "type": "CallExpression"
+              "type": "CallExpressionKw",
+              "type": "CallExpressionKw",
+              "unlabeled": {
+                "abs_path": false,
+                "commentStart": 26,
+                "end": 28,
+                "name": {
+                  "commentStart": 26,
+                  "end": 28,
+                  "name": "XY",
+                  "start": 26,
+                  "type": "Identifier"
+                },
+                "path": [],
+                "start": 26,
+                "type": "Name",
+                "type": "Name"
+              }
             },
             {
               "arguments": [

rust/kcl-lib/src/parsing/snapshots/kcl_lib__parsing__parser__snapshot_tests__ad.snap

@@ -63,7 +63,6 @@ expression: actual
       "commentStart": 56,
       "end": 74,
       "expression": {
-        "arguments": [],
         "callee": {
           "abs_path": false,
           "commentStart": 56,
@@ -82,8 +81,9 @@ expression: actual
         "commentStart": 56,
         "end": 74,
         "start": 56,
-        "type": "CallExpression",
-        "type": "CallExpression"
+        "type": "CallExpressionKw",
+        "type": "CallExpressionKw",
+        "unlabeled": null
       },
       "start": 56,
       "type": "ExpressionStatement",