From cda53b6cdaf81633db424f3e55fbcf4b821d9642 Mon Sep 17 00:00:00 2001
From: Jason Gedge <jason.gedge@shopify.com>
Date: Sat, 1 Aug 2020 14:39:55 -0400
Subject: [PATCH] Return incomplete parse from lite_parse (#2284)

* Move lite_parse tests into a submodule

* Have lite_parse return partial parses when error encountered.

Although a parse fails, we can generally still return what was successfully
parsed. This is useful, for example, when figuring out completions at some
cursor position, because we can map the cursor to something more structured
(e.g., cursor is at a flag name).
---
 crates/nu-parser/src/errors.rs     |  19 +
 crates/nu-parser/src/lib.rs        |  12 +-
 crates/nu-parser/src/lite_parse.rs | 535 ++++++++++++++++-------------
 crates/nu-parser/src/parse.rs      |  18 +-
 4 files changed, 331 insertions(+), 253 deletions(-)
 create mode 100644 crates/nu-parser/src/errors.rs

diff --git a/crates/nu-parser/src/errors.rs b/crates/nu-parser/src/errors.rs
new file mode 100644
index 0000000000..a9b8d2b9f1
--- /dev/null
+++ b/crates/nu-parser/src/errors.rs
@@ -0,0 +1,19 @@
+use std::fmt::Debug;
+
+/// A combination of an informative parse error, and what has been successfully parsed so far
+#[derive(Debug)]
+pub struct ParseError<T: Debug> {
+    /// An informative cause for this parse error
+    pub(crate) cause: nu_errors::ParseError,
+
+    /// What has been successfully parsed, if anything
+    pub(crate) partial: Option<T>,
+}
+
+pub type ParseResult<T> = Result<T, ParseError<T>>;
+
+impl<T: Debug> From<ParseError<T>> for nu_errors::ShellError {
+    fn from(e: ParseError<T>) -> Self {
+        e.cause.into()
+    }
+}
diff --git a/crates/nu-parser/src/lib.rs b/crates/nu-parser/src/lib.rs
index c6f2f672e8..156ac21bd1 100644
--- a/crates/nu-parser/src/lib.rs
+++ b/crates/nu-parser/src/lib.rs
@@ -1,11 +1,13 @@
+mod errors;
 mod lite_parse;
 mod parse;
 mod path;
 mod shapes;
 mod signature;
 
-pub use crate::lite_parse::{lite_parse, LiteBlock};
-pub use crate::parse::{classify_block, garbage, parse_full_column_path};
-pub use crate::path::expand_ndots;
-pub use crate::shapes::shapes;
-pub use crate::signature::{Signature, SignatureRegistry};
+pub use errors::{ParseError, ParseResult};
+pub use lite_parse::{lite_parse, LiteBlock};
+pub use parse::{classify_block, garbage, parse_full_column_path};
+pub use path::expand_ndots;
+pub use shapes::shapes;
+pub use signature::{Signature, SignatureRegistry};
diff --git a/crates/nu-parser/src/lite_parse.rs b/crates/nu-parser/src/lite_parse.rs
index ae2c2cb857..c15d578e86 100644
--- a/crates/nu-parser/src/lite_parse.rs
+++ b/crates/nu-parser/src/lite_parse.rs
@@ -1,9 +1,10 @@
 use std::iter::Peekable;
 use std::str::CharIndices;
 
-use nu_errors::ParseError;
 use nu_source::{Span, Spanned, SpannedItem};
 
+use crate::errors::{ParseError, ParseResult};
+
 type Input<'t> = Peekable<CharIndices<'t>>;
 
 #[derive(Debug, Clone)]
@@ -39,6 +40,12 @@ pub struct LiteBlock {
     pub block: Vec<LitePipeline>,
 }
 
+impl From<Spanned<String>> for LiteCommand {
+    fn from(v: Spanned<String>) -> LiteCommand {
+        LiteCommand::new(v)
+    }
+}
+
 fn skip_whitespace(src: &mut Input) {
     while let Some((_, x)) = src.peek() {
         if x.is_whitespace() {
@@ -55,7 +62,7 @@ enum BlockKind {
     SquareBracket,
 }
 
-fn bare(src: &mut Input, span_offset: usize) -> Result<Spanned<String>, ParseError> {
+fn bare(src: &mut Input, span_offset: usize) -> ParseResult<Spanned<String>> {
     skip_whitespace(src);
 
     let mut bare = String::new();
@@ -107,199 +114,113 @@ fn bare(src: &mut Input, span_offset: usize) -> Result<Spanned<String>, ParseErr
     );
 
     if let Some(block) = block_level.last() {
-        return Err(ParseError::unexpected_eof(
-            match block {
-                BlockKind::Paren => ")",
-                BlockKind::SquareBracket => "]",
-                BlockKind::CurlyBracket => "}",
-            },
-            span,
-        ));
+        return Err(ParseError {
+            cause: nu_errors::ParseError::unexpected_eof(
+                match block {
+                    BlockKind::Paren => ")",
+                    BlockKind::SquareBracket => "]",
+                    BlockKind::CurlyBracket => "}",
+                },
+                span,
+            ),
+            partial: Some(bare.spanned(span)),
+        });
     }
 
     if let Some(delimiter) = inside_quote {
-        return Err(ParseError::unexpected_eof(delimiter.to_string(), span));
+        // The non-lite parse trims quotes on both sides, so we add the expected quote so that
+        // anyone wanting to consume this partial parse (e.g., completions) will be able to get
+        // correct information from the non-lite parse.
+        bare.push(delimiter);
+
+        let span = Span::new(
+            start_offset + span_offset,
+            start_offset + span_offset + bare.len(),
+        );
+
+        return Err(ParseError {
+            cause: nu_errors::ParseError::unexpected_eof(delimiter.to_string(), span),
+            partial: Some(bare.spanned(span)),
+        });
+    }
+
+    if bare.is_empty() {
+        return Err(ParseError {
+            cause: nu_errors::ParseError::unexpected_eof("command", span),
+            partial: Some(bare.spanned(span)),
+        });
     }
 
     Ok(bare.spanned(span))
 }
 
-#[test]
-fn bare_simple_1() -> Result<(), ParseError> {
-    let input = "foo bar baz";
+fn command(src: &mut Input, span_offset: usize) -> ParseResult<LiteCommand> {
+    let mut cmd = match bare(src, span_offset) {
+        Ok(v) => LiteCommand::new(v),
+        Err(e) => {
+            return Err(ParseError {
+                cause: e.cause,
+                partial: e.partial.map(LiteCommand::new),
+            });
+        }
+    };
 
-    let input = &mut input.char_indices().peekable();
-    let result = bare(input, 0)?;
+    loop {
+        skip_whitespace(src);
 
-    assert_eq!(result.span.start(), 0);
-    assert_eq!(result.span.end(), 3);
+        if let Some((_, c)) = src.peek() {
+            // The first character tells us a lot about each argument
+            match c {
+                ';' => {
+                    // this is the end of the command and the end of the pipeline
+                    break;
+                }
+                '|' => {
+                    let _ = src.next();
+                    if let Some((pos, next_c)) = src.peek() {
+                        if *next_c == '|' {
+                            // this isn't actually a pipeline but a comparison
+                            let span = Span::new(pos - 1 + span_offset, pos + 1 + span_offset);
+                            cmd.args.push("||".to_string().spanned(span));
+                            let _ = src.next();
+                        } else {
+                            // this is the end of this command
+                            break;
+                        }
+                    } else {
+                        // this is the end of this command
+                        break;
+                    }
+                }
+                _ => {
+                    // basic argument
+                    match bare(src, span_offset) {
+                        Ok(v) => {
+                            cmd.args.push(v);
+                        }
 
-    Ok(())
-}
+                        Err(e) => {
+                            if let Some(v) = e.partial {
+                                cmd.args.push(v);
+                            }
 
-#[test]
-fn bare_simple_2() -> Result<(), ParseError> {
-    let input = "'foo bar' baz";
-
-    let input = &mut input.char_indices().peekable();
-    let result = bare(input, 0)?;
-
-    assert_eq!(result.span.start(), 0);
-    assert_eq!(result.span.end(), 9);
-
-    Ok(())
-}
-
-#[test]
-fn bare_simple_3() -> Result<(), ParseError> {
-    let input = "'foo\" bar' baz";
-
-    let input = &mut input.char_indices().peekable();
-    let result = bare(input, 0)?;
-
-    assert_eq!(result.span.start(), 0);
-    assert_eq!(result.span.end(), 10);
-
-    Ok(())
-}
-
-#[test]
-fn bare_simple_4() -> Result<(), ParseError> {
-    let input = "[foo bar] baz";
-
-    let input = &mut input.char_indices().peekable();
-    let result = bare(input, 0)?;
-
-    assert_eq!(result.span.start(), 0);
-    assert_eq!(result.span.end(), 9);
-
-    Ok(())
-}
-
-#[test]
-fn bare_simple_5() -> Result<(), ParseError> {
-    let input = "'foo 'bar baz";
-
-    let input = &mut input.char_indices().peekable();
-    let result = bare(input, 0)?;
-
-    assert_eq!(result.span.start(), 0);
-    assert_eq!(result.span.end(), 9);
-
-    Ok(())
-}
-
-#[test]
-fn bare_simple_6() -> Result<(), ParseError> {
-    let input = "''foo baz";
-
-    let input = &mut input.char_indices().peekable();
-    let result = bare(input, 0)?;
-
-    assert_eq!(result.span.start(), 0);
-    assert_eq!(result.span.end(), 5);
-
-    Ok(())
-}
-
-#[test]
-fn bare_simple_7() -> Result<(), ParseError> {
-    let input = "'' foo";
-
-    let input = &mut input.char_indices().peekable();
-    let result = bare(input, 0)?;
-
-    assert_eq!(result.span.start(), 0);
-    assert_eq!(result.span.end(), 2);
-
-    Ok(())
-}
-
-#[test]
-fn bare_simple_8() -> Result<(), ParseError> {
-    let input = " '' foo";
-
-    let input = &mut input.char_indices().peekable();
-    let result = bare(input, 0)?;
-
-    assert_eq!(result.span.start(), 1);
-    assert_eq!(result.span.end(), 3);
-
-    Ok(())
-}
-
-#[test]
-fn bare_simple_9() -> Result<(), ParseError> {
-    let input = " 'foo' foo";
-
-    let input = &mut input.char_indices().peekable();
-    let result = bare(input, 0)?;
-
-    assert_eq!(result.span.start(), 1);
-    assert_eq!(result.span.end(), 6);
-
-    Ok(())
-}
-
-#[test]
-fn bare_ignore_future() -> Result<(), ParseError> {
-    let input = "foo 'bar";
-
-    let input = &mut input.char_indices().peekable();
-    let result = bare(input, 0)?;
-
-    assert_eq!(result.span.start(), 0);
-    assert_eq!(result.span.end(), 3);
-
-    Ok(())
-}
-
-#[test]
-fn bare_invalid_1() -> Result<(), ParseError> {
-    let input = "'foo bar";
-
-    let input = &mut input.char_indices().peekable();
-    let result = bare(input, 0);
-
-    assert_eq!(result.is_ok(), false);
-
-    Ok(())
-}
-
-#[test]
-fn bare_invalid_2() -> Result<(), ParseError> {
-    let input = "'bar";
-
-    let input = &mut input.char_indices().peekable();
-    let result = bare(input, 0);
-
-    assert_eq!(result.is_ok(), false);
-
-    Ok(())
-}
-
-#[test]
-fn bare_invalid_4() -> Result<(), ParseError> {
-    let input = " 'bar";
-
-    let input = &mut input.char_indices().peekable();
-    let result = bare(input, 0);
-
-    assert_eq!(result.is_ok(), false);
-
-    Ok(())
-}
-
-fn command(src: &mut Input, span_offset: usize) -> Result<LiteCommand, ParseError> {
-    let command = bare(src, span_offset)?;
-    if command.item.is_empty() {
-        Err(ParseError::unexpected_eof("command", command.span))
-    } else {
-        Ok(LiteCommand::new(command))
+                            return Err(ParseError {
+                                cause: e.cause,
+                                partial: Some(cmd),
+                            });
+                        }
+                    }
+                }
+            }
+        } else {
+            break;
+        }
     }
+
+    Ok(cmd)
 }
 
-fn pipeline(src: &mut Input, span_offset: usize) -> Result<LiteBlock, ParseError> {
+fn pipeline(src: &mut Input, span_offset: usize) -> ParseResult<LiteBlock> {
     let mut block = vec![];
     let mut commands = vec![];
 
@@ -307,49 +228,21 @@ fn pipeline(src: &mut Input, span_offset: usize) -> Result<LiteBlock, ParseError
 
     while src.peek().is_some() {
         // If there is content there, let's parse it
+        let cmd = match command(src, span_offset) {
+            Ok(v) => v,
+            Err(e) => {
+                if let Some(partial) = e.partial {
+                    commands.push(partial);
+                    block.push(LitePipeline { commands });
+                }
 
-        let mut cmd = match command(src, span_offset) {
-            Ok(cmd) => cmd,
-            Err(e) => return Err(e),
+                return Err(ParseError {
+                    cause: e.cause,
+                    partial: Some(LiteBlock { block }),
+                });
+            }
         };
 
-        loop {
-            skip_whitespace(src);
-
-            if let Some((_, c)) = src.peek() {
-                // The first character tells us a lot about each argument
-                match c {
-                    ';' => {
-                        // this is the end of the command and the end of the pipeline
-                        break;
-                    }
-                    '|' => {
-                        let _ = src.next();
-                        if let Some((pos, next_c)) = src.peek() {
-                            if *next_c == '|' {
-                                // this isn't actually a pipeline but a comparison
-                                let span = Span::new(pos - 1 + span_offset, pos + 1 + span_offset);
-                                cmd.args.push("||".to_string().spanned(span));
-                                let _ = src.next();
-                            } else {
-                                // this is the end of this command
-                                break;
-                            }
-                        } else {
-                            // this is the end of this command
-                            break;
-                        }
-                    }
-                    _ => {
-                        // basic argument
-                        let arg = bare(src, span_offset)?;
-                        cmd.args.push(arg);
-                    }
-                }
-            } else {
-                break;
-            }
-        }
         commands.push(cmd);
         skip_whitespace(src);
 
@@ -370,28 +263,190 @@ fn pipeline(src: &mut Input, span_offset: usize) -> Result<LiteBlock, ParseError
     Ok(LiteBlock { block })
 }
 
-pub fn lite_parse(src: &str, span_offset: usize) -> Result<LiteBlock, ParseError> {
+pub fn lite_parse(src: &str, span_offset: usize) -> ParseResult<LiteBlock> {
     pipeline(&mut src.char_indices().peekable(), span_offset)
 }
 
-#[test]
-fn lite_simple_1() -> Result<(), ParseError> {
-    let result = lite_parse("foo", 0)?;
-    assert_eq!(result.block.len(), 1);
-    assert_eq!(result.block[0].commands.len(), 1);
-    assert_eq!(result.block[0].commands[0].name.span.start(), 0);
-    assert_eq!(result.block[0].commands[0].name.span.end(), 3);
+#[cfg(test)]
+mod tests {
+    use super::*;
 
-    Ok(())
-}
-
-#[test]
-fn lite_simple_offset() -> Result<(), ParseError> {
-    let result = lite_parse("foo", 10)?;
-    assert_eq!(result.block.len(), 1);
-    assert_eq!(result.block[0].commands.len(), 1);
-    assert_eq!(result.block[0].commands[0].name.span.start(), 10);
-    assert_eq!(result.block[0].commands[0].name.span.end(), 13);
-
-    Ok(())
+    mod bare {
+        use super::*;
+
+        #[test]
+        fn simple_1() {
+            let input = "foo bar baz";
+
+            let input = &mut input.char_indices().peekable();
+            let result = bare(input, 0).unwrap();
+
+            assert_eq!(result.span.start(), 0);
+            assert_eq!(result.span.end(), 3);
+        }
+
+        #[test]
+        fn simple_2() {
+            let input = "'foo bar' baz";
+
+            let input = &mut input.char_indices().peekable();
+            let result = bare(input, 0).unwrap();
+
+            assert_eq!(result.span.start(), 0);
+            assert_eq!(result.span.end(), 9);
+        }
+
+        #[test]
+        fn simple_3() {
+            let input = "'foo\" bar' baz";
+
+            let input = &mut input.char_indices().peekable();
+            let result = bare(input, 0).unwrap();
+
+            assert_eq!(result.span.start(), 0);
+            assert_eq!(result.span.end(), 10);
+        }
+
+        #[test]
+        fn simple_4() {
+            let input = "[foo bar] baz";
+
+            let input = &mut input.char_indices().peekable();
+            let result = bare(input, 0).unwrap();
+
+            assert_eq!(result.span.start(), 0);
+            assert_eq!(result.span.end(), 9);
+        }
+
+        #[test]
+        fn simple_5() {
+            let input = "'foo 'bar baz";
+
+            let input = &mut input.char_indices().peekable();
+            let result = bare(input, 0).unwrap();
+
+            assert_eq!(result.span.start(), 0);
+            assert_eq!(result.span.end(), 9);
+        }
+
+        #[test]
+        fn simple_6() {
+            let input = "''foo baz";
+
+            let input = &mut input.char_indices().peekable();
+            let result = bare(input, 0).unwrap();
+
+            assert_eq!(result.span.start(), 0);
+            assert_eq!(result.span.end(), 5);
+        }
+
+        #[test]
+        fn simple_7() {
+            let input = "'' foo";
+
+            let input = &mut input.char_indices().peekable();
+            let result = bare(input, 0).unwrap();
+
+            assert_eq!(result.span.start(), 0);
+            assert_eq!(result.span.end(), 2);
+        }
+
+        #[test]
+        fn simple_8() {
+            let input = " '' foo";
+
+            let input = &mut input.char_indices().peekable();
+            let result = bare(input, 0).unwrap();
+
+            assert_eq!(result.span.start(), 1);
+            assert_eq!(result.span.end(), 3);
+        }
+
+        #[test]
+        fn simple_9() {
+            let input = " 'foo' foo";
+
+            let input = &mut input.char_indices().peekable();
+            let result = bare(input, 0).unwrap();
+
+            assert_eq!(result.span.start(), 1);
+            assert_eq!(result.span.end(), 6);
+        }
+
+        #[test]
+        fn ignore_future() {
+            let input = "foo 'bar";
+
+            let input = &mut input.char_indices().peekable();
+            let result = bare(input, 0).unwrap();
+
+            assert_eq!(result.span.start(), 0);
+            assert_eq!(result.span.end(), 3);
+        }
+
+        #[test]
+        fn invalid_1() {
+            let input = "'foo bar";
+
+            let input = &mut input.char_indices().peekable();
+            let result = bare(input, 0);
+
+            assert_eq!(result.is_ok(), false);
+        }
+
+        #[test]
+        fn invalid_2() {
+            let input = "'bar";
+
+            let input = &mut input.char_indices().peekable();
+            let result = bare(input, 0);
+
+            assert_eq!(result.is_ok(), false);
+        }
+
+        #[test]
+        fn invalid_4() {
+            let input = " 'bar";
+
+            let input = &mut input.char_indices().peekable();
+            let result = bare(input, 0);
+
+            assert_eq!(result.is_ok(), false);
+        }
+    }
+
+    mod lite_parse {
+        use super::*;
+
+        #[test]
+        fn simple_1() {
+            let result = lite_parse("foo", 0).unwrap();
+            assert_eq!(result.block.len(), 1);
+            assert_eq!(result.block[0].commands.len(), 1);
+            assert_eq!(result.block[0].commands[0].name.span.start(), 0);
+            assert_eq!(result.block[0].commands[0].name.span.end(), 3);
+        }
+
+        #[test]
+        fn simple_offset() {
+            let result = lite_parse("foo", 10).unwrap();
+            assert_eq!(result.block.len(), 1);
+            assert_eq!(result.block[0].commands.len(), 1);
+            assert_eq!(result.block[0].commands[0].name.span.start(), 10);
+            assert_eq!(result.block[0].commands[0].name.span.end(), 13);
+        }
+
+        #[test]
+        fn incomplete_result() {
+            let result = lite_parse("my_command \"foo' --test", 10).unwrap_err();
+            assert!(matches!(result.cause.reason(), nu_errors::ParseErrorReason::Eof { .. }));
+
+            let result = result.partial.unwrap();
+            assert_eq!(result.block.len(), 1);
+            assert_eq!(result.block[0].commands.len(), 1);
+            assert_eq!(result.block[0].commands[0].name.item, "my_command");
+            assert_eq!(result.block[0].commands[0].args.len(), 1);
+            assert_eq!(result.block[0].commands[0].args[0].item, "\"foo' --test\"");
+        }
+    }
 }
diff --git a/crates/nu-parser/src/parse.rs b/crates/nu-parser/src/parse.rs
index c5ab69e684..620f228ea2 100644
--- a/crates/nu-parser/src/parse.rs
+++ b/crates/nu-parser/src/parse.rs
@@ -1,8 +1,5 @@
 use std::path::Path;
 
-use crate::lite_parse::{lite_parse, LiteBlock, LiteCommand, LitePipeline};
-use crate::path::expand_path;
-use crate::signature::SignatureRegistry;
 use log::trace;
 use nu_errors::{ArgumentError, ParseError};
 use nu_protocol::hir::{
@@ -14,6 +11,11 @@ use nu_protocol::{NamedType, PositionalType, Signature, SyntaxShape, UnspannedPa
 use nu_source::{Span, Spanned, SpannedItem};
 use num_bigint::BigInt;
 
+//use crate::errors::{ParseError, ParseResult};
+use crate::lite_parse::{lite_parse, LiteBlock, LiteCommand, LitePipeline};
+use crate::path::expand_path;
+use crate::signature::SignatureRegistry;
+
 /// Parses a simple column path, one without a variable (implied or explicit) at the head
 fn parse_simple_column_path(lite_arg: &Spanned<String>) -> (SpannedExpression, Option<ParseError>) {
     let mut delimiter = '.';
@@ -116,7 +118,7 @@ pub fn parse_full_column_path(
                 // We haven't done much with the inner string, so let's go ahead and work with it
                 let lite_block = match lite_parse(&string, lite_arg.span.start() + 2) {
                     Ok(lp) => lp,
-                    Err(e) => return (garbage(lite_arg.span), Some(e)),
+                    Err(e) => return (garbage(lite_arg.span), Some(e.cause)),
                 };
 
                 let classified_block = classify_block(&lite_block, registry);
@@ -166,7 +168,7 @@ pub fn parse_full_column_path(
                 // We haven't done much with the inner string, so let's go ahead and work with it
                 let lite_block = match lite_parse(&string, lite_arg.span.start() + 2) {
                     Ok(lp) => lp,
-                    Err(e) => return (garbage(lite_arg.span), Some(e)),
+                    Err(e) => return (garbage(lite_arg.span), Some(e.cause)),
                 };
 
                 let classified_block = classify_block(&lite_block, registry);
@@ -647,7 +649,7 @@ fn parse_arg(
                     // We haven't done much with the inner string, so let's go ahead and work with it
                     let lite_block = match lite_parse(&string, lite_arg.span.start() + 1) {
                         Ok(lb) => lb,
-                        Err(e) => return (garbage(lite_arg.span), Some(e)),
+                        Err(e) => return (garbage(lite_arg.span), Some(e.cause)),
                     };
 
                     if lite_block.block.is_empty() {
@@ -707,7 +709,7 @@ fn parse_arg(
                     // We haven't done much with the inner string, so let's go ahead and work with it
                     let lite_block = match lite_parse(&string, lite_arg.span.start() + 1) {
                         Ok(lp) => lp,
-                        Err(e) => return (garbage(lite_arg.span), Some(e)),
+                        Err(e) => return (garbage(lite_arg.span), Some(e.cause)),
                     };
 
                     let classified_block = classify_block(&lite_block, registry);
@@ -902,7 +904,7 @@ fn parse_parenthesized_expression(
             // We haven't done much with the inner string, so let's go ahead and work with it
             let lite_block = match lite_parse(&string, lite_arg.span.start() + 1) {
                 Ok(lb) => lb,
-                Err(e) => return (garbage(lite_arg.span), Some(e)),
+                Err(e) => return (garbage(lite_arg.span), Some(e.cause)),
             };
 
             if lite_block.block.len() != 1 {