From 661f1f9b5b7286b07937e393e0f35a99d7a3d734 Mon Sep 17 00:00:00 2001 From: "v.razuvaev" Date: Sat, 15 Aug 2026 14:47:12 +0300 Subject: [PATCH] test: migrate to property-testing-testo and pin the catalogue boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Swap the frozen rasuvaeff/property-testing 2.x for the split family's Testo adapter, pinned at ^0.4 (core ^0.3) — ^0.1, which MIGRATION.md still shows, installs core 0.1 and leaves every 0.2/0.3 engine feature unreachable. Both existing properties only ever asked about names that are in the catalogue, so nothing said what happens to a name that is not. That boundary is the whole safety story of this rule: a rewriter that mapped a name it does not know would rewrite user code into a call that does not exist, and one that answered yes across catalogues would treat an already-immutable factory as needing the rewrite it is the target of. eachLookupAnswersYesOnlyForItsOwnCatalogue checks all three lookups against all three catalogues, generating catalogued names alongside near misses built from the same alphabets through Gen::regex — so `date_created` and `createFromImmutable` are ordinary draws rather than lucky ones. Examples() pins createFromImmutable specifically: the source documents it as deliberately absent, and nothing enforced that. The case-insensitivity property gained a floor on runs where the mask actually changes the name — a mask of zero asserts nothing about case handling. Also wire rasuvaeff/rector-named-literals and apply it, carry the property regression corpus across CI runs (restore and save split, because the combined cache action's post-if: success() never saves on the red run that recorded the counterexample), and give mutation its own narrow change filter. --- .github/workflows/build.yml | 37 ++++++++++- composer.json | 3 +- rector.php | 4 +- src/Cli/MigrationApplication.php | 14 ++-- src/DateTimeImmutableRector.php | 12 ++-- src/Internal/DateTimeMutatorCatalog.php | 2 +- src/Internal/DocblockTypeRewriter.php | 4 +- src/Internal/FactoryCallMap.php | 4 +- src/LostDateTimeMutationRector.php | 6 +- src/MutableDateTimeBoundaryRector.php | 6 +- tests/DateTimeImmutableRectorTest.php | 2 +- tests/FactoryCallMapTest.php | 71 +++++++++++++++++++++ tests/FixtureSuite.php | 2 +- tests/LostDateTimeMutationRectorTest.php | 2 +- tests/MigrationApplicationTest.php | 4 +- tests/MutableDateTimeBoundaryRectorTest.php | 2 +- 16 files changed, 142 insertions(+), 33 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6341739..01ae421 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -20,6 +20,7 @@ jobs: outputs: run: ${{ steps.filter.outputs.run }} + mutation: ${{ steps.filter.outputs.mutation }} steps: - name: Checkout @@ -38,7 +39,11 @@ jobs: if [ "$EVENT_NAME" = "pull_request" ]; then BASE="$BASE_SHA" elif [ "$BEFORE_SHA" = "0000000000000000000000000000000000000000" ]; then + # First push to a new branch: there is no base to diff against, so + # every filter says yes. Both outputs must be set here — an unset + # one reads as false downstream and silently skips its job. echo "run=true" >> "$GITHUB_OUTPUT" + echo "mutation=true" >> "$GITHUB_OUTPUT" exit 0 else BASE="$BEFORE_SHA" @@ -53,6 +58,20 @@ jobs: echo "run=true" >> "$GITHUB_OUTPUT" fi + # Mutation is by far the most expensive job, so it gets a narrower + # filter than the rest: only what can change which mutants exist or + # which tests kill them. Style and static-analysis configs, examples + # and this workflow cannot. Explicit trade: editing the mutation STEP + # here no longer re-runs it — a break surfaces on the next source + # change or a manual re-run, which beats paying the full mutation run + # on every docs and CI edit. + if git diff --quiet "$BASE" "$GITHUB_SHA" -- \ + src tests composer.json testo.php infection.json5; then + echo "mutation=false" >> "$GITHUB_OUTPUT" + else + echo "mutation=true" >> "$GITHUB_OUTPUT" + fi + build: name: PHP ${{ matrix.php }} runs-on: ubuntu-latest @@ -128,7 +147,7 @@ jobs: coverage: name: Coverage & Mutation needs: changes - if: ${{ !cancelled() && (needs.changes.result != 'success' || needs.changes.outputs.run == 'true') }} + if: ${{ !cancelled() && (needs.changes.result != 'success' || needs.changes.outputs.mutation == 'true') }} runs-on: ubuntu-latest steps: @@ -155,8 +174,24 @@ jobs: - name: Install dependencies run: composer install --no-interaction --no-progress --prefer-dist + - name: Restore property regression corpus + uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: build/property-db + key: property-db-${{ github.run_id }}-${{ github.run_attempt }} + restore-keys: property-db- + - name: Test with coverage run: composer test:coverage:ci + env: + PROPERTY_DB: ${{ github.workspace }}/build/property-db + + - name: Save property regression corpus + if: ${{ !cancelled() }} + uses: actions/cache/save@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: build/property-db + key: property-db-${{ github.run_id }}-${{ github.run_attempt }} - name: Mutation testing run: composer mutation diff --git a/composer.json b/composer.json index d75dac7..ecb4df2 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,8 @@ "friendsofphp/php-cs-fixer": "^3.95", "infection/infection": "^0.33 || ^0.34", "maglnet/composer-require-checker": "^4.17", - "rasuvaeff/property-testing": "^2.4", + "rasuvaeff/property-testing-testo": "^0.4", + "rasuvaeff/rector-named-literals": "^1.0", "roave/backward-compatibility-check": "^8.0", "testo/bridge-infection": "^0.1.6", "testo/testo": "^0.10.25", diff --git a/rector.php b/rector.php index aa560f9..1a1e18a 100644 --- a/rector.php +++ b/rector.php @@ -2,6 +2,7 @@ declare(strict_types=1); +use Rasuvaeff\RectorNamedLiterals\AddNameToLiteralArgumentRector; use Rector\Config\RectorConfig; use Rector\DeadCode\Rector\Property\RemoveUselessVarTagRector; @@ -17,4 +18,5 @@ // suppresses Psalm's MixedAssignment at the untyped getAttribute() // boundary (UseImportUsageScanner). RemoveUselessVarTagRector::class, - ]); + ]) + ->withRules([AddNameToLiteralArgumentRector::class]); diff --git a/src/Cli/MigrationApplication.php b/src/Cli/MigrationApplication.php index f0a3dad..26e872d 100644 --- a/src/Cli/MigrationApplication.php +++ b/src/Cli/MigrationApplication.php @@ -197,7 +197,7 @@ private function parseArguments(array $arguments): array continue; } - if (in_array($argument, ['--max-passes', '--preflight-config', '--config', '--report-config', '--rector', '--format'], true) + if (in_array($argument, ['--max-passes', '--preflight-config', '--config', '--report-config', '--rector', '--format'], strict: true) ) { ++$index; @@ -300,7 +300,7 @@ private function assignOption( } if ($option === '--format') { - if (!in_array($value, self::FORMATS, true)) { + if (!in_array($value, self::FORMATS, strict: true)) { throw new InvalidArgumentException('--format must be one of: human, github, json'); } @@ -788,7 +788,7 @@ private function createWorkspace(array $paths): array { $root = sys_get_temp_dir() . '/rector-datetime-immutable-dry-' . bin2hex(random_bytes(6)); - if (!mkdir($root, 0o777, true)) { + if (!mkdir($root, 0o777, recursive: true)) { throw new RuntimeException(sprintf('Unable to create the dry-run workspace "%s"', $root)); } @@ -821,7 +821,7 @@ private function copyPath(string $source, string $target): void { $targetDirectory = is_file($source) ? \dirname($target) : $target; - if (!is_dir($targetDirectory) && !mkdir($targetDirectory, 0o777, true)) { + if (!is_dir($targetDirectory) && !mkdir($targetDirectory, 0o777, recursive: true)) { throw new RuntimeException(sprintf('Unable to create the dry-run directory "%s"', $targetDirectory)); } @@ -846,7 +846,7 @@ private function copyPath(string $source, string $target): void $destination = $target . substr($item->getPathname(), \strlen($source)); if ($item->isDir()) { - if (!is_dir($destination) && !mkdir($destination, 0o777, true)) { + if (!is_dir($destination) && !mkdir($destination, 0o777, recursive: true)) { throw new RuntimeException(sprintf('Unable to create the dry-run directory "%s"', $destination)); } @@ -1009,7 +1009,7 @@ private function invokeRector( $acceptedExitCodes = $dryRun ? [0, 2] : [0]; - if (!in_array($process['exitCode'], $acceptedExitCodes, true)) { + if (!in_array($process['exitCode'], $acceptedExitCodes, strict: true)) { throw new RuntimeException(sprintf( "Rector exited with code %d.\n%s", $process['exitCode'], @@ -1093,7 +1093,7 @@ private function runProcess(array $command): array private function decodeRectorOutput(string $output): array { try { - $decoded = json_decode($output, true, 512, JSON_THROW_ON_ERROR); + $decoded = json_decode($output, associative: true, depth: 512, flags: JSON_THROW_ON_ERROR); } catch (JsonException $exception) { throw new RuntimeException('Rector did not return valid JSON', $exception->getCode(), previous: $exception); } diff --git a/src/DateTimeImmutableRector.php b/src/DateTimeImmutableRector.php index b4196e5..daefb77 100644 --- a/src/DateTimeImmutableRector.php +++ b/src/DateTimeImmutableRector.php @@ -792,7 +792,7 @@ private function storageKey(Expr $expr): ?string $expr instanceof StaticPropertyFetch && $expr->class instanceof Name && $expr->name instanceof VarLikeIdentifier - && \in_array(strtolower($expr->class->toString()), ['self', 'static'], true) + && \in_array(strtolower($expr->class->toString()), ['self', 'static'], strict: true) ) { return $this->propertyStorageKey($expr, $expr->name->toString()); } @@ -865,7 +865,7 @@ private function ancestorDeclaresMethod(ClassReflection $classReflection, string } } - foreach ($classReflection->getTraits(true) as $trait) { + foreach ($classReflection->getTraits(recursive: true) as $trait) { if ($trait->hasNativeMethod($methodName) && $trait->getNativeMethod($methodName)->isAbstract()) { return true; } @@ -892,7 +892,7 @@ private function ancestorDeclaresProperty(ClassReflection $classReflection, stri $ancestors = [ ...$classReflection->getParents(), ...$classReflection->getInterfaces(), - ...array_values($classReflection->getTraits(true)), + ...array_values($classReflection->getTraits(recursive: true)), ]; foreach ($ancestors as $ancestor) { @@ -974,19 +974,19 @@ private function isPropertyAssignmentTarget( && $expr->var->name === 'this' && $expr->name instanceof Identifier ) { - return \in_array($expr->name->toString(), $propertyNames, true); + return \in_array($expr->name->toString(), $propertyNames, strict: true); } if ( !$expr instanceof StaticPropertyFetch || !$expr->name instanceof VarLikeIdentifier - || !\in_array($expr->name->toString(), $propertyNames, true) + || !\in_array($expr->name->toString(), $propertyNames, strict: true) || !$expr->class instanceof Name ) { return false; } - if (\in_array(strtolower($expr->class->toString()), ['self', 'static', 'parent'], true)) { + if (\in_array(strtolower($expr->class->toString()), ['self', 'static', 'parent'], strict: true)) { return true; } diff --git a/src/Internal/DateTimeMutatorCatalog.php b/src/Internal/DateTimeMutatorCatalog.php index b67f25a..58cf8f3 100644 --- a/src/Internal/DateTimeMutatorCatalog.php +++ b/src/Internal/DateTimeMutatorCatalog.php @@ -29,6 +29,6 @@ public function isMutator(string $methodName): bool { - return \in_array(strtolower($methodName), self::MUTATORS, true); + return \in_array(strtolower($methodName), self::MUTATORS, strict: true); } } diff --git a/src/Internal/DocblockTypeRewriter.php b/src/Internal/DocblockTypeRewriter.php index de42b49..7481641 100644 --- a/src/Internal/DocblockTypeRewriter.php +++ b/src/Internal/DocblockTypeRewriter.php @@ -112,9 +112,9 @@ private function typeToken(string $line, int $offset): ?array while ($end < $length) { $character = $line[$end]; - if (in_array($character, ['<', '(', '{', '['], true)) { + if (in_array($character, ['<', '(', '{', '['], strict: true)) { ++$depth; - } elseif (in_array($character, ['>', ')', '}', ']'], true)) { + } elseif (in_array($character, ['>', ')', '}', ']'], strict: true)) { if ($depth === 0) { break; } diff --git a/src/Internal/FactoryCallMap.php b/src/Internal/FactoryCallMap.php index f442af4..3c581b5 100644 --- a/src/Internal/FactoryCallMap.php +++ b/src/Internal/FactoryCallMap.php @@ -43,11 +43,11 @@ public function immutableEquivalent(string $functionName): ?string */ public function isProceduralImmutableFactory(string $functionName): bool { - return \in_array(strtolower($functionName), self::FUNCTION_MAP, true); + return \in_array(strtolower($functionName), self::FUNCTION_MAP, strict: true); } public function isSharedStaticFactory(string $methodName): bool { - return \in_array(strtolower($methodName), self::SHARED_STATIC_FACTORIES, true); + return \in_array(strtolower($methodName), self::SHARED_STATIC_FACTORIES, strict: true); } } diff --git a/src/LostDateTimeMutationRector.php b/src/LostDateTimeMutationRector.php index e27afd0..d295419 100644 --- a/src/LostDateTimeMutationRector.php +++ b/src/LostDateTimeMutationRector.php @@ -196,7 +196,7 @@ public function refactor(Node $node): ?Node } $assignment = new Assign(new Variable($call->var->name), $call); - $assignment->setAttribute(self::UNCONDITIONAL_ASSIGNMENT, true); + $assignment->setAttribute(self::UNCONDITIONAL_ASSIGNMENT, value: true); $node->expr = $assignment; return $node; @@ -281,7 +281,7 @@ private function markUnconditionalAssignments( ): void { foreach ($scope->stmts ?? [] as $statement) { if ($statement instanceof Expression && $statement->expr instanceof Assign) { - $statement->expr->setAttribute(self::UNCONDITIONAL_ASSIGNMENT, true); + $statement->expr->setAttribute(self::UNCONDITIONAL_ASSIGNMENT, value: true); } } } @@ -340,7 +340,7 @@ private function isExactBuiltInExpression(Expr $expr, string $scopeKey): bool return \in_array( strtolower($expr->name->toString()), ['createfromformat', 'createfrominterface', 'createfrommutable', 'createfromtimestamp'], - true, + strict: true, ); } diff --git a/src/MutableDateTimeBoundaryRector.php b/src/MutableDateTimeBoundaryRector.php index e6835fb..ae3cd00 100644 --- a/src/MutableDateTimeBoundaryRector.php +++ b/src/MutableDateTimeBoundaryRector.php @@ -369,7 +369,7 @@ private function ownPropertyName(Expr $expr): ?string $expr instanceof StaticPropertyFetch && $expr->class instanceof Name && $expr->name instanceof VarLikeIdentifier - && \in_array(strtolower($expr->class->toString()), ['self', 'static'], true) + && \in_array(strtolower($expr->class->toString()), ['self', 'static'], strict: true) ) { return $expr->name->toString(); } @@ -418,7 +418,7 @@ private function ancestorDeclaresMethod(ClassReflection $classReflection, string } } - foreach ($classReflection->getTraits(true) as $trait) { + foreach ($classReflection->getTraits(recursive: true) as $trait) { if ($trait->hasNativeMethod($methodName) && $trait->getNativeMethod($methodName)->isAbstract()) { return true; } @@ -445,7 +445,7 @@ private function ancestorDeclaresProperty(ClassReflection $classReflection, stri $ancestors = [ ...$classReflection->getParents(), ...$classReflection->getInterfaces(), - ...array_values($classReflection->getTraits(true)), + ...array_values($classReflection->getTraits(recursive: true)), ]; foreach ($ancestors as $ancestor) { diff --git a/tests/DateTimeImmutableRectorTest.php b/tests/DateTimeImmutableRectorTest.php index fcba362..e096a52 100644 --- a/tests/DateTimeImmutableRectorTest.php +++ b/tests/DateTimeImmutableRectorTest.php @@ -52,7 +52,7 @@ public function rejectsUnknownConfigurationKey(): void PHP); $workDir = sys_get_temp_dir() . '/rector-datetime-immutable-invalid-' . bin2hex(random_bytes(4)); - mkdir($workDir, 0o777, true); + mkdir($workDir, 0o777, recursive: true); file_put_contents($workDir . '/Sample.php', "immutableEquivalent($mixedCase), $map->immutableEquivalent($name)); Assert::same($map->isSharedStaticFactory($mixedCase), $map->isSharedStaticFactory($name)); Assert::same($map->isProceduralImmutableFactory($mixedCase), $map->isProceduralImmutableFactory($name)); @@ -98,4 +105,68 @@ public static function lookupsAreCaseInsensitiveGenerators(): array 'caseMask' => Gen::intBetween(0, (1 << 16) - 1), ]; } + + #[Property(runs: 400, timeoutMs: 1000)] + public function eachLookupAnswersYesOnlyForItsOwnCatalogue(string $name): void + { + $map = new FactoryCallMap(); + $lower = \strtolower($name); + + $mutableFactory = \in_array($lower, ['date_create', 'date_create_from_format'], strict: true); + $immutableFactory = \in_array($lower, ['date_create_immutable', 'date_create_immutable_from_format'], strict: true); + $sharedStatic = \in_array($lower, ['createfromformat', 'createfrominterface', 'createfromtimestamp'], strict: true); + + Classify::cover($mutableFactory, 'a mutable procedural factory', 10.0); + Classify::cover($immutableFactory, 'an immutable procedural factory', 10.0); + Classify::cover($sharedStatic, 'a shared static factory', 10.0); + Classify::cover( + !$mutableFactory && !$immutableFactory && !$sharedStatic, + 'outside every catalogue', + 25.0, + ); + + // Three catalogues that must not bleed into one another. A rewriter + // that answered yes for a name it does not know would rewrite user code + // into a call that does not exist, and one that answered yes across + // catalogues would treat an already-immutable factory as needing the + // rewrite it is the target of. + Assert::same($map->immutableEquivalent($name) !== null, $mutableFactory); + Assert::same($map->isProceduralImmutableFactory($name), $immutableFactory); + Assert::same($map->isSharedStaticFactory($name), $sharedStatic); + } + + /** + * @return array + */ + public static function eachLookupAnswersYesOnlyForItsOwnCatalogueGenerators(): array + { + return [ + 'name' => Gen::frequency([ + [1, Gen::elements(['date_create', 'date_create_from_format'])], + [1, Gen::elements(['date_create_immutable', 'date_create_immutable_from_format'])], + [1, Gen::elements(['createFromFormat', 'createFromInterface', 'createFromTimestamp'])], + // Names from the same alphabets, so a near miss such as + // `date_created` or `createFromImmutable` is an ordinary draw + // rather than a lucky one. + [1, Gen::regex('date_[a-z_]{0,12}')], + [1, Gen::regex('createFrom[A-Za-z]{0,10}')], + ]), + ]; + } + + /** + * @return iterable + */ + public static function eachLookupAnswersYesOnlyForItsOwnCatalogueExamples(): iterable + { + yield 'empty name' => ['']; + yield 'catalogued, mixed case' => ['DaTe_CrEaTe']; + yield 'the immutable twin is not itself a target' => ['date_create_immutable']; + // Documented as deliberately absent: it has no counterpart on + // DateTimeImmutable and marks code that wants mutability. + yield 'createFromImmutable is deliberately absent' => ['createFromImmutable']; + yield 'one character longer' => ['date_created']; + yield 'one character shorter' => ['date_creat']; + yield 'prefixed' => ['my_date_create']; + } } diff --git a/tests/FixtureSuite.php b/tests/FixtureSuite.php index a9a8f8b..0e2fc8f 100644 --- a/tests/FixtureSuite.php +++ b/tests/FixtureSuite.php @@ -30,7 +30,7 @@ public static function assertTransformed( $fixtureDir = __DIR__ . '/fixture/' . $suite; $workDir = sys_get_temp_dir() . '/rector-datetime-immutable-' . $suite . '-' . bin2hex(random_bytes(4)); - mkdir($workDir, 0o777, true); + mkdir($workDir, 0o777, recursive: true); try { $fixtures = glob($fixtureDir . '/*.php.fixture') ?: []; diff --git a/tests/LostDateTimeMutationRectorTest.php b/tests/LostDateTimeMutationRectorTest.php index 16a28d9..a9fa4fd 100644 --- a/tests/LostDateTimeMutationRectorTest.php +++ b/tests/LostDateTimeMutationRectorTest.php @@ -58,7 +58,7 @@ public function rejectsInvalidMode(): void PHP); $workDir = sys_get_temp_dir() . '/rector-datetime-immutable-invalid-' . bin2hex(random_bytes(4)); - mkdir($workDir, 0o777, true); + mkdir($workDir, 0o777, recursive: true); file_put_contents($workDir . '/Sample.php', "modify('+1 day');\n"); try { diff --git a/tests/MigrationApplicationTest.php b/tests/MigrationApplicationTest.php index eb03c56..32507f5 100644 --- a/tests/MigrationApplicationTest.php +++ b/tests/MigrationApplicationTest.php @@ -242,7 +242,7 @@ public function jsonFormatEmitsSingleMachineReadableObject(): void try { $exitCode = $this->application($stdout, $stderr)->run(['--format=json', $file]); - $payload = json_decode($stdout, true, 512, JSON_THROW_ON_ERROR); + $payload = json_decode($stdout, associative: true, depth: 512, flags: JSON_THROW_ON_ERROR); Assert::same($exitCode, 0); Assert::false(str_contains($stdout, 'Migration pass')); @@ -273,7 +273,7 @@ function moveDate(\DateTimeImmutable $date): void try { $exitCode = $this->application($stdout, $stderr)->run(['--format=json', $file]); - $payload = json_decode($stdout, true, 512, JSON_THROW_ON_ERROR); + $payload = json_decode($stdout, associative: true, depth: 512, flags: JSON_THROW_ON_ERROR); Assert::same($exitCode, 2); Assert::same($payload['status'], 'manual-review'); diff --git a/tests/MutableDateTimeBoundaryRectorTest.php b/tests/MutableDateTimeBoundaryRectorTest.php index 772c6d6..ef34cd7 100644 --- a/tests/MutableDateTimeBoundaryRectorTest.php +++ b/tests/MutableDateTimeBoundaryRectorTest.php @@ -41,7 +41,7 @@ public function rejectsInvalidMode(): void PHP); $workDir = sys_get_temp_dir() . '/rector-datetime-immutable-mode-' . bin2hex(random_bytes(4)); - mkdir($workDir, 0o777, true); + mkdir($workDir, 0o777, recursive: true); file_put_contents($workDir . '/Sample.php', "