Skip to content

Commit

Permalink
feat: Do not swallow exceptions when scoping
Browse files Browse the repository at this point in the history
Closes #847.

Afer reviewing the linked issue, I think a better way to handle errors
is to leave alone non valid PHP files (i.e. files that PHP-Parser could
not parse) but otherwise fail.

This can cause problems however, I suspect mostly during tests, where
one may want to try the final scopped result depsite a failure. For this
reason, a new option `--continue-on-failure` has been added to the
`add-prefix` command.

The option `--stop-on-failure` has now been deprecated and will be
removed a future version.
  • Loading branch information
theofidry committed Nov 1, 2023
1 parent 4fdcd61 commit 13d0be4
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 94 deletions.
4 changes: 3 additions & 1 deletion fixtures/set002/original/composer/installed.json
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
{"just here": "for the detection"}
{
"packages": []
}
4 changes: 3 additions & 1 deletion fixtures/set002/scoped/composer/installed.json
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
{"just here": "for the detection"}
{
"packages": []
}
12 changes: 4 additions & 8 deletions src/Console/ConsoleScoper.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use Humbug\PhpScoper\Scoper\ScoperFactory;
use Humbug\PhpScoper\Symbol\SymbolsRegistry;
use Humbug\PhpScoper\Throwable\Exception\ParsingException;
use PhpParser\Error as PhpParserError;
use Symfony\Component\Filesystem\Filesystem;
use Symfony\Component\Filesystem\Path;
use Throwable;
Expand All @@ -33,7 +34,6 @@
use function preg_match as native_preg_match;
use function Safe\file_get_contents;
use function Safe\fileperms;
use function sprintf;
use function str_replace;
use function strlen;
use function usort;
Expand Down Expand Up @@ -230,13 +230,9 @@ private function scopeFile(
$inputFilePath,
$inputContents,
);
} catch (Throwable $throwable) {
$exception = new ParsingException(
sprintf(
'Could not parse the file "%s".',
$inputFilePath,
),
0,
} catch (PhpParserError $throwable) {
$exception = ParsingException::forFile(
$inputFilePath,
$throwable,
);

Expand Down
12 changes: 12 additions & 0 deletions src/Throwable/Exception/ParsingException.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,18 @@

namespace Humbug\PhpScoper\Throwable\Exception;

use Throwable;

final class ParsingException extends RuntimeException
{
public static function forFile(string $filePath, Throwable $previous): self
{
return new self(
sprintf(
'Could not parse the file "%s".',
$filePath,
),
previous: $previous,
);
}
}
16 changes: 2 additions & 14 deletions tests/Console/Command/AddPrefixCommandIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public function test_scope_in_verbose_mode(): void
PhpScoper version TestVersion 28/01/2020
* [NO] /path/to/composer/installed.json
* [OK] /path/to/composer/installed.json
* [OK] /path/to/executable-file.php
* [OK] /path/to/file.php
* [NO] /path/to/invalid-file.php
Expand Down Expand Up @@ -245,19 +245,7 @@ public function test_scope_in_very_verbose_mode(): void
PhpScoper version TestVersion 28/01/2020
* [NO] /path/to/composer/installed.json
Could not parse the file "/path/to/composer/installed.json".: InvalidArgumentException
Stack trace:
#0
#1
#2
#3
#4
#5
#6
#7
#8
#9
* [OK] /path/to/composer/installed.json
* [OK] /path/to/executable-file.php
* [OK] /path/to/file.php
* [NO] /path/to/invalid-file.php
Expand Down
73 changes: 3 additions & 70 deletions tests/Console/Command/AddPrefixCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@
use Humbug\PhpScoper\Symbol\EnrichedReflectorFactory;
use Humbug\PhpScoper\Symbol\Reflector;
use InvalidArgumentException;
use PhpParser\Error as PhpParserError;
use Prophecy\Argument;
use Prophecy\PhpUnit\ProphecyTrait;
use Prophecy\Prophecy\ObjectProphecy;
use RuntimeException as RootRuntimeException;
use Symfony\Component\Console\Exception\RuntimeException;
use Symfony\Component\Console\Tester\ApplicationTester;
use Symfony\Component\Filesystem\Filesystem;
Expand Down Expand Up @@ -104,7 +104,7 @@ public function test_scope_the_given_paths(): void
$this->fileSystemProphecy->remove(Argument::cetera())->shouldNotBeCalled();

$expectedFiles = [
'composer/installed.json' => 'f1',
'composer/installed.json' => '{"packages": []}}',
'executable-file.php' => 'f5',
'file.php' => 'f2',
'invalid-file.php' => 'f3',
Expand Down Expand Up @@ -145,73 +145,6 @@ public function test_scope_the_given_paths(): void
->shouldHaveBeenCalledTimes(count($expectedFiles));
}

public function test_let_the_file_unchanged_when_cannot_scope_a_file(): void
{
$input = [
'add-prefix',
'--prefix' => 'MyPrefix',
'paths' => [
$root = self::FIXTURE_PATH.'/set002/original',
],
'--output-dir' => $this->tmp,
'--no-interaction',
'--no-config' => null,
];

$this->fileSystemProphecy->isAbsolutePath($root)->willReturn(true);
$this->fileSystemProphecy->isAbsolutePath($this->tmp)->willReturn(true);

$this->fileSystemProphecy->mkdir($this->tmp)->shouldBeCalled();
$this->fileSystemProphecy->exists(Argument::cetera())->willReturn(false);
$this->fileSystemProphecy->remove(Argument::cetera())->shouldNotBeCalled();

$expectedFiles = [
'composer/installed.json' => 'f1',
'executable-file.php' => 'f5',
'file.php' => 'f2',
'invalid-file.php' => 'f3',
'scoper.inc.php' => null,
];

$root = realpath($root);

$this->fileSystemProphecy
->chmod(
$this->tmp.'/executable-file.php',
493,
)
->shouldBeCalled();

foreach ($expectedFiles as $expectedFile => $prefixedContents) {
$inputPath = FS::escapePath($root.'/'.$expectedFile);
$outputPath = FS::escapePath($this->tmp.'/'.$expectedFile);

$inputContents = file_get_contents($inputPath);

if (null !== $prefixedContents) {
$this->scoperProphecy
->scope($inputPath, $inputContents)
->willReturn($prefixedContents);

$this->fileSystemProphecy->dumpFile($outputPath, $prefixedContents)->shouldBeCalled();
} else {
$this->scoperProphecy
->scope($inputPath, $inputContents)
->willThrow(new RootRuntimeException('Scoping of the file failed'));

$this->fileSystemProphecy->dumpFile($outputPath, $inputContents)->shouldBeCalled();
}
}

$this->appTester->run($input);

self::assertSame(0, $this->appTester->getStatusCode());

$this->scoperProphecy
->scope(Argument::cetera())
->shouldHaveBeenCalledTimes(count($expectedFiles));
}

public function test_do_not_scope_duplicated_given_paths(): void
{
$input = [
Expand Down Expand Up @@ -582,7 +515,7 @@ public function test_can_scope_projects_with_invalid_files(): void

$this->scoperProphecy
->scope($inputPath, $fileContents)
->willThrow(new RuntimeException('Could not scope file'));
->willThrow(new PhpParserError('Could not scope file'));

$this->fileSystemProphecy->dumpFile($outputPath, $fileContents)->shouldBeCalled();
}
Expand Down

0 comments on commit 13d0be4

Please sign in to comment.