From 4115634bfc04f9140602c4cda9fb8701f8e2aab5 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Mon, 2 Dec 2019 08:14:05 -0800 Subject: [PATCH] Try to re-apply #1039 --- crates/nu-errors/src/lib.rs | 130 ++++++++++---------- crates/nu-parser/src/hir/tokens_iterator.rs | 2 +- crates/nu-parser/src/parse/flag.rs | 2 +- crates/nu-parser/src/parse/operator.rs | 2 +- crates/nu-parser/src/parse/pipeline.rs | 2 +- crates/nu-protocol/src/call_info.rs | 2 +- crates/nu-protocol/src/return_value.rs | 2 +- crates/nu-protocol/src/value.rs | 7 ++ crates/nu-protocol/src/value/convert.rs | 2 +- src/cli.rs | 110 +---------------- src/commands/classified/mod.rs | 1 + src/commands/classified/pipeline.rs | 102 ++++++++++----- 12 files changed, 151 insertions(+), 213 deletions(-) diff --git a/crates/nu-errors/src/lib.rs b/crates/nu-errors/src/lib.rs index a7734bec54..1abeae9db4 100644 --- a/crates/nu-errors/src/lib.rs +++ b/crates/nu-errors/src/lib.rs @@ -9,64 +9,41 @@ use serde::{Deserialize, Serialize}; use std::fmt; use std::ops::Range; -// TODO: Spanned -> HasSpanAndItem ? - -#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash, Serialize, Deserialize)] -pub enum Description { - Source(Spanned), - Synthetic(String), -} - -impl Description { - fn from_spanned(item: Spanned>) -> Description { - Description::Source(item.map(|s| s.into())) - } - - fn into_label(self) -> Result, String> { - match self { - Description::Source(s) => Ok(Label::new_primary(s.span).with_message(s.item)), - Description::Synthetic(s) => Err(s), - } - } -} - -impl PrettyDebug for Description { - fn pretty(&self) -> DebugDocBuilder { - match self { - Description::Source(s) => b::description(&s.item), - Description::Synthetic(s) => b::description(s), - } - } -} - +/// A structured reason for a ParseError. Note that parsing in nu is more like macro expansion in +/// other languages, so the kinds of errors that can occur during parsing are more contextual than +/// you might expect. #[derive(Debug, Clone)] pub enum ParseErrorReason { - Eof { - expected: &'static str, - span: Span, - }, + /// The parser encountered an EOF rather than what it was expecting + Eof { expected: &'static str, span: Span }, + /// The parser encountered something other than what it was expecting Mismatch { expected: &'static str, actual: Spanned, }, + /// The parser tried to parse an argument for a command, but it failed for + /// some reason ArgumentError { command: Spanned, error: ArgumentError, }, } +/// A newtype for `ParseErrorReason` #[derive(Debug, Clone)] pub struct ParseError { reason: ParseErrorReason, } impl ParseError { + /// Construct a [ParseErrorReason::Eof](ParseErrorReason::Eof) pub fn unexpected_eof(expected: &'static str, span: Span) -> ParseError { ParseError { reason: ParseErrorReason::Eof { expected, span }, } } + /// Construct a [ParseErrorReason::Mismatch](ParseErrorReason::Mismatch) pub fn mismatch(expected: &'static str, actual: Spanned>) -> ParseError { let Spanned { span, item } = actual; @@ -78,6 +55,7 @@ impl ParseError { } } + /// Construct a [ParseErrorReason::ArgumentError](ParseErrorReason::ArgumentError) pub fn argument_error(command: Spanned>, kind: ArgumentError) -> ParseError { ParseError { reason: ParseErrorReason::ArgumentError { @@ -88,6 +66,7 @@ impl ParseError { } } +/// Convert a [ParseError](ParseError) into a [ShellError](ShellError) impl From for ShellError { fn from(error: ParseError) -> ShellError { match error.reason { @@ -102,11 +81,20 @@ impl From for ShellError { } } +/// ArgumentError describes various ways that the parser could fail because of unexpected arguments. +/// Nu commands are like a combination of functions and macros, and these errors correspond to +/// problems that could be identified during expansion based on the syntactic signature of a +/// command. #[derive(Debug, Eq, PartialEq, Clone, Ord, Hash, PartialOrd, Serialize, Deserialize)] pub enum ArgumentError { + /// The command specified a mandatory flag, but it was missing. MissingMandatoryFlag(String), + /// The command specified a mandatory positional argument, but it was missing. MissingMandatoryPositional(String), + /// A flag was found, and it should have been followed by a value, but no value was found MissingValueForName(String), + /// A sequence of characters was found that was not syntactically valid (but would have + /// been valid if the command was an external command) InvalidExternalWord, } @@ -133,12 +121,16 @@ impl PrettyDebug for ArgumentError { } } +/// A `ShellError` is a proximate error and a possible cause, which could have its own cause, +/// creating a cause chain. #[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Clone, Serialize, Deserialize, Hash)] pub struct ShellError { error: ProximateShellError, - cause: Option>, + cause: Option>, } +/// `PrettyDebug` is for internal debugging. For user-facing debugging, [to_diagnostic](ShellError::to_diagnostic) +/// is used, which prints an error, highlighting spans. impl PrettyDebug for ShellError { fn pretty(&self) -> DebugDocBuilder { match &self.error { @@ -171,12 +163,12 @@ impl PrettyDebug for ShellError { "(", b::description("expr:") + b::space() - + expr.pretty() + + b::description(&expr.item) + b::description(",") + b::space() + b::description("subpath:") + b::space() - + subpath.pretty(), + + b::description(&subpath.item), ")", ) } @@ -185,7 +177,7 @@ impl PrettyDebug for ShellError { + b::space() + b::delimit( "(", - b::description("subpath:") + b::space() + subpath.pretty(), + b::description("subpath:") + b::space() + b::description(&subpath.item), ")", ) } @@ -295,8 +287,8 @@ impl ShellError { expr: Spanned>, ) -> ShellError { ProximateShellError::MissingProperty { - subpath: Description::from_spanned(subpath), - expr: Description::from_spanned(expr), + subpath: subpath.map(|s| s.into()), + expr: expr.map(|e| e.into()), } .start() } @@ -306,7 +298,7 @@ impl ShellError { integer: impl Into, ) -> ShellError { ProximateShellError::InvalidIntegerIndex { - subpath: Description::from_spanned(subpath), + subpath: subpath.map(|s| s.into()), integer: integer.into(), } .start() @@ -489,7 +481,7 @@ impl ShellError { Label::new_primary(span).with_message(format!( "Expected to convert {} to {} while {}, but it was out of range", item, - kind.desc(), + kind.display(), operation )), ), @@ -504,31 +496,33 @@ impl ShellError { .with_label(Label::new_primary(span).with_message(item)), ProximateShellError::MissingProperty { subpath, expr, .. } => { - let subpath = subpath.into_label(); - let expr = expr.into_label(); let mut diag = Diagnostic::new(Severity::Error, "Missing property"); - match subpath { - Ok(label) => diag = diag.with_label(label), - Err(ty) => diag.message = format!("Missing property (for {})", ty), - } + if subpath.span == Span::unknown() { + diag.message = format!("Missing property (for {})", subpath.item); + } else { + let subpath = Label::new_primary(subpath.span).with_message(subpath.item); + diag = diag.with_label(subpath); + + if expr.span != Span::unknown() { + let expr = Label::new_primary(expr.span).with_message(expr.item); + diag = diag.with_label(expr) + } - if let Ok(label) = expr { - diag = diag.with_label(label); } diag } ProximateShellError::InvalidIntegerIndex { subpath,integer } => { - let subpath = subpath.into_label(); - let mut diag = Diagnostic::new(Severity::Error, "Invalid integer property"); - match subpath { - Ok(label) => diag = diag.with_label(label), - Err(ty) => diag.message = format!("Invalid integer property (for {})", ty) + if subpath.span == Span::unknown() { + diag.message = format!("Invalid integer property (for {})", subpath.item) + } else { + let label = Label::new_primary(subpath.span).with_message(subpath.item); + diag = diag.with_label(label) } diag = diag.with_label(Label::new_secondary(integer).with_message("integer")); @@ -586,6 +580,10 @@ impl ShellError { } } +/// `ExpectedRange` describes a range of values that was expected by a command. In addition +/// to typical ranges, this enum allows an error to specify that the range of allowed values +/// corresponds to a particular numeric type (which is a dominant use-case for the +/// [RangeError](ProximateShellError::RangeError) error type). #[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Clone, Serialize, Deserialize)] pub enum ExpectedRange { I8, @@ -607,6 +605,7 @@ pub enum ExpectedRange { Range { start: usize, end: usize }, } +/// Convert a Rust range into an [ExpectedRange](ExpectedRange). impl From> for ExpectedRange { fn from(range: Range) -> Self { ExpectedRange::Range { @@ -618,13 +617,7 @@ impl From> for ExpectedRange { impl PrettyDebug for ExpectedRange { fn pretty(&self) -> DebugDocBuilder { - b::description(self.desc()) - } -} - -impl ExpectedRange { - fn desc(&self) -> String { - match self { + b::description(match self { ExpectedRange::I8 => "an 8-bit signed integer", ExpectedRange::I16 => "a 16-bit signed integer", ExpectedRange::I32 => "a 32-bit signed integer", @@ -641,9 +634,10 @@ impl ExpectedRange { ExpectedRange::Size => "a list offset", ExpectedRange::BigDecimal => "a decimal", ExpectedRange::BigInt => "an integer", - ExpectedRange::Range { start, end } => return format!("{} to {}", start, end), - } - .to_string() + ExpectedRange::Range { start, end } => { + return b::description(format!("{} to {}", start, end)) + } + }) } } @@ -661,11 +655,11 @@ pub enum ProximateShellError { actual: Spanned>, }, MissingProperty { - subpath: Description, - expr: Description, + subpath: Spanned, + expr: Spanned, }, InvalidIntegerIndex { - subpath: Description, + subpath: Spanned, integer: Span, }, MissingValue { diff --git a/crates/nu-parser/src/hir/tokens_iterator.rs b/crates/nu-parser/src/hir/tokens_iterator.rs index 5b44fcf3e8..49ddd25a9b 100644 --- a/crates/nu-parser/src/hir/tokens_iterator.rs +++ b/crates/nu-parser/src/hir/tokens_iterator.rs @@ -8,7 +8,7 @@ use crate::TokenNode; #[allow(unused)] use getset::{Getters, MutGetters}; use nu_errors::{ParseError, ShellError}; -use nu_source::{HasFallibleSpan, Span, SpannedItem, Spanned, HasSpan, Tag, Text}; +use nu_source::{HasFallibleSpan, HasSpan, Span, Spanned, SpannedItem, Tag, Text}; cfg_if::cfg_if! { if #[cfg(coloring_in_tokens)] { diff --git a/crates/nu-parser/src/parse/flag.rs b/crates/nu-parser/src/parse/flag.rs index ea3bd98578..5ee7ff02b5 100644 --- a/crates/nu-parser/src/parse/flag.rs +++ b/crates/nu-parser/src/parse/flag.rs @@ -1,7 +1,7 @@ use crate::hir::syntax_shape::flat_shape::FlatShape; use derive_new::new; use getset::Getters; -use nu_source::{Span, b, Spanned, SpannedItem, PrettyDebugWithSource, DebugDocBuilder}; +use nu_source::{b, DebugDocBuilder, PrettyDebugWithSource, Span, Spanned, SpannedItem}; use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Deserialize, Serialize)] diff --git a/crates/nu-parser/src/parse/operator.rs b/crates/nu-parser/src/parse/operator.rs index 3a5df5b4ce..99e5b5499e 100644 --- a/crates/nu-parser/src/parse/operator.rs +++ b/crates/nu-parser/src/parse/operator.rs @@ -1,5 +1,5 @@ +use nu_source::{b, DebugDocBuilder, PrettyDebug}; use serde::{Deserialize, Serialize}; -use nu_source::{b, PrettyDebug, DebugDocBuilder}; use std::str::FromStr; diff --git a/crates/nu-parser/src/parse/pipeline.rs b/crates/nu-parser/src/parse/pipeline.rs index b2bfe99af3..9752ce6117 100644 --- a/crates/nu-parser/src/parse/pipeline.rs +++ b/crates/nu-parser/src/parse/pipeline.rs @@ -1,7 +1,7 @@ use crate::TokenNode; use derive_new::new; use getset::Getters; -use nu_source::{b, DebugDocBuilder, PrettyDebugWithSource, Span, Spanned, HasSpan}; +use nu_source::{b, DebugDocBuilder, HasSpan, PrettyDebugWithSource, Span, Spanned}; #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Getters, new)] pub struct Pipeline { diff --git a/crates/nu-protocol/src/call_info.rs b/crates/nu-protocol/src/call_info.rs index 333890cc20..1a58aabb43 100644 --- a/crates/nu-protocol/src/call_info.rs +++ b/crates/nu-protocol/src/call_info.rs @@ -1,7 +1,7 @@ -use nu_errors::ShellError; use crate::value::Value; use derive_new::new; use indexmap::IndexMap; +use nu_errors::ShellError; use nu_source::Tag; use serde::{Deserialize, Serialize}; diff --git a/crates/nu-protocol/src/return_value.rs b/crates/nu-protocol/src/return_value.rs index e79b495446..42e948b197 100644 --- a/crates/nu-protocol/src/return_value.rs +++ b/crates/nu-protocol/src/return_value.rs @@ -1,5 +1,5 @@ -use nu_errors::ShellError; use crate::value::Value; +use nu_errors::ShellError; use nu_source::{b, DebugDocBuilder, PrettyDebug}; use serde::{Deserialize, Serialize}; diff --git a/crates/nu-protocol/src/value.rs b/crates/nu-protocol/src/value.rs index 72b4bf7dda..8e116315a7 100644 --- a/crates/nu-protocol/src/value.rs +++ b/crates/nu-protocol/src/value.rs @@ -139,6 +139,13 @@ impl Value { } } + pub fn as_forgiving_string(&self) -> Result<&str, ShellError> { + match &self.value { + UntaggedValue::Primitive(Primitive::String(string)) => Ok(&string[..]), + _ => Err(ShellError::type_error("string", self.spanned_type_name())), + } + } + pub fn as_path(&self) -> Result { match &self.value { UntaggedValue::Primitive(Primitive::Path(path)) => Ok(path.clone()), diff --git a/crates/nu-protocol/src/value/convert.rs b/crates/nu-protocol/src/value/convert.rs index 75dd3d0ad9..739d87b6e1 100644 --- a/crates/nu-protocol/src/value/convert.rs +++ b/crates/nu-protocol/src/value/convert.rs @@ -1,8 +1,8 @@ -use nu_errors::{CoerceInto, ShellError}; use crate::type_name::SpannedTypeName; use crate::value::dict::Dictionary; use crate::value::primitive::Primitive; use crate::value::{UntaggedValue, Value}; +use nu_errors::{CoerceInto, ShellError}; use nu_source::TaggedItem; impl std::convert::TryFrom<&Value> for i64 { diff --git a/src/cli.rs b/src/cli.rs index 717147393f..2c813c9dc4 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,5 +1,4 @@ -use crate::commands::classified::external::{run_external_command, StreamNext}; -use crate::commands::classified::internal::run_internal_command; +use crate::commands::classified::pipeline::run_pipeline; use crate::commands::classified::ClassifiedInputStream; use crate::commands::plugin::JsonRpc; use crate::commands::plugin::{PluginCommand, PluginSink}; @@ -14,7 +13,7 @@ use nu_parser::{ expand_syntax, hir, ClassifiedCommand, ClassifiedPipeline, InternalCommand, PipelineShape, TokenNode, TokensIterator, }; -use nu_protocol::{ReturnSuccess, Signature, UntaggedValue, Value}; +use nu_protocol::{Signature, UntaggedValue, Value}; use log::{debug, log_enabled, trace}; use rustyline::error::ReadlineError; @@ -582,113 +581,16 @@ async fn process_line(readline: Result, ctx: &mut Context })), } - let mut input = ClassifiedInputStream::new(); - let mut iter = pipeline.commands.list.into_iter().peekable(); - // Check the config to see if we need to update the path // TODO: make sure config is cached so we don't path this load every call set_env_from_config(); - loop { - let item: Option = iter.next(); - let next: Option<&ClassifiedCommand> = iter.peek(); + let input = ClassifiedInputStream::new(); - input = match (item, next) { - (None, _) => break, - - (Some(ClassifiedCommand::Dynamic(_)), _) - | (_, Some(ClassifiedCommand::Dynamic(_))) => { - return LineResult::Error( - line.to_string(), - ShellError::unimplemented("Dynamic commands"), - ) - } - - (Some(ClassifiedCommand::Expr(_)), _) => { - return LineResult::Error( - line.to_string(), - ShellError::unimplemented("Expression-only commands"), - ) - } - - (_, Some(ClassifiedCommand::Expr(_))) => { - return LineResult::Error( - line.to_string(), - ShellError::unimplemented("Expression-only commands"), - ) - } - - ( - Some(ClassifiedCommand::Internal(left)), - Some(ClassifiedCommand::External(_)), - ) => match run_internal_command(left, ctx, input, Text::from(line)).await { - Ok(val) => ClassifiedInputStream::from_input_stream(val), - Err(err) => return LineResult::Error(line.to_string(), err), - }, - - (Some(ClassifiedCommand::Internal(left)), Some(_)) => { - match run_internal_command(left, ctx, input, Text::from(line)).await { - Ok(val) => ClassifiedInputStream::from_input_stream(val), - Err(err) => return LineResult::Error(line.to_string(), err), - } - } - - (Some(ClassifiedCommand::Internal(left)), None) => { - match run_internal_command(left, ctx, input, Text::from(line)).await { - Ok(val) => { - use futures::stream::TryStreamExt; - - let mut output_stream: OutputStream = val.into(); - loop { - match output_stream.try_next().await { - Ok(Some(ReturnSuccess::Value(Value { - value: UntaggedValue::Error(e), - .. - }))) => { - return LineResult::Error(line.to_string(), e); - } - Ok(Some(_item)) => { - if ctx.ctrl_c.load(Ordering::SeqCst) { - break; - } - } - _ => { - break; - } - } - } - - return LineResult::Success(line.to_string()); - } - Err(err) => return LineResult::Error(line.to_string(), err), - } - } - - ( - Some(ClassifiedCommand::External(left)), - Some(ClassifiedCommand::External(_)), - ) => match run_external_command(left, ctx, input, StreamNext::External).await { - Ok(val) => val, - Err(err) => return LineResult::Error(line.to_string(), err), - }, - - (Some(ClassifiedCommand::External(left)), Some(_)) => { - match run_external_command(left, ctx, input, StreamNext::Internal).await { - Ok(val) => val, - Err(err) => return LineResult::Error(line.to_string(), err), - } - } - - (Some(ClassifiedCommand::External(left)), None) => { - match run_external_command(left, ctx, input, StreamNext::Last).await { - Ok(val) => val, - Err(err) => return LineResult::Error(line.to_string(), err), - } - } - }; + match run_pipeline(pipeline, ctx, input, line).await { + Ok(_) => LineResult::Success(line.to_string()), + Err(err) => LineResult::Error(line.to_string(), err), } - - LineResult::Success(line.to_string()) } Err(ReadlineError::Interrupted) => LineResult::CtrlC, Err(ReadlineError::Eof) => LineResult::Break, diff --git a/src/commands/classified/mod.rs b/src/commands/classified/mod.rs index 652cc90836..f786c8dabd 100644 --- a/src/commands/classified/mod.rs +++ b/src/commands/classified/mod.rs @@ -4,6 +4,7 @@ use crate::prelude::*; mod dynamic; pub(crate) mod external; pub(crate) mod internal; +pub(crate) mod pipeline; #[allow(unused_imports)] pub(crate) use dynamic::Command as DynamicCommand; diff --git a/src/commands/classified/pipeline.rs b/src/commands/classified/pipeline.rs index f40b627437..253f7b0c27 100644 --- a/src/commands/classified/pipeline.rs +++ b/src/commands/classified/pipeline.rs @@ -1,40 +1,74 @@ -use super::ClassifiedCommand; -use crate::prelude::*; +use crate::commands::classified::external::{run_external_command, StreamNext}; +use crate::commands::classified::internal::run_internal_command; +use crate::commands::classified::ClassifiedInputStream; +use crate::context::Context; +use crate::stream::OutputStream; +use nu_errors::ShellError; +use nu_parser::{ClassifiedCommand, ClassifiedPipeline}; +use nu_protocol::{ReturnSuccess, UntaggedValue, Value}; +use nu_source::Text; +use std::sync::atomic::Ordering; -#[derive(Debug, Clone)] -pub(crate) struct Pipeline { - pub(crate) commands: ClassifiedCommands, -} +pub(crate) async fn run_pipeline( + pipeline: ClassifiedPipeline, + ctx: &mut Context, + mut input: ClassifiedInputStream, + line: &str, +) -> Result<(), ShellError> { + let mut iter = pipeline.commands.list.into_iter().peekable(); -impl Pipeline { - pub fn commands(list: Vec, span: impl Into) -> Pipeline { - Pipeline { - commands: ClassifiedCommands { - list, - span: span.into(), - }, + loop { + let item: Option = iter.next(); + let next: Option<&ClassifiedCommand> = iter.peek(); + + input = match (item, next) { + (Some(ClassifiedCommand::Dynamic(_)), _) | (_, Some(ClassifiedCommand::Dynamic(_))) => { + return Err(ShellError::unimplemented("Dynamic commands")) + } + + (Some(ClassifiedCommand::Expr(_)), _) | (_, Some(ClassifiedCommand::Expr(_))) => { + return Err(ShellError::unimplemented("Expression-only commands")) + } + + (Some(ClassifiedCommand::Internal(left)), _) => { + let stream = run_internal_command(left, ctx, input, Text::from(line)).await?; + ClassifiedInputStream::from_input_stream(stream) + } + + (Some(ClassifiedCommand::External(left)), Some(ClassifiedCommand::External(_))) => { + run_external_command(left, ctx, input, StreamNext::External).await? + } + + (Some(ClassifiedCommand::External(left)), Some(_)) => { + run_external_command(left, ctx, input, StreamNext::Internal).await? + } + + (Some(ClassifiedCommand::External(left)), None) => { + run_external_command(left, ctx, input, StreamNext::Last).await? + } + + (None, _) => break, + }; + } + + use futures::stream::TryStreamExt; + let mut output_stream: OutputStream = input.objects.into(); + loop { + match output_stream.try_next().await { + Ok(Some(ReturnSuccess::Value(Value { + value: UntaggedValue::Error(e), + .. + }))) => return Err(e), + Ok(Some(_item)) => { + if ctx.ctrl_c.load(Ordering::SeqCst) { + break; + } + } + _ => { + break; + } } } -} -#[derive(Debug, Clone)] -pub struct ClassifiedCommands { - pub list: Vec, - pub span: Span, -} - -impl HasSpan for Pipeline { - fn span(&self) -> Span { - self.commands.span - } -} - -impl PrettyDebugWithSource for Pipeline { - fn pretty_debug(&self, source: &str) -> DebugDocBuilder { - b::intersperse( - self.commands.list.iter().map(|c| c.pretty_debug(source)), - b::operator(" | "), - ) - .or(b::delimit("<", b::description("empty pipeline"), ">")) - } + Ok(()) }