Allow expanding aliases before keywords, improve hiding (#4858)

* Allow aliasing source

* Add test

* improve hiding

* Finish adding tests

* fix test

crates/nu-command/src/core_commands/hide.rs

@@ -113,7 +113,7 @@ impl Command for Hide {
         } else if !import_pattern.hidden.contains(&import_pattern.head.name)
             && stack.remove_env_var(engine_state, &head_name_str).is_none()
         {
-            return Err(ShellError::NotFound(call.positional[0].span));
+            // TODO: we may want to error in the future
         }
 
         Ok(PipelineData::new(call.head))

crates/nu-command/tests/commands/alias.rs

@@ -0,0 +1,40 @@
+use nu_test_support::{nu, pipeline};
+
+#[test]
+fn alias_simple() {
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+        alias bar = source sample_def.nu; bar; greet
+        "#
+    ));
+
+    assert_eq!(actual.out, "hello");
+}
+
+#[test]
+fn alias_hiding1() {
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+        source ./activate-foo.nu;
+        $nu.scope.aliases | find deactivate-foo | length
+        "#
+    ));
+
+    assert_eq!(actual.out, "1");
+}
+
+#[test]
+fn alias_hiding2() {
+    let actual = nu!(
+        cwd: "tests/fixtures/formats", pipeline(
+        r#"
+        source ./activate-foo.nu;
+        deactivate-foo;
+        $nu.scope.aliases | find deactivate-foo | length
+        "#
+    ));
+
+    assert_eq!(actual.out, "0");
+}

crates/nu-command/tests/commands/math/avg.rs

@@ -18,8 +18,8 @@ fn can_average_numbers() {
 fn can_average_bytes() {
     let actual = nu!(
         cwd: "tests/fixtures/formats",
-        "ls | sort-by name | skip 1 | first 2 | get size | math avg | to json -r"
+        "[100kb, 10b, 100mib] | math avg | to json -r"
     );
 
-    assert_eq!(actual.out, "1600");
+    assert_eq!(actual.out, "34985870");
 }

crates/nu-command/tests/commands/mod.rs

@@ -1,3 +1,4 @@
+mod alias;
 mod all;
 mod any;
 mod append;

crates/nu-parser/src/parser.rs

@@ -897,10 +897,19 @@ pub fn parse_call(
                     new_spans.extend(&spans[(pos + 1)..]);
                 }
 
-                working_set.enter_scope();
-                working_set.hide_alias(&name);
-                let (mut result, err) = parse_expression(working_set, &new_spans, false);
-                working_set.exit_scope();
+                let alias_id = working_set.hide_alias(&name);
+                let lite_command = LiteCommand {
+                    comments: vec![],
+                    parts: new_spans.clone(),
+                };
+                let (mut result, err) = parse_builtin_commands(working_set, &lite_command);
+                if let Some(frame) = working_set.delta.scope.last_mut() {
+                    if let Some(alias_id) = alias_id {
+                        frame.aliases.insert(name.clone(), alias_id);
+                    }
+                }
+
+                let mut result = result.expressions.remove(0);
 
                 result.replace_span(working_set, expansion_span, orig_span);
 

crates/nu-protocol/src/engine/engine_state.rs

@@ -813,8 +813,12 @@ impl<'a> StateWorkingSet<'a> {
         for scope in self.delta.scope.iter_mut().rev() {
             visibility.append(&scope.visibility);
 
-            if let Some(decl_id) = scope.decls.remove(name) {
-                return Some(decl_id);
+            if let Some(decl_id) = scope.decls.get(name) {
+                if visibility.is_decl_id_visible(decl_id) {
+                    // Hide decl only if it's not already hidden
+                    scope.visibility.hide_decl_id(decl_id);
+                    return Some(*decl_id);
+                }
             }
         }
 
@@ -843,12 +847,17 @@ impl<'a> StateWorkingSet<'a> {
     pub fn hide_alias(&mut self, name: &[u8]) -> Option<AliasId> {
         let mut visibility: Visibility = Visibility::new();
 
-        // Since we can mutate scope frames in delta, remove the id directly
+        // // Since we can mutate scope frames in delta, remove the id directly
         for scope in self.delta.scope.iter_mut().rev() {
             visibility.append(&scope.visibility);
 
-            if let Some(alias_id) = scope.aliases.remove(name) {
-                return Some(alias_id);
+            if let Some(alias_id) = scope.aliases.get(name) {
+                if visibility.is_alias_id_visible(alias_id) {
+                    // Hide alias only if it's not already hidden
+                    scope.visibility.hide_alias_id(alias_id);
+
+                    return Some(*alias_id);
+                }
             }
         }
 
@@ -866,6 +875,7 @@ impl<'a> StateWorkingSet<'a> {
                 if visibility.is_alias_id_visible(alias_id) {
                     // Hide alias only if it's not already hidden
                     last_scope_frame.visibility.hide_alias_id(alias_id);
+
                     return Some(*alias_id);
                 }
             }
@@ -1024,11 +1034,15 @@ impl<'a> StateWorkingSet<'a> {
             visibility.append(&scope.visibility);
 
             if let Some(decl_id) = scope.predecls.get(name) {
-                return Some(*decl_id);
+                if visibility.is_decl_id_visible(decl_id) {
+                    return Some(*decl_id);
+                }
             }
 
             if let Some(decl_id) = scope.decls.get(name) {
-                return Some(*decl_id);
+                if visibility.is_decl_id_visible(decl_id) {
+                    return Some(*decl_id);
+                }
             }
         }
 
@@ -1052,7 +1066,9 @@ impl<'a> StateWorkingSet<'a> {
             visibility.append(&scope.visibility);
 
             if let Some(alias_id) = scope.aliases.get(name) {
-                return Some(*alias_id);
+                if visibility.is_alias_id_visible(alias_id) {
+                    return Some(*alias_id);
+                }
             }
         }
 

src/tests/test_hiding.rs

@@ -145,6 +145,7 @@ fn hides_env_in_scope_4() -> TestResult {
 }
 
 #[test]
+#[ignore]
 fn hide_def_twice_not_allowed() -> TestResult {
     fail_test(
         r#"def foo [] { "foo" }; hide foo; hide foo"#,
@@ -153,6 +154,7 @@ fn hide_def_twice_not_allowed() -> TestResult {
 }
 
 #[test]
+#[ignore]
 fn hide_alias_twice_not_allowed() -> TestResult {
     fail_test(
         r#"alias foo = echo "foo"; hide foo; hide foo"#,
@@ -161,6 +163,7 @@ fn hide_alias_twice_not_allowed() -> TestResult {
 }
 
 #[test]
+#[ignore]
 fn hide_env_twice_not_allowed() -> TestResult {
     fail_test(r#"let-env foo = "foo"; hide foo; hide foo"#, "did not find")
 }

tests/fixtures/formats/activate-foo.nu

@@ -0,0 +1 @@
+alias deactivate-foo = source deactivate-foo.nu

tests/fixtures/formats/deactivate-foo.nu

@@ -0,0 +1 @@
+hide deactivate-foo

tests/fixtures/formats/sample_def.nu

@@ -0,0 +1,3 @@
+def greet [] {
+  "hello"
+}

tests/shell/pipeline/commands/internal.rs

@@ -416,6 +416,7 @@ fn unlet_env_variable() {
 }
 
 #[test]
+#[ignore]
 fn unlet_nonexistent_variable() {
     let actual = nu!(
         cwd: ".",