Warn on a helper-derived property id; reject DSN credentials

CHANGELOG.md

@@ -1,5 +1,19 @@
 # Changelog
 
+## 0.5.2 — 2026-08-20
+
+- `forAll()` called from a helper (not directly in the test method) now warns
+  on stderr, the way a closure-derived id already did. Its id names the helper,
+  so every test using that helper shares one corpus entry and overwrites the
+  others' counterexample — a stable-looking id that is silently wrong. Pin it
+  with `->id()` to silence the warning.
+- `PROPERTY_DB` with credentials in its userinfo (`redis://user:pass@host`) is
+  rejected instead of silently dropped — `parse_url` would discard them and the
+  connection would go without AUTH. The error never echoes the DSN.
+- The resolved corpus is memoized per `PROPERTY_DB` value, so a suite sharing a
+  Redis corpus builds one client (and opens one connection) rather than one per
+  property. Mirrors the Testo adapter.
+
 ## 0.5.1 — 2026-08-20
 
 - `PROPERTY_DB` with a non-`redis` URI scheme is now a configuration error

README.md

@@ -233,7 +233,9 @@ deleted when the job ends. The Redis form is the same corpus, in the same
 document, shared. It needs `ext-redis` or `predis/predis`; neither installed is
 an error rather than a silent fall back to the filesystem. A `PROPERTY_DB` with
 any other scheme — a `rediss://` typo, another backend — is likewise an error,
-never a directory named after the scheme.
+never a directory named after the scheme. Credentials in the DSN
+(`redis://user:pass@host`) are rejected rather than silently dropped; configure
+Redis AUTH out of band.
 
 The corpus format is exactly the one `rasuvaeff/property-testing` 2.8 wrote —
 a corpus recorded under Testo (or under 2.x) replays here and vice versa. On

README.ru.md

@@ -232,7 +232,8 @@ PROPERTY_DB=redis://redis:6379/suite-a:  vendor/bin/phpunit   # общий се
 документе, но общий. Нужен `ext-redis` или `predis/predis`; отсутствие обоих —
 ошибка, а не тихий откат на файловую систему. `PROPERTY_DB` с любой другой
 схемой — опечатка `rediss://`, другой бэкенд — тоже ошибка, а не каталог с
-именем схемы.
+именем схемы. Учётные данные в DSN (`redis://user:pass@host`) отклоняются, а не
+молча отбрасываются; настраивайте Redis AUTH отдельно.
 
 Формат корпуса — ровно тот, что писал `rasuvaeff/property-testing` 2.8:
 корпус, записанный под Testo (или под 2.x), реплеится здесь, и наоборот. При

psalm.xml

@@ -38,6 +38,14 @@
             <errorLevel type="suppress">
                 <referencedMethod name="PHPUnit\Framework\TestCase::addToAssertionCount"/>
                 <referencedMethod name="PHPUnit\Framework\AssertionFailedError::__construct"/>
+                <!--
+                    TestCase::name() is the running test method's name, used to
+                    tell a direct forAll() call (id derived from the test method)
+                    from an indirect one (derived from a helper, a colliding
+                    corpus key). Public and stable across PHPUnit 11-13, but
+                    @internal by policy like the rest of the surface.
+                -->
+                <referencedMethod name="PHPUnit\Framework\TestCase::name"/>
             </errorLevel>
         </InternalMethod>
     </issueHandlers>

src/PhpUnit/CorpusFromEnv.php

@@ -36,6 +36,15 @@ final class CorpusFromEnv
      */
     private const string SCHEME_PATTERN = '#^([a-zA-Z][a-zA-Z0-9+.\-]*)://#';
 
+    /**
+     * One corpus per distinct `PROPERTY_DB` value. Resolving happens once per
+     * property, so without this a suite sharing a Redis corpus would build a
+     * client — and open a connection on first recall — for every property.
+     *
+     * @var array<string, Corpus>
+     */
+    private static array $cache = [];
+
     private function __construct()
     {
         // Static helper; not instantiable.
@@ -53,6 +62,11 @@ public static function resolve(): ?Corpus
             return null;
         }
 
+        return self::$cache[$dsn] ??= self::build($dsn);
+    }
+
+    private static function build(string $dsn): Corpus
+    {
         if (preg_match(self::SCHEME_PATTERN, $dsn, $matches) !== 1) {
             return new FilesystemCorpus($dsn);
         }

src/PhpUnit/PropertyCheck.php

@@ -98,6 +98,7 @@
         private string $id,
         private string $name,
         private readonly array $generators,
+        private bool $idDerivedIndirectly = false,
     ) {}
 
     /**
@@ -120,6 +121,9 @@
     {
         $this->id = $id;
         $this->name = $id;
+        // Pinned explicitly: the derivation no longer matters, so drop the
+        // indirect-derivation warning.
+        $this->idDerivedIndirectly = false;
 
         return $this;
     }
@@ -305,7 +309,7 @@
      */
     public function listeners(PropertyListener ...$listeners): self
     {
         $this->listeners = array_values($listeners);
 
         return $this;
     }
@@ -420,7 +424,7 @@
     private function resolveGenerators(\ReflectionFunction $property, array $parameterNames): array
     {
         if (!$this->auto) {
             return $this->generators;
         }
 
         $parameters = array_flip($parameterNames);
@@ -447,7 +451,7 @@
         $classifications = $statistics->classifications;
         $checks = $statistics->checks;
 
         if ($classifications === [] || $checks <= 0) {
             return;
         }
 
@@ -458,7 +462,7 @@
             $parts[] = sprintf(
                 '%s %d%% (%d/%d)',
                 $label,
                 (int) round(((float) $count / (float) $checks) * 100.0),
                 $count,
                 $checks,
             );
@@ -475,7 +479,7 @@
      */
     private function warnOnUnstableId(): void
     {
-        $warning = PropertyId::unstableWarning($this->id);
+        $warning = PropertyId::unstableWarning($this->id) ?? $this->indirectIdWarning();
 
         if ($warning === null) {
             return;
@@ -484,6 +488,25 @@
         fwrite($this->stderr, $warning . "\n");
     }
 
+    /**
+     * `forAll()` was called from a helper (or otherwise not the running test
+     * method), so its derived id names the helper, not the test — every caller
+     * of that helper shares one corpus entry and overwrites the others'
+     * counterexample. `PropertyId::unstableWarning()` misses this because the id
+     * is a stable-looking `Class::method`, just the wrong method.
+     */
+    private function indirectIdWarning(): ?string
+    {
+        if (!$this->idDerivedIndirectly) {
+            return null;
+        }
+
+        return sprintf(
+            'Property id "%s" was derived from a helper, not the test method that ran, so every call site shares one corpus entry: call forAll() directly in the test method or pass an explicit property id with ->id()',
+            $this->id,
+        );
+    }
+
     private function warnOnExcessiveSkips(RunStatistics $statistics): void
     {
         $skips = $statistics->discards;
@@ -498,7 +521,7 @@
             $this->name,
             $skips,
             $attempts,
             (int) round(((float) $skips / (float) $attempts) * 100.0),
         ) . "\n");
     }
 
@@ -605,7 +628,7 @@
             return null;
         }
 
         if (preg_match('/^\d+\z/', $env) !== 1 || (int) $env < 1) {
             throw new \InvalidArgumentException(sprintf('PROPERTY_RUNS must be a positive integer, got "%s"', $env));
         }
 

src/PhpUnit/PropertyTesting.php

@@ -37,7 +37,7 @@
      */
     final protected function forAll(array $generators = []): PropertyCheck
     {
         $frame = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2)[1];
         $method = $frame['function'];
 
         return new PropertyCheck(
@@ -45,6 +45,10 @@
             id: static::class . '::' . $method,
             name: $method,
             generators: $generators,
+            // Derived from something other than the running test method — a
+            // helper or a closure — so the id is shared across call sites and
+            // collides in the corpus. PropertyCheck warns unless id() pins it.
+            idDerivedIndirectly: $method !== $this->name(),
         );
     }
 }

src/PhpUnit/RedisDsn.php

@@ -45,14 +45,27 @@ public function toPredisParameters(): array
     }
 
     /**
-     * @param string $dsn The value of `PROPERTY_DB`, already known to start with `redis://`.
+     * @param string $dsn The value of `PROPERTY_DB`, already known to use the `redis` scheme.
      */
     public static function parse(string $dsn): self
     {
         $parts = parse_url($dsn);
+
+        if (is_array($parts) && (isset($parts['user']) || isset($parts['pass']))) {
+            // Reject credentials rather than drop them silently: parse_url would
+            // discard the userinfo, so the connection would go without AUTH
+            // while the operator believes it authenticated. The message never
+            // echoes the DSN — it would carry the password into the CI log.
+            throw new \InvalidArgumentException(
+                'PROPERTY_DB carries credentials in its userinfo, which is not supported; configure Redis AUTH out of band',
+            );
+        }
+
         $host = is_array($parts) ? ($parts['host'] ?? null) : null;
 
         if (!is_string($host) || $host === '') {
+            // Safe to quote the DSN: a credentialed one was already rejected
+            // above, so whatever reaches here carries no userinfo.
             throw new \InvalidArgumentException(sprintf(
                 'PROPERTY_DB="%s" is not a usable Redis DSN; expected redis://host[:port][/key-prefix]',
                 $dsn,

tests/CorpusFromEnvTest.php

@@ -172,4 +172,20 @@ public function testTheRedisSchemeIsMatchedCaseInsensitively(): void
             $restore();
         }
     }
+
+    /**
+     * Resolving runs once per property, so the same `PROPERTY_DB` must hand back
+     * the same corpus — otherwise a Redis suite builds a client, and opens a
+     * connection on first recall, for every property.
+     */
+    public function testResolvesTheSameCorpusInstanceForTheSameDsn(): void
+    {
+        $restore = Env::set('PROPERTY_DB', sys_get_temp_dir() . '/property-testing-phpunit-memoized');
+
+        try {
+            self::assertSame(CorpusFromEnv::resolve(), CorpusFromEnv::resolve());
+        } finally {
+            $restore();
+        }
+    }
 }

tests/EnvironmentParityTest.php

@@ -474,6 +474,72 @@ public function testANamedPropertyWarnsAboutNothing(): void
         self::assertSame('', (string) stream_get_contents($stderr));
     }
 
+    public function testAHelperDerivedIdIsReportedOnStderr(): void
+    {
+        $stderr = fopen('php://memory', 'w+');
+        self::assertIsResource($stderr);
+
+        // forAll() is called inside checkViaHelper(), so the derived id names
+        // the helper, not this test method — every test using the helper would
+        // share one corpus entry.
+        $this->checkViaHelper($stderr);
+
+        rewind($stderr);
+        $warning = (string) stream_get_contents($stderr);
+
+        self::assertMatchesRegularExpression(
+            '/^Property id "[^"]*::checkViaHelper" was derived from a helper, not the test method that ran, .*->id\(\)\n$/s',
+            $warning,
+        );
+    }
+
+    public function testAHelperDerivedIdIsSilencedByAnExplicitId(): void
+    {
+        $stderr = fopen('php://memory', 'w+');
+        self::assertIsResource($stderr);
+
+        $this->checkViaHelper($stderr, 'pinned::property');
+
+        rewind($stderr);
+
+        self::assertSame('', (string) stream_get_contents($stderr));
+    }
+
+    public function testADirectCallDerivesAStableIdWithoutWarning(): void
+    {
+        $stderr = fopen('php://memory', 'w+');
+        self::assertIsResource($stderr);
+
+        $this->forAll(['value' => Gen::intBetween(0, 10)])
+            ->runs(3)
+            ->output(STDOUT, $stderr)
+            ->check(static function (int $value): void {
+                self::assertGreaterThanOrEqual(0, $value);
+            });
+
+        rewind($stderr);
+
+        self::assertSame('', (string) stream_get_contents($stderr));
+    }
+
+    /**
+     * @param resource $stderr
+     */
+    private function checkViaHelper($stderr, ?string $id = null): void
+    {
+        $check = $this->forAll(['value' => Gen::intBetween(0, 10)])
+            ->runs(3)
+            ->output(STDOUT, $stderr);
+
+        if ($id !== null) {
+            $check->id($id);
+        }
+
+        $check->check(static function (int $value): void {
+            self::assertGreaterThanOrEqual(0, $value);
+        });
+    }
+
     public function testEdgeCasesOffKeepsBoundaryValuesOutOfTheRun(): void
     {
         // The knob's whole purpose: a property that cannot use the edges would
@@ -626,9 +692,10 @@ private function runFalsifiableProperty(
         // practically impossible, so falsification is certain either way.
         $check = $this->forAll(['value' => Gen::intBetween(0, 10_000)])->runs(100);
 
-        if ($id !== null) {
-            $check->id($id);
-        }
+        // Pin the id: forAll() here is one level down from the test method, so
+        // its derived id would be flagged as helper-derived. Default to exactly
+        // what it would have derived, so the corpus key is unchanged.
+        $check->id($id ?? self::class . '::runFalsifiableProperty');
 
         if ($listener instanceof RecordingListener) {
             $check->listeners($listener);

tests/PropertyCheckTest.php

@@ -485,6 +485,8 @@ private function falsifiedOriginal(): mixed
     {
         try {
             $this->forAll(['value' => Gen::intBetween(0, 100_000)])
+                // Pinned: forAll() is one level down from the test method here.
+                ->id(self::class . '::falsifiedOriginal')
                 ->runs(100)
                 ->seed(2026)
                 ->check(static function (int $value): void {

tests/RedisDsnTest.php

@@ -57,6 +57,29 @@ public function testADsnWithoutAHostIsAConfigurationError(): void
         }
     }
 
+    #[DataProvider('credentialledProvider')]
+    public function testADsnWithCredentialsIsRejectedWithoutEchoingThePassword(string $dsn): void
+    {
+        try {
+            RedisDsn::parse($dsn);
+
+            self::fail('expected an InvalidArgumentException');
+        } catch (\InvalidArgumentException $e) {
+            self::assertStringContainsString('credentials', $e->getMessage());
+            self::assertStringNotContainsString('s3cret', $e->getMessage());
+        }
+    }
+
+    /**
+     * @return iterable<string, array{string}>
+     */
+    public static function credentialledProvider(): iterable
+    {
+        yield 'user and password' => ['redis://user:s3cret@redis:6379'];
+        yield 'password only' => ['redis://:s3cret@redis:6379'];
+        yield 'user only' => ['redis://user@redis:6379'];
+    }
+
     #[DataProvider('malformedProvider')]
     public function testAMalformedDsnIsAConfigurationError(string $dsn): void
     {