Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ jobs:

outputs:
run: ${{ steps.filter.outputs.run }}
mutation: ${{ steps.filter.outputs.mutation }}

steps:
- name: Checkout
Expand All @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 3 additions & 1 deletion rector.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

declare(strict_types=1);

use Rasuvaeff\RectorNamedLiterals\AddNameToLiteralArgumentRector;
use Rector\Config\RectorConfig;
use Rector\DeadCode\Rector\Property\RemoveUselessVarTagRector;

Expand All @@ -17,4 +18,5 @@
// suppresses Psalm's MixedAssignment at the untyped getAttribute()
// boundary (UseImportUsageScanner).
RemoveUselessVarTagRector::class,
]);
])
->withRules([AddNameToLiteralArgumentRector::class]);
14 changes: 7 additions & 7 deletions src/Cli/MigrationApplication.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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');
}

Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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));
}

Expand All @@ -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));
}

Expand Down Expand Up @@ -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'],
Expand Down Expand Up @@ -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);
}
Expand Down
12 changes: 6 additions & 6 deletions src/DateTimeImmutableRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Internal/DateTimeMutatorCatalog.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
4 changes: 2 additions & 2 deletions src/Internal/DocblockTypeRewriter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Internal/FactoryCallMap.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
6 changes: 3 additions & 3 deletions src/LostDateTimeMutationRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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,
);
}

Expand Down
6 changes: 3 additions & 3 deletions src/MutableDateTimeBoundaryRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion tests/DateTimeImmutableRectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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', "<?php\n\$d = new \\DateTime();\n");

try {
Expand Down
71 changes: 71 additions & 0 deletions tests/FactoryCallMapTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Rasuvaeff\RectorDateTimeImmutable\Tests;

use Rasuvaeff\PropertyTesting\ArbitraryInterface;
use Rasuvaeff\PropertyTesting\Classify;
use Rasuvaeff\PropertyTesting\Gen;
use Rasuvaeff\PropertyTesting\Property;
use Rasuvaeff\RectorDateTimeImmutable\Internal\FactoryCallMap;
Expand Down Expand Up @@ -74,6 +75,12 @@ public function lookupsAreCaseInsensitive(string $name, int $caseMask): void
$map = new FactoryCallMap();
$mixedCase = CaseMask::apply($name, $caseMask);

// A mask of zero leaves the name untouched, which asserts nothing
// about case handling; the gate keeps the runs that do the work from
// quietly becoming a minority.
Classify::cover($mixedCase !== $name, 'case actually changed', 60.0);
Classify::when($caseMask === 0, 'name left as written');

Assert::same($map->immutableEquivalent($mixedCase), $map->immutableEquivalent($name));
Assert::same($map->isSharedStaticFactory($mixedCase), $map->isSharedStaticFactory($name));
Assert::same($map->isProceduralImmutableFactory($mixedCase), $map->isProceduralImmutableFactory($name));
Expand All @@ -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<string, ArbitraryInterface>
*/
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<string, array{string}>
*/
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'];
}
}
Loading
Loading