From 8d8304cf9108bcd6cb9b40a90a4bc92364000e92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sat, 13 May 2023 01:20:33 +0300 Subject: [PATCH] Allow recursive module dirs; Require mod.nu in dirs (#9185) --- crates/nu-parser/src/parse_keywords.rs | 84 +++++++++---------- crates/nu-protocol/src/parse_error.rs | 8 ++ tests/modules/mod.rs | 28 +++++++ tests/modules/samples/missing_mod_nu/test.nu | 0 tests/modules/samples/not_allowed/mod.nu | 0 tests/modules/samples/spam/bacon/beans/foo.nu | 1 + tests/modules/samples/spam/bacon/beans/mod.nu | 3 + tests/modules/samples/spam/bacon/foo.nu | 1 + tests/modules/samples/spam/bacon/mod.nu | 3 + 9 files changed, 85 insertions(+), 43 deletions(-) create mode 100644 tests/modules/samples/missing_mod_nu/test.nu create mode 100644 tests/modules/samples/not_allowed/mod.nu create mode 100644 tests/modules/samples/spam/bacon/beans/foo.nu create mode 100644 tests/modules/samples/spam/bacon/beans/mod.nu create mode 100644 tests/modules/samples/spam/bacon/foo.nu create mode 100644 tests/modules/samples/spam/bacon/mod.nu diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 35a3e42018..de158e0c0a 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -1825,14 +1825,22 @@ pub fn parse_module_file_or_dir( return None; }; - let mut file_paths = vec![]; + let mod_nu_path = module_path.join("mod.nu"); + + if !(mod_nu_path.exists() && mod_nu_path.is_file()) { + working_set.error(ParseError::ModuleMissingModNuFile(path_span)); + return None; + } + + let mut paths = vec![]; for entry in dir_contents.flatten() { let entry_path = entry.path(); - if entry_path.is_file() + if (entry_path.is_file() && entry_path.extension() == Some(OsStr::new("nu")) - && entry_path.file_stem() != Some(OsStr::new("mod")) + && entry_path.file_stem() != Some(OsStr::new("mod"))) + || (entry_path.is_dir() && entry_path.join("mod.nu").exists()) { if entry_path.file_stem() == Some(OsStr::new(&module_name)) { working_set.error(ParseError::InvalidModuleFileName( @@ -1843,64 +1851,54 @@ pub fn parse_module_file_or_dir( return None; } - file_paths.push(entry_path); + paths.push(entry_path); } } - file_paths.sort(); + paths.sort(); // working_set.enter_scope(); let mut submodules = vec![]; - for file_path in file_paths { - if let Some(submodule_id) = - parse_module_file(working_set, file_path, path_span, None) - { + for p in paths { + if let Some(submodule_id) = parse_module_file_or_dir( + working_set, + p.to_string_lossy().as_bytes(), + path_span, + None, + ) { let submodule_name = working_set.get_module(submodule_id).name(); submodules.push((submodule_name, submodule_id)); } } - let mod_nu_path = module_path.join("mod.nu"); - - if mod_nu_path.exists() && mod_nu_path.is_file() { - if let Some(module_id) = parse_module_file( - working_set, - mod_nu_path, - path_span, - name_override.or(Some(module_name)), - ) { - let mut module = working_set.get_module(module_id).clone(); - - for (submodule_name, submodule_id) in submodules { - module.add_submodule(submodule_name, submodule_id); - } - - let module_name = String::from_utf8_lossy(&module.name).to_string(); - - let module_comments = - if let Some(comments) = working_set.get_module_comments(module_id) { - comments.to_vec() - } else { - vec![] - }; - - let new_module_id = - working_set.add_module(&module_name, module, module_comments); - - Some(new_module_id) - } else { - None - } - } else { - let mut module = Module::new(module_name.as_bytes().to_vec()); + if let Some(module_id) = parse_module_file( + working_set, + mod_nu_path, + path_span, + name_override.or(Some(module_name)), + ) { + let mut module = working_set.get_module(module_id).clone(); for (submodule_name, submodule_id) in submodules { module.add_submodule(submodule_name, submodule_id); } - Some(working_set.add_module(&module_name, module, vec![])) + let module_name = String::from_utf8_lossy(&module.name).to_string(); + + let module_comments = + if let Some(comments) = working_set.get_module_comments(module_id) { + comments.to_vec() + } else { + vec![] + }; + + let new_module_id = working_set.add_module(&module_name, module, module_comments); + + Some(new_module_id) + } else { + None } } else { working_set.error(ParseError::ModuleNotFound(path_span)); diff --git a/crates/nu-protocol/src/parse_error.rs b/crates/nu-protocol/src/parse_error.rs index 3736d31ecc..6b7d36b574 100644 --- a/crates/nu-protocol/src/parse_error.rs +++ b/crates/nu-protocol/src/parse_error.rs @@ -193,6 +193,13 @@ pub enum ParseError { )] ModuleNotFound(#[label = "module not found"] Span), + #[error("Missing mod.nu file.")] + #[diagnostic( + code(nu::parser::module_missing_mod_nu_file), + help("When importing a directory as a Nushell module, it needs to contain a mod.nu file (can be empty). Alternatively, you can use .nu files in the directory as modules individually.") + )] + ModuleMissingModNuFile(#[label = "module directory is missing a mod.nu file"] Span), + #[error("Cyclical module import.")] #[diagnostic(code(nu::parser::cyclical_module_import), help("{0}"))] CyclicalModuleImport(String, #[label = "detected cyclical module import"] Span), @@ -484,6 +491,7 @@ impl ParseError { ParseError::AliasNotValid(s) => *s, ParseError::CommandDefNotValid(s) => *s, ParseError::ModuleNotFound(s) => *s, + ParseError::ModuleMissingModNuFile(s) => *s, ParseError::NamedAsModule(_, _, _, s) => *s, ParseError::ModuleDoubleMain(_, s) => *s, ParseError::InvalidModuleFileName(_, _, s) => *s, diff --git a/tests/modules/mod.rs b/tests/modules/mod.rs index 29bbebd1a5..217b0c755a 100644 --- a/tests/modules/mod.rs +++ b/tests/modules/mod.rs @@ -641,6 +641,27 @@ fn module_dir() { assert_eq!(actual.out, "spambaz"); } +#[test] +fn module_dir_deep() { + let import = "use samples/spam"; + + let inp = &[import, "spam bacon"]; + let actual_repl = nu!(cwd: "tests/modules", pipeline(&inp.join("; "))); + assert_eq!(actual_repl.out, "bacon"); + + let inp = &[import, "spam bacon foo"]; + let actual_repl = nu!(cwd: "tests/modules", pipeline(&inp.join("; "))); + assert_eq!(actual_repl.out, "bacon foo"); + + let inp = &[import, "spam bacon beans"]; + let actual_repl = nu!(cwd: "tests/modules", pipeline(&inp.join("; "))); + assert_eq!(actual_repl.out, "beans"); + + let inp = &[import, "spam bacon beans foo"]; + let actual_repl = nu!(cwd: "tests/modules", pipeline(&inp.join("; "))); + assert_eq!(actual_repl.out, "beans foo"); +} + #[test] fn module_dir_import_twice_no_panic() { let import = "use samples/spam"; @@ -656,6 +677,13 @@ fn not_allowed_submodule_file() { assert!(actual.err.contains("invalid_module_file_name")); } +#[test] +fn module_dir_missing_mod_nu() { + let inp = &["use samples/missing_mod_nu"]; + let actual = nu!(cwd: "tests/modules", pipeline(&inp.join("; "))); + assert!(actual.err.contains("module_missing_mod_nu_file")); +} + #[test] fn allowed_local_module() { let inp = &["module spam { module spam {} }"]; diff --git a/tests/modules/samples/missing_mod_nu/test.nu b/tests/modules/samples/missing_mod_nu/test.nu new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/modules/samples/not_allowed/mod.nu b/tests/modules/samples/not_allowed/mod.nu new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/modules/samples/spam/bacon/beans/foo.nu b/tests/modules/samples/spam/bacon/beans/foo.nu new file mode 100644 index 0000000000..714df54559 --- /dev/null +++ b/tests/modules/samples/spam/bacon/beans/foo.nu @@ -0,0 +1 @@ +export def main [] { 'beans foo' } diff --git a/tests/modules/samples/spam/bacon/beans/mod.nu b/tests/modules/samples/spam/bacon/beans/mod.nu new file mode 100644 index 0000000000..b9614067f0 --- /dev/null +++ b/tests/modules/samples/spam/bacon/beans/mod.nu @@ -0,0 +1,3 @@ +export def test [] { 'test' } + +export def main [] { 'beans' } diff --git a/tests/modules/samples/spam/bacon/foo.nu b/tests/modules/samples/spam/bacon/foo.nu new file mode 100644 index 0000000000..89bad910da --- /dev/null +++ b/tests/modules/samples/spam/bacon/foo.nu @@ -0,0 +1 @@ +export def main [] { 'bacon foo' } diff --git a/tests/modules/samples/spam/bacon/mod.nu b/tests/modules/samples/spam/bacon/mod.nu new file mode 100644 index 0000000000..f89c55ed32 --- /dev/null +++ b/tests/modules/samples/spam/bacon/mod.nu @@ -0,0 +1,3 @@ +export def test [] { 'test' } + +export def main [] { 'bacon' }