From fb0f83e5742103a53a28f312b1d0f94f03a0004d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= <kubouch@gmail.com>
Date: Sat, 2 Oct 2021 00:12:30 +0300
Subject: [PATCH] Disallow hiding the same def twice; Add tests

Tests got removed after rebase.
---
 crates/nu-protocol/src/engine/engine_state.rs | 13 +++-
 src/tests.rs                                  | 64 +++++++++++++++++++
 2 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs
index 0653ed73c..ea75692eb 100644
--- a/crates/nu-protocol/src/engine/engine_state.rs
+++ b/crates/nu-protocol/src/engine/engine_state.rs
@@ -337,8 +337,12 @@ impl<'a> StateWorkingSet<'a> {
     }
 
     pub fn hide_decl(&mut self, name: &[u8]) -> Option<DeclId> {
+        let mut hiding: HashSet<DeclId> = HashSet::new();
+
         // Since we can mutate scope frames in delta, remove the id directly
         for scope in self.delta.scope.iter_mut().rev() {
+            hiding.extend(&scope.hiding);
+
             if let Some(decl_id) = scope.decls.remove(name) {
                 return Some(decl_id);
             }
@@ -352,9 +356,14 @@ impl<'a> StateWorkingSet<'a> {
             .expect("internal error: missing required scope frame");
 
         for scope in self.permanent_state.scope.iter().rev() {
+            hiding.extend(&scope.hiding);
+
             if let Some(decl_id) = scope.decls.get(name) {
-                last_scope_frame.hiding.insert(*decl_id);
-                return Some(*decl_id);
+                if !hiding.contains(decl_id) {
+                    // Do not hide already hidden decl
+                    last_scope_frame.hiding.insert(*decl_id);
+                    return Some(*decl_id);
+                }
             }
         }
 
diff --git a/src/tests.rs b/src/tests.rs
index 77e888a89..c947d1a22 100644
--- a/src/tests.rs
+++ b/src/tests.rs
@@ -383,6 +383,70 @@ fn module_imports_5() -> TestResult {
     )
 }
 
+#[test]
+fn module_import_uses_internal_command() -> TestResult {
+    run_test(
+        r#"module foo { def b [] { 2 }; export def a [] { b }  }; use foo; foo.a"#,
+        "2",
+    )
+}
+
+#[test]
+fn hides_def() -> TestResult {
+    fail_test(
+        r#"def foo [] { "foo" }; hide foo; foo"#,
+        "command not found",
+    )
+}
+
+#[test]
+fn hides_def_then_redefines() -> TestResult {
+    fail_test(
+        r#"def foo [] { "foo" }; hide foo; def foo [] { "bar" }; foo"#,
+        "defined more than once",
+    )
+}
+
+#[test]
+fn hides_def_in_scope_1() -> TestResult {
+    fail_test(
+        r#"def foo [] { "foo" }; do { hide foo; foo }"#,
+        "command not found",
+    )
+}
+
+#[test]
+fn hides_def_in_scope_2() -> TestResult {
+    run_test(
+        r#"def foo [] { "foo" }; do { def foo [] { "bar" }; hide foo; foo }"#,
+        "foo",
+    )
+}
+
+#[test]
+fn hides_def_in_scope_3() -> TestResult {
+    fail_test(
+        r#"def foo [] { "foo" }; do { hide foo; def foo [] { "bar" }; hide foo; foo }"#,
+        "command not found",
+    )
+}
+
+#[test]
+fn hides_def_in_scope_4() -> TestResult {
+    fail_test(
+        r#"def foo [] { "foo" }; do { def foo [] { "bar" }; hide foo; hide foo; foo }"#,
+        "command not found",
+    )
+}
+
+#[test]
+fn hide_twice_not_allowed() -> TestResult {
+    fail_test(
+        r#"def foo [] { "foo" }; hide foo; hide foo"#,
+        "unknown command",
+    )
+}
+
 #[test]
 fn from_json_1() -> TestResult {
     run_test(r#"('{"name": "Fred"}' | from json).name"#, "Fred")