From 25c2ca16bbbc4113b786b62152b36b3dca410c1b Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 21 Dec 2025 06:29:04 +0000 Subject: [PATCH] Refactor call_bound_method to use dispatch pattern Refactored the monolithic `call_bound_method` in `eldritch-core` to use a dispatch pattern with specific handlers for each type (`handle_list_methods`, `handle_dict_methods`, etc.). Introduced an `ArgCheck` trait to standardize and DRY up argument validation. Standardized error messages to match Python-style exceptions (TypeError, ValueError, etc.) and updated relevant tests. Preserved deadlock safety logic for List `extend`. Improved maintainability and readability of the method dispatch logic. --- .../eldritch-core/src/interpreter/methods.rs | 563 ++++++++++-------- .../eldritch-core/tests/method_coverage.rs | 78 +-- 2 files changed, 346 insertions(+), 295 deletions(-) diff --git a/implants/lib/eldritchv2/eldritch-core/src/interpreter/methods.rs b/implants/lib/eldritchv2/eldritch-core/src/interpreter/methods.rs index 0b0ac6d3f..1fc0e8aa4 100644 --- a/implants/lib/eldritchv2/eldritch-core/src/interpreter/methods.rs +++ b/implants/lib/eldritchv2/eldritch-core/src/interpreter/methods.rs @@ -1,7 +1,7 @@ use super::super::ast::Value; use super::introspection::{find_best_match, get_type_name, is_truthy}; use super::operations::{compare_values, values_equal}; -use alloc::collections::BTreeSet; +use alloc::collections::{BTreeMap, BTreeSet}; use alloc::format; use alloc::string::{String, ToString}; use alloc::sync::Arc; @@ -10,6 +10,37 @@ use alloc::vec::Vec; use core::cmp::Ordering; use spin::RwLock; +// --- Argument Validation Helper --- + +trait ArgCheck { + fn require(&self, count: usize, name: &str) -> Result<(), String>; + fn require_range(&self, min: usize, max: usize, name: &str) -> Result<(), String>; +} + +impl ArgCheck for [Value] { + fn require(&self, count: usize, name: &str) -> Result<(), String> { + if self.len() != count { + return Err(format!( + "TypeError: {}() takes exactly {} argument{}", + name, + count, + if count != 1 { "s" } else { "" } + )); + } + Ok(()) + } + + fn require_range(&self, min: usize, max: usize, name: &str) -> Result<(), String> { + if self.len() < min || self.len() > max { + return Err(format!( + "TypeError: {}() takes between {} and {} arguments", + name, min, max + )); + } + Ok(()) + } +} + pub fn get_native_methods(value: &Value) -> Vec { match value { Value::List(_) => vec![ @@ -92,50 +123,50 @@ fn get_set_elements(v: &Value) -> Result, String> { Value::Tuple(t) => Ok(t.iter().cloned().collect()), Value::Dictionary(d) => Ok(d.read().keys().cloned().collect()), Value::String(s) => Ok(s.chars().map(|c| Value::String(c.to_string())).collect()), - _ => Err(format!("'{}' object is not iterable", get_type_name(v))), + _ => Err(format!( + "TypeError: '{}' object is not iterable", + get_type_name(v) + )), } } -pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Result { - match (receiver, method) { - (Value::List(l), "append") => { - if args.len() != 1 { - return Err("append() takes exactly one argument".into()); - } +// --- Specific Handlers --- + +fn handle_list_methods( + l: &Arc>>, + method: &str, + args: &[Value], +) -> Option> { + match method { + "append" => Some((|| { + args.require(1, "append")?; l.write().push(args[0].clone()); Ok(Value::None) - } - (Value::List(l), "extend") => { - if args.len() != 1 { - return Err("extend() takes exactly one argument".into()); - } + })()), + "extend" => Some((|| { + args.require(1, "extend")?; let iterable = &args[0]; match iterable { Value::List(other) => { // DEADLOCK FIX: Read other first, then write l. - // If l and other are the same list, l.write().extend(other.read().clone()) - // would acquire the write lock on l, then try to acquire the read lock on other - // (which is l), causing a deadlock. let items = other.read().clone(); l.write().extend(items); } Value::Tuple(other) => l.write().extend(other.clone()), _ => { return Err(format!( - "extend() expects an iterable, got {}", + "TypeError: extend() expects an iterable, got {}", get_type_name(iterable) - )); + )) } } Ok(Value::None) - } - (Value::List(l), "insert") => { - if args.len() != 2 { - return Err("insert() takes exactly two arguments".into()); - } + })()), + "insert" => Some((|| { + args.require(2, "insert")?; let idx = match args[0] { Value::Int(i) => i, - _ => return Err("insert() index must be an integer".into()), + _ => return Err("TypeError: insert() index must be an integer".into()), }; let val = args[1].clone(); let mut vec = l.write(); @@ -147,11 +178,9 @@ pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Resu }; vec.insert(index, val); Ok(Value::None) - } - (Value::List(l), "remove") => { - if args.len() != 1 { - return Err("remove() takes exactly one argument".into()); - } + })()), + "remove" => Some((|| { + args.require(1, "remove")?; let target = &args[0]; let mut vec = l.write(); if let Some(pos) = vec.iter().position(|x| values_equal(x, target)) { @@ -160,11 +189,9 @@ pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Resu } else { Err("ValueError: list.remove(x): x not in list".into()) } - } - (Value::List(l), "index") => { - if args.len() != 1 { - return Err("index() takes exactly one argument".into()); - } + })()), + "index" => Some((|| { + args.require(1, "index")?; let target = &args[0]; let vec = l.read(); if let Some(pos) = vec.iter().position(|x| values_equal(x, target)) { @@ -172,40 +199,52 @@ pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Resu } else { Err("ValueError: list.index(x): x not in list".into()) } - } - (Value::List(l), "pop") => { + })()), + "pop" => Some((|| { + args.require(0, "pop")?; if let Some(v) = l.write().pop() { Ok(v) } else { - Err("pop from empty list".into()) + Err("IndexError: pop from empty list".into()) } - } - (Value::List(l), "sort") => { + })()), + "sort" => Some((|| { + args.require(0, "sort")?; let mut vec = l.write(); vec.sort_by(|a, b| compare_values(a, b).unwrap_or(Ordering::Equal)); Ok(Value::None) - } + })()), + _ => None, + } +} - (Value::Dictionary(d), "keys") => { +fn handle_dict_methods( + d: &Arc>>, + method: &str, + args: &[Value], +) -> Option> { + match method { + "keys" => Some((|| { + args.require(0, "keys")?; let keys: Vec = d.read().keys().cloned().collect(); Ok(Value::List(Arc::new(RwLock::new(keys)))) - } - (Value::Dictionary(d), "values") => { + })()), + "values" => Some((|| { + args.require(0, "values")?; let values: Vec = d.read().values().cloned().collect(); Ok(Value::List(Arc::new(RwLock::new(values)))) - } - (Value::Dictionary(d), "items") => { + })()), + "items" => Some((|| { + args.require(0, "items")?; let items: Vec = d .read() .iter() .map(|(k, v)| Value::Tuple(vec![k.clone(), v.clone()])) .collect(); Ok(Value::List(Arc::new(RwLock::new(items)))) - } - (Value::Dictionary(d), "get") => { - if args.is_empty() || args.len() > 2 { - return Err("get() takes 1 or 2 arguments".into()); - } + })()), + "get" => Some((|| { + args.require_range(1, 2, "get")?; let key = &args[0]; let default = if args.len() == 2 { args[1].clone() @@ -216,165 +255,155 @@ pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Resu Some(v) => Ok(v.clone()), None => Ok(default), } - } - (Value::Dictionary(d), "update") => { - if args.len() != 1 { - return Err("update() takes exactly one argument".into()); - } + })()), + "update" => Some((|| { + args.require(1, "update")?; match &args[0] { Value::Dictionary(other) => { let other_map = other.read().clone(); d.write().extend(other_map); Ok(Value::None) } - _ => Err("update() requires a dictionary".into()), + _ => Err("TypeError: update() requires a dictionary".into()), } - } - (Value::Dictionary(d), "popitem") => { + })()), + "popitem" => Some((|| { + args.require(0, "popitem")?; let mut map = d.write(); let last_key = map.keys().next_back().cloned(); if let Some(k) = last_key { let v = map.remove(&k).unwrap(); Ok(Value::Tuple(vec![k, v])) } else { - Err("popitem(): dictionary is empty".into()) + Err("KeyError: popitem(): dictionary is empty".into()) } - } + })()), + _ => None, + } +} - (Value::Set(s), "add") => { - if args.len() != 1 { - return Err("add() takes exactly one argument".into()); - } +fn handle_set_methods( + s: &Arc>>, + method: &str, + args: &[Value], +) -> Option> { + match method { + "add" => Some((|| { + args.require(1, "add")?; s.write().insert(args[0].clone()); Ok(Value::None) - } - (Value::Set(s), "clear") => { + })()), + "clear" => Some((|| { + args.require(0, "clear")?; s.write().clear(); Ok(Value::None) - } - (Value::Set(s), "contains") => { - if args.len() != 1 { - return Err("contains() takes exactly one argument".into()); - } + })()), + "contains" => Some((|| { + args.require(1, "contains")?; Ok(Value::Bool(s.read().contains(&args[0]))) - } - (Value::Set(s), "difference") => { - if args.len() != 1 { - return Err("difference() takes exactly one argument".into()); - } + })()), + "difference" => Some((|| { + args.require(1, "difference")?; let other_set = get_set_elements(&args[0])?; let diff: BTreeSet = s.read().difference(&other_set).cloned().collect(); Ok(Value::Set(Arc::new(RwLock::new(diff)))) - } - (Value::Set(s), "discard") => { - if args.len() != 1 { - return Err("discard() takes exactly one argument".into()); - } + })()), + "discard" => Some((|| { + args.require(1, "discard")?; s.write().remove(&args[0]); Ok(Value::None) - } - (Value::Set(s), "intersection") => { - if args.len() != 1 { - return Err("intersection() takes exactly one argument".into()); - } + })()), + "intersection" => Some((|| { + args.require(1, "intersection")?; let other_set = get_set_elements(&args[0])?; let inter: BTreeSet = s.read().intersection(&other_set).cloned().collect(); Ok(Value::Set(Arc::new(RwLock::new(inter)))) - } - (Value::Set(s), "isdisjoint") => { - if args.len() != 1 { - return Err("isdisjoint() takes exactly one argument".into()); - } + })()), + "isdisjoint" => Some((|| { + args.require(1, "isdisjoint")?; let other_set = get_set_elements(&args[0])?; Ok(Value::Bool(s.read().is_disjoint(&other_set))) - } - (Value::Set(s), "issubset") => { - if args.len() != 1 { - return Err("issubset() takes exactly one argument".into()); - } + })()), + "issubset" => Some((|| { + args.require(1, "issubset")?; let other_set = get_set_elements(&args[0])?; Ok(Value::Bool(s.read().is_subset(&other_set))) - } - (Value::Set(s), "issuperset") => { - if args.len() != 1 { - return Err("issuperset() takes exactly one argument".into()); - } + })()), + "issuperset" => Some((|| { + args.require(1, "issuperset")?; let other_set = get_set_elements(&args[0])?; Ok(Value::Bool(s.read().is_superset(&other_set))) - } - (Value::Set(s), "pop") => { - // Remove the LAST element, per user request (and consistent with list.pop()) - // BTreeSet is ordered, so this removes the largest element. + })()), + "pop" => Some((|| { + args.require(0, "pop")?; let mut set = s.write(); if set.is_empty() { - return Err("pop from empty set".into()); + return Err("KeyError: pop from empty set".into()); } - // Use iterator to get last element without nightly features let last = set.iter().next_back().cloned(); if let Some(v) = last { set.remove(&v); Ok(v) } else { - Err("pop from empty set".into()) - } - } - (Value::Set(s), "remove") => { - if args.len() != 1 { - return Err("remove() takes exactly one argument".into()); + Err("KeyError: pop from empty set".into()) } + })()), + "remove" => Some((|| { + args.require(1, "remove")?; if !s.write().remove(&args[0]) { return Err(format!("KeyError: {}", args[0])); } Ok(Value::None) - } - (Value::Set(s), "symmetric_difference") => { - if args.len() != 1 { - return Err("symmetric_difference() takes exactly one argument".into()); - } + })()), + "symmetric_difference" => Some((|| { + args.require(1, "symmetric_difference")?; let other_set = get_set_elements(&args[0])?; - let sym: BTreeSet = s.read().symmetric_difference(&other_set).cloned().collect(); + let sym: BTreeSet = + s.read().symmetric_difference(&other_set).cloned().collect(); Ok(Value::Set(Arc::new(RwLock::new(sym)))) - } - (Value::Set(s), "union") => { - if args.len() != 1 { - return Err("union() takes exactly one argument".into()); - } + })()), + "union" => Some((|| { + args.require(1, "union")?; let other_set = get_set_elements(&args[0])?; let u: BTreeSet = s.read().union(&other_set).cloned().collect(); Ok(Value::Set(Arc::new(RwLock::new(u)))) - } - (Value::Set(s), "update") => { - if args.len() != 1 { - return Err("update() takes exactly one argument".into()); - } + })()), + "update" => Some((|| { + args.require(1, "update")?; let other_set = get_set_elements(&args[0])?; s.write().extend(other_set); Ok(Value::None) - } + })()), + _ => None, + } +} - (Value::String(s), "split") => { +fn handle_string_methods( + s: &str, + method: &str, + args: &[Value], +) -> Option> { + match method { + "split" => Some((|| { let parts: Vec = if args.is_empty() { - // Default split: split by whitespace (runs of whitespace are one separator) s.split_whitespace() .map(|p| Value::String(p.to_string())) .collect() } else { - // Split by specific delimiter let delim = args[0].to_string(); s.split(&delim) .map(|p| Value::String(p.to_string())) .collect() }; Ok(Value::List(Arc::new(RwLock::new(parts)))) - } - (Value::String(s), "splitlines") => { + })()), + "splitlines" => Some((|| { let keepends = if !args.is_empty() { is_truthy(&args[0]) } else { false }; let lines: Vec = if keepends { - // Not perfectly matching python's splitlines(keepends=True) split behavior on all boundaries, but roughly s.split_inclusive('\n') .map(|p| Value::String(p.to_string())) .collect() @@ -382,13 +411,29 @@ pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Resu s.lines().map(|p| Value::String(p.to_string())).collect() }; Ok(Value::List(Arc::new(RwLock::new(lines)))) - } - (Value::String(s), "strip") => Ok(Value::String(s.trim().to_string())), - (Value::String(s), "lstrip") => Ok(Value::String(s.trim_start().to_string())), - (Value::String(s), "rstrip") => Ok(Value::String(s.trim_end().to_string())), - (Value::String(s), "lower") => Ok(Value::String(s.to_lowercase())), - (Value::String(s), "upper") => Ok(Value::String(s.to_uppercase())), - (Value::String(s), "capitalize") => { + })()), + "strip" => Some((|| { + args.require(0, "strip")?; + Ok(Value::String(s.trim().to_string())) + })()), + "lstrip" => Some((|| { + args.require(0, "lstrip")?; + Ok(Value::String(s.trim_start().to_string())) + })()), + "rstrip" => Some((|| { + args.require(0, "rstrip")?; + Ok(Value::String(s.trim_end().to_string())) + })()), + "lower" => Some((|| { + args.require(0, "lower")?; + Ok(Value::String(s.to_lowercase())) + })()), + "upper" => Some((|| { + args.require(0, "upper")?; + Ok(Value::String(s.to_uppercase())) + })()), + "capitalize" => Some((|| { + args.require(0, "capitalize")?; let mut c = s.chars(); match c.next() { None => Ok(Value::String(String::new())), @@ -397,10 +442,9 @@ pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Resu Ok(Value::String(res)) } } - } - (Value::String(s), "title") => { - // Simplified title case: capitalize first letter of each word - // We removed the unused _res variable + })()), + "title" => Some((|| { + args.require(0, "title")?; let mut result = String::new(); let mut cap_next = true; for c in s.chars() { @@ -417,106 +461,84 @@ pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Resu } } Ok(Value::String(result)) - } - (Value::String(s), "startswith") => { - if args.len() != 1 { - return Err("startswith() takes 1 argument".into()); - } + })()), + "startswith" => Some((|| { + args.require(1, "startswith")?; let prefix = args[0].to_string(); Ok(Value::Bool(s.starts_with(&prefix))) - } - (Value::String(s), "endswith") => { - if args.len() != 1 { - return Err("endswith() takes 1 argument".into()); - } + })()), + "endswith" => Some((|| { + args.require(1, "endswith")?; let suffix = args[0].to_string(); Ok(Value::Bool(s.ends_with(&suffix))) - } - (Value::String(s), "removeprefix") => { - if args.len() != 1 { - return Err("removeprefix() takes 1 argument".into()); - } + })()), + "removeprefix" => Some((|| { + args.require(1, "removeprefix")?; let prefix = args[0].to_string(); if s.starts_with(&prefix) { Ok(Value::String(s[prefix.len()..].to_string())) } else { - Ok(Value::String(s.clone())) - } - } - (Value::String(s), "removesuffix") => { - if args.len() != 1 { - return Err("removesuffix() takes 1 argument".into()); + Ok(Value::String(s.to_string())) } + })()), + "removesuffix" => Some((|| { + args.require(1, "removesuffix")?; let suffix = args[0].to_string(); if s.ends_with(&suffix) { Ok(Value::String(s[..s.len() - suffix.len()].to_string())) } else { - Ok(Value::String(s.clone())) - } - } - (Value::String(s), "find") => { - if args.len() != 1 { - return Err("find() takes 1 argument".into()); + Ok(Value::String(s.to_string())) } + })()), + "find" => Some((|| { + args.require(1, "find")?; let sub = args[0].to_string(); match s.find(&sub) { Some(idx) => Ok(Value::Int(idx as i64)), None => Ok(Value::Int(-1)), } - } - (Value::String(s), "index") => { - if args.len() != 1 { - return Err("index() takes 1 argument".into()); - } + })()), + "index" => Some((|| { + args.require(1, "index")?; let sub = args[0].to_string(); match s.find(&sub) { Some(idx) => Ok(Value::Int(idx as i64)), None => Err("ValueError: substring not found".into()), } - } - (Value::String(s), "rfind") => { - if args.len() != 1 { - return Err("rfind() takes 1 argument".into()); - } + })()), + "rfind" => Some((|| { + args.require(1, "rfind")?; let sub = args[0].to_string(); match s.rfind(&sub) { Some(idx) => Ok(Value::Int(idx as i64)), None => Ok(Value::Int(-1)), } - } - (Value::String(s), "rindex") => { - if args.len() != 1 { - return Err("rindex() takes 1 argument".into()); - } + })()), + "rindex" => Some((|| { + args.require(1, "rindex")?; let sub = args[0].to_string(); match s.rfind(&sub) { Some(idx) => Ok(Value::Int(idx as i64)), None => Err("ValueError: substring not found".into()), } - } - (Value::String(s), "count") => { - if args.len() != 1 { - return Err("count() takes 1 argument".into()); - } + })()), + "count" => Some((|| { + args.require(1, "count")?; let sub = args[0].to_string(); if sub.is_empty() { return Ok(Value::Int((s.len() + 1) as i64)); } let count = s.matches(&sub).count(); Ok(Value::Int(count as i64)) - } - (Value::String(s), "replace") => { - if args.len() != 2 { - return Err("replace() takes 2 arguments".into()); - } + })()), + "replace" => Some((|| { + args.require(2, "replace")?; let old = args[0].to_string(); let new = args[1].to_string(); Ok(Value::String(s.replace(&old, &new))) - } - (Value::String(s), "join") => { - if args.len() != 1 { - return Err("join() takes 1 argument".into()); - } + })()), + "join" => Some((|| { + args.require(1, "join")?; match &args[0] { Value::List(l) => { let list = l.read(); @@ -524,15 +546,15 @@ pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Resu .iter() .map(|v| match v { Value::String(ss) => Ok(ss.clone()), - _ => Err("join() expects list of strings".to_string()), + _ => Err("TypeError: join() expects list of strings".to_string()), }) .collect(); Ok(Value::String(strs?.join(s))) } - _ => Err("join() expects a list".into()), + _ => Err("TypeError: join() expects a list".into()), } - } - (Value::String(s), "format") => { + })()), + "format" => Some((|| { let mut result = String::new(); let mut arg_idx = 0; let chars: Vec = s.chars().collect(); @@ -540,7 +562,7 @@ pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Resu while i < chars.len() { if chars[i] == '{' && i + 1 < chars.len() && chars[i + 1] == '}' { if arg_idx >= args.len() { - return Err("tuple index out of range".into()); + return Err("IndexError: tuple index out of range".into()); } result.push_str(&args[arg_idx].to_string()); arg_idx += 1; @@ -551,14 +573,12 @@ pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Resu } } Ok(Value::String(result)) - } - (Value::String(s), "partition") => { - if args.len() != 1 { - return Err("partition() takes 1 argument".into()); - } + })()), + "partition" => Some((|| { + args.require(1, "partition")?; let sep = args[0].to_string(); if let Some(idx) = s.find(&sep) { - let sep_len = sep.len(); // Clone logic handled by creating strings below + let sep_len = sep.len(); Ok(Value::Tuple(vec![ Value::String(s[..idx].to_string()), Value::String(sep), @@ -566,16 +586,14 @@ pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Resu ])) } else { Ok(Value::Tuple(vec![ - Value::String(s.clone()), + Value::String(s.to_string()), Value::String("".to_string()), Value::String("".to_string()), ])) } - } - (Value::String(s), "rpartition") => { - if args.len() != 1 { - return Err("rpartition() takes 1 argument".into()); - } + })()), + "rpartition" => Some((|| { + args.require(1, "rpartition")?; let sep = args[0].to_string(); if let Some(idx) = s.rfind(&sep) { let sep_len = sep.len(); @@ -588,57 +606,75 @@ pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Resu Ok(Value::Tuple(vec![ Value::String("".to_string()), Value::String("".to_string()), - Value::String(s.clone()), + Value::String(s.to_string()), ])) } - } - (Value::String(s), "rsplit") => { + })()), + "rsplit" => Some((|| { let delim = if !args.is_empty() { args[0].to_string() } else { " ".to_string() }; - // Note: Rust's rsplit is an iterator that yields from end to start. - // Python's rsplit returns list in forward order. let mut parts: Vec = s .rsplit(&delim) .map(|p| Value::String(p.to_string())) .collect(); parts.reverse(); Ok(Value::List(Arc::new(RwLock::new(parts)))) - } - (Value::String(s), "codepoints") => { + })()), + "codepoints" => Some((|| { + args.require(0, "codepoints")?; let points: Vec = s.chars().map(|c| Value::Int(c as i64)).collect(); Ok(Value::List(Arc::new(RwLock::new(points)))) - } - (Value::String(s), "elems") => { + })()), + "elems" => Some((|| { + args.require(0, "elems")?; let chars: Vec = s.chars().map(|c| Value::String(c.to_string())).collect(); Ok(Value::List(Arc::new(RwLock::new(chars)))) - } - (Value::String(s), "isalnum") => Ok(Value::Bool( - !s.is_empty() && s.chars().all(|c| c.is_alphanumeric()), - )), - (Value::String(s), "isalpha") => Ok(Value::Bool( - !s.is_empty() && s.chars().all(|c| c.is_alphabetic()), - )), - (Value::String(s), "isdigit") => Ok(Value::Bool( - !s.is_empty() && s.chars().all(|c| c.is_ascii_digit()), - )), // Python isdigit is unicode digits, but ascii is safer bet for now - (Value::String(s), "islower") => Ok(Value::Bool( - !s.is_empty() && s.chars().any(|c| c.is_alphabetic()) && s == &s.to_lowercase(), - )), - (Value::String(s), "isupper") => Ok(Value::Bool( - !s.is_empty() && s.chars().any(|c| c.is_alphabetic()) && s == &s.to_uppercase(), - )), - (Value::String(s), "isspace") => Ok(Value::Bool( - !s.is_empty() && s.chars().all(|c| c.is_whitespace()), - )), - (Value::String(s), "istitle") => { + })()), + "isalnum" => Some((|| { + args.require(0, "isalnum")?; + Ok(Value::Bool( + !s.is_empty() && s.chars().all(|c| c.is_alphanumeric()), + )) + })()), + "isalpha" => Some((|| { + args.require(0, "isalpha")?; + Ok(Value::Bool( + !s.is_empty() && s.chars().all(|c| c.is_alphabetic()), + )) + })()), + "isdigit" => Some((|| { + args.require(0, "isdigit")?; + Ok(Value::Bool( + !s.is_empty() && s.chars().all(|c| c.is_ascii_digit()), + )) + })()), + "islower" => Some((|| { + args.require(0, "islower")?; + Ok(Value::Bool( + !s.is_empty() && s.chars().any(|c| c.is_alphabetic()) && s == &s.to_lowercase(), + )) + })()), + "isupper" => Some((|| { + args.require(0, "isupper")?; + Ok(Value::Bool( + !s.is_empty() && s.chars().any(|c| c.is_alphabetic()) && s == &s.to_uppercase(), + )) + })()), + "isspace" => Some((|| { + args.require(0, "isspace")?; + Ok(Value::Bool( + !s.is_empty() && s.chars().all(|c| c.is_whitespace()), + )) + })()), + "istitle" => Some((|| { + args.require(0, "istitle")?; if s.is_empty() { return Ok(Value::Bool(false)); } let mut cased = false; - let mut _prev_cased = false; let mut expected_upper = true; for c in s.chars() { if c.is_uppercase() { @@ -647,22 +683,37 @@ pub fn call_bound_method(receiver: &Value, method: &str, args: &[Value]) -> Resu } expected_upper = false; cased = true; - _prev_cased = true; } else if c.is_lowercase() { if expected_upper { return Ok(Value::Bool(false)); } cased = true; - _prev_cased = true; } else { expected_upper = true; - _prev_cased = false; } } Ok(Value::Bool(cased)) - } + })()), + _ => None, + } +} + +pub fn call_bound_method( + receiver: &Value, + method: &str, + args: &[Value], +) -> Result { + let result = match receiver { + Value::List(l) => handle_list_methods(l, method, args), + Value::Dictionary(d) => handle_dict_methods(d, method, args), + Value::Set(s) => handle_set_methods(s, method, args), + Value::String(s) => handle_string_methods(s, method, args), + _ => None, + }; - _ => { + match result { + Some(res) => res, + None => { let mut msg = format!( "Object of type '{}' has no method '{}'", get_type_name(receiver), diff --git a/implants/lib/eldritchv2/eldritch-core/tests/method_coverage.rs b/implants/lib/eldritchv2/eldritch-core/tests/method_coverage.rs index 54dc552e6..deaf47a38 100644 --- a/implants/lib/eldritchv2/eldritch-core/tests/method_coverage.rs +++ b/implants/lib/eldritchv2/eldritch-core/tests/method_coverage.rs @@ -21,12 +21,12 @@ fn test_list_edge_cases() { "l=[1]; l.insert('a', 1)", "insert() index must be an integer", ); - assert::fail("l=[1]; l.insert()", "insert() takes exactly two arguments"); - assert::fail("l=[1]; l.append()", "append() takes exactly one argument"); - assert::fail("l=[1]; l.extend()", "extend() takes exactly one argument"); - assert::fail("l=[1]; l.extend(1)", "extend() expects an iterable"); - assert::fail("l=[1]; l.remove()", "remove() takes exactly one argument"); - assert::fail("l=[1]; l.index()", "index() takes exactly one argument"); + assert::fail("l=[1]; l.insert()", "TypeError: insert() takes exactly 2 arguments"); + assert::fail("l=[1]; l.append()", "TypeError: append() takes exactly 1 argument"); + assert::fail("l=[1]; l.extend()", "TypeError: extend() takes exactly 1 argument"); + assert::fail("l=[1]; l.extend(1)", "TypeError: extend() expects an iterable"); + assert::fail("l=[1]; l.remove()", "TypeError: remove() takes exactly 1 argument"); + assert::fail("l=[1]; l.index()", "TypeError: index() takes exactly 1 argument"); } #[test] @@ -42,50 +42,50 @@ fn test_dict_edge_cases() { ); // Errors - assert::fail("d={}; d.get()", "get() takes 1 or 2 arguments"); - assert::fail("d={}; d.get('a', 'b', 'c')", "get() takes 1 or 2 arguments"); - assert::fail("d={}; d.update()", "update() takes exactly one argument"); - assert::fail("d={}; d.update(1)", "update() requires a dictionary"); + assert::fail("d={}; d.get()", "TypeError: get() takes between 1 and 2 arguments"); + assert::fail("d={}; d.get('a', 'b', 'c')", "TypeError: get() takes between 1 and 2 arguments"); + assert::fail("d={}; d.update()", "TypeError: update() takes exactly 1 argument"); + assert::fail("d={}; d.update(1)", "TypeError: update() requires a dictionary"); } #[test] fn test_set_edge_cases() { // Errors - assert::fail("s={1}; s.add()", "add() takes exactly one argument"); + assert::fail("s={1}; s.add()", "TypeError: add() takes exactly 1 argument"); assert::fail( "s={1}; s.contains()", - "contains() takes exactly one argument", + "TypeError: contains() takes exactly 1 argument", ); assert::fail( "s={1}; s.difference()", - "difference() takes exactly one argument", + "TypeError: difference() takes exactly 1 argument", ); - assert::fail("s={1}; s.difference(1)", "is not iterable"); - assert::fail("s={1}; s.discard()", "discard() takes exactly one argument"); + assert::fail("s={1}; s.difference(1)", "TypeError: 'int' object is not iterable"); + assert::fail("s={1}; s.discard()", "TypeError: discard() takes exactly 1 argument"); assert::fail( "s={1}; s.intersection()", - "intersection() takes exactly one argument", + "TypeError: intersection() takes exactly 1 argument", ); assert::fail( "s={1}; s.isdisjoint()", - "isdisjoint() takes exactly one argument", + "TypeError: isdisjoint() takes exactly 1 argument", ); assert::fail( "s={1}; s.issubset()", - "issubset() takes exactly one argument", + "TypeError: issubset() takes exactly 1 argument", ); assert::fail( "s={1}; s.issuperset()", - "issuperset() takes exactly one argument", + "TypeError: issuperset() takes exactly 1 argument", ); - assert::fail("s={1}; s.remove()", "remove() takes exactly one argument"); + assert::fail("s={1}; s.remove()", "TypeError: remove() takes exactly 1 argument"); assert::fail("s={1}; s.remove(99)", "KeyError"); assert::fail( "s={1}; s.symmetric_difference()", - "symmetric_difference() takes exactly one argument", + "TypeError: symmetric_difference() takes exactly 1 argument", ); - assert::fail("s={1}; s.union()", "union() takes exactly one argument"); - assert::fail("s={1}; s.update()", "update() takes exactly one argument"); + assert::fail("s={1}; s.union()", "TypeError: union() takes exactly 1 argument"); + assert::fail("s={1}; s.update()", "TypeError: update() takes exactly 1 argument"); } #[test] @@ -102,24 +102,24 @@ fn test_string_edge_cases() { ); // Errors - assert::fail("''.startswith()", "startswith() takes 1 argument"); - assert::fail("''.endswith()", "endswith() takes 1 argument"); - assert::fail("''.removeprefix()", "removeprefix() takes 1 argument"); - assert::fail("''.removesuffix()", "removesuffix() takes 1 argument"); - assert::fail("''.find()", "find() takes 1 argument"); - assert::fail("''.index()", "index() takes 1 argument"); + assert::fail("''.startswith()", "TypeError: startswith() takes exactly 1 argument"); + assert::fail("''.endswith()", "TypeError: endswith() takes exactly 1 argument"); + assert::fail("''.removeprefix()", "TypeError: removeprefix() takes exactly 1 argument"); + assert::fail("''.removesuffix()", "TypeError: removesuffix() takes exactly 1 argument"); + assert::fail("''.find()", "TypeError: find() takes exactly 1 argument"); + assert::fail("''.index()", "TypeError: index() takes exactly 1 argument"); assert::fail("''.index('z')", "ValueError: substring not found"); - assert::fail("''.rfind()", "rfind() takes 1 argument"); - assert::fail("''.rindex()", "rindex() takes 1 argument"); + assert::fail("''.rfind()", "TypeError: rfind() takes exactly 1 argument"); + assert::fail("''.rindex()", "TypeError: rindex() takes exactly 1 argument"); assert::fail("''.rindex('z')", "ValueError: substring not found"); - assert::fail("''.count()", "count() takes 1 argument"); - assert::fail("''.replace('a')", "replace() takes 2 arguments"); - assert::fail("''.join()", "join() takes 1 argument"); - assert::fail("''.join(1)", "join() expects a list"); - assert::fail("''.join([1])", "join() expects list of strings"); - assert::fail("''.partition()", "partition() takes 1 argument"); - assert::fail("''.rpartition()", "rpartition() takes 1 argument"); - assert::fail("'{}'.format()", "tuple index out of range"); + assert::fail("''.count()", "TypeError: count() takes exactly 1 argument"); + assert::fail("''.replace('a')", "TypeError: replace() takes exactly 2 arguments"); + assert::fail("''.join()", "TypeError: join() takes exactly 1 argument"); + assert::fail("''.join(1)", "TypeError: join() expects a list"); + assert::fail("''.join([1])", "TypeError: join() expects list of strings"); + assert::fail("''.partition()", "TypeError: partition() takes exactly 1 argument"); + assert::fail("''.rpartition()", "TypeError: rpartition() takes exactly 1 argument"); + assert::fail("'{}'.format()", "IndexError: tuple index out of range"); // Coverage for format logic assert::pass(