diff --git a/CHANGELOG.md b/CHANGELOG.md index ea0a04710..b900929fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,15 +9,16 @@ * [Changed] Updated `rhai` to 0.19 and then 0.20 [#391] * [Changed] `#each` helper now renders else block for non-iterable data [#380] * [Changed] `TemplateError` and `ScriptError` is now a cause of `RenderError` [#395] -* [Changed] `RenderContext::get_partial` now returns `Option<&Template>` -* [Changed] Capitalize names like `HtmlExpression` and `IoError` based on clippy recommendations [#424] +* [Changed] **Breaking** `RenderContext::get_partial` now returns `Option<&Template>` +* [Changed] **Breaking** Capitalize names like `HtmlExpression` and `IoError` based on clippy recommendations [#424] +* [Changed] **Breaking** Improved return type of `call_inner` from `HelperDef` to avoid misleading [#437] * [Fixed] reference starts with `null`, `true` and `false` were parsed incorrectly [#382] * [Fixed] dir source path separator bug on windows [#389] [#405] * [Fixed] stack overflow with nested `@partial-block` [#401] * [Fixed] value access issue when upper block has a base value [#419] * [Fixed] escape rules for Json string literal [#423] -* [Removed] option to disable source map is removed [#395] -* [Removed] `TemplateFileError` and `TemplateRenderError` are removed and merged into +* [Removed] **Breaking** option to disable source map is removed [#395] +* [Removed] **Breaking** `TemplateFileError` and `TemplateRenderError` are removed and merged into `TemplateError` and `RenderError` [#395] ## [3.4.0](https://github.com/sunng87/handlebars-rust/compare/3.3.0...3.4.0) - 2020-08-14 diff --git a/src/error.rs b/src/error.rs index 6d83ad5f5..f4721623f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -12,13 +12,14 @@ use walkdir::Error as WalkdirError; use rhai::{EvalAltResult, ParseError}; /// Error when rendering data on template. -#[derive(Debug)] +#[derive(Debug, Default)] pub struct RenderError { pub desc: String, pub template_name: Option, pub line_no: Option, pub column_no: Option, cause: Option>, + unimplemented: bool, } impl fmt::Display for RenderError { @@ -93,10 +94,14 @@ impl RenderError { pub fn new>(desc: T) -> RenderError { RenderError { desc: desc.as_ref().to_owned(), - template_name: None, - line_no: None, - column_no: None, - cause: None, + ..Default::default() + } + } + + pub(crate) fn unimplemented() -> RenderError { + RenderError { + unimplemented: true, + ..Default::default() } } @@ -117,6 +122,11 @@ impl RenderError { e } + + #[inline] + pub(crate) fn is_unimplemented(&self) -> bool { + self.unimplemented + } } quick_error! { diff --git a/src/helpers/helper_lookup.rs b/src/helpers/helper_lookup.rs index 556eb4150..bf887debe 100644 --- a/src/helpers/helper_lookup.rs +++ b/src/helpers/helper_lookup.rs @@ -17,7 +17,7 @@ impl HelperDef for LookupHelper { r: &'reg Registry<'reg>, _: &'rc Context, _: &mut RenderContext<'reg, 'rc>, - ) -> Result>, RenderError> { + ) -> Result, RenderError> { let collection_value = h .param(0) .ok_or_else(|| RenderError::new("Param not found for helper \"lookup\""))?; @@ -30,18 +30,18 @@ impl HelperDef for LookupHelper { .value() .as_u64() .and_then(|u| v.get(u as usize)) - .map(|i| ScopedJson::Derived(i.clone())), + .unwrap_or(&Json::Null), Json::Object(ref m) => index .value() .as_str() .and_then(|k| m.get(k)) - .map(|i| ScopedJson::Derived(i.clone())), - _ => None, + .unwrap_or(&Json::Null), + _ => &Json::Null, }; - if r.strict_mode() && value.is_none() { + if r.strict_mode() && value.is_null() { Err(RenderError::strict_error(None)) } else { - Ok(value) + Ok(value.clone().into()) } } } diff --git a/src/helpers/mod.rs b/src/helpers/mod.rs index a0bd0d0b8..450f3717a 100644 --- a/src/helpers/mod.rs +++ b/src/helpers/mod.rs @@ -82,8 +82,8 @@ pub trait HelperDef { _: &'reg Registry<'reg>, _: &'rc Context, _: &mut RenderContext<'reg, 'rc>, - ) -> Result>, RenderError> { - Ok(None) + ) -> Result, RenderError> { + Err(RenderError::unimplemented()) } fn call<'reg: 'rc, 'rc>( @@ -94,17 +94,26 @@ pub trait HelperDef { rc: &mut RenderContext<'reg, 'rc>, out: &mut dyn Output, ) -> HelperResult { - if let Some(result) = self.call_inner(h, r, ctx, rc)? { - if r.strict_mode() && result.is_missing() { - return Err(RenderError::strict_error(None)); - } else { - // auto escape according to settings - let output = do_escape(r, rc, result.render()); - out.write(output.as_ref())?; + match self.call_inner(h, r, ctx, rc) { + Ok(result) => { + if r.strict_mode() && result.is_missing() { + Err(RenderError::strict_error(None)) + } else { + // auto escape according to settings + let output = do_escape(r, rc, result.render()); + out.write(output.as_ref())?; + Ok(()) + } + } + Err(e) => { + if e.is_unimplemented() { + // default implementation, do nothing + Ok(()) + } else { + Err(e) + } } } - - Ok(()) } } diff --git a/src/helpers/scripting.rs b/src/helpers/scripting.rs index cbd2145a4..cec3b9763 100644 --- a/src/helpers/scripting.rs +++ b/src/helpers/scripting.rs @@ -22,7 +22,7 @@ fn call_script_helper<'reg: 'rc, 'rc>( hash: &BTreeMap<&'reg str, PathAndJson<'reg, 'rc>>, engine: &Engine, script: &AST, -) -> Result>, RenderError> { +) -> Result, RenderError> { let params: Dynamic = to_dynamic(params.iter().map(|p| p.value()).collect::>())?; let hash: Dynamic = to_dynamic( @@ -41,7 +41,7 @@ fn call_script_helper<'reg: 'rc, 'rc>( let result_json: Json = from_dynamic(&result)?; - Ok(Some(ScopedJson::Derived(result_json))) + Ok(ScopedJson::Derived(result_json)) } impl HelperDef for ScriptHelper { @@ -51,7 +51,7 @@ impl HelperDef for ScriptHelper { reg: &'reg Registry<'reg>, _ctx: &'rc Context, _rc: &mut RenderContext<'reg, 'rc>, - ) -> Result>, RenderError> { + ) -> Result, RenderError> { call_script_helper(h.params(), h.hash(), ®.engine, &self.script) } } @@ -104,7 +104,6 @@ mod test { }; let result = call_script_helper(¶ms, &hash, &engine, &ast) - .unwrap() .unwrap() .render(); assert_eq!("1,true,2,no", &result); diff --git a/src/macros.rs b/src/macros.rs index 5e324b2ab..fde09d9e5 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -54,7 +54,7 @@ macro_rules! handlebars_helper { _: &'reg $crate::Handlebars<'reg>, _: &'rc $crate::Context, _: &mut $crate::RenderContext<'reg, 'rc>, - ) -> Result>, $crate::RenderError> { + ) -> Result<$crate::ScopedJson<'reg, 'rc>, $crate::RenderError> { let mut param_idx = 0; $( @@ -97,7 +97,7 @@ macro_rules! handlebars_helper { $(let $kwargs = h.hash().iter().map(|(k, v)| (k.to_owned(), v.value())).collect::>();)? let result = $body; - Ok(Some($crate::ScopedJson::Derived($crate::JsonValue::from(result)))) + Ok($crate::ScopedJson::Derived($crate::JsonValue::from(result))) } } }; diff --git a/src/registry.rs b/src/registry.rs index 07fd9b391..e7f5f1cfd 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -862,8 +862,8 @@ mod test { _: &'reg Registry<'reg>, _: &'rc Context, _: &mut RenderContext<'reg, 'rc>, - ) -> Result>, RenderError> { - Ok(Some(ScopedJson::Missing)) + ) -> Result, RenderError> { + Ok(ScopedJson::Missing) } } diff --git a/src/render.rs b/src/render.rs index 58689d496..188ea221a 100644 --- a/src/render.rs +++ b/src/render.rs @@ -539,25 +539,30 @@ fn call_helper_for_value<'reg: 'rc, 'rc>( ctx: &'rc Context, rc: &mut RenderContext<'reg, 'rc>, ) -> Result, RenderError> { - if let Some(result) = hd.call_inner(ht, r, ctx, rc)? { - Ok(PathAndJson::new(None, result)) - } else { - // parse value from output - let mut so = StringOutput::new(); - - // here we don't want subexpression result escaped, - // so we temporarily disable it - let disable_escape = rc.is_disable_escape(); - rc.set_disable_escape(true); - - hd.call(ht, r, ctx, rc, &mut so)?; - rc.set_disable_escape(disable_escape); - - let string = so.into_string().map_err(RenderError::from)?; - Ok(PathAndJson::new( - None, - ScopedJson::Derived(Json::String(string)), - )) + match hd.call_inner(ht, r, ctx, rc) { + Ok(result) => Ok(PathAndJson::new(None, result)), + Err(e) => { + if e.is_unimplemented() { + // parse value from output + let mut so = StringOutput::new(); + + // here we don't want subexpression result escaped, + // so we temporarily disable it + let disable_escape = rc.is_disable_escape(); + rc.set_disable_escape(true); + + hd.call(ht, r, ctx, rc, &mut so)?; + rc.set_disable_escape(disable_escape); + + let string = so.into_string().map_err(RenderError::from)?; + Ok(PathAndJson::new( + None, + ScopedJson::Derived(Json::String(string)), + )) + } else { + Err(e) + } + } } } diff --git a/tests/subexpression.rs b/tests/subexpression.rs index f85e1c65d..d1803b1a7 100644 --- a/tests/subexpression.rs +++ b/tests/subexpression.rs @@ -2,6 +2,9 @@ extern crate handlebars; #[macro_use] extern crate serde_json; +use std::sync::atomic::{AtomicU16, Ordering}; +use std::sync::Arc; + use handlebars::{Context, Handlebars, Helper, HelperDef, RenderContext, RenderError, ScopedJson}; #[test] @@ -75,11 +78,11 @@ impl HelperDef for MyHelper { _: &'reg Handlebars, _: &'rc Context, _: &mut RenderContext<'reg, 'rc>, - ) -> Result>, RenderError> { - Ok(Some(ScopedJson::Derived(json!({ + ) -> Result, RenderError> { + Ok(ScopedJson::Derived(json!({ "a": 1, "b": 2, - })))) + }))) } } @@ -95,3 +98,57 @@ fn test_lookup_with_subexpression() { assert_eq!("1", result); } + +struct CallCounterHelper { + pub(crate) c: Arc, +} + +impl HelperDef for CallCounterHelper { + fn call_inner<'reg: 'rc, 'rc>( + &self, + h: &Helper<'reg, 'rc>, + _: &'reg Handlebars, + _: &'rc Context, + _: &mut RenderContext<'reg, 'rc>, + ) -> Result, RenderError> { + // inc counter + self.c.fetch_add(1, Ordering::SeqCst); + + if let Some(_) = h.param(0) { + Ok(json!({ + "a": 1, + }) + .into()) + } else { + Ok(json!(null).into()) + } + } +} + +#[test] +fn test_helper_call_count() { + let mut registry = Handlebars::new(); + + let counter = Arc::new(AtomicU16::new(0)); + let helper = Box::new(CallCounterHelper { c: counter.clone() }); + + registry.register_helper("myhelper", helper); + + registry + .render_template( + "{{#if (myhelper a)}}something{{else}}nothing{{/if}}", + &json!(null), + ) // If returns true + .unwrap(); + + assert_eq!(1, counter.load(Ordering::SeqCst)); + + registry + .render_template( + "{{#if (myhelper)}}something{{else}}nothing{{/if}}", + &json!(null), + ) // If returns false + .unwrap(); + + assert_eq!(2, counter.load(Ordering::SeqCst)); +}