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

feature: streamline exceptions #87

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 6 additions & 5 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ includes:
- /tools/.composer/vendor-bin/phpstan/vendor/phpstan/phpstan-deprecation-rules/rules.neon
- /tools/.composer/vendor-bin/phpstan/vendor/phpstan/phpstan-strict-rules/rules.neon
- /tools/.composer/vendor-bin/phpstan/vendor/phpstan/phpstan-phpunit/extension.neon
# - /tools/.composer/vendor-bin/phpstan/vendor/pepakriz/phpstan-exception-rules/extension.neon
- /tools/.composer/vendor-bin/phpstan/vendor/pepakriz/phpstan-exception-rules/extension.neon
parameters:
tmpDir: %currentWorkingDirectory%/var/phpstan
level: max
Expand All @@ -12,10 +12,8 @@ parameters:
- var/
- vendor/
exceptionRules:
uncheckedExceptions:
- LogicException
- PHPUnit\Framework\Exception
- PHPUnit\Framework\MockObject\RuntimeException
checkedExceptions:
- RuntimeException
ignoreErrors:
- message: '#Access to an undefined property object\:\:\$scalar\.#'
path: %currentWorkingDirectory%/tests/Xezilaires/FilterIteratorTest.php
Expand All @@ -25,3 +23,6 @@ parameters:
paths:
- %currentWorkingDirectory%/tests/Xezilaires/Bridge/Spout/SpreadsheetTest.php
- %currentWorkingDirectory%/tests/Xezilaires/Functional/FunctionalTestCase.php
- message: '#Missing @throws .* annotation#'
paths:
- %currentWorkingDirectory%/tests
36 changes: 20 additions & 16 deletions psalm.xml.dist
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
<?xml version="1.0"?>
<psalm xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
<psalm xmlns="https://getpsalm.org/schema/config"
cacheDirectory="var/psalm"
strictBinaryOperands="false"
checkForThrowsDocblock="true"
totallyTyped="true">

<projectFiles>
<directory name="bin"/>
<directory name="bin/"/>
<directory name="docs/"/>
<directory name="src/"/>
<directory name="tests/"/>
<ignoreFiles>
Expand All @@ -18,37 +17,42 @@
<issueHandlers>
<DeprecatedMethod>
<errorLevel type="info">
<file name="src/Xezilaires/Metadata/Annotation/AnnotationDriver.php" />
</errorLevel>
<file name="src/Xezilaires/Metadata/Annotation/AnnotationDriver.php"/>
</errorLevel>
</DeprecatedMethod>
<InvalidCatch>
<errorLevel type="info">
<file name="src/Xezilaires/Metadata/Mapping.php" />
<file name="src/Xezilaires/Metadata/Mapping.php"/>
</errorLevel>
</InvalidCatch>
<MissingConstructor>
<errorLevel type="info">
<directory name="src/Xezilaires/Annotation/" />
<directory name="tests/Xezilaires/Model/" />
<directory name="src/Xezilaires/Annotation/"/>
<directory name="tests/Xezilaires/Model/"/>
</errorLevel>
</MissingConstructor>
<MissingThrowsDocblock>
<errorLevel type="info">
<directory name="tests/"/>
</errorLevel>
</MissingThrowsDocblock>
<RedundantCondition>
<errorLevel type="info">
<file name="src/Xezilaires/Metadata/Annotation/AnnotationDriver.php" />
<file name="src/Xezilaires/Metadata/Annotation/AnnotationDriver.php"/>
</errorLevel>
</RedundantCondition>
<PropertyNotSetInConstructor>
<errorLevel type="info">
<directory name="tests/Xezilaires/" />
<directory name="tests/Xezilaires/"/>
</errorLevel>
</PropertyNotSetInConstructor>

<!-- false positives -->
<MixedMethodCall>
<errorLevel type="info">
<file name="tests/Xezilaires/Bridge/PhpSpreadsheet/RowIteratorTest.php" />
<file name="tests/Xezilaires/Bridge/Spout/RowIteratorTest.php" />
<file name="tests/Xezilaires/SpreadsheetIteratorTest.php" />
<file name="tests/Xezilaires/Bridge/PhpSpreadsheet/RowIteratorTest.php"/>
<file name="tests/Xezilaires/Bridge/Spout/RowIteratorTest.php"/>
<file name="tests/Xezilaires/SpreadsheetIteratorTest.php"/>
</errorLevel>
</MixedMethodCall>
</issueHandlers>
Expand Down
29 changes: 13 additions & 16 deletions src/Xezilaires/Bridge/PhpSpreadsheet/Spreadsheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ public function __construct(\SplFileObject $file)
$this->file = $file;
}

/**
* {@inheritdoc}
*/
public function createIterator(int $startRowIndex): void
{
if (null !== $this->iterator) {
Expand All @@ -63,9 +60,6 @@ public function createIterator(int $startRowIndex): void
$this->iterator = new RowIterator($sheet->getRowIterator($startRowIndex));
}

/**
* {@inheritdoc}
*/
public function getIterator(): Iterator
{
if (null === $this->iterator) {
Expand All @@ -75,9 +69,6 @@ public function getIterator(): Iterator
return $this->iterator;
}

/**
* {@inheritdoc}
*/
public function getCurrentRow(): array
{
/** @var Row $row */
Expand All @@ -86,9 +77,6 @@ public function getCurrentRow(): array
return $this->getRow($row->getRowIndex());
}

/**
* {@inheritdoc}
*/
public function getRow(int $rowIndex): array
{
$data = [];
Expand All @@ -103,14 +91,14 @@ public function getRow(int $rowIndex): array
return $data;
}

/**
* {@inheritdoc}
*/
public function getHighestRow(): int
{
return $this->getActiveWorksheet()->getHighestRow();
}

/**
* @throws SpreadsheetException
*/
private function getSpreadsheet(): PhpSpreadsheet
{
if (null === $this->spreadsheet) {
Expand All @@ -130,6 +118,9 @@ private function getSpreadsheet(): PhpSpreadsheet
return $this->spreadsheet;
}

/**
* @throws SpreadsheetException
*/
private function getActiveWorksheet(): Worksheet
{
try {
Expand All @@ -140,12 +131,18 @@ private function getActiveWorksheet(): Worksheet
}

/**
* @throws SpreadsheetException
*
* @return null|float|int|string
*/
private function fetchCell(string $columnName, int $rowIndex)
{
$worksheet = $this->getActiveWorksheet();
$columnIndex = Coordinate::columnIndexFromString($columnName);
try {
$columnIndex = Coordinate::columnIndexFromString($columnName);
} catch (\Exception $exception) {
throw SpreadsheetException::invalidCell($exception);
}

/** @var null|Cell $cell */
$cell = $worksheet->getCellByColumnAndRow($columnIndex, $rowIndex, self::CELL_NO_AUTO_CREATE);
Expand Down
21 changes: 6 additions & 15 deletions src/Xezilaires/Bridge/Spout/Spreadsheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,6 @@ public function __construct(\SplFileObject $file)
$this->file = $file;
}

/**
* {@inheritdoc}
*/
public function createIterator(int $startRowIndex): void
{
if (null !== $this->iterator) {
Expand All @@ -68,9 +65,6 @@ public function createIterator(int $startRowIndex): void
$this->iterator = new RowIterator($iterator, $startRowIndex);
}

/**
* {@inheritdoc}
*/
public function getIterator(): Iterator
{
if (null === $this->iterator) {
Expand All @@ -80,9 +74,6 @@ public function getIterator(): Iterator
return $this->iterator;
}

/**
* {@inheritdoc}
*/
public function getRow(int $rowIndex): array
{
$iterator = $this->getIterator();
Expand All @@ -94,9 +85,6 @@ public function getRow(int $rowIndex): array
return $row;
}

/**
* {@inheritdoc}
*/
public function getCurrentRow(): array
{
/** @var \ArrayObject $rowArrayObject */
Expand All @@ -118,9 +106,6 @@ public function getCurrentRow(): array
return $row;
}

/**
* {@inheritdoc}
*/
public function getHighestRow(): int
{
if (null === $this->iterator) {
Expand Down Expand Up @@ -151,6 +136,9 @@ private static function stringFromColumnIndex(int $columnIndex): string
return self::$indexCache[$columnIndex];
}

/**
* @throws SpreadsheetException
*/
private function getReader(): ReaderInterface
{
if (null === $this->reader) {
Expand All @@ -170,6 +158,9 @@ private function getReader(): ReaderInterface
return $this->reader;
}

/**
* @throws SpreadsheetException
*/
private function getActiveWorksheet(): SheetInterface
{
try {
Expand Down
13 changes: 8 additions & 5 deletions src/Xezilaires/Bridge/Symfony/Command/SerializeCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
namespace Xezilaires\Bridge\Symfony\Command;

use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Exception\InvalidArgumentException;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
Expand Down Expand Up @@ -42,6 +43,7 @@ final class SerializeCommand extends Command

public function __construct(SpreadsheetIteratorFactory $iteratorFactory, Serializer $serializer)
{
/** @psalm-suppress MissingThrowsDocblock */
parent::__construct('xezilaires:serialize');

$this->setDescription('Serialize Excel files into JSON, XML, CSV');
Expand All @@ -51,7 +53,7 @@ public function __construct(SpreadsheetIteratorFactory $iteratorFactory, Seriali
}

/**
* {@inheritdoc}
* @throws InvalidArgumentException
*/
protected function configure(): void
{
Expand All @@ -65,7 +67,7 @@ protected function configure(): void
}

/**
* @throws \ReflectionException
* @throws InvalidArgumentException
* @throws \RuntimeException
* @throws \ReflectionException
*/
Expand All @@ -86,22 +88,23 @@ protected function execute(InputInterface $input, OutputInterface $output): ?int
$xmlRoot = $input->getOption('xml-root');

if (null === $format) {
throw new \RuntimeException('Format is required');
throw new \UnexpectedValueException('Format is required');
}

$context = [];
switch ($format) {
case 'csv':
if (false === class_exists(CsvEncoder::class)) {
throw new \RuntimeException('CSV format is only available with Symfony 4.0+');
/** @psalm-suppress MissingThrowsDocblock */
throw new \LogicException('CSV format is only available with Symfony 4.0+');
}
break;
case 'json':
$context['json_encode_options'] = JSON_PRETTY_PRINT;
break;
case 'xml':
if (null === $xmlRoot) {
throw new \RuntimeException('XML root node name cannot be empty if XML format requested');
throw new \UnexpectedValueException('XML root node name cannot be empty if XML format requested');
}
$context['xml_root_node_name'] = $xmlRoot;
break;
Expand Down
2 changes: 1 addition & 1 deletion src/Xezilaires/Exception/AnnotationException.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use Doctrine\Common\Annotations\AnnotationException as DoctrineAnnotationException;
use Xezilaires\Exception;

final class AnnotationException extends \InvalidArgumentException implements Exception
final class AnnotationException extends \UnexpectedValueException implements Exception
{
public static function unsupportedAnnotation(): self
{
Expand Down
2 changes: 1 addition & 1 deletion src/Xezilaires/Exception/DenormalizerException.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use Xezilaires\Bridge\Symfony\Serializer\Exception as SerializerException;
use Xezilaires\Exception;

final class DenormalizerException extends \InvalidArgumentException implements Exception
final class DenormalizerException extends \UnexpectedValueException implements Exception
{
public static function denormalizationFailed(SerializerException $exception): self
{
Expand Down
2 changes: 1 addition & 1 deletion src/Xezilaires/Exception/MappingException.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use Symfony\Component\OptionsResolver\Exception\ExceptionInterface;
use Xezilaires\Exception;

final class MappingException extends \InvalidArgumentException implements Exception
final class MappingException extends \UnexpectedValueException implements Exception
{
public static function missingHeaderOption(): self
{
Expand Down
2 changes: 1 addition & 1 deletion src/Xezilaires/Exception/NestableIteratorException.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

use Xezilaires\Exception;

final class NestableIteratorException extends \InvalidArgumentException implements Exception
final class NestableIteratorException extends \UnexpectedValueException implements Exception
{
public static function iteratorMustBeNestable(): self
{
Expand Down
2 changes: 1 addition & 1 deletion src/Xezilaires/Exception/SpreadsheetException.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

use Xezilaires\Exception;

final class SpreadsheetException extends \InvalidArgumentException implements Exception
final class SpreadsheetException extends \UnexpectedValueException implements Exception
{
public static function noSpreadsheetFound(): self
{
Expand Down
2 changes: 1 addition & 1 deletion src/Xezilaires/Metadata/Annotation/AnnotationDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ final class AnnotationDriver
public function __construct(AnnotationReader $reader = null)
{
if (false === class_exists(AnnotationReader::class)) {
throw new \RuntimeException('Xezilaires annotations support requires Doctrine Annotations component. Install "doctrine/annotations" to use it.');
throw new \LogicException('Xezilaires annotations support requires Doctrine Annotations component. Install "doctrine/annotations" to use it.');
}
AnnotationRegistry::registerUniqueLoader('class_exists');

Expand Down
Loading