Skip to content

Commit eb16f16

Browse files
windelbouwmanwindelbouwman
committed
Improve syntax error with line information.
1 parent 615a121 commit eb16f16

File tree

8 files changed

+101
-32
lines changed

8 files changed

+101
-32
lines changed

‎src/main.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ extern crate rustyline;
1010
use clap::{App, Arg};
1111
use rustpython_parser::error::ParseError;
1212
use rustpython_vm::{
13-
compile, error::CompileError, frame::Scope, import, obj::objstr, print_exception,
13+
compile, error::CompileError, error::CompileErrorType, frame::Scope, import, obj::objstr, print_exception,
1414
pyobject::PyResult, util, VirtualMachine,
1515
};
1616
use rustyline::{error::ReadlineError, Editor};
@@ -115,7 +115,7 @@ fn shell_exec(vm: &VirtualMachine, source: &str, scope: Scope) -> Result<(), Com
115115
Ok(())
116116
}
117117
// Don't inject syntax errors for line continuation
118-
Err(err @ CompileError::Parse(ParseError::EOF(_))) => Err(err),
118+
Err(err @ CompileError{ error: CompileErrorType::Parse(ParseError::EOF(_)), .. }) => Err(err),
119119
Err(err) => {
120120
let exc = vm.new_syntax_error(&err);
121121
print_exception(vm, &exc);
@@ -188,7 +188,7 @@ fn run_shell(vm: &VirtualMachine) -> PyResult {
188188
}
189189

190190
match shell_exec(vm, &input, vars.clone()) {
191-
Err(CompileError::Parse(ParseError::EOF(_))) => {
191+
Err(CompileError { error: CompileErrorType::Parse(ParseError::EOF(_)), .. }) => {
192192
continuing = true;
193193
continue;
194194
}

‎tests/snippets/invalid_syntax.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
2+
3+
src = """
4+
def valid_func():
5+
pass
6+
7+
yield 2
8+
"""
9+
10+
try:
11+
compile(src, 'test.py', 'exec')
12+
except SyntaxError as ex:
13+
assert ex.lineno == 5
14+
else:
15+
raise AssertionError("Must throw syntax error")
16+

‎vm/src/builtins.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -751,6 +751,7 @@ pub fn make_module(vm: &VirtualMachine, module: PyObjectRef) {
751751
"OverflowError" => ctx.exceptions.overflow_error.clone(),
752752
"RuntimeError" => ctx.exceptions.runtime_error.clone(),
753753
"ReferenceError" => ctx.exceptions.reference_error.clone(),
754+
"SyntaxError" => ctx.exceptions.syntax_error.clone(),
754755
"NotImplementedError" => ctx.exceptions.not_implemented_error.clone(),
755756
"TypeError" => ctx.exceptions.type_error.clone(),
756757
"ValueError" => ctx.exceptions.value_error.clone(),

‎vm/src/compile.rs

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
//! https://github.com/micropython/micropython/blob/master/py/compile.c
77
88
use crate::bytecode::{self, CallType, CodeObject, Instruction, Varargs};
9-
use crate::error::CompileError;
9+
use crate::error::{CompileError, CompileErrorType};
1010
use crate::obj::objcode;
1111
use crate::obj::objcode::PyCodeRef;
1212
use crate::pyobject::PyValue;
@@ -40,17 +40,17 @@ pub fn compile(
4040

4141
match mode {
4242
Mode::Exec => {
43-
let ast = parser::parse_program(source).map_err(CompileError::Parse)?;
43+
let ast = parser::parse_program(source)?;
4444
let symbol_table = make_symbol_table(&ast);
4545
compiler.compile_program(&ast, symbol_table)
4646
}
4747
Mode::Eval => {
48-
let statement = parser::parse_statement(source).map_err(CompileError::Parse)?;
48+
let statement = parser::parse_statement(source)?;
4949
let symbol_table = statements_to_symbol_table(&statement);
5050
compiler.compile_statement_eval(&statement, symbol_table)
5151
}
5252
Mode::Single => {
53-
let ast = parser::parse_program(source).map_err(CompileError::Parse)?;
53+
let ast = parser::parse_program(source)?;
5454
let symbol_table = make_symbol_table(&ast);
5555
compiler.compile_program_single(&ast, symbol_table)
5656
}
@@ -158,7 +158,10 @@ impl Compiler {
158158
if let ast::Statement::Expression { ref expression } = statement.node {
159159
self.compile_expression(expression)?;
160160
} else {
161-
return Err(CompileError::ExpectExpr);
161+
return Err(CompileError {
162+
error: CompileErrorType::ExpectExpr,
163+
location: statement.location.clone(),
164+
});
162165
}
163166
}
164167
self.emit(Instruction::ReturnValue);
@@ -397,19 +400,28 @@ impl Compiler {
397400
}
398401
ast::Statement::Break => {
399402
if !self.in_loop {
400-
return Err(CompileError::InvalidBreak);
403+
return Err(CompileError {
404+
error: CompileErrorType::InvalidBreak,
405+
location: statement.location.clone(),
406+
});
401407
}
402408
self.emit(Instruction::Break);
403409
}
404410
ast::Statement::Continue => {
405411
if !self.in_loop {
406-
return Err(CompileError::InvalidContinue);
412+
return Err(CompileError {
413+
error: CompileErrorType::InvalidContinue,
414+
location: statement.location.clone(),
415+
});
407416
}
408417
self.emit(Instruction::Continue);
409418
}
410419
ast::Statement::Return { value } => {
411420
if !self.in_function_def {
412-
return Err(CompileError::InvalidReturn);
421+
return Err(CompileError {
422+
error: CompileErrorType::InvalidReturn,
423+
location: statement.location.clone(),
424+
});
413425
}
414426
match value {
415427
Some(v) => {
@@ -462,7 +474,10 @@ impl Compiler {
462474
self.emit(Instruction::DeleteSubscript);
463475
}
464476
_ => {
465-
return Err(CompileError::Delete(target.name()));
477+
return Err(CompileError {
478+
error: CompileErrorType::Delete(target.name()),
479+
location: self.current_source_location.clone(),
480+
});
466481
}
467482
}
468483
}
@@ -1021,7 +1036,10 @@ impl Compiler {
10211036
for (i, element) in elements.iter().enumerate() {
10221037
if let ast::Expression::Starred { .. } = element {
10231038
if seen_star {
1024-
return Err(CompileError::StarArgs);
1039+
return Err(CompileError {
1040+
error: CompileErrorType::StarArgs,
1041+
location: self.current_source_location.clone(),
1042+
});
10251043
} else {
10261044
seen_star = true;
10271045
self.emit(Instruction::UnpackEx {
@@ -1047,7 +1065,10 @@ impl Compiler {
10471065
}
10481066
}
10491067
_ => {
1050-
return Err(CompileError::Assign(target.name()));
1068+
return Err(CompileError {
1069+
error: CompileErrorType::Assign(target.name()),
1070+
location: self.current_source_location.clone(),
1071+
});
10511072
}
10521073
}
10531074

@@ -1237,7 +1258,10 @@ impl Compiler {
12371258
}
12381259
ast::Expression::Yield { value } => {
12391260
if !self.in_function_def {
1240-
return Err(CompileError::InvalidYield);
1261+
return Err(CompileError {
1262+
error: CompileErrorType::InvalidYield,
1263+
location: self.current_source_location.clone(),
1264+
});
12411265
}
12421266
self.mark_generator();
12431267
match value {

‎vm/src/error.rs

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,26 @@
11
use rustpython_parser::error::ParseError;
2+
use rustpython_parser::lexer::Location;
23

34
use std::error::Error;
45
use std::fmt;
56

67
#[derive(Debug)]
7-
pub enum CompileError {
8+
pub struct CompileError {
9+
pub error: CompileErrorType,
10+
pub location: Location,
11+
}
12+
13+
impl From<ParseError> for CompileError {
14+
fn from(error: ParseError) -> Self {
15+
CompileError {
16+
error: CompileErrorType::Parse(error),
17+
location: Default::default(), // TODO: extract location from parse error!
18+
}
19+
}
20+
}
21+
22+
#[derive(Debug)]
23+
pub enum CompileErrorType {
824
/// Invalid assignment, cannot store value in target.
925
Assign(&'static str),
1026
/// Invalid delete
@@ -25,17 +41,20 @@ pub enum CompileError {
2541

2642
impl fmt::Display for CompileError {
2743
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
28-
match self {
29-
CompileError::Assign(target) => write!(f, "can't assign to {}", target),
30-
CompileError::Delete(target) => write!(f, "can't delete {}", target),
31-
CompileError::ExpectExpr => write!(f, "Expecting expression, got statement"),
32-
CompileError::Parse(err) => write!(f, "{}", err),
33-
CompileError::StarArgs => write!(f, "Two starred expressions in assignment"),
34-
CompileError::InvalidBreak => write!(f, "'break' outside loop"),
35-
CompileError::InvalidContinue => write!(f, "'continue' outside loop"),
36-
CompileError::InvalidReturn => write!(f, "'return' outside function"),
37-
CompileError::InvalidYield => write!(f, "'yield' outside function"),
38-
}
44+
match &self.error {
45+
CompileErrorType::Assign(target) => write!(f, "can't assign to {}", target),
46+
CompileErrorType::Delete(target) => write!(f, "can't delete {}", target),
47+
CompileErrorType::ExpectExpr => write!(f, "Expecting expression, got statement"),
48+
CompileErrorType::Parse(err) => write!(f, "{}", err),
49+
CompileErrorType::StarArgs => write!(f, "Two starred expressions in assignment"),
50+
CompileErrorType::InvalidBreak => write!(f, "'break' outside loop"),
51+
CompileErrorType::InvalidContinue => write!(f, "'continue' outside loop"),
52+
CompileErrorType::InvalidReturn => write!(f, "'return' outside function"),
53+
CompileErrorType::InvalidYield => write!(f, "'yield' outside function"),
54+
}?;
55+
56+
// Print line number:
57+
write!(f, " at line {:?}", self.location.get_row())
3958
}
4059
}
4160

‎vm/src/exceptions.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,10 @@ pub fn print_exception_inner(vm: &VirtualMachine, exc: &PyObjectRef) {
6767
"<error>".to_string()
6868
};
6969

70-
println!(" File {}, line {}, in {}", filename, lineno, obj_name);
70+
println!(
71+
r##" File "{}", line {}, in {}"##,
72+
filename, lineno, obj_name
73+
);
7174
} else {
7275
println!(" File ??");
7376
}

‎vm/src/vm.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use std::sync::{Mutex, MutexGuard};
1414

1515
use crate::builtins;
1616
use crate::bytecode;
17+
use crate::error::CompileError;
1718
use crate::frame::{ExecutionResult, Frame, FrameRef, Scope};
1819
use crate::function::PyFuncArgs;
1920
use crate::obj::objbool;
@@ -234,9 +235,12 @@ impl VirtualMachine {
234235
self.new_exception(overflow_error, msg)
235236
}
236237

237-
pub fn new_syntax_error<T: ToString>(&self, msg: &T) -> PyObjectRef {
238-
let syntax_error = self.ctx.exceptions.syntax_error.clone();
239-
self.new_exception(syntax_error, msg.to_string())
238+
pub fn new_syntax_error(&self, error: &CompileError) -> PyObjectRef {
239+
let syntax_error_type = self.ctx.exceptions.syntax_error.clone();
240+
let syntax_error = self.new_exception(syntax_error_type, error.to_string());
241+
let lineno = self.new_int(error.location.get_row());
242+
self.set_attr(&syntax_error, "lineno", lineno).unwrap();
243+
syntax_error
240244
}
241245

242246
pub fn get_none(&self) -> PyObjectRef {

‎wasm/lib/src/vm_class.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,9 @@ impl WASMVirtualMachine {
266266
let code = compile::compile(vm, &source, &mode, "<wasm>".to_string());
267267
let code = code.map_err(|err| {
268268
let js_err = SyntaxError::new(&format!("Error parsing Python code: {}", err));
269-
if let rustpython_vm::error::CompileError::Parse(ref parse_error) = err {
269+
if let rustpython_vm::error::CompileErrorType::Parse(ref parse_error) =
270+
err.error
271+
{
270272
use rustpython_parser::error::ParseError;
271273
if let ParseError::EOF(Some(ref loc))
272274
| ParseError::ExtraToken((ref loc, ..))

0 commit comments

Comments
 (0)