Skip to content

Commit

Permalink
[BUGFIX] Fix math expression error verbosity
Browse files Browse the repository at this point in the history
Prevents seeing the “non-numeric value encountered”
error from PHP when a template attempts to do math
on a variable that is not declared, or does not have the
right type. See code comments for exact behavior.

The intention is to match PHP type coercion as close
as possible - among other things by inferring NULL as
value when a variable is most likely not defined.
  • Loading branch information
NamelessCoder committed Jul 16, 2018
1 parent a67b31f commit 73d8cf2
Showing 1 changed file with 42 additions and 1 deletion.
43 changes: 42 additions & 1 deletion src/Core/Parser/SyntaxTree/Expression/MathExpressionNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,17 @@ public static function evaluateExpression(RenderingContextInterface $renderingCo
if (in_array($part, $operators)) {
$operator = $part;
} else {
$part = static::getTemplateVariableOrValueItself($part, $renderingContext);
if (!is_numeric($part)) {
$newPart = static::getTemplateVariableOrValueItself($part, $renderingContext);
if ($newPart === $part) {
// Pitfall: the expression part was not numeric and did not resolve to a variable. We null the
// value - although this means the edge case of a variable's value being the same as its name,
// results in the expression part being treated as zero. Which is different from how PHP would
// coerce types in earlier versions, implying that a non-numeric string just counts as "1".
// Here, it counts as zero with the intention of error prevention on undeclared variables.
$part = null;
}
}
$result = self::evaluateOperation($result, $operator, $part);
}
}
Expand All @@ -69,6 +79,19 @@ public static function evaluateExpression(RenderingContextInterface $renderingCo
*/
protected static function evaluateOperation($left, $operator, $right)
{
// Special case: the "+" operator can be used with two arrays which will combine the two arrays. But it is
// only allowable if both sides are in fact arrays and only for this one operator. Please see PHP documentation
// about "union" on https://secure.php.net/manual/en/language.operators.array.php for specific behavior!
if ($operator === '+' && is_array($left) && is_array($right)) {
return $left + $right;
}

// Guard: if left or right side are not numeric values, infer a value for the expression part based on how
// PHP would coerce types in versions that are not strict typed. We do this to avoid fatal PHP errors about
// encountering non-numeric values.
$left = static::coerceNumericValue($left);
$right = static::coerceNumericValue($right);

if ($operator === '%') {
return $left % $right;
} elseif ($operator === '-') {
Expand All @@ -84,4 +107,22 @@ protected static function evaluateOperation($left, $operator, $right)
}
return 0;
}

protected static function coerceNumericValue($value)
{
if (is_object($value) && method_exists($object, '__toString')) {
// Delegate to another coercion call after casting to string
return static::coerceNumericValue((string) $value);
}
if (is_null($value)) {
return 0;
}
if (is_bool($value)) {
return $value ? 1 : 0;
}
if (is_numeric($value)) {
return $value;
}
return 0;
}
}

0 comments on commit 73d8cf2

Please sign in to comment.