KCL optional parameters (#1087)

Part of https://github.com/KittyCAD/modeling-app/issues/1006#issuecomment-1816978586

This adds support for optional parameters to the AST. They are declared with a ? suffix, e.g. `(x, tag?) => {...}`.

This PR does not actually _use_ these optional parameters anywhere. In particular, it does not change the KCL stdlib or any existing function definitions. That will happen in a follow-up PR.

src/lang/abstractSyntaxTree.test.ts

@@ -169,16 +169,24 @@ describe('testing function declaration', () => {
               end: 39,
               params: [
                 {
-                  type: 'Identifier',
+                  type: 'Parameter',
-                  start: 12,
+                  identifier: {
-                  end: 13,
+                    type: 'Identifier',
-                  name: 'a',
+                    start: 12,
+                    end: 13,
+                    name: 'a',
+                  },
+                  optional: false,
                 },
                 {
-                  type: 'Identifier',
+                  type: 'Parameter',
-                  start: 15,
+                  identifier: {
-                  end: 16,
+                    type: 'Identifier',
-                  name: 'b',
+                    start: 15,
+                    end: 16,
+                    name: 'b',
+                  },
+                  optional: false,
                 },
               ],
               body: {
@@ -244,16 +252,24 @@ const myVar = funcN(1, 2)`
               end: 37,
               params: [
                 {
-                  type: 'Identifier',
+                  type: 'Parameter',
-                  start: 12,
+                  identifier: {
-                  end: 13,
+                    type: 'Identifier',
-                  name: 'a',
+                    start: 12,
+                    end: 13,
+                    name: 'a',
+                  },
+                  optional: false,
                 },
                 {
-                  type: 'Identifier',
+                  type: 'Parameter',
-                  start: 15,
+                  identifier: {
-                  end: 16,
+                    type: 'Identifier',
-                  name: 'b',
+                    start: 15,
+                    end: 16,
+                    name: 'b',
+                  },
+                  optional: false,
                 },
               ],
               body: {

src/lang/getNodePathFromSourceRange.test.ts

@@ -1,5 +1,5 @@
 import { getNodePathFromSourceRange, getNodeFromPath } from './queryAst'
-import { Identifier, parse, initPromise } from './wasm'
+import { Identifier, parse, initPromise, Parameter } from './wasm'
 
 beforeAll(() => initPromise)
 
@@ -46,7 +46,7 @@ const b1 = cube([0,0], 10)`
 
     const ast = parse(code)
     const nodePath = getNodePathFromSourceRange(ast, sourceRange)
-    const node = getNodeFromPath<Identifier>(ast, nodePath).node
+    const node = getNodeFromPath<Parameter>(ast, nodePath).node
 
     expect(nodePath).toEqual([
       ['body', ''],
@@ -57,8 +57,8 @@ const b1 = cube([0,0], 10)`
       ['params', 'FunctionExpression'],
       [0, 'index'],
     ])
-    expect(node.type).toBe('Identifier')
+    expect(node.type).toBe('Parameter')
-    expect(node.name).toBe('pos')
+    expect(node.identifier.name).toBe('pos')
   })
   it('gets path right for deep within function definition body', () => {
     const code = `fn cube = (pos, scale) => {

src/lang/queryAst.ts

@@ -247,10 +247,10 @@ function moreNodePathFromSourceRange(
   if (_node.type === 'FunctionExpression' && isInRange) {
     for (let i = 0; i < _node.params.length; i++) {
       const param = _node.params[i]
-      if (param.start <= start && param.end >= end) {
+      if (param.identifier.start <= start && param.identifier.end >= end) {
         path.push(['params', 'FunctionExpression'])
         path.push([i, 'index'])
-        return moreNodePathFromSourceRange(param, sourceRange, path)
+        return moreNodePathFromSourceRange(param.identifier, sourceRange, path)
       }
     }
     if (_node.body.start <= start && _node.body.end >= end) {

src/lang/wasm.ts

@@ -20,6 +20,7 @@ export type { ObjectExpression } from '../wasm-lib/kcl/bindings/ObjectExpression
 export type { MemberExpression } from '../wasm-lib/kcl/bindings/MemberExpression'
 export type { PipeExpression } from '../wasm-lib/kcl/bindings/PipeExpression'
 export type { VariableDeclaration } from '../wasm-lib/kcl/bindings/VariableDeclaration'
+export type { Parameter } from '../wasm-lib/kcl/bindings/Parameter'
 export type { PipeSubstitution } from '../wasm-lib/kcl/bindings/PipeSubstitution'
 export type { Identifier } from '../wasm-lib/kcl/bindings/Identifier'
 export type { UnaryExpression } from '../wasm-lib/kcl/bindings/UnaryExpression'

src/wasm-lib/kcl/src/ast/types.rs

@@ -221,11 +221,11 @@ impl Program {
             if let Some(Value::FunctionExpression(ref mut function_expression)) = &mut value {
                 // Check if the params to the function expression contain the position.
                 for param in &mut function_expression.params {
-                    let param_source_range: SourceRange = param.clone().into();
+                    let param_source_range: SourceRange = (&param.identifier).into();
                     if param_source_range.contains(pos) {
-                        let old_name = param.name.clone();
+                        let old_name = param.identifier.name.clone();
                         // Rename the param.
-                        param.rename(&old_name, new_name);
+                        param.identifier.rename(&old_name, new_name);
                         // Now rename all the identifiers in the rest of the program.
                         function_expression.body.rename_identifiers(&old_name, new_name);
                         return;
@@ -1014,7 +1014,11 @@ impl CallExpression {
                 // Add the arguments to the memory.
                 let mut fn_memory = memory.clone();
                 for (index, param) in function_expression.params.iter().enumerate() {
-                    fn_memory.add(&param.name, fn_args.get(index).unwrap().clone(), param.into())?;
+                    fn_memory.add(
+                        &param.identifier.name,
+                        fn_args.get(index).unwrap().clone(),
+                        param.identifier.clone().into(),
+                    )?;
                 }
 
                 // Call the stdlib function
@@ -1249,10 +1253,10 @@ impl VariableDeclaration {
                     symbol_kind = SymbolKind::FUNCTION;
                     let mut children = vec![];
                     for param in &function_expression.params {
-                        let param_source_range: SourceRange = param.into();
+                        let param_source_range: SourceRange = (&param.identifier).into();
                         #[allow(deprecated)]
                         children.push(DocumentSymbol {
-                            name: param.name.clone(),
+                            name: param.identifier.name.clone(),
                             detail: None,
                             kind: SymbolKind::VARIABLE,
                             range: param_source_range.to_lsp_range(code),
@@ -1442,7 +1446,7 @@ impl From<&Box<Literal>> for MemoryItem {
     }
 }
 
-#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema, Bake)]
+#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema, Bake, Eq)]
 #[databake(path = kcl_lib::ast::types)]
 #[ts(export)]
 #[serde(tag = "type")]
@@ -2621,6 +2625,18 @@ async fn execute_pipe_body(
     }
 }
 
+/// Parameter of a KCL function.
+#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema, Bake)]
+#[databake(path = kcl_lib::ast::types)]
+#[ts(export)]
+#[serde(tag = "type")]
+pub struct Parameter {
+    /// The parameter's label or name.
+    pub identifier: Identifier,
+    /// Is the parameter optional?
+    pub optional: bool,
+}
+
 #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema, Bake)]
 #[databake(path = kcl_lib::ast::types)]
 #[ts(export)]
@@ -2628,7 +2644,7 @@ async fn execute_pipe_body(
 pub struct FunctionExpression {
     pub start: usize,
     pub end: usize,
-    pub params: Vec<Identifier>,
+    pub params: Vec<Parameter>,
     pub body: Program,
 }
 
@@ -2654,7 +2670,7 @@ impl FunctionExpression {
             "({}) => {{\n{}{}\n}}",
             self.params
                 .iter()
-                .map(|param| param.name.clone())
+                .map(|param| param.identifier.name.clone())
                 .collect::<Vec<String>>()
                 .join(", "),
             options.get_indentation(indentation_level + 1),

src/wasm-lib/kcl/src/executor.rs

@@ -908,9 +908,9 @@ pub async fn execute(
                                         // Add the arguments to the memory.
                                         for (index, param) in function_expression.params.iter().enumerate() {
                                             fn_memory.add(
-                                                &param.name,
+                                                &param.identifier.name,
                                                 args.get(index).unwrap().clone(),
-                                                param.into(),
+                                                (&param.identifier).into(),
                                             )?;
                                         }
 

src/wasm-lib/kcl/src/parser/parser_impl.rs

@@ -12,7 +12,7 @@ use crate::{
         ArrayExpression, BinaryExpression, BinaryOperator, BinaryPart, BodyItem, CallExpression, CommentStyle,
         ExpressionStatement, FunctionExpression, Identifier, Literal, LiteralIdentifier, LiteralValue,
         MemberExpression, MemberObject, NonCodeMeta, NonCodeNode, NonCodeValue, ObjectExpression, ObjectProperty,
-        PipeExpression, PipeSubstitution, Program, ReturnStatement, UnaryExpression, UnaryOperator, Value,
+        Parameter, PipeExpression, PipeSubstitution, Program, ReturnStatement, UnaryExpression, UnaryOperator, Value,
         VariableDeclaration, VariableDeclarator, VariableKind,
     },
     errors::{KclError, KclErrorDetails},
@@ -1208,26 +1208,62 @@ fn arguments(i: TokenSlice) -> PResult<Vec<Value>> {
         .parse_next(i)
 }
 
-fn not_close_paren(i: TokenSlice) -> PResult<Token> {
+fn required_param(i: TokenSlice) -> PResult<Token> {
     any.verify(|token: &Token| !matches!(token.token_type, TokenType::Brace) || token.value != ")")
         .parse_next(i)
 }
 
+fn optional_param(i: TokenSlice) -> PResult<Token> {
+    let token = required_param.parse_next(i)?;
+    let _question_mark = one_of(TokenType::QuestionMark).parse_next(i)?;
+    Ok(token)
+}
+
 /// Parameters are declared in a function signature, and used within a function.
-fn parameters(i: TokenSlice) -> PResult<Vec<Identifier>> {
+fn parameters(i: TokenSlice) -> PResult<Vec<Parameter>> {
     // Get all tokens until the next ), because that ends the parameter list.
-    let candidates: Vec<Token> = separated(0.., not_close_paren, comma_sep)
+    let candidates: Vec<_> = separated(
-        .context(expected("function parameters"))
+        0..,
-        .parse_next(i)?;
+        alt((optional_param.map(|t| (t, true)), required_param.map(|t| (t, false)))),
+        comma_sep,
+    )
+    .context(expected("function parameters"))
+    .parse_next(i)?;
+
     // Make sure all those tokens are valid parameters.
-    let params = candidates
+    let params: Vec<Parameter> = candidates
         .into_iter()
-        .map(|token| Identifier::try_from(token).and_then(Identifier::into_valid_binding_name))
+        .map(|(token, optional)| {
+            let identifier = Identifier::try_from(token).and_then(Identifier::into_valid_binding_name)?;
+            Ok(Parameter { identifier, optional })
+        })
         .collect::<Result<_, _>>()
-        .map_err(|e| ErrMode::Backtrack(ContextError::from(e)))?;
+        .map_err(|e: KclError| ErrMode::Backtrack(ContextError::from(e)))?;
+
+    // Make sure optional parameters are last.
+    if let Err(e) = optional_after_required(&params) {
+        return Err(ErrMode::Cut(ContextError::from(e)));
+    }
     Ok(params)
 }
 
+fn optional_after_required(params: &[Parameter]) -> Result<(), KclError> {
+    let mut found_optional = false;
+    for p in params {
+        if p.optional {
+            found_optional = true;
+        }
+        if !p.optional && found_optional {
+            let e = KclError::Syntax(KclErrorDetails {
+                source_ranges: vec![(&p.identifier).into()],
+                message: "mandatory parameters must be declared before optional parameters".to_owned(),
+            });
+            return Err(e);
+        }
+    }
+    Ok(())
+}
+
 impl Identifier {
     fn into_valid_binding_name(self) -> Result<Identifier, KclError> {
         // Make sure they are not assigning a variable to a stdlib function.
@@ -1895,7 +1931,7 @@ const mySk1 = startSketchAt([0, 0])"#;
             let tokens = crate::token::lexer(input);
             let actual = parameters.parse(&tokens);
             assert!(actual.is_ok(), "could not parse test {i}");
-            let actual_ids: Vec<_> = actual.unwrap().into_iter().map(|id| id.name).collect();
+            let actual_ids: Vec<_> = actual.unwrap().into_iter().map(|p| p.identifier.name).collect();
             assert_eq!(actual_ids, expected);
         }
     }
@@ -2322,6 +2358,82 @@ e
         assert!(result.err().unwrap().to_string().contains("Unexpected token"));
     }
 
+    #[test]
+    fn test_optional_param_order() {
+        for (i, (params, expect_ok)) in [
+            (
+                vec![Parameter {
+                    identifier: Identifier {
+                        start: 0,
+                        end: 0,
+                        name: "a".to_owned(),
+                    },
+                    optional: true,
+                }],
+                true,
+            ),
+            (
+                vec![Parameter {
+                    identifier: Identifier {
+                        start: 0,
+                        end: 0,
+                        name: "a".to_owned(),
+                    },
+                    optional: false,
+                }],
+                true,
+            ),
+            (
+                vec![
+                    Parameter {
+                        identifier: Identifier {
+                            start: 0,
+                            end: 0,
+                            name: "a".to_owned(),
+                        },
+                        optional: false,
+                    },
+                    Parameter {
+                        identifier: Identifier {
+                            start: 0,
+                            end: 0,
+                            name: "b".to_owned(),
+                        },
+                        optional: true,
+                    },
+                ],
+                true,
+            ),
+            (
+                vec![
+                    Parameter {
+                        identifier: Identifier {
+                            start: 0,
+                            end: 0,
+                            name: "a".to_owned(),
+                        },
+                        optional: true,
+                    },
+                    Parameter {
+                        identifier: Identifier {
+                            start: 0,
+                            end: 0,
+                            name: "b".to_owned(),
+                        },
+                        optional: false,
+                    },
+                ],
+                false,
+            ),
+        ]
+        .into_iter()
+        .enumerate()
+        {
+            let actual = optional_after_required(&params);
+            assert_eq!(actual.is_ok(), expect_ok, "failed test {i}");
+        }
+    }
+
     #[test]
     fn test_parse_expand_array() {
         let code = "const myArray = [0..10]";
@@ -2801,4 +2913,5 @@ mod snapshot_tests {
     snapshot_test!(ar, r#"5 + "a""#);
     snapshot_test!(at, "line([0, l], %)");
     snapshot_test!(au, include_str!("../../../tests/executor/inputs/cylinder.kcl"));
+    snapshot_test!(av, "fn f = (angle?) => { return default(angle, 360) }");
 }

src/wasm-lib/kcl/src/parser/snapshots/kcl_lib__parser__parser_impl__snapshot_tests__ae.snap

@@ -29,10 +29,13 @@ expression: actual
             "end": 49,
             "params": [
               {
-                "type": "Identifier",
+                "identifier": {
-                "start": 12,
+                  "type": "Identifier",
-                "end": 17,
+                  "start": 12,
-                "name": "param"
+                  "end": 17,
+                  "name": "param"
+                },
+                "optional": false
               }
             ],
             "body": {

src/wasm-lib/kcl/src/parser/snapshots/kcl_lib__parser__parser_impl__snapshot_tests__av.snap

@@ -0,0 +1,97 @@
+---
+source: kcl/src/parser/parser_impl.rs
+expression: actual
+---
+{
+  "start": 0,
+  "end": 49,
+  "body": [
+    {
+      "type": "VariableDeclaration",
+      "type": "VariableDeclaration",
+      "start": 0,
+      "end": 49,
+      "declarations": [
+        {
+          "type": "VariableDeclarator",
+          "start": 3,
+          "end": 49,
+          "id": {
+            "type": "Identifier",
+            "start": 3,
+            "end": 4,
+            "name": "f"
+          },
+          "init": {
+            "type": "FunctionExpression",
+            "type": "FunctionExpression",
+            "start": 7,
+            "end": 49,
+            "params": [
+              {
+                "identifier": {
+                  "type": "Identifier",
+                  "start": 8,
+                  "end": 13,
+                  "name": "angle"
+                },
+                "optional": true
+              }
+            ],
+            "body": {
+              "start": 19,
+              "end": 49,
+              "body": [
+                {
+                  "type": "ReturnStatement",
+                  "type": "ReturnStatement",
+                  "start": 21,
+                  "end": 47,
+                  "argument": {
+                    "type": "CallExpression",
+                    "type": "CallExpression",
+                    "start": 28,
+                    "end": 47,
+                    "callee": {
+                      "type": "Identifier",
+                      "start": 28,
+                      "end": 35,
+                      "name": "default"
+                    },
+                    "arguments": [
+                      {
+                        "type": "Identifier",
+                        "type": "Identifier",
+                        "start": 36,
+                        "end": 41,
+                        "name": "angle"
+                      },
+                      {
+                        "type": "Literal",
+                        "type": "Literal",
+                        "start": 43,
+                        "end": 46,
+                        "value": 360,
+                        "raw": "360"
+                      }
+                    ],
+                    "optional": false
+                  }
+                }
+              ],
+              "nonCodeMeta": {
+                "nonCodeNodes": {},
+                "start": []
+              }
+            }
+          }
+        }
+      ],
+      "kind": "fn"
+    }
+  ],
+  "nonCodeMeta": {
+    "nonCodeNodes": {},
+    "start": []
+  }
+}

src/wasm-lib/kcl/src/token.rs

@@ -47,6 +47,8 @@ pub enum TokenType {
     Function,
     /// Unknown lexemes.
     Unknown,
+    /// The ? symbol, used for optional values.
+    QuestionMark,
 }
 
 /// Most KCL tokens correspond to LSP semantic tokens (but not all).
@@ -58,6 +60,7 @@ impl TryFrom<TokenType> for SemanticTokenType {
             TokenType::Word => Self::VARIABLE,
             TokenType::Keyword => Self::KEYWORD,
             TokenType::Operator => Self::OPERATOR,
+            TokenType::QuestionMark => Self::OPERATOR,
             TokenType::String => Self::STRING,
             TokenType::LineComment => Self::COMMENT,
             TokenType::BlockComment => Self::COMMENT,

src/wasm-lib/kcl/src/token/tokeniser.rs

@@ -21,6 +21,7 @@ pub fn token(i: &mut Located<&str>) -> PResult<Token> {
         '{' | '(' | '[' => brace_start,
         '}' | ')' | ']' => brace_end,
         ',' => comma,
+        '?' => question_mark,
         '0'..='9' => number,
         ':' => colon,
         '.' => alt((number, double_period, period)),
@@ -108,6 +109,11 @@ fn comma(i: &mut Located<&str>) -> PResult<Token> {
     Ok(Token::from_range(range, TokenType::Comma, value.to_string()))
 }
 
+fn question_mark(i: &mut Located<&str>) -> PResult<Token> {
+    let (value, range) = '?'.with_span().parse_next(i)?;
+    Ok(Token::from_range(range, TokenType::QuestionMark, value.to_string()))
+}
+
 fn colon(i: &mut Located<&str>) -> PResult<Token> {
     let (value, range) = ':'.with_span().parse_next(i)?;
     Ok(Token::from_range(range, TokenType::Colon, value.to_string()))