Fix: standalone inverse tags ({{^helper args}}) drop params/hash arguments - #1188
Open
tomasbjerre wants to merge 1 commit into
Open
tomasbjerre wants to merge 1 commit into
tomasbjerre wants to merge 1 commit into
Conversation
visitUnless() (handling standalone {{^helper args}}...{{/helper}}
tags) always built its Block with Collections.emptyList()/emptyMap()
for params/hash, discarding whatever the parser had actually parsed
from ctx.sexpr(). The sibling visitBlock() (handling {{#helper
args}}...{{/helper}}), which shares the same sexpr grammar rule,
correctly does params(sexpr.param()) / hash(sexpr.hash()) - visitUnless
just never got the same treatment.
For a helper without a body context binding, this is mostly invisible.
But for a custom helper that relies on its own hash arguments (e.g.
{{^ifSomething . scope="x"}}...{{/ifSomething}}), Options.hash("scope")
silently comes back null. Depending on what the helper does with the
missing value, this can surface as a confusing NullPointerException
deep inside the helper rather than any indication that the arguments
were dropped.
Fix mirrors visitBlock() exactly: params(sexpr.param()) and
hash(sexpr.hash()) instead of the hardcoded empty collections.
Added UnlessBlockHelperArgsTest covering both the context (first
positional param) and hash argument pass-through on a standalone
inverse tag. Ran the full handlebars module test suite
(1030 tests, 0 failures/errors) to confirm no regressions.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4LemHUKaqfXULRkmxfxwx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
visitUnless()inTemplateBuilder.java(which handles a standaloneinverse tag,
{{^helper args}}...{{/helper}}) always constructs itsBlockwithCollections.emptyList()/Collections.emptyMap()forparams/hash, discarding whatever
ctx.sexpr()actually parsed:The sibling
visitBlock(), which handles the positive form(
{{#helper args}}...{{/helper}}) and shares the exact samesexprgrammar rule (
sexpr : QID param* hash*), correctly does:So the grammar parses hash/param arguments on a standalone inverse tag
just fine (
unless : UNLESS sexpr blockParams? END body END_BLOCK nameEnd=QID END) - the visitor just never wires them through for thatone code path.
Impact
For a helper that only checks truthiness of a plain value this is
invisible, since most such usages don't pass extra args. But a custom
helper that relies on its own hash argument, e.g.
silently gets
options.hash("scope") == nullinstead of"x".Depending on what the helper does with that, this can surface as a
confusing
NullPointerExceptiondeep inside otherwise-correct helpercode, with no indication that the argument was dropped by the parser
rather than by the helper.
I ran into this in a Handlebars template that used
{{^ifCommitScope . scope="deps"}}...{{/ifCommitScope}}(a customhelper mirroring the existing
ifCommitScopepositive-form pattern)and got an NPE from inside
Options.hash(String)that took a while totrace back to this.
Fix
Mirror
visitBlock()exactly:params(sexpr.param())/hash(sexpr.hash())instead of the hardcoded empty collections.Tests
Added
UnlessBlockHelperArgsTestwith two cases:contextargument(matching how
visitBlock()/the positive form behaves)available via
options.param(0)/options.hash(...)Ran the full
handlebarsmodule test suite locally:1030 tests, 0 failures, 0 errors(3 pre-existing skips, unrelated).No other files are touched; I intentionally left the
spotless:applyformatting drift on unrelated pre-existing files (
Handlebars.java,StringHelpers.java,Text.java, a few test files) out of this PR tokeep the diff scoped to the actual fix.