adjusted SyntaxError args and __str__() to match CPython (#5294)

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:

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

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<W: Write>(
+        &self,
+        output: &mut W,
+        exc: &PyBaseExceptionRef,
+        exc_type: &Py<PyType>,
+        args_repr: &[PyRef<PyStr>],
+    ) -> 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("<lineno str() failed>"))
+        });
+        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("<text str() failed>"))
+        });
+
+        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("<string>"),
+                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<isize> =
+                getattr("offset").and_then(|obj| obj.try_to_value::<isize>(vm).ok());
+
+            if let Some(offset) = maybe_offset {
+                let maybe_end_offset: Option<isize> =
+                    getattr("end_offset").and_then(|obj| obj.try_to_value::<isize>(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::<Vec<_>>()[..colno as usize]
+                        .iter()
+                        .map(|c| if c.is_whitespace() { *c } else { ' ' })
+                        .collect::<String>();
+
+                    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("<lineno str() failed>"))
+            });
+            let maybe_filename = exc.as_object().get_attr("filename", vm).ok().map(|obj| {
+                obj.str(vm)
+                    .unwrap_or_else(|_| vm.ctx.new_str("<filename str() failed>"))
+            });
+
+            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 {}

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;

vm/src/source.rs

@@ -1,20 +0,0 @@
-use crate::source_code::SourceLocation;
-
-// pub(crate) fn new_location_error(
-//     index: usize,
-//     field: &str,
-//     vm: &VirtualMachine,
-// ) -> PyRef<PyBaseException> {
-//     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}",)
-    }
-}

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);