Pass ParseError location to CompileError

RustPython · windelbouwman · Jul 6, 2019 · Jul 5, 2019 · Jul 6, 2019 · Jul 6, 2019
commit 0e74d1649892ba66095a77dc6920f2f08272565e
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,10 +56,7 @@ impl fmt::Display for CompileError {
         }?;
 
         // Print line number:
-        match &self.error {
-            CompileErrorType::Parse(..) => Ok(()),
-            _ => 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,69 +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(LexicalError),
+    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),
-            InnerError::User { error } => ParseError::Other(error),
+            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)
             }
-            ParseError::Other(ref error) => write!(f, "{}", error),
+            ParseErrorType::Lexical(ref error) => write!(f, "{}", error),
         }
     }
 }

diff --git a/parser/src/lexer.rs b/parser/src/lexer.rs
@@ -75,23 +75,17 @@ pub enum LexicalErrorType {
     OtherError(String),
 }
 
-impl fmt::Display for LexicalError {
+impl fmt::Display for LexicalErrorType {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self.error {
+        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),
-        }?;
-        write!(
-            f,
-            " at line {} column {}",
-            self.location.row(),
-            self.location.column()
-        )
+        }
     }
 }
 
@@ -101,6 +95,12 @@ pub struct Location {
     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;