Skip to content

Commit

Permalink
[BUGFIX] Consistent return types for condition-based ViewHelpers
Browse files Browse the repository at this point in the history
In most cases, condition-based ViewHelpers already return an empty string
as default value, for example if the verdict is false and else is not set. However,
there are a few cases where `null` would be returned instead, which would
in the template implicitly converted to an empty string in the vast majority of
cases anyway.

This change makes sure that all closures or direct ViewHelper node evaluations
default to an empty string if `null` is returned, making the behavior consistent
to other code paths where an empty string is returned explicitly.
  • Loading branch information
s2b committed Feb 11, 2025
1 parent 87e9c0a commit fddf029
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 8 deletions.
10 changes: 5 additions & 5 deletions src/Core/ViewHelper/AbstractConditionViewHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ protected function renderThenChild()

// Closure might be present if ViewHelper is called from a cached template
if ($this->thenClosure !== null) {
return ($this->thenClosure)();
return ($this->thenClosure)() ?? '';
}

// The following code can only be evaluated for uncached templates where the node structure
Expand All @@ -123,7 +123,7 @@ protected function renderThenChild()
if ($elseViewHelperEncountered) {
return '';
}
return $this->renderChildren();
return $this->renderChildren() ?? '';
}

/**
Expand All @@ -142,7 +142,7 @@ protected function renderElseChild()
// the "body" closure if condition is met
foreach ($this->elseIfClosures as $elseIf) {
if ($elseIf['condition']()) {
return $elseIf['body']();
return $elseIf['body']() ?? '';
}
}
}
Expand All @@ -156,7 +156,7 @@ protected function renderElseChild()

// Closure might be present if ViewHelper is called from a cached template
if ($this->elseClosure !== null) {
return ($this->elseClosure)();
return ($this->elseClosure)() ?? '';
}

// The following code can only be evaluated for uncached templates where the node structure
Expand Down Expand Up @@ -188,7 +188,7 @@ protected function renderElseChild()
return $this->arguments['else'];
}

return $elseNode instanceof ViewHelperNode ? $elseNode->evaluate($this->renderingContext) : '';
return $elseNode instanceof ViewHelperNode ? $elseNode->evaluate($this->renderingContext) ?? '' : '';
}

/**
Expand Down
5 changes: 2 additions & 3 deletions tests/Functional/ViewHelpers/IfThenElseViewHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public static function renderDataProvider(): \Generator
yield 'else argument, verdict true' => [
'<f:if condition="{verdict}" else="elseArgument" />',
['verdict' => true],
null,
'',
];
yield 'else argument, verdict false' => [
'<f:if condition="{verdict}" else="elseArgument" />',
Expand Down Expand Up @@ -396,8 +396,7 @@ public static function renderDataProvider(): \Generator
yield 'inline syntax, else argument, verdict true' => [
'{f:if(condition:\'{verdict}\', else:\'elseArgument\')}',
['verdict' => true],
// @todo: wtf?
null,
'',
];
yield 'inline syntax, else argument, verdict false' => [
'{f:if(condition:\'{verdict}\', else:\'elseArgument\')}',
Expand Down

0 comments on commit fddf029

Please sign in to comment.