From eb16f1656647e3707364bb422da1ed493cae49d8 Mon Sep 17 00:00:00 2001 From: Windel Bouwman Date: Wed, 17 Apr 2019 20:02:24 +0200 Subject: [PATCH 1/2] Improve syntax error with line information. --- src/main.rs | 6 ++-- tests/snippets/invalid_syntax.py | 16 +++++++++++ vm/src/builtins.rs | 1 + vm/src/compile.rs | 48 ++++++++++++++++++++++++-------- vm/src/error.rs | 43 ++++++++++++++++++++-------- vm/src/exceptions.rs | 5 +++- vm/src/vm.rs | 10 +++++-- wasm/lib/src/vm_class.rs | 4 ++- 8 files changed, 101 insertions(+), 32 deletions(-) create mode 100644 tests/snippets/invalid_syntax.py diff --git a/src/main.rs b/src/main.rs index 2a673736ef..326e0bc3e0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,7 +10,7 @@ extern crate rustyline; use clap::{App, Arg}; use rustpython_parser::error::ParseError; use rustpython_vm::{ - compile, error::CompileError, frame::Scope, import, obj::objstr, print_exception, + compile, error::CompileError, error::CompileErrorType, frame::Scope, import, obj::objstr, print_exception, pyobject::PyResult, util, VirtualMachine, }; use rustyline::{error::ReadlineError, Editor}; @@ -115,7 +115,7 @@ fn shell_exec(vm: &VirtualMachine, source: &str, scope: Scope) -> Result<(), Com Ok(()) } // Don't inject syntax errors for line continuation - Err(err @ CompileError::Parse(ParseError::EOF(_))) => Err(err), + Err(err @ CompileError{ error: CompileErrorType::Parse(ParseError::EOF(_)), .. }) => Err(err), Err(err) => { let exc = vm.new_syntax_error(&err); print_exception(vm, &exc); @@ -188,7 +188,7 @@ fn run_shell(vm: &VirtualMachine) -> PyResult { } match shell_exec(vm, &input, vars.clone()) { - Err(CompileError::Parse(ParseError::EOF(_))) => { + Err(CompileError { error: CompileErrorType::Parse(ParseError::EOF(_)), .. }) => { continuing = true; continue; } diff --git a/tests/snippets/invalid_syntax.py b/tests/snippets/invalid_syntax.py new file mode 100644 index 0000000000..bbbd08a41e --- /dev/null +++ b/tests/snippets/invalid_syntax.py @@ -0,0 +1,16 @@ + + +src = """ +def valid_func(): + pass + +yield 2 +""" + +try: + compile(src, 'test.py', 'exec') +except SyntaxError as ex: + assert ex.lineno == 5 +else: + raise AssertionError("Must throw syntax error") + diff --git a/vm/src/builtins.rs b/vm/src/builtins.rs index f2ca1e4c12..d054c34fca 100644 --- a/vm/src/builtins.rs +++ b/vm/src/builtins.rs @@ -751,6 +751,7 @@ pub fn make_module(vm: &VirtualMachine, module: PyObjectRef) { "OverflowError" => ctx.exceptions.overflow_error.clone(), "RuntimeError" => ctx.exceptions.runtime_error.clone(), "ReferenceError" => ctx.exceptions.reference_error.clone(), + "SyntaxError" => ctx.exceptions.syntax_error.clone(), "NotImplementedError" => ctx.exceptions.not_implemented_error.clone(), "TypeError" => ctx.exceptions.type_error.clone(), "ValueError" => ctx.exceptions.value_error.clone(), diff --git a/vm/src/compile.rs b/vm/src/compile.rs index 8635e21ac8..9042bf576a 100644 --- a/vm/src/compile.rs +++ b/vm/src/compile.rs @@ -6,7 +6,7 @@ //! https://github.com/micropython/micropython/blob/master/py/compile.c use crate::bytecode::{self, CallType, CodeObject, Instruction, Varargs}; -use crate::error::CompileError; +use crate::error::{CompileError, CompileErrorType}; use crate::obj::objcode; use crate::obj::objcode::PyCodeRef; use crate::pyobject::PyValue; @@ -40,17 +40,17 @@ pub fn compile( match mode { Mode::Exec => { - let ast = parser::parse_program(source).map_err(CompileError::Parse)?; + let ast = parser::parse_program(source)?; let symbol_table = make_symbol_table(&ast); compiler.compile_program(&ast, symbol_table) } Mode::Eval => { - let statement = parser::parse_statement(source).map_err(CompileError::Parse)?; + let statement = parser::parse_statement(source)?; let symbol_table = statements_to_symbol_table(&statement); compiler.compile_statement_eval(&statement, symbol_table) } Mode::Single => { - let ast = parser::parse_program(source).map_err(CompileError::Parse)?; + let ast = parser::parse_program(source)?; let symbol_table = make_symbol_table(&ast); compiler.compile_program_single(&ast, symbol_table) } @@ -158,7 +158,10 @@ impl Compiler { if let ast::Statement::Expression { ref expression } = statement.node { self.compile_expression(expression)?; } else { - return Err(CompileError::ExpectExpr); + return Err(CompileError { + error: CompileErrorType::ExpectExpr, + location: statement.location.clone(), + }); } } self.emit(Instruction::ReturnValue); @@ -397,19 +400,28 @@ impl Compiler { } ast::Statement::Break => { if !self.in_loop { - return Err(CompileError::InvalidBreak); + return Err(CompileError { + error: CompileErrorType::InvalidBreak, + location: statement.location.clone(), + }); } self.emit(Instruction::Break); } ast::Statement::Continue => { if !self.in_loop { - return Err(CompileError::InvalidContinue); + return Err(CompileError { + error: CompileErrorType::InvalidContinue, + location: statement.location.clone(), + }); } self.emit(Instruction::Continue); } ast::Statement::Return { value } => { if !self.in_function_def { - return Err(CompileError::InvalidReturn); + return Err(CompileError { + error: CompileErrorType::InvalidReturn, + location: statement.location.clone(), + }); } match value { Some(v) => { @@ -462,7 +474,10 @@ impl Compiler { self.emit(Instruction::DeleteSubscript); } _ => { - return Err(CompileError::Delete(target.name())); + return Err(CompileError { + error: CompileErrorType::Delete(target.name()), + location: self.current_source_location.clone(), + }); } } } @@ -1021,7 +1036,10 @@ impl Compiler { for (i, element) in elements.iter().enumerate() { if let ast::Expression::Starred { .. } = element { if seen_star { - return Err(CompileError::StarArgs); + return Err(CompileError { + error: CompileErrorType::StarArgs, + location: self.current_source_location.clone(), + }); } else { seen_star = true; self.emit(Instruction::UnpackEx { @@ -1047,7 +1065,10 @@ impl Compiler { } } _ => { - return Err(CompileError::Assign(target.name())); + return Err(CompileError { + error: CompileErrorType::Assign(target.name()), + location: self.current_source_location.clone(), + }); } } @@ -1237,7 +1258,10 @@ impl Compiler { } ast::Expression::Yield { value } => { if !self.in_function_def { - return Err(CompileError::InvalidYield); + return Err(CompileError { + error: CompileErrorType::InvalidYield, + location: self.current_source_location.clone(), + }); } self.mark_generator(); match value { diff --git a/vm/src/error.rs b/vm/src/error.rs index 06eb387e2e..81836fa444 100644 --- a/vm/src/error.rs +++ b/vm/src/error.rs @@ -1,10 +1,26 @@ use rustpython_parser::error::ParseError; +use rustpython_parser::lexer::Location; use std::error::Error; use std::fmt; #[derive(Debug)] -pub enum CompileError { +pub struct CompileError { + pub error: CompileErrorType, + pub location: Location, +} + +impl From for CompileError { + fn from(error: ParseError) -> Self { + CompileError { + error: CompileErrorType::Parse(error), + location: Default::default(), // TODO: extract location from parse error! + } + } +} + +#[derive(Debug)] +pub enum CompileErrorType { /// Invalid assignment, cannot store value in target. Assign(&'static str), /// Invalid delete @@ -25,17 +41,20 @@ pub enum CompileError { impl fmt::Display for CompileError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - CompileError::Assign(target) => write!(f, "can't assign to {}", target), - CompileError::Delete(target) => write!(f, "can't delete {}", target), - CompileError::ExpectExpr => write!(f, "Expecting expression, got statement"), - CompileError::Parse(err) => write!(f, "{}", err), - CompileError::StarArgs => write!(f, "Two starred expressions in assignment"), - CompileError::InvalidBreak => write!(f, "'break' outside loop"), - CompileError::InvalidContinue => write!(f, "'continue' outside loop"), - CompileError::InvalidReturn => write!(f, "'return' outside function"), - CompileError::InvalidYield => write!(f, "'yield' outside function"), - } + match &self.error { + CompileErrorType::Assign(target) => write!(f, "can't assign to {}", target), + CompileErrorType::Delete(target) => write!(f, "can't delete {}", target), + CompileErrorType::ExpectExpr => write!(f, "Expecting expression, got statement"), + CompileErrorType::Parse(err) => write!(f, "{}", err), + CompileErrorType::StarArgs => write!(f, "Two starred expressions in assignment"), + CompileErrorType::InvalidBreak => write!(f, "'break' outside loop"), + CompileErrorType::InvalidContinue => write!(f, "'continue' outside loop"), + CompileErrorType::InvalidReturn => write!(f, "'return' outside function"), + CompileErrorType::InvalidYield => write!(f, "'yield' outside function"), + }?; + + // Print line number: + write!(f, " at line {:?}", self.location.get_row()) } } diff --git a/vm/src/exceptions.rs b/vm/src/exceptions.rs index 58a0f929d7..a8b753e1a3 100644 --- a/vm/src/exceptions.rs +++ b/vm/src/exceptions.rs @@ -67,7 +67,10 @@ pub fn print_exception_inner(vm: &VirtualMachine, exc: &PyObjectRef) { "".to_string() }; - println!(" File {}, line {}, in {}", filename, lineno, obj_name); + println!( + r##" File "{}", line {}, in {}"##, + filename, lineno, obj_name + ); } else { println!(" File ??"); } diff --git a/vm/src/vm.rs b/vm/src/vm.rs index e360b8e5ec..6e77105168 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -14,6 +14,7 @@ use std::sync::{Mutex, MutexGuard}; use crate::builtins; use crate::bytecode; +use crate::error::CompileError; use crate::frame::{ExecutionResult, Frame, FrameRef, Scope}; use crate::function::PyFuncArgs; use crate::obj::objbool; @@ -234,9 +235,12 @@ impl VirtualMachine { self.new_exception(overflow_error, msg) } - pub fn new_syntax_error(&self, msg: &T) -> PyObjectRef { - let syntax_error = self.ctx.exceptions.syntax_error.clone(); - self.new_exception(syntax_error, msg.to_string()) + pub fn new_syntax_error(&self, error: &CompileError) -> PyObjectRef { + let syntax_error_type = self.ctx.exceptions.syntax_error.clone(); + let syntax_error = self.new_exception(syntax_error_type, error.to_string()); + let lineno = self.new_int(error.location.get_row()); + self.set_attr(&syntax_error, "lineno", lineno).unwrap(); + syntax_error } pub fn get_none(&self) -> PyObjectRef { diff --git a/wasm/lib/src/vm_class.rs b/wasm/lib/src/vm_class.rs index ff72dddbb6..2c330e6eeb 100644 --- a/wasm/lib/src/vm_class.rs +++ b/wasm/lib/src/vm_class.rs @@ -266,7 +266,9 @@ impl WASMVirtualMachine { let code = compile::compile(vm, &source, &mode, "".to_string()); let code = code.map_err(|err| { let js_err = SyntaxError::new(&format!("Error parsing Python code: {}", err)); - if let rustpython_vm::error::CompileError::Parse(ref parse_error) = err { + if let rustpython_vm::error::CompileErrorType::Parse(ref parse_error) = + err.error + { use rustpython_parser::error::ParseError; if let ParseError::EOF(Some(ref loc)) | ParseError::ExtraToken((ref loc, ..)) From b34784e9efdcdc1449299a67f84cc3b07634b156 Mon Sep 17 00:00:00 2001 From: Windel Bouwman Date: Wed, 17 Apr 2019 20:52:53 +0200 Subject: [PATCH 2/2] Format main.rs --- src/main.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/main.rs b/src/main.rs index 326e0bc3e0..24b46821b6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,8 +10,8 @@ extern crate rustyline; use clap::{App, Arg}; use rustpython_parser::error::ParseError; use rustpython_vm::{ - compile, error::CompileError, error::CompileErrorType, frame::Scope, import, obj::objstr, print_exception, - pyobject::PyResult, util, VirtualMachine, + compile, error::CompileError, error::CompileErrorType, frame::Scope, import, obj::objstr, + print_exception, pyobject::PyResult, util, VirtualMachine, }; use rustyline::{error::ReadlineError, Editor}; use std::path::{Path, PathBuf}; @@ -115,7 +115,12 @@ fn shell_exec(vm: &VirtualMachine, source: &str, scope: Scope) -> Result<(), Com Ok(()) } // Don't inject syntax errors for line continuation - Err(err @ CompileError{ error: CompileErrorType::Parse(ParseError::EOF(_)), .. }) => Err(err), + Err( + err @ CompileError { + error: CompileErrorType::Parse(ParseError::EOF(_)), + .. + }, + ) => Err(err), Err(err) => { let exc = vm.new_syntax_error(&err); print_exception(vm, &exc); @@ -188,7 +193,10 @@ fn run_shell(vm: &VirtualMachine) -> PyResult { } match shell_exec(vm, &input, vars.clone()) { - Err(CompileError { error: CompileErrorType::Parse(ParseError::EOF(_)), .. }) => { + Err(CompileError { + error: CompileErrorType::Parse(ParseError::EOF(_)), + .. + }) => { continuing = true; continue; }