From e94d1d2758dd11c0ff472c3097203a0398744eef Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Sat, 8 Jun 2019 10:35:07 +1200 Subject: [PATCH] Add pretty errors to commands --- src/cli.rs | 49 ++++++++++++++++++++++---------------- src/commands/cd.rs | 26 ++++++++++++++++++-- src/commands/classified.rs | 12 ++++++++-- src/commands/command.rs | 40 +++++-------------------------- src/commands/first.rs | 25 ++++++++++++++++++- src/commands/get.rs | 12 ++++++++-- src/commands/ls.rs | 24 ++++++++++++++++--- src/commands/open.rs | 40 ++++++++++++++++++++++++++----- src/commands/pick.rs | 12 ++++++++-- src/commands/reject.rs | 12 ++++++++-- src/commands/save.rs | 9 ++++--- src/commands/skip.rs | 25 ++++++++++++++++++- src/commands/view.rs | 29 ++++++++++++++++++---- src/context.rs | 9 +++---- src/errors.rs | 26 +++++++++++++------- src/evaluate/evaluator.rs | 39 ++++++++++++++++++++---------- src/format.rs | 3 +-- src/format/entries.rs | 33 ------------------------- src/format/generic.rs | 2 +- src/format/tree.rs | 4 +++- src/object/base.rs | 3 ++- src/parser.rs | 6 ++--- src/parser/lexer.rs | 3 +-- src/parser/registry.rs | 35 +++++++++++++++++---------- src/prelude.rs | 2 +- src/stream.rs | 4 ---- 26 files changed, 318 insertions(+), 166 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index e8aa1935cc..71a84b09b1 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -122,11 +122,12 @@ pub async fn cli() -> Result<(), Box> { rl.add_history_entry(line.clone()); } - LineResult::Error(err) => match err { - ShellError::Diagnostic(diag, source) => { + LineResult::Error(mut line, err) => match err { + ShellError::Diagnostic(diag) => { let host = context.host.lock().unwrap(); let writer = host.err_termcolor(); - let files = crate::parser::span::Files::new(source); + line.push_str(" "); + let files = crate::parser::span::Files::new(line); language_reporting::emit( &mut writer.lock(), @@ -149,7 +150,7 @@ pub async fn cli() -> Result<(), Box> { .unwrap() .stdout(&format!("Missing property {}", subpath)), - ShellError::String(s) => context.host.lock().unwrap().stdout(&format!("{:?}", s)), + ShellError::String(_) => context.host.lock().unwrap().stdout(&format!("{}", err)), }, LineResult::Break => { @@ -172,7 +173,7 @@ pub async fn cli() -> Result<(), Box> { enum LineResult { Success(String), - Error(ShellError), + Error(String, ShellError), Break, #[allow(unused)] @@ -186,13 +187,13 @@ impl std::ops::Try for LineResult { fn into_result(self) -> Result, ShellError> { match self { LineResult::Success(s) => Ok(Some(s)), - LineResult::Error(s) => Err(s), + LineResult::Error(_, s) => Err(s), LineResult::Break => Ok(None), LineResult::FatalError(err) => Err(err), } } fn from_error(v: ShellError) -> Self { - LineResult::Error(v) + LineResult::Error(String::new(), v) } fn from_ok(v: Option) -> Self { @@ -212,7 +213,7 @@ async fn process_line(readline: Result, ctx: &mut Context Ok(line) => { let result = match crate::parser::parse(&line) { Err(err) => { - return LineResult::Error(err); + return LineResult::Error(line.to_string(), err); } Ok(val) => val, @@ -228,6 +229,7 @@ async fn process_line(readline: Result, ctx: &mut Context Some(ClassifiedCommand::External(_)) => {} _ => pipeline.commands.push(ClassifiedCommand::Sink(SinkCommand { command: sink("autoview", autoview::autoview), + name_span: None, args: Args { positional: vec![], named: indexmap::IndexMap::new(), @@ -247,19 +249,19 @@ async fn process_line(readline: Result, ctx: &mut Context (None, _) => break, (Some(ClassifiedCommand::Expr(_)), _) => { - return LineResult::Error(ShellError::unimplemented( + return LineResult::Error(line.to_string(), ShellError::unimplemented( "Expression-only commands", )) } (_, Some(ClassifiedCommand::Expr(_))) => { - return LineResult::Error(ShellError::unimplemented( + return LineResult::Error(line.to_string(), ShellError::unimplemented( "Expression-only commands", )) } (Some(ClassifiedCommand::Sink(_)), Some(_)) => { - return LineResult::Error(ShellError::string("Commands like table, save, and autoview must come last in the pipeline")) + return LineResult::Error(line.to_string(), ShellError::string("Commands like table, save, and autoview must come last in the pipeline")) } (Some(ClassifiedCommand::Sink(left)), None) => { @@ -276,7 +278,7 @@ async fn process_line(readline: Result, ctx: &mut Context Some(ClassifiedCommand::External(_)), ) => match left.run(ctx, input).await { Ok(val) => ClassifiedInputStream::from_input_stream(val), - Err(err) => return LineResult::Error(err), + Err(err) => return LineResult::Error(line.to_string(), err), }, ( @@ -284,13 +286,13 @@ async fn process_line(readline: Result, ctx: &mut Context Some(_), ) => match left.run(ctx, input).await { Ok(val) => ClassifiedInputStream::from_input_stream(val), - Err(err) => return LineResult::Error(err), + Err(err) => return LineResult::Error(line.to_string(), err), }, (Some(ClassifiedCommand::Internal(left)), None) => { match left.run(ctx, input).await { Ok(val) => ClassifiedInputStream::from_input_stream(val), - Err(err) => return LineResult::Error(err), + Err(err) => return LineResult::Error(line.to_string(), err), } } @@ -299,7 +301,7 @@ async fn process_line(readline: Result, ctx: &mut Context Some(ClassifiedCommand::External(_)), ) => match left.run(ctx, input, StreamNext::External).await { Ok(val) => val, - Err(err) => return LineResult::Error(err), + Err(err) => return LineResult::Error(line.to_string(), err), }, ( @@ -307,13 +309,13 @@ async fn process_line(readline: Result, ctx: &mut Context Some(_), ) => match left.run(ctx, input, StreamNext::Internal).await { Ok(val) => val, - Err(err) => return LineResult::Error(err), + Err(err) => return LineResult::Error(line.to_string(), err), }, (Some(ClassifiedCommand::External(left)), None) => { match left.run(ctx, input, StreamNext::Last).await { Ok(val) => val, - Err(err) => return LineResult::Error(err), + Err(err) => return LineResult::Error(line.to_string(), err), } } } @@ -321,7 +323,9 @@ async fn process_line(readline: Result, ctx: &mut Context LineResult::Success(line.to_string()) } - Err(ReadlineError::Interrupted) => LineResult::Error(ShellError::string("CTRL-C")), + Err(ReadlineError::Interrupted) => { + LineResult::Error("".to_string(), ShellError::string("CTRL-C")) + } Err(ReadlineError::Eof) => { println!("CTRL-D"); LineResult::Break @@ -365,7 +369,7 @@ fn classify_command( ( Expression { expr: RawExpression::Leaf(Leaf::Bare(name)), - .. + span, }, args, ) => match context.has_command(&name.to_string()) { @@ -381,6 +385,7 @@ fn classify_command( Ok(ClassifiedCommand::Internal(InternalCommand { command, + name_span: Some(span.clone()), args, })) } @@ -395,7 +400,11 @@ fn classify_command( None => Args::default(), }; - Ok(ClassifiedCommand::Sink(SinkCommand { command, args })) + Ok(ClassifiedCommand::Sink(SinkCommand { + command, + name_span: Some(span.clone()), + args, + })) } false => { let arg_list_strings: Vec = match args { diff --git a/src/commands/cd.rs b/src/commands/cd.rs index f39c9161e3..7fbe5d3a05 100644 --- a/src/commands/cd.rs +++ b/src/commands/cd.rs @@ -11,12 +11,34 @@ pub fn cd(args: CommandArgs) -> Result { }, Some(v) => { let target = v.as_string()?.clone(); - dunce::canonicalize(cwd.join(&target).as_path())? + match dunce::canonicalize(cwd.join(&target).as_path()) { + Ok(p) => p, + Err(_) => { + return Err(ShellError::labeled_error( + "Can not change to directory", + "directory not found", + args.positional[0].span.clone(), + )); + } + } } }; let mut stream = VecDeque::new(); - let _ = env::set_current_dir(&path); + match env::set_current_dir(&path) { + Ok(_) => {} + Err(_) => { + if args.positional.len() > 0 { + return Err(ShellError::labeled_error( + "Can not change to directory", + "directory not found", + args.positional[0].span.clone(), + )); + } else { + return Err(ShellError::string("Can not change to directory")); + } + } + } stream.push_back(ReturnValue::change_cwd(path)); Ok(stream.boxed()) } diff --git a/src/commands/classified.rs b/src/commands/classified.rs index e10496f6ad..d29a903619 100644 --- a/src/commands/classified.rs +++ b/src/commands/classified.rs @@ -1,5 +1,6 @@ use crate::commands::command::Sink; use crate::parser::ast::Expression; +use crate::parser::lexer::Span; use crate::parser::registry::Args; use crate::prelude::*; use bytes::{BufMut, BytesMut}; @@ -86,17 +87,19 @@ crate enum ClassifiedCommand { crate struct SinkCommand { crate command: Arc, + crate name_span: Option, crate args: Args, } impl SinkCommand { crate fn run(self, context: &mut Context, input: Vec) -> Result<(), ShellError> { - context.run_sink(self.command, self.args, input) + context.run_sink(self.command, self.name_span.clone(), self.args, input) } } crate struct InternalCommand { crate command: Arc, + crate name_span: Option, crate args: Args, } @@ -106,7 +109,12 @@ impl InternalCommand { context: &mut Context, input: ClassifiedInputStream, ) -> Result { - let result = context.run_command(self.command, self.args, input.objects)?; + let result = context.run_command( + self.command, + self.name_span.clone(), + self.args, + input.objects, + )?; let env = context.env.clone(); let stream = result.filter_map(move |v| match v { diff --git a/src/commands/command.rs b/src/commands/command.rs index cf98bb5ac7..2aff777692 100644 --- a/src/commands/command.rs +++ b/src/commands/command.rs @@ -1,56 +1,28 @@ use crate::errors::ShellError; use crate::object::Value; +use crate::parser::lexer::Span; +use crate::parser::lexer::Spanned; use crate::parser::CommandConfig; use crate::prelude::*; -use core::future::Future; use std::path::PathBuf; pub struct CommandArgs { pub host: Arc>, pub env: Arc>, - pub positional: Vec, + pub name_span: Option, + pub positional: Vec>, pub named: indexmap::IndexMap, pub input: InputStream, } -impl CommandArgs { - crate fn from_context( - ctx: &'caller mut Context, - positional: Vec, - input: InputStream, - ) -> CommandArgs { - CommandArgs { - host: ctx.host.clone(), - env: ctx.env.clone(), - positional, - named: indexmap::IndexMap::default(), - input, - } - } -} - pub struct SinkCommandArgs { pub ctx: Context, - pub positional: Vec, + pub name_span: Option, + pub positional: Vec>, pub named: indexmap::IndexMap, pub input: Vec, } -impl SinkCommandArgs { - crate fn from_context( - ctx: &'caller mut Context, - positional: Vec, - input: Vec, - ) -> SinkCommandArgs { - SinkCommandArgs { - ctx: ctx.clone(), - positional, - named: indexmap::IndexMap::default(), - input, - } - } -} - #[derive(Debug)] pub enum CommandAction { ChangeCwd(PathBuf), diff --git a/src/commands/first.rs b/src/commands/first.rs index 9f7748ced2..afc1ff8f52 100644 --- a/src/commands/first.rs +++ b/src/commands/first.rs @@ -4,7 +4,30 @@ use crate::prelude::*; // TODO: "Amount remaining" wrapper pub fn first(args: CommandArgs) -> Result { - let amount = args.positional[0].as_i64()?; + if args.positional.len() == 0 { + if let Some(span) = args.name_span { + return Err(ShellError::labeled_error( + "First requires an amount", + "needs parameter", + span, + )); + } else { + return Err(ShellError::string("first requires an amount.")); + } + } + + let amount = args.positional[0].as_i64(); + + let amount = match amount { + Ok(o) => o, + Err(_) => { + return Err(ShellError::labeled_error( + "Value is not a number", + "expected integer", + args.positional[0].span, + )) + } + }; let input = args.input; diff --git a/src/commands/get.rs b/src/commands/get.rs index ca51e01500..078e9e8dc6 100644 --- a/src/commands/get.rs +++ b/src/commands/get.rs @@ -20,8 +20,16 @@ fn get_member(path: &str, obj: &Value) -> Option { } pub fn get(args: CommandArgs) -> Result { - if args.positional.is_empty() { - return Err(ShellError::string("select requires a field")); + if args.positional.len() == 0 { + if let Some(span) = args.name_span { + return Err(ShellError::labeled_error( + "Get requires a field or field path", + "needs parameter", + span, + )); + } else { + return Err(ShellError::string("get requires fields.")); + } } let fields: Result, _> = args.positional.iter().map(|a| a.as_string()).collect(); diff --git a/src/commands/ls.rs b/src/commands/ls.rs index c65bc2cc48..63927fdb4c 100644 --- a/src/commands/ls.rs +++ b/src/commands/ls.rs @@ -1,5 +1,6 @@ use crate::errors::ShellError; use crate::object::{dir_entry_dict, Primitive, Value}; +use crate::parser::lexer::Spanned; use crate::prelude::*; use std::path::{Path, PathBuf}; @@ -7,12 +8,29 @@ pub fn ls(args: CommandArgs) -> Result { let cwd = args.env.lock().unwrap().cwd().to_path_buf(); let mut full_path = PathBuf::from(cwd); match &args.positional.get(0) { - Some(Value::Primitive(Primitive::String(s))) => full_path.push(Path::new(s)), + Some(Spanned { + item: Value::Primitive(Primitive::String(s)), + .. + }) => full_path.push(Path::new(s)), _ => {} } - let entries = - std::fs::read_dir(&full_path).map_err(|e| ShellError::string(format!("{:?}", e)))?; + let entries = std::fs::read_dir(&full_path); + + let entries = match entries { + Err(e) => { + if let Some(s) = args.positional.get(0) { + return Err(ShellError::labeled_error( + e.to_string(), + e.to_string(), + s.span, + )); + } else { + return Err(ShellError::string(e.to_string())); + } + } + Ok(o) => o, + }; let mut shell_entries = VecDeque::new(); diff --git a/src/commands/open.rs b/src/commands/open.rs index 6e009e8fc2..d9e9840daf 100644 --- a/src/commands/open.rs +++ b/src/commands/open.rs @@ -1,5 +1,6 @@ use crate::errors::ShellError; use crate::object::{Primitive, Value}; +use crate::parser::lexer::Spanned; use crate::prelude::*; use std::path::{Path, PathBuf}; @@ -10,17 +11,44 @@ pub fn open(args: CommandArgs) -> Result { let cwd = args.env.lock().unwrap().cwd().to_path_buf(); let mut full_path = PathBuf::from(cwd); - match &args.positional[0] { - Value::Primitive(Primitive::String(s)) => full_path.push(Path::new(s)), - _ => {} - } - let contents = std::fs::read_to_string(&full_path).unwrap(); + let contents = match &args.positional[0].item { + Value::Primitive(Primitive::String(s)) => { + full_path.push(Path::new(&s)); + match std::fs::read_to_string(&full_path) { + Ok(s) => s, + Err(_) => { + return Err(ShellError::labeled_error( + "File cound not be opened", + "file not found", + args.positional[0].span, + )); + } + } + } + _ => { + return Err(ShellError::labeled_error( + "Expected string value for filename", + "expected filename", + args.positional[0].span, + )); + } + }; let mut stream = VecDeque::new(); let open_raw = match args.positional.get(1) { - Some(Value::Primitive(Primitive::String(s))) if s == "--raw" => true, + Some(Spanned { + item: Value::Primitive(Primitive::String(s)), + .. + }) if s == "--raw" => true, + Some(v) => { + return Err(ShellError::labeled_error( + "Unknown flag for open", + "unknown flag", + v.span, + )) + } _ => false, }; diff --git a/src/commands/pick.rs b/src/commands/pick.rs index 7cbf08536a..73e953ad0c 100644 --- a/src/commands/pick.rs +++ b/src/commands/pick.rs @@ -4,8 +4,16 @@ use crate::object::Value; use crate::prelude::*; pub fn pick(args: CommandArgs) -> Result { - if args.positional.is_empty() { - return Err(ShellError::string("select requires a field")); + if args.positional.len() == 0 { + if let Some(span) = args.name_span { + return Err(ShellError::labeled_error( + "Pick requires fields", + "needs parameter", + span, + )); + } else { + return Err(ShellError::string("pick requires fields.")); + } } let fields: Result, _> = args.positional.iter().map(|a| a.as_string()).collect(); diff --git a/src/commands/reject.rs b/src/commands/reject.rs index be447eb6cc..372adcf7c5 100644 --- a/src/commands/reject.rs +++ b/src/commands/reject.rs @@ -4,8 +4,16 @@ use crate::object::Value; use crate::prelude::*; pub fn reject(args: CommandArgs) -> Result { - if args.positional.is_empty() { - return Err(ShellError::string("select requires a field")); + if args.positional.len() == 0 { + if let Some(span) = args.name_span { + return Err(ShellError::labeled_error( + "Reject requires fields", + "needs parameter", + span, + )); + } else { + return Err(ShellError::string("reject requires fields.")); + } } let fields: Result, _> = args.positional.iter().map(|a| a.as_string()).collect(); diff --git a/src/commands/save.rs b/src/commands/save.rs index 1b66294a08..30ed386faf 100644 --- a/src/commands/save.rs +++ b/src/commands/save.rs @@ -1,7 +1,7 @@ use crate::commands::command::SinkCommandArgs; use crate::errors::ShellError; use crate::object::{Primitive, Value}; -use crate::prelude::*; +use crate::parser::lexer::Spanned; use std::path::{Path, PathBuf}; pub fn save(args: SinkCommandArgs) -> Result<(), ShellError> { @@ -11,13 +11,16 @@ pub fn save(args: SinkCommandArgs) -> Result<(), ShellError> { let cwd = args.ctx.env.lock().unwrap().cwd().to_path_buf(); let mut full_path = PathBuf::from(cwd); - match &args.positional[0] { + match &(args.positional[0].item) { Value::Primitive(Primitive::String(s)) => full_path.push(Path::new(s)), _ => {} } let save_raw = match args.positional.get(1) { - Some(Value::Primitive(Primitive::String(s))) if s == "--raw" => true, + Some(Spanned { + item: Value::Primitive(Primitive::String(s)), + .. + }) if s == "--raw" => true, _ => false, }; diff --git a/src/commands/skip.rs b/src/commands/skip.rs index 83309e4d82..38af0c1a79 100644 --- a/src/commands/skip.rs +++ b/src/commands/skip.rs @@ -2,7 +2,30 @@ use crate::errors::ShellError; use crate::prelude::*; pub fn skip(args: CommandArgs) -> Result { - let amount = args.positional[0].as_i64()?; + if args.positional.len() == 0 { + if let Some(span) = args.name_span { + return Err(ShellError::labeled_error( + "Skip requires an amount", + "needs parameter", + span, + )); + } else { + return Err(ShellError::string("skip requires an amount.")); + } + } + + let amount = args.positional[0].as_i64(); + + let amount = match amount { + Ok(o) => o, + Err(_) => { + return Err(ShellError::labeled_error( + "Value is not a number", + "expected integer", + args.positional[0].span, + )) + } + }; let input = args.input; diff --git a/src/commands/view.rs b/src/commands/view.rs index 3125cd6e60..e7c13efbaa 100644 --- a/src/commands/view.rs +++ b/src/commands/view.rs @@ -3,10 +3,31 @@ use crate::prelude::*; use prettyprint::PrettyPrinter; pub fn view(args: CommandArgs) -> Result { - let target = match args.positional.first() { - // TODO: This needs better infra - None => return Err(ShellError::string(format!("cat must take one arg"))), - Some(v) => v.as_string()?.clone(), + if args.positional.len() == 0 { + if let Some(span) = args.name_span { + return Err(ShellError::labeled_error( + "View requires a filename", + "needs parameter", + span, + )); + } else { + return Err(ShellError::string("view requires a filename.")); + } + } + + let target = match args.positional[0].as_string() { + Ok(s) => s.clone(), + Err(e) => { + if let Some(span) = args.name_span { + return Err(ShellError::labeled_error( + "Expected a string", + "not a filename", + span, + )); + } else { + return Err(e); + } + } }; let cwd = args.env.lock().unwrap().cwd().to_path_buf(); diff --git a/src/context.rs b/src/context.rs index b63252f767..7beba4a748 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,5 +1,6 @@ use crate::commands::command::Sink; use crate::commands::command::SinkCommandArgs; +use crate::parser::lexer::Span; use crate::parser::Args; use crate::prelude::*; @@ -37,10 +38,6 @@ impl Context { } } - pub fn clone_sinks(&self) -> indexmap::IndexMap> { - self.sinks.clone() - } - crate fn has_sink(&self, name: &str) -> bool { self.sinks.contains_key(name) } @@ -52,11 +49,13 @@ impl Context { crate fn run_sink( &mut self, command: Arc, + name_span: Option, args: Args, input: Vec, ) -> Result<(), ShellError> { let command_args = SinkCommandArgs { ctx: self.clone(), + name_span, positional: args.positional, named: args.named, input, @@ -80,12 +79,14 @@ impl Context { crate fn run_command( &mut self, command: Arc, + name_span: Option, args: Args, input: InputStream, ) -> Result { let command_args = CommandArgs { host: self.host.clone(), env: self.env.clone(), + name_span, positional: args.positional, named: args.named, input, diff --git a/src/errors.rs b/src/errors.rs index 89d403add0..3acb91a499 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -2,7 +2,7 @@ use crate::parser::lexer::{Span, SpannedToken}; #[allow(unused)] use crate::prelude::*; use derive_new::new; -use language_reporting::Diagnostic; +use language_reporting::{Diagnostic, Label, Severity}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use serde_derive::{Deserialize, Serialize}; @@ -11,13 +11,12 @@ pub enum ShellError { String(StringError), TypeError(String), MissingProperty { subpath: String, expr: String }, - Diagnostic(ShellDiagnostic, String), + Diagnostic(ShellDiagnostic), } impl ShellError { crate fn parse_error( error: lalrpop_util::ParseError, - source: String, ) -> ShellError { use lalrpop_util::ParseError; use language_reporting::*; @@ -33,15 +32,26 @@ impl ShellError { ) .with_label(Label::new_primary(Span::from((start, end)))); - ShellError::diagnostic(diagnostic, source) + ShellError::diagnostic(diagnostic) } - + ParseError::User { error } => error, other => ShellError::string(format!("{:?}", other)), } } - crate fn diagnostic(diagnostic: Diagnostic, source: String) -> ShellError { - ShellError::Diagnostic(ShellDiagnostic { diagnostic }, source) + crate fn diagnostic(diagnostic: Diagnostic) -> ShellError { + ShellError::Diagnostic(ShellDiagnostic { diagnostic }) + } + + crate fn labeled_error( + msg: impl Into, + label: impl Into, + span: Span, + ) -> ShellError { + ShellError::diagnostic( + Diagnostic::new(Severity::Error, msg.into()) + .with_label(Label::new_primary(span).with_message(label.into())), + ) } crate fn string(title: impl Into) -> ShellError { @@ -134,7 +144,7 @@ impl std::fmt::Display for ShellError { ShellError::String(s) => write!(f, "{}", &s.title), ShellError::TypeError { .. } => write!(f, "TypeError"), ShellError::MissingProperty { .. } => write!(f, "MissingProperty"), - ShellError::Diagnostic(_, _) => write!(f, ""), + ShellError::Diagnostic(_) => write!(f, ""), } } } diff --git a/src/evaluate/evaluator.rs b/src/evaluate/evaluator.rs index 3e0531afa3..c60b7fe08d 100644 --- a/src/evaluate/evaluator.rs +++ b/src/evaluate/evaluator.rs @@ -1,5 +1,6 @@ use crate::object::Primitive; use crate::parser::ast; +use crate::parser::lexer::Spanned; use crate::prelude::*; use derive_new::new; use indexmap::IndexMap; @@ -20,17 +21,25 @@ impl Scope { } } -crate fn evaluate_expr(expr: &ast::Expression, scope: &Scope) -> Result { +crate fn evaluate_expr( + expr: &ast::Expression, + scope: &Scope, +) -> Result, ShellError> { use ast::*; match &expr.expr { RawExpression::Call(_) => Err(ShellError::unimplemented("Evaluating call expression")), - RawExpression::Leaf(l) => Ok(evaluate_leaf(l)), + RawExpression::Leaf(l) => Ok(Spanned::from_item(evaluate_leaf(l), expr.span.clone())), RawExpression::Parenthesized(p) => evaluate_expr(&p.expr, scope), - RawExpression::Flag(f) => Ok(Value::Primitive(Primitive::String(f.print()))), + RawExpression::Flag(f) => Ok(Spanned::from_item( + Value::Primitive(Primitive::String(f.print())), + expr.span.clone(), + )), RawExpression::Block(b) => evaluate_block(&b, scope), RawExpression::Path(p) => evaluate_path(&p, scope), RawExpression::Binary(b) => evaluate_binary(b, scope), - RawExpression::VariableReference(r) => evaluate_reference(r, scope), + RawExpression::VariableReference(r) => { + evaluate_reference(r, scope).map(|x| Spanned::from_item(x, expr.span.clone())) + } } } @@ -59,12 +68,15 @@ fn evaluate_reference(r: &ast::Variable, scope: &Scope) -> Result Result { +fn evaluate_binary(binary: &ast::Binary, scope: &Scope) -> Result, ShellError> { let left = evaluate_expr(&binary.left, scope)?; let right = evaluate_expr(&binary.right, scope)?; match left.compare(&binary.operator, &right) { - Some(v) => Ok(Value::boolean(v)), + Some(v) => Ok(Spanned::from_item( + Value::boolean(v), + binary.operator.span.clone(), + )), None => Err(ShellError::TypeError(format!( "Can't compare {} and {}", left.type_name(), @@ -73,13 +85,16 @@ fn evaluate_binary(binary: &ast::Binary, scope: &Scope) -> Result Result { - Ok(Value::block(block.expr.clone())) +fn evaluate_block(block: &ast::Block, _scope: &Scope) -> Result, ShellError> { + Ok(Spanned::from_item( + Value::block(block.expr.clone()), + block.expr.span.clone(), + )) } -fn evaluate_path(path: &ast::Path, scope: &Scope) -> Result { +fn evaluate_path(path: &ast::Path, scope: &Scope) -> Result, ShellError> { let head = path.head(); - let mut value = &evaluate_expr(head, scope)?; + let mut value = evaluate_expr(head, scope)?; let mut seen = vec![]; for name in path.tail() { @@ -93,9 +108,9 @@ fn evaluate_path(path: &ast::Path, scope: &Scope) -> Result { subpath: itertools::join(seen, "."), }); } - Some(v) => value = v, + Some(v) => value = Spanned::from_item(v.copy(), name.span.clone()), } } - Ok(value.copy()) + Ok(value) } diff --git a/src/format.rs b/src/format.rs index 43c2c1db91..9215c18824 100644 --- a/src/format.rs +++ b/src/format.rs @@ -6,9 +6,8 @@ crate mod tree; use crate::prelude::*; -crate use entries::{EntriesListView, EntriesView}; +crate use entries::EntriesView; crate use generic::GenericView; -crate use list::ListView; crate use table::TableView; crate use tree::TreeView; diff --git a/src/format/entries.rs b/src/format/entries.rs index 773c0ba1ee..a39462caa0 100644 --- a/src/format/entries.rs +++ b/src/format/entries.rs @@ -55,36 +55,3 @@ impl RenderView for EntriesView { Ok(()) } } - -pub struct EntriesListView { - values: VecDeque, -} - -impl EntriesListView { - crate async fn from_stream(values: InputStream) -> EntriesListView { - EntriesListView { - values: values.collect().await, - } - } -} - -impl RenderView for EntriesListView { - fn render_view(&self, host: &mut dyn Host) -> Result<(), ShellError> { - if self.values.len() == 0 { - return Ok(()); - } - - let last = self.values.len() - 1; - - for (i, item) in self.values.iter().enumerate() { - let view = EntriesView::from_value(item); - view.render_view(host)?; - - if i != last { - host.stdout("\n"); - } - } - - Ok(()) - } -} diff --git a/src/format/generic.rs b/src/format/generic.rs index 18b7c0d582..da5954398f 100644 --- a/src/format/generic.rs +++ b/src/format/generic.rs @@ -1,4 +1,4 @@ -use crate::format::{EntriesView, ListView, RenderView, TableView, TreeView}; +use crate::format::{EntriesView, RenderView, TableView}; use crate::object::Value; use crate::prelude::*; use derive_new::new; diff --git a/src/format/tree.rs b/src/format/tree.rs index cb2b650907..9afd138a0c 100644 --- a/src/format/tree.rs +++ b/src/format/tree.rs @@ -21,7 +21,9 @@ pub struct TreeView { impl TreeView { fn from_value_helper(value: &Value, mut builder: &mut TreeBuilder) { match value { - Value::Primitive(p) => builder = builder.add_empty_child(p.format(None)), + Value::Primitive(p) => { + let _ = builder.add_empty_child(p.format(None)); + } Value::Object(o) => { for (k, v) in o.entries.iter() { builder = builder.begin_child(k.name.display().to_string()); diff --git a/src/object/base.rs b/src/object/base.rs index 050916574d..f2dca67342 100644 --- a/src/object/base.rs +++ b/src/object/base.rs @@ -2,6 +2,7 @@ use crate::errors::ShellError; use crate::evaluate::{evaluate_expr, Scope}; use crate::object::DataDescriptor; use crate::parser::ast::{self, Operator}; +use crate::parser::lexer::Spanned; use crate::prelude::*; use ansi_term::Color; use chrono::{DateTime, Utc}; @@ -142,7 +143,7 @@ impl Deserialize<'de> for Block { } impl Block { - pub fn invoke(&self, value: &Value) -> Result { + pub fn invoke(&self, value: &Value) -> Result, ShellError> { let scope = Scope::new(value.copy()); evaluate_expr(&self.expression, &scope) } diff --git a/src/parser.rs b/src/parser.rs index c96f681381..da1d7ce88a 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -26,7 +26,7 @@ pub fn parse(input: &str) -> Result { match parser.parse(tokens) { Ok(val) => Ok(val), - Err(err) => Err(ShellError::parse_error(err, input.to_string())), + Err(err) => Err(ShellError::parse_error(err)), } } @@ -39,11 +39,11 @@ mod tests { fn assert_parse(source: &str, expected: Pipeline) { let parsed = match parse(source) { Ok(p) => p, - Err(ShellError::Diagnostic(diag, source)) => { + Err(ShellError::Diagnostic(diag)) => { use language_reporting::termcolor; let writer = termcolor::StandardStream::stdout(termcolor::ColorChoice::Auto); - let files = crate::parser::span::Files::new(source); + let files = crate::parser::span::Files::new(source.to_string()); language_reporting::emit( &mut writer.lock(), diff --git a/src/parser/lexer.rs b/src/parser/lexer.rs index 53dce2c1bf..672eaef468 100644 --- a/src/parser/lexer.rs +++ b/src/parser/lexer.rs @@ -558,13 +558,12 @@ impl Iterator for Lexer<'source> { } } -fn lex_error(range: &Range, source: &str) -> ShellError { +fn lex_error(range: &Range, _source: &str) -> ShellError { use language_reporting::*; ShellError::diagnostic( Diagnostic::new(Severity::Error, "Lex error") .with_label(Label::new_primary(Span::new(range))), - source.to_string(), ) } diff --git a/src/parser/registry.rs b/src/parser/registry.rs index 84fedce19a..a1adc63b9c 100644 --- a/src/parser/registry.rs +++ b/src/parser/registry.rs @@ -1,4 +1,5 @@ use crate::evaluate::{evaluate_expr, Scope}; +use crate::parser::lexer::Spanned; use crate::prelude::*; use indexmap::IndexMap; @@ -37,14 +38,18 @@ impl PositionalType { } } - crate fn evaluate(&self, arg: ast::Expression, scope: &Scope) -> Result { + crate fn evaluate( + &self, + arg: ast::Expression, + scope: &Scope, + ) -> Result, ShellError> { match self { PositionalType::Value(_) => evaluate_expr(&arg, scope), PositionalType::Block(_) => match arg { ast::Expression { expr: ast::RawExpression::Block(b), .. - } => Ok(Value::block(b.expr)), + } => Ok(Spanned::from_item(Value::block(b.expr), arg.span.clone())), ast::Expression { expr: ast::RawExpression::Binary(binary), .. @@ -52,11 +57,14 @@ impl PositionalType { // TODO: Use original spans let mut b = ast::ExpressionBuilder::new(); if let Some(s) = binary.left.as_string() { - Ok(Value::block(b.binary(( - &|b| b.path((&|b| b.var("it"), vec![s.clone()])), - &|_| binary.operator.clone(), - &|_| binary.right.clone(), - )))) + Ok(Spanned::from_item( + Value::block(b.binary(( + &|b| b.path((&|b| b.var("it"), vec![s.clone()])), + &|_| binary.operator.clone(), + &|_| binary.right.clone(), + ))), + arg.span.clone(), + )) } else { let mut b = ast::ExpressionBuilder::new(); let expr = b.binary(( @@ -65,10 +73,13 @@ impl PositionalType { &|_| binary.right.clone(), )); - Ok(Value::block(expr)) + Ok(Spanned::from_item(Value::block(expr), arg.span.clone())) } } - other => Ok(Value::block(other)), // other => + other => { + let span = other.span.clone(); + Ok(Spanned::from_item(Value::block(other), span)) + } }, } } @@ -85,7 +96,7 @@ pub struct CommandConfig { #[derive(Debug, Default)] pub struct Args { - pub positional: Vec, + pub positional: Vec>, pub named: IndexMap, } @@ -95,7 +106,7 @@ impl CommandConfig { args: impl Iterator, scope: &Scope, ) -> Result { - let mut positional: Vec = vec![]; + let mut positional: Vec> = vec![]; let mut named: IndexMap = IndexMap::default(); let mut args: Vec = args.cloned().collect(); @@ -152,7 +163,7 @@ impl CommandConfig { } if self.rest_positional { - let rest: Result, _> = + let rest: Result>, _> = args.map(|i| evaluate_expr(&i, &Scope::empty())).collect(); positional.extend(rest?); } else { diff --git a/src/prelude.rs b/src/prelude.rs index 208625ba7c..d943d90827 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -7,7 +7,7 @@ crate use crate::errors::ShellError; crate use crate::object::Value; crate use crate::parser::ast; crate use crate::stream::{single_output, InputStream, OutputStream}; -crate use futures::{FutureExt, SinkExt, StreamExt}; +crate use futures::{FutureExt, StreamExt}; crate use std::collections::VecDeque; crate use std::pin::Pin; crate use std::sync::{Arc, Mutex}; diff --git a/src/stream.rs b/src/stream.rs index dc4def8cba..8a536950fb 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -4,10 +4,6 @@ use futures::stream::BoxStream; pub type InputStream = BoxStream<'static, Value>; pub type OutputStream = BoxStream<'static, ReturnValue>; -crate fn empty_stream() -> OutputStream { - VecDeque::new().boxed() -} - crate fn single_output(item: Value) -> OutputStream { let value = ReturnValue::Value(item); let mut vec = VecDeque::new();