fix subdirs (opening kcl-samples from kcl-samples dir) (#5171)

* WIP: Add the KCL file path into the executor

* reuse the stuff that works with settings.project_directory

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fixes on both sides

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fixes on both sides

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* helper method

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fixes

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* update kcl-samples tests to not change dirs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* update kcl-samples tests to not change dirs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fmt

Signed-off-by: Jess Frazelle <github@jessfraz.com>

---------

Signed-off-by: Jess Frazelle <github@jessfraz.com>
Co-authored-by: Jonathan Tran <jonnytran@gmail.com>

src/lang/KclSingleton.ts

@@ -322,6 +322,7 @@ export class KclManager {
     await this.ensureWasmInit()
     const { logs, errors, execState, isInterrupted } = await executeAst({
       ast,
+      path: codeManager.currentFilePath || undefined,
       engineCommandManager: this.engineCommandManager,
     })
 

src/lang/codeManager.ts

@@ -80,6 +80,10 @@ export default class CodeManager {
     }))
   }
 
+  get currentFilePath(): string | null {
+    return this._currentFilePath
+  }
+
   updateCurrentFilePath(path: string) {
     this._currentFilePath = path
   }

src/lang/kclSamples.test.ts

@@ -52,27 +52,22 @@ afterAll(async () => {
   } catch (e) {}
 })
 
-afterEach(() => {
+describe('Test KCL Samples from public Github repository', () => {
-  process.chdir('..')
+  describe('when performing enginelessExecutor', () => {
-})
-
-// The tests have to be sequential because we need to change directories
-// to support `import` working properly.
-// @ts-expect-error
-describe.sequential('Test KCL Samples from public Github repository', () => {
-  // @ts-expect-error
-  describe.sequential('when performing enginelessExecutor', () => {
     manifest.forEach((file: KclSampleFile) => {
-      // @ts-expect-error
+      it(
-      it.sequential(
         `should execute ${file.title} (${file.file}) successfully`,
         async () => {
-          const [dirProject, fileKcl] =
+          const code = await fs.readFile(
-            file.pathFromProjectDirectoryToFirstFile.split('/')
+            file.pathFromProjectDirectoryToFirstFile,
-          process.chdir(dirProject)
+            'utf-8'
-          const code = await fs.readFile(fileKcl, 'utf-8')
+          )
           const ast = assertParse(code)
-          await enginelessExecutor(ast, programMemoryInit())
+          await enginelessExecutor(
+            ast,
+            programMemoryInit(),
+            file.pathFromProjectDirectoryToFirstFile
+          )
         },
         files.length * 1000
       )

src/lang/langHelpers.ts

@@ -46,12 +46,14 @@ export const toolTips: Array<ToolTip> = [
 
 export async function executeAst({
   ast,
+  path,
   engineCommandManager,
   // If you set programMemoryOverride we assume you mean mock mode. Since that
   // is the only way to go about it.
   programMemoryOverride,
 }: {
   ast: Node<Program>
+  path?: string
   engineCommandManager: EngineCommandManager
   programMemoryOverride?: ProgramMemory
   isInterrupted?: boolean
@@ -63,8 +65,8 @@ export async function executeAst({
 }> {
   try {
     const execState = await (programMemoryOverride
-      ? enginelessExecutor(ast, programMemoryOverride)
+      ? enginelessExecutor(ast, programMemoryOverride, path)
-      : executor(ast, engineCommandManager))
+      : executor(ast, engineCommandManager, path))
 
     await engineCommandManager.waitForAllCommands()
 

src/lang/std/fileSystemManager.ts

@@ -31,6 +31,9 @@ class FileSystemManager {
   }
 
   async join(dir: string, path: string): Promise<string> {
+    if (path.startsWith(dir)) {
+      path = path.slice(dir.length)
+    }
     return Promise.resolve(window.electron.path.join(dir, path))
   }
 

src/lang/wasm.ts

@@ -566,9 +566,19 @@ export function sketchFromKclValue(
   return result
 }
 
+/**
+ * Execute a KCL program.
+ * @param node The AST of the program to execute.
+ * @param path The full path of the file being executed.  Use `null` for
+ * expressions that don't have a file, like expressions in the command bar.
+ * @param programMemoryOverride If this is not `null`, this will be used as the
+ * initial program memory, and the execution will be engineless (AKA mock
+ * execution).
+ */
 export const executor = async (
   node: Node<Program>,
   engineCommandManager: EngineCommandManager,
+  path?: string,
   programMemoryOverride: ProgramMemory | Error | null = null
 ): Promise<ExecState> => {
   if (programMemoryOverride !== null && err(programMemoryOverride))
@@ -590,6 +600,7 @@ export const executor = async (
     }
     const execOutcome: RustExecOutcome = await execute(
       JSON.stringify(node),
+      path,
       JSON.stringify(programMemoryOverride?.toRaw() || null),
       JSON.stringify({ settings: jsAppSettings }),
       engineCommandManager,

src/lib/testHelpers.ts

@@ -80,7 +80,8 @@ class MockEngineCommandManager {
 
 export async function enginelessExecutor(
   ast: Node<Program>,
-  pmo: ProgramMemory | Error = ProgramMemory.empty()
+  pmo: ProgramMemory | Error = ProgramMemory.empty(),
+  path?: string
 ): Promise<ExecState> {
   if (pmo !== null && err(pmo)) return Promise.reject(pmo)
 
@@ -90,7 +91,7 @@ export async function enginelessExecutor(
   }) as any as EngineCommandManager
   // eslint-disable-next-line @typescript-eslint/no-floating-promises
   mockEngineCommandManager.startNewSession()
-  const execState = await executor(ast, mockEngineCommandManager, pmo)
+  const execState = await executor(ast, mockEngineCommandManager, path, pmo)
   await mockEngineCommandManager.waitForAllCommands()
   return execState
 }

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

@@ -131,7 +131,7 @@ pub struct ExecOutcome {
 impl ExecState {
     pub fn new(exec_settings: &ExecutorSettings) -> Self {
         ExecState {
-            global: GlobalState::new(),
+            global: GlobalState::new(exec_settings),
             mod_local: ModuleState::new(exec_settings),
         }
     }
@@ -142,7 +142,7 @@ impl ExecState {
         // This is for the front end to keep track of the ids.
         id_generator.next_id = 0;
 
-        let mut global = GlobalState::new();
+        let mut global = GlobalState::new(exec_settings);
         global.id_generator = id_generator;
 
         *self = ExecState {
@@ -204,7 +204,7 @@ impl ExecState {
 }
 
 impl GlobalState {
-    fn new() -> Self {
+    fn new(settings: &ExecutorSettings) -> Self {
         let mut global = GlobalState {
             id_generator: Default::default(),
             path_to_source_id: Default::default(),
@@ -215,9 +215,8 @@ impl GlobalState {
             artifact_graph: Default::default(),
         };
 
-        // TODO(#4434): Use the top-level file's path.
-        let root_path = PathBuf::new();
         let root_id = ModuleId::default();
+        let root_path = settings.current_file.clone().unwrap_or_default();
         global.module_infos.insert(
             root_id,
             ModuleInfo {
@@ -1752,6 +1751,9 @@ pub struct ExecutorSettings {
     /// The directory of the current project.  This is used for resolving import
     /// paths.  If None is given, the current working directory is used.
     pub project_directory: Option<PathBuf>,
+    /// This is the path to the current file being executed.
+    /// We use this for preventing cyclic imports.
+    pub current_file: Option<PathBuf>,
 }
 
 impl Default for ExecutorSettings {
@@ -1763,6 +1765,7 @@ impl Default for ExecutorSettings {
             show_grid: false,
             replay: None,
             project_directory: None,
+            current_file: None,
         }
     }
 }
@@ -1776,6 +1779,7 @@ impl From<crate::settings::types::Configuration> for ExecutorSettings {
             show_grid: config.settings.modeling.show_scale_grid,
             replay: None,
             project_directory: None,
+            current_file: None,
         }
     }
 }
@@ -1789,6 +1793,7 @@ impl From<crate::settings::types::project::ProjectConfiguration> for ExecutorSet
             show_grid: config.settings.modeling.show_scale_grid,
             replay: None,
             project_directory: None,
+            current_file: None,
         }
     }
 }
@@ -1802,6 +1807,25 @@ impl From<crate::settings::types::ModelingSettings> for ExecutorSettings {
             show_grid: modeling.show_scale_grid,
             replay: None,
             project_directory: None,
+            current_file: None,
+        }
+    }
+}
+
+impl ExecutorSettings {
+    /// Add the current file path to the executor settings.
+    pub fn with_current_file(&mut self, current_file: PathBuf) {
+        // We want the parent directory of the file.
+        if current_file.extension() == Some(std::ffi::OsStr::new("kcl")) {
+            self.current_file = Some(current_file.clone());
+            // Get the parent directory.
+            if let Some(parent) = current_file.parent() {
+                self.project_directory = Some(parent.to_path_buf());
+            } else {
+                self.project_directory = Some(std::path::PathBuf::from(""));
+            }
+        } else {
+            self.project_directory = Some(current_file.clone());
         }
     }
 }
@@ -2019,6 +2043,7 @@ impl ExecutorContext {
                 show_grid: false,
                 replay: None,
                 project_directory: None,
+                current_file: None,
             },
             None,
             engine_addr,
@@ -2578,7 +2603,20 @@ impl ExecutorContext {
         let info = exec_state.global.module_infos[&module_id].clone();
 
         match &info.repr {
-            ModuleRepr::Root => unreachable!(),
+            ModuleRepr::Root => Err(KclError::ImportCycle(KclErrorDetails {
+                message: format!(
+                    "circular import of modules is not allowed: {} -> {}",
+                    exec_state
+                        .mod_local
+                        .import_stack
+                        .iter()
+                        .map(|p| p.as_path().to_string_lossy())
+                        .collect::<Vec<_>>()
+                        .join(" -> "),
+                    info.path.display()
+                ),
+                source_ranges: vec![source_range],
+            })),
             ModuleRepr::Kcl(program) => {
                 let mut local_state = ModuleState {
                     import_stack: exec_state.mod_local.import_stack.clone(),

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

@@ -87,7 +87,7 @@ async fn execute(test_name: &str, render_to_png: bool) {
     let exec_res = crate::test_server::execute_and_snapshot_ast(
         ast.into(),
         crate::settings::types::UnitLength::Mm,
-        Some(Path::new("tests").join(test_name)),
+        Some(Path::new("tests").join(test_name).join("input.kcl").to_owned()),
     )
     .await;
     match exec_res {

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

@@ -21,9 +21,9 @@ pub struct RequestBody {
 pub async fn execute_and_snapshot(
     code: &str,
     units: UnitLength,
-    project_directory: Option<PathBuf>,
+    current_file: Option<PathBuf>,
 ) -> Result<image::DynamicImage, ExecError> {
-    let ctx = new_context(units, true, project_directory).await?;
+    let ctx = new_context(units, true, current_file).await?;
     let program = Program::parse_no_errs(code).map_err(KclErrorWithOutputs::no_outputs)?;
     let res = do_execute_and_snapshot(&ctx, program)
         .await
@@ -38,9 +38,9 @@ pub async fn execute_and_snapshot(
 pub async fn execute_and_snapshot_ast(
     ast: Program,
     units: UnitLength,
-    project_directory: Option<PathBuf>,
+    current_file: Option<PathBuf>,
 ) -> Result<(ExecState, image::DynamicImage), ExecErrorWithState> {
-    let ctx = new_context(units, true, project_directory).await?;
+    let ctx = new_context(units, true, current_file).await?;
     let res = do_execute_and_snapshot(&ctx, ast).await;
     ctx.close().await;
     res
@@ -49,9 +49,9 @@ pub async fn execute_and_snapshot_ast(
 pub async fn execute_and_snapshot_no_auth(
     code: &str,
     units: UnitLength,
-    project_directory: Option<PathBuf>,
+    current_file: Option<PathBuf>,
 ) -> Result<image::DynamicImage, ExecError> {
-    let ctx = new_context(units, false, project_directory).await?;
+    let ctx = new_context(units, false, current_file).await?;
     let program = Program::parse_no_errs(code).map_err(KclErrorWithOutputs::no_outputs)?;
     let res = do_execute_and_snapshot(&ctx, program)
         .await
@@ -88,7 +88,7 @@ async fn do_execute_and_snapshot(
 pub async fn new_context(
     units: UnitLength,
     with_auth: bool,
-    project_directory: Option<PathBuf>,
+    current_file: Option<PathBuf>,
 ) -> Result<ExecutorContext, ConnectionError> {
     let mut client = new_zoo_client(if with_auth { None } else { Some("bad_token".to_string()) }, None)
         .map_err(ConnectionError::CouldNotMakeClient)?;
@@ -99,18 +99,20 @@ pub async fn new_context(
         client.set_base_url("https://api.zoo.dev".to_string());
     }
 
-    let ctx = ExecutorContext::new(
+    let mut settings = ExecutorSettings {
-        &client,
+        units,
-        ExecutorSettings {
+        highlight_edges: true,
-            units,
+        enable_ssao: false,
-            highlight_edges: true,
+        show_grid: false,
-            enable_ssao: false,
+        replay: None,
-            show_grid: false,
+        project_directory: None,
-            replay: None,
+        current_file: None,
-            project_directory,
+    };
-        },
+    if let Some(current_file) = current_file {
-    )
+        settings.with_current_file(current_file);
-    .await
+    }
-    .map_err(ConnectionError::Establishing)?;
+    let ctx = ExecutorContext::new(&client, settings)
+        .await
+        .map_err(ConnectionError::Establishing)?;
     Ok(ctx)
 }

src/wasm-lib/kcl/tests/import_cycle1/execution_error.snap

@@ -1,12 +1,13 @@
 ---
 source: kcl/src/simulation_tests.rs
 description: Error from executing import_cycle1.kcl
+snapshot_kind: text
 ---
 KCL ImportCycle error
 
   × import cycle: circular import of modules is not allowed: tests/
   │ import_cycle1/import_cycle2.kcl -> tests/import_cycle1/import_cycle3.kcl
-  │ -> tests/import_cycle1/input.kcl -> tests/import_cycle1/import_cycle2.kcl
+  │ -> tests/import_cycle1/input.kcl
    ╭─[1:1]
  1 │ import two from "import_cycle2.kcl"
    · ───────────────────────────────────

src/wasm-lib/src/wasm.rs

@@ -58,6 +58,7 @@ pub async fn clear_scene_and_bust_cache(
 #[wasm_bindgen]
 pub async fn execute(
     program_ast_json: &str,
+    path: Option<String>,
     program_memory_override_str: &str,
     settings: &str,
     engine_manager: kcl_lib::wasm_engine::EngineCommandManager,
@@ -73,11 +74,16 @@ pub async fn execute(
     // You cannot override the memory in non-mock mode.
     let is_mock = program_memory_override.is_some();
 
-    let settings: kcl_lib::Configuration = serde_json::from_str(settings).map_err(|e| e.to_string())?;
+    let config: kcl_lib::Configuration = serde_json::from_str(settings).map_err(|e| e.to_string())?;
+    let mut settings: kcl_lib::ExecutorSettings = config.into();
+    if let Some(path) = path {
+        settings.with_current_file(std::path::PathBuf::from(path));
+    }
+
     let ctx = if is_mock {
-        kcl_lib::ExecutorContext::new_mock(fs_manager, settings.into()).await?
+        kcl_lib::ExecutorContext::new_mock(fs_manager, settings).await?
     } else {
-        kcl_lib::ExecutorContext::new(engine_manager, fs_manager, settings.into()).await?
+        kcl_lib::ExecutorContext::new(engine_manager, fs_manager, settings).await?
     };
 
     let mut exec_state = ExecState::new(&ctx.settings);