Skip to content

Commit 4b04cc1

Browse files
oleibmanoleibman
committed
Propagate Errors in Text Functions
Fix #2581 (not obvious - see next paragraph for explanation). This continues the work of PR #2902 (and also PR #3467) to have errors propagated through function calculations rather than treating them as strings. All text functions, and the concatenation operator, are addressed in this PR. In the original issue, the spreadsheet being loaded uses the result of an unimplemented function as an argument to another function. When `getCalculatedValue` is used on the cell in question, the result is returned as `#VALUE!`. If the cell had just contained a function call to the unimplemented function, getCalculatedValue would have recognized the situation and returned oldCalculatedValue as the result. Not perfect, but good enough most of the time. User would like oldCalculatedValue returned here as well, which seems like a reasonable request. PhpSpreadsheet always returns `#Not Yet Implemented` as the result for a function which it knows about but which is not yet implemented. That is the key to the `Cell` class being able to substitute oldCalculatedValue in the first place. However, in order to do that for the issue in question, that result has to be propagated to any functions for which the result is an argument. I don't want to add unimplemented to the list of known error codes, but I am willing to add a parameter to `ErrorValue::isError` to indicate whether that value should be considered an error (default is "no"). The first use of that new parameter would be by the text functions. They go through a common Helper routine, so it is pretty easily implemented. And, as it turns out, most of the text functions do not currently propagate errors, e.g. if A1 results in a value error, `=LEFT(A1,2)` will result in `#V` rather than `#VALUE!`. With this PR, they will now be handled correctly.
1 parent 318a82e commit 4b04cc1

File tree

15 files changed

+248
-40
lines changed

15 files changed

+248
-40
lines changed

‎src/PhpSpreadsheet/Calculation/Calculation.php

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4792,13 +4792,20 @@ private function processTokenStack(mixed $tokens, ?string $cellID = null, ?Cell
47924792

47934793
for ($row = 0; $row < $rows; ++$row) {
47944794
for ($column = 0; $column < $columns; ++$column) {
4795-
$operand1[$row][$column]
4796-
= Shared\StringHelper::substring(
4797-
self::boolToString($operand1[$row][$column])
4798-
. self::boolToString($operand2[$row][$column]),
4799-
0,
4800-
DataType::MAX_STRING_LENGTH
4801-
);
4795+
$op1x = self::boolToString($operand1[$row][$column]);
4796+
$op2x = self::boolToString($operand2[$row][$column]);
4797+
if (Information\ErrorValue::isError($op1x)) {
4798+
// no need to do anything
4799+
} elseif (Information\ErrorValue::isError($op2x)) {
4800+
$operand1[$row][$column] = $op2x;
4801+
} else {
4802+
$operand1[$row][$column]
4803+
= Shared\StringHelper::substring(
4804+
$op1x . $op2x,
4805+
0,
4806+
DataType::MAX_STRING_LENGTH
4807+
);
4808+
}
48024809
}
48034810
}
48044811
$result = $operand1;
@@ -4808,7 +4815,13 @@ private function processTokenStack(mixed $tokens, ?string $cellID = null, ?Cell
48084815
// using the concatenation operator
48094816
// with literals that fits in 32K,
48104817
// so I don't think we can overflow here.
4811-
$result = self::FORMULA_STRING_QUOTE . str_replace('""', self::FORMULA_STRING_QUOTE, self::unwrapResult($operand1) . self::unwrapResult($operand2)) . self::FORMULA_STRING_QUOTE;
4818+
if (Information\ErrorValue::isError($operand1)) {
4819+
$result = $operand1;
4820+
} elseif (Information\ErrorValue::isError($operand2)) {
4821+
$result = $operand2;
4822+
} else {
4823+
$result = self::FORMULA_STRING_QUOTE . str_replace('""', self::FORMULA_STRING_QUOTE, self::unwrapResult($operand1) . self::unwrapResult($operand2)) . self::FORMULA_STRING_QUOTE;
4824+
}
48124825
}
48134826
$this->debugLog->writeDebugLog('Evaluation Result is %s', $this->showTypeDetails($result));
48144827
$stack->push('Value', $result);

‎src/PhpSpreadsheet/Calculation/Functions.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ class Functions
2626
const RETURNDATE_PHP_DATETIME_OBJECT = 'O';
2727
const RETURNDATE_EXCEL = 'E';
2828

29+
public const NOT_YET_IMPLEMENTED = '#Not Yet Implemented';
30+
2931
/**
3032
* Compatibility mode to use for error checking and responses.
3133
*/
@@ -123,7 +125,7 @@ public static function getReturnDateType(): string
123125
*/
124126
public static function DUMMY(): string
125127
{
126-
return '#Not Yet Implemented';
128+
return self::NOT_YET_IMPLEMENTED;
127129
}
128130

129131
public static function isMatrixValue(mixed $idx): bool

‎src/PhpSpreadsheet/Calculation/Information/ErrorValue.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace PhpOffice\PhpSpreadsheet\Calculation\Information;
44

55
use PhpOffice\PhpSpreadsheet\Calculation\ArrayEnabled;
6+
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
67

78
class ErrorValue
89
{
@@ -35,7 +36,7 @@ public static function isErr(mixed $value = ''): array|bool
3536
* @return array|bool If an array of numbers is passed as an argument, then the returned result will also be an array
3637
* with the same dimensions
3738
*/
38-
public static function isError(mixed $value = ''): array|bool
39+
public static function isError(mixed $value = '', bool $tryNotImplemented = false): array|bool
3940
{
4041
if (is_array($value)) {
4142
return self::evaluateSingleArgumentArray([self::class, __FUNCTION__], $value);
@@ -44,6 +45,9 @@ public static function isError(mixed $value = ''): array|bool
4445
if (!is_string($value)) {
4546
return false;
4647
}
48+
if ($tryNotImplemented && $value === Functions::NOT_YET_IMPLEMENTED) {
49+
return true;
50+
}
4751

4852
return in_array($value, ExcelError::ERROR_CODES, true);
4953
}

‎src/PhpSpreadsheet/Calculation/TextData/CaseConvert.php

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace PhpOffice\PhpSpreadsheet\Calculation\TextData;
44

55
use PhpOffice\PhpSpreadsheet\Calculation\ArrayEnabled;
6+
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp;
67
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
78

89
class CaseConvert
@@ -26,7 +27,11 @@ public static function lower(mixed $mixedCaseValue): array|string
2627
return self::evaluateSingleArgumentArray([self::class, __FUNCTION__], $mixedCaseValue);
2728
}
2829

29-
$mixedCaseValue = Helpers::extractString($mixedCaseValue);
30+
try {
31+
$mixedCaseValue = Helpers::extractString($mixedCaseValue, true);
32+
} catch (CalcExp $e) {
33+
return $e->getMessage();
34+
}
3035

3136
return StringHelper::strToLower($mixedCaseValue);
3237
}
@@ -48,7 +53,11 @@ public static function upper(mixed $mixedCaseValue): array|string
4853
return self::evaluateSingleArgumentArray([self::class, __FUNCTION__], $mixedCaseValue);
4954
}
5055

51-
$mixedCaseValue = Helpers::extractString($mixedCaseValue);
56+
try {
57+
$mixedCaseValue = Helpers::extractString($mixedCaseValue, true);
58+
} catch (CalcExp $e) {
59+
return $e->getMessage();
60+
}
5261

5362
return StringHelper::strToUpper($mixedCaseValue);
5463
}
@@ -70,7 +79,11 @@ public static function proper(mixed $mixedCaseValue): array|string
7079
return self::evaluateSingleArgumentArray([self::class, __FUNCTION__], $mixedCaseValue);
7180
}
7281

73-
$mixedCaseValue = Helpers::extractString($mixedCaseValue);
82+
try {
83+
$mixedCaseValue = Helpers::extractString($mixedCaseValue, true);
84+
} catch (CalcExp $e) {
85+
return $e->getMessage();
86+
}
7487

7588
return StringHelper::strToTitle($mixedCaseValue);
7689
}

‎src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace PhpOffice\PhpSpreadsheet\Calculation\TextData;
44

55
use PhpOffice\PhpSpreadsheet\Calculation\ArrayEnabled;
6+
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp;
67
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
78
use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
89

@@ -26,7 +27,12 @@ public static function character(mixed $character): array|string
2627
return self::evaluateSingleArgumentArray([self::class, __FUNCTION__], $character);
2728
}
2829

29-
$character = Helpers::validateInt($character);
30+
try {
31+
$character = Helpers::validateInt($character, true);
32+
} catch (CalcExp $e) {
33+
return $e->getMessage();
34+
}
35+
3036
$min = Functions::getCompatibilityMode() === Functions::COMPATIBILITY_OPENOFFICE ? 0 : 1;
3137
if ($character < $min || $character > 255) {
3238
return ExcelError::VALUE();
@@ -52,7 +58,12 @@ public static function code(mixed $characters): array|string|int
5258
return self::evaluateSingleArgumentArray([self::class, __FUNCTION__], $characters);
5359
}
5460

55-
$characters = Helpers::extractString($characters);
61+
try {
62+
$characters = Helpers::extractString($characters, true);
63+
} catch (CalcExp $e) {
64+
return $e->getMessage();
65+
}
66+
5667
if ($characters === '') {
5768
return ExcelError::VALUE();
5869
}

‎src/PhpSpreadsheet/Calculation/TextData/Concatenate.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public static function CONCATENATE(...$args): string
2727

2828
foreach ($aArgs as $arg) {
2929
$value = Helpers::extractString($arg);
30-
if (ErrorValue::isError($value)) {
30+
if (ErrorValue::isError($value, true)) {
3131
$returnValue = $value;
3232

3333
break;
@@ -85,7 +85,7 @@ private static function evaluateTextJoinArray(bool $ignoreEmpty, array &$aArgs):
8585
{
8686
foreach ($aArgs as $key => &$arg) {
8787
$value = Helpers::extractString($arg);
88-
if (ErrorValue::isError($value)) {
88+
if (ErrorValue::isError($value, true)) {
8989
return $value;
9090
}
9191

@@ -123,7 +123,7 @@ public static function builtinREPT(mixed $stringValue, mixed $repeatCount): arra
123123

124124
if (!is_numeric($repeatCount) || $repeatCount < 0) {
125125
$returnValue = ExcelError::VALUE();
126-
} elseif (ErrorValue::isError($stringValue)) {
126+
} elseif (ErrorValue::isError($stringValue, true)) {
127127
$returnValue = $stringValue;
128128
} else {
129129
$returnValue = str_repeat($stringValue, (int) $repeatCount);

‎src/PhpSpreadsheet/Calculation/TextData/Extract.php

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public static function left(mixed $value, mixed $chars = 1): array|string
3131
}
3232

3333
try {
34-
$value = Helpers::extractString($value);
34+
$value = Helpers::extractString($value, true);
3535
$chars = Helpers::extractInt($chars, 0, 1);
3636
} catch (CalcExp $e) {
3737
return $e->getMessage();
@@ -61,7 +61,7 @@ public static function mid(mixed $value, mixed $start, mixed $chars): array|stri
6161
}
6262

6363
try {
64-
$value = Helpers::extractString($value);
64+
$value = Helpers::extractString($value, true);
6565
$start = Helpers::extractInt($start, 1);
6666
$chars = Helpers::extractInt($chars, 0);
6767
} catch (CalcExp $e) {
@@ -90,7 +90,7 @@ public static function right(mixed $value, mixed $chars = 1): array|string
9090
}
9191

9292
try {
93-
$value = Helpers::extractString($value);
93+
$value = Helpers::extractString($value, true);
9494
$chars = Helpers::extractInt($chars, 0, 1);
9595
} catch (CalcExp $e) {
9696
return $e->getMessage();
@@ -132,7 +132,13 @@ public static function before(mixed $text, $delimiter, mixed $instance = 1, mixe
132132
return self::evaluateArrayArgumentsIgnore([self::class, __FUNCTION__], 1, $text, $delimiter, $instance, $matchMode, $matchEnd, $ifNotFound);
133133
}
134134

135-
$text = Helpers::extractString($text ?? '');
135+
try {
136+
$text = Helpers::extractString($text ?? '', true);
137+
Helpers::extractString(Functions::flattenSingleValue($delimiter ?? ''), true);
138+
} catch (CalcExp $e) {
139+
return $e->getMessage();
140+
}
141+
136142
$instance = (int) $instance;
137143
$matchMode = (int) $matchMode;
138144
$matchEnd = (int) $matchEnd;
@@ -190,7 +196,13 @@ public static function after(mixed $text, $delimiter, mixed $instance = 1, mixed
190196
return self::evaluateArrayArgumentsIgnore([self::class, __FUNCTION__], 1, $text, $delimiter, $instance, $matchMode, $matchEnd, $ifNotFound);
191197
}
192198

193-
$text = Helpers::extractString($text ?? '');
199+
try {
200+
$text = Helpers::extractString($text ?? '', true);
201+
Helpers::extractString(Functions::flattenSingleValue($delimiter ?? ''), true);
202+
} catch (CalcExp $e) {
203+
return $e->getMessage();
204+
}
205+
194206
$instance = (int) $instance;
195207
$matchMode = (int) $matchMode;
196208
$matchEnd = (int) $matchEnd;

‎src/PhpSpreadsheet/Calculation/TextData/Format.php

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use PhpOffice\PhpSpreadsheet\Calculation\DateTimeExcel;
99
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp;
1010
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
11+
use PhpOffice\PhpSpreadsheet\Calculation\Information\ErrorValue;
1112
use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
1213
use PhpOffice\PhpSpreadsheet\Calculation\MathTrig;
1314
use PhpOffice\PhpSpreadsheet\RichText\RichText;
@@ -123,8 +124,13 @@ public static function TEXTFORMAT(mixed $value, mixed $format): array|string
123124
return self::evaluateArrayArguments([self::class, __FUNCTION__], $value, $format);
124125
}
125126

126-
$value = Helpers::extractString($value);
127-
$format = Helpers::extractString($format);
127+
try {
128+
$value = Helpers::extractString($value, true);
129+
$format = Helpers::extractString($format, true);
130+
} catch (CalcExp $e) {
131+
return $e->getMessage();
132+
}
133+
128134
$format = (string) NumberFormat::convertSystemFormats($format);
129135

130136
if (!is_numeric($value) && Date::isDateTimeFormatCode($format)) {
@@ -152,6 +158,9 @@ private static function convertValue(mixed $value, bool $spacesMeanZero = false)
152158
}
153159
if (is_string($value)) {
154160
$value = trim($value);
161+
if (ErrorValue::isError($value, true)) {
162+
throw new CalcExp($value);
163+
}
155164
if ($spacesMeanZero && $value === '') {
156165
$value = 0;
157166
}
@@ -220,7 +229,7 @@ public static function VALUE(mixed $value = '')
220229
}
221230

222231
/**
223-
* TEXT.
232+
* VALUETOTEXT.
224233
*
225234
* @param mixed $value The value to format
226235
* Or can be an array of values

‎src/PhpSpreadsheet/Calculation/TextData/Helpers.php

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public static function extractString(mixed $value, bool $throwIfError = false):
2727
if (is_bool($value)) {
2828
return self::convertBooleanValue($value);
2929
}
30-
if ($throwIfError && is_string($value) && ErrorValue::isError($value)) {
30+
if ($throwIfError && is_string($value) && ErrorValue::isError($value, true)) {
3131
throw new CalcExp($value);
3232
}
3333

@@ -63,18 +63,28 @@ public static function extractFloat(mixed $value): float
6363
$value = (float) $value;
6464
}
6565
if (!is_numeric($value)) {
66+
if (is_string($value) && ErrorValue::isError($value, true)) {
67+
throw new CalcExp($value);
68+
}
69+
6670
throw new CalcExp(ExcelError::VALUE());
6771
}
6872

6973
return (float) $value;
7074
}
7175

72-
public static function validateInt(mixed $value): int
76+
public static function validateInt(mixed $value, bool $throwIfError = false): int
7377
{
7478
if ($value === null) {
7579
$value = 0;
7680
} elseif (is_bool($value)) {
7781
$value = (int) $value;
82+
} elseif ($throwIfError && is_string($value) && !is_numeric($value)) {
83+
if (!ErrorValue::isError($value, true)) {
84+
$value = ExcelError::VALUE();
85+
}
86+
87+
throw new CalcExp($value);
7888
}
7989

8090
return (int) $value;

‎src/PhpSpreadsheet/Calculation/TextData/Search.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ public static function sensitive(mixed $needle, mixed $haystack, mixed $offset =
3232
}
3333

3434
try {
35-
$needle = Helpers::extractString($needle);
36-
$haystack = Helpers::extractString($haystack);
35+
$needle = Helpers::extractString($needle, true);
36+
$haystack = Helpers::extractString($haystack, true);
3737
$offset = Helpers::extractInt($offset, 1, 0, true);
3838
} catch (CalcExp $e) {
3939
return $e->getMessage();
@@ -74,8 +74,8 @@ public static function insensitive(mixed $needle, mixed $haystack, mixed $offset
7474
}
7575

7676
try {
77-
$needle = Helpers::extractString($needle);
78-
$haystack = Helpers::extractString($haystack);
77+
$needle = Helpers::extractString($needle, true);
78+
$haystack = Helpers::extractString($haystack, true);
7979
$offset = Helpers::extractInt($offset, 1, 0, true);
8080
} catch (CalcExp $e) {
8181
return $e->getMessage();

0 commit comments

Comments
 (0)