Remove the last intermediate throws and guard against new ones

.github/workflows/throws.yaml

@@ -24,7 +24,7 @@ jobs:
               run: composer update --prefer-dist --no-progress --no-suggest
             - name: Run the throw counting tests
               run: composer test:throws
-            - name: Report intermediate throws over the whole suite
+            - name: Report and fail on intermediate throws over the whole suite
               run: |
                   status=0
                   composer throws:report > throws-report.txt 2>&1 || status=$?

composer.json

@@ -36,7 +36,7 @@
         "php-standard-library/vec": "^6.1",
         "veewee/reflecta": "^1.1",
         "veewee/xml": "^4.10",
-        "php-soap/engine": "^2.20",
+        "php-soap/engine": "^2.21",
         "php-soap/wsdl": "^1.19",
         "php-soap/xml": "^1.10",
         "php-soap/wsdl-reader": "~0.32"

src/Encoder/SoapEnc/SoapArrayAccess.php

@@ -11,7 +11,6 @@
 use Soap\WsdlReader\Model\Definitions\BindingUse;
 use Soap\WsdlReader\Parser\Xml\QnameParser;
 use Soap\Xml\Xmlns;
-use function Psl\Result\try_catch;
 
 final class SoapArrayAccess
 {
@@ -38,16 +37,14 @@ public static function forContext(Context $context): self
         $itemNodeName = $meta->arrayNodeName()->unwrapOr(null);
         $xsiType = ltrim($nodePrefix . ':' . $nodeType, ':');
 
-        $xsdType = try_catch(
-            static fn (): XsdType => $context->metadata->getTypes()
-                ->fetchByNameAndXmlNamespace($nodeType, $namespace)
-                ->getXsdType(),
-            static fn (): XsdType => XsdType::any()
+        $xsdType = $context->metadata->getTypes()
+            ->findByNameAndXmlNamespace($nodeType, $namespace)
+            ?->getXsdType()
+            ?? XsdType::any()
                 ->copy($nodeType)
                 ->withXmlTypeName($nodeType)
                 ->withXmlNamespace($namespace)
-                ->withMeta(static fn (TypeMeta $meta): TypeMeta => $meta->withIsElement(true))
-        );
+                ->withMeta(static fn (TypeMeta $meta): TypeMeta => $meta->withIsElement(true));
 
         if ($context->bindingUse === BindingUse::ENCODED || $itemNodeName !== null) {
             $xsdType = $xsdType->withXmlTargetNodeName($itemNodeName ?? 'item');

tests/Integration/NoIntermediateThrowsTest.php

@@ -16,7 +16,6 @@
 use Soap\Encoding\Test\Unit\ContextCreatorTrait;
 use Soap\Engine\Metadata\Model\TypeMeta;
 use Soap\Engine\Metadata\Model\XsdType;
-use function in_array;
 use function var_export;
 
 #[CoversNothing]
@@ -26,13 +25,6 @@ final class NoIntermediateThrowsTest extends TestCase
     use AssertsNoThrows;
     use ContextCreatorTrait;
 
-    /**
-     * The engine type lookup for soap-enc arrays of a builtin item type still throws.
-     */
-    private const STILL_THROWING = [
-        'decode soap-enc array of a builtin type',
-    ];
-
     /**
      * @param Closure(): mixed $operation
      */
@@ -97,10 +89,6 @@ public static function provideUntypedDecodes(): iterable
      */
     public static function provideScenarios(): iterable
     {
-        foreach (IntermediateThrowScenariosTest::provideScenarios() as $name => $scenario) {
-            if (!in_array($name, self::STILL_THROWING, true)) {
-                yield $name => $scenario;
-            }
-        }
+        return IntermediateThrowScenariosTest::provideScenarios();
     }
 }

tests/Support/Throws/AllowedThrows.php

@@ -0,0 +1,95 @@
+<?php
+declare(strict_types=1);
+
+namespace Soap\Encoding\Test\Support\Throws;
+
+use Throwable;
+use function min;
+use function sprintf;
+
+/**
+ * The intermediate throws the suite makes on purpose, listed per test and site in allowed-throws.php.
+ * Each entry allows exactly the listed number of throws, so a new throw in the same test still counts.
+ * The test is PHPUnit's test id, so a test with a data provider is listed as Class::method#data set name.
+ *
+ * @psalm-type Entry = array{test: string, throw: class-string<Throwable>, via: string, throws: int, reason: string}
+ */
+final class AllowedThrows
+{
+    /** @var array<string, Entry> */
+    private array $entries = [];
+    /** @var array<string, int> */
+    private array $seen = [];
+
+    /**
+     * @param list<Entry> $entries
+     */
+    public function __construct(array $entries)
+    {
+        foreach ($entries as $entry) {
+            $this->entries[self::key($entry['test'], $entry['throw'], $entry['via'])] = $entry;
+        }
+    }
+
+    public static function fromFile(): self
+    {
+        /** @var list<Entry> $entries */
+        $entries = require __DIR__.'/allowed-throws.php';
+
+        return new self($entries);
+    }
+
+    public function allows(string $test, ThrowEvent $event): bool
+    {
+        $key = self::key($test, $event->class, $event->srcCall() ?? '');
+        if (!isset($this->entries[$key])) {
+            return false;
+        }
+
+        $this->seen[$key] = ($this->seen[$key] ?? 0) + 1;
+
+        return $this->seen[$key] <= $this->entries[$key]['throws'];
+    }
+
+    public function allowed(): int
+    {
+        $allowed = 0;
+        foreach ($this->seen as $key => $seen) {
+            $allowed += min($seen, $this->entries[$key]['throws']);
+        }
+
+        return $allowed;
+    }
+
+    /**
+     * Entries whose test ran but threw less often than listed, so the list can shrink.
+     *
+     * @param array<string, true> $ranTests
+     *
+     * @return list<string>
+     */
+    public function unused(array $ranTests): array
+    {
+        $unused = [];
+        foreach ($this->entries as $key => $entry) {
+            $seen = $this->seen[$key] ?? 0;
+            if (isset($ranTests[$entry['test']]) && $seen < $entry['throws']) {
+                $unused[] = sprintf(
+                    '%s: %s via %s threw %d of %d times',
+                    $entry['test'],
+                    $entry['throw'],
+                    $entry['via'],
+                    $seen,
+                    $entry['throws'],
+                );
+            }
+        }
+
+        return $unused;
+    }
+
+    private static function key(string $test, string $throw, string $via): string
+    {
+        return $test."\0".$throw."\0".$via;
+    }
+}

tests/Support/Throws/ThrowAttribution.php

@@ -6,6 +6,7 @@
 use PHPUnit\Event\Code\Test;
 use PHPUnit\Event\Code\TestMethod;
 use ReflectionMethod;
+use function array_keys;
 use function array_slice;
 use function count;
 use function file;
@@ -16,16 +17,20 @@
 
 /**
  * Attributes the throws of a suite run to their sites, keeping only the intermediate ones:
- * thrown while the stack passes through src/, in a test that does not expect an exception.
+ * thrown while the stack passes through src/, in a test that does not expect an exception
+ * and not listed as deliberate in allowed-throws.php.
  */
 final class ThrowAttribution
 {
+    private const SHOWN_TESTS_PER_SITE = 3;
+
     /** @var array<string, string>|null */
     private ?array $previousIni = null;
     private string $currentTest = '';
     private ?string $currentExclusion = null;
     private int $all = 0;
     private int $outsideSrc = 0;
+    private int $intermediate = 0;
     /** @var array{expectException: int, catch: int} */
     private array $excluded = ['expectException' => 0, 'catch' => 0];
     /** @var array<string, list<string>> */
@@ -36,10 +41,18 @@ final class ThrowAttribution
     private array $tests = [];
     /** @var array<string, true> */
     private array $unsuccessfulTests = [];
+    /** @var array<string, true> */
+    private array $ranTests = [];
+
+    public function __construct(
+        private readonly AllowedThrows $allowedThrows,
+    ) {
+    }
 
     public function testStarted(Test $test): void
     {
         $this->currentTest = $test->id();
+        $this->ranTests[$this->currentTest] = true;
         $this->currentExclusion = $test instanceof TestMethod ? $this->exclusion($test) : null;
         $this->previousIni ??= ThrowCounter::enable();
     }
@@ -56,7 +69,11 @@ public function testPrinted(string $output): void
                 $this->outsideSrc++;
                 continue;
             }
+            if ($this->allowedThrows->allows($this->currentTest, $event)) {
+                continue;
+            }
 
+            $this->intermediate++;
             $site = $event->describe();
             $this->sites[$site] ??= ['throws' => 0, 'tests' => []];
             $this->sites[$site]['throws']++;
@@ -83,32 +100,50 @@ public function stopTracing(): void
         }
     }
 
+    /**
+     * True when the run has intermediate throws, allowlist entries that no longer throw as listed,
+     * or failed tests, whose throws cannot be trusted.
+     */
+    public function failed(): bool
+    {
+        return $this->intermediate > 0
+            || $this->allowedThrows->unused($this->ranTests) !== []
+            || $this->unsuccessfulTests !== [];
+    }
+
     public function render(): string
     {
         $sites = $this->sites;
         uasort($sites, static fn (array $a, array $b): int => $b['throws'] <=> $a['throws']);
 
         $lines = [
+            sprintf('Intermediate throws: %d in %d tests', $this->intermediate, count($this->tests)),
             sprintf(
-                'Intermediate throws: %d in %d tests',
-                $this->all - $this->outsideSrc - $this->excluded['expectException'] - $this->excluded['catch'],
-                count($this->tests),
-            ),
-            sprintf(
-                'Not counted: %d outside src/, %d in tests using expectException(), %d in tests catching an exception themselves (%d throws in total)',
+                'Not counted: %d outside src/, %d in tests using expectException(), %d in tests catching an exception themselves, %d allowed in tests/Support/Throws/allowed-throws.php (%d throws in total)',
                 $this->outsideSrc,
                 $this->excluded['expectException'],
                 $this->excluded['catch'],
+                $this->allowedThrows->allowed(),
                 $this->all,
             ),
         ];
         if ($this->unsuccessfulTests !== []) {
             $lines[] = sprintf('Warning: %d tests failed or errored, run composer test to see them', count($this->unsuccessfulTests));
         }
+        foreach ($this->allowedThrows->unused($this->ranTests) as $unused) {
+            $lines[] = 'Allowed throw no longer thrown as listed, update allowed-throws.php: '.$unused;
+        }
         $lines[] = '';
         $lines[] = sprintf('%7s %6s  %s', 'throws', 'tests', 'site');
         foreach ($sites as $site => $count) {
             $lines[] = sprintf('%7d %6d  %s', $count['throws'], count($count['tests']), $site);
+            $tests = array_keys($count['tests']);
+            foreach (array_slice($tests, 0, self::SHOWN_TESTS_PER_SITE) as $test) {
+                $lines[] = sprintf('%15s  %s', '', $test);
+            }
+            if (count($tests) > self::SHOWN_TESTS_PER_SITE) {
+                $lines[] = sprintf('%15s  and %d more', '', count($tests) - self::SHOWN_TESTS_PER_SITE);
+            }
         }
 
         return implode("\n", $lines)."\n";

tests/Support/Throws/ThrowAttributionExtension.php

@@ -3,6 +3,8 @@
 
 namespace Soap\Encoding\Test\Support\Throws;
 
+use PHPUnit\Event\Application\Finished as ApplicationFinished;
+use PHPUnit\Event\Application\FinishedSubscriber as ApplicationFinishedSubscriber;
 use PHPUnit\Event\Test\Errored;
 use PHPUnit\Event\Test\ErroredSubscriber;
 use PHPUnit\Event\Test\Failed;
@@ -24,7 +26,7 @@
 use RuntimeException;
 
 /**
- * Prints the intermediate throws of a suite run per site, see composer throws:report.
+ * Prints the intermediate throws of a suite run per site and fails the run when there are any, see composer throws:report.
  *
  * Xdebug prints each exception trace into the output buffer of the running test, so every test that throws
  * counts as printing output. The composer script runs with --no-results and --do-not-fail-on-risky for that
@@ -38,7 +40,7 @@ public function bootstrap(Configuration $configuration, Facade $facade, Paramete
             throw new RuntimeException('The throws report needs Xdebug with xdebug.mode=develop, run composer throws:report.');
         }
 
-        $attribution = new ThrowAttribution();
+        $attribution = new ThrowAttribution(AllowedThrows::fromFile());
 
         $facade->registerSubscribers(
             new class($attribution) implements PreparationStartedSubscriber {
@@ -112,6 +114,19 @@ public function notify(ExecutionFinished $event): void
                     print "\n".$this->attribution->render();
                 }
             },
+            new class($attribution) implements ApplicationFinishedSubscriber {
+                public function __construct(private readonly ThrowAttribution $attribution)
+                {
+                }
+
+                public function notify(ApplicationFinished $event): void
+                {
+                    // An extension cannot change PHPUnit's exit code, and this is the last event before PHPUnit exits.
+                    if ($event->shellExitCode() === 0 && $this->attribution->failed()) {
+                        exit(1);
+                    }
+                }
+            },
         );
     }
 }

tests/Support/Throws/ThrowEvent.php

@@ -40,9 +40,31 @@ public function srcCaller(): ?string
             return $this->site();
         }
 
+        $frame = $this->srcFrame();
+
+        return $frame !== null ? self::relative($frame->file).':'.$frame->line.' -> '.$frame->function : null;
+    }
+
+    /**
+     * The innermost file in src/ that led to this throw and the function it called there, without line numbers,
+     * so it stays the same when unrelated lines in that file move.
+     */
+    public function srcCall(): ?string
+    {
+        if (self::isSrc($this->file)) {
+            return self::relative($this->file);
+        }
+
+        $frame = $this->srcFrame();
+
+        return $frame !== null ? self::relative($frame->file).' -> '.$frame->function : null;
+    }
+
+    private function srcFrame(): ?ThrowFrame
+    {
         foreach (array_reverse($this->frames) as $frame) {
             if (self::isSrc($frame->file)) {
-                return self::relative($frame->file).':'.$frame->line.' -> '.$frame->function;
+                return $frame;
             }
         }