Merge pull request RustPython#1106 from palaviv/syntex-error

windelbouwman · web-flow · commit 70c20543f833 · 2019-07-06T13:02:11.000+02:00
Pass more information in user defined parse error
diff --git a/compiler/src/error.rs b/compiler/src/error.rs
@@ -1,4 +1,4 @@
-use rustpython_parser::error::ParseError;
+use rustpython_parser::error::{ParseError, ParseErrorType};
 use rustpython_parser::lexer::Location;
 
 use std::error::Error;
@@ -13,8 +13,8 @@ pub struct CompileError {
 impl From<ParseError> for CompileError {
     fn from(error: ParseError) -> Self {
         CompileError {
-            error: CompileErrorType::Parse(error),
-            location: Default::default(), // TODO: extract location from parse error!
+            error: CompileErrorType::Parse(error.error),
+            location: error.location,
         }
     }
 }
@@ -28,7 +28,7 @@ pub enum CompileErrorType {
     /// Expected an expression got a statement
     ExpectExpr,
     /// Parser error
-    Parse(ParseError),
+    Parse(ParseErrorType),
     SyntaxError(String),
     /// Multiple `*` detected
     StarArgs,
@@ -56,7 +56,7 @@ impl fmt::Display for CompileError {
         }?;
 
         // Print line number:
-        write!(f, " at line {:?}", self.location.row())
+        write!(f, " at {}", self.location)
     }
 }
 
diff --git a/parser/src/error.rs b/parser/src/error.rs
@@ -3,71 +3,83 @@
 extern crate lalrpop_util;
 use self::lalrpop_util::ParseError as InnerError;
 
-use crate::lexer::{LexicalError, Location};
+use crate::lexer::{LexicalError, LexicalErrorType, Location};
 use crate::token::Tok;
 
 use std::error::Error;
 use std::fmt;
 
-// A token of type `Tok` was observed, with a span given by the two Location values
-type TokSpan = (Location, Tok, Location);
-
 /// Represents an error during parsing
 #[derive(Debug, PartialEq)]
-pub enum ParseError {
+pub struct ParseError {
+    pub error: ParseErrorType,
+    pub location: Location,
+}
+
+#[derive(Debug, PartialEq)]
+pub enum ParseErrorType {
     /// Parser encountered an unexpected end of input
-    EOF(Option<Location>),
+    EOF,
     /// Parser encountered an extra token
-    ExtraToken(TokSpan),
+    ExtraToken(Tok),
     /// Parser encountered an invalid token
-    InvalidToken(Location),
+    InvalidToken,
     /// Parser encountered an unexpected token
-    UnrecognizedToken(TokSpan, Vec<String>),
+    UnrecognizedToken(Tok, Vec<String>),
     /// Maps to `User` type from `lalrpop-util`
-    Other,
+    Lexical(LexicalErrorType),
 }
 
 /// Convert `lalrpop_util::ParseError` to our internal type
 impl From<InnerError<Location, Tok, LexicalError>> for ParseError {
     fn from(err: InnerError<Location, Tok, LexicalError>) -> Self {
         match err {
             // TODO: Are there cases where this isn't an EOF?
-            InnerError::InvalidToken { location } => ParseError::EOF(Some(location)),
-            InnerError::ExtraToken { token } => ParseError::ExtraToken(token),
-            // Inner field is a unit-like enum `LexicalError::StringError` with no useful info
-            InnerError::User { .. } => ParseError::Other,
+            InnerError::InvalidToken { location } => ParseError {
+                error: ParseErrorType::EOF,
+                location,
+            },
+            InnerError::ExtraToken { token } => ParseError {
+                error: ParseErrorType::ExtraToken(token.1),
+                location: token.0,
+            },
+            InnerError::User { error } => ParseError {
+                error: ParseErrorType::Lexical(error.error),
+                location: error.location,
+            },
             InnerError::UnrecognizedToken { token, expected } => {
                 match token {
-                    Some(tok) => ParseError::UnrecognizedToken(tok, expected),
+                    Some(tok) => ParseError {
+                        error: ParseErrorType::UnrecognizedToken(tok.1, expected),
+                        location: tok.0,
+                    },
                     // EOF was observed when it was unexpected
-                    None => ParseError::EOF(None),
+                    None => ParseError {
+                        error: ParseErrorType::EOF,
+                        location: Default::default(),
+                    },
                 }
             }
         }
     }
 }
 
 impl fmt::Display for ParseError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{} at {}", self.error, self.location)
+    }
+}
+
+impl fmt::Display for ParseErrorType {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
-            ParseError::EOF(ref location) => {
-                if let Some(l) = location {
-                    write!(f, "Got unexpected EOF at: {:?}", l)
-                } else {
-                    write!(f, "Got unexpected EOF")
-                }
-            }
-            ParseError::ExtraToken(ref t_span) => {
-                write!(f, "Got extraneous token: {:?} at: {:?}", t_span.1, t_span.0)
-            }
-            ParseError::InvalidToken(ref location) => {
-                write!(f, "Got invalid token at: {:?}", location)
-            }
-            ParseError::UnrecognizedToken(ref t_span, _) => {
-                write!(f, "Got unexpected token: {:?} at {:?}", t_span.1, t_span.0)
+            ParseErrorType::EOF => write!(f, "Got unexpected EOF"),
+            ParseErrorType::ExtraToken(ref tok) => write!(f, "Got extraneous token: {:?}", tok),
+            ParseErrorType::InvalidToken => write!(f, "Got invalid token"),
+            ParseErrorType::UnrecognizedToken(ref tok, _) => {
+                write!(f, "Got unexpected token: {:?}", tok)
             }
-            // This is user defined, it probably means a more useful error should have been given upstream.
-            ParseError::Other => write!(f, "Got unsupported token(s)"),
+            ParseErrorType::Lexical(ref error) => write!(f, "{}", error),
         }
     }
 }
diff --git a/parser/src/lexer.rs b/parser/src/lexer.rs
@@ -10,6 +10,7 @@ use num_traits::Num;
 use serde::{Deserialize, Serialize};
 use std::cmp::Ordering;
 use std::collections::HashMap;
+use std::fmt;
 use std::str::FromStr;
 use unic_emoji_char::is_emoji_presentation;
 use unicode_xid::UnicodeXID;
@@ -59,13 +60,13 @@ pub struct Lexer<T: Iterator<Item = char>> {
     keywords: HashMap<String, Tok>,
 }
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq)]
 pub struct LexicalError {
     pub error: LexicalErrorType,
     pub location: Location,
 }
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq)]
 pub enum LexicalErrorType {
     StringError,
     UnicodeError,
@@ -74,12 +75,32 @@ pub enum LexicalErrorType {
     OtherError(String),
 }
 
+impl fmt::Display for LexicalErrorType {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match *self {
+            LexicalErrorType::StringError => write!(f, "Got unexpected string"),
+            LexicalErrorType::UnicodeError => write!(f, "Got unexpected unicode"),
+            LexicalErrorType::NestingError => write!(f, "Got unexpected nesting"),
+            LexicalErrorType::UnrecognizedToken { tok } => {
+                write!(f, "Got unexpected token {}", tok)
+            }
+            LexicalErrorType::OtherError(ref msg) => write!(f, "{}", msg),
+        }
+    }
+}
+
 #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)]
 pub struct Location {
     row: usize,
     column: usize,
 }
 
+impl fmt::Display for Location {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "line {} column {}", self.row, self.column)
+    }
+}
+
 impl Location {
     pub fn new(row: usize, column: usize) -> Self {
         Location { row, column }
diff --git a/src/main.rs b/src/main.rs
@@ -7,7 +7,7 @@ extern crate rustyline;
 
 use clap::{App, Arg};
 use rustpython_compiler::{compile, error::CompileError, error::CompileErrorType};
-use rustpython_parser::error::ParseError;
+use rustpython_parser::error::ParseErrorType;
 use rustpython_vm::{
     frame::Scope,
     import,
@@ -181,7 +181,7 @@ fn shell_exec(vm: &VirtualMachine, source: &str, scope: Scope) -> Result<(), Com
         // Don't inject syntax errors for line continuation
         Err(
             err @ CompileError {
-                error: CompileErrorType::Parse(ParseError::EOF(_)),
+                error: CompileErrorType::Parse(ParseErrorType::EOF),
                 ..
             },
         ) => Err(err),
@@ -258,7 +258,7 @@ fn run_shell(vm: &VirtualMachine) -> PyResult {
 
                 match shell_exec(vm, &input, vars.clone()) {
                     Err(CompileError {
-                        error: CompileErrorType::Parse(ParseError::EOF(_)),
+                        error: CompileErrorType::Parse(ParseErrorType::EOF),
                         ..
                     }) => {
                         continuing = true;
diff --git a/wasm/lib/src/vm_class.rs b/wasm/lib/src/vm_class.rs
@@ -281,30 +281,13 @@ impl WASMVirtualMachine {
                 let code = vm.compile(&source, &mode, "<wasm>".to_string());
                 let code = code.map_err(|err| {
                     let js_err = SyntaxError::new(&format!("Error parsing Python code: {}", err));
-                    if let CompileErrorType::Parse(ref parse_error) = err.error {
-                        use rustpython_parser::error::ParseError;
-                        if let ParseError::EOF(Some(ref loc))
-                        | ParseError::ExtraToken((ref loc, ..))
-                        | ParseError::InvalidToken(ref loc)
-                        | ParseError::UnrecognizedToken((ref loc, ..), _) = parse_error
-                        {
-                            let _ =
-                                Reflect::set(&js_err, &"row".into(), &(loc.row() as u32).into());
-                            let _ =
-                                Reflect::set(&js_err, &"col".into(), &(loc.column() as u32).into());
-                        }
-                        if let ParseError::ExtraToken((_, _, ref loc))
-                        | ParseError::UnrecognizedToken((_, _, ref loc), _) = parse_error
-                        {
-                            let _ =
-                                Reflect::set(&js_err, &"endrow".into(), &(loc.row() as u32).into());
-                            let _ = Reflect::set(
-                                &js_err,
-                                &"endcol".into(),
-                                &(loc.column() as u32).into(),
-                            );
-                        }
-                    }
+                    let _ =
+                        Reflect::set(&js_err, &"row".into(), &(err.location.row() as u32).into());
+                    let _ = Reflect::set(
+                        &js_err,
+                        &"col".into(),
+                        &(err.location.column() as u32).into(),
+                    );
                     js_err
                 })?;
                 let result = vm.run_code_obj(code, scope.borrow().clone());

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-use rustpython_parser::error::ParseError;`
	`1`	`+use rustpython_parser::error::{ParseError, ParseErrorType};`
`2`	`2`	`use rustpython_parser::lexer::Location;`
`3`	`3`
`4`	`4`	`use std::error::Error;`
`@@ -13,8 +13,8 @@ pub struct CompileError {`
`13`	`13`	`impl From<ParseError> for CompileError {`
`14`	`14`	`fn from(error: ParseError) -> Self {`
`15`	`15`	`CompileError {`
`16`		`- error: CompileErrorType::Parse(error),`
`17`		`- location: Default::default(), // TODO: extract location from parse error!`
	`16`	`+ error: CompileErrorType::Parse(error.error),`
	`17`	`+ location: error.location,`
`18`	`18`	`}`
`19`	`19`	`}`
`20`	`20`	`}`
`@@ -28,7 +28,7 @@ pub enum CompileErrorType {`
`28`	`28`	`/// Expected an expression got a statement`
`29`	`29`	`ExpectExpr,`
`30`	`30`	`/// Parser error`
`31`		`- Parse(ParseError),`
	`31`	`+ Parse(ParseErrorType),`
`32`	`32`	`SyntaxError(String),`
`33`	`33`	/// Multiple `*` detected
`34`	`34`	`StarArgs,`
`@@ -56,7 +56,7 @@ impl fmt::Display for CompileError {`
`56`	`56`	`}?;`
`57`	`57`
`58`	`58`	`// Print line number:`
`59`		`- write!(f, " at line {:?}", self.location.row())`
	`59`	`+ write!(f, " at {}", self.location)`
`60`	`60`	`}`
`61`	`61`	`}`
`62`	`62`