-
Notifications
You must be signed in to change notification settings - Fork 232
feat: Add the pipeline operator |> for chained method calls. #1061
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,3 +13,4 @@ doc/rhai.json | |
| .idea | ||
| .idea/* | ||
| src/eval/chaining.rs | ||
| *.core | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2329,7 +2329,7 @@ | |
| namespace: crate::ast::Namespace::NONE, | ||
| name: self.get_interned_string(&op), | ||
| hashes: FnCallHashes::from_native_only(hash), | ||
| args: IntoIterator::into_iter([root, rhs]).collect(), | ||
| args: IntoIterator::into_iter([root.clone(), rhs.clone()]).collect(), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also is it necessary to clone the arguments? |
||
| op_token: native_only.then(|| op_token.clone()), | ||
| capture_parent_scope: false, | ||
| }; | ||
|
|
@@ -2430,6 +2430,129 @@ | |
| op_base.into_fn_call_expr(pos) | ||
| } | ||
|
|
||
| Token::PipeArrow => { | ||
| // Pipeline: lhs |> fn(args...) => fn(lhs, args...) | ||
| match rhs { | ||
| Expr::FnCall(f, func_pos) => { | ||
| // take inner FnCallExpr | ||
| let mut f = *f; | ||
|
|
||
| let args_len = f.args.len() + 1; | ||
| f.args.insert(0, root); | ||
|
|
||
| // Recalculate hash for the new argument count, preserving namespace if any | ||
| #[cfg(not(feature = "no_module"))] | ||
| { | ||
| let hash = if f.namespace.is_empty() { | ||
| calc_fn_hash(None, &f.name, args_len) | ||
| } else { | ||
| calc_fn_hash( | ||
| f.namespace.path.iter().map(Ident::as_str), | ||
| &f.name, | ||
| args_len, | ||
| ) | ||
| }; | ||
| f.hashes = if is_valid_function_name(&f.name) { | ||
| FnCallHashes::from_hash(hash) | ||
| } else { | ||
| FnCallHashes::from_native_only(hash) | ||
| }; | ||
| } | ||
| #[cfg(feature = "no_module")] | ||
| { | ||
| f.hashes = if is_valid_function_name(&f.name) { | ||
| FnCallHashes::from_hash(calc_fn_hash(None, &f.name, args_len)) | ||
| } else { | ||
| FnCallHashes::from_native_only(calc_fn_hash( | ||
| None, &f.name, args_len, | ||
| )) | ||
| }; | ||
| } | ||
|
|
||
| Expr::FnCall(f.into(), func_pos) | ||
| } | ||
| Expr::MethodCall(f, func_pos) => { | ||
| let mut f = *f; | ||
|
|
||
| let args_len = f.args.len() + 1; | ||
| f.args.insert(0, root); | ||
|
|
||
| // Recalculate hash for the new argument count | ||
| f.hashes = if is_valid_function_name(&f.name) { | ||
| #[cfg(not(feature = "no_function"))] | ||
| { | ||
| FnCallHashes::from_hash(calc_fn_hash(None, &f.name, args_len)) | ||
| } | ||
| #[cfg(feature = "no_function")] | ||
| { | ||
| FnCallHashes::from_native_only(calc_fn_hash( | ||
| None, &f.name, args_len, | ||
| )) | ||
| } | ||
| } else { | ||
| FnCallHashes::from_native_only(calc_fn_hash( | ||
| None, &f.name, args_len, | ||
| )) | ||
| }; | ||
|
|
||
| Expr::FnCall(f.into(), func_pos) | ||
| } | ||
| Expr::Variable(x, ..) => { | ||
| // Pipeline into a bare function name: lhs |> func => func(lhs) | ||
| let x = *x; // move out | ||
|
|
||
| #[cfg(not(feature = "no_module"))] | ||
| let (_index, name, namespace, _hash) = x; | ||
| #[cfg(feature = "no_module")] | ||
| let (index, name) = x; | ||
|
Check warning on line 2507 in src/parser.rs
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CI seems to complain about |
||
|
|
||
| let args_len = 1usize; | ||
|
|
||
| #[cfg(not(feature = "no_module"))] | ||
| let hashes = if is_valid_function_name(&name) { | ||
| FnCallHashes::from_hash(calc_fn_hash( | ||
| namespace.path.iter().map(Ident::as_str), | ||
| &name, | ||
| args_len, | ||
| )) | ||
| } else { | ||
| FnCallHashes::from_native_only(calc_fn_hash( | ||
| namespace.path.iter().map(Ident::as_str), | ||
| &name, | ||
| args_len, | ||
| )) | ||
| }; | ||
|
|
||
| #[cfg(feature = "no_module")] | ||
| let hashes = if is_valid_function_name(&name) { | ||
| FnCallHashes::from_hash(calc_fn_hash(None, &name, args_len)) | ||
| } else { | ||
| FnCallHashes::from_native_only(calc_fn_hash(None, &name, args_len)) | ||
| }; | ||
|
|
||
| let fn_call = FnCallExpr { | ||
| #[cfg(not(feature = "no_module"))] | ||
| namespace: { | ||
| #[cfg(not(feature = "no_module"))] | ||
| { | ||
| let mut ns = crate::ast::Namespace::NONE; | ||
| ns.path = namespace.path; | ||
| ns.index = namespace.index; | ||
| ns | ||
| } | ||
| }, | ||
| name: name.clone(), | ||
| hashes, | ||
| args: IntoIterator::into_iter([root]).collect(), | ||
| capture_parent_scope: false, | ||
| op_token: None, | ||
| }; | ||
|
|
||
| Expr::FnCall(fn_call.into(), pos) | ||
| } | ||
| _ => op_base.into_fn_call_expr(pos), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'd need parsing errors if the stuff following the operator is not a proper function call, instead of allowing it to go through. |
||
| } | ||
| } | ||
| _ => op_base.into_fn_call_expr(pos), | ||
| }; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -199,6 +199,8 @@ pub enum Token { | |
| Bang, | ||
| /// `|` | ||
| Pipe, | ||
| /// `|>` | ||
| PipeArrow, | ||
| /// `||` | ||
| Or, | ||
| /// `^` | ||
|
|
@@ -786,6 +788,7 @@ impl Token { | |
| EqualsTo => "==", | ||
| NotEqualsTo => "!=", | ||
| Pipe => "|", | ||
| PipeArrow => "|>", | ||
| Or => "||", | ||
| Ampersand => "&", | ||
| And => "&&", | ||
|
|
@@ -1034,7 +1037,7 @@ impl Token { | |
| use Token::*; | ||
|
|
||
| Precedence::new(match self { | ||
| Or | XOr | Pipe => 30, | ||
| Or | XOr | Pipe | PipeArrow => 30, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need a lower precedence for |>? |
||
|
|
||
| And | Ampersand => 60, | ||
|
|
||
|
|
@@ -2350,7 +2353,7 @@ fn get_next_token_inner( | |
| } | ||
| ('|', '>') => { | ||
| stream.eat_next_and_advance(pos); | ||
| return (Token::Reserved(Box::new("|>".into())), start_pos); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have to be careful here... some users may depend on the fact that |
||
| return (Token::PipeArrow, start_pos); | ||
| } | ||
| ('|', ..) => return (Token::Pipe, start_pos), | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| #![cfg(not(feature = "no_function"))] | ||
| use rhai::{Engine, INT}; | ||
|
|
||
| #[test] | ||
| fn test_pipeline_basic() { | ||
| let engine = Engine::new(); | ||
|
|
||
| // Simple function chaining: value is passed as first argument to the function on the right | ||
| let result = engine.eval::<INT>("fn inc(x) { x + 1 }; 1 |> inc |> inc").unwrap(); | ||
|
|
||
| assert_eq!(result, 3); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_pipeline_with_extra_arg() { | ||
| let engine = Engine::new(); | ||
|
|
||
| // Pipeline into a function that takes additional arguments | ||
| let result = engine.eval::<INT>("fn add(a, b) { a + b }; 1 |> add(2)").unwrap(); | ||
|
|
||
| assert_eq!(result, 3); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_pipeline_into_method_call_style() { | ||
| let engine = Engine::new(); | ||
|
|
||
| // Pipeline into a method-call-style builtin (abs). | ||
| // The pipeline passes the left-hand value as the first argument of the call on the right. | ||
| let result = engine.eval::<INT>("let x = -123; x |> abs(); x").unwrap(); | ||
|
|
||
| // abs should not have mutated `x` here, so `x` remains -123 | ||
| assert_eq!(result, -123); | ||
| } | ||
|
|
||
| #[cfg(not(feature = "no_object"))] | ||
| mod pipeline_method_tests { | ||
| use rhai::{Engine, INT}; | ||
|
|
||
| #[derive(Debug, Clone, Eq, PartialEq)] | ||
| struct TestStruct { | ||
| x: INT, | ||
| } | ||
|
|
||
| impl TestStruct { | ||
| fn update(&mut self, n: INT) { | ||
| self.x += n; | ||
| } | ||
|
|
||
| fn new() -> Self { | ||
| Self { x: 1 } | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_pipeline_into_registered_method() { | ||
| let mut engine = Engine::new(); | ||
|
|
||
| engine.register_type::<TestStruct>().register_fn("update", TestStruct::update).register_fn("new_ts", TestStruct::new); | ||
|
|
||
| // Pipeline into a registered method should forward the left-hand value as the first argument | ||
| let result = engine.eval::<TestStruct>("let x = new_ts(); x |> update(1000); x").unwrap(); | ||
|
|
||
| assert_eq!(result, TestStruct { x: 1001 }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove the derive attributes?