feat: Do not swallow exceptions when scoping (#869)

Closes #847.

As reported in #847, the current strategy is not ideal: if a failure occurs, the file is preserved. You can only see that there was a failure by paying attention to the output or having a more verbose output, unless you pass the `--stop-on-failure` option.

I think historically it has been that way because in the early days of PHP-Scoper, a failure was non infrequent and it was very frustrating to not be able to examine the result despite the failure.

The tool is now more robust and the only exception that I can see regularly coming still is invalid PHP code (e.g. when it is a template). As such, this PR proposes to:

- Leave the file unchanged if a PHP-Parser parsing error occurred (it is still marked as failed and appears in the logs as before).
- Fail on any other failure (i.e. proceed as if `--stop-on-failure` was always provided).
- Introduce a new counterpart option `--continue-on-failure`.

The option `--stop-on-failure` has now been deprecated and will be removed a future version as it is now useless.

Author: theofidry

fixtures/set002/original/composer/installed.json

@@ -1 +1,3 @@
-{"just here": "for the detection"}
+{
+    "packages": []
+}

fixtures/set002/scoped/composer/installed.json

@@ -1 +1,3 @@
-{"just here": "for the detection"}
+{
+    "packages": []
+}

phpstan.neon.dist

@@ -23,8 +23,6 @@ parameters:
           path: 'src/PhpParser/NodeVisitor/Resolver/OriginalNameResolver.php'
         - message: '#NamespaceManipulator::getOriginalName\(\) should return#'
           path: 'src/PhpParser/NodeVisitor/NamespaceStmt/NamespaceManipulator.php'
-        - message: '#Dead catch#'
-          path: 'tests/Scoper/PhpScoperTest.php'
         - message: '#Dead catch#'
           path: 'tests/Scoper/PhpScoperSpecTest.php'
         - message: '#DummyScoperFactory extends @final#'

src/Console/Command/AddPrefixCommand.php

@@ -51,6 +51,7 @@ final class AddPrefixCommand implements Command, CommandAware
     private const OUTPUT_DIR_OPT = 'output-dir';
     private const FORCE_OPT = 'force';
     private const STOP_ON_FAILURE_OPT = 'stop-on-failure';
+    private const CONTINUE_ON_FAILURE_OPT = 'continue-on-failure';
     private const CONFIG_FILE_OPT = 'config';
     private const NO_CONFIG_OPT = 'no-config';
 
@@ -107,6 +108,12 @@ public function getConfiguration(): CommandConfiguration
                     InputOption::VALUE_NONE,
                     'Stops on failure.',
                 ),
+                new InputOption(
+                    self::CONTINUE_ON_FAILURE_OPT,
+                    null,
+                    InputOption::VALUE_NONE,
+                    'Continue on non PHP-Parser parsing failures.',
+                ),
                 new InputOption(
                     self::CONFIG_FILE_OPT,
                     'c',
@@ -150,12 +157,40 @@ public function execute(IO $io): int
             $config,
             $paths,
             $outputDir,
-            $io->getOption(self::STOP_ON_FAILURE_OPT)->asBoolean(),
+            self::getStopOnFailure($io),
         );
 
         return ExitCode::SUCCESS;
     }
 
+    private static function getStopOnFailure(IO $io): bool
+    {
+        $stopOnFailure = $io->getOption(self::STOP_ON_FAILURE_OPT)->asBoolean();
+        $continueOnFailure = $io->getOption(self::CONTINUE_ON_FAILURE_OPT)->asBoolean();
+
+        if ($stopOnFailure) {
+            $io->info(
+                sprintf(
+                    'Using the option "%s" is now deprecated. Any non PHP-Parser parsing error will now halt the process. To continue upon non-PHP-Parser parsing errors, use the option "%s".',
+                    self::STOP_ON_FAILURE_OPT,
+                    self::CONTINUE_ON_FAILURE_OPT,
+                ),
+            );
+        }
+
+        if (true === $stopOnFailure && true === $continueOnFailure) {
+            $io->info(
+                sprintf(
+                    'Only one of the two options "%s" and "%s" can be used at the same time.',
+                    self::STOP_ON_FAILURE_OPT,
+                    self::CONTINUE_ON_FAILURE_OPT,
+                ),
+            );
+        }
+
+        return $stopOnFailure;
+    }
+
     /**
      * @return non-empty-string
      */

src/Console/ConsoleScoper.php

@@ -33,7 +33,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;
@@ -225,18 +224,23 @@ private function scopeFile(
         bool $stopOnFailure,
         ScoperLogger $logger
     ): void {
+        $successfullyScoped = false;
+
         try {
             $scoppedContent = $scoper->scope(
                 $inputFilePath,
                 $inputContents,
             );
+
+            $successfullyScoped = true;
+        } catch (ParsingException $parsingException) {
+            $logger->outputWarnOfFailure($inputFilePath, $parsingException);
+
+            // Fallback on unchanged content
+            $scoppedContent = file_get_contents($inputFilePath);
         } catch (Throwable $throwable) {
-            $exception = new ParsingException(
-                sprintf(
-                    'Could not parse the file "%s".',
-                    $inputFilePath,
-                ),
-                0,
+            $exception = ParsingException::forFile(
+                $inputFilePath,
                 $throwable,
             );
 
@@ -256,7 +260,7 @@ private function scopeFile(
             $outputFilePath,
         );
 
-        if (!isset($exception)) {
+        if ($successfullyScoped) {
             $logger->outputSuccess($inputFilePath);
         }
     }

src/Scoper/PhpScoper.php

@@ -16,6 +16,7 @@
 
 use Humbug\PhpScoper\PhpParser\Printer\Printer;
 use Humbug\PhpScoper\PhpParser\TraverserFactory;
+use Humbug\PhpScoper\Throwable\Exception\ParsingException;
 use PhpParser\Error as PhpParserError;
 use PhpParser\Lexer;
 use PhpParser\Parser;
@@ -42,16 +43,18 @@ public function __construct(
 
     /**
      * Scopes PHP files.
-     *
-     * @throws PhpParserError
      */
     public function scope(string $filePath, string $contents): string
     {
         if (!self::isPhpFile($filePath, $contents)) {
             return $this->decoratedScoper->scope(...func_get_args());
         }
 
-        return $this->scopePhp($contents);
+        try {
+            return $this->scopePhp($contents);
+        } catch (PhpParserError $parsingException) {
+            throw ParsingException::forFile($filePath, $parsingException);
+        }
     }
 
     public function scopePhp(string $php): string

src/Throwable/Exception/ParsingException.php

@@ -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,
+        );
+    }
 }

tests/Console/Command/AddPrefixCommandIntegrationTest.php

@@ -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
@@ -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

tests/Console/Command/AddPrefixCommandTest.php

@@ -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;
@@ -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',
@@ -145,7 +145,7 @@ public function test_scope_the_given_paths(): void
             ->shouldHaveBeenCalledTimes(count($expectedFiles));
     }
 
-    public function test_let_the_file_unchanged_when_cannot_scope_a_file(): void
+    public function test_let_the_file_unchanged_when_cannot_scope_a_file_but_is_marked_as_continue_on_failure(): void
     {
         $input = [
             'add-prefix',
@@ -156,6 +156,7 @@ public function test_let_the_file_unchanged_when_cannot_scope_a_file(): void
             '--output-dir' => $this->tmp,
             '--no-interaction',
             '--no-config' => null,
+            '--continue-on-failure' => null,
         ];
 
         $this->fileSystemProphecy->isAbsolutePath($root)->willReturn(true);
@@ -197,7 +198,7 @@ public function test_let_the_file_unchanged_when_cannot_scope_a_file(): void
             } else {
                 $this->scoperProphecy
                     ->scope($inputPath, $inputContents)
-                    ->willThrow(new RootRuntimeException('Scoping of the file failed'));
+                    ->willThrow(new RuntimeException('Scoping of the file failed'));
 
                 $this->fileSystemProphecy->dumpFile($outputPath, $inputContents)->shouldBeCalled();
             }
@@ -582,7 +583,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();
         }

tests/Scoper/PhpScoperTest.php

@@ -23,6 +23,7 @@
 use Humbug\PhpScoper\Symbol\EnrichedReflector;
 use Humbug\PhpScoper\Symbol\Reflector;
 use Humbug\PhpScoper\Symbol\SymbolsRegistry;
+use Humbug\PhpScoper\Throwable\Exception\ParsingException;
 use LogicException;
 use PhpParser\Error as PhpParserError;
 use PhpParser\Lexer;
@@ -267,18 +268,14 @@ public function test_cannot_scope_an_invalid_php_file(): void
 
             PHP;
 
-        try {
-            $this->scoper->scope($filePath, $contents);
-
-            self::fail('Expected exception to have been thrown.');
-        } catch (PhpParserError $error) {
-            self::assertEquals(
-                'Syntax error, unexpected \';\' on line 3',
-                $error->getMessage(),
-            );
-            self::assertSame(0, $error->getCode());
-            self::assertNull($error->getPrevious());
-        }
+        $this->expectExceptionObject(
+            new ParsingException(
+                'Could not parse the file "invalid-file.php".',
+                previous: new PhpParserError('Syntax error, unexpected \';\' on line 3'),
+            ),
+        );
+
+        $this->scoper->scope($filePath, $contents);
     }
 
     public function test_creates_a_new_traverser_for_each_file(): void