From 156232fe082c684ccc3eb036edf38bdd6afa86c7 Mon Sep 17 00:00:00 2001 From: Antoine Stevan <44101798+amtoine@users.noreply.github.com> Date: Fri, 15 Dec 2023 12:37:55 +0100 Subject: [PATCH] disable directory submodule auto export (#11157) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit should - close https://github.com/nushell/nushell/issues/11133 # Description to allow more freedom when writing complex modules, we are disabling the auto-export of director modules. the change was as simple as removing the crawling of files and modules next to any `mod.nu` and update the standard library. # User-Facing Changes users will have to explicitely use `export module ` to define submodules and `export use ` to re-export definitions, e.g. ```nushell # my-module/mod.nu export module foo.nu # export a submodule export use bar.nu bar-1 # re-export an internal command export def top [] { print "`top` from `mod.nu`" } ``` ```nushell # my-module/foo.nu export def "foo-1" [] { print "`foo-1` from `lib/foo.nu`" } export def "foo-2" [] { print "`foo-2` from `lib/foo.nu`" } ``` ```nushell # my-module/bar.nu export def "bar-1" [] { print "`bar-1` from `lib/bar.nu`" } ``` # Tests + Formatting i had to add `export module` calls in the `tests/modules/samples/spam` directory module and allow the `not_allowed` module to not give an error, it is just empty, which is fine. # After Submitting - mention in the release note - update the following repos ``` #┬─────name─────┬version┬─type─┬─────────repo───────── 0│nu-git-manager│0.4.0 │module│amtoine/nu-git-manager 1│nu-scripts │0.1.0 │module│amtoine/scripts 2│nu-zellij │0.1.0 │module│amtoine/zellij-layouts 3│nu-scripts │0.1.0 │module│nushell/nu_scripts 4│nupm │0.1.0 │module│nushell/nupm ─┴──────────────┴───────┴──────┴────────────────────── ``` --- crates/nu-parser/src/parse_keywords.rs | 46 +------------------ crates/nu-std/std/mod.nu | 12 ++++- tests/modules/mod.rs | 7 --- tests/modules/samples/not_allowed/mod.nu | 0 .../samples/not_allowed/not_allowed.nu | 0 tests/modules/samples/spam/bacon/beans/mod.nu | 2 + tests/modules/samples/spam/bacon/mod.nu | 3 ++ tests/modules/samples/spam/mod.nu | 3 ++ 8 files changed, 21 insertions(+), 52 deletions(-) delete mode 100644 tests/modules/samples/not_allowed/mod.nu delete mode 100644 tests/modules/samples/not_allowed/not_allowed.nu diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 4e84740986..1c7eee261a 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -17,7 +17,6 @@ use nu_protocol::{ ResolvedImportPattern, Span, Spanned, SyntaxShape, Type, VarId, }; use std::collections::{HashMap, HashSet}; -use std::ffi::OsStr; use std::path::{Path, PathBuf}; pub const LIB_DIRS_VAR: &str = "NU_LIB_DIRS"; @@ -2011,7 +2010,7 @@ pub fn parse_module_file_or_dir( }; if module_path.is_dir() { - let Some(dir_contents) = module_path.read_dir() else { + if module_path.read_dir().is_none() { working_set.error(ParseError::ModuleNotFound(path_span)); return None; }; @@ -2033,54 +2032,13 @@ pub fn parse_module_file_or_dir( return None; } - let mut paths = vec![]; - - for entry_path in dir_contents { - if (entry_path.is_file() - && entry_path.extension() == Some(OsStr::new("nu")) - && entry_path.file_stem() != Some(OsStr::new("mod"))) - || (entry_path.is_dir() && entry_path.clone().join("mod.nu").exists()) - { - if entry_path.file_stem() == Some(OsStr::new(&module_name)) { - working_set.error(ParseError::InvalidModuleFileName( - module_path.path().to_string_lossy().to_string(), - module_name, - path_span, - )); - return None; - } - - paths.push(entry_path); - } - } - - paths.sort(); - - let mut submodules = vec![]; - - for p in paths { - if let Some(submodule_id) = parse_module_file_or_dir( - working_set, - p.path().to_string_lossy().as_bytes(), - path_span, - None, - ) { - let submodule_name = working_set.get_module(submodule_id).name(); - submodules.push((submodule_name, submodule_id)); - } - } - 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 = working_set.get_module(module_id).clone(); let module_name = String::from_utf8_lossy(&module.name).to_string(); diff --git a/crates/nu-std/std/mod.nu b/crates/nu-std/std/mod.nu index 2c6a24092c..581c728ae4 100644 --- a/crates/nu-std/std/mod.nu +++ b/crates/nu-std/std/mod.nu @@ -1,12 +1,22 @@ # std.nu, `used` to load all standard library components +export module assert.nu +export module dirs.nu +export module dt.nu +export module formats.nu +export module help.nu +export module input.nu +export module iter.nu +export module log.nu +export module math.nu +export module testing.nu +export module xml.nu export-env { use dirs.nu [] use log.nu [] } use dt.nu [datetime-diff, pretty-print-duration] -use log.nu # Add the given paths to the PATH. # diff --git a/tests/modules/mod.rs b/tests/modules/mod.rs index 55a080303a..93fa3147e6 100644 --- a/tests/modules/mod.rs +++ b/tests/modules/mod.rs @@ -670,13 +670,6 @@ fn module_dir_import_twice_no_panic() { assert_eq!(actual_repl.out, "spam"); } -#[test] -fn not_allowed_submodule_file() { - let inp = &["use samples/not_allowed"]; - let actual = nu!(cwd: "tests/modules", &inp.join("; ")); - assert!(actual.err.contains("invalid_module_file_name")); -} - #[test] fn module_dir_missing_mod_nu() { let inp = &["use samples/missing_mod_nu"]; diff --git a/tests/modules/samples/not_allowed/mod.nu b/tests/modules/samples/not_allowed/mod.nu deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/modules/samples/not_allowed/not_allowed.nu b/tests/modules/samples/not_allowed/not_allowed.nu deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/modules/samples/spam/bacon/beans/mod.nu b/tests/modules/samples/spam/bacon/beans/mod.nu index b9614067f0..c18d0706b9 100644 --- a/tests/modules/samples/spam/bacon/beans/mod.nu +++ b/tests/modules/samples/spam/bacon/beans/mod.nu @@ -1,3 +1,5 @@ +export module foo.nu + export def test [] { 'test' } export def main [] { 'beans' } diff --git a/tests/modules/samples/spam/bacon/mod.nu b/tests/modules/samples/spam/bacon/mod.nu index f89c55ed32..9a82f59f5b 100644 --- a/tests/modules/samples/spam/bacon/mod.nu +++ b/tests/modules/samples/spam/bacon/mod.nu @@ -1,3 +1,6 @@ +export module foo.nu +export module beans + export def test [] { 'test' } export def main [] { 'bacon' } diff --git a/tests/modules/samples/spam/mod.nu b/tests/modules/samples/spam/mod.nu index 5b55b86a12..06419870c5 100644 --- a/tests/modules/samples/spam/mod.nu +++ b/tests/modules/samples/spam/mod.nu @@ -1,4 +1,7 @@ export module eggs.nu +export module foo.nu +export module bar.nu +export module bacon export def main [] { 'spam' }