Add environment variable support for modules (#331)

* Add 'expor env' dummy command

* (WIP) Abstract away module exportables as Overlay

* Switch to Overlays for use/hide

Works for decls only right now.

* Fix passing import patterns of hide to eval

* Simplify use/hide of decls

* Add ImportPattern as Expr; Add use env eval

Still no parsing of "export env" so I can't test it yet.

* Refactor export parsing; Add InternalError

* Add env var export and activation; Misc changes

Now it is possible to `use` env var that was exported from a module.

This commit also adds some new errors and other small changes.

* Add env var hiding

* Fix eval not recognizing hidden decls

Without this change, calling `hide foo`, the evaluator does not know
whether a custom command named "foo" was hidden during parsing,
therefore, it is not possible to reliably throw an error about the "foo"
name not found.

* Add use/hide/export env var tests; Cleanup; Notes

* Ignore hide env related tests for now

* Fix main branch merge mess

* Fixed multi-word export def

* Fix hiding tests on Windows

* Remove env var hiding for now

crates/nu-protocol/src/ast/block.rs

@@ -1,6 +1,6 @@
 use std::ops::{Index, IndexMut};
 
-use crate::{DeclId, Signature, VarId};
+use crate::{Overlay, Signature, VarId};
 
 use super::Statement;
 
@@ -8,7 +8,7 @@ use super::Statement;
 pub struct Block {
     pub signature: Box<Signature>,
     pub stmts: Vec<Statement>,
-    pub exports: Vec<(Vec<u8>, DeclId)>, // Assuming just defs for now
+    pub overlay: Overlay,
     pub captures: Vec<VarId>,
 }
 
@@ -47,16 +47,16 @@ impl Block {
         Self {
             signature: Box::new(Signature::new("")),
             stmts: vec![],
-            exports: vec![],
+            overlay: Overlay::new(),
             captures: vec![],
         }
     }
 
-    pub fn with_exports(self, exports: Vec<(Vec<u8>, DeclId)>) -> Self {
+    pub fn with_overlay(self, overlay: Overlay) -> Self {
         Self {
             signature: self.signature,
             stmts: self.stmts,
-            exports,
+            overlay,
             captures: self.captures,
         }
     }
@@ -70,7 +70,7 @@ where
         Self {
             signature: Box::new(Signature::new("")),
             stmts: stmts.collect(),
-            exports: vec![],
+            overlay: Overlay::new(),
             captures: vec![],
         }
     }

crates/nu-protocol/src/ast/expr.rs

@@ -1,5 +1,5 @@
 use super::{Call, CellPath, Expression, FullCellPath, Operator, RangeOperator};
-use crate::{BlockId, Signature, Span, Spanned, Unit, VarId};
+use crate::{ast::ImportPattern, BlockId, Signature, Span, Spanned, Unit, VarId};
 
 #[derive(Debug, Clone)]
 pub enum Expr {
@@ -31,6 +31,7 @@ pub enum Expr {
     String(String),
     CellPath(CellPath),
     FullCellPath(Box<FullCellPath>),
+    ImportPattern(ImportPattern),
     Signature(Box<Signature>),
     Garbage,
 }

crates/nu-protocol/src/ast/expression.rs

@@ -131,6 +131,7 @@ impl Expression {
                 }
                 false
             }
+            Expr::ImportPattern(_) => false,
             Expr::Filepath(_) => false,
             Expr::Float(_) => false,
             Expr::FullCellPath(full_cell_path) => {
@@ -282,6 +283,7 @@ impl Expression {
                     .head
                     .replace_in_variable(working_set, new_var_id);
             }
+            Expr::ImportPattern(_) => {}
             Expr::Garbage => {}
             Expr::GlobPattern(_) => {}
             Expr::Int(_) => {}

crates/nu-protocol/src/ast/import_pattern.rs

@@ -1,4 +1,4 @@
-use crate::Span;
+use crate::{span, Span};
 
 #[derive(Debug, Clone)]
 pub enum ImportPatternMember {
@@ -7,8 +7,34 @@ pub enum ImportPatternMember {
     List { names: Vec<(Vec<u8>, Span)> },
 }
 
+#[derive(Debug, Clone)]
+pub struct ImportPatternHead {
+    pub name: Vec<u8>,
+    pub span: Span,
+}
+
 #[derive(Debug, Clone)]
 pub struct ImportPattern {
-    pub head: Vec<u8>,
+    pub head: ImportPatternHead,
     pub members: Vec<ImportPatternMember>,
 }
+
+impl ImportPattern {
+    pub fn span(&self) -> Span {
+        let mut spans = vec![self.head.span];
+
+        for member in &self.members {
+            match member {
+                ImportPatternMember::Glob { span } => spans.push(*span),
+                ImportPatternMember::Name { name: _, span } => spans.push(*span),
+                ImportPatternMember::List { names } => {
+                    for (_, span) in names {
+                        spans.push(*span);
+                    }
+                }
+            }
+        }
+
+        span(&spans)
+    }
+}

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

@@ -7,41 +7,41 @@ use std::{
 };
 
 // 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>,
+    decl_ids: HashMap<DeclId, bool>,
 }
 
 impl Visibility {
     fn new() -> Self {
         Visibility {
-            ids: HashMap::new(),
+            decl_ids: HashMap::new(),
         }
     }
 
-    fn is_id_visible(&self, id: &DeclId) -> bool {
-        *self.ids.get(id).unwrap_or(&true) // by default it's visible
+    fn is_decl_id_visible(&self, decl_id: &DeclId) -> bool {
+        *self.decl_ids.get(decl_id).unwrap_or(&true) // by default it's visible
     }
 
-    fn hide_id(&mut self, id: &DeclId) {
-        self.ids.insert(*id, false);
+    fn hide_decl_id(&mut self, decl_id: &DeclId) {
+        self.decl_ids.insert(*decl_id, false);
     }
 
-    fn use_id(&mut self, id: &DeclId) {
-        self.ids.insert(*id, true);
+    fn use_decl_id(&mut self, decl_id: &DeclId) {
+        self.decl_ids.insert(*decl_id, true);
     }
 
     fn merge_with(&mut self, other: Visibility) {
         // overwrite own values with the other
-        self.ids.extend(other.ids);
+        self.decl_ids.extend(other.decl_ids);
+        // self.env_var_ids.extend(other.env_var_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);
+        for (decl_id, visible) in other.decl_ids.iter() {
+            if !self.decl_ids.contains_key(decl_id) {
+                self.decl_ids.insert(*decl_id, *visible);
             }
         }
     }
@@ -53,6 +53,7 @@ pub struct ScopeFrame {
     predecls: HashMap<Vec<u8>, DeclId>, // temporary storage for predeclarations
     pub decls: HashMap<Vec<u8>, DeclId>,
     pub aliases: HashMap<Vec<u8>, Vec<Span>>,
+    pub env_vars: HashMap<Vec<u8>, BlockId>,
     pub modules: HashMap<Vec<u8>, BlockId>,
     visibility: Visibility,
 }
@@ -64,6 +65,7 @@ impl ScopeFrame {
             predecls: HashMap::new(),
             decls: HashMap::new(),
             aliases: HashMap::new(),
+            env_vars: HashMap::new(),
             modules: HashMap::new(),
             visibility: Visibility::new(),
         }
@@ -232,7 +234,7 @@ impl EngineState {
             visibility.append(&scope.visibility);
 
             if let Some(decl_id) = scope.decls.get(name) {
-                if visibility.is_id_visible(decl_id) {
+                if visibility.is_decl_id_visible(decl_id) {
                     return Some(*decl_id);
                 }
             }
@@ -241,6 +243,16 @@ impl EngineState {
         None
     }
 
+    pub fn find_module(&self, name: &[u8]) -> Option<BlockId> {
+        for scope in self.scope.iter().rev() {
+            if let Some(block_id) = scope.modules.get(name) {
+                return Some(*block_id);
+            }
+        }
+
+        None
+    }
+
     pub fn find_commands_by_prefix(&self, name: &[u8]) -> Vec<Vec<u8>> {
         let mut output = vec![];
 
@@ -457,11 +469,24 @@ 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);
+        scope_frame.visibility.use_decl_id(&decl_id);
 
         decl_id
     }
 
+    pub fn add_decls(&mut self, decls: Vec<(Vec<u8>, DeclId)>) {
+        let scope_frame = self
+            .delta
+            .scope
+            .last_mut()
+            .expect("internal error: missing required scope frame");
+
+        for (name, decl_id) in decls {
+            scope_frame.decls.insert(name, decl_id);
+            scope_frame.visibility.use_decl_id(&decl_id);
+        }
+    }
+
     pub fn add_predecl(&mut self, decl: Box<dyn Command>) -> Option<DeclId> {
         let name = decl.name().as_bytes().to_vec();
 
@@ -486,7 +511,7 @@ impl<'a> StateWorkingSet<'a> {
 
         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);
+            scope_frame.visibility.use_decl_id(&decl_id);
 
             return Some(decl_id);
         }
@@ -517,9 +542,9 @@ impl<'a> StateWorkingSet<'a> {
             visibility.append(&scope.visibility);
 
             if let Some(decl_id) = scope.decls.get(name) {
-                if visibility.is_id_visible(decl_id) {
+                if visibility.is_decl_id_visible(decl_id) {
                     // Hide decl only if it's not already hidden
-                    last_scope_frame.visibility.hide_id(decl_id);
+                    last_scope_frame.visibility.hide_decl_id(decl_id);
                     return Some(*decl_id);
                 }
             }
@@ -528,12 +553,34 @@ impl<'a> StateWorkingSet<'a> {
         None
     }
 
+    pub fn hide_decls(&mut self, decls: &[(Vec<u8>, DeclId)]) {
+        for decl in decls.iter() {
+            self.hide_decl(&decl.0); // let's assume no errors
+        }
+    }
+
     pub fn add_block(&mut self, block: Block) -> BlockId {
         self.delta.blocks.push(block);
 
         self.num_blocks() - 1
     }
 
+    pub fn add_env_var(&mut self, name_span: Span, block: Block) -> BlockId {
+        self.delta.blocks.push(block);
+        let block_id = self.num_blocks() - 1;
+        let name = self.get_span_contents(name_span).to_vec();
+
+        let scope_frame = self
+            .delta
+            .scope
+            .last_mut()
+            .expect("internal error: missing required scope frame");
+
+        scope_frame.env_vars.insert(name, block_id);
+
+        block_id
+    }
+
     pub fn add_module(&mut self, name: &str, block: Block) -> BlockId {
         let name = name.as_bytes().to_vec();
 
@@ -551,19 +598,6 @@ impl<'a> StateWorkingSet<'a> {
         block_id
     }
 
-    pub fn activate_overlay(&mut self, overlay: Vec<(Vec<u8>, DeclId)>) {
-        let scope_frame = self
-            .delta
-            .scope
-            .last_mut()
-            .expect("internal error: missing required scope frame");
-
-        for (name, decl_id) in overlay {
-            scope_frame.decls.insert(name, decl_id);
-            scope_frame.visibility.use_id(&decl_id);
-        }
-    }
-
     pub fn next_span_start(&self) -> usize {
         let permanent_span_start = self.permanent_state.next_span_start();
 
@@ -665,7 +699,7 @@ impl<'a> StateWorkingSet<'a> {
             visibility.append(&scope.visibility);
 
             if let Some(decl_id) = scope.decls.get(name) {
-                if visibility.is_id_visible(decl_id) {
+                if visibility.is_decl_id_visible(decl_id) {
                     return Some(*decl_id);
                 }
             }

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

@@ -38,6 +38,7 @@ impl Stack {
             env_vars: HashMap::new(),
         }
     }
+
     pub fn get_var(&self, var_id: VarId) -> Result<Value, ShellError> {
         if let Some(v) = self.vars.get(&var_id) {
             return Ok(v.clone());
@@ -87,6 +88,10 @@ impl Stack {
         None
     }
 
+    pub fn remove_env_var(&mut self, name: &str) -> Option<String> {
+        self.env_vars.remove(name)
+    }
+
     pub fn get_config(&self) -> Result<Config, ShellError> {
         let config = self.get_var(CONFIG_VARIABLE_ID);
 

crates/nu-protocol/src/exportable.rs

@@ -0,0 +1,6 @@
+use crate::{BlockId, DeclId};
+
+pub enum Exportable {
+    Decl(DeclId),
+    EnvVar(BlockId),
+}

crates/nu-protocol/src/lib.rs

@@ -2,7 +2,9 @@ pub mod ast;
 mod config;
 pub mod engine;
 mod example;
+mod exportable;
 mod id;
+mod overlay;
 mod pipeline_data;
 mod shell_error;
 mod signature;
@@ -15,7 +17,9 @@ pub use value::Value;
 pub use config::*;
 pub use engine::{CONFIG_VARIABLE_ID, IN_VARIABLE_ID, NU_VARIABLE_ID, SCOPE_VARIABLE_ID};
 pub use example::*;
+pub use exportable::*;
 pub use id::*;
+pub use overlay::*;
 pub use pipeline_data::*;
 pub use shell_error::*;
 pub use signature::*;

crates/nu-protocol/src/overlay.rs

@@ -0,0 +1,121 @@
+use crate::{BlockId, DeclId};
+
+use std::collections::HashMap;
+
+// 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 Overlay {
+    pub decls: HashMap<Vec<u8>, DeclId>,
+    pub env_vars: HashMap<Vec<u8>, BlockId>,
+}
+
+impl Overlay {
+    pub fn new() -> Self {
+        Overlay {
+            decls: HashMap::new(),
+            env_vars: HashMap::new(),
+        }
+    }
+
+    pub fn add_decl(&mut self, name: &[u8], decl_id: DeclId) -> Option<DeclId> {
+        self.decls.insert(name.to_vec(), decl_id)
+    }
+
+    pub fn add_env_var(&mut self, name: &[u8], block_id: BlockId) -> Option<BlockId> {
+        self.env_vars.insert(name.to_vec(), block_id)
+    }
+
+    pub fn extend(&mut self, other: &Overlay) {
+        self.decls.extend(other.decls.clone());
+        self.env_vars.extend(other.env_vars.clone());
+    }
+
+    pub fn is_empty(&self) -> bool {
+        self.decls.is_empty() && self.env_vars.is_empty()
+    }
+
+    pub fn get_decl_id(&self, name: &[u8]) -> Option<DeclId> {
+        self.decls.get(name).copied()
+    }
+
+    pub fn has_decl(&self, name: &[u8]) -> bool {
+        self.decls.contains_key(name)
+    }
+
+    pub fn decl_with_head(&self, name: &[u8], head: &[u8]) -> Option<(Vec<u8>, DeclId)> {
+        if let Some(id) = self.get_decl_id(name) {
+            let mut new_name = head.to_vec();
+            new_name.push(b' ');
+            new_name.extend(name);
+            Some((new_name, id))
+        } else {
+            None
+        }
+    }
+
+    pub fn decls_with_head(&self, head: &[u8]) -> Vec<(Vec<u8>, DeclId)> {
+        self.decls
+            .iter()
+            .map(|(name, id)| {
+                let mut new_name = head.to_vec();
+                new_name.push(b' ');
+                new_name.extend(name);
+                (new_name, *id)
+            })
+            .collect()
+    }
+
+    pub fn decls(&self) -> Vec<(Vec<u8>, DeclId)> {
+        self.decls
+            .iter()
+            .map(|(name, id)| (name.clone(), *id))
+            .collect()
+    }
+
+    pub fn get_env_var_id(&self, name: &[u8]) -> Option<BlockId> {
+        self.env_vars.get(name).copied()
+    }
+
+    pub fn has_env_var(&self, name: &[u8]) -> bool {
+        self.env_vars.contains_key(name)
+    }
+
+    pub fn env_var_with_head(&self, name: &[u8], head: &[u8]) -> Option<(Vec<u8>, BlockId)> {
+        if let Some(id) = self.get_env_var_id(name) {
+            let mut new_name = head.to_vec();
+            new_name.push(b' ');
+            new_name.extend(name);
+            Some((new_name, id))
+        } else {
+            None
+        }
+    }
+
+    pub fn env_vars_with_head(&self, head: &[u8]) -> Vec<(Vec<u8>, BlockId)> {
+        self.env_vars
+            .iter()
+            .map(|(name, id)| {
+                let mut new_name = head.to_vec();
+                new_name.push(b' ');
+                new_name.extend(name);
+                (new_name, *id)
+            })
+            .collect()
+    }
+
+    pub fn env_vars(&self) -> Vec<(Vec<u8>, BlockId)> {
+        self.env_vars
+            .iter()
+            .map(|(name, id)| (name.clone(), *id))
+            .collect()
+    }
+}
+
+impl Default for Overlay {
+    fn default() -> Self {
+        Self::new()
+    }
+}

crates/nu-protocol/src/shell_error.rs

@@ -84,6 +84,18 @@ pub enum ShellError {
     #[diagnostic(code(nu::shell::variable_not_found), url(docsrs))]
     VariableNotFoundAtRuntime(#[label = "variable not found"] Span),
 
+    #[error("Environment variable not found")]
+    #[diagnostic(code(nu::shell::variable_not_found), url(docsrs))]
+    EnvVarNotFoundAtRuntime(#[label = "environment variable not found"] Span),
+
+    #[error("Environment variable is not a string")]
+    #[diagnostic(code(nu::shell::variable_not_found), url(docsrs))]
+    EnvVarNotAString(#[label = "does not evaluate to a string"] Span),
+
+    #[error("Not found.")]
+    #[diagnostic(code(nu::parser::not_found), url(docsrs))]
+    NotFound(#[label = "did not find anything under this name"] Span),
+
     #[error("Can't convert to {0}.")]
     #[diagnostic(code(nu::shell::cant_convert), url(docsrs))]
     CantConvert(String, String, #[label("can't convert {1} to {0}")] Span),
@@ -190,6 +202,10 @@ pub enum ShellError {
     #[error("Name not found")]
     #[diagnostic(code(nu::shell::name_not_found), url(docsrs))]
     DidYouMean(String, #[label("did you mean '{0}'?")] Span),
+
+    #[error("Non-UTF8 string.")]
+    #[diagnostic(code(nu::parser::non_utf8), url(docsrs))]
+    NonUtf8(#[label = "non-UTF8 string"] Span),
 }
 
 impl From<std::io::Error> for ShellError {