KCL parser: Allow comments in multi-line object expression (#3607)

Like my previous PR to array expressions (https://github.com/KittyCAD/modeling-app/pull/3539), but for object expressions. Closes https://github.com/KittyCAD/modeling-app/issues/1528

src/lang/modifyAst.ts

@@ -563,7 +563,7 @@ export function createArrayExpression(
     start: 0,
     end: 0,
     digest: null,
-    nonCodeMeta: { nonCodeNodes: {}, start: [], digest: null },
+    nonCodeMeta: nonCodeMetaEmpty(),
     elements,
   }
 }
@@ -577,7 +577,7 @@ export function createPipeExpression(
     end: 0,
     digest: null,
     body,
-    nonCodeMeta: { nonCodeNodes: {}, start: [], digest: null },
+    nonCodeMeta: nonCodeMetaEmpty(),
   }
 }
 
@@ -613,6 +613,7 @@ export function createObjectExpression(properties: {
     start: 0,
     end: 0,
     digest: null,
+    nonCodeMeta: nonCodeMetaEmpty(),
     properties: Object.entries(properties).map(([key, value]) => ({
       type: 'ObjectProperty',
       start: 0,
@@ -1065,3 +1066,7 @@ export async function deleteFromSelection(
 
   return new Error('Selection not recognised, could not delete')
 }
+
+const nonCodeMetaEmpty = () => {
+  return { nonCodeNodes: {}, start: [], digest: null }
+}

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

@@ -2466,11 +2466,13 @@ impl ArrayExpression {
 #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema, Bake)]
 #[databake(path = kcl_lib::ast::types)]
 #[ts(export)]
-#[serde(tag = "type")]
+#[serde(rename_all = "camelCase", tag = "type")]
 pub struct ObjectExpression {
     pub start: usize,
     pub end: usize,
     pub properties: Vec<ObjectProperty>,
+    #[serde(default, skip_serializing_if = "NonCodeMeta::is_empty")]
+    pub non_code_meta: NonCodeMeta,
 
     pub digest: Option<Digest>,
 }
@@ -2481,6 +2483,7 @@ impl ObjectExpression {
             start: 0,
             end: 0,
             properties,
+            non_code_meta: Default::default(),
             digest: None,
         }
     }
@@ -2514,6 +2517,14 @@ impl ObjectExpression {
     }
 
     fn recast(&self, options: &FormatOptions, indentation_level: usize, is_in_pipe: bool) -> String {
+        if self
+            .non_code_meta
+            .non_code_nodes
+            .values()
+            .any(|nc| nc.iter().any(|nc| nc.value.should_cause_array_newline()))
+        {
+            return self.recast_multi_line(options, indentation_level, is_in_pipe);
+        }
         let flat_recast = format!(
             "{{ {} }}",
             self.properties
@@ -2529,35 +2540,49 @@ impl ObjectExpression {
                 .join(", ")
         );
         let max_array_length = 40;
-        if flat_recast.len() > max_array_length {
+        let needs_multiple_lines = flat_recast.len() > max_array_length;
+        if !needs_multiple_lines {
+            return flat_recast;
+        }
+        self.recast_multi_line(options, indentation_level, is_in_pipe)
+    }
+
+    /// Recast, but always outputs the object with newlines between each property.
+    fn recast_multi_line(&self, options: &FormatOptions, indentation_level: usize, is_in_pipe: bool) -> String {
         let inner_indentation = if is_in_pipe {
             options.get_indentation_offset_pipe(indentation_level + 1)
         } else {
             options.get_indentation(indentation_level + 1)
         };
-            format!(
+        let num_items = self.properties.len() + self.non_code_meta.non_code_nodes_len();
-                "{{\n{}{}\n{}}}",
+        let mut props = self.properties.iter();
-                inner_indentation,
+        let format_items: Vec<_> = (0..num_items)
-                self.properties
+            .flat_map(|i| {
-                    .iter()
+                if let Some(noncode) = self.non_code_meta.non_code_nodes.get(&i) {
-                    .map(|prop| {
+                    noncode.iter().map(|nc| nc.format("")).collect::<Vec<_>>()
-                        format!(
+                } else {
-                            "{}: {}",
+                    let prop = props.next().unwrap();
+                    // Use a comma unless it's the last item
+                    let comma = if i == num_items - 1 { "" } else { ",\n" };
+                    let s = format!(
+                        "{}: {}{comma}",
                         prop.key.name,
-                            prop.value.recast(options, indentation_level + 1, is_in_pipe)
+                        prop.value.recast(options, indentation_level + 1, is_in_pipe).trim()
-                        )
+                    );
+                    vec![s]
+                }
             })
-                    .collect::<Vec<String>>()
+            .collect();
-                    .join(format!(",\n{}", inner_indentation).as_str()),
+        dbg!(&format_items);
-                if is_in_pipe {
+        let end_indent = if is_in_pipe {
             options.get_indentation_offset_pipe(indentation_level)
         } else {
             options.get_indentation(indentation_level)
-                },
+        };
+        format!(
+            "{{\n{inner_indentation}{}\n{end_indent}}}",
+            format_items.join(&inner_indentation),
         )
-        } else {
-            flat_recast
-        }
     }
 
     /// Returns a hover value that includes the given character position.
@@ -5897,6 +5922,66 @@ const thickness = sqrt(distance * p * FOS * 6 / (sigmaAllow * width))"#;
         }
     }
 
+    #[test]
+    fn recast_objects_no_comments() {
+        let input = r#"
+const sketch002 = startSketchOn({
+       plane: {
+    origin: { x: 1, y: 2, z: 3 },
+    x_axis: { x: 4, y: 5, z: 6 },
+    y_axis: { x: 7, y: 8, z: 9 },
+    z_axis: { x: 10, y: 11, z: 12 }
+       }
+  })
+"#;
+        let expected = r#"const sketch002 = startSketchOn({
+  plane: {
+    origin: { x: 1, y: 2, z: 3 },
+    x_axis: { x: 4, y: 5, z: 6 },
+    y_axis: { x: 7, y: 8, z: 9 },
+    z_axis: { x: 10, y: 11, z: 12 }
+  }
+})
+"#;
+        let tokens = crate::token::lexer(input).unwrap();
+        let p = crate::parser::Parser::new(tokens);
+        let ast = p.ast().unwrap();
+        let actual = ast.recast(&FormatOptions::new(), 0);
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn recast_objects_with_comments() {
+        use winnow::Parser;
+        for (i, (input, expected, reason)) in [(
+            "\
+{
+  a: 1,
+  // b: 2,
+  c: 3
+}",
+            "\
+{
+  a: 1,
+  // b: 2,
+  c: 3
+}",
+            "preserves comments",
+        )]
+        .into_iter()
+        .enumerate()
+        {
+            let tokens = crate::token::lexer(input).unwrap();
+            crate::parser::parser_impl::print_tokens(&tokens);
+            let expr = crate::parser::parser_impl::object.parse(&tokens).unwrap();
+            assert_eq!(
+                expr.recast(&FormatOptions::new(), 0, false),
+                expected,
+                "failed test {i}, which is testing that recasting {reason}"
+            );
+        }
+    }
+
     #[test]
     fn recast_array_with_comments() {
         use winnow::Parser;

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

@@ -586,22 +586,60 @@ fn object_property(i: TokenSlice) -> PResult<ObjectProperty> {
     })
 }
 
+/// Match something that separates properties of an object.
+fn property_separator(i: TokenSlice) -> PResult<()> {
+    alt((
+        // Normally you need a comma.
+        comma_sep,
+        // But, if the array is ending, no need for a comma.
+        peek(preceded(opt(whitespace), close_brace)).void(),
+    ))
+    .parse_next(i)
+}
+
 /// Parse a KCL object value.
-fn object(i: TokenSlice) -> PResult<ObjectExpression> {
+pub(crate) fn object(i: TokenSlice) -> PResult<ObjectExpression> {
     let start = open_brace(i)?.start;
     ignore_whitespace(i);
-    let properties = separated(0.., object_property, comma_sep)
+    let properties: Vec<_> = repeat(
+        0..,
+        alt((
+            terminated(non_code_node.map(NonCodeOr::NonCode), whitespace),
+            terminated(object_property, property_separator).map(NonCodeOr::Code),
+        )),
+    )
     .context(expected(
         "a comma-separated list of key-value pairs, e.g. 'height: 4, width: 3'",
     ))
     .parse_next(i)?;
+
+    // Sort the object's properties from the noncode nodes.
+    let (properties, non_code_nodes): (Vec<_>, HashMap<usize, _>) = properties.into_iter().enumerate().fold(
+        (Vec::new(), HashMap::new()),
+        |(mut properties, mut non_code_nodes), (i, e)| {
+            match e {
+                NonCodeOr::NonCode(x) => {
+                    non_code_nodes.insert(i, vec![x]);
+                }
+                NonCodeOr::Code(x) => {
+                    properties.push(x);
+                }
+            }
+            (properties, non_code_nodes)
+        },
+    );
     ignore_trailing_comma(i);
     ignore_whitespace(i);
     let end = close_brace(i)?.end;
+    let non_code_meta = NonCodeMeta {
+        non_code_nodes,
+        ..Default::default()
+    };
     Ok(ObjectExpression {
         start,
         end,
         properties,
+        non_code_meta,
         digest: None,
     })
 }
@@ -3056,12 +3094,6 @@ e
     }
 
     #[allow(unused)]
-    fn print_tokens(tokens: &[Token]) {
-        for (i, tok) in tokens.iter().enumerate() {
-            println!("{i:.2}: ({:?}):) '{}'", tok.token_type, tok.value.replace("\n", "\\n"));
-        }
-    }
-
     #[test]
     fn array_linesep_no_trailing_comma() {
         let program = r#"[
@@ -3259,6 +3291,7 @@ mod snapshot_tests {
             #[test]
             fn $func_name() {
                 let tokens = crate::token::lexer($test_kcl_program).unwrap();
+                print_tokens(&tokens);
                 let actual = match program.parse(&tokens) {
                     Ok(x) => x,
                     Err(e) => panic!("could not parse test: {e:?}"),
@@ -3404,4 +3437,28 @@ mod snapshot_tests {
             // B,
         ]"
     );
+    snapshot_test!(
+        ay,
+        "let props = {
+            a: 1,
+            // b: 2,
+            c: 3,
+        }"
+    );
+    snapshot_test!(
+        az,
+        "let props = {
+            a: 1,
+            // b: 2,
+            c: 3
+        }"
+    );
+}
+
+#[allow(unused)]
+#[cfg(test)]
+pub(crate) fn print_tokens(tokens: &[Token]) {
+    for (i, tok) in tokens.iter().enumerate() {
+        println!("{i:.2}: ({:?}):) '{}'", tok.token_type, tok.value.replace("\n", "\\n"));
+    }
 }

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

@@ -0,0 +1,111 @@
+---
+source: kcl/src/parser/parser_impl.rs
+expression: actual
+---
+{
+  "start": 0,
+  "end": 80,
+  "body": [
+    {
+      "type": "VariableDeclaration",
+      "type": "VariableDeclaration",
+      "start": 0,
+      "end": 80,
+      "declarations": [
+        {
+          "type": "VariableDeclarator",
+          "start": 4,
+          "end": 80,
+          "id": {
+            "type": "Identifier",
+            "start": 4,
+            "end": 9,
+            "name": "props",
+            "digest": null
+          },
+          "init": {
+            "type": "ObjectExpression",
+            "type": "ObjectExpression",
+            "start": 12,
+            "end": 80,
+            "properties": [
+              {
+                "type": "ObjectProperty",
+                "start": 26,
+                "end": 30,
+                "key": {
+                  "type": "Identifier",
+                  "start": 26,
+                  "end": 27,
+                  "name": "a",
+                  "digest": null
+                },
+                "value": {
+                  "type": "Literal",
+                  "type": "Literal",
+                  "start": 29,
+                  "end": 30,
+                  "value": 1,
+                  "raw": "1",
+                  "digest": null
+                },
+                "digest": null
+              },
+              {
+                "type": "ObjectProperty",
+                "start": 65,
+                "end": 69,
+                "key": {
+                  "type": "Identifier",
+                  "start": 65,
+                  "end": 66,
+                  "name": "c",
+                  "digest": null
+                },
+                "value": {
+                  "type": "Literal",
+                  "type": "Literal",
+                  "start": 68,
+                  "end": 69,
+                  "value": 3,
+                  "raw": "3",
+                  "digest": null
+                },
+                "digest": null
+              }
+            ],
+            "nonCodeMeta": {
+              "nonCodeNodes": {
+                "1": [
+                  {
+                    "type": "NonCodeNode",
+                    "start": 44,
+                    "end": 52,
+                    "value": {
+                      "type": "blockComment",
+                      "value": "b: 2,",
+                      "style": "line"
+                    },
+                    "digest": null
+                  }
+                ]
+              },
+              "start": [],
+              "digest": null
+            },
+            "digest": null
+          },
+          "digest": null
+        }
+      ],
+      "kind": "let",
+      "digest": null
+    }
+  ],
+  "nonCodeMeta": {
+    "nonCodeNodes": {},
+    "start": [],
+    "digest": null
+  },
+  "digest": null
+}

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

@@ -0,0 +1,111 @@
+---
+source: kcl/src/parser/parser_impl.rs
+expression: actual
+---
+{
+  "start": 0,
+  "end": 79,
+  "body": [
+    {
+      "type": "VariableDeclaration",
+      "type": "VariableDeclaration",
+      "start": 0,
+      "end": 79,
+      "declarations": [
+        {
+          "type": "VariableDeclarator",
+          "start": 4,
+          "end": 79,
+          "id": {
+            "type": "Identifier",
+            "start": 4,
+            "end": 9,
+            "name": "props",
+            "digest": null
+          },
+          "init": {
+            "type": "ObjectExpression",
+            "type": "ObjectExpression",
+            "start": 12,
+            "end": 79,
+            "properties": [
+              {
+                "type": "ObjectProperty",
+                "start": 26,
+                "end": 30,
+                "key": {
+                  "type": "Identifier",
+                  "start": 26,
+                  "end": 27,
+                  "name": "a",
+                  "digest": null
+                },
+                "value": {
+                  "type": "Literal",
+                  "type": "Literal",
+                  "start": 29,
+                  "end": 30,
+                  "value": 1,
+                  "raw": "1",
+                  "digest": null
+                },
+                "digest": null
+              },
+              {
+                "type": "ObjectProperty",
+                "start": 65,
+                "end": 69,
+                "key": {
+                  "type": "Identifier",
+                  "start": 65,
+                  "end": 66,
+                  "name": "c",
+                  "digest": null
+                },
+                "value": {
+                  "type": "Literal",
+                  "type": "Literal",
+                  "start": 68,
+                  "end": 69,
+                  "value": 3,
+                  "raw": "3",
+                  "digest": null
+                },
+                "digest": null
+              }
+            ],
+            "nonCodeMeta": {
+              "nonCodeNodes": {
+                "1": [
+                  {
+                    "type": "NonCodeNode",
+                    "start": 44,
+                    "end": 52,
+                    "value": {
+                      "type": "blockComment",
+                      "value": "b: 2,",
+                      "style": "line"
+                    },
+                    "digest": null
+                  }
+                ]
+              },
+              "start": [],
+              "digest": null
+            },
+            "digest": null
+          },
+          "digest": null
+        }
+      ],
+      "kind": "let",
+      "digest": null
+    }
+  ],
+  "nonCodeMeta": {
+    "nonCodeNodes": {},
+    "start": [],
+    "digest": null
+  },
+  "digest": null
+}