Add 'as' keyword to 'overlay add' (#6314)

crates/nu-command/src/core_commands/overlay/add.rs

@@ -24,6 +24,11 @@ impl Command for OverlayAdd {
                 SyntaxShape::String,
                 "Module name to create overlay for",
             )
+            .optional(
+                "as",
+                SyntaxShape::Keyword(b"as".to_vec(), Box::new(SyntaxShape::String)),
+                "as keyword followed by a new name",
+            )
             .switch(
                 "prefix",
                 "Prepend module name to the imported commands and aliases",
@@ -50,67 +55,86 @@ impl Command for OverlayAdd {
     ) -> Result<PipelineData, ShellError> {
         let name_arg: Spanned<String> = call.req(engine_state, stack, 0)?;
 
-        let maybe_overlay_name = if engine_state
+        let (overlay_name, overlay_name_span) = if let Some(kw_expression) = call.positional_nth(1)
+        {
+            // If renamed via the 'as' keyword, use the new name as the overlay name
+            if let Some(new_name_expression) = kw_expression.as_keyword() {
+                if let Some(new_name) = new_name_expression.as_string() {
+                    (new_name, new_name_expression.span)
+                } else {
+                    return Err(ShellError::NushellFailedSpanned(
+                        "Wrong keyword type".to_string(),
+                        "keyword argument not a string".to_string(),
+                        new_name_expression.span,
+                    ));
+                }
+            } else {
+                return Err(ShellError::NushellFailedSpanned(
+                    "Wrong keyword type".to_string(),
+                    "keyword argument not a keyword".to_string(),
+                    kw_expression.span,
+                ));
+            }
+        } else if engine_state
             .find_overlay(name_arg.item.as_bytes())
             .is_some()
         {
-            Some(name_arg.item.clone())
+            (name_arg.item, name_arg.span)
         } else if let Some(os_str) = Path::new(&name_arg.item).file_stem() {
-            os_str.to_str().map(|name| name.to_string())
+            if let Some(name) = os_str.to_str() {
-        } else {
+                (name.to_string(), name_arg.span)
-            None
-        };
-
-        if let Some(overlay_name) = maybe_overlay_name {
-            if let Some(overlay_id) = engine_state.find_overlay(overlay_name.as_bytes()) {
-                let old_module_id = engine_state.get_overlay(overlay_id).origin;
-
-                stack.add_overlay(overlay_name.clone());
-
-                if let Some(new_module_id) = engine_state.find_module(overlay_name.as_bytes(), &[])
-                {
-                    if !stack.has_env_overlay(&overlay_name, engine_state)
-                        || (old_module_id != new_module_id)
-                    {
-                        // Add environment variables only if:
-                        // a) adding a new overlay
-                        // b) refreshing an active overlay (the origin module changed)
-                        let module = engine_state.get_module(new_module_id);
-
-                        for (name, block_id) in module.env_vars() {
-                            let name = if let Ok(s) = String::from_utf8(name.clone()) {
-                                s
-                            } else {
-                                return Err(ShellError::NonUtf8(call.head));
-                            };
-
-                            let block = engine_state.get_block(block_id);
-
-                            let val = eval_block(
-                                engine_state,
-                                stack,
-                                block,
-                                PipelineData::new(call.head),
-                                false,
-                                true,
-                            )?
-                            .into_value(call.head);
-
-                            stack.add_env_var(name, val);
-                        }
-                    }
-                }
             } else {
-                return Err(ShellError::OverlayNotFoundAtRuntime(
+                return Err(ShellError::NonUtf8(name_arg.span));
-                    name_arg.item,
-                    name_arg.span,
-                ));
             }
         } else {
             return Err(ShellError::OverlayNotFoundAtRuntime(
                 name_arg.item,
                 name_arg.span,
             ));
+        };
+
+        if let Some(overlay_id) = engine_state.find_overlay(overlay_name.as_bytes()) {
+            let old_module_id = engine_state.get_overlay(overlay_id).origin;
+
+            stack.add_overlay(overlay_name.clone());
+
+            if let Some(new_module_id) = engine_state.find_module(overlay_name.as_bytes(), &[]) {
+                if !stack.has_env_overlay(&overlay_name, engine_state)
+                    || (old_module_id != new_module_id)
+                {
+                    // Add environment variables only if:
+                    // a) adding a new overlay
+                    // b) refreshing an active overlay (the origin module changed)
+                    let module = engine_state.get_module(new_module_id);
+
+                    for (name, block_id) in module.env_vars() {
+                        let name = if let Ok(s) = String::from_utf8(name.clone()) {
+                            s
+                        } else {
+                            return Err(ShellError::NonUtf8(call.head));
+                        };
+
+                        let block = engine_state.get_block(block_id);
+
+                        let val = eval_block(
+                            engine_state,
+                            stack,
+                            block,
+                            PipelineData::new(call.head),
+                            false,
+                            true,
+                        )?
+                        .into_value(call.head);
+
+                        stack.add_env_var(name, val);
+                    }
+                }
+            }
+        } else {
+            return Err(ShellError::OverlayNotFoundAtRuntime(
+                overlay_name,
+                overlay_name_span,
+            ));
         }
 
         Ok(PipelineData::new(call.head))

crates/nu-parser/src/errors.rs

@@ -160,6 +160,10 @@ pub enum ParseError {
     )]
     CantRemoveDefaultOverlay(String, #[label = "can't remove overlay"] Span),
 
+    #[error("Cannot add overlay.")]
+    #[diagnostic(code(nu::parser::cant_add_overlay_help), url(docsrs), help("{0}"))]
+    CantAddOverlayHelp(String, #[label = "cannot add this overlay"] Span),
+
     #[error("Not found.")]
     #[diagnostic(code(nu::parser::not_found), url(docsrs))]
     NotFound(#[label = "did not find anything under this name"] Span),
@@ -342,6 +346,7 @@ impl ParseError {
             ParseError::OverlayPrefixMismatch(_, _, s) => *s,
             ParseError::CantRemoveLastOverlay(s) => *s,
             ParseError::CantRemoveDefaultOverlay(_, s) => *s,
+            ParseError::CantAddOverlayHelp(_, s) => *s,
             ParseError::NotFound(s) => *s,
             ParseError::DuplicateCommandDef(s) => *s,
             ParseError::UnknownCommand(s) => *s,

crates/nu-parser/src/parse_keywords.rs

@@ -5,7 +5,7 @@ use nu_protocol::{
         ImportPatternMember, Pipeline,
     },
     engine::{StateWorkingSet, DEFAULT_OVERLAY_NAME},
-    span, Exportable, Module, PositionalArg, Span, SyntaxShape, Type,
+    span, Exportable, Module, PositionalArg, Span, Spanned, SyntaxShape, Type,
 };
 use std::collections::HashSet;
 use std::path::{Path, PathBuf};
@@ -2126,6 +2126,36 @@ pub fn parse_overlay_add(
         );
     };
 
+    let new_name = if let Some(kw_expression) = call.positional_nth(1) {
+        if let Some(new_name_expression) = kw_expression.as_keyword() {
+            if let Some(new_name) = new_name_expression.as_string() {
+                Some(Spanned {
+                    item: new_name,
+                    span: new_name_expression.span,
+                })
+            } else {
+                return (
+                    garbage_pipeline(spans),
+                    Some(ParseError::TypeMismatch(
+                        Type::String,
+                        new_name_expression.ty.clone(),
+                        new_name_expression.span,
+                    )),
+                );
+            }
+        } else {
+            return (
+                garbage_pipeline(spans),
+                Some(ParseError::ExpectedKeyword(
+                    "as keyword".to_string(),
+                    kw_expression.span,
+                )),
+            );
+        }
+    } else {
+        None
+    };
+
     let has_prefix = call.has_flag("prefix");
 
     let pipeline = Pipeline::from_vec(vec![Expression {
@@ -2162,6 +2192,18 @@ pub fn parse_overlay_add(
             );
         }
 
+        if let Some(new_name) = new_name {
+            if new_name.item != overlay_name {
+                return (
+                    pipeline,
+                    Some(ParseError::CantAddOverlayHelp(
+                            format!("Cannot add overlay as '{}' because it already exsits under the name '{}'", new_name.item, overlay_name),
+                            new_name.span,
+                    )),
+                );
+            }
+        }
+
         // Activate existing overlay
         let module_id = overlay_frame.origin;
 
@@ -2186,7 +2228,7 @@ pub fn parse_overlay_add(
             working_set.find_module(overlay_name.as_bytes())
         {
             Some((
-                overlay_name,
+                new_name.map(|spanned| spanned.item).unwrap_or(overlay_name),
                 working_set.get_module(module_id).clone(),
                 module_id,
             ))
@@ -2236,7 +2278,11 @@ pub fn parse_overlay_add(
                         let _ = working_set.add_block(block);
                         let module_id = working_set.add_module(&overlay_name, module.clone());
 
-                        Some((overlay_name, module, module_id))
+                        Some((
+                            new_name.map(|spanned| spanned.item).unwrap_or(overlay_name),
+                            module,
+                            module_id,
+                        ))
                     } else {
                         return (
                             pipeline,

tests/overlays/mod.rs

@@ -633,3 +633,106 @@ fn overlay_keep_pwd() {
     assert_eq!(actual.out, "samples");
     assert_eq!(actual_repl.out, "samples");
 }
+
+#[test]
+fn overlay_wrong_rename_type() {
+    let inp = &[r#"module spam {}"#, r#"overlay add spam as { echo foo }"#];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+
+    assert!(actual.err.contains("parse_mismatch"));
+}
+
+#[test]
+fn overlay_add_renamed() {
+    let inp = &[
+        r#"module spam { export def foo [] { "foo" } }"#,
+        r#"overlay add spam as eggs --prefix"#,
+        r#"eggs foo"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp));
+
+    assert_eq!(actual.out, "foo");
+    assert_eq!(actual_repl.out, "foo");
+}
+
+#[test]
+fn overlay_add_renamed_from_file() {
+    let inp = &[
+        r#"overlay add samples/spam.nu as eggs --prefix"#,
+        r#"eggs foo"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp));
+
+    assert_eq!(actual.out, "foo");
+    assert_eq!(actual_repl.out, "foo");
+}
+
+#[test]
+fn overlay_cant_rename_existing_overlay() {
+    let inp = &[
+        r#"module spam { export def foo [] { "foo" } }"#,
+        r#"overlay add spam"#,
+        r#"overlay remove spam"#,
+        r#"overlay add spam as eggs"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp));
+
+    assert!(actual.err.contains("cant_add_overlay_help"));
+    assert!(actual_repl.err.contains("cant_add_overlay_help"));
+}
+
+#[test]
+fn overlay_can_add_renamed_overlay() {
+    let inp = &[
+        r#"module spam { export def foo [] { "foo" } }"#,
+        r#"overlay add spam as eggs --prefix"#,
+        r#"overlay add spam --prefix"#,
+        r#"(spam foo) + (eggs foo)"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp));
+
+    assert_eq!(actual.out, "foofoo");
+    assert_eq!(actual_repl.out, "foofoo");
+}
+
+#[test]
+fn overlay_remove_renamed_overlay() {
+    let inp = &[
+        r#"module spam { export def foo [] { "foo" } }"#,
+        r#"overlay add spam as eggs"#,
+        r#"overlay remove eggs"#,
+        r#"foo"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp));
+
+    assert!(actual.err.contains("did you mean"));
+    assert!(actual_repl.err.contains("did you mean"));
+}
+
+#[test]
+fn overlay_remove_and_add_renamed_overlay() {
+    let inp = &[
+        r#"module spam { export def foo [] { "foo" } }"#,
+        r#"overlay add spam as eggs"#,
+        r#"overlay remove eggs"#,
+        r#"overlay add eggs"#,
+        r#"foo"#,
+    ];
+
+    let actual = nu!(cwd: "tests/overlays", pipeline(&inp.join("; ")));
+    let actual_repl = nu!(cwd: "tests/overlays", nu_repl_code(inp));
+
+    assert_eq!(actual.out, "foo");
+    assert_eq!(actual_repl.out, "foo");
+}