Skip to content

feat: add strings.repeat builtin #35

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
anakrish wants to merge 1 commit into from
Closed

feat: add strings.repeat builtin #35

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
20 changes: 20 additions & 0 deletions src/builtins/strings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub fn register(m: &mut builtins::BuiltinsMap<&'static str, builtins::BuiltinFcn
m.insert("strings.any_suffix_match", (any_suffix_match, 2));
m.insert("strings.count", (strings_count, 2));
m.insert("strings.replace_n", (replace_n, 2));
m.insert("strings.repeat", (repeat, 2));

Copilot AI Apr 25, 2026

Copy link

Choose a reason for hiding this comment

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

strings.repeat is now registered but the builtin list in docs/builtins.md has a dedicated Strings table that enumerates supported builtins (e.g., strings.replace_n, strings.reverse). Add strings.repeat there as well so the documented builtin surface matches what the engine exposes.

Copilot uses AI. Check for mistakes.
m.insert("strings.reverse", (reverse, 1));
m.insert("substring", (substring, 3));
m.insert("trim", (trim, 2));
Expand Down Expand Up @@ -591,6 +592,25 @@ fn reverse(span: &Span, params: &[Ref<Expr>], args: &[Value], _strict: bool) ->
Ok(Value::String(s.chars().rev().collect::<String>().into()))
}

fn repeat(span: &Span, params: &[Ref<Expr>], args: &[Value], _strict: bool) -> Result<Value> {
let name = "strings.repeat";
ensure_args_count(span, name, params, args, 2)?;

let s = ensure_string(name, &params[0], &args[0])?;
let count = args[1].as_f64().unwrap() as usize;
Comment on lines +595 to +600

Copilot AI Apr 25, 2026

Copy link

Choose a reason for hiding this comment

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

args[1].as_f64().unwrap() as usize can panic (unwrap), silently truncates non-integers, and will produce a huge usize for negative counts due to casting. This violates the project’s no-panics requirement and yields incorrect semantics for invalid count values. Parse count via ensure_numeric and validate it as a non-negative integer (e.g., using validate_integer_arg), then convert using an explicit checked/option-based conversion (returning Undefined or a strict-mode error as appropriate).

Suggested change
fn repeat(span: &Span, params: &[Ref<Expr>], args: &[Value], _strict: bool) -> Result<Value> {
let name = "strings.repeat";
ensure_args_count(span, name, params, args, 2)?;
let s = ensure_string(name, &params[0], &args[0])?;
let count = args[1].as_f64().unwrap() as usize;
fn repeat(span: &Span, params: &[Ref<Expr>], args: &[Value], strict: bool) -> Result<Value> {
let name = "strings.repeat";
ensure_args_count(span, name, params, args, 2)?;
let s = ensure_string(name, &params[0], &args[0])?;
let count = ensure_numeric(name, &params[1], &args[1])?;
let count = match count.as_i64().and_then(|value| usize::try_from(value).ok()) {
Some(count) => count,
None if strict => {
bail!(params[1]
.span()
.error("`strings.repeat` expects a non-negative integer count"))
}
None => return Ok(Value::Undefined),
};

Copilot uses AI. Check for mistakes.

if count == 0 {
return Ok(Value::String("".into()));
}

let mut result = String::new();
Comment on lines +605 to +606

Copilot AI Apr 25, 2026

Copy link

Choose a reason for hiding this comment

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

result is built with String::new() and repeated push_str, which can lead to repeated reallocations for larger counts. Consider reserving capacity up front using a checked length computation (e.g., checked_mul of s.len() and count) or otherwise minimizing reallocations, while ensuring the approach can’t panic on overflow.

Suggested change
let mut result = String::new();
let capacity = s
.len()
.checked_mul(count)
.ok_or_else(|| params[1].span().error("repeat count produces a string that is too large"))?;
let mut result = String::new();
result
.try_reserve(capacity)
.map_err(|_| params[1].span().error("repeat count produces a string that is too large"))?;

Copilot uses AI. Check for mistakes.
for _ in 0..count {
Comment on lines +605 to +607

Copilot AI Apr 25, 2026

Copy link

Choose a reason for hiding this comment

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

The repeat loop grows result potentially many times without any call to enforce_limit(). Other builtins that accumulate in loops call enforce_limit() to respect the engine’s memory limits; without it, strings.repeat can more easily trigger OOM/DoS for large count. Add limit enforcement during repetition (and/or before growth) so large repeats fail gracefully under configured limits.

Suggested change
let mut result = String::new();
for _ in 0..count {
let part_len = s.len();
let mut result = String::new();
for _ in 0..count {
let next_len = match result.len().checked_add(part_len) {
Some(next_len) => next_len,
None => bail!(params[1].span().error("repeat result exceeds supported size")),
};
enforce_limit(next_len)?;

Copilot uses AI. Check for mistakes.
result.push_str(&s);
}

Ok(Value::String(result.into()))
}
Comment on lines +595 to +612

Copilot AI Apr 25, 2026

Copy link

Choose a reason for hiding this comment

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

A new builtin (strings.repeat) is being added but there are no corresponding interpreter YAML test cases under tests/interpreter/cases/builtins/strings/ (this directory already contains coverage for other string builtins like concat, contains, etc.). Add tests for basic behavior (e.g., count 0/1/N), Unicode handling, and invalid count cases (non-integer / negative) to lock down semantics across execution paths.

Copilot uses AI. Check for mistakes.

fn substring(span: &Span, params: &[Ref<Expr>], args: &[Value], strict: bool) -> Result<Value> {
let name = "substring";
ensure_args_count(span, name, params, args, 3)?;
Expand Down
Loading