Skip to content

fix: chain {{else helper ...}} under {{#if}} and {{#unless}} - #684

Open
EnesKartal wants to merge 1 commit into
Handlebars-Net:masterfrom
EnesKartal:fix/else-block-helper-under-if
Open

EnesKartal wants to merge 1 commit into
Handlebars-Net:masterfrom
EnesKartal:fix/else-block-helper-under-if

Conversation

@EnesKartal

Copy link
Copy Markdown

Fixes #683.

ConditionalBlockAccumulatorContext handled every chained {{else ...}} itself, but only recognised a two-argument {{else if x}}:

  • {{#if a}}A{{else ifEq b "x"}}B{{else}}C{{/if}} took {{else ifEq b "x"}} as a plain {{else}}, so the next {{else}} built Expression.IfThen(null, ...) and Compile threw ArgumentNullException (Parameter 'test').
  • Without the final {{else}} the ifEq condition was silently dropped and B always rendered.
  • A two-argument {{else with b}} was evaluated as {{else if b}}.
  • {{else unless x}} under {{#if}} was evaluated as {{else if x}} (the condition was not negated).

Change

  • HandlesChainedElseInternally becomes HandlesChainedElse(item). The if/unless context returns true only for {{else if ...}} and {{else unless ...}}. Any other {{else name ...}} goes through the desugaring from feat: support else-if chaining for block helpers #648, the same as under other block helpers.
  • {{else unless x}} is negated. {{else if}} / {{else unless}} with the wrong number of arguments now throw HandlebarsCompilerException ("#if requires exactly one argument"), like {{#if}} / {{#unless}} since fix: throw HandlebarsCompilerException for #if/#unless with wrong argument count #678.
  • The desugaring returns AccumulatedBlock without passing the closing tag to the outer context, so the conditional context now builds its last branch in AccumulatedBlock from the buffer instead of in IsClosingElement. The result is the same expression as before for templates that were already supported.

Test plan

New tests next to the #263 chained-else tests in IssueTests, plus two InlineData rows in ExceptionTests.TestConditionalBlockWithWrongArgumentCountException.

Before the fix (dotnet test --filter ..., library changes stashed):

Failed ExceptionTests.TestConditionalBlockWithWrongArgumentCountException(source: "{{#if a}}yes{{else unless b c}}no{{/if}}", helper: "#unless")
Failed ExceptionTests.TestConditionalBlockWithWrongArgumentCountException(source: "{{#if a}}yes{{else if b c}}no{{/if}}", helper: "#if")
Failed IssueTests.ElseWithCustomBlockHelperInvocationChainsUnderIf          (ArgumentNullException: Value cannot be null. (Parameter 'test'))
Failed IssueTests.ElseWithCustomBlockHelperInvocationUnderIfKeepsItsCondition
Failed IssueTests.ElseWithSingleArgumentBlockHelperUnderIfIsNotTreatedAsElseIf
Failed IssueTests.ElseUnlessChainsUnderIf                                   (Expected "B", Actual "C")
Failed IssueTests.ElseWithCustomBlockHelperInvocationChainsUnderUnless      (ArgumentNullException)
Failed!  - Failed: 7, Passed: 10, Total: 17

After the fix, whole test project:

Passed!  - Failed: 0, Passed: 2241, Skipped: 0, Total: 2241 - Handlebars.Test.dll (net10.0)

dotnet build source/Handlebars/Handlebars.csproj -f netstandard2.0 also builds.

The if/unless context handled every chained else itself, but only understood a two-argument {{else if x}}. {{else ifEq b "x"}} was taken as a plain {{else}}, which dropped its condition and made a following {{else}} throw ArgumentNullException at compile time, and a two-argument {{else with b}} was evaluated as {{else if b}}.

The context now only keeps {{else if x}} and {{else unless x}} for itself. Any other {{else name ...}} is desugared into a nested block by BlockAccumulator, as for other block helpers. {{else unless x}} is now negated, and {{else if}} / {{else unless}} with the wrong number of arguments throw HandlebarsCompilerException like {{#if}} / {{#unless}}.

Fixes Handlebars-Net#683

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

{{else customHelper args}} inside {{#if}} / {{#unless}} throws ArgumentNullException (Parameter 'test') at compile time

1 participant