From 651181e62cf69746d77d6ce65dfd77101c9efc87 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Tue, 20 May 2025 18:13:17 +1200 Subject: [PATCH] Restrict subdirectory imports to main.kcl (#7094) Signed-off-by: Nick Cameron --- docs/kcl-lang/modules.md | 19 ++++++++-- rust/kcl-lib/src/parsing/ast/types/mod.rs | 35 ++++++++++++++++--- rust/kcl-lib/src/parsing/parser.rs | 20 +++++++++-- .../tests/nested_windows_main_kcl/ast.snap | 8 +---- .../tests/nested_windows_main_kcl/input.kcl | 2 +- .../nested_windows_main_kcl/unparsed.snap | 2 +- 6 files changed, 67 insertions(+), 19 deletions(-) diff --git a/docs/kcl-lang/modules.md b/docs/kcl-lang/modules.md index 0b9b53271..29322e043 100644 --- a/docs/kcl-lang/modules.md +++ b/docs/kcl-lang/modules.md @@ -27,9 +27,6 @@ import increment from "util.kcl" answer = increment(41) ``` -Imported files _must_ be in the same project so that units are uniform across -modules. This means that it must be in the same directory. - Import statements must be at the top-level of a file. It is not allowed to have an `import` statement inside a function or in the body of an if‑else. @@ -58,6 +55,9 @@ Imported symbols can be renamed for convenience or to avoid name collisions. import increment as inc, decrement as dec from "util.kcl" ``` +You can import files from the current directory or from subdirectories, but if importing from a +subdirectory you can only import `main.kcl`. + --- ## Functions vs `clone` @@ -229,6 +229,19 @@ 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. +The name of the file or subdirectory is used as the name of the variable within the importing program. +If you want to use a different name, you can do so by using the `as` keyword: + +```kcl,norun +import "cube.kcl" // Introduces a new variable called `cube`. +import "cube.kcl" as block // Introduces a new variable called `block`. +import "cube/main.kcl" // Introduces a new variable called `cube`. +import "cube/main.kcl" as block // Introduces a new variable called `block`. +``` + +If the filename includes hyphens (`-`) or starts with an underscore (`_`), then you must specify a +variable name. + --- ## Multiple instances of the same import diff --git a/rust/kcl-lib/src/parsing/ast/types/mod.rs b/rust/kcl-lib/src/parsing/ast/types/mod.rs index 7b66f548c..703b69208 100644 --- a/rust/kcl-lib/src/parsing/ast/types/mod.rs +++ b/rust/kcl-lib/src/parsing/ast/types/mod.rs @@ -1811,14 +1811,25 @@ impl ImportStatement { match &self.path { ImportPath::Kcl { filename: s } | ImportPath::Foreign { path: s } => { - let name = s.file_name().map(|f| f.to_string()); + let name = s.to_string_lossy(); + if name.ends_with("/main.kcl") || name.ends_with("\\main.kcl") { + let name = &name[..name.len() - 9]; + let start = name.rfind(['/', '\\']).map(|s| s + 1).unwrap_or(0); + return Some(name[start..].to_owned()); + } + + let name = s.file_name().map(|f| f.to_string())?; + if name.contains('\\') || name.contains('/') { + return None; + } + // Remove the extension if it exists. let extension = s.extension(); - if let Some(extension) = extension { - name.map(|n| n.trim_end_matches(extension).trim_end_matches('.').to_string()) + Some(if let Some(extension) = extension { + name.trim_end_matches(extension).trim_end_matches('.').to_string() } else { name - } + }) } ImportPath::Std { path } => path.last().cloned(), } @@ -4330,4 +4341,20 @@ startSketchOn(XY) "# ); } + + #[test] + fn module_name() { + #[track_caller] + fn assert_mod_name(stmt: &str, name: &str) { + let tokens = crate::parsing::token::lex(stmt, ModuleId::default()).unwrap(); + let stmt = crate::parsing::parser::import_stmt(&mut tokens.as_slice()).unwrap(); + assert_eq!(stmt.module_name().unwrap(), name); + } + + assert_mod_name("import 'foo.kcl'", "foo"); + assert_mod_name("import 'foo.kcl' as bar", "bar"); + assert_mod_name("import 'main.kcl'", "main"); + assert_mod_name("import 'foo/main.kcl'", "foo"); + assert_mod_name("import 'foo\\bar\\main.kcl'", "bar"); + } } diff --git a/rust/kcl-lib/src/parsing/parser.rs b/rust/kcl-lib/src/parsing/parser.rs index fd997e3af..0cca44a39 100644 --- a/rust/kcl-lib/src/parsing/parser.rs +++ b/rust/kcl-lib/src/parsing/parser.rs @@ -1729,7 +1729,7 @@ fn glob(i: &mut TokenSlice) -> PResult { .parse_next(i) } -fn import_stmt(i: &mut TokenSlice) -> PResult> { +pub(super) fn import_stmt(i: &mut TokenSlice) -> PResult> { let (visibility, visibility_token) = opt(terminated(item_visibility, whitespace)) .parse_next(i)? .map_or((ItemVisibility::Default, None), |pair| (pair.0, Some(pair.1))); @@ -1867,7 +1867,7 @@ fn validate_path_string(path_string: String, var_name: bool, path_range: SourceR return Err(ErrMode::Cut( CompilationError::fatal( path_range, - "import path may only contain alphanumeric characters, underscore, hyphen, and period. KCL files in other directories are not yet supported.", + "import path may only contain alphanumeric characters, `_`, `-`, `.`, `/`, and `\\`.", ) .into(), )); @@ -1894,6 +1894,15 @@ fn validate_path_string(path_string: String, var_name: bool, path_range: SourceR )); } + if (path_string.contains('/') || path_string.contains('\\')) + && !(path_string.ends_with("/main.kcl") || path_string.ends_with("\\main.kcl")) + { + return Err(ErrMode::Cut( + CompilationError::fatal(path_range, "import path to a subdirectory must only refer to main.kcl.") + .into(), + )); + } + ImportPath::Kcl { filename: TypedPath::new(&path_string), } @@ -4569,9 +4578,14 @@ e ); assert_err( r#"import cube from "C:\cube.kcl""#, - "import path may only contain alphanumeric characters, underscore, hyphen, and period. KCL files in other directories are not yet supported.", + "import path may only contain alphanumeric characters, `_`, `-`, `.`, `/`, and `\\`.", [17, 30], ); + assert_err( + r#"import cube from "cube/cube.kcl""#, + "import path to a subdirectory must only refer to main.kcl.", + [17, 32], + ); assert_err( r#"import * as foo from "dsfs""#, "as is not the 'from' keyword", diff --git a/rust/kcl-lib/tests/nested_windows_main_kcl/ast.snap b/rust/kcl-lib/tests/nested_windows_main_kcl/ast.snap index 15c73d126..84e19afbb 100644 --- a/rust/kcl-lib/tests/nested_windows_main_kcl/ast.snap +++ b/rust/kcl-lib/tests/nested_windows_main_kcl/ast.snap @@ -14,13 +14,7 @@ description: Result of parsing nested_windows_main_kcl.kcl }, "selector": { "type": "None", - "alias": { - "commentStart": 0, - "end": 0, - "name": "bar", - "start": 0, - "type": "Identifier" - } + "alias": null }, "start": 0, "type": "ImportStatement", diff --git a/rust/kcl-lib/tests/nested_windows_main_kcl/input.kcl b/rust/kcl-lib/tests/nested_windows_main_kcl/input.kcl index 553539e49..b4056cfb4 100644 --- a/rust/kcl-lib/tests/nested_windows_main_kcl/input.kcl +++ b/rust/kcl-lib/tests/nested_windows_main_kcl/input.kcl @@ -1,3 +1,3 @@ -import "nested\foo\bar\main.kcl" as bar +import "nested\foo\bar\main.kcl" bar diff --git a/rust/kcl-lib/tests/nested_windows_main_kcl/unparsed.snap b/rust/kcl-lib/tests/nested_windows_main_kcl/unparsed.snap index 3e84064eb..031230d1b 100644 --- a/rust/kcl-lib/tests/nested_windows_main_kcl/unparsed.snap +++ b/rust/kcl-lib/tests/nested_windows_main_kcl/unparsed.snap @@ -2,6 +2,6 @@ source: kcl-lib/src/simulation_tests.rs description: Result of unparsing nested_windows_main_kcl.kcl --- -import "nested/foo/bar/main.kcl" as bar +import "nested/foo/bar/main.kcl" bar