From a8ea67178d4d675781f7f2eb679c53bdefc62575 Mon Sep 17 00:00:00 2001 From: Jonathan Rotter Date: Tue, 7 May 2024 22:57:03 -0500 Subject: [PATCH] adjusted SyntaxError args and __str__() to match CPython (#5294) --- Lib/test/test_fstring.py | 2 - Lib/test/test_future_stmt/test_future.py | 16 --- vm/src/exceptions.rs | 171 ++++++++++++++++++++++- vm/src/lib.rs | 1 - vm/src/source.rs | 20 --- vm/src/vm/vm_new.rs | 32 +---- 6 files changed, 170 insertions(+), 72 deletions(-) delete mode 100644 vm/src/source.rs diff --git a/Lib/test/test_fstring.py b/Lib/test/test_fstring.py index f074bcfcd..b1cd9b160 100644 --- a/Lib/test/test_fstring.py +++ b/Lib/test/test_fstring.py @@ -1196,8 +1196,6 @@ x = ( r"f'{1000:j}'", ]) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_filename_in_syntaxerror(self): # see issue 38964 with temp_cwd() as cwd: diff --git a/Lib/test/test_future_stmt/test_future.py b/Lib/test/test_future_stmt/test_future.py index b00beb6b8..34b4c8583 100644 --- a/Lib/test/test_future_stmt/test_future.py +++ b/Lib/test/test_future_stmt/test_future.py @@ -52,57 +52,41 @@ class FutureTest(unittest.TestCase): ): from test.test_future_stmt import test_future_multiple_features - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_badfuture3(self): with self.assertRaises(SyntaxError) as cm: from test.test_future_stmt import badsyntax_future3 self.check_syntax_error(cm.exception, "badsyntax_future3", 3) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_badfuture4(self): with self.assertRaises(SyntaxError) as cm: from test.test_future_stmt import badsyntax_future4 self.check_syntax_error(cm.exception, "badsyntax_future4", 3) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_badfuture5(self): with self.assertRaises(SyntaxError) as cm: from test.test_future_stmt import badsyntax_future5 self.check_syntax_error(cm.exception, "badsyntax_future5", 4) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_badfuture6(self): with self.assertRaises(SyntaxError) as cm: from test.test_future_stmt import badsyntax_future6 self.check_syntax_error(cm.exception, "badsyntax_future6", 3) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_badfuture7(self): with self.assertRaises(SyntaxError) as cm: from test.test_future_stmt import badsyntax_future7 self.check_syntax_error(cm.exception, "badsyntax_future7", 3, 54) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_badfuture8(self): with self.assertRaises(SyntaxError) as cm: from test.test_future_stmt import badsyntax_future8 self.check_syntax_error(cm.exception, "badsyntax_future8", 3) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_badfuture9(self): with self.assertRaises(SyntaxError) as cm: from test.test_future_stmt import badsyntax_future9 self.check_syntax_error(cm.exception, "badsyntax_future9", 3) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_badfuture10(self): with self.assertRaises(SyntaxError) as cm: from test.test_future_stmt import badsyntax_future10 diff --git a/vm/src/exceptions.rs b/vm/src/exceptions.rs index dfee43a02..0ca193731 100644 --- a/vm/src/exceptions.rs +++ b/vm/src/exceptions.rs @@ -142,6 +142,11 @@ impl VirtualMachine { let args_repr = vm.exception_args_as_string(varargs, true); let exc_class = exc.class(); + + if exc_class.fast_issubclass(vm.ctx.exceptions.syntax_error) { + return self.write_syntaxerror(output, exc, exc_class, &args_repr); + } + let exc_name = exc_class.name(); match args_repr.len() { 0 => write!(output, "{exc_name}"), @@ -150,7 +155,119 @@ impl VirtualMachine { output, "{}: ({})", exc_name, - args_repr.into_iter().format(", ") + args_repr.into_iter().format(", "), + ), + }?; + + match offer_suggestions(exc, vm) { + Some(suggestions) => writeln!(output, ". Did you mean: '{suggestions}'?"), + None => writeln!(output), + } + } + + /// Format and write a SyntaxError + /// This logic is derived from TracebackException._format_syntax_error + /// + /// The logic has support for `end_offset` to highlight a range in the source code, + /// but it looks like `end_offset` is not used yet when SyntaxErrors are created. + fn write_syntaxerror( + &self, + output: &mut W, + exc: &PyBaseExceptionRef, + exc_type: &Py, + args_repr: &[PyRef], + ) -> Result<(), W::Error> { + let vm = self; + debug_assert!(exc_type.fast_issubclass(vm.ctx.exceptions.syntax_error)); + + let getattr = |attr: &'static str| exc.as_object().get_attr(attr, vm).ok(); + + let maybe_lineno = getattr("lineno").map(|obj| { + obj.str(vm) + .unwrap_or_else(|_| vm.ctx.new_str("")) + }); + let maybe_filename = getattr("filename").and_then(|obj| obj.str(vm).ok()); + + let maybe_text = getattr("text").map(|obj| { + obj.str(vm) + .unwrap_or_else(|_| vm.ctx.new_str("")) + }); + + let mut filename_suffix = String::new(); + + if let Some(lineno) = maybe_lineno { + writeln!( + output, + r##" File "{}", line {}"##, + maybe_filename + .as_ref() + .map(|s| s.as_str()) + .unwrap_or(""), + lineno + )?; + } else if let Some(filename) = maybe_filename { + filename_suffix = format!(" ({})", filename); + } + + if let Some(text) = maybe_text { + // if text ends with \n, remove it + let rtext = text.as_str().trim_end_matches('\n'); + let ltext = rtext.trim_start_matches([' ', '\n', '\x0c']); // \x0c is \f + let spaces = (rtext.len() - ltext.len()) as isize; + + writeln!(output, " {}", ltext)?; + + let maybe_offset: Option = + getattr("offset").and_then(|obj| obj.try_to_value::(vm).ok()); + + if let Some(offset) = maybe_offset { + let maybe_end_offset: Option = + getattr("end_offset").and_then(|obj| obj.try_to_value::(vm).ok()); + + let mut end_offset = match maybe_end_offset { + Some(0) | None => offset, + Some(end_offset) => end_offset, + }; + + if offset == end_offset || end_offset == -1 { + end_offset = offset + 1; + } + + // Convert 1-based column offset to 0-based index into stripped text + let colno = offset - 1 - spaces; + let end_colno = end_offset - 1 - spaces; + if colno >= 0 { + let caretspace = ltext.chars().collect::>()[..colno as usize] + .iter() + .map(|c| if c.is_whitespace() { *c } else { ' ' }) + .collect::(); + + let mut error_width = end_colno - colno; + if error_width < 1 { + error_width = 1; + } + + writeln!( + output, + " {}{}", + caretspace, + "^".repeat(error_width as usize) + )?; + } + } + } + + let exc_name = exc_type.name(); + + match args_repr.len() { + 0 => write!(output, "{exc_name}{filename_suffix}"), + 1 => write!(output, "{}: {}{}", exc_name, args_repr[0], filename_suffix), + _ => write!( + output, + "{}: ({}){}", + exc_name, + args_repr.iter().format(", "), + filename_suffix ), }?; @@ -1461,10 +1578,60 @@ pub(super) mod types { #[derive(Debug)] pub struct PyRecursionError {} - #[pyexception(name, base = "PyException", ctx = "syntax_error", impl)] + #[pyexception(name, base = "PyException", ctx = "syntax_error")] #[derive(Debug)] pub struct PySyntaxError {} + #[pyexception] + impl PySyntaxError { + #[pymethod(magic)] + fn str(exc: PyBaseExceptionRef, vm: &VirtualMachine) -> PyStrRef { + fn basename(filename: &str) -> &str { + let splitted = if cfg!(windows) { + filename.rsplit(&['/', '\\']).next() + } else { + filename.rsplit('/').next() + }; + splitted.unwrap_or(filename) + } + + let maybe_lineno = exc.as_object().get_attr("lineno", vm).ok().map(|obj| { + obj.str(vm) + .unwrap_or_else(|_| vm.ctx.new_str("")) + }); + let maybe_filename = exc.as_object().get_attr("filename", vm).ok().map(|obj| { + obj.str(vm) + .unwrap_or_else(|_| vm.ctx.new_str("")) + }); + + let args = exc.args(); + + let msg = if args.len() == 1 { + vm.exception_args_as_string(args, false) + .into_iter() + .exactly_one() + .unwrap() + } else { + return exc.str(vm); + }; + + let msg_with_location_info: String = match (maybe_lineno, maybe_filename) { + (Some(lineno), Some(filename)) => { + format!("{} ({}, line {})", msg, basename(filename.as_str()), lineno) + } + (Some(lineno), None) => { + format!("{} (line {})", msg, lineno) + } + (None, Some(filename)) => { + format!("{} ({})", msg, basename(filename.as_str())) + } + (None, None) => msg.to_string(), + }; + + vm.ctx.new_str(msg_with_location_info) + } + } + #[pyexception(name, base = "PySyntaxError", ctx = "indentation_error", impl)] #[derive(Debug)] pub struct PyIndentationError {} diff --git a/vm/src/lib.rs b/vm/src/lib.rs index 3c99e382f..cb2e77327 100644 --- a/vm/src/lib.rs +++ b/vm/src/lib.rs @@ -73,7 +73,6 @@ pub mod scope; pub mod sequence; pub mod signal; pub mod sliceable; -mod source; pub mod stdlib; pub mod suggestion; pub mod types; diff --git a/vm/src/source.rs b/vm/src/source.rs deleted file mode 100644 index 231c2f01c..000000000 --- a/vm/src/source.rs +++ /dev/null @@ -1,20 +0,0 @@ -use crate::source_code::SourceLocation; - -// pub(crate) fn new_location_error( -// index: usize, -// field: &str, -// vm: &VirtualMachine, -// ) -> PyRef { -// vm.new_value_error(format!("value {index} is too large for location {field}")) -// } - -pub(crate) struct AtLocation<'a>(pub Option<&'a SourceLocation>); - -impl std::fmt::Display for AtLocation<'_> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let (row, column) = self - .0 - .map_or((0, 0), |l| (l.row.to_usize(), l.column.to_usize())); - write!(f, " at line {row} column {column}",) - } -} diff --git a/vm/src/vm/vm_new.rs b/vm/src/vm/vm_new.rs index 4677d196f..ea8fa21c1 100644 --- a/vm/src/vm/vm_new.rs +++ b/vm/src/vm/vm_new.rs @@ -262,7 +262,6 @@ impl VirtualMachine { error: &crate::compiler::CompileError, source: Option<&str>, ) -> PyBaseExceptionRef { - use crate::source::AtLocation; use crate::source_code::SourceLocation; let syntax_error_type = match &error.error { @@ -293,36 +292,7 @@ impl VirtualMachine { None }; - fn fmt( - error: &crate::compiler::CompileError, - statement: Option<&str>, - f: &mut impl std::fmt::Write, - ) -> std::fmt::Result { - let loc = error.location; - if let Some(ref stmt) = statement { - // visualize the error when location and statement are provided - write!( - f, - "{error}{at_location}\n{stmt}{arrow:>pad$}", - error = error.error, - at_location = AtLocation(loc.as_ref()), - pad = loc.map_or(0, |loc| loc.column.to_usize()), - arrow = "^" - ) - } else { - write!( - f, - "{error}{at_location}", - error = error.error, - at_location = AtLocation(loc.as_ref()), - ) - } - } - - let mut msg = String::new(); - fmt(error, statement.as_deref(), &mut msg).unwrap(); - - let syntax_error = self.new_exception_msg(syntax_error_type, msg); + let syntax_error = self.new_exception_msg(syntax_error_type, error.error.to_string()); let (lineno, offset) = error.python_location(); let lineno = self.ctx.new_int(lineno); let offset = self.ctx.new_int(offset);