From 6455f6c4018e3f74c631d3696950cedebb59611b Mon Sep 17 00:00:00 2001 From: Lily Acorn Date: Fri, 1 Aug 2025 09:50:06 +0100 Subject: [PATCH 1/6] Use PyO3 declarative module syntax --- src/lib.rs | 207 +++++++++++++++++++++++++++-------------------------- 1 file changed, 106 insertions(+), 101 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 563442a..0dafc9e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,123 +13,128 @@ use pyo3::create_exception; create_exception!(rustfluent, ParserError, pyo3::exceptions::PyException); #[pymodule] -fn rustfluent(m: &Bound<'_, PyModule>) -> PyResult<()> { - m.add_class::()?; - m.add("ParserError", m.py().get_type::())?; - Ok(()) -} +mod rustfluent { + use super::*; -#[pyclass] -struct Bundle { - bundle: FluentBundle, -} + #[pymodule_export] + use super::ParserError; -#[pymethods] -impl Bundle { - #[new] - #[pyo3(signature = (language, ftl_filenames, strict=false))] - fn new(language: &str, ftl_filenames: &'_ Bound<'_, PyList>, strict: bool) -> PyResult { - let langid: LanguageIdentifier = language.parse().expect("Parsing failed"); - let mut bundle = FluentBundle::new_concurrent(vec![langid]); + #[pyclass] + struct Bundle { + bundle: FluentBundle, + } - for file_path in ftl_filenames.iter() { - let path_string = file_path.to_string(); - let contents = fs::read_to_string(path_string) - .map_err(|_| PyFileNotFoundError::new_err(file_path.to_string()))?; + #[pymethods] + impl Bundle { + #[new] + #[pyo3(signature = (language, ftl_filenames, strict=false))] + fn new( + language: &str, + ftl_filenames: &'_ Bound<'_, PyList>, + strict: bool, + ) -> PyResult { + let langid: LanguageIdentifier = language.parse().expect("Parsing failed"); + let mut bundle = FluentBundle::new_concurrent(vec![langid]); - let resource = match FluentResource::try_new(contents) { - Ok(resource) => resource, - Err(_) if strict => { - return Err(ParserError::new_err(format!( - "Error when parsing {file_path}." - ))); - } - Err(error) => { - // The first element of the error is the parsed resource, minus any - // invalid messages. - error.0 - } - }; - bundle.add_resource_overriding(resource); - } + for file_path in ftl_filenames.iter() { + let path_string = file_path.to_string(); + let contents = fs::read_to_string(path_string) + .map_err(|_| PyFileNotFoundError::new_err(file_path.to_string()))?; - Ok(Self { bundle }) - } + let resource = match FluentResource::try_new(contents) { + Ok(resource) => resource, + Err(_) if strict => { + return Err(ParserError::new_err(format!( + "Error when parsing {file_path}." + ))); + } + Err(error) => { + // The first element of the error is the parsed resource, minus any + // invalid messages. + error.0 + } + }; + bundle.add_resource_overriding(resource); + } - #[pyo3(signature = (identifier, variables=None, use_isolating=true))] - pub fn get_translation( - &mut self, - identifier: &str, - variables: Option<&Bound<'_, PyDict>>, - use_isolating: bool, - ) -> PyResult { - self.bundle.set_use_isolating(use_isolating); + Ok(Self { bundle }) + } - let msg = self - .bundle - .get_message(identifier) - .ok_or_else(|| (PyValueError::new_err(format!("{identifier} not found"))))?; + #[pyo3(signature = (identifier, variables=None, use_isolating=true))] + pub fn get_translation( + &mut self, + identifier: &str, + variables: Option<&Bound<'_, PyDict>>, + use_isolating: bool, + ) -> PyResult { + self.bundle.set_use_isolating(use_isolating); - let mut errors = vec![]; - let pattern = msg.value().ok_or_else(|| { - PyValueError::new_err(format!("{identifier} - Message has no value.",)) - })?; + let msg = self + .bundle + .get_message(identifier) + .ok_or_else(|| (PyValueError::new_err(format!("{identifier} not found"))))?; - let mut args = FluentArgs::new(); + let mut errors = vec![]; + let pattern = msg.value().ok_or_else(|| { + PyValueError::new_err(format!("{identifier} - Message has no value.",)) + })?; - if let Some(variables) = variables { - for variable in variables { - // Make sure the variable key is a Python string, - // raising a TypeError if not. - let python_key = variable.0; - if !python_key.is_instance_of::() { - return Err(PyTypeError::new_err(format!( - "Variable key not a str, got {python_key}." - ))); - } - let key = python_key.to_string(); - // Set the variable value as a string or integer, - // raising a TypeError if not. - let python_value = variable.1; - if python_value.is_instance_of::() { - args.set(key, python_value.to_string()); - } else if python_value.is_instance_of::() { - match python_value.extract::() { - Ok(int_value) => { - args.set(key, int_value); - } - _ => { - // The Python integer overflowed i32. - // Fall back to displaying the variable key as its value. - let fallback_value = key.clone(); - args.set(key, fallback_value); - } + let mut args = FluentArgs::new(); + + if let Some(variables) = variables { + for variable in variables { + // Make sure the variable key is a Python string, + // raising a TypeError if not. + let python_key = variable.0; + if !python_key.is_instance_of::() { + return Err(PyTypeError::new_err(format!( + "Variable key not a str, got {python_key}." + ))); } - } else if python_value.is_instance_of::() { - // Display the Python date as YYYY-MM-DD. - match python_value.extract::() { - Ok(chrono_date) => { - args.set(key, chrono_date.format("%Y-%m-%d").to_string()); + let key = python_key.to_string(); + // Set the variable value as a string or integer, + // raising a TypeError if not. + let python_value = variable.1; + if python_value.is_instance_of::() { + args.set(key, python_value.to_string()); + } else if python_value.is_instance_of::() { + match python_value.extract::() { + Ok(int_value) => { + args.set(key, int_value); + } + _ => { + // The Python integer overflowed i32. + // Fall back to displaying the variable key as its value. + let fallback_value = key.clone(); + args.set(key, fallback_value); + } } - _ => { - // Could not convert. - // Fall back to displaying the variable key as its value. - let fallback_value = key.clone(); - args.set(key, fallback_value); + } else if python_value.is_instance_of::() { + // Display the Python date as YYYY-MM-DD. + match python_value.extract::() { + Ok(chrono_date) => { + args.set(key, chrono_date.format("%Y-%m-%d").to_string()); + } + _ => { + // Could not convert. + // Fall back to displaying the variable key as its value. + let fallback_value = key.clone(); + args.set(key, fallback_value); + } } + } else { + // The variable value was of an unsupported type. + // Fall back to displaying the variable key as its value. + let fallback_value = key.clone(); + args.set(key, fallback_value); } - } else { - // The variable value was of an unsupported type. - // Fall back to displaying the variable key as its value. - let fallback_value = key.clone(); - args.set(key, fallback_value); } } - } - let value = self - .bundle - .format_pattern(pattern, Some(&args), &mut errors); - Ok(value.to_string()) + let value = self + .bundle + .format_pattern(pattern, Some(&args), &mut errors); + Ok(value.to_string()) + } } } From 9dc68e6911a44730547790843d3bda03633a4d22 Mon Sep 17 00:00:00 2001 From: Lily Acorn Date: Fri, 1 Aug 2025 10:27:14 +0100 Subject: [PATCH 2/6] Raise a ValueError instead of panicking for invalid language --- src/lib.rs | 9 ++++++++- tests/test_python_interface.py | 7 +++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 0dafc9e..c916b98 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -33,7 +33,14 @@ mod rustfluent { ftl_filenames: &'_ Bound<'_, PyList>, strict: bool, ) -> PyResult { - let langid: LanguageIdentifier = language.parse().expect("Parsing failed"); + let langid: LanguageIdentifier = match language.parse() { + Ok(langid) => langid, + Err(_) => { + return Err(PyValueError::new_err(format!( + "Invalid language: '{language}'" + ))); + } + }; let mut bundle = FluentBundle::new_concurrent(vec![langid]); for file_path in ftl_filenames.iter() { diff --git a/tests/test_python_interface.py b/tests/test_python_interface.py index 4d6aa32..03be29d 100644 --- a/tests/test_python_interface.py +++ b/tests/test_python_interface.py @@ -69,6 +69,13 @@ def test_variables_of_different_types(description, identifier, variables, expect assert result == expected +def test_invalid_language(): + with pytest.raises(ValueError) as exc_info: + fluent.Bundle("$", []) + + assert str(exc_info.value) == "Invalid language: '$'" + + @pytest.mark.parametrize( "key", ( From 76af155df099730e3ebb25a680882f36736eead9 Mon Sep 17 00:00:00 2001 From: Lily Acorn Date: Fri, 1 Aug 2025 10:43:19 +0100 Subject: [PATCH 3/6] Use tuple unpacking in for loop --- src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c916b98..011c6b6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -89,10 +89,9 @@ mod rustfluent { let mut args = FluentArgs::new(); if let Some(variables) = variables { - for variable in variables { + for (python_key, python_value) in variables { // Make sure the variable key is a Python string, // raising a TypeError if not. - let python_key = variable.0; if !python_key.is_instance_of::() { return Err(PyTypeError::new_err(format!( "Variable key not a str, got {python_key}." @@ -101,7 +100,6 @@ mod rustfluent { let key = python_key.to_string(); // Set the variable value as a string or integer, // raising a TypeError if not. - let python_value = variable.1; if python_value.is_instance_of::() { args.set(key, python_value.to_string()); } else if python_value.is_instance_of::() { From 245266e23a930cf3e5722c197212e4c0ae350687 Mon Sep 17 00:00:00 2001 From: Lily Acorn Date: Fri, 1 Aug 2025 10:53:12 +0100 Subject: [PATCH 4/6] Simplify setting FluentArgs based on Python type --- src/lib.rs | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 011c6b6..8996515 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -102,31 +102,14 @@ mod rustfluent { // raising a TypeError if not. if python_value.is_instance_of::() { args.set(key, python_value.to_string()); - } else if python_value.is_instance_of::() { - match python_value.extract::() { - Ok(int_value) => { - args.set(key, int_value); - } - _ => { - // The Python integer overflowed i32. - // Fall back to displaying the variable key as its value. - let fallback_value = key.clone(); - args.set(key, fallback_value); - } - } - } else if python_value.is_instance_of::() { - // Display the Python date as YYYY-MM-DD. - match python_value.extract::() { - Ok(chrono_date) => { - args.set(key, chrono_date.format("%Y-%m-%d").to_string()); - } - _ => { - // Could not convert. - // Fall back to displaying the variable key as its value. - let fallback_value = key.clone(); - args.set(key, fallback_value); - } - } + } else if python_value.is_instance_of::() + && let Ok(int_value) = python_value.extract::() + { + args.set(key, int_value); + } else if python_value.is_instance_of::() + && let Ok(chrono_date) = python_value.extract::() + { + args.set(key, chrono_date.format("%Y-%m-%d").to_string()); } else { // The variable value was of an unsupported type. // Fall back to displaying the variable key as its value. From ab7c4b1c0547c1a0c32625a90653e641e902edc8 Mon Sep 17 00:00:00 2001 From: Lily Acorn Date: Fri, 1 Aug 2025 11:13:40 +0100 Subject: [PATCH 5/6] Move errors Vec next to bundle.format_pattern call site --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 8996515..668d944 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -81,7 +81,6 @@ mod rustfluent { .get_message(identifier) .ok_or_else(|| (PyValueError::new_err(format!("{identifier} not found"))))?; - let mut errors = vec![]; let pattern = msg.value().ok_or_else(|| { PyValueError::new_err(format!("{identifier} - Message has no value.",)) })?; @@ -119,6 +118,7 @@ mod rustfluent { } } + let mut errors = vec![]; let value = self .bundle .format_pattern(pattern, Some(&args), &mut errors); From e0e068d5bc44b4a884f118b4fd44b2d3cd19f521 Mon Sep 17 00:00:00 2001 From: Lily Acorn Date: Fri, 1 Aug 2025 14:44:59 +0100 Subject: [PATCH 6/6] Simplify non-strict error handling with tuple unpacking --- src/lib.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 668d944..ea2bf12 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -55,11 +55,7 @@ mod rustfluent { "Error when parsing {file_path}." ))); } - Err(error) => { - // The first element of the error is the parsed resource, minus any - // invalid messages. - error.0 - } + Err((resource, _errors)) => resource, }; bundle.add_resource_overriding(resource); }