From aebff4584bd44426039328ea0617117281e3ee4a Mon Sep 17 00:00:00 2001 From: Binyamin Klein Date: Sun, 8 Feb 2026 14:39:33 +0200 Subject: [PATCH 1/5] DEV-6802 Add FIND, SEARCH, and ISERROR functions with corresponding calculations and argument handling --- src/calculate/args.rs | 29 ++++++ src/calculate/operation/function.rs | 118 ++++++++++++++++++++- src/calculate/operation/string.rs | 156 +++++++++++++++++++++++++++- src/grammar.pest | 5 +- src/parse_formula.rs | 6 ++ src/types.rs | 3 + tests/common/mod.rs | 30 ++++++ tests/find.rs | 83 +++++++++++++++ tests/iserror.rs | 49 +++++++++ tests/search.rs | 62 +++++++++++ 10 files changed, 536 insertions(+), 5 deletions(-) create mode 100644 tests/common/mod.rs create mode 100644 tests/find.rs create mode 100644 tests/iserror.rs create mode 100644 tests/search.rs diff --git a/src/calculate/args.rs b/src/calculate/args.rs index df574ed..6bf94a6 100644 --- a/src/calculate/args.rs +++ b/src/calculate/args.rs @@ -58,6 +58,35 @@ where (first, middle, last) } +/// Returns (find_text, within_text, start_num) for FIND/SEARCH. +/// start_num defaults to 1 if only two arguments are provided. +pub fn get_find_args( + mut exp: types::Expression, + f: Option<&impl Fn(String) -> types::Value>, +) -> (types::Value, types::Value, types::Value) +where + N: XlNum, + ::Err: Debug, +{ + let start_num = if exp.values.len() >= 3 { + match exp.values.pop() { + Some(formula) => calculate_formula(formula, f), + None => types::Value::Number(N::one()), + } + } else { + types::Value::Number(N::one()) + }; + let within_text = match exp.values.pop() { + Some(formula) => calculate_formula(formula, f), + None => types::Value::Error(types::Error::Argument), + }; + let find_text = match exp.values.pop() { + Some(formula) => calculate_formula(formula, f), + None => types::Value::Error(types::Error::Argument), + }; + (find_text, within_text, start_num) +} + pub fn get_number_and_string_values( mut exp: types::Expression, f: Option<&impl Fn(String) -> types::Value>, diff --git a/src/calculate/operation/function.rs b/src/calculate/operation/function.rs index c98b3ae..93be5b8 100644 --- a/src/calculate/operation/function.rs +++ b/src/calculate/operation/function.rs @@ -5,13 +5,16 @@ use super::{ calculate_abs, calculate_average, calculate_collective_operator, calculate_collective_product_operator, }, + string::{ + find_position_case_sensitive, search_position_with_wildcards, value_to_string_for_find, + }, }; use crate::{ calculate::args::{ - get_binary_function_args, get_number_and_string_values, get_ternary_function_args, - get_unary_function_arg, + get_binary_function_args, get_find_args, get_number_and_string_values, + get_ternary_function_args, get_unary_function_arg, }, - types::{self, XlNum}, + types::{self, Boolean, Error, XlNum}, }; use std::{fmt::Debug, str::FromStr}; @@ -135,5 +138,114 @@ where types::Function::Year => calculate_year(get_unary_function_arg(exp, f)), types::Function::Month => calculate_month(get_unary_function_arg(exp, f)), types::Function::Day => calculate_day(get_unary_function_arg(exp, f)), + types::Function::Find => calculate_find(get_find_args(exp, f)), + types::Function::Search => calculate_search(get_find_args(exp, f)), + types::Function::IsError => calculate_iserror(get_unary_function_arg(exp, f)), + } +} + +fn calculate_find( + (find_text, within_text, start_num): ( + types::Value, + types::Value, + types::Value, + ), +) -> types::Value +where + N: XlNum, + ::Err: Debug, +{ + if let types::Value::Error(e) = find_text { + return types::Value::Error(e); + } + if let types::Value::Error(e) = within_text { + return types::Value::Error(e); + } + if let types::Value::Error(e) = start_num { + return types::Value::Error(e); + } + let find_s = match value_to_string_for_find(&find_text) { + Ok(s) => s, + Err(v) => return v, + }; + let within_s = match value_to_string_for_find(&within_text) { + Ok(s) => s, + Err(v) => return v, + }; + let start_i64: i64 = match &start_num { + types::Value::Number(n) => n.as_(), + _ => return types::Value::Error(Error::Value), + }; + if start_i64 <= 0 { + return types::Value::Error(Error::Value); + } + let within_len = within_s.chars().count() as i64; + if start_i64 > within_len { + return types::Value::Error(Error::Value); + } + match find_position_case_sensitive(&find_s, &within_s, start_i64) { + Some(pos) => types::Value::Number( + N::from_i64(pos).unwrap_or_else(|| N::from_f64(1.0).unwrap()), + ), + None => types::Value::Error(Error::Value), + } +} + +fn calculate_search( + (find_text, within_text, start_num): ( + types::Value, + types::Value, + types::Value, + ), +) -> types::Value +where + N: XlNum, + ::Err: Debug, +{ + if let types::Value::Error(e) = find_text { + return types::Value::Error(e); + } + if let types::Value::Error(e) = within_text { + return types::Value::Error(e); + } + if let types::Value::Error(e) = start_num { + return types::Value::Error(e); } + let find_s = match value_to_string_for_find(&find_text) { + Ok(s) => s, + Err(v) => return v, + }; + let within_s = match value_to_string_for_find(&within_text) { + Ok(s) => s, + Err(v) => return v, + }; + let start_i64: i64 = match &start_num { + types::Value::Number(n) => n.as_(), + _ => return types::Value::Error(Error::Value), + }; + if start_i64 <= 0 { + return types::Value::Error(Error::Value); + } + let within_len = within_s.chars().count() as i64; + if start_i64 > within_len { + return types::Value::Error(Error::Value); + } + match search_position_with_wildcards(&find_s, &within_s, start_i64) { + Some(pos) => types::Value::Number( + N::from_i64(pos).unwrap_or_else(|| N::from_f64(1.0).unwrap()), + ), + None => types::Value::Error(Error::Value), + } +} + +fn calculate_iserror(arg: types::Value) -> types::Value +where + N: XlNum, +{ + let is_err = matches!(arg, types::Value::Error(_)); + types::Value::Boolean(if is_err { + Boolean::True + } else { + Boolean::False + }) } diff --git a/src/calculate/operation/string.rs b/src/calculate/operation/string.rs index b46f02c..2b282c7 100644 --- a/src/calculate/operation/string.rs +++ b/src/calculate/operation/string.rs @@ -1,4 +1,4 @@ -use crate::types::{self, XlNum}; +use crate::types::{self, Boolean, XlNum}; use std::{fmt::Debug, str::FromStr}; pub fn calculate_concat_operator(str1: &str, str2: &str) -> String { @@ -56,3 +56,157 @@ where { types::Value::Boolean(f(string1, string2).into()) } + +/// Coerce a formula value to string for FIND/SEARCH (Excel semantics). +/// Returns Ok(s) or Err(Value::Error) for Date/Iterator. +pub fn value_to_string_for_find(v: &types::Value) -> Result> +where + N: XlNum, + ::Err: Debug, +{ + match v { + types::Value::Error(e) => Err(types::Value::Error(*e)), + types::Value::Number(n) => Ok(n.to_string()), + types::Value::Text(s) => Ok(s.clone()), + types::Value::Boolean(b) => Ok(match b { + Boolean::True => "TRUE".to_string(), + Boolean::False => "FALSE".to_string(), + }), + types::Value::Blank => Ok(String::new()), + types::Value::Date(_) | types::Value::Iterator(_) => { + Err(types::Value::Error(types::Error::Value)) + } + } +} + +/// FIND: case-sensitive, no wildcards. Returns 1-based character position or None. +pub fn find_position_case_sensitive( + find_text: &str, + within_text: &str, + start_num_1based: i64, +) -> Option { + let start = (start_num_1based - 1) as usize; + let char_count = within_text.chars().count(); + if start_num_1based < 1 || start >= char_count { + return None; + } + if find_text.is_empty() { + return Some(start_num_1based); + } + let rest: String = within_text.chars().skip(start).collect(); + let byte_offset = rest.find(find_text)?; + let chars_before = rest[..byte_offset].chars().count(); + let one_based = (start + chars_before + 1) as i64; + Some(one_based) +} + +/// SEARCH: case-insensitive with wildcards ? * and ~ escape. Returns 1-based character position or None. +pub fn search_position_with_wildcards( + find_text: &str, + within_text: &str, + start_num_1based: i64, +) -> Option { + let start = (start_num_1based - 1) as usize; + let char_count = within_text.chars().count(); + if start_num_1based < 1 || start >= char_count { + return None; + } + if find_text.is_empty() { + return Some(start_num_1based); + } + let rest: String = within_text.chars().skip(start).collect(); + let rest_lower: String = rest.to_lowercase(); + let pattern_orig: Vec = find_text.chars().collect(); + let pattern_lower: Vec = find_text.to_lowercase().chars().collect(); + let rest_orig: Vec = rest.chars().collect(); + let rest_chars: Vec = rest_lower.chars().collect(); + let mut best: Option = None; + for i in 0..=rest_chars.len() { + if let Some(offset) = match_pattern( + &pattern_lower, + &pattern_orig, + &rest_chars, + &rest_orig, + i, + 0, + i, + ) { + let pos = start + offset + 1; + best = Some(match best { + None => pos, + Some(p) => p.min(pos), + }); + } + } + best.map(|p| p as i64) +} + +/// Match pattern (with ? * ~) against text from position. Returns Some(offset) where offset +/// is the 0-based position in text where the matched substring starts (for leading *, this +/// is where the remainder matches, not where * started). +fn match_pattern( + pattern_lower: &[char], + pattern_orig: &[char], + text_lower: &[char], + text_orig: &[char], + text_pos: usize, + pattern_pos: usize, + match_start: usize, +) -> Option { + if pattern_pos >= pattern_lower.len() { + return Some(match_start); + } + let mut p = pattern_pos; + let mut t = text_pos; + while p < pattern_lower.len() { + if t > text_lower.len() { + return None; + } + if p + 1 < pattern_orig.len() && pattern_orig[p] == '~' { + let next = pattern_orig[p + 1]; + if next == '?' || next == '*' || next == '~' { + if t >= text_orig.len() || text_orig[t] != next { + return None; + } + p += 2; + t += 1; + continue; + } + } + match pattern_lower[p] { + '?' => { + if t >= text_lower.len() { + return None; + } + p += 1; + t += 1; + } + '*' => { + let rest_pat_lower: Vec = pattern_lower[p + 1..].to_vec(); + let rest_pat_orig: Vec = pattern_orig[p + 1..].to_vec(); + for skip in 0..=(text_lower.len() - t) { + if let Some(off) = match_pattern( + &rest_pat_lower, + &rest_pat_orig, + text_lower, + text_orig, + t + skip, + 0, + t + skip, + ) { + return Some(off); + } + } + return None; + } + c => { + if t >= text_lower.len() || text_lower[t] != c { + return None; + } + p += 1; + t += 1; + } + } + } + Some(match_start) +} diff --git a/src/grammar.pest b/src/grammar.pest index 30a80e7..0f72991 100644 --- a/src/grammar.pest +++ b/src/grammar.pest @@ -36,7 +36,7 @@ logical_operator = _{ equal | not_equal | greater_or_equal | greater greater_or_equal= { ">=" } less_or_equal = { "<=" } -function = _{ abs | sum | product | average | negate | days | year | month | day | right | left | iff | isblank | custom_function } +function = _{ abs | sum | product | average | negate | days | year | month | day | right | left | iff | isblank | find | search | iserror | custom_function } abs = { ^"ABS" ~ "(" ~ expr ~ ")" } sum = { ^"SUM" ~ function_param_with_atomic_expr} product = { ^"PRODUCT" ~ function_param_with_atomic_expr} @@ -50,6 +50,9 @@ function = _{ abs | sum | product | average | negate | days | year | month | day left = { ^"LEFT" ~ function_param } iff = { ^"IF" ~ three_params } isblank = { ^"ISBLANK" ~ function_param} + find = { ^"FIND" ~ function_param } + search = { ^"SEARCH" ~ function_param } + iserror = { ^"ISERROR" ~ function_param } custom_function = { reference ~ (function_param | empty_param) } logical_function = _{ or | and | xor | not } diff --git a/src/parse_formula.rs b/src/parse_formula.rs index a9d0eb7..7c0bbdc 100644 --- a/src/parse_formula.rs +++ b/src/parse_formula.rs @@ -194,6 +194,9 @@ fn rule_to_function_operator(collective_operation: Rule) -> types::Operator { Rule::year => types::Operator::Function(types::Function::Year), Rule::month => types::Operator::Function(types::Function::Month), Rule::day => types::Operator::Function(types::Function::Day), + Rule::find => types::Operator::Function(types::Function::Find), + Rule::search => types::Operator::Function(types::Function::Search), + Rule::iserror => types::Operator::Function(types::Function::IsError), _ => unreachable!(), } } @@ -404,6 +407,9 @@ where Rule::custom_function => build_formula_custom_function(pair, f), Rule::iff => build_formula_iff(pair, f), Rule::isblank => build_formula_collective_operator(Rule::isblank, pair, f), + Rule::find => build_formula_collective_operator(Rule::find, pair, f), + Rule::search => build_formula_collective_operator(Rule::search, pair, f), + Rule::iserror => build_formula_collective_operator(Rule::iserror, pair, f), _ => unreachable!(), }) .map_infix( diff --git a/src/types.rs b/src/types.rs index 90dcb8c..70d9bfb 100644 --- a/src/types.rs +++ b/src/types.rs @@ -24,6 +24,9 @@ pub enum Function { Year, Month, Day, + Find, + Search, + IsError, } /// Defines Excel Operators. diff --git a/tests/common/mod.rs b/tests/common/mod.rs new file mode 100644 index 0000000..93196b9 --- /dev/null +++ b/tests/common/mod.rs @@ -0,0 +1,30 @@ +use std::fmt::Debug; +use std::str::FromStr; +use xlformula_engine::{ + calculate, + parse_formula, + types::{self, XlNum}, + NoCustomFunction, NoReference, +}; + +/// Evaluate formula string and return the result as string (e.g. "1", "#VALUE!", "TRUE"). +pub fn evaluate_formula_string(s: &str) -> String +where + N: XlNum, + ::Err: Debug, +{ + let formula = parse_formula::parse_string_to_formula(s, None::>); + let result = calculate::calculate_formula(formula, None::>); + calculate::result_to_string(result) +} + +/// Evaluate formula and return the raw Value (for checking Error variant etc.). +#[allow(dead_code)] +pub fn evaluate_formula_value(s: &str) -> types::Value +where + N: XlNum, + ::Err: Debug, +{ + let formula = parse_formula::parse_string_to_formula(s, None::>); + calculate::calculate_formula(formula, None::>) +} diff --git a/tests/find.rs b/tests/find.rs new file mode 100644 index 0000000..17c0024 --- /dev/null +++ b/tests/find.rs @@ -0,0 +1,83 @@ +mod common; + +use std::fmt::Debug; +use std::str::FromStr; +use xlformula_engine::types::XlNum; + +fn eval(s: &str) -> String +where + N: XlNum, + ::Err: Debug, +{ + common::evaluate_formula_string::(s) +} + +#[test] +fn find_basic() { + assert_eq!(eval::(r#"=FIND("a","abc")"#), "1"); + assert_eq!(eval::(r#"=FIND("b","abc")"#), "2"); + assert_eq!(eval::(r#"=FIND("c","abc")"#), "3"); +} + +#[test] +fn find_not_found() { + assert_eq!(eval::(r#"=FIND("x","abc")"#), "#VALUE!"); +} + +#[test] +fn find_from_position() { + assert_eq!(eval::(r#"=FIND("a","abc",2)"#), "#VALUE!"); + assert_eq!(eval::(r#"=FIND("b","abc",2)"#), "2"); + assert_eq!(eval::(r#"=FIND("Y","AYF0093.YoungMensApparel",8)"#), "9"); +} + +#[test] +fn find_empty_find_text() { + assert_eq!(eval::(r#"=FIND("","abc")"#), "1"); + assert_eq!(eval::(r#"=FIND("","abc",2)"#), "2"); + // empty find_text with start past length returns #VALUE! + assert_eq!(eval::(r#"=FIND("","abc",4)"#), "#VALUE!"); +} + +#[test] +fn find_case_sensitive() { + assert_eq!(eval::(r#"=FIND("A","abc")"#), "#VALUE!"); + // Second "M" in "MiriamMcGovern" starting from position 3 is at 1-based position 7 + assert_eq!(eval::(r#"=FIND("M","MiriamMcGovern",3)"#), "7"); +} + +#[test] +fn find_start_num_bounds() { + assert_eq!(eval::(r#"=FIND("a","abc",0)"#), "#VALUE!"); + assert_eq!(eval::(r#"=FIND("a","abc",-1)"#), "#VALUE!"); + assert_eq!(eval::(r#"=FIND("a","abc",4)"#), "#VALUE!"); +} + +#[test] +fn find_start_num_truncate() { + assert_eq!(eval::(r#"=FIND("b","abc",2.9)"#), "2"); +} + +#[test] +fn find_coercion() { + assert_eq!(eval::(r#"=FIND(1,"121")"#), "1"); + assert_eq!(eval::(r#"=FIND(TRUE,"TRUEx")"#), "1"); + assert_eq!(eval::(r#"=FIND("1","121",1)"#), "1"); +} + +#[test] +fn find_character_based() { + assert_eq!(eval::(r#"=FIND("ö","föö")"#), "2"); +} + +#[test] +fn find_error_propagation() { + assert_eq!(eval::(r#"=FIND(1/0,"abc")"#), "#DIV/0!"); + assert_eq!(eval::(r#"=FIND("a",1/0)"#), "#DIV/0!"); +} + +#[test] +fn find_f32() { + assert_eq!(eval::(r#"=FIND("a","abc")"#), "1"); + assert_eq!(eval::(r#"=FIND("x","abc")"#), "#VALUE!"); +} diff --git a/tests/iserror.rs b/tests/iserror.rs new file mode 100644 index 0000000..16b9dc2 --- /dev/null +++ b/tests/iserror.rs @@ -0,0 +1,49 @@ +mod common; + +use std::fmt::Debug; +use std::str::FromStr; +use xlformula_engine::types::{self, XlNum}; + +fn eval(s: &str) -> String +where + N: XlNum, + ::Err: Debug, +{ + common::evaluate_formula_string::(s) +} + +fn value(s: &str) -> types::Value +where + N: XlNum, + ::Err: Debug, +{ + common::evaluate_formula_value::(s) +} + +#[test] +fn iserror_true_for_errors() { + assert_eq!(eval::(r#"=ISERROR(1/0)"#), "TRUE"); + assert_eq!(eval::(r#"=ISERROR(FIND("x","abc"))"#), "TRUE"); +} + +#[test] +fn iserror_false_for_non_errors() { + assert_eq!(eval::(r#"=ISERROR(1)"#), "FALSE"); + assert_eq!(eval::(r#"=ISERROR("")"#), "FALSE"); + assert_eq!(eval::(r#"=ISERROR(TRUE)"#), "FALSE"); + assert_eq!(eval::(r#"=ISERROR(FIND("a","abc"))"#), "FALSE"); +} + +#[test] +fn iserror_detects_error_value() { + let v = value::(r#"=ISERROR(1/0)"#); + assert!(matches!(v, types::Value::Boolean(types::Boolean::True))); + let v = value::(r#"=ISERROR(42)"#); + assert!(matches!(v, types::Value::Boolean(types::Boolean::False))); +} + +#[test] +fn iserror_f32() { + assert_eq!(eval::(r#"=ISERROR(1/0)"#), "TRUE"); + assert_eq!(eval::(r#"=ISERROR(1)"#), "FALSE"); +} diff --git a/tests/search.rs b/tests/search.rs new file mode 100644 index 0000000..4610bfc --- /dev/null +++ b/tests/search.rs @@ -0,0 +1,62 @@ +mod common; + +use std::fmt::Debug; +use std::str::FromStr; +use xlformula_engine::types::XlNum; + +fn eval(s: &str) -> String +where + N: XlNum, + ::Err: Debug, +{ + common::evaluate_formula_string::(s) +} + +#[test] +fn search_case_insensitive() { + assert_eq!(eval::(r#"=SEARCH("m","MiriamMcGovern")"#), "1"); + assert_eq!(eval::(r#"=SEARCH("M","MiriamMcGovern")"#), "1"); + assert_eq!(eval::(r#"=SEARCH("gloves","Gloves (Youth)")"#), "1"); + assert_eq!(eval::(r#"=SEARCH("M","abc")"#), "#VALUE!"); +} + +#[test] +fn search_wildcard_question() { + assert_eq!(eval::(r#"=SEARCH("sm?th","smith")"#), "1"); + assert_eq!(eval::(r#"=SEARCH("sm?th","smyth")"#), "1"); +} + +#[test] +fn search_wildcard_star() { + // "*east" matches "east" in "Northeast"; "east" starts at 1-based position 6 + assert_eq!(eval::(r#"=SEARCH("*east","Northeast")"#), "6"); +} + +#[test] +fn search_literal_tilde() { + assert_eq!(eval::(r#"=SEARCH("~?","a?b")"#), "2"); +} + +#[test] +fn search_empty_find_text() { + assert_eq!(eval::(r#"=SEARCH("","abc")"#), "1"); + assert_eq!(eval::(r#"=SEARCH("","abc",2)"#), "2"); + // empty find_text with start past length returns #VALUE! + assert_eq!(eval::(r#"=SEARCH("","abc",4)"#), "#VALUE!"); +} + +#[test] +fn search_not_found() { + assert_eq!(eval::(r#"=SEARCH("x","abc")"#), "#VALUE!"); +} + +#[test] +fn search_start_num_bounds() { + assert_eq!(eval::(r#"=SEARCH("a","abc",0)"#), "#VALUE!"); + assert_eq!(eval::(r#"=SEARCH("a","abc",4)"#), "#VALUE!"); +} + +#[test] +fn search_f32() { + assert_eq!(eval::(r#"=SEARCH("m","Miriam")"#), "1"); +} From abe5e14fc504fe32111c199271413157ca66069f Mon Sep 17 00:00:00 2001 From: Binyamin Klein Date: Mon, 9 Feb 2026 09:31:55 +0200 Subject: [PATCH 2/5] Refactor to use CoerceForFind enum for better clarity. Bug fixes --- src/calculate/operation/function.rs | 17 ++++---- src/calculate/operation/string.rs | 61 ++++++++++++++++++++++------- tests/common/mod.rs | 3 +- tests/find.rs | 3 +- tests/iserror.rs | 3 +- tests/search.rs | 4 +- 6 files changed, 61 insertions(+), 30 deletions(-) diff --git a/src/calculate/operation/function.rs b/src/calculate/operation/function.rs index 93be5b8..2265dc1 100644 --- a/src/calculate/operation/function.rs +++ b/src/calculate/operation/function.rs @@ -7,6 +7,7 @@ use super::{ }, string::{ find_position_case_sensitive, search_position_with_wildcards, value_to_string_for_find, + CoerceForFind, }, }; use crate::{ @@ -165,12 +166,12 @@ where return types::Value::Error(e); } let find_s = match value_to_string_for_find(&find_text) { - Ok(s) => s, - Err(v) => return v, + CoerceForFind::Coerced(s) => s, + CoerceForFind::Propagate(v) => return v, }; let within_s = match value_to_string_for_find(&within_text) { - Ok(s) => s, - Err(v) => return v, + CoerceForFind::Coerced(s) => s, + CoerceForFind::Propagate(v) => return v, }; let start_i64: i64 = match &start_num { types::Value::Number(n) => n.as_(), @@ -212,12 +213,12 @@ where return types::Value::Error(e); } let find_s = match value_to_string_for_find(&find_text) { - Ok(s) => s, - Err(v) => return v, + CoerceForFind::Coerced(s) => s, + CoerceForFind::Propagate(v) => return v, }; let within_s = match value_to_string_for_find(&within_text) { - Ok(s) => s, - Err(v) => return v, + CoerceForFind::Coerced(s) => s, + CoerceForFind::Propagate(v) => return v, }; let start_i64: i64 = match &start_num { types::Value::Number(n) => n.as_(), diff --git a/src/calculate/operation/string.rs b/src/calculate/operation/string.rs index 2b282c7..0e7efd5 100644 --- a/src/calculate/operation/string.rs +++ b/src/calculate/operation/string.rs @@ -1,4 +1,4 @@ -use crate::types::{self, Boolean, XlNum}; +use crate::types::{self, XlNum}; use std::{fmt::Debug, str::FromStr}; pub fn calculate_concat_operator(str1: &str, str2: &str) -> String { @@ -57,24 +57,32 @@ where types::Value::Boolean(f(string1, string2).into()) } +/// Result of coercing a formula value to string for FIND/SEARCH (Excel semantics). +#[derive(Debug, Clone)] +pub enum CoerceForFind +where + N: XlNum, +{ + /// Value was coerced to string; use for FIND/SEARCH. + Coerced(String), + /// Propagate this value as the formula result (e.g. existing error or #VALUE! for Date/Iterator). + Propagate(types::Value), +} + /// Coerce a formula value to string for FIND/SEARCH (Excel semantics). -/// Returns Ok(s) or Err(Value::Error) for Date/Iterator. -pub fn value_to_string_for_find(v: &types::Value) -> Result> +pub fn value_to_string_for_find(v: &types::Value) -> CoerceForFind where N: XlNum, ::Err: Debug, { match v { - types::Value::Error(e) => Err(types::Value::Error(*e)), - types::Value::Number(n) => Ok(n.to_string()), - types::Value::Text(s) => Ok(s.clone()), - types::Value::Boolean(b) => Ok(match b { - Boolean::True => "TRUE".to_string(), - Boolean::False => "FALSE".to_string(), - }), - types::Value::Blank => Ok(String::new()), + types::Value::Error(e) => CoerceForFind::Propagate(types::Value::Error(*e)), + types::Value::Number(n) => CoerceForFind::Coerced(n.to_string()), + types::Value::Text(s) => CoerceForFind::Coerced(s.clone()), + types::Value::Boolean(b) => CoerceForFind::Coerced(b.to_string()), + types::Value::Blank => CoerceForFind::Coerced(String::new()), types::Value::Date(_) | types::Value::Iterator(_) => { - Err(types::Value::Error(types::Error::Value)) + CoerceForFind::Propagate(types::Value::Error(types::Error::Value)) } } } @@ -85,9 +93,13 @@ pub fn find_position_case_sensitive( within_text: &str, start_num_1based: i64, ) -> Option { + if start_num_1based < 1 { + return None; + } let start = (start_num_1based - 1) as usize; + // Character count (not byte len) for Excel 1-based character position semantics and UTF-8. let char_count = within_text.chars().count(); - if start_num_1based < 1 || start >= char_count { + if start >= char_count { return None; } if find_text.is_empty() { @@ -106,9 +118,13 @@ pub fn search_position_with_wildcards( within_text: &str, start_num_1based: i64, ) -> Option { + if start_num_1based < 1 { + return None; + } let start = (start_num_1based - 1) as usize; + // Character count (not byte len) for Excel 1-based character position semantics and UTF-8. let char_count = within_text.chars().count(); - if start_num_1based < 1 || start >= char_count { + if start >= char_count { return None; } if find_text.is_empty() { @@ -210,3 +226,20 @@ fn match_pattern( } Some(match_start) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn find_position_rejects_invalid_start() { + assert_eq!(find_position_case_sensitive("a", "abc", 0), None); + assert_eq!(find_position_case_sensitive("a", "abc", -1), None); + } + + #[test] + fn search_position_rejects_invalid_start() { + assert_eq!(search_position_with_wildcards("a", "abc", 0), None); + assert_eq!(search_position_with_wildcards("a", "abc", -1), None); + } +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 93196b9..cbecd7d 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -1,5 +1,4 @@ -use std::fmt::Debug; -use std::str::FromStr; +use std::{fmt::Debug, str::FromStr}; use xlformula_engine::{ calculate, parse_formula, diff --git a/tests/find.rs b/tests/find.rs index 17c0024..a15f049 100644 --- a/tests/find.rs +++ b/tests/find.rs @@ -1,7 +1,6 @@ mod common; -use std::fmt::Debug; -use std::str::FromStr; +use std::{fmt::Debug, str::FromStr}; use xlformula_engine::types::XlNum; fn eval(s: &str) -> String diff --git a/tests/iserror.rs b/tests/iserror.rs index 16b9dc2..d795f3f 100644 --- a/tests/iserror.rs +++ b/tests/iserror.rs @@ -1,7 +1,6 @@ mod common; -use std::fmt::Debug; -use std::str::FromStr; +use std::{fmt::Debug, str::FromStr}; use xlformula_engine::types::{self, XlNum}; fn eval(s: &str) -> String diff --git a/tests/search.rs b/tests/search.rs index 4610bfc..d4c819c 100644 --- a/tests/search.rs +++ b/tests/search.rs @@ -1,7 +1,6 @@ mod common; -use std::fmt::Debug; -use std::str::FromStr; +use std::{fmt::Debug, str::FromStr}; use xlformula_engine::types::XlNum; fn eval(s: &str) -> String @@ -53,6 +52,7 @@ fn search_not_found() { #[test] fn search_start_num_bounds() { assert_eq!(eval::(r#"=SEARCH("a","abc",0)"#), "#VALUE!"); + assert_eq!(eval::(r#"=SEARCH("a","abc",-1)"#), "#VALUE!"); assert_eq!(eval::(r#"=SEARCH("a","abc",4)"#), "#VALUE!"); } From 1375593cd7b22e8650cd79ec9c47aa12fa673019 Mon Sep 17 00:00:00 2001 From: Binyamin Klein Date: Mon, 9 Feb 2026 10:14:03 +0200 Subject: [PATCH 3/5] Remove unnecessary tests from internal string operations --- src/calculate/operation/string.rs | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/calculate/operation/string.rs b/src/calculate/operation/string.rs index 0e7efd5..3b898e9 100644 --- a/src/calculate/operation/string.rs +++ b/src/calculate/operation/string.rs @@ -226,20 +226,3 @@ fn match_pattern( } Some(match_start) } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn find_position_rejects_invalid_start() { - assert_eq!(find_position_case_sensitive("a", "abc", 0), None); - assert_eq!(find_position_case_sensitive("a", "abc", -1), None); - } - - #[test] - fn search_position_rejects_invalid_start() { - assert_eq!(search_position_with_wildcards("a", "abc", 0), None); - assert_eq!(search_position_with_wildcards("a", "abc", -1), None); - } -} From 332f666ccca1364a0dddd0e226c7d3e864dfc6d4 Mon Sep 17 00:00:00 2001 From: Binyamin Klein Date: Mon, 9 Feb 2026 11:40:02 +0200 Subject: [PATCH 4/5] Using try_into() for checked conversion --- src/calculate/operation/string.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/calculate/operation/string.rs b/src/calculate/operation/string.rs index 3b898e9..63f383f 100644 --- a/src/calculate/operation/string.rs +++ b/src/calculate/operation/string.rs @@ -93,10 +93,7 @@ pub fn find_position_case_sensitive( within_text: &str, start_num_1based: i64, ) -> Option { - if start_num_1based < 1 { - return None; - } - let start = (start_num_1based - 1) as usize; + let start: usize = (start_num_1based - 1).try_into().ok()?; // Character count (not byte len) for Excel 1-based character position semantics and UTF-8. let char_count = within_text.chars().count(); if start >= char_count { @@ -118,10 +115,7 @@ pub fn search_position_with_wildcards( within_text: &str, start_num_1based: i64, ) -> Option { - if start_num_1based < 1 { - return None; - } - let start = (start_num_1based - 1) as usize; + let start: usize = (start_num_1based - 1).try_into().ok()?; // Character count (not byte len) for Excel 1-based character position semantics and UTF-8. let char_count = within_text.chars().count(); if start >= char_count { From aa1042dd2bab9b2c5c6b4d120bb39be337781b7e Mon Sep 17 00:00:00 2001 From: Binyamin Klein Date: Tue, 10 Feb 2026 10:28:23 +0200 Subject: [PATCH 5/5] Use checked subtraction for safer index calculations. Remove unwraps. Simplify boolean evaluations --- src/calculate/operation/function.rs | 35 ++++++++++------------------- src/calculate/operation/string.rs | 14 ++++++------ 2 files changed, 19 insertions(+), 30 deletions(-) diff --git a/src/calculate/operation/function.rs b/src/calculate/operation/function.rs index 2265dc1..e937323 100644 --- a/src/calculate/operation/function.rs +++ b/src/calculate/operation/function.rs @@ -15,7 +15,7 @@ use crate::{ get_binary_function_args, get_find_args, get_number_and_string_values, get_ternary_function_args, get_unary_function_arg, }, - types::{self, Boolean, Error, XlNum}, + types::{self, Error, XlNum}, }; use std::{fmt::Debug, str::FromStr}; @@ -146,11 +146,7 @@ where } fn calculate_find( - (find_text, within_text, start_num): ( - types::Value, - types::Value, - types::Value, - ), + (find_text, within_text, start_num): (types::Value, types::Value, types::Value), ) -> types::Value where N: XlNum, @@ -185,19 +181,16 @@ where return types::Value::Error(Error::Value); } match find_position_case_sensitive(&find_s, &within_s, start_i64) { - Some(pos) => types::Value::Number( - N::from_i64(pos).unwrap_or_else(|| N::from_f64(1.0).unwrap()), - ), + Some(pos) => N::from_i64(pos) + .or_else(|| N::from_f64(1.0)) + .map(types::Value::Number) + .unwrap_or_else(|| types::Value::Error(Error::Value)), None => types::Value::Error(Error::Value), } } fn calculate_search( - (find_text, within_text, start_num): ( - types::Value, - types::Value, - types::Value, - ), + (find_text, within_text, start_num): (types::Value, types::Value, types::Value), ) -> types::Value where N: XlNum, @@ -232,9 +225,10 @@ where return types::Value::Error(Error::Value); } match search_position_with_wildcards(&find_s, &within_s, start_i64) { - Some(pos) => types::Value::Number( - N::from_i64(pos).unwrap_or_else(|| N::from_f64(1.0).unwrap()), - ), + Some(pos) => N::from_i64(pos) + .or_else(|| N::from_f64(1.0)) + .map(types::Value::Number) + .unwrap_or_else(|| types::Value::Error(Error::Value)), None => types::Value::Error(Error::Value), } } @@ -243,10 +237,5 @@ fn calculate_iserror(arg: types::Value) -> types::Value where N: XlNum, { - let is_err = matches!(arg, types::Value::Error(_)); - types::Value::Boolean(if is_err { - Boolean::True - } else { - Boolean::False - }) + types::Value::Boolean(matches!(arg, types::Value::Error(_)).into()) } diff --git a/src/calculate/operation/string.rs b/src/calculate/operation/string.rs index 63f383f..2334d4a 100644 --- a/src/calculate/operation/string.rs +++ b/src/calculate/operation/string.rs @@ -93,7 +93,7 @@ pub fn find_position_case_sensitive( within_text: &str, start_num_1based: i64, ) -> Option { - let start: usize = (start_num_1based - 1).try_into().ok()?; + let start: usize = start_num_1based.checked_sub(1)?.try_into().ok()?; // Character count (not byte len) for Excel 1-based character position semantics and UTF-8. let char_count = within_text.chars().count(); if start >= char_count { @@ -115,7 +115,7 @@ pub fn search_position_with_wildcards( within_text: &str, start_num_1based: i64, ) -> Option { - let start: usize = (start_num_1based - 1).try_into().ok()?; + let start: usize = start_num_1based.checked_sub(1)?.try_into().ok()?; // Character count (not byte len) for Excel 1-based character position semantics and UTF-8. let char_count = within_text.chars().count(); if start >= char_count { @@ -192,12 +192,12 @@ fn match_pattern( t += 1; } '*' => { - let rest_pat_lower: Vec = pattern_lower[p + 1..].to_vec(); - let rest_pat_orig: Vec = pattern_orig[p + 1..].to_vec(); - for skip in 0..=(text_lower.len() - t) { + let rest_pat_lower = &pattern_lower[p + 1..]; + let rest_pat_orig = &pattern_orig[p + 1..]; + for skip in 0..=text_lower.len().saturating_sub(t) { if let Some(off) = match_pattern( - &rest_pat_lower, - &rest_pat_orig, + rest_pat_lower, + rest_pat_orig, text_lower, text_orig, t + skip,