Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return an ExcelException object rather than a string to represent MS Excel formula errors #1605

Draft
wants to merge 39 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
692c5aa
Return an ExcelException object rather than a string to represent MS …
Aug 4, 2020
7cd9acf
phpcs fixes
Aug 4, 2020
4307e85
phpcs fixes
Aug 4, 2020
3a9110c
And the moral of the story: fixing one phpcs issue can actually creat…
Aug 4, 2020
3d96ebc
This should fix all the scrutinizer issues with the actual code, leav…
Aug 4, 2020
208fc1d
Simplify some of our nested ifs
Aug 4, 2020
ecb3c10
Refactor of xls writer for formulae
Aug 4, 2020
4b95bf4
Fix phpcs issue
Aug 4, 2020
51037e5
Fix phpcs issue
Aug 4, 2020
cd0ff30
Fix exception return datatypes in Engineering functions
Aug 4, 2020
5b84ebb
Fix exception return datatypes in Engineering and Statistical functions
Aug 4, 2020
a9f50a5
Fix some more phpcs issues
Aug 4, 2020
6b40b74
Minor refactoring of HTML Cell writer to eliminate pass by reference
Aug 5, 2020
5111ee9
Eliminate some redundant code
Aug 5, 2020
640bce8
Try to let scrutinizer now that we have identified object types in tests
Aug 5, 2020
f8dc0d0
Ok, let's try and keep phpcs happy with our attempted hackaround for …
Aug 5, 2020
2abec34
Seriously?!? `/**` is acceptable outside a method, but inside the met…
Aug 5, 2020
e189a3c
So let's see in Scrutinizer prefers this approach
Aug 5, 2020
01dd73f
So let's see in Scrutinizer prefers this approach
Aug 5, 2020
6b617d9
For the sake of a space
Aug 5, 2020
7c2c927
Scrutinizer really is pretty f***ing stupid
Aug 5, 2020
82b4a20
Fix some return typehinting for Date and Financial functions
Aug 5, 2020
c70ac2f
Fix some return typehinting for Financial functions
Aug 5, 2020
557277a
Fix some return type docblocks for DateTime and Financial functions
Aug 5, 2020
cc8a6d4
Excel error returns in unit tests are now ExcelException rather than …
Aug 6, 2020
0336b0c
Minor changes to typehinting, and a shortcut for date conversion
Aug 7, 2020
86902ff
Minor tweaks to return datatype docblocks
Aug 7, 2020
c73079f
Minor tweaks to return datatype docblocks
Aug 7, 2020
6cfd4ec
Trying to find that nice balance line between PHPStorm complaining th…
Aug 7, 2020
8c0f965
Trying to find that nice balance line between PHPStorm complaining th…
Aug 7, 2020
80480bf
So lets see what this achieves
Aug 7, 2020
5f31aaa
Extra typehinting
Aug 10, 2020
332b1af
Extra typehinting
Aug 10, 2020
9741159
Extra typehinting
Aug 11, 2020
2426c59
Extra typehinting
Aug 11, 2020
bf084fc
Extra typehinting
Aug 11, 2020
839bd11
Extra typehinting
Aug 12, 2020
bfe278b
Extra typehinting
Aug 26, 2020
464b57f
Update NamedFormula.php (#1701)
rachvela94 Jan 29, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 27 additions & 22 deletions src/PhpSpreadsheet/Calculation/Calculation.php
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ class Calculation
* Excel constant string translations to their PHP equivalents
* Constant conversion from text name/value to actual (datatyped) value.
*
* @var string[]
* @var mixed[]
*/
private static $excelConstants = [
'TRUE' => true,
Expand Down Expand Up @@ -3659,6 +3659,10 @@ private function showValue($value)
private function showTypeDetails($value)
{
if ($this->debugLog->getWriteDebugLog()) {
if ($value instanceof ExcelException) {
return "an Excel {$value->errorName()} Error";
}

$testArray = Functions::flattenArray($value);
if (count($testArray) == 1) {
$value = array_pop($testArray);
Expand All @@ -3677,14 +3681,14 @@ private function showTypeDetails($value)
} else {
if ($value == '') {
return 'an empty string';
} elseif ($value[0] == '#') {
return 'a ' . $value . ' error';
}
$typeString = 'a string';
}

return $typeString . ' with a value of ' . $this->showValue($value);
}

return null;
}

/**
Expand Down Expand Up @@ -4039,9 +4043,11 @@ private function internalParseFormula($formula, ?Cell $pCell = null)
$length = strlen($val);
if (preg_match('/^' . self::CALCULATION_REGEXP_FUNCTION . '$/miu', $val, $matches)) {
$val = preg_replace('/\s/u', '', $val);
$reference = null;
if (isset(self::$phpSpreadsheetFunctions[strtoupper($matches[1])]) || isset(self::$controlFunctions[strtoupper($matches[1])])) { // it's a function
$valToUpper = strtoupper($val);
} else {
$reference = strtoupper($val);
$valToUpper = 'NAME.ERROR(';
}
// here $matches[1] will contain values like "IF"
Expand All @@ -4057,7 +4063,7 @@ private function internalParseFormula($formula, ?Cell $pCell = null)
}
}

$stack->push('Function', $valToUpper, null, $currentCondition, $currentOnlyIf, $currentOnlyIfNot);
$stack->push('Function', $valToUpper, $reference, $currentCondition, $currentOnlyIf, $currentOnlyIfNot);
// tests if the function is closed right after opening
$ax = preg_match('/^\s*\)/u', substr($formula, $index + $length));
if ($ax) {
Expand Down Expand Up @@ -4493,7 +4499,7 @@ private function processTokenStack($tokens, $cellID = null, ?Cell $pCell = null)
$result = $matrixResult->getArray();
} catch (\Exception $ex) {
$this->debugLog->writeDebugLog('JAMA Matrix Exception: ', $ex->getMessage());
$result = '#VALUE!';
$result = ExcelException::VALUE();
}
} else {
$result = self::FORMULA_STRING_QUOTE . str_replace('""', self::FORMULA_STRING_QUOTE, self::unwrapResult($operand1) . self::unwrapResult($operand2)) . self::FORMULA_STRING_QUOTE;
Expand Down Expand Up @@ -4551,7 +4557,7 @@ private function processTokenStack($tokens, $cellID = null, ?Cell $pCell = null)
$result = $matrixResult->getArray();
} catch (\Exception $ex) {
$this->debugLog->writeDebugLog('JAMA Matrix Exception: ', $ex->getMessage());
$result = '#VALUE!';
$result = ExcelException::VALUE();
}
$this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails($result));
$stack->push('Value', $result);
Expand Down Expand Up @@ -4769,6 +4775,13 @@ private function processTokenStack($tokens, $cellID = null, ?Cell $pCell = null)

private function validateBinaryOperand(&$operand, &$stack)
{
if ($operand instanceof ExcelException) {
$stack->push('Error', $operand);
$this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails($operand));

return false;
}

if (is_array($operand)) {
if ((count($operand, COUNT_RECURSIVE) - count($operand)) == 1) {
do {
Expand All @@ -4784,20 +4797,12 @@ private function validateBinaryOperand(&$operand, &$stack)
$operand = self::unwrapResult($operand);
}
// If the string is a numeric value, we treat it as a numeric, so no further testing
if (!is_numeric($operand)) {
// If not a numeric, test to see if the value is an Excel error, and so can't be used in normal binary operations
if ($operand > '' && $operand[0] == '#') {
$stack->push('Value', $operand);
$this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails($operand));

return false;
} elseif (!Shared\StringHelper::convertToNumberIfFraction($operand)) {
// If not a numeric or a fraction, then it's a text string, and so can't be used in mathematical binary operations
$stack->push('Error', '#VALUE!');
$this->debugLog->writeDebugLog('Evaluation Result is a ', $this->showTypeDetails('#VALUE!'));

return false;
}
if (!is_numeric($operand) && !Shared\StringHelper::convertToNumberIfFraction($operand)) {
// If not a numeric or a fraction, then it's a text string, and so can't be used in mathematical binary operations
$stack->push('Error', ExcelException::VALUE());
$this->debugLog->writeDebugLog('Evaluation Result is a ', $this->showTypeDetails('#VALUE!'));

return false;
}
}

Expand Down Expand Up @@ -4992,7 +4997,7 @@ private function executeNumericBinaryOperation($operand1, $operand2, $operation,
$result = $matrixResult->getArray();
} catch (\Exception $ex) {
$this->debugLog->writeDebugLog('JAMA Matrix Exception: ', $ex->getMessage());
$result = '#VALUE!';
$result = ExcelException::VALUE();
}
} else {
if (
Expand Down Expand Up @@ -5023,7 +5028,7 @@ private function executeNumericBinaryOperation($operand1, $operand2, $operation,
case '/':
if ($operand2 == 0) {
// Trap for Divide by Zero error
$stack->push('Error', '#DIV/0!');
$stack->push('Error', ExcelException::DIV0());
$this->debugLog->writeDebugLog('Evaluation Result is ', $this->showTypeDetails('#DIV/0!'));

return false;
Expand Down
2 changes: 1 addition & 1 deletion src/PhpSpreadsheet/Calculation/Database.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ private static function getFilteredColumn($database, $field, $criteria)
* the column label in which you specify a condition for the
* column.
*
* @return float|string
* @return ExcelException|float
*/
public static function DAVERAGE($database, $field, $criteria)
{
Expand Down
Loading