Change so that var definitions can be a module's return value (#6504)

* Change so that var definitions can be a module's return value

* Change car wheel assembly to use the new return mechanism

* Add sim test

* Update output

* Update module docs

* Add safety check to only work with modules

* Fix to use updated keyword args

docs/kcl/modules.md

@@ -191,10 +191,11 @@ In `cube.kcl`, you cannot have multiple objects. It has to be a single part. If
 you have multiple objects, you will get an error. This is because the module is
 expected to return a single object that can be used as a variable.
 
-You also cannot assign that object to a variable. This is because the module is
+The last expression or variable definition becomes the module's return value.
-expected to return a single object that can be used as a variable.
+The module is expected to return a single object that can be used as a variable
+by whatever imports it.
 
-So for example, this is not allowed:
+So for example, this is allowed:
 
 ```norun
 ... a bunch of code to create cube and cube2 ...
@@ -202,7 +203,7 @@ So for example, this is not allowed:
 myUnion = union([cube, cube2])
 ```
 
-What you need to do instead is:
+You can also do this:
 
 ```norun
 ... a bunch of code to create cube and cube2 ...
@@ -210,7 +211,7 @@ What you need to do instead is:
 union([cube, cube2])
 ```
 
-That way the last line will return the union of the two objects.
+Either way, the last line will return the union of the two objects.
 
 Or what you could do instead is:
 
@@ -221,9 +222,12 @@ myUnion = union([cube, cube2])
 myUnion
 ```
 
-This will return the union of the two objects, but it will not be assigned to a
+This will assign the union of the two objects to a variable, and then return it
-variable. This is because the module is expected to return a single object that
+on the last statement. It's simply another way of doing the same thing.
-can be used as a variable.
+
+The final statement is what's important because it's the return value of the
+entire module. The module is expected to return a single object that can be used
+as a variable by the file that imports it.
 
 ---
 

public/kcl-samples/car-wheel-assembly/car-rotor.kcl

@@ -85,5 +85,5 @@ secondRotorSlottedSketch = startSketchOn(secondRotor, face = END)
        rotateDuplicates = true,
      )
 
-extrude(secondRotorSlottedSketch, length = -rotorSinglePlateThickness / 2)
+carRotor = extrude(secondRotorSlottedSketch, length = -rotorSinglePlateThickness / 2)
   |> appearance(color = "#dbcd70", roughness = 90, metalness = 90)

public/kcl-samples/car-wheel-assembly/car-tire.kcl

@@ -41,5 +41,5 @@ tireSketch = startSketchOn(XY)
   |> close()
 
 // Revolve the sketch to create the tire
-revolve(tireSketch, axis = Y)
+carTire = revolve(tireSketch, axis = Y)
   |> appearance(color = "#0f0f0f", roughness = 80)

public/kcl-samples/car-wheel-assembly/lug-nut.kcl

@@ -35,4 +35,4 @@ fn lug(plane, length, diameter) {
   return lugSketch
 }
 
-lug(customPlane, lugLength, lugDiameter)
+lugNut = lug(customPlane, lugLength, lugDiameter)

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

@@ -278,7 +278,7 @@ impl ExecutorContext {
 
                     let annotations = &variable_declaration.outer_attrs;
 
-                    let memory_item = self
+                    let value = self
                         .execute_expr(
                             &variable_declaration.declaration.init,
                             exec_state,
@@ -289,13 +289,14 @@ impl ExecutorContext {
                         .await?;
                     exec_state
                         .mut_stack()
-                        .add(var_name.clone(), memory_item, source_range)?;
+                        .add(var_name.clone(), value.clone(), source_range)?;
 
                     // Track exports.
                     if let ItemVisibility::Export = variable_declaration.visibility {
                         exec_state.mod_local.module_exports.push(var_name);
                     }
-                    last_expr = None;
+                    // Variable declaration can be the return value of a module.
+                    last_expr = matches!(body_type, BodyType::Root).then_some(value);
                 }
                 BodyItem::TypeDeclaration(ty) => {
                     let metadata = Metadata::from(&**ty);

rust/kcl-lib/src/simulation_tests.rs

@@ -2240,6 +2240,28 @@ mod multi_transform {
     }
 }
 
+mod module_return_using_var {
+    const TEST_NAME: &str = "module_return_using_var";
+
+    /// Test parsing KCL.
+    #[test]
+    fn parse() {
+        super::parse(TEST_NAME)
+    }
+
+    /// Test that parsing and unparsing KCL produces the original KCL input.
+    #[tokio::test(flavor = "multi_thread")]
+    async fn unparse() {
+        super::unparse(TEST_NAME).await
+    }
+
+    /// Test that KCL is executed correctly.
+    #[tokio::test(flavor = "multi_thread")]
+    async fn kcl_test_execute() {
+        super::execute(TEST_NAME, true).await
+    }
+}
+
 mod import_transform {
     const TEST_NAME: &str = "import_transform";
 

rust/kcl-lib/tests/kcl_samples/car-wheel-assembly/artifact_graph_flowchart.snap.md

@@ -276,7 +276,7 @@ flowchart LR
   103["Sweep Extrusion<br>[2763, 2831, 7]"]
   104["Sweep Extrusion<br>[2763, 2831, 7]"]
   105["Sweep Extrusion<br>[2763, 2831, 7]"]
-  113["Sweep Extrusion<br>[3242, 3316, 7]"]
+  113["Sweep Extrusion<br>[3253, 3327, 7]"]
   114[Wall]
   115[Wall]
   116[Wall]
@@ -290,10 +290,10 @@ flowchart LR
   124["SweepEdge Adjacent"]
   125["SweepEdge Opposite"]
   126["SweepEdge Adjacent"]
-  127["Sweep Extrusion<br>[3242, 3316, 7]"]
+  127["Sweep Extrusion<br>[3253, 3327, 7]"]
-  128["Sweep Extrusion<br>[3242, 3316, 7]"]
+  128["Sweep Extrusion<br>[3253, 3327, 7]"]
-  129["Sweep Extrusion<br>[3242, 3316, 7]"]
+  129["Sweep Extrusion<br>[3253, 3327, 7]"]
-  130["Sweep Extrusion<br>[3242, 3316, 7]"]
+  130["Sweep Extrusion<br>[3253, 3327, 7]"]
   131["Plane<br>[331, 348, 6]"]
   138["Sweep Extrusion<br>[487, 520, 6]"]
   139[Wall]
@@ -500,7 +500,7 @@ flowchart LR
   436["SweepEdge Opposite"]
   437["SweepEdge Adjacent"]
   438["Plane<br>[464, 481, 10]"]
-  455["Sweep Revolve<br>[1474, 1503, 10]"]
+  455["Sweep Revolve<br>[1484, 1513, 10]"]
   456[Wall]
   457[Wall]
   458[Wall]

rust/kcl-lib/tests/module_return_using_var/artifact_commands.snap

@@ -0,0 +1,331 @@
+---
+source: kcl-lib/src/simulation_tests.rs
+description: Artifact commands module_return_using_var.kcl
+---
+[
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "edge_lines_visible",
+      "hidden": false
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "object_visible",
+      "object_id": "[uuid]",
+      "hidden": true
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "object_visible",
+      "object_id": "[uuid]",
+      "hidden": true
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "set_object_transform",
+      "object_id": "[uuid]",
+      "transforms": [
+        {
+          "translate": {
+            "property": {
+              "x": 5.0,
+              "y": 0.0,
+              "z": 0.0
+            },
+            "set": false,
+            "is_local": true
+          },
+          "rotate_rpy": null,
+          "rotate_angle_axis": null,
+          "scale": null
+        }
+      ]
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "make_plane",
+      "origin": {
+        "x": 0.0,
+        "y": 0.0,
+        "z": 0.0
+      },
+      "x_axis": {
+        "x": 1.0,
+        "y": 0.0,
+        "z": 0.0
+      },
+      "y_axis": {
+        "x": 0.0,
+        "y": 1.0,
+        "z": 0.0
+      },
+      "size": 60.0,
+      "clobber": false,
+      "hide": true
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "enable_sketch_mode",
+      "entity_id": "[uuid]",
+      "ortho": false,
+      "animated": false,
+      "adjust_camera": false,
+      "planar_normal": {
+        "x": 0.0,
+        "y": 0.0,
+        "z": 1.0
+      }
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "start_path"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "move_path_pen",
+      "path": "[uuid]",
+      "to": {
+        "x": 0.0,
+        "y": 0.0,
+        "z": 0.0
+      }
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "sketch_mode_disable"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "extend_path",
+      "path": "[uuid]",
+      "segment": {
+        "type": "line",
+        "end": {
+          "x": 1.0,
+          "y": 0.0,
+          "z": 0.0
+        },
+        "relative": true
+      }
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "extend_path",
+      "path": "[uuid]",
+      "segment": {
+        "type": "line",
+        "end": {
+          "x": 0.0,
+          "y": 1.0,
+          "z": 0.0
+        },
+        "relative": true
+      }
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "extend_path",
+      "path": "[uuid]",
+      "segment": {
+        "type": "line",
+        "end": {
+          "x": -1.0,
+          "y": 0.0,
+          "z": 0.0
+        },
+        "relative": true
+      }
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "extend_path",
+      "path": "[uuid]",
+      "segment": {
+        "type": "line",
+        "end": {
+          "x": 0.0,
+          "y": -1.0,
+          "z": 0.0
+        },
+        "relative": true
+      }
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "close_path",
+      "path_id": "[uuid]"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "enable_sketch_mode",
+      "entity_id": "[uuid]",
+      "ortho": false,
+      "animated": false,
+      "adjust_camera": false,
+      "planar_normal": {
+        "x": 0.0,
+        "y": 0.0,
+        "z": 1.0
+      }
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "extrude",
+      "target": "[uuid]",
+      "distance": 1.0,
+      "faces": null,
+      "opposite": "None"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "sketch_mode_disable"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "object_bring_to_front",
+      "object_id": "[uuid]"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "solid3d_get_extrusion_face_info",
+      "object_id": "[uuid]",
+      "edge_id": "[uuid]"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "solid3d_get_opposite_edge",
+      "object_id": "[uuid]",
+      "edge_id": "[uuid]",
+      "face_id": "[uuid]"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "solid3d_get_next_adjacent_edge",
+      "object_id": "[uuid]",
+      "edge_id": "[uuid]",
+      "face_id": "[uuid]"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "solid3d_get_opposite_edge",
+      "object_id": "[uuid]",
+      "edge_id": "[uuid]",
+      "face_id": "[uuid]"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "solid3d_get_next_adjacent_edge",
+      "object_id": "[uuid]",
+      "edge_id": "[uuid]",
+      "face_id": "[uuid]"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "solid3d_get_opposite_edge",
+      "object_id": "[uuid]",
+      "edge_id": "[uuid]",
+      "face_id": "[uuid]"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "solid3d_get_next_adjacent_edge",
+      "object_id": "[uuid]",
+      "edge_id": "[uuid]",
+      "face_id": "[uuid]"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "solid3d_get_opposite_edge",
+      "object_id": "[uuid]",
+      "edge_id": "[uuid]",
+      "face_id": "[uuid]"
+    }
+  },
+  {
+    "cmdId": "[uuid]",
+    "range": [],
+    "command": {
+      "type": "solid3d_get_next_adjacent_edge",
+      "object_id": "[uuid]",
+      "edge_id": "[uuid]",
+      "face_id": "[uuid]"
+    }
+  }
+]

rust/kcl-lib/tests/module_return_using_var/artifact_graph_flowchart.snap

@@ -0,0 +1,6 @@
+---
+source: kcl-lib/src/simulation_tests.rs
+description: Artifact graph flowchart module_return_using_var.kcl
+extension: md
+snapshot_kind: binary
+---

rust/kcl-lib/tests/module_return_using_var/artifact_graph_flowchart.snap.md

@@ -0,0 +1,62 @@
+```mermaid
+flowchart LR
+  subgraph path2 [Path]
+    2["Path<br>[80, 105, 6]"]
+    3["Segment<br>[111, 128, 6]"]
+    4["Segment<br>[134, 151, 6]"]
+    5["Segment<br>[157, 175, 6]"]
+    6["Segment<br>[181, 199, 6]"]
+    7["Segment<br>[205, 213, 6]"]
+    8[Solid2d]
+  end
+  1["Plane<br>[57, 74, 6]"]
+  9["Sweep Extrusion<br>[219, 238, 6]"]
+  10[Wall]
+  11[Wall]
+  12[Wall]
+  13[Wall]
+  14["Cap Start"]
+  15["Cap End"]
+  16["SweepEdge Opposite"]
+  17["SweepEdge Adjacent"]
+  18["SweepEdge Opposite"]
+  19["SweepEdge Adjacent"]
+  20["SweepEdge Opposite"]
+  21["SweepEdge Adjacent"]
+  22["SweepEdge Opposite"]
+  23["SweepEdge Adjacent"]
+  1 --- 2
+  2 --- 3
+  2 --- 4
+  2 --- 5
+  2 --- 6
+  2 --- 7
+  2 ---- 9
+  2 --- 8
+  3 --- 10
+  3 --- 16
+  3 --- 17
+  4 --- 11
+  4 --- 18
+  4 --- 19
+  5 --- 12
+  5 --- 20
+  5 --- 21
+  6 --- 13
+  6 --- 22
+  6 --- 23
+  9 --- 10
+  9 --- 11
+  9 --- 12
+  9 --- 13
+  9 --- 14
+  9 --- 15
+  9 --- 16
+  9 --- 17
+  9 --- 18
+  9 --- 19
+  9 --- 20
+  9 --- 21
+  9 --- 22
+  9 --- 23
+```

rust/kcl-lib/tests/module_return_using_var/ast.snap

@@ -0,0 +1,112 @@
+---
+source: kcl-lib/src/simulation_tests.rs
+description: Result of parsing module_return_using_var.kcl
+---
+{
+  "Ok": {
+    "body": [
+      {
+        "commentStart": 0,
+        "end": 0,
+        "path": {
+          "type": "Kcl",
+          "filename": "cube.kcl"
+        },
+        "selector": {
+          "type": "None",
+          "alias": null
+        },
+        "start": 0,
+        "type": "ImportStatement",
+        "type": "ImportStatement"
+      },
+      {
+        "commentStart": 0,
+        "end": 0,
+        "expression": {
+          "body": [
+            {
+              "abs_path": false,
+              "commentStart": 0,
+              "end": 0,
+              "name": {
+                "commentStart": 0,
+                "end": 0,
+                "name": "cube",
+                "start": 0,
+                "type": "Identifier"
+              },
+              "path": [],
+              "start": 0,
+              "type": "Name",
+              "type": "Name"
+            },
+            {
+              "arguments": [
+                {
+                  "type": "LabeledArg",
+                  "label": {
+                    "commentStart": 0,
+                    "end": 0,
+                    "name": "x",
+                    "start": 0,
+                    "type": "Identifier"
+                  },
+                  "arg": {
+                    "commentStart": 0,
+                    "end": 0,
+                    "raw": "5",
+                    "start": 0,
+                    "type": "Literal",
+                    "type": "Literal",
+                    "value": {
+                      "value": 5.0,
+                      "suffix": "None"
+                    }
+                  }
+                }
+              ],
+              "callee": {
+                "abs_path": false,
+                "commentStart": 0,
+                "end": 0,
+                "name": {
+                  "commentStart": 0,
+                  "end": 0,
+                  "name": "translate",
+                  "start": 0,
+                  "type": "Identifier"
+                },
+                "path": [],
+                "start": 0,
+                "type": "Name"
+              },
+              "commentStart": 0,
+              "end": 0,
+              "start": 0,
+              "type": "CallExpressionKw",
+              "type": "CallExpressionKw",
+              "unlabeled": null
+            }
+          ],
+          "commentStart": 0,
+          "end": 0,
+          "start": 0,
+          "type": "PipeExpression",
+          "type": "PipeExpression"
+        },
+        "preComments": [
+          "",
+          "",
+          "// Use the module's return value."
+        ],
+        "start": 0,
+        "type": "ExpressionStatement",
+        "type": "ExpressionStatement"
+      }
+    ],
+    "commentStart": 0,
+    "end": 0,
+    "start": 0
+  }
+}

rust/kcl-lib/tests/module_return_using_var/cube.kcl

@@ -0,0 +1,9 @@
+// The last statement is a variable definition.
+myCube = startSketchOn(XY)
+  |> startProfile(at = [0, 0])
+  |> xLine(length = 1)
+  |> yLine(length = 1)
+  |> xLine(length = -1)
+  |> yLine(length = -1)
+  |> close(%)
+  |> extrude(length = 1)

rust/kcl-lib/tests/module_return_using_var/input.kcl

@@ -0,0 +1,5 @@
+import 'cube.kcl'
+
+// Use the module's return value.
+cube
+  |> translate(x = 5)

rust/kcl-lib/tests/module_return_using_var/ops.snap

@@ -0,0 +1,65 @@
+---
+source: kcl-lib/src/simulation_tests.rs
+description: Operations executed module_return_using_var.kcl
+---
+[
+  {
+    "type": "GroupBegin",
+    "group": {
+      "type": "ModuleInstance",
+      "name": "cube",
+      "moduleId": 6
+    },
+    "sourceRange": []
+  },
+  {
+    "labeledArgs": {
+      "planeOrSolid": {
+        "value": {
+          "type": "Plane",
+          "artifact_id": "[uuid]"
+        },
+        "sourceRange": []
+      }
+    },
+    "name": "startSketchOn",
+    "sourceRange": [],
+    "type": "StdLibCall",
+    "unlabeledArg": null
+  },
+  {
+    "labeledArgs": {
+      "length": {
+        "value": {
+          "type": "Number",
+          "value": 1.0,
+          "ty": {
+            "type": "Default",
+            "len": {
+              "type": "Mm"
+            },
+            "angle": {
+              "type": "Degrees"
+            }
+          }
+        },
+        "sourceRange": []
+      }
+    },
+    "name": "extrude",
+    "sourceRange": [],
+    "type": "StdLibCall",
+    "unlabeledArg": {
+      "value": {
+        "type": "Sketch",
+        "value": {
+          "artifactId": "[uuid]"
+        }
+      },
+      "sourceRange": []
+    }
+  },
+  {
+    "type": "GroupEnd"
+  }
+]

rust/kcl-lib/tests/module_return_using_var/program_memory.snap

@@ -0,0 +1,10 @@
+---
+source: kcl-lib/src/simulation_tests.rs
+description: Variables in memory after executing module_return_using_var.kcl
+---
+{
+  "cube": {
+    "type": "Module",
+    "value": 6
+  }
+}

rust/kcl-lib/tests/module_return_using_var/unparsed.snap

@@ -0,0 +1,9 @@
+---
+source: kcl-lib/src/simulation_tests.rs
+description: Result of unparsing module_return_using_var.kcl
+---
+import "cube.kcl"
+
+// Use the module's return value.
+cube
+  |> translate(x = 5)

rust/kcl-lib/tests/module_return_using_var/unparsed@cube.kcl.snap

@@ -0,0 +1,13 @@
+---
+source: kcl-lib/src/simulation_tests.rs
+description: Result of unparsing tests/module_return_using_var/cube.kcl
+---
+// The last statement is a variable definition.
+myCube = startSketchOn(XY)
+  |> startProfile(at = [0, 0])
+  |> xLine(length = 1)
+  |> yLine(length = 1)
+  |> xLine(length = -1)
+  |> yLine(length = -1)
+  |> close(%)
+  |> extrude(length = 1)