Merge branch 'nushell:main' into main

crates/nu-parser/src/parse_keywords.rs

@@ -110,8 +110,10 @@ pub fn parse_def(
                             *declaration = signature.into_block_command(block_id);
                         } else {
                             error = error.or_else(|| {
+                                // TODO: Add InternalError variant
                                 Some(ParseError::UnknownState(
-                                    "Could not define hidden command".into(),
+                                    "Internal error: Predeclaration failed to add declaration"
+                                        .into(),
                                     spans[1],
                                 ))
                             });
@@ -318,7 +320,7 @@ pub fn parse_module(
     spans: &[Span],
 ) -> (Statement, Option<ParseError>) {
     // TODO: Currently, module is closing over its parent scope (i.e., defs in the parent scope are
-    // visible and usable in this module's scope). We might want to disable that. How?
+    // visible and usable in this module's scope). We want to disable that for files.
 
     let mut error = None;
     let bytes = working_set.get_span_contents(spans[0]);

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

@@ -1,10 +1,7 @@
 use super::Command;
 use crate::{ast::Block, BlockId, DeclId, Example, Signature, Span, Type, VarId};
 use core::panic;
-use std::{
+use std::{collections::HashMap, slice::Iter};
-    collections::{HashMap, HashSet},
-    slice::Iter,
-};
 
 pub struct EngineState {
     files: Vec<(String, usize, usize)>,
@@ -15,23 +12,66 @@ pub struct EngineState {
     pub scope: Vec<ScopeFrame>,
 }
 
+// Tells whether a decl etc. is visible or not
+// TODO: When adding new exportables (env vars, aliases, etc.), parametrize the ID type with generics
+#[derive(Debug, Clone)]
+struct Visibility {
+    ids: HashMap<DeclId, bool>,
+}
+
+impl Visibility {
+    fn new() -> Self {
+        Visibility {
+            ids: HashMap::new(),
+        }
+    }
+
+    fn is_id_visible(&self, id: &DeclId) -> bool {
+        *self.ids.get(id).unwrap_or(&true) // by default it's visible
+    }
+
+    fn hide_id(&mut self, id: &DeclId) {
+        self.ids.insert(*id, false);
+    }
+
+    fn use_id(&mut self, id: &DeclId) {
+        self.ids.insert(*id, true);
+    }
+
+    fn merge_with(&mut self, other: Visibility) {
+        // overwrite own values with the other
+        self.ids.extend(other.ids);
+    }
+
+    fn append(&mut self, other: &Visibility) {
+        // take new values from other but keep own values
+        for (id, visible) in other.ids.iter() {
+            if !self.ids.contains_key(id) {
+                self.ids.insert(*id, *visible);
+            }
+        }
+    }
+}
+
 #[derive(Debug)]
 pub struct ScopeFrame {
     pub vars: HashMap<Vec<u8>, VarId>,
+    predecls: HashMap<Vec<u8>, DeclId>, // temporary storage for predeclarations
     decls: HashMap<Vec<u8>, DeclId>,
     aliases: HashMap<Vec<u8>, Vec<Span>>,
     modules: HashMap<Vec<u8>, BlockId>,
-    hiding: HashSet<DeclId>,
+    visibility: Visibility,
 }
 
 impl ScopeFrame {
     pub fn new() -> Self {
         Self {
             vars: HashMap::new(),
+            predecls: HashMap::new(),
             decls: HashMap::new(),
             aliases: HashMap::new(),
             modules: HashMap::new(),
-            hiding: HashSet::new(),
+            visibility: Visibility::new(),
         }
     }
 
@@ -86,9 +126,7 @@ impl EngineState {
             for item in first.modules.into_iter() {
                 last.modules.insert(item.0, item.1);
             }
-            for item in first.hiding.into_iter() {
+            last.visibility.merge_with(first.visibility);
-                last.hiding.insert(item);
-            }
         }
     }
 
@@ -132,13 +170,13 @@ impl EngineState {
     }
 
     pub fn find_decl(&self, name: &[u8]) -> Option<DeclId> {
-        let mut hiding: HashSet<DeclId> = HashSet::new();
+        let mut visibility: Visibility = Visibility::new();
 
         for scope in self.scope.iter().rev() {
-            hiding.extend(&scope.hiding);
+            visibility.append(&scope.visibility);
 
             if let Some(decl_id) = scope.decls.get(name) {
-                if !hiding.contains(decl_id) {
+                if visibility.is_id_visible(decl_id) {
                     return Some(*decl_id);
                 }
             }
@@ -270,7 +308,6 @@ pub struct StateDelta {
     vars: Vec<Type>,              // indexed by VarId
     decls: Vec<Box<dyn Command>>, // indexed by DeclId
     blocks: Vec<Block>,           // indexed by BlockId
-    predecls: HashMap<Vec<u8>, DeclId>, // this should get erased after every def call
     pub scope: Vec<ScopeFrame>,
 }
 
@@ -304,7 +341,6 @@ impl<'a> StateWorkingSet<'a> {
                 file_contents: vec![],
                 vars: vec![],
                 decls: vec![],
-                predecls: HashMap::new(),
                 blocks: vec![],
                 scope: vec![ScopeFrame::new()],
             },
@@ -337,6 +373,7 @@ impl<'a> StateWorkingSet<'a> {
             .expect("internal error: missing required scope frame");
 
         scope_frame.decls.insert(name, decl_id);
+        scope_frame.visibility.use_id(&decl_id);
 
         decl_id
     }
@@ -347,18 +384,25 @@ impl<'a> StateWorkingSet<'a> {
         self.delta.decls.push(decl);
         let decl_id = self.num_decls() - 1;
 
-        self.delta.predecls.insert(name, decl_id)
-    }
-
-    pub fn merge_predecl(&mut self, name: &[u8]) -> Option<DeclId> {
-        if let Some(decl_id) = self.delta.predecls.remove(name) {
         let scope_frame = self
             .delta
             .scope
             .last_mut()
             .expect("internal error: missing required scope frame");
 
+        scope_frame.predecls.insert(name, decl_id)
+    }
+
+    pub fn merge_predecl(&mut self, name: &[u8]) -> Option<DeclId> {
+        let scope_frame = self
+            .delta
+            .scope
+            .last_mut()
+            .expect("internal error: missing required scope frame");
+
+        if let Some(decl_id) = scope_frame.predecls.remove(name) {
             scope_frame.decls.insert(name.into(), decl_id);
+            scope_frame.visibility.use_id(&decl_id);
 
             return Some(decl_id);
         }
@@ -367,11 +411,11 @@ impl<'a> StateWorkingSet<'a> {
     }
 
     pub fn hide_decl(&mut self, name: &[u8]) -> Option<DeclId> {
-        let mut hiding: HashSet<DeclId> = HashSet::new();
+        let mut visibility: Visibility = Visibility::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);
+            visibility.append(&scope.visibility);
 
             if let Some(decl_id) = scope.decls.remove(name) {
                 return Some(decl_id);
@@ -386,12 +430,12 @@ impl<'a> StateWorkingSet<'a> {
             .expect("internal error: missing required scope frame");
 
         for scope in self.permanent_state.scope.iter().rev() {
-            hiding.extend(&scope.hiding);
+            visibility.append(&scope.visibility);
 
             if let Some(decl_id) = scope.decls.get(name) {
-                if !hiding.contains(decl_id) {
+                if visibility.is_id_visible(decl_id) {
                     // Hide decl only if it's not already hidden
-                    last_scope_frame.hiding.insert(*decl_id);
+                    last_scope_frame.visibility.hide_id(decl_id);
                     return Some(*decl_id);
                 }
             }
@@ -432,6 +476,7 @@ impl<'a> StateWorkingSet<'a> {
 
         for (name, decl_id) in overlay {
             scope_frame.decls.insert(name, decl_id);
+            scope_frame.visibility.use_id(&decl_id);
         }
     }
 
@@ -505,14 +550,14 @@ impl<'a> StateWorkingSet<'a> {
     }
 
     pub fn find_decl(&self, name: &[u8]) -> Option<DeclId> {
-        let mut hiding: HashSet<DeclId> = HashSet::new();
+        let mut visibility: Visibility = Visibility::new();
-
-        if let Some(decl_id) = self.delta.predecls.get(name) {
-            return Some(*decl_id);
-        }
 
         for scope in self.delta.scope.iter().rev() {
-            hiding.extend(&scope.hiding);
+            visibility.append(&scope.visibility);
+
+            if let Some(decl_id) = scope.predecls.get(name) {
+                return Some(*decl_id);
+            }
 
             if let Some(decl_id) = scope.decls.get(name) {
                 return Some(*decl_id);
@@ -520,10 +565,10 @@ impl<'a> StateWorkingSet<'a> {
         }
 
         for scope in self.permanent_state.scope.iter().rev() {
-            hiding.extend(&scope.hiding);
+            visibility.append(&scope.visibility);
 
             if let Some(decl_id) = scope.decls.get(name) {
-                if !hiding.contains(decl_id) {
+                if visibility.is_id_visible(decl_id) {
                     return Some(*decl_id);
                 }
             }

src/tests.rs

@@ -399,6 +399,7 @@ fn module_import_uses_internal_command() -> 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"#, not_found_msg())
@@ -500,7 +501,6 @@ fn def_twice_should_fail() -> TestResult {
     )
 }
 
-// TODO: This test fails if executed each command on a separate line in REPL
 #[test]
 fn use_import_after_hide() -> TestResult {
     run_test(
@@ -509,6 +509,22 @@ fn use_import_after_hide() -> TestResult {
     )
 }
 
+#[test]
+fn hide_shadowed_decl() -> TestResult {
+    run_test(
+        r#"module spam { export def foo [] { "bar" } }; def foo [] { "foo" }; do { use spam.foo; hide foo; foo }"#,
+        "foo",
+    )
+}
+
+#[test]
+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"#,
+        not_found_msg(),
+    )
+}
+
 #[test]
 fn from_json_1() -> TestResult {
     run_test(r#"('{"name": "Fred"}' | from json).name"#, "Fred")