From 3552d03f6c041200f110c549c74d15ead59bbd78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 22 Jan 2023 21:34:15 +0200 Subject: [PATCH] Allow main command to define top-level module command (#7764) --- .../src/core_commands/help_modules.rs | 10 +- crates/nu-command/src/system/nu_check.rs | 6 +- crates/nu-command/tests/commands/help.rs | 28 ++ crates/nu-command/tests/commands/use_.rs | 93 ++++++ crates/nu-engine/src/scope.rs | 6 +- crates/nu-parser/src/errors.rs | 22 ++ crates/nu-parser/src/parse_keywords.rs | 276 ++++++++++++++---- crates/nu-parser/src/parser.rs | 6 +- crates/nu-protocol/src/engine/engine_state.rs | 20 +- crates/nu-protocol/src/module.rs | 66 +++-- src/tests/test_hiding.rs | 80 +++-- tests/modules/mod.rs | 88 ++++++ tests/overlays/mod.rs | 67 +++++ 13 files changed, 633 insertions(+), 135 deletions(-) diff --git a/crates/nu-command/src/core_commands/help_modules.rs b/crates/nu-command/src/core_commands/help_modules.rs index ad98a855b..ed4de90dc 100644 --- a/crates/nu-command/src/core_commands/help_modules.rs +++ b/crates/nu-command/src/core_commands/help_modules.rs @@ -151,15 +151,11 @@ pub fn help_modules( long_desc.push_str(&format!("{G}Module{RESET}: {C}{name}{RESET}")); long_desc.push_str("\n\n"); - if !module.decls.is_empty() { + if !module.decls.is_empty() || module.main.is_some() { let commands: Vec<(Vec, DeclId)> = engine_state.get_decls_sorted(false).collect(); - let mut module_commands: Vec<(&[u8], DeclId)> = module - .decls - .iter() - .map(|(name, id)| (name.as_ref(), *id)) - .collect(); - module_commands.sort_by(|a, b| a.0.cmp(b.0)); + let mut module_commands = module.decls(); + module_commands.sort_by(|a, b| a.0.cmp(&b.0)); let commands_str = module_commands .iter() diff --git a/crates/nu-command/src/system/nu_check.rs b/crates/nu-command/src/system/nu_check.rs index e8a9145e4..25e5d314b 100644 --- a/crates/nu-command/src/system/nu_check.rs +++ b/crates/nu-command/src/system/nu_check.rs @@ -302,12 +302,14 @@ fn parse_module( is_debug: bool, span: Span, ) -> Result { + let filename = filename.unwrap_or_else(|| "empty".to_string()); + let start = working_set.next_span_start(); - working_set.add_file(filename.unwrap_or_else(|| "empty".to_string()), contents); + working_set.add_file(filename.clone(), contents); let end = working_set.next_span_start(); let new_span = Span::new(start, end); - let (_, _, _, err) = parse_module_block(working_set, new_span, &[]); + let (_, _, _, err) = parse_module_block(working_set, new_span, filename.as_bytes(), &[]); if err.is_some() { if is_debug { diff --git a/crates/nu-command/tests/commands/help.rs b/crates/nu-command/tests/commands/help.rs index 98515628a..865659a8b 100644 --- a/crates/nu-command/tests/commands/help.rs +++ b/crates/nu-command/tests/commands/help.rs @@ -314,3 +314,31 @@ fn help_usage_extra_usage() { assert!(!actual.out.contains("alias_line2")); }) } + +#[test] +fn help_modules_main_1() { + let inp = &[ + r#"module spam { + export def main [] { 'foo' }; + }"#, + "help spam", + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert!(actual.out.contains(" spam")); +} + +#[test] +fn help_modules_main_2() { + let inp = &[ + r#"module spam { + export def main [] { 'foo' }; + }"#, + "help modules | where name == spam | get 0.commands.0", + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, "spam"); +} diff --git a/crates/nu-command/tests/commands/use_.rs b/crates/nu-command/tests/commands/use_.rs index ab1a84a6f..5c36fdae2 100644 --- a/crates/nu-command/tests/commands/use_.rs +++ b/crates/nu-command/tests/commands/use_.rs @@ -211,3 +211,96 @@ fn use_module_creates_accurate_did_you_mean_2() { "command 'foo' was not found but it exists in module 'spam'; try importing it with `use`" )); } + +#[test] +fn use_main_1() { + let inp = &[ + r#"module spam { export def main [] { "spam" } }"#, + r#"use spam"#, + r#"spam"#, + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, "spam"); +} + +#[test] +fn use_main_2() { + let inp = &[ + r#"module spam { export def main [] { "spam" } }"#, + r#"use spam main"#, + r#"spam"#, + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, "spam"); +} + +#[test] +fn use_main_3() { + let inp = &[ + r#"module spam { export def main [] { "spam" } }"#, + r#"use spam [ main ]"#, + r#"spam"#, + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, "spam"); +} + +#[test] +fn use_main_4() { + let inp = &[ + r#"module spam { export def main [] { "spam" } }"#, + r#"use spam *"#, + r#"spam"#, + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, "spam"); +} + +#[test] +fn use_main_def_env() { + let inp = &[ + r#"module spam { export def-env main [] { let-env SPAM = "spam" } }"#, + r#"use spam"#, + r#"spam"#, + r#"$env.SPAM"#, + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, "spam"); +} + +#[test] +fn use_main_def_known_external() { + // note: requires installed cargo + let inp = &[ + r#"module cargo { export extern main [] }"#, + r#"use cargo"#, + r#"cargo --version"#, + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert!(actual.out.contains("cargo")); +} + +#[test] +fn use_main_not_exported() { + let inp = &[ + r#"module spam { def main [] { "spam" } }"#, + r#"use spam"#, + r#"spam"#, + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert!(actual.err.contains("external_command")); +} diff --git a/crates/nu-engine/src/scope.rs b/crates/nu-engine/src/scope.rs index 448652905..467cb0998 100644 --- a/crates/nu-engine/src/scope.rs +++ b/crates/nu-engine/src/scope.rs @@ -505,9 +505,9 @@ impl<'e, 's> ScopeData<'e, 's> { let module = self.engine_state.get_module(**module_id); let export_commands: Vec = module - .decls - .keys() - .map(|bytes| Value::string(String::from_utf8_lossy(bytes), span)) + .decls() + .iter() + .map(|(bytes, _)| Value::string(String::from_utf8_lossy(bytes), span)) .collect(); let export_aliases: Vec = module diff --git a/crates/nu-parser/src/errors.rs b/crates/nu-parser/src/errors.rs index 92ac4597a..89604f141 100644 --- a/crates/nu-parser/src/errors.rs +++ b/crates/nu-parser/src/errors.rs @@ -213,6 +213,26 @@ pub enum ParseError { #[diagnostic(code(nu::parser::cyclical_module_import), url(docsrs), help("{0}"))] CyclicalModuleImport(String, #[label = "detected cyclical module import"] Span), + #[error("Can't export {0} named same as the module.")] + #[diagnostic( + code(nu::parser::named_as_module), + url(docsrs), + help("Module {1} can't export {0} named the same as the module. Either change the module name, or export `main` custom command.") + )] + NamedAsModule( + String, + String, + #[label = "can't export from module {1}"] Span, + ), + + #[error("Can't export alias defined as 'main'.")] + #[diagnostic( + code(nu::parser::export_main_alias_not_allowed), + url(docsrs), + help("Exporting aliases as 'main' is not allowed. Either rename the alias or convert it to a custom command.") + )] + ExportMainAliasNotAllowed(#[label = "can't export from module"] Span), + #[error("Active overlay not found.")] #[diagnostic(code(nu::parser::active_overlay_not_found), url(docsrs))] ActiveOverlayNotFound(#[label = "not an active overlay"] Span), @@ -450,6 +470,8 @@ impl ParseError { ParseError::AliasNotValid(s) => *s, ParseError::CommandDefNotValid(s) => *s, ParseError::ModuleNotFound(s) => *s, + ParseError::NamedAsModule(_, _, s) => *s, + ParseError::ExportMainAliasNotAllowed(s) => *s, ParseError::CyclicalModuleImport(_, s) => *s, ParseError::ModuleOrOverlayNotFound(s) => *s, ParseError::ActiveOverlayNotFound(s) => *s, diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 93577f449..6ae9d5b11 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -232,6 +232,7 @@ pub fn parse_for( pub fn parse_def( working_set: &mut StateWorkingSet, lite_command: &LiteCommand, + module_name: Option<&[u8]>, expand_aliases_denylist: &[usize], ) -> (Pipeline, Option) { let spans = &lite_command.parts[..]; @@ -337,9 +338,39 @@ pub fn parse_def( let mut error = None; - if let (Some(name), Some(mut signature), Some(block_id)) = - (&name_expr.as_string(), sig.as_signature(), block.as_block()) - { + let name = if let Some(name) = name_expr.as_string() { + if let Some(mod_name) = module_name { + if name.as_bytes() == mod_name { + let name_expr_span = name_expr.span; + + return ( + Pipeline::from_vec(vec![Expression { + expr: Expr::Call(call), + span: call_span, + ty: Type::Any, + custom_completion: None, + }]), + Some(ParseError::NamedAsModule( + "command".to_string(), + name, + name_expr_span, + )), + ); + } + } + + name + } else { + return ( + garbage_pipeline(spans), + Some(ParseError::UnknownState( + "Could not get string from string expression".into(), + name_expr.span, + )), + ); + }; + + if let (Some(mut signature), Some(block_id)) = (sig.as_signature(), block.as_block()) { if let Some(decl_id) = working_set.find_predecl(name.as_bytes()) { let declaration = working_set.get_decl_mut(decl_id); @@ -398,17 +429,8 @@ pub fn parse_def( }; } - if let Some(name) = name_expr.as_string() { - // It's OK if it returns None: The decl was already merged in previous parse pass. - working_set.merge_predecl(name.as_bytes()); - } else { - error = error.or_else(|| { - Some(ParseError::UnknownState( - "Could not get string from string expression".into(), - name_expr.span, - )) - }); - } + // It's OK if it returns None: The decl was already merged in previous parse pass. + working_set.merge_predecl(name.as_bytes()); ( Pipeline::from_vec(vec![Expression { @@ -424,6 +446,7 @@ pub fn parse_def( pub fn parse_extern( working_set: &mut StateWorkingSet, lite_command: &LiteCommand, + module_name: Option<&[u8]>, expand_aliases_denylist: &[usize], ) -> (Pipeline, Option) { let spans = &lite_command.parts; @@ -495,16 +518,45 @@ pub fn parse_extern( if let (Some(name_expr), Some(sig)) = (name_expr, sig) { if let (Some(name), Some(mut signature)) = (&name_expr.as_string(), sig.as_signature()) { + if let Some(mod_name) = module_name { + if name.as_bytes() == mod_name { + let name_expr_span = name_expr.span; + return ( + Pipeline::from_vec(vec![Expression { + expr: Expr::Call(call), + span: call_span, + ty: Type::Any, + custom_completion: None, + }]), + Some(ParseError::NamedAsModule( + "known external".to_string(), + name.clone(), + name_expr_span, + )), + ); + } + } + if let Some(decl_id) = working_set.find_predecl(name.as_bytes()) { let declaration = working_set.get_decl_mut(decl_id); - signature.name = name.clone(); + let external_name = if let Some(mod_name) = module_name { + if name.as_bytes() == b"main" { + String::from_utf8_lossy(mod_name).to_string() + } else { + name.clone() + } + } else { + name.clone() + }; + + signature.name = external_name.clone(); signature.usage = usage.clone(); signature.extra_usage = extra_usage.clone(); signature.allows_unknown_args = true; let decl = KnownExternal { - name: name.to_string(), + name: external_name, usage: [usage, extra_usage].join("\n"), signature, }; @@ -546,6 +598,7 @@ pub fn parse_extern( pub fn parse_alias( working_set: &mut StateWorkingSet, lite_command: &LiteCommand, + module_name: Option<&[u8]>, expand_aliases_denylist: &[usize], ) -> (Pipeline, Option) { let spans = &lite_command.parts; @@ -644,6 +697,37 @@ pub fn parse_alias( } else { alias_name.to_vec() }; + + if let Some(mod_name) = module_name { + if alias_name == mod_name { + return ( + Pipeline::from_vec(vec![Expression { + expr: Expr::Call(call), + span: span(spans), + ty: output, + custom_completion: None, + }]), + Some(ParseError::NamedAsModule( + "alias".to_string(), + String::from_utf8_lossy(&alias_name).to_string(), + spans[split_id], + )), + ); + } + + if &alias_name == b"main" { + return ( + Pipeline::from_vec(vec![Expression { + expr: Expr::Call(call), + span: span(spans), + ty: output, + custom_completion: None, + }]), + Some(ParseError::ExportMainAliasNotAllowed(spans[split_id])), + ); + } + } + let _equals = working_set.get_span_contents(spans[split_id + 1]); let replacement = spans[(split_id + 2)..].to_vec(); @@ -773,16 +857,16 @@ pub fn parse_export_in_block( } match full_name.as_slice() { - b"export alias" => parse_alias(working_set, lite_command, expand_aliases_denylist), + b"export alias" => parse_alias(working_set, lite_command, None, expand_aliases_denylist), b"export def" | b"export def-env" => { - parse_def(working_set, lite_command, expand_aliases_denylist) + parse_def(working_set, lite_command, None, expand_aliases_denylist) } b"export use" => { let (pipeline, _, err) = parse_use(working_set, &lite_command.parts, expand_aliases_denylist); (pipeline, err) } - b"export extern" => parse_extern(working_set, lite_command, expand_aliases_denylist), + b"export extern" => parse_extern(working_set, lite_command, None, expand_aliases_denylist), _ => ( garbage_pipeline(&lite_command.parts), Some(ParseError::UnexpectedKeyword( @@ -797,6 +881,7 @@ pub fn parse_export_in_block( pub fn parse_export_in_module( working_set: &mut StateWorkingSet, lite_command: &LiteCommand, + module_name: &[u8], expand_aliases_denylist: &[usize], ) -> (Pipeline, Vec, Option) { let spans = &lite_command.parts[..]; @@ -856,8 +941,12 @@ pub fn parse_export_in_module( comments: lite_command.comments.clone(), parts: spans[1..].to_vec(), }; - let (pipeline, err) = - parse_def(working_set, &lite_command, expand_aliases_denylist); + let (pipeline, err) = parse_def( + working_set, + &lite_command, + Some(module_name), + expand_aliases_denylist, + ); error = error.or(err); let export_def_decl_id = @@ -924,8 +1013,12 @@ pub fn parse_export_in_module( comments: lite_command.comments.clone(), parts: spans[1..].to_vec(), }; - let (pipeline, err) = - parse_def(working_set, &lite_command, expand_aliases_denylist); + let (pipeline, err) = parse_def( + working_set, + &lite_command, + Some(module_name), + expand_aliases_denylist, + ); error = error.or(err); let export_def_decl_id = @@ -993,8 +1086,12 @@ pub fn parse_export_in_module( comments: lite_command.comments.clone(), parts: spans[1..].to_vec(), }; - let (pipeline, err) = - parse_extern(working_set, &lite_command, expand_aliases_denylist); + let (pipeline, err) = parse_extern( + working_set, + &lite_command, + Some(module_name), + expand_aliases_denylist, + ); error = error.or(err); let export_def_decl_id = @@ -1062,8 +1159,12 @@ pub fn parse_export_in_module( comments: lite_command.comments.clone(), parts: spans[1..].to_vec(), }; - let (pipeline, err) = - parse_alias(working_set, &lite_command, expand_aliases_denylist); + let (pipeline, err) = parse_alias( + working_set, + &lite_command, + Some(module_name), + expand_aliases_denylist, + ); error = error.or(err); let export_alias_decl_id = @@ -1349,6 +1450,7 @@ fn collect_first_comments(tokens: &[Token]) -> Vec { pub fn parse_module_block( working_set: &mut StateWorkingSet, span: Span, + module_name: &[u8], expand_aliases_denylist: &[usize], ) -> (Block, Module, Vec, Option) { let mut error = None; @@ -1373,7 +1475,7 @@ pub fn parse_module_block( } } - let mut module = Module::from_span(span); + let mut module = Module::from_span(module_name.to_vec(), span); let block: Block = output .block @@ -1386,20 +1488,32 @@ pub fn parse_module_block( let (pipeline, err) = match name { b"def" | b"def-env" => { - let (pipeline, err) = - parse_def(working_set, command, expand_aliases_denylist); + let (pipeline, err) = parse_def( + working_set, + command, + None, // using commands named as the module locally is OK + expand_aliases_denylist, + ); (pipeline, err) } b"extern" => { - let (pipeline, err) = - parse_extern(working_set, command, expand_aliases_denylist); + let (pipeline, err) = parse_extern( + working_set, + command, + None, + expand_aliases_denylist, + ); (pipeline, err) } b"alias" => { - let (pipeline, err) = - parse_alias(working_set, command, expand_aliases_denylist); + let (pipeline, err) = parse_alias( + working_set, + command, + None, // using aliases named as the module locally is OK + expand_aliases_denylist, + ); (pipeline, err) } @@ -1413,6 +1527,7 @@ pub fn parse_module_block( let (pipe, exportables, err) = parse_export_in_module( working_set, command, + module_name, expand_aliases_denylist, ); @@ -1420,7 +1535,11 @@ pub fn parse_module_block( for exportable in exportables { match exportable { Exportable::Decl { name, id } => { - module.add_decl(name, id); + if &name == b"main" { + module.main = Some(id); + } else { + module.add_decl(name, id); + } } Exportable::Alias { name, id } => { module.add_alias(name, id); @@ -1519,8 +1638,12 @@ pub fn parse_module( let block_span = Span::new(start, end); - let (block, module, inner_comments, err) = - parse_module_block(working_set, block_span, expand_aliases_denylist); + let (block, module, inner_comments, err) = parse_module_block( + working_set, + block_span, + module_name.as_bytes(), + expand_aliases_denylist, + ); error = error.or(err); let block_id = working_set.add_block(block); @@ -1762,6 +1885,7 @@ pub fn parse_use( let (block, module, module_comments, err) = parse_module_block( working_set, Span::new(span_start, span_end), + module_name.as_bytes(), expand_aliases_denylist, ); error = error.or(err); @@ -1801,14 +1925,16 @@ pub fn parse_use( ); } } else { - error = error.or(Some(ParseError::ModuleNotFound(import_pattern.head.span))); - - let old_span = import_pattern.head.span; - - let mut import_pattern = ImportPattern::new(); - import_pattern.head.span = old_span; - - (import_pattern, Module::new()) + return ( + Pipeline::from_vec(vec![Expression { + expr: Expr::Call(call), + span: span(spans), + ty: Type::Any, + custom_completion: None, + }]), + vec![], + Some(ParseError::ModuleNotFound(import_pattern.head.span)), + ); } } else { return ( @@ -1831,12 +1957,18 @@ pub fn parse_use( let mut decl_output = vec![]; let mut alias_output = vec![]; - if let Some(id) = module.get_decl_id(name) { + if name == b"main" { + if let Some(id) = &module.main { + decl_output.push((import_pattern.head.name.clone(), *id)); + } else { + error = error.or(Some(ParseError::ExportNotFound(*span))); + } + } else if let Some(id) = module.get_decl_id(name) { decl_output.push((name.clone(), id)); } else if let Some(id) = module.get_alias_id(name) { alias_output.push((name.clone(), id)); } else { - error = error.or(Some(ParseError::ExportNotFound(*span))) + error = error.or(Some(ParseError::ExportNotFound(*span))); } (decl_output, alias_output) @@ -1846,7 +1978,13 @@ pub fn parse_use( let mut alias_output = vec![]; for (name, span) in names { - if let Some(id) = module.get_decl_id(name) { + if name == b"main" { + if let Some(id) = &module.main { + decl_output.push((import_pattern.head.name.clone(), *id)); + } else { + error = error.or(Some(ParseError::ExportNotFound(*span))); + } + } else if let Some(id) = module.get_decl_id(name) { decl_output.push((name.clone(), id)); } else if let Some(id) = module.get_alias_id(name) { alias_output.push((name.clone(), id)); @@ -1992,6 +2130,7 @@ pub fn parse_hide( error = error.or(err); } + // module used only internally, not saved anywhere let (is_module, module) = if let Some(module_id) = working_set.find_module(&import_pattern.head.name) { @@ -2000,19 +2139,19 @@ pub fn parse_hide( // The pattern head can be: if let Some(id) = working_set.find_alias(&import_pattern.head.name) { // an alias, - let mut module = Module::new(); + let mut module = Module::new(b"tmp".to_vec()); module.add_alias(import_pattern.head.name.clone(), id); (false, module) } else if let Some(id) = working_set.find_decl(&import_pattern.head.name, &Type::Any) { // a custom command, - let mut module = Module::new(); + let mut module = Module::new(b"tmp".to_vec()); module.add_decl(import_pattern.head.name.clone(), id); (false, module) } else { // , or it could be an env var (handled by the engine) - (false, Module::new()) + (false, Module::new(b"tmp".to_vec())) } } else { return ( @@ -2038,7 +2177,14 @@ pub fn parse_hide( let mut aliases = vec![]; let mut decls = vec![]; - if let Some(item) = module.alias_name_with_head(name, &import_pattern.head.name) + if name == b"main" { + if module.main.is_some() { + decls.push(import_pattern.head.name.clone()); + } else { + error = error.or(Some(ParseError::ExportNotFound(*span))); + } + } else if let Some(item) = + module.alias_name_with_head(name, &import_pattern.head.name) { aliases.push(item); } else if let Some(item) = @@ -2056,7 +2202,14 @@ pub fn parse_hide( let mut decls = vec![]; for (name, span) in names { - if let Some(item) = + if name == b"main" { + if module.main.is_some() { + decls.push(import_pattern.head.name.clone()); + } else { + error = error.or(Some(ParseError::ExportNotFound(*span))); + break; + } + } else if let Some(item) = module.alias_name_with_head(name, &import_pattern.head.name) { aliases.push(item); @@ -2346,7 +2499,11 @@ pub fn parse_overlay_new( custom_completion: None, }]); - let module_id = working_set.add_module(&overlay_name, Module::new(), vec![]); + let module_id = working_set.add_module( + &overlay_name, + Module::new(overlay_name.as_bytes().to_vec()), + vec![], + ); working_set.add_overlay( overlay_name.as_bytes().to_vec(), @@ -2525,7 +2682,12 @@ pub fn parse_overlay_use( if let Some(new_module_id) = working_set.find_module(overlay_name.as_bytes()) { if !do_reload && (module_id == new_module_id) { - (overlay_name, Module::new(), module_id, false) + ( + overlay_name, + Module::new(working_set.get_module(module_id).name.clone()), + module_id, + false, + ) } else { // The origin module of an overlay changed => update it ( @@ -2536,7 +2698,8 @@ pub fn parse_overlay_use( ) } } else { - (overlay_name, Module::new(), module_id, true) + let module_name = overlay_name.as_bytes().to_vec(); + (overlay_name, Module::new(module_name), module_id, true) } } else { // Create a new overlay from a module @@ -2586,6 +2749,7 @@ pub fn parse_overlay_use( let (block, module, module_comments, err) = parse_module_block( working_set, Span::new(span_start, span_end), + overlay_name.as_bytes(), expand_aliases_denylist, ); error = error.or(err); diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 88370cebd..776853c6c 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -5291,8 +5291,8 @@ pub fn parse_builtin_commands( let name = working_set.get_span_contents(lite_command.parts[0]); match name { - b"def" | b"def-env" => parse_def(working_set, lite_command, expand_aliases_denylist), - b"extern" => parse_extern(working_set, lite_command, expand_aliases_denylist), + b"def" | b"def-env" => parse_def(working_set, lite_command, None, expand_aliases_denylist), + b"extern" => parse_extern(working_set, lite_command, None, expand_aliases_denylist), b"let" | b"const" => { parse_let_or_const(working_set, &lite_command.parts, expand_aliases_denylist) } @@ -5301,7 +5301,7 @@ pub fn parse_builtin_commands( let (expr, err) = parse_for(working_set, &lite_command.parts, expand_aliases_denylist); (Pipeline::from_vec(vec![expr]), err) } - b"alias" => parse_alias(working_set, lite_command, expand_aliases_denylist), + b"alias" => parse_alias(working_set, lite_command, None, expand_aliases_denylist), b"module" => parse_module(working_set, lite_command, expand_aliases_denylist), b"use" => { let (pipeline, _, err) = diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index ac31a6638..a93f485af 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -170,7 +170,7 @@ impl EngineState { decls: vec![], aliases: vec![], blocks: vec![], - modules: vec![Module::new()], + modules: vec![Module::new(DEFAULT_OVERLAY_NAME.as_bytes().to_vec())], usage: Usage::new(), // make sure we have some default overlay: scope: ScopeFrame::with_empty_overlay( @@ -651,24 +651,6 @@ impl EngineState { } None - - // for (module_id, m) in self.modules.iter().enumerate() { - // if m.has_decl(name) { - // for overlay_frame in self.active_overlays(&[]).iter() { - // let module_name = overlay_frame.modules.iter().find_map(|(key, &val)| { - // if val == module_id { - // Some(key) - // } else { - // None - // } - // }); - // if let Some(final_name) = module_name { - // return Some(&final_name[..]); - // } - // } - // } - // } - // None } pub fn find_overlay(&self, name: &[u8]) -> Option { diff --git a/crates/nu-protocol/src/module.rs b/crates/nu-protocol/src/module.rs index f9d542898..699c1b310 100644 --- a/crates/nu-protocol/src/module.rs +++ b/crates/nu-protocol/src/module.rs @@ -2,33 +2,36 @@ use crate::{AliasId, BlockId, DeclId, Span}; use indexmap::IndexMap; -// TODO: Move the import pattern matching logic here from use/hide commands and -// parse_use/parse_hide - /// Collection of definitions that can be exported from a module #[derive(Debug, Clone)] pub struct Module { + pub name: Vec, pub decls: IndexMap, DeclId>, pub aliases: IndexMap, AliasId>, - pub env_block: Option, + pub env_block: Option, // `export-env { ... }` block + pub main: Option, // `export def main` pub span: Option, } impl Module { - pub fn new() -> Self { + pub fn new(name: Vec) -> Self { Module { + name, decls: IndexMap::new(), aliases: IndexMap::new(), env_block: None, + main: None, span: None, } } - pub fn from_span(span: Span) -> Self { + pub fn from_span(name: Vec, span: Span) -> Self { Module { + name, decls: IndexMap::new(), aliases: IndexMap::new(), env_block: None, + main: None, span: Some(span), } } @@ -63,6 +66,10 @@ impl Module { } pub fn has_decl(&self, name: &[u8]) -> bool { + if name == self.name && self.main.is_some() { + return true; + } + self.decls.contains_key(name) } @@ -93,7 +100,8 @@ impl Module { } pub fn decls_with_head(&self, head: &[u8]) -> Vec<(Vec, DeclId)> { - self.decls + let mut result: Vec<(Vec, DeclId)> = self + .decls .iter() .map(|(name, id)| { let mut new_name = head.to_vec(); @@ -101,11 +109,18 @@ impl Module { new_name.extend(name); (new_name, *id) }) - .collect() + .collect(); + + if let Some(decl_id) = self.main { + result.push((self.name.clone(), decl_id)); + } + + result } pub fn decl_names_with_head(&self, head: &[u8]) -> Vec> { - self.decls + let mut result: Vec> = self + .decls .keys() .map(|name| { let mut new_name = head.to_vec(); @@ -113,7 +128,13 @@ impl Module { new_name.extend(name); new_name }) - .collect() + .collect(); + + if self.main.is_some() { + result.push(self.name.clone()); + } + + result } pub fn aliases_with_head(&self, head: &[u8]) -> Vec<(Vec, AliasId)> { @@ -141,14 +162,27 @@ impl Module { } pub fn decls(&self) -> Vec<(Vec, DeclId)> { - self.decls + let mut result: Vec<(Vec, DeclId)> = self + .decls .iter() .map(|(name, id)| (name.clone(), *id)) - .collect() + .collect(); + + if let Some(decl_id) = self.main { + result.push((self.name.clone(), decl_id)); + } + + result } pub fn decl_names(&self) -> Vec> { - self.decls.keys().cloned().collect() + let mut result: Vec> = self.decls.keys().cloned().collect(); + + if self.main.is_some() { + result.push(self.name.clone()); + } + + result } pub fn alias_names(&self) -> Vec> { @@ -162,9 +196,3 @@ impl Module { .collect() } } - -impl Default for Module { - fn default() -> Self { - Self::new() - } -} diff --git a/src/tests/test_hiding.rs b/src/tests/test_hiding.rs index 0485de9c2..3455ed81f 100644 --- a/src/tests/test_hiding.rs +++ b/src/tests/test_hiding.rs @@ -3,17 +3,14 @@ use crate::tests::{fail_test, run_test, TestResult}; // TODO: Test the use/hide tests also as separate lines in REPL (i.e., with merging the delta in between) #[test] fn hides_def() -> TestResult { - fail_test( - r#"def foo [] { "foo" }; hide foo; foo"#, - "", // we just care if it errors - ) + fail_test(r#"def foo [] { "foo" }; hide foo; foo"#, "external_command") } #[test] fn hides_alias() -> TestResult { fail_test( r#"alias foo = echo "foo"; hide foo; foo"#, - "", // we just care if it errors + "external_command", ) } @@ -52,7 +49,7 @@ fn hides_env_then_redefines() -> TestResult { fn hides_def_in_scope_1() -> TestResult { fail_test( r#"def foo [] { "foo" }; do { hide foo; foo }"#, - "", // we just care if it errors + "external_command", ) } @@ -68,7 +65,7 @@ fn hides_def_in_scope_2() -> TestResult { fn hides_def_in_scope_3() -> TestResult { fail_test( r#"def foo [] { "foo" }; do { hide foo; def foo [] { "bar" }; hide foo; foo }"#, - "", // we just care if it errors + "external_command", ) } @@ -76,7 +73,7 @@ fn hides_def_in_scope_3() -> TestResult { fn hides_def_in_scope_4() -> TestResult { fail_test( r#"def foo [] { "foo" }; do { def foo [] { "bar" }; hide foo; hide foo; foo }"#, - "", // we just care if it errors + "external_command", ) } @@ -84,7 +81,7 @@ fn hides_def_in_scope_4() -> TestResult { fn hides_alias_in_scope_1() -> TestResult { fail_test( r#"alias foo = echo "foo"; do { hide foo; foo }"#, - "", // we just care if it errors + "external_command", ) } @@ -100,7 +97,7 @@ fn hides_alias_in_scope_2() -> TestResult { fn hides_alias_in_scope_3() -> TestResult { fail_test( r#"alias foo = echo "foo"; do { hide foo; alias foo = echo "bar"; hide foo; foo }"#, - "", // we just care if it errors + "external_command", ) } @@ -108,7 +105,7 @@ fn hides_alias_in_scope_3() -> TestResult { fn hides_alias_in_scope_4() -> TestResult { fail_test( r#"alias foo = echo "foo"; do { alias foo = echo "bar"; hide foo; hide foo; foo }"#, - "", // we just care if it errors + "external_command", ) } @@ -213,7 +210,7 @@ fn hides_alias_runs_def_2() -> TestResult { fn hides_alias_and_def() -> TestResult { fail_test( r#"alias foo = echo "foo"; def foo [] { "bar" }; hide foo; hide foo; foo"#, - "", // we just care if it errors + "external_command", ) } @@ -221,7 +218,7 @@ fn hides_alias_and_def() -> TestResult { fn hides_def_import_1() -> TestResult { fail_test( r#"module spam { export def foo [] { "foo" } }; use spam; hide spam foo; spam foo"#, - "", // we just care if it errors + "external_command", ) } @@ -229,7 +226,7 @@ fn hides_def_import_1() -> TestResult { fn hides_def_import_2() -> TestResult { fail_test( r#"module spam { export def foo [] { "foo" } }; use spam; hide spam; spam foo"#, - "", // we just care if it errors + "external_command", ) } @@ -237,7 +234,7 @@ fn hides_def_import_2() -> TestResult { fn hides_def_import_3() -> TestResult { fail_test( r#"module spam { export def foo [] { "foo" } }; use spam; hide spam [foo]; spam foo"#, - "", // we just care if it errors + "external_command", ) } @@ -245,7 +242,7 @@ fn hides_def_import_3() -> TestResult { fn hides_def_import_4() -> TestResult { fail_test( r#"module spam { export def foo [] { "foo" } }; use spam foo; hide foo; foo"#, - "", // we just care if it errors + "external_command", ) } @@ -253,7 +250,7 @@ fn hides_def_import_4() -> TestResult { fn hides_def_import_5() -> TestResult { fail_test( r#"module spam { export def foo [] { "foo" } }; use spam *; hide foo; foo"#, - "", // we just care if it errors + "external_command", ) } @@ -261,7 +258,7 @@ fn hides_def_import_5() -> TestResult { fn hides_def_import_6() -> TestResult { fail_test( r#"module spam { export def foo [] { "foo" } }; use spam *; hide spam *; foo"#, - "", // we just care if it errors + "external_command", ) } @@ -277,7 +274,7 @@ fn hides_def_import_then_reimports() -> TestResult { fn hides_alias_import_1() -> TestResult { fail_test( r#"module spam { export alias foo = "foo" }; use spam; hide spam foo; spam foo"#, - "", // we just care if it errors + "external_command", ) } @@ -285,7 +282,7 @@ fn hides_alias_import_1() -> TestResult { fn hides_alias_import_2() -> TestResult { fail_test( r#"module spam { export alias foo = "foo" }; use spam; hide spam; spam foo"#, - "", // we just care if it errors + "external_command", ) } @@ -293,7 +290,7 @@ fn hides_alias_import_2() -> TestResult { fn hides_alias_import_3() -> TestResult { fail_test( r#"module spam { export alias foo = "foo" }; use spam; hide spam [foo]; spam foo"#, - "", // we just care if it errors + "external_command", ) } @@ -301,7 +298,7 @@ fn hides_alias_import_3() -> TestResult { fn hides_alias_import_4() -> TestResult { fail_test( r#"module spam { export alias foo = "foo" }; use spam foo; hide foo; foo"#, - "", // we just care if it errors + "external_command", ) } @@ -309,7 +306,7 @@ fn hides_alias_import_4() -> TestResult { fn hides_alias_import_5() -> TestResult { fail_test( r#"module spam { export alias foo = "foo" }; use spam *; hide foo; foo"#, - "", // we just care if it errors + "external_command", ) } @@ -317,7 +314,7 @@ fn hides_alias_import_5() -> TestResult { fn hides_alias_import_6() -> TestResult { fail_test( r#"module spam { export alias foo = "foo" }; use spam *; hide spam *; foo"#, - "", // we just care if it errors + "external_command", ) } @@ -338,7 +335,6 @@ fn hides_env_import_1() -> TestResult { } #[test] -#[ignore = "Re-enable after virtualenv update"] fn hides_def_runs_env_import() -> TestResult { run_test( r#"module spam { export-env { let-env foo = "foo" }; export def foo [] { "bar" } }; use spam foo; hide foo; $env.foo"#, @@ -390,7 +386,7 @@ fn hide_shadowed_env() -> TestResult { fn hides_all_decls_within_scope() -> TestResult { fail_test( r#"module spam { export def foo [] { "bar" } }; def foo [] { "foo" }; use spam foo; hide foo; foo"#, - "", // we just care if it errors + "external_command", ) } @@ -401,3 +397,35 @@ fn hides_all_envs_within_scope() -> TestResult { "", ) } + +#[test] +fn hides_main_import_1() -> TestResult { + fail_test( + r#"module spam { export def main [] { "foo" } }; use spam; hide spam; spam"#, + "external_command", + ) +} + +#[test] +fn hides_main_import_2() -> TestResult { + fail_test( + r#"module spam { export def main [] { "foo" } }; use spam; hide spam main; spam"#, + "external_command", + ) +} + +#[test] +fn hides_main_import_3() -> TestResult { + fail_test( + r#"module spam { export def main [] { "foo" } }; use spam; hide spam [ main ]; spam"#, + "external_command", + ) +} + +#[test] +fn hides_main_import_4() -> TestResult { + fail_test( + r#"module spam { export def main [] { "foo" } }; use spam; hide spam *; spam"#, + "external_command", + ) +} diff --git a/tests/modules/mod.rs b/tests/modules/mod.rs index d01f0970b..f53d6920d 100644 --- a/tests/modules/mod.rs +++ b/tests/modules/mod.rs @@ -474,3 +474,91 @@ fn module_import_const_module_name() { assert_eq!(actual.out, "foo"); }) } + +#[test] +fn module_valid_def_name() { + let inp = &[r#"module spam { def spam [] { "spam" } }"#]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, ""); +} + +#[test] +fn module_invalid_def_name() { + let inp = &[r#"module spam { export def spam [] { "spam" } }"#]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert!(actual.err.contains("named_as_module")); +} + +#[test] +fn module_valid_alias_name_1() { + let inp = &[r#"module spam { alias spam = "spam" }"#]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, ""); +} + +#[test] +fn module_valid_alias_name_2() { + let inp = &[r#"module spam { alias main = "spam" }"#]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, ""); +} + +#[test] +fn module_invalid_alias_name() { + let inp = &[r#"module spam { export alias spam = "spam" }"#]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert!(actual.err.contains("named_as_module")); +} + +#[test] +fn module_main_alias_not_allowed() { + let inp = &[r#"module spam { export alias main = 'spam' }"#]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert!(actual.err.contains("export_main_alias_not_allowed")); +} + +#[test] +fn module_valid_known_external_name() { + let inp = &[r#"module spam { extern spam [] }"#]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, ""); +} + +#[test] +fn module_invalid_known_external_name() { + let inp = &[r#"module spam { export extern spam [] }"#]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert!(actual.err.contains("named_as_module")); +} + +#[test] +fn main_inside_module_is_main() { + let inp = &[ + r#"module spam { + export def main [] { 'foo' }; + export def foo [] { main } + }"#, + "use spam foo", + "foo", + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, "foo"); +} diff --git a/tests/overlays/mod.rs b/tests/overlays/mod.rs index 4b2efc0c5..96d7c1dd4 100644 --- a/tests/overlays/mod.rs +++ b/tests/overlays/mod.rs @@ -1193,3 +1193,70 @@ fn overlay_use_and_reolad_keep_custom() { assert_eq!(actual.out, "newfoonewfoonewfoo"); assert_eq!(actual_repl.out, "newfoonewfoonewfoo"); } + +#[test] +fn overlay_use_main() { + let inp = &[ + r#"module spam { export def main [] { "spam" } }"#, + r#"overlay use spam"#, + r#"spam"#, + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, "spam"); +} + +#[test] +fn overlay_use_main_prefix() { + let inp = &[ + r#"module spam { export def main [] { "spam" } }"#, + r#"overlay use spam --prefix"#, + r#"spam"#, + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, "spam"); +} + +#[test] +fn overlay_use_main_def_env() { + let inp = &[ + r#"module spam { export def-env main [] { let-env SPAM = "spam" } }"#, + r#"overlay use spam"#, + r#"spam"#, + r#"$env.SPAM"#, + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert_eq!(actual.out, "spam"); +} + +#[test] +fn overlay_use_main_def_known_external() { + // note: requires installed cargo + let inp = &[ + r#"module cargo { export extern main [] }"#, + r#"overlay use cargo"#, + r#"cargo --version"#, + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert!(actual.out.contains("cargo")); +} + +#[test] +fn overlay_use_main_not_exported() { + let inp = &[ + r#"module spam { def main [] { "spam" } }"#, + r#"overlay use spam"#, + r#"spam"#, + ]; + + let actual = nu!(cwd: ".", pipeline(&inp.join("; "))); + + assert!(actual.err.contains("external_command")); +}