Cleanup parsing of use and hide commands (#705)

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

@@ -12,7 +12,7 @@ impl Command for Hide {
 
     fn signature(&self) -> nu_protocol::Signature {
         Signature::build("hide")
-            .required("pattern", SyntaxShape::String, "import pattern")
+            .required("pattern", SyntaxShape::ImportPattern, "import pattern")
             .category(Category::Core)
     }
 
@@ -68,7 +68,10 @@ impl Command for Hide {
                         {
                             output.push((name, id));
                         } else if !overlay.has_decl(name) {
-                            return Err(ShellError::EnvVarNotFoundAtRuntime(*span));
+                            return Err(ShellError::EnvVarNotFoundAtRuntime(
+                                String::from_utf8_lossy(name).into(),
+                                *span,
+                            ));
                         }
 
                         output
@@ -82,7 +85,10 @@ impl Command for Hide {
                             {
                                 output.push((name, id));
                             } else if !overlay.has_decl(name) {
-                                return Err(ShellError::EnvVarNotFoundAtRuntime(*span));
+                                return Err(ShellError::EnvVarNotFoundAtRuntime(
+                                    String::from_utf8_lossy(name).into(),
+                                    *span,
+                                ));
                             }
                         }
 

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

@@ -17,7 +17,7 @@ impl Command for Use {
 
     fn signature(&self) -> nu_protocol::Signature {
         Signature::build("use")
-            .rest("pattern", SyntaxShape::String, "import pattern parts")
+            .required("pattern", SyntaxShape::ImportPattern, "import pattern")
             .category(Category::Core)
     }
 
@@ -56,7 +56,10 @@ impl Command for Use {
                         if let Some(id) = overlay.get_env_var_id(name) {
                             output.push((name.clone(), id));
                         } else if !overlay.has_decl(name) {
-                            return Err(ShellError::EnvVarNotFoundAtRuntime(*span));
+                            return Err(ShellError::EnvVarNotFoundAtRuntime(
+                                String::from_utf8_lossy(name).into(),
+                                *span,
+                            ));
                         }
 
                         output
@@ -68,7 +71,10 @@ impl Command for Use {
                             if let Some(id) = overlay.get_env_var_id(name) {
                                 output.push((name.clone(), id));
                             } else if !overlay.has_decl(name) {
-                                return Err(ShellError::EnvVarNotFoundAtRuntime(*span));
+                                return Err(ShellError::EnvVarNotFoundAtRuntime(
+                                    String::from_utf8_lossy(name).into(),
+                                    *span,
+                                ));
                             }
                         }
 
@@ -94,7 +100,17 @@ impl Command for Use {
                 stack.add_env_var(name, val);
             }
         } else {
-            return Err(ShellError::EnvVarNotFoundAtRuntime(call.positional[0].span));
+            // TODO: This is a workaround since call.positional[0].span points at 0 for some reason
+            // when this error is triggered
+            let bytes = engine_state.get_span_contents(&call.positional[0].span);
+            return Err(ShellError::SpannedLabeledError(
+                format!(
+                    "Could not use '{}' import pattern",
+                    String::from_utf8_lossy(bytes)
+                ),
+                "called here".to_string(),
+                call.head,
+            ));
         }
 
         Ok(PipelineData::new(call.head))

crates/nu-parser/src/parse_keywords.rs

@@ -13,8 +13,8 @@ use crate::{
     lex, lite_parse,
     parser::{
         check_call, check_name, garbage, garbage_statement, parse, parse_block_expression,
-        parse_import_pattern, parse_internal_call, parse_multispan_value, parse_signature,
-        parse_string, parse_var_with_opt_type, trim_quotes,
+        parse_internal_call, parse_multispan_value, parse_signature, parse_string,
+        parse_var_with_opt_type, trim_quotes,
     },
     ParseError,
 };
@@ -636,161 +636,279 @@ pub fn parse_use(
     working_set: &mut StateWorkingSet,
     spans: &[Span],
 ) -> (Statement, Option<ParseError>) {
-    let mut error = None;
-    let bytes = working_set.get_span_contents(spans[0]);
+    if working_set.get_span_contents(spans[0]) != b"use" {
+        return (
+            garbage_statement(spans),
+            Some(ParseError::UnknownState(
+                "internal error: Wrong call name for 'use' command".into(),
+                span(spans),
+            )),
+        );
+    }
 
-    if bytes == b"use" && spans.len() >= 2 {
-        let cwd = working_set.get_cwd();
-        for span in spans[1..].iter() {
-            let (_, err) = parse_string(working_set, *span);
-            error = error.or(err);
+    let (call, call_span, use_decl_id) = match working_set.find_decl(b"use") {
+        Some(decl_id) => {
+            let (call, mut err) = parse_internal_call(working_set, spans[0], &spans[1..], decl_id);
+            let decl = working_set.get_decl(decl_id);
+
+            let call_span = span(spans);
+
+            err = check_call(call_span, &decl.signature(), &call).or(err);
+            if err.is_some() || call.has_flag("help") {
+                return (
+                    Statement::Pipeline(Pipeline::from_vec(vec![Expression {
+                        expr: Expr::Call(call),
+                        span: call_span,
+                        ty: Type::Unknown,
+                        custom_completion: None,
+                    }])),
+                    err,
+                );
+            }
+
+            (call, call_span, decl_id)
         }
+        None => {
+            return (
+                garbage_statement(spans),
+                Some(ParseError::UnknownState(
+                    "internal error: 'use' declaration not found".into(),
+                    span(spans),
+                )),
+            )
+        }
+    };
 
-        // TODO: Add checking for importing too long import patterns, e.g.:
-        // > use spam foo non existent names here do not throw error
-        let (import_pattern, err) = parse_import_pattern(working_set, &spans[1..]);
-        error = error.or(err);
+    let import_pattern = if let Some(expr) = call.nth(0) {
+        if let Some(pattern) = expr.as_import_pattern() {
+            pattern
+        } else {
+            return (
+                garbage_statement(spans),
+                Some(ParseError::UnknownState(
+                    "internal error: Import pattern positional is not import pattern".into(),
+                    call_span,
+                )),
+            );
+        }
+    } else {
+        return (
+            garbage_statement(spans),
+            Some(ParseError::UnknownState(
+                "internal error: Missing required positional after call parsing".into(),
+                call_span,
+            )),
+        );
+    };
 
-        let (import_pattern, overlay) =
-            if let Some(overlay_id) = working_set.find_overlay(&import_pattern.head.name) {
-                (import_pattern, working_set.get_overlay(overlay_id).clone())
-            } else {
-                // TODO: Do not close over when loading module from file
-                // It could be a file
-                if let Ok(module_filename) = String::from_utf8(import_pattern.head.name) {
-                    if let Ok(module_path) = canonicalize_with(&module_filename, cwd) {
-                        let module_name = if let Some(stem) = module_path.file_stem() {
-                            stem.to_string_lossy().to_string()
-                        } else {
-                            return (
-                                garbage_statement(spans),
-                                Some(ParseError::ModuleNotFound(spans[1])),
-                            );
-                        };
+    let cwd = working_set.get_cwd();
 
-                        if let Ok(contents) = std::fs::read(module_path) {
-                            let span_start = working_set.next_span_start();
-                            working_set.add_file(module_filename, &contents);
-                            let span_end = working_set.next_span_start();
+    let mut error = None;
 
-                            let (block, overlay, err) =
-                                parse_module_block(working_set, Span::new(span_start, span_end));
-                            error = error.or(err);
-
-                            let _ = working_set.add_block(block);
-                            let _ = working_set.add_overlay(&module_name, overlay.clone());
-
-                            (
-                                ImportPattern {
-                                    head: ImportPatternHead {
-                                        name: module_name.into(),
-                                        span: spans[1],
-                                    },
-                                    members: import_pattern.members,
-                                    hidden: HashSet::new(),
-                                },
-                                overlay,
-                            )
-                        } else {
-                            return (
-                                garbage_statement(spans),
-                                Some(ParseError::ModuleNotFound(spans[1])),
-                            );
-                        }
+    // TODO: Add checking for importing too long import patterns, e.g.:
+    // > use spam foo non existent names here do not throw error
+    let (import_pattern, overlay) =
+        if let Some(overlay_id) = working_set.find_overlay(&import_pattern.head.name) {
+            (import_pattern, working_set.get_overlay(overlay_id).clone())
+        } else {
+            // TODO: Do not close over when loading module from file
+            // It could be a file
+            if let Ok(module_filename) = String::from_utf8(import_pattern.head.name) {
+                if let Ok(module_path) = canonicalize_with(&module_filename, cwd) {
+                    let module_name = if let Some(stem) = module_path.file_stem() {
+                        stem.to_string_lossy().to_string()
                     } else {
-                        error = error.or(Some(ParseError::FileNotFound(
-                            module_filename,
-                            import_pattern.head.span,
-                        )));
-                        (ImportPattern::new(), Overlay::new())
+                        return (
+                            Statement::Pipeline(Pipeline::from_vec(vec![Expression {
+                                expr: Expr::Call(call),
+                                span: call_span,
+                                ty: Type::Unknown,
+                                custom_completion: None,
+                            }])),
+                            Some(ParseError::ModuleNotFound(spans[1])),
+                        );
+                    };
+
+                    if let Ok(contents) = std::fs::read(module_path) {
+                        let span_start = working_set.next_span_start();
+                        working_set.add_file(module_filename, &contents);
+                        let span_end = working_set.next_span_start();
+
+                        let (block, overlay, err) =
+                            parse_module_block(working_set, Span::new(span_start, span_end));
+                        error = error.or(err);
+
+                        let _ = working_set.add_block(block);
+                        let _ = working_set.add_overlay(&module_name, overlay.clone());
+
+                        (
+                            ImportPattern {
+                                head: ImportPatternHead {
+                                    name: module_name.into(),
+                                    span: spans[1],
+                                },
+                                members: import_pattern.members,
+                                hidden: HashSet::new(),
+                            },
+                            overlay,
+                        )
+                    } else {
+                        return (
+                            Statement::Pipeline(Pipeline::from_vec(vec![Expression {
+                                expr: Expr::Call(call),
+                                span: call_span,
+                                ty: Type::Unknown,
+                                custom_completion: None,
+                            }])),
+                            Some(ParseError::ModuleNotFound(spans[1])),
+                        );
                     }
                 } else {
-                    return (
-                        garbage_statement(spans),
-                        Some(ParseError::NonUtf8(spans[1])),
-                    );
-                }
-            };
-
-        let decls_to_use = if import_pattern.members.is_empty() {
-            overlay.decls_with_head(&import_pattern.head.name)
-        } else {
-            match &import_pattern.members[0] {
-                ImportPatternMember::Glob { .. } => overlay.decls(),
-                ImportPatternMember::Name { name, span } => {
-                    let mut output = vec![];
-
-                    if let Some(id) = overlay.get_decl_id(name) {
-                        output.push((name.clone(), id));
-                    } else if !overlay.has_env_var(name) {
-                        error = error.or(Some(ParseError::ExportNotFound(*span)))
-                    }
-
-                    output
-                }
-                ImportPatternMember::List { names } => {
-                    let mut output = vec![];
-
-                    for (name, span) in names {
-                        if let Some(id) = overlay.get_decl_id(name) {
-                            output.push((name.clone(), id));
-                        } else if !overlay.has_env_var(name) {
-                            error = error.or(Some(ParseError::ExportNotFound(*span)));
-                            break;
-                        }
-                    }
-
-                    output
+                    error = error.or(Some(ParseError::FileNotFound(
+                        module_filename,
+                        import_pattern.head.span,
+                    )));
+                    (ImportPattern::new(), Overlay::new())
                 }
+            } else {
+                return (
+                    garbage_statement(spans),
+                    Some(ParseError::NonUtf8(spans[1])),
+                );
             }
         };
 
-        // Extend the current scope with the module's overlay
-        working_set.use_decls(decls_to_use);
-
-        // Create the Use command call
-        let use_decl_id = working_set
-            .find_decl(b"use")
-            .expect("internal error: missing use command");
-
-        let import_pattern_expr = Expression {
-            expr: Expr::ImportPattern(import_pattern),
-            span: span(&spans[1..]),
-            ty: Type::List(Box::new(Type::String)),
-            custom_completion: None,
-        };
-
-        let call = Box::new(Call {
-            head: spans[0],
-            decl_id: use_decl_id,
-            positional: vec![import_pattern_expr],
-            named: vec![],
-        });
-
-        (
-            Statement::Pipeline(Pipeline::from_vec(vec![Expression {
-                expr: Expr::Call(call),
-                span: span(spans),
-                ty: Type::Unknown,
-                custom_completion: None,
-            }])),
-            error,
-        )
+    let decls_to_use = if import_pattern.members.is_empty() {
+        overlay.decls_with_head(&import_pattern.head.name)
     } else {
-        (
-            garbage_statement(spans),
-            Some(ParseError::UnknownState(
-                "Expected structure: use <name>".into(),
-                span(spans),
-            )),
-        )
-    }
+        match &import_pattern.members[0] {
+            ImportPatternMember::Glob { .. } => overlay.decls(),
+            ImportPatternMember::Name { name, span } => {
+                let mut output = vec![];
+
+                if let Some(id) = overlay.get_decl_id(name) {
+                    output.push((name.clone(), id));
+                } else if !overlay.has_env_var(name) {
+                    error = error.or(Some(ParseError::ExportNotFound(*span)))
+                }
+
+                output
+            }
+            ImportPatternMember::List { names } => {
+                let mut output = vec![];
+
+                for (name, span) in names {
+                    if let Some(id) = overlay.get_decl_id(name) {
+                        output.push((name.clone(), id));
+                    } else if !overlay.has_env_var(name) {
+                        error = error.or(Some(ParseError::ExportNotFound(*span)));
+                        break;
+                    }
+                }
+
+                output
+            }
+        }
+    };
+
+    // Extend the current scope with the module's overlay
+    working_set.use_decls(decls_to_use);
+
+    // Create a new Use command call to pass the new import pattern
+    let import_pattern_expr = Expression {
+        expr: Expr::ImportPattern(import_pattern),
+        span: span(&spans[1..]),
+        ty: Type::List(Box::new(Type::String)),
+        custom_completion: None,
+    };
+
+    let call = Box::new(Call {
+        head: spans[0],
+        decl_id: use_decl_id,
+        positional: vec![import_pattern_expr],
+        named: vec![],
+    });
+
+    (
+        Statement::Pipeline(Pipeline::from_vec(vec![Expression {
+            expr: Expr::Call(call),
+            span: span(spans),
+            ty: Type::Unknown,
+            custom_completion: None,
+        }])),
+        error,
+    )
 }
 
 pub fn parse_hide(
     working_set: &mut StateWorkingSet,
     spans: &[Span],
 ) -> (Statement, Option<ParseError>) {
+    if working_set.get_span_contents(spans[0]) != b"hide" {
+        return (
+            garbage_statement(spans),
+            Some(ParseError::UnknownState(
+                "internal error: Wrong call name for 'hide' command".into(),
+                span(spans),
+            )),
+        );
+    }
+
+    let (call, call_span, hide_decl_id) = match working_set.find_decl(b"hide") {
+        Some(decl_id) => {
+            let (call, mut err) = parse_internal_call(working_set, spans[0], &spans[1..], decl_id);
+            let decl = working_set.get_decl(decl_id);
+
+            let call_span = span(spans);
+
+            err = check_call(call_span, &decl.signature(), &call).or(err);
+            if err.is_some() || call.has_flag("help") {
+                return (
+                    Statement::Pipeline(Pipeline::from_vec(vec![Expression {
+                        expr: Expr::Call(call),
+                        span: call_span,
+                        ty: Type::Unknown,
+                        custom_completion: None,
+                    }])),
+                    err,
+                );
+            }
+
+            (call, call_span, decl_id)
+        }
+        None => {
+            return (
+                garbage_statement(spans),
+                Some(ParseError::UnknownState(
+                    "internal error: 'hide' declaration not found".into(),
+                    span(spans),
+                )),
+            )
+        }
+    };
+
+    let import_pattern = if let Some(expr) = call.nth(0) {
+        if let Some(pattern) = expr.as_import_pattern() {
+            pattern
+        } else {
+            return (
+                garbage_statement(spans),
+                Some(ParseError::UnknownState(
+                    "internal error: Import pattern positional is not import pattern".into(),
+                    call_span,
+                )),
+            );
+        }
+    } else {
+        return (
+            garbage_statement(spans),
+            Some(ParseError::UnknownState(
+                "internal error: Missing required positional after call parsing".into(),
+                call_span,
+            )),
+        );
+    };
+
     let mut error = None;
     let bytes = working_set.get_span_contents(spans[0]);
 
@@ -800,9 +918,6 @@ pub fn parse_hide(
             error = error.or(err);
         }
 
-        let (import_pattern, err) = parse_import_pattern(working_set, &spans[1..]);
-        error = error.or(err);
-
         let (is_module, overlay) =
             if let Some(overlay_id) = working_set.find_overlay(&import_pattern.head.name) {
                 (true, working_set.get_overlay(overlay_id).clone())
@@ -881,11 +996,7 @@ pub fn parse_hide(
         let import_pattern = import_pattern
             .with_hidden(decls_to_hide.iter().map(|(name, _)| name.clone()).collect());
 
-        // Create the Hide command call
-        let hide_decl_id = working_set
-            .find_decl(b"hide")
-            .expect("internal error: missing hide command");
-
+        // Create a new Use command call to pass the new import pattern
         let import_pattern_expr = Expression {
             expr: Expr::ImportPattern(import_pattern),
             span: span(&spans[1..]),

crates/nu-parser/src/parser.rs

@@ -481,6 +481,15 @@ pub fn parse_multispan_value(
 
             (arg, error)
         }
+        SyntaxShape::ImportPattern => {
+            trace!("parsing: import pattern");
+
+            let (arg, err) = parse_import_pattern(working_set, &spans[*spans_idx..]);
+            error = error.or(err);
+            *spans_idx = spans.len() - 1;
+
+            (arg, error)
+        }
         SyntaxShape::Keyword(keyword, arg) => {
             trace!(
                 "parsing: keyword({}) {:?}",
@@ -1951,7 +1960,7 @@ pub fn parse_type(_working_set: &StateWorkingSet, bytes: &[u8]) -> Type {
 pub fn parse_import_pattern(
     working_set: &mut StateWorkingSet,
     spans: &[Span],
-) -> (ImportPattern, Option<ParseError>) {
+) -> (Expression, Option<ParseError>) {
     let mut error = None;
 
     let (head, head_span) = if let Some(head_span) = spans.get(0) {
@@ -1961,19 +1970,12 @@ pub fn parse_import_pattern(
         )
     } else {
         return (
-            ImportPattern {
-                head: ImportPatternHead {
-                    name: vec![],
-                    span: span(spans),
-                },
-                members: vec![],
-                hidden: HashSet::new(),
-            },
+            garbage(span(spans)),
             Some(ParseError::WrongImportPattern(span(spans))),
         );
     };
 
-    if let Some(tail_span) = spans.get(1) {
+    let (import_pattern, err) = if let Some(tail_span) = spans.get(1) {
         // FIXME: expand this to handle deeper imports once we support module imports
         let tail = working_set.get_span_contents(*tail_span);
         if tail == b"*" {
@@ -1986,7 +1988,7 @@ pub fn parse_import_pattern(
                     members: vec![ImportPatternMember::Glob { span: *tail_span }],
                     hidden: HashSet::new(),
                 },
-                error,
+                None,
             )
         } else if tail.starts_with(b"[") {
             let (result, err) =
@@ -2014,7 +2016,7 @@ pub fn parse_import_pattern(
                             members: vec![ImportPatternMember::List { names: output }],
                             hidden: HashSet::new(),
                         },
-                        error,
+                        None,
                     )
                 }
                 _ => (
@@ -2043,7 +2045,7 @@ pub fn parse_import_pattern(
                     }],
                     hidden: HashSet::new(),
                 },
-                error,
+                None,
             )
         }
     } else {
@@ -2058,7 +2060,17 @@ pub fn parse_import_pattern(
             },
             None,
         )
-    }
+    };
+
+    (
+        Expression {
+            expr: Expr::ImportPattern(import_pattern),
+            span: span(&spans[1..]),
+            ty: Type::List(Box::new(Type::String)),
+            custom_completion: None,
+        },
+        error.or(err),
+    )
 }
 
 pub fn parse_var_with_opt_type(

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

@@ -1,4 +1,5 @@
 use super::{Expr, Operator, Statement};
+use crate::ast::ImportPattern;
 use crate::{engine::StateWorkingSet, BlockId, Signature, Span, Type, VarId, IN_VARIABLE_ID};
 
 #[derive(Debug, Clone)]
@@ -96,6 +97,13 @@ impl Expression {
         }
     }
 
+    pub fn as_import_pattern(&self) -> Option<ImportPattern> {
+        match &self.expr {
+            Expr::ImportPattern(pattern) => Some(pattern.clone()),
+            _ => None,
+        }
+    }
+
     pub fn has_in_variable(&self, working_set: &StateWorkingSet) -> bool {
         match &self.expr {
             Expr::BinaryOp(left, _, right) => {

crates/nu-protocol/src/shell_error.rs

@@ -95,9 +95,9 @@ pub enum ShellError {
     #[diagnostic(code(nu::shell::variable_not_found), url(docsrs))]
     VariableNotFoundAtRuntime(#[label = "variable not found"] Span),
 
-    #[error("Environment variable not found")]
+    #[error("Environment variable '{0}' not found")]
     #[diagnostic(code(nu::shell::env_variable_not_found), url(docsrs))]
-    EnvVarNotFoundAtRuntime(#[label = "environment variable not found"] Span),
+    EnvVarNotFoundAtRuntime(String, #[label = "environment variable not found"] Span),
 
     #[error("Not found.")]
     #[diagnostic(code(nu::parser::not_found), url(docsrs))]