From 36909a2b738e16b9818014a67e5d62a9772fe37d Mon Sep 17 00:00:00 2001 From: Wind Date: Thu, 17 Oct 2024 10:25:45 +0800 Subject: [PATCH] `use` command: Don't create a variable with empty record if it doesn't define any constants (#14051) # Description Fixes: #13967 The key changes lays in `nu-protocol/src/module.rs`, when resolving import pattern, nushell only needs to bring `$module` with a record value if it defines any constants. # User-Facing Changes ```nushell module spam {} use spam ``` Will no longer create a `$spam` variable with an empty record. # Tests + Formatting Adjusted some tests and added some tests. --- crates/nu-protocol/src/module.rs | 29 ++++++++++++---------- tests/const_/mod.rs | 41 +++++++++++++++++++++----------- tests/repl/test_modules.rs | 5 ++++ 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/crates/nu-protocol/src/module.rs b/crates/nu-protocol/src/module.rs index 6b4aec0c60..02bac131dc 100644 --- a/crates/nu-protocol/src/module.rs +++ b/crates/nu-protocol/src/module.rs @@ -161,20 +161,25 @@ impl Module { } let span = self.span.unwrap_or(backup_span); - let const_record = Value::record( - const_rows - .into_iter() - .map(|(name, val)| (String::from_utf8_lossy(&name).to_string(), val)) - .collect(), - span, - ); + + // only needs to bring `$module` with a record value if it defines any constants. + let constants = if const_rows.is_empty() { + vec![] + } else { + vec![( + final_name.clone(), + Value::record( + const_rows + .into_iter() + .map(|(name, val)| (String::from_utf8_lossy(&name).to_string(), val)) + .collect(), + span, + ), + )] + }; return ( - ResolvedImportPattern::new( - decls, - vec![(final_name.clone(), self_id)], - vec![(final_name, const_record)], - ), + ResolvedImportPattern::new(decls, vec![(final_name.clone(), self_id)], constants), errors, ); }; diff --git a/tests/const_/mod.rs b/tests/const_/mod.rs index f736b785ca..0c4ff5d77c 100644 --- a/tests/const_/mod.rs +++ b/tests/const_/mod.rs @@ -238,13 +238,30 @@ fn complex_const_export() { let actual = nu!(&inp.join("; ")); assert_eq!(actual.out, "eats"); - let inp = &[ - MODULE_SETUP, - "use spam", - "($spam.eggs.bacon.none | is-empty)", - ]; + let inp = &[MODULE_SETUP, "use spam", "'none' in $spam.eggs.bacon"]; let actual = nu!(&inp.join("; ")); - assert_eq!(actual.out, "true"); + assert_eq!(actual.out, "false"); +} + +#[test] +fn only_nested_module_have_const() { + let setup = r#" + module spam { + export module eggs { + export module bacon { + export const viking = 'eats' + export module none {} + } + } + } + "#; + let inp = &[setup, "use spam", "$spam.eggs.bacon.viking"]; + let actual = nu!(&inp.join("; ")); + assert_eq!(actual.out, "eats"); + + let inp = &[setup, "use spam", "'none' in $spam.eggs.bacon"]; + let actual = nu!(&inp.join("; ")); + assert_eq!(actual.out, "false"); } #[test] @@ -261,20 +278,16 @@ fn complex_const_glob_export() { let actual = nu!(&inp.join("; ")); assert_eq!(actual.out, "eats"); - let inp = &[MODULE_SETUP, "use spam *", "($eggs.bacon.none | is-empty)"]; + let inp = &[MODULE_SETUP, "use spam *", "'none' in $eggs.bacon"]; let actual = nu!(&inp.join("; ")); - assert_eq!(actual.out, "true"); + assert_eq!(actual.out, "false"); } #[test] fn complex_const_drill_export() { - let inp = &[ - MODULE_SETUP, - "use spam eggs bacon none", - "($none | is-empty)", - ]; + let inp = &[MODULE_SETUP, "use spam eggs bacon none", "$none"]; let actual = nu!(&inp.join("; ")); - assert_eq!(actual.out, "true"); + assert!(actual.err.contains("variable not found")); } #[test] diff --git a/tests/repl/test_modules.rs b/tests/repl/test_modules.rs index dd62c74b77..289efd291a 100644 --- a/tests/repl/test_modules.rs +++ b/tests/repl/test_modules.rs @@ -120,6 +120,11 @@ fn export_consts() -> TestResult { ) } +#[test] +fn dont_export_module_name_as_a_variable() -> TestResult { + fail_test(r#"module spam { }; use spam; $spam"#, "variable not found") +} + #[test] fn func_use_consts() -> TestResult { run_test(