Skip to content

Commit

Permalink
PHP-CS-Fixer#7968 added new ruleset, fixed tests
Browse files Browse the repository at this point in the history
  • Loading branch information
krzysztof-ciszewski committed May 9, 2024
1 parent 2c8ca76 commit 0193955
Show file tree
Hide file tree
Showing 17 changed files with 233 additions and 26 deletions.
2 changes: 1 addition & 1 deletion doc/ruleSets/PHPUnit100MigrationRisky.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Using this rule set may lead to changes in your code's logic and behaviour. Use
Rules
-----

- `@PHPUnit84Migration:risky <./PHPUnit84MigrationRisky.rst>`_
- `@PHPUnit91Migration:risky <./PHPUnit91MigrationRisky.rst>`_
- `php_unit_data_provider_static <./../rules/php_unit/php_unit_data_provider_static.rst>`_ with config:

``['force' => true]``
Expand Down
22 changes: 22 additions & 0 deletions doc/ruleSets/PHPUnit91MigrationRisky.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
======================================
Rule set ``@PHPUnit91Migration:risky``
======================================

Rules to improve tests code for PHPUnit 9.1 compatibility.

Warning
-------

This set contains rules that are risky
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Using this rule set may lead to changes in your code's logic and behaviour. Use it with caution and review changes before incorporating them into your code base.

Rules
-----

- `@PHPUnit84Migration:risky <./PHPUnit84MigrationRisky.rst>`_
- `php_unit_dedicate_assert <./../rules/php_unit/php_unit_dedicate_assert.rst>`_ with config:

``['target' => '9.1']``

1 change: 1 addition & 0 deletions doc/ruleSets/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ List of Available Rule sets
- `@PHPUnit60Migration:risky <./PHPUnit60MigrationRisky.rst>`_
- `@PHPUnit75Migration:risky <./PHPUnit75MigrationRisky.rst>`_
- `@PHPUnit84Migration:risky <./PHPUnit84MigrationRisky.rst>`_
- `@PHPUnit91Migration:risky <./PHPUnit91MigrationRisky.rst>`_
- `@PHPUnit100Migration:risky <./PHPUnit100MigrationRisky.rst>`_
- `@PSR1 <./PSR1.rst>`_
- `@PSR2 <./PSR2.rst>`_
Expand Down
8 changes: 6 additions & 2 deletions doc/rules/php_unit/php_unit_dedicate_assert.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Configuration

Target version of PHPUnit.

Allowed values: ``'3.0'``, ``'3.5'``, ``'5.0'``, ``'5.6'`` and ``'newest'``
Allowed values: ``'3.0'``, ``'3.5'``, ``'5.0'``, ``'5.6'``, ``'9.1'`` and ``'newest'``

Default value: ``'newest'``

Expand Down Expand Up @@ -133,9 +133,13 @@ The rule is part of the following rule sets:

``['target' => '5.6']``

- `@PHPUnit91Migration:risky <./../../ruleSets/PHPUnit91MigrationRisky.rst>`_ with config:

``['target' => '9.1']``

- `@PHPUnit100Migration:risky <./../../ruleSets/PHPUnit100MigrationRisky.rst>`_ with config:

``['target' => '5.6']``
``['target' => '9.1']``


References
Expand Down
4 changes: 4 additions & 0 deletions doc/rules/php_unit/php_unit_dedicate_assert_internal_type.rst
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ The rule is part of the following rule sets:

``['target' => '7.5']``

- `@PHPUnit91Migration:risky <./../../ruleSets/PHPUnit91MigrationRisky.rst>`_ with config:

``['target' => '7.5']``

- `@PHPUnit100Migration:risky <./../../ruleSets/PHPUnit100MigrationRisky.rst>`_ with config:

``['target' => '7.5']``
Expand Down
4 changes: 4 additions & 0 deletions doc/rules/php_unit/php_unit_expectation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ The rule is part of the following rule sets:

``['target' => '8.4']``

- `@PHPUnit91Migration:risky <./../../ruleSets/PHPUnit91MigrationRisky.rst>`_ with config:

``['target' => '8.4']``

- `@PHPUnit100Migration:risky <./../../ruleSets/PHPUnit100MigrationRisky.rst>`_ with config:

``['target' => '8.4']``
Expand Down
4 changes: 4 additions & 0 deletions doc/rules/php_unit/php_unit_mock.rst
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ The rule is part of the following rule sets:

``['target' => '5.5']``

- `@PHPUnit91Migration:risky <./../../ruleSets/PHPUnit91MigrationRisky.rst>`_ with config:

``['target' => '5.5']``

- `@PHPUnit100Migration:risky <./../../ruleSets/PHPUnit100MigrationRisky.rst>`_ with config:

``['target' => '5.5']``
Expand Down
4 changes: 4 additions & 0 deletions doc/rules/php_unit/php_unit_namespaced.rst
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ The rule is part of the following rule sets:

``['target' => '6.0']``

- `@PHPUnit91Migration:risky <./../../ruleSets/PHPUnit91MigrationRisky.rst>`_ with config:

``['target' => '6.0']``

- `@PHPUnit100Migration:risky <./../../ruleSets/PHPUnit100MigrationRisky.rst>`_ with config:

``['target' => '6.0']``
Expand Down
4 changes: 4 additions & 0 deletions doc/rules/php_unit/php_unit_no_expectation_annotation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,10 @@ The rule is part of the following rule sets:

``['target' => '4.3']``

- `@PHPUnit91Migration:risky <./../../ruleSets/PHPUnit91MigrationRisky.rst>`_ with config:

``['target' => '4.3']``

- `@PHPUnit100Migration:risky <./../../ruleSets/PHPUnit100MigrationRisky.rst>`_ with config:

``['target' => '4.3']``
Expand Down
105 changes: 83 additions & 22 deletions src/Fixer/PhpUnit/PhpUnitDedicateAssertFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,26 +36,89 @@ final class PhpUnitDedicateAssertFixer extends AbstractPhpUnitFixer implements C
/**
* @var array<string, array<string, bool|int|string>|true>
*/
private static array $fixMap = [];
private array $fixMap = [
'array_key_exists' => [
'positive' => 'assertArrayHasKey',
'negative' => 'assertArrayNotHasKey',
'argument_count' => 2,
],
'empty' => [
'positive' => 'assertEmpty',
'negative' => 'assertNotEmpty',
],
'file_exists' => [
'positive' => 'assertFileExists',
'negative' => 'assertFileNotExists',
],
'is_array' => true,
'is_bool' => true,
'is_callable' => true,
'is_dir' => [
'positive' => 'assertDirectoryExists',
'negative' => 'assertDirectoryNotExists',
],
'is_double' => true,
'is_float' => true,
'is_infinite' => [
'positive' => 'assertInfinite',
'negative' => 'assertFinite',
],
'is_int' => true,
'is_integer' => true,
'is_long' => true,
'is_nan' => [
'positive' => 'assertNan',
'negative' => false,
],
'is_null' => [
'positive' => 'assertNull',
'negative' => 'assertNotNull',
],
'is_numeric' => true,
'is_object' => true,
'is_readable' => [
'positive' => 'assertIsReadable',
'negative' => 'assertNotIsReadable',
],
'is_real' => true,
'is_resource' => true,
'is_scalar' => true,
'is_string' => true,
'is_writable' => [
'positive' => 'assertIsWritable',
'negative' => 'assertNotIsWritable',
],
'str_contains' => [ // since 7.5
'positive' => 'assertStringContainsString',
'negative' => 'assertStringNotContainsString',
'argument_count' => 2,
'swap_arguments' => true,
],
'str_ends_with' => [ // since 3.4
'positive' => 'assertStringEndsWith',
'negative' => 'assertStringEndsNotWith',
'argument_count' => 2,
'swap_arguments' => true,
],
'str_starts_with' => [ // since 3.4
'positive' => 'assertStringStartsWith',
'negative' => 'assertStringStartsNotWith',
'argument_count' => 2,
'swap_arguments' => true,
],
];

/**
* @var list<string>
*/
private array $functions = [];

public function __construct()
{
parent::__construct();

if ([] === self::$fixMap) {
self::$fixMap = $this->getFixMap();
}
}

public function configure(array $configuration): void
{
parent::configure($configuration);

$this->fixMap = $this->getFixMap();

// assertions added in 3.0: assertArrayNotHasKey assertArrayHasKey assertFileNotExists assertFileExists assertNotNull, assertNull
$this->functions = [
'array_key_exists',
Expand Down Expand Up @@ -110,34 +173,32 @@ public function configure(array $configuration): void
}

if (PhpUnitTargetVersion::fulfills($this->configuration['target'], PhpUnitTargetVersion::VERSION_9_1)) {
self::$fixMap = array_merge(self::$fixMap, [
$this->fixMap = array_merge($this->fixMap, [
'is_readable' => array_merge(
self::$fixMap['is_readable'] ?? [],
$this->fixMap['is_readable'] ?? [],
[
'negative' => 'assertIsNotReadable',
]
),
'is_writable' => array_merge(
self::$fixMap['is_writable'] ?? [],
$this->fixMap['is_writable'] ?? [],
[
'negative' => 'assertIsNotWritable',
]
),
'file_exists' => array_merge(
self::$fixMap['file_exists'] ?? [],
$this->fixMap['file_exists'] ?? [],
[
'negative' => 'assertFileDoesNotExist',
]
),
'is_dir' => array_merge(
self::$fixMap['is_dir'] ?? [],
$this->fixMap['is_dir'] ?? [],
[
'negative' => 'assertDirectoryDoesNotExist',
]
),
]);
} else {
self::$fixMap = $this->getFixMap();
}
}

Expand Down Expand Up @@ -283,23 +344,23 @@ private function fixAssertTrueFalse(Tokens $tokens, ArgumentsAnalyzer $arguments
$arguments = $argumentsAnalyzer->getArguments($tokens, $testOpenIndex, $testCloseIndex);
$isPositive = 'asserttrue' === $assertCall['loweredName'];

if (\is_array(self::$fixMap[$content])) {
$expectedCount = self::$fixMap[$content]['argument_count'] ?? 1;
if (\is_array($this->fixMap[$content])) {
$expectedCount = $this->fixMap[$content]['argument_count'] ?? 1;

if ($expectedCount !== \count($arguments)) {
return;
}

$isPositive = $isPositive ? 'positive' : 'negative';

if (false === self::$fixMap[$content][$isPositive]) {
if (false === $this->fixMap[$content][$isPositive]) {
return;
}

$tokens[$assertCall['index']] = new Token([T_STRING, self::$fixMap[$content][$isPositive]]);
$tokens[$assertCall['index']] = new Token([T_STRING, $this->fixMap[$content][$isPositive]]);
$this->removeFunctionCall($tokens, $testDefaultNamespaceTokenIndex, $testIndex, $testOpenIndex, $testCloseIndex);

if (self::$fixMap[$content]['swap_arguments'] ?? false) {
if ($this->fixMap[$content]['swap_arguments'] ?? false) {
if (2 !== $expectedCount) {
throw new \RuntimeException('Can only swap two arguments, please update map or logic.');
}
Expand Down
2 changes: 1 addition & 1 deletion src/RuleSet/Sets/PHPUnit100MigrationRiskySet.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ final class PHPUnit100MigrationRiskySet extends AbstractMigrationSetDescription
public function getRules(): array
{
return [
'@PHPUnit84Migration:risky' => true,
'@PHPUnit91Migration:risky' => true,
'php_unit_data_provider_static' => ['force' => true],
];
}
Expand Down
34 changes: 34 additions & 0 deletions src/RuleSet/Sets/PHPUnit91MigrationRiskySet.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

declare(strict_types=1);

/*
* This file is part of PHP CS Fixer.
*
* (c) Fabien Potencier <fabien@symfony.com>
* Dariusz Rumiński <dariusz.ruminski@gmail.com>
*
* This source file is subject to the MIT license that is bundled
* with this source code in the file LICENSE.
*/

namespace PhpCsFixer\RuleSet\Sets;

use PhpCsFixer\Fixer\PhpUnit\PhpUnitTargetVersion;
use PhpCsFixer\RuleSet\AbstractMigrationSetDescription;

/**
* @internal
*/
final class PHPUnit91MigrationRiskySet extends AbstractMigrationSetDescription
{
public function getRules(): array
{
return [
'@PHPUnit84Migration:risky' => true,
'php_unit_dedicate_assert' => [
'target' => PhpUnitTargetVersion::VERSION_9_1,
],
];
}
}
4 changes: 4 additions & 0 deletions tests/Fixtures/Integration/set/@PHPUnit91Migration-risky.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
--TEST--
Integration of @PHPUnit91Migration:risky.
--RULESET--
{"@PHPUnit91Migration:risky": true}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

use PHPUnit_Framework_Assert;
use PHPUnit_Framework_BaseTestListener;
use PHPUnit_Framework_TestListener;
use PHPUnit_Aaa;
use PHPUnit_Aaa_Bbb;
use PHPUnit_Aaa_Bbb_Ccc;
use PHPUnit_Aaa_Bbb_Ccc_Ddd;
use PHPUnit_Aaa_Bbb_Ccc_Ddd_Eee;

class FooTest extends \PHPUnit_Framework_TestCase {
public function test_dedicate_assert($foo) {
$this->assertFalse(is_dir($foo));
$this->assertFalse(is_writable($foo));
$this->assertFalse(file_exists($foo));
$this->assertFalse(is_readable($foo));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

use PHPUnit\Framework\Assert;
use PHPUnit\Framework\BaseTestListener;
use PHPUnit\Framework\TestListener;
use PHPUnit\Aaa;
use PHPUnit\Aaa\Bbb;
use PHPUnit\Aaa\Bbb\Ccc;
use PHPUnit\Aaa\Bbb\Ccc\Ddd;
use PHPUnit\Aaa\Bbb\Ccc\Ddd\Eee;

class FooTest extends \PHPUnit\Framework\TestCase {
public function test_dedicate_assert($foo) {
$this->assertDirectoryDoesNotExist($foo);
$this->assertIsNotWritable($foo);
$this->assertFileDoesNotExist($foo);
$this->assertIsNotReadable($foo);
}
}
1 change: 1 addition & 0 deletions tests/RuleSet/RuleSetsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ public function testHasIntegrationTest(string $setDefinitionName): void
'@PHPUnit55Migration:risky',
'@PHPUnit75Migration:risky',
'@PHPUnit84Migration:risky',
'@PHPUnit91Migration:risky',
'@PHPUnit100Migration:risky',
'@PSR1',
];
Expand Down

0 comments on commit 0193955

Please sign in to comment.