Extend expression engine with built-in functions, new operators, and Value::map()#62
Conversation
Agent-Logs-Url: https://github.com/ashyanSpada/expression_engine_rs/sessions/4fa8d4bb-a3b9-451f-a5c8-f2bfd0df5242 Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #62 +/- ##
==========================================
+ Coverage 89.56% 90.66% +1.10%
==========================================
Files 11 11
Lines 1064 1211 +147
==========================================
+ Hits 953 1098 +145
- Misses 111 113 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@copilot check failed. Please review your changes and conduct related fix. |
Agent-Logs-Url: https://github.com/ashyanSpada/expression_engine_rs/sessions/dbc10470-ecea-4681-bc83-ee686fd44aef Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Extends the expression engine with additional built-in utility functions, adds contains/notIn infix operators, and introduces a missing Value::map() accessor to support map operations.
Changes:
- Added
Value::map()accessor plus unit tests. - Registered new infix operators
notIn(list non-membership) andcontains(string containment), with tests. - Added a set of new built-in functions (math/string/list/map) and documented them in the README.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/value.rs |
Adds Value::map() extractor to complement existing accessors and supports map-focused built-ins. |
src/operator.rs |
Registers notIn/contains as infix operators and adds operator-level tests. |
src/function.rs |
Implements new built-in functions and adds a dedicated test module for them. |
README.md |
Documents the new operators and provides a built-in functions reference table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "abs", | ||
| Arc::new(|params| { | ||
| if params.is_empty() { | ||
| return Err(Error::ParamEmpty("abs".to_string())); | ||
| } | ||
| let val = params.into_iter().next().unwrap().decimal()?; | ||
| Ok(Value::Number(val.abs())) |
There was a problem hiding this comment.
abs (and several other unary functions added here) only checks params.is_empty() and then uses the first argument, silently ignoring any extra arguments (e.g., abs(1, 2) would return 1). Since the README documents these as single-argument functions, consider enforcing the exact arity (e.g., params.len() == 1) and returning Error::ParamInvalid() otherwise.
| "pow", | ||
| Arc::new(|params| { | ||
| if params.len() < 2 { | ||
| return Err(Error::ParamEmpty("pow".to_string())); | ||
| } | ||
| let mut iter = params.into_iter(); | ||
| let base = iter.next().unwrap().float()?; | ||
| let exp = iter.next().unwrap().float()?; | ||
| Ok(Value::from(base.powf(exp))) |
There was a problem hiding this comment.
pow uses Error::ParamEmpty("pow") when fewer than 2 args are provided, but that error message is misleading for the pow(2) case (params aren’t empty, just insufficient). It also ignores any extra args. Consider validating the arity explicitly (exactly 2 args) and returning Error::ParamInvalid() on mismatch.
| let base = iter.next().unwrap().float()?; | ||
| let exp = iter.next().unwrap().float()?; | ||
| Ok(Value::from(base.powf(exp))) | ||
| }), | ||
| ); | ||
|
|
||
| self.register( | ||
| "sqrt", | ||
| Arc::new(|params| { | ||
| if params.is_empty() { | ||
| return Err(Error::ParamEmpty("sqrt".to_string())); | ||
| } | ||
| let val = params.into_iter().next().unwrap().float()?; | ||
| Ok(Value::from(val.sqrt())) | ||
| }), |
There was a problem hiding this comment.
pow/sqrt return Value::from(f64) results. In Value::from(f64) the conversion uses Decimal::from_f64(...).unwrap_or_default(), so non-finite results (NaN/±inf) or values outside Decimal’s representable range will silently turn into 0 instead of erroring. Consider converting via Decimal::from_f64(...) here and returning an error (e.g., Error::InvalidFloat/Error::InvalidNumber) when conversion fails or the result is non-finite.
|
|
||
| #[cfg(test)] | ||
| mod tetst { | ||
| #[cfg(test)] |
There was a problem hiding this comment.
There are two consecutive #[cfg(test)] attributes applied to the tests module, which is redundant. Consider removing one of them to keep the module declaration clean.
| #[cfg(test)] |
The engine had only 4 built-in functions (
min,max,sum,mul) and lacked common math, string, list, and map utilities. This PR adds 18 new built-in functions, 2 new infix operators, and the missingValue::map()accessor.New built-in functions
Math:
abs,floor,ceil,round,avg,pow,sqrtString/conversion:
len(string/list/map),upper,lower,trim,concat,str,numList:
first,lastMap:
keys,valuesNew infix operators
contains— string containment, completing thebeginWith/endWith/infamilynotIn— list non-membership, complement of the existinginoperatorValue::map()accessorShouldBeMaperror variant existed but had no corresponding extractor method. AddedValue::map() -> Result<Vec<(Value, Value)>>to mirrorValue::list(), used internally bykeys/values.Notes
containsis registered as an infix operator only (not a function) because the tokenizer prioritizes operator recognition over function names for alphabetic tokens — consistent with howbeginWith,endWith, andinare handled.