Skip to content

Commit

Permalink
Integrate Exceptions Better with PhpStorm (#3765)
Browse files Browse the repository at this point in the history 
Fix #3754. Because `PhpSpreadsheetException` extends `Exception`, PhpStorm warns that calls to `setValueExplicit` and `setValue`, among others, do not handle the exception, because PhpStorm treats `Exception`, by default, as "checked". On the other hand, PhpStorm treats `RunTimeException` as "unchecked" so won't flag such calls. It is reasonable to let `PhpSpreadsheetException` extend `RuntTimeException`, and will eliminate the problem, without having to wrap code in all-but-useless try-catch blocks (e.g. for `setValue`, the code would raise an exception only if you try to set the cell's value to an unstringable object).
  • Loading branch information
@oleibman
oleibman authored Nov 7, 2023
1 parent 98b5c77 commit ef3890a
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 32 deletions.
78 changes: 49 additions & 29 deletions src/PhpSpreadsheet/Cell/Cell.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
namespace PhpOffice\PhpSpreadsheet\Cell;

use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalculationException;
use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError;
use PhpOffice\PhpSpreadsheet\Collection\Cells;
use PhpOffice\PhpSpreadsheet\Exception;
use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException;
use PhpOffice\PhpSpreadsheet\RichText\RichText;
use PhpOffice\PhpSpreadsheet\Shared\Date as SharedDate;
use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
Expand Down Expand Up @@ -76,13 +77,13 @@ class Cell implements Stringable
/**
* Update the cell into the cell collection.
*
* @return $this
* @throws SpreadsheetException
*/
public function updateInCollection(): self
{
$parent = $this->parent;
if ($parent === null) {
throw new Exception('Cannot update when cell is not bound to a worksheet');
throw new SpreadsheetException('Cannot update when cell is not bound to a worksheet');
}
$parent->update($this);

Expand All @@ -101,6 +102,8 @@ public function attach(Cells $parent): void

/**
* Create a new Cell.
*
* @throws SpreadsheetException
*/
public function __construct(mixed $value, ?string $dataType, Worksheet $worksheet)
{
Expand All @@ -117,32 +120,36 @@ public function __construct(mixed $value, ?string $dataType, Worksheet $workshee
}
$this->dataType = $dataType;
} elseif (self::getValueBinder()->bindValue($this, $value) === false) {
throw new Exception('Value could not be bound to cell.');
throw new SpreadsheetException('Value could not be bound to cell.');
}
$this->ignoredErrors = new IgnoredErrors();
}

/**
* Get cell coordinate column.
*
* @throws SpreadsheetException
*/
public function getColumn(): string
{
$parent = $this->parent;
if ($parent === null) {
throw new Exception('Cannot get column when cell is not bound to a worksheet');
throw new SpreadsheetException('Cannot get column when cell is not bound to a worksheet');
}

return $parent->getCurrentColumn();
}

/**
* Get cell coordinate row.
*
* @throws SpreadsheetException
*/
public function getRow(): int
{
$parent = $this->parent;
if ($parent === null) {
throw new Exception('Cannot get row when cell is not bound to a worksheet');
throw new SpreadsheetException('Cannot get row when cell is not bound to a worksheet');
}

return $parent->getCurrentRow();
Expand All @@ -151,9 +158,9 @@ public function getRow(): int
/**
* Get cell coordinate.
*
* @return string
* @throws SpreadsheetException
*/
public function getCoordinate()
public function getCoordinate(): string
{
$parent = $this->parent;
if ($parent !== null) {
Expand All @@ -162,7 +169,7 @@ public function getCoordinate()
$coordinate = null;
}
if ($coordinate === null) {
throw new Exception('Coordinate no longer exists');
throw new SpreadsheetException('Coordinate no longer exists');
}

return $coordinate;
Expand Down Expand Up @@ -216,15 +223,13 @@ protected static function updateIfCellIsTableHeader(?Worksheet $workSheet, self
* @param mixed $value Value
* @param null|IValueBinder $binder Value Binder to override the currently set Value Binder
*
* @throws Exception
*
* @return $this
* @throws SpreadsheetException
*/
public function setValue(mixed $value, ?IValueBinder $binder = null): self
{
$binder ??= self::getValueBinder();
if (!$binder->bindValue($this, $value)) {
throw new Exception('Value could not be bound to cell.');
throw new SpreadsheetException('Value could not be bound to cell.');
}

return $this;
Expand All @@ -241,9 +246,9 @@ public function setValue(mixed $value, ?IValueBinder $binder = null): self
* If you do mismatch value and datatype, then the value you enter may be changed to match the datatype
* that you specify.
*
* @return Cell
* @throws SpreadsheetException
*/
public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE_STRING)
public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE_STRING): self
{
$oldValue = $this->value;

Expand All @@ -265,7 +270,7 @@ public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE
break;
case DataType::TYPE_NUMERIC:
if (is_string($value) && !is_numeric($value)) {
throw new Exception('Invalid numeric value for datatype Numeric');
throw new SpreadsheetException('Invalid numeric value for datatype Numeric');
}
$this->value = 0 + $value;

Expand All @@ -288,7 +293,7 @@ public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE

break;
default:
throw new Exception('Invalid datatype: ' . $dataType);
throw new SpreadsheetException('Invalid datatype: ' . $dataType);
}

// set the datatype
Expand All @@ -313,11 +318,12 @@ public static function getCalculateDateTimeType(): int
return self::$calculateDateTimeType;
}

/** @throws CalculationException*/
public static function setCalculateDateTimeType(int $calculateDateTimeType): void
{
self::$calculateDateTimeType = match ($calculateDateTimeType) {
self::CALCULATE_DATE_TIME_ASIS, self::CALCULATE_DATE_TIME_FLOAT, self::CALCULATE_TIME_FLOAT => $calculateDateTimeType,
default => throw new \PhpOffice\PhpSpreadsheet\Calculation\Exception("Invalid value $calculateDateTimeType for calculated date time type"),
default => throw new CalculationException("Invalid value $calculateDateTimeType for calculated date time type"),
};
}

Expand Down Expand Up @@ -348,9 +354,9 @@ private function convertDateTimeInt(mixed $result)
*
* @param bool $resetLog Whether the calculation engine logger should be reset or not
*
* @return mixed
* @throws CalculationException
*/
public function getCalculatedValue(bool $resetLog = true)
public function getCalculatedValue(bool $resetLog = true): mixed
{
if ($this->dataType === DataType::TYPE_FORMULA) {
try {
Expand All @@ -368,14 +374,14 @@ public function getCalculatedValue(bool $resetLog = true)
$result = array_shift($result);
}
}
} catch (Exception $ex) {
} catch (SpreadsheetException $ex) {
if (($ex->getMessage() === 'Unable to access External Workbook') && ($this->calculatedValue !== null)) {
return $this->calculatedValue; // Fallback for calculations referencing external files.
} elseif (preg_match('/[Uu]ndefined (name|offset: 2|array key 2)/', $ex->getMessage()) === 1) {
return ExcelError::NAME();
}

throw new \PhpOffice\PhpSpreadsheet\Calculation\Exception(
throw new CalculationException(
$this->getWorksheet()->getTitle() . '!' . $this->getCoordinate() . ' -> ' . $ex->getMessage(),
$ex->getCode(),
$ex
Expand Down Expand Up @@ -453,35 +459,41 @@ public function isFormula(): bool

/**
* Does this cell contain Data validation rules?
*
* @throws SpreadsheetException
*/
public function hasDataValidation(): bool
{
if (!isset($this->parent)) {
throw new Exception('Cannot check for data validation when cell is not bound to a worksheet');
throw new SpreadsheetException('Cannot check for data validation when cell is not bound to a worksheet');
}

return $this->getWorksheet()->dataValidationExists($this->getCoordinate());
}

/**
* Get Data validation rules.
*
* @throws SpreadsheetException
*/
public function getDataValidation(): DataValidation
{
if (!isset($this->parent)) {
throw new Exception('Cannot get data validation for cell that is not bound to a worksheet');
throw new SpreadsheetException('Cannot get data validation for cell that is not bound to a worksheet');
}

return $this->getWorksheet()->getDataValidation($this->getCoordinate());
}

/**
* Set Data validation rules.
*
* @throws SpreadsheetException
*/
public function setDataValidation(?DataValidation $dataValidation = null): self
{
if (!isset($this->parent)) {
throw new Exception('Cannot set data validation for cell that is not bound to a worksheet');
throw new SpreadsheetException('Cannot set data validation for cell that is not bound to a worksheet');
}

$this->getWorksheet()->setDataValidation($this->getCoordinate(), $dataValidation);
Expand All @@ -501,35 +513,41 @@ public function hasValidValue(): bool

/**
* Does this cell contain a Hyperlink?
*
* @throws SpreadsheetException
*/
public function hasHyperlink(): bool
{
if (!isset($this->parent)) {
throw new Exception('Cannot check for hyperlink when cell is not bound to a worksheet');
throw new SpreadsheetException('Cannot check for hyperlink when cell is not bound to a worksheet');
}

return $this->getWorksheet()->hyperlinkExists($this->getCoordinate());
}

/**
* Get Hyperlink.
*
* @throws SpreadsheetException
*/
public function getHyperlink(): Hyperlink
{
if (!isset($this->parent)) {
throw new Exception('Cannot get hyperlink for cell that is not bound to a worksheet');
throw new SpreadsheetException('Cannot get hyperlink for cell that is not bound to a worksheet');
}

return $this->getWorksheet()->getHyperlink($this->getCoordinate());
}

/**
* Set Hyperlink.
*
* @throws SpreadsheetException
*/
public function setHyperlink(?Hyperlink $hyperlink = null): self
{
if (!isset($this->parent)) {
throw new Exception('Cannot set hyperlink for cell that is not bound to a worksheet');
throw new SpreadsheetException('Cannot set hyperlink for cell that is not bound to a worksheet');
}

$this->getWorksheet()->setHyperlink($this->getCoordinate(), $hyperlink);
Expand All @@ -549,6 +567,8 @@ public function getParent()

/**
* Get parent worksheet.
*
* @throws SpreadsheetException
*/
public function getWorksheet(): Worksheet
{
Expand All @@ -560,7 +580,7 @@ public function getWorksheet(): Worksheet
}

if ($worksheet === null) {
throw new Exception('Worksheet no longer exists');
throw new SpreadsheetException('Worksheet no longer exists');
}

return $worksheet;
Expand Down
4 changes: 3 additions & 1 deletion src/PhpSpreadsheet/Exception.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace PhpOffice\PhpSpreadsheet;

class Exception extends \Exception
use RuntimeException;

class Exception extends RuntimeException
{
}
4 changes: 2 additions & 2 deletions src/PhpSpreadsheet/Writer/Ods/Content.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

namespace PhpOffice\PhpSpreadsheet\Writer\Ods;

use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalculationException;
use PhpOffice\PhpSpreadsheet\Cell\Cell;
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
use PhpOffice\PhpSpreadsheet\Cell\DataType;
use PhpOffice\PhpSpreadsheet\Shared\XMLWriter;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Worksheet\RowCellIterator;
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
use PhpOffice\PhpSpreadsheet\Writer\Exception;
use PhpOffice\PhpSpreadsheet\Writer\Ods;
use PhpOffice\PhpSpreadsheet\Writer\Ods\Cell\Comment;
use PhpOffice\PhpSpreadsheet\Writer\Ods\Cell\Style;
Expand Down Expand Up @@ -225,7 +225,7 @@ private function writeCells(XMLWriter $objWriter, RowCellIterator $cells): void
if ($this->getParentWriter()->getPreCalculateFormulas()) {
try {
$formulaValue = $cell->getCalculatedValue();
} catch (Exception) {
} catch (CalculationException $e) {
// don't do anything
}
}
Expand Down

0 comments on commit ef3890a

Please sign in to comment.