Python: Adds location data to SyntaxError

.cargo/build_config.py

@@ -63,7 +63,9 @@ FLAGS_BLACKLIST = {
 }
 
 build_flags = set(DEFAULT_BUILD_FLAGS)
-with urlopen(f"https://rust-lang.github.io/rust-clippy/rust-{MSRV}/lints.json") as response:
+with urlopen(
+    f"https://rust-lang.github.io/rust-clippy/rust-{MSRV}/lints.json"
+) as response:
     for lint in json.load(response):
         if lint["level"] == "allow" and lint["group"] != "nursery":
             build_flags.add(f"-Wclippy::{lint['id'].replace('_', '-')}")
@@ -78,5 +80,5 @@ with open("./config.toml", "wt") as fp:
     fp.write("[build]\n")
     fp.write("rustflags = [\n")
     for flag in sorted(build_flags):
-        fp.write(f"    \"{flag}\",\n")
+        fp.write(f'    "{flag}",\n')
     fp.write("]\n")

python/src/io.rs

@@ -12,6 +12,7 @@ use std::error::Error;
 use std::fs::File;
 use std::io::{self, BufWriter, Cursor, Read, Write};
 use std::path::{Path, PathBuf};
+use std::sync::OnceLock;
 
 pub fn add_to_module(module: &PyModule) -> PyResult<()> {
     module.add_wrapped(wrap_pyfunction!(parse))?;
@@ -62,8 +63,9 @@ pub fn parse(
     py: Python<'_>,
 ) -> PyResult<PyObject> {
     let format = rdf_format(format)?;
-    let input = if let Ok(path) = input.extract::<PathBuf>(py) {
-        PyReadable::from_file(&path, py).map_err(map_io_err)?
+    let file_path = input.extract::<PathBuf>(py).ok();
+    let input = if let Some(file_path) = &file_path {
+        PyReadable::from_file(file_path, py).map_err(map_io_err)?
     } else {
         PyReadable::from_data(input, py)
     };
@@ -81,6 +83,7 @@ pub fn parse(
     }
     Ok(PyQuadReader {
         inner: parser.parse_read(input),
+        file_path,
     }
     .into_py(py))
 }
@@ -151,6 +154,7 @@ pub fn serialize(input: &PyAny, output: PyObject, format: &str, py: Python<'_>)
 #[pyclass(name = "QuadReader", module = "pyoxigraph")]
 pub struct PyQuadReader {
     inner: FromReadQuadReader<PyReadable>,
+    file_path: Option<PathBuf>,
 }
 
 #[pymethods]
@@ -163,7 +167,10 @@ impl PyQuadReader {
         py.allow_threads(|| {
             self.inner
                 .next()
-                .map(|q| Ok(q.map_err(map_parse_error)?.into()))
+                .map(|q| {
+                    Ok(q.map_err(|e| map_parse_error(e, self.file_path.clone()))?
+                        .into())
+                })
                 .transpose()
         })
     }
@@ -314,9 +321,38 @@ pub fn map_io_err(error: io::Error) -> PyErr {
     }
 }
 
-pub fn map_parse_error(error: ParseError) -> PyErr {
+pub fn map_parse_error(error: ParseError, file_path: Option<PathBuf>) -> PyErr {
     match error {
-        ParseError::Syntax(error) => PySyntaxError::new_err(error.to_string()),
+        ParseError::Syntax(error) => {
+            // Python 3.9 does not support end line and end column
+            if python_version() >= (3, 10, 0) {
+                let params = if let Some(location) = error.location() {
+                    (
+                        file_path,
+                        Some(location.start.line + 1),
+                        Some(location.start.column + 1),
+                        None::<Vec<u8>>,
+                        Some(location.end.line + 1),
+                        Some(location.end.column + 1),
+                    )
+                } else {
+                    (None, None, None, None, None, None)
+                };
+                PySyntaxError::new_err((error.to_string(), params))
+            } else {
+                let params = if let Some(location) = error.location() {
+                    (
+                        file_path,
+                        Some(location.start.line + 1),
+                        Some(location.start.column + 1),
+                        None::<Vec<u8>>,
+                    )
+                } else {
+                    (None, None, None, None)
+                };
+                PySyntaxError::new_err((error.to_string(), params))
+            }
+        }
         ParseError::Io(error) => map_io_err(error),
     }
 }
@@ -345,3 +381,13 @@ pub fn allow_threads_unsafe<T>(f: impl FnOnce() -> T) -> T {
     let _guard = RestoreGuard { tstate };
     f()
 }
+
+fn python_version() -> (u8, u8, u8) {
+    static VERSION: OnceLock<(u8, u8, u8)> = OnceLock::new();
+    *VERSION.get_or_init(|| {
+        Python::with_gil(|py| {
+            let v = py.version_info();
+            (v.major, v.minor, v.patch)
+        })
+    })
+}

python/src/sparql.rs

@@ -1,4 +1,4 @@
-use crate::io::{allow_threads_unsafe, map_io_err};
+use crate::io::{allow_threads_unsafe, map_io_err, map_parse_error};
 use crate::map_storage_error;
 use crate::model::*;
 use oxigraph::model::Term;
@@ -233,10 +233,7 @@ pub fn map_evaluation_error(error: EvaluationError) -> PyErr {
     match error {
         EvaluationError::Parsing(error) => PySyntaxError::new_err(error.to_string()),
         EvaluationError::Storage(error) => map_storage_error(error),
-        EvaluationError::GraphParsing(error) => match error {
-            oxigraph::io::ParseError::Syntax(error) => PySyntaxError::new_err(error.to_string()),
-            oxigraph::io::ParseError::Io(error) => map_io_err(error),
-        },
+        EvaluationError::GraphParsing(error) => map_parse_error(error, None),
         EvaluationError::ResultsParsing(error) => match error {
             oxigraph::sparql::results::ParseError::Syntax(error) => {
                 PySyntaxError::new_err(error.to_string())

python/src/store.rs

@@ -392,8 +392,9 @@ impl PyStore {
         } else {
             None
         };
-        let input = if let Ok(path) = input.extract::<PathBuf>(py) {
-            PyReadable::from_file(&path, py).map_err(map_io_err)?
+        let file_path = input.extract::<PathBuf>(py).ok();
+        let input = if let Some(file_path) = &file_path {
+            PyReadable::from_file(file_path, py).map_err(map_io_err)?
         } else {
             PyReadable::from_data(input, py)
         };
@@ -404,7 +405,7 @@ impl PyStore {
             } else {
                 self.inner.load_dataset(input, format, base_iri)
             }
-            .map_err(map_loader_error)
+            .map_err(|e| map_loader_error(e, file_path))
         })
     }
 
@@ -460,8 +461,9 @@ impl PyStore {
         } else {
             None
         };
-        let input = if let Ok(path) = input.extract::<PathBuf>(py) {
-            PyReadable::from_file(&path, py).map_err(map_io_err)?
+        let file_path = input.extract::<PathBuf>(py).ok();
+        let input = if let Some(file_path) = &file_path {
+            PyReadable::from_file(file_path, py).map_err(map_io_err)?
         } else {
             PyReadable::from_data(input, py)
         };
@@ -475,7 +477,7 @@ impl PyStore {
                     .bulk_loader()
                     .load_dataset(input, format, base_iri)
             }
-            .map_err(map_loader_error)
+            .map_err(|e| map_loader_error(e, file_path))
         })
     }
 
@@ -833,10 +835,10 @@ pub fn map_storage_error(error: StorageError) -> PyErr {
     }
 }
 
-pub fn map_loader_error(error: LoaderError) -> PyErr {
+pub fn map_loader_error(error: LoaderError, file_path: Option<PathBuf>) -> PyErr {
     match error {
         LoaderError::Storage(error) => map_storage_error(error),
-        LoaderError::Parsing(error) => map_parse_error(error),
+        LoaderError::Parsing(error) => map_parse_error(error, file_path),
         LoaderError::InvalidBaseIri { .. } => PyValueError::new_err(error.to_string()),
     }
 }

python/tests/test_io.py

@@ -1,3 +1,4 @@
+import sys
 import unittest
 from io import BytesIO, StringIO, UnsupportedOperation
 from tempfile import NamedTemporaryFile, TemporaryFile
@@ -83,6 +84,20 @@ class TestParse(unittest.TestCase):
             [EXAMPLE_QUAD],
         )
 
+    def test_parse_syntax_error(self) -> None:
+        with NamedTemporaryFile() as fp:
+            fp.write(b"@base <http://example.com/> .\n")
+            fp.write(b'<foo> "p" "1"')
+            fp.flush()
+            with self.assertRaises(SyntaxError) as ctx:
+                list(parse(fp.name, "text/turtle"))
+            self.assertEqual(ctx.exception.filename, fp.name)
+            self.assertEqual(ctx.exception.lineno, 2)
+            self.assertEqual(ctx.exception.offset, 7)
+            if sys.version_info >= (3, 10):
+                self.assertEqual(ctx.exception.end_lineno, 2)
+                self.assertEqual(ctx.exception.end_offset, 10)
+
     def test_parse_without_named_graphs(self) -> None:
         with self.assertRaises(SyntaxError) as _:
             list(