Skip to content

[JULES] Refactor text stdlib to reduce duplication #374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
logbie wants to merge 1 commit into from
Closed

[JULES] Refactor text stdlib to reduce duplication #374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions src/stdlib/helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,51 @@ pub fn expect_number(value: &Value) -> Result<f64, RuntimeError> {
}
}

/// Helper for unary text operations (text -> text)
///
/// Handles argument count checking, type extraction, operation execution,
/// and result wrapping.
///
/// # Arguments
///
/// * `func_name` - Name of the function for error messages
/// * `args` - Arguments passed to the function
/// * `op` - The text operation to perform
pub fn unary_text_op<F, R>(func_name: &str, args: Vec<Value>, op: F) -> Result<Value, RuntimeError>
where
F: Fn(&str) -> R,
R: Into<Arc<str>>,
{
check_arg_count(func_name, &args, 1)?;
let text = expect_text(&args[0])?;
Ok(Value::Text(op(&text).into()))
}

/// Helper for binary text predicates ((text, text) -> bool)
///
/// Handles argument count checking, type extraction, operation execution,
/// and result wrapping.
///
/// # Arguments
///
/// * `func_name` - Name of the function for error messages
/// * `args` - Arguments passed to the function
/// * `op` - The text predicate to perform
pub fn binary_text_predicate<F>(
func_name: &str,
args: Vec<Value>,
op: F,
) -> Result<Value, RuntimeError>
where
F: Fn(&str, &str) -> bool,
{
check_arg_count(func_name, &args, 2)?;
let text = expect_text(&args[0])?;
let other = expect_text(&args[1])?;
Ok(Value::Bool(op(&text, &other)))
}


/// Helper for unary math operations (f64 -> f64)
///
/// Handles argument count checking, type extraction, operation execution,
Expand Down Expand Up @@ -473,3 +518,48 @@ pub fn expect_datetime(value: &Value) -> Result<Rc<chrono::NaiveDateTime>, Runti
)),
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_unary_text_op() {
let op = |s: &str| s.to_uppercase();
let result = unary_text_op("test_op", vec![Value::Text(Arc::from("hello"))], op).unwrap();
assert_eq!(result, Value::Text(Arc::from("HELLO")));
}

#[test]
fn test_unary_text_op_arg_count() {
let op = |s: &str| s.to_uppercase();
assert!(unary_text_op("test_op", vec![], op).is_err());
}

#[test]
fn test_unary_text_op_wrong_type() {
let op = |s: &str| s.to_uppercase();
assert!(unary_text_op("test_op", vec![Value::Number(1.0)], op).is_err());
}

#[test]
fn test_binary_text_predicate() {
let op = |a: &str, b: &str| a.starts_with(b);
let result = binary_text_predicate(
"test_pred",
vec![
Value::Text(Arc::from("hello world")),
Value::Text(Arc::from("hello")),
],
op,
)
.unwrap();
assert_eq!(result, Value::Bool(true));
}

#[test]
fn test_binary_text_predicate_arg_count() {
let op = |a: &str, b: &str| a.starts_with(b);
assert!(binary_text_predicate("test_pred", vec![], op).is_err());
}
Comment on lines +560 to +564

Copilot AI Feb 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test coverage for binary_text_predicate is incomplete. While there are tests for successful execution and incorrect argument count, there's no test verifying that the function correctly rejects arguments of the wrong type (e.g., passing a Number instead of Text). Consider adding a test similar to test_unary_text_op_wrong_type to ensure type validation works correctly.

Copilot uses AI. Check for mistakes.
}
100 changes: 42 additions & 58 deletions src/stdlib/text.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use super::helpers::{check_arg_count, expect_number, expect_text};
use super::helpers::{
binary_text_predicate, check_arg_count, expect_number, expect_text, unary_text_op,
};
use crate::interpreter::environment::Environment;
use crate::interpreter::error::RuntimeError;
use crate::interpreter::value::Value;
Expand Down Expand Up @@ -74,19 +76,11 @@ fn parse_key_value_pairs(input: &str, delimiter: char) -> std::collections::Hash
// which handles both text and lists

pub fn native_touppercase(args: Vec<Value>) -> Result<Value, RuntimeError> {
check_arg_count("touppercase", &args, 1)?;

let text = expect_text(&args[0])?;
let uppercase = text.to_uppercase();
Ok(Value::Text(Arc::from(uppercase)))
unary_text_op("touppercase", args, |s| s.to_uppercase())
}

pub fn native_tolowercase(args: Vec<Value>) -> Result<Value, RuntimeError> {
check_arg_count("tolowercase", &args, 1)?;

let text = expect_text(&args[0])?;
let lowercase = text.to_lowercase();
Ok(Value::Text(Arc::from(lowercase)))
unary_text_op("tolowercase", args, |s| s.to_lowercase())
}

pub fn native_substring(args: Vec<Value>) -> Result<Value, RuntimeError> {
Expand Down Expand Up @@ -135,29 +129,15 @@ pub fn native_string_split(args: Vec<Value>) -> Result<Value, RuntimeError> {
}

pub fn native_trim(args: Vec<Value>) -> Result<Value, RuntimeError> {
check_arg_count("trim", &args, 1)?;

let text = expect_text(&args[0])?;
let trimmed = text.trim();
Ok(Value::Text(Arc::from(trimmed)))
unary_text_op("trim", args, |s| Arc::from(s.trim()))

Copilot AI Feb 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The explicit Arc::from() call in the trim closure is unnecessary. The helper function unary_text_op already converts the result to Arc<str> via .into() at line 216 of helpers.rs. For consistency with touppercase, tolowercase, and other text operations, and to avoid a redundant conversion, the closure should return &str or String directly. Change to: unary_text_op("trim", args, |s| s.trim())

Copilot uses AI. Check for mistakes.
}

pub fn native_starts_with(args: Vec<Value>) -> Result<Value, RuntimeError> {
check_arg_count("starts_with", &args, 2)?;

let text = expect_text(&args[0])?;
let prefix = expect_text(&args[1])?;
let result = text.starts_with(prefix.as_ref());
Ok(Value::Bool(result))
binary_text_predicate("starts_with", args, |text, prefix| text.starts_with(prefix))
}

pub fn native_ends_with(args: Vec<Value>) -> Result<Value, RuntimeError> {
check_arg_count("ends_with", &args, 2)?;

let text = expect_text(&args[0])?;
let suffix = expect_text(&args[1])?;
let result = text.ends_with(suffix.as_ref());
Ok(Value::Bool(result))
binary_text_predicate("ends_with", args, |text, suffix| text.ends_with(suffix))
}

/// Parse query string into WFL object
Expand Down Expand Up @@ -249,8 +229,17 @@ fn validated_pad_width(raw: f64) -> Result<usize, RuntimeError> {
Ok((raw as usize).min(MAX_PAD_WIDTH))
}

pub fn native_padleft(args: Vec<Value>) -> Result<Value, RuntimeError> {
check_arg_count("padleft", &args, 2)?;
enum PadDirection {
Left,
Right,
}

fn perform_pad(args: Vec<Value>, direction: PadDirection) -> Result<Value, RuntimeError> {
let func_name = match direction {
PadDirection::Left => "padleft",
PadDirection::Right => "padright",
};
check_arg_count(func_name, &args, 2)?;

let text = expect_text(&args[0])?;
let width = validated_pad_width(expect_number(&args[1])?)?;
Expand All @@ -259,45 +248,40 @@ pub fn native_padleft(args: Vec<Value>) -> Result<Value, RuntimeError> {
Ok(Value::Text(Arc::clone(&text)))
} else {
let padding = " ".repeat(width - len);
Ok(Value::Text(Arc::from(format!("{}{}", padding, text))))
let result = match direction {
PadDirection::Left => format!("{}{}", padding, text),
PadDirection::Right => format!("{}{}", text, padding),
};
Ok(Value::Text(Arc::from(result)))
}
}

pub fn native_padright(args: Vec<Value>) -> Result<Value, RuntimeError> {
check_arg_count("padright", &args, 2)?;
pub fn native_padleft(args: Vec<Value>) -> Result<Value, RuntimeError> {
perform_pad(args, PadDirection::Left)
}

let text = expect_text(&args[0])?;
let width = validated_pad_width(expect_number(&args[1])?)?;
let len = text.chars().count();
if len >= width {
Ok(Value::Text(Arc::clone(&text)))
} else {
let padding = " ".repeat(width - len);
Ok(Value::Text(Arc::from(format!("{}{}", text, padding))))
}
pub fn native_padright(args: Vec<Value>) -> Result<Value, RuntimeError> {
perform_pad(args, PadDirection::Right)
}

pub fn native_capitalize(args: Vec<Value>) -> Result<Value, RuntimeError> {
check_arg_count("capitalize", &args, 1)?;

let text = expect_text(&args[0])?;
let mut chars = text.chars();
let result = match chars.next() {
Some(c) => {
let upper: String = c.to_uppercase().collect();
format!("{}{}", upper, chars.as_str())
unary_text_op("capitalize", args, |text| {
let mut chars = text.chars();
match chars.next() {
Some(c) => {
let upper: String = c.to_uppercase().collect();
format!("{}{}", upper, chars.as_str())
}
None => String::new(),
}
None => String::new(),
};
Ok(Value::Text(Arc::from(result)))
})
}

pub fn native_reverse_text(args: Vec<Value>) -> Result<Value, RuntimeError> {
check_arg_count("reverse", &args, 1)?;

let text = expect_text(&args[0])?;
let reversed: String = text.chars().rev().collect();
Ok(Value::Text(Arc::from(reversed)))
unary_text_op("reverse", args, |text| {
let reversed: String = text.chars().rev().collect();
reversed
})
}

pub fn register_text(env: &mut Environment) {
Expand Down