Skip to content

Improve syntax error with line information. #844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 18, 2019
Merged

Improve syntax error with line information. #844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb16f16
Improve syntax error with line information.
windelbouwman Apr 17, 2019
b34784e
Format main.rs
windelbouwman Apr 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ 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,
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};
Expand Down Expand Up @@ -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::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);
Expand Down Expand Up @@ -188,7 +193,10 @@ 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;
}
Expand Down
16 changes: 16 additions & 0 deletions tests/snippets/invalid_syntax.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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")

1 change: 1 addition & 0 deletions vm/src/builtins.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
48 changes: 36 additions & 12 deletions vm/src/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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(),
});
}
}
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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(),
});
}
}

Expand Down Expand Up @@ -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 {
Expand Down
43 changes: 31 additions & 12 deletions vm/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
Copy link
Member

@coolreader18 coolreader18 Apr 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like problem or issue for naming here instead of error/CompileErrorType?

pub location: Location,
}

impl From<ParseError> 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
Expand All @@ -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())
}
}

Expand Down
5 changes: 4 additions & 1 deletion vm/src/exceptions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ pub fn print_exception_inner(vm: &VirtualMachine, exc: &PyObjectRef) {
"<error>".to_string()
};

println!(" File {}, line {}, in {}", filename, lineno, obj_name);
println!(
r##" File "{}", line {}, in {}"##,
filename, lineno, obj_name
);
} else {
println!(" File ??");
}
Expand Down
10 changes: 7 additions & 3 deletions vm/src/vm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -234,9 +235,12 @@ impl VirtualMachine {
self.new_exception(overflow_error, msg)
}

pub fn new_syntax_error<T: ToString>(&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 {
Expand Down
4 changes: 3 additions & 1 deletion wasm/lib/src/vm_class.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,9 @@ impl WASMVirtualMachine {
let code = compile::compile(vm, &source, &mode, "<wasm>".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, ..))
Expand Down