Skip to content

Commit

Permalink
feature #4328 Add PhpUnitDedicateAssertInternalTypeFixer (Slamdunk)
Browse files Browse the repository at this point in the history
This PR was squashed before being merged into the 2.15-dev branch (closes #4328).

Discussion
----------

Add PhpUnitDedicateAssertInternalTypeFixer

Closes #4144

- [x] Update `RuleSet`
- [x] Add integration test with `PhpUnitDedicateAssertInternalTypeFixer`

I intentionally created a new fixer instead of updating [PhpUnitDedicateAssertInternalTypeFixer](https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/v2.14.2/src/Fixer/PhpUnit/PhpUnitDedicateAssertFixer.php) because the codes need too different logics.

I don't really like the names, they may confuse the users, any suggestion is welcome, but I would like to keep the two fixers in two different classes.

Commits
-------

f721b4b Add PhpUnitDedicateAssertInternalTypeFixer
  • Loading branch information
SpacePossum committed Mar 19, 2019
2 parents 47b2711 + f721b4b commit bb4651c
Show file tree
Hide file tree
Showing 7 changed files with 438 additions and 5 deletions.
22 changes: 17 additions & 5 deletions README.rst
Expand Up @@ -1266,7 +1266,7 @@ Choose from the list of available rules:
defaults to ``['assertEquals', 'assertSame', 'assertNotEquals',
'assertNotSame']``

* **php_unit_dedicate_assert** [@PHPUnit30Migration:risky, @PHPUnit32Migration:risky, @PHPUnit35Migration:risky, @PHPUnit43Migration:risky, @PHPUnit48Migration:risky, @PHPUnit50Migration:risky, @PHPUnit52Migration:risky, @PHPUnit54Migration:risky, @PHPUnit55Migration:risky, @PHPUnit56Migration:risky, @PHPUnit57Migration:risky, @PHPUnit60Migration:risky]
* **php_unit_dedicate_assert** [@PHPUnit30Migration:risky, @PHPUnit32Migration:risky, @PHPUnit35Migration:risky, @PHPUnit43Migration:risky, @PHPUnit48Migration:risky, @PHPUnit50Migration:risky, @PHPUnit52Migration:risky, @PHPUnit54Migration:risky, @PHPUnit55Migration:risky, @PHPUnit56Migration:risky, @PHPUnit57Migration:risky, @PHPUnit60Migration:risky, @PHPUnit75Migration:risky]

PHPUnit assertions like ``assertInternalType``, ``assertFileExists``, should
be used over ``assertTrue``.
Expand All @@ -1284,7 +1284,19 @@ Choose from the list of available rules:
- ``target`` (``'3.0'``, ``'3.5'``, ``'5.0'``, ``'5.6'``, ``'newest'``): target version of
PHPUnit; defaults to ``'5.0'``

* **php_unit_expectation** [@PHPUnit52Migration:risky, @PHPUnit54Migration:risky, @PHPUnit55Migration:risky, @PHPUnit56Migration:risky, @PHPUnit57Migration:risky, @PHPUnit60Migration:risky]
* **php_unit_dedicate_assert_internal_type** [@PHPUnit75Migration:risky]

PHPUnit assertions like ``assertIsArray`` should be used over
``assertInternalType``.

*Risky rule: risky when PHPUnit methods are overridden or when project has PHPUnit incompatibilities.*

Configuration options:

- ``target`` (``'7.5'``, ``'newest'``): target version of PHPUnit; defaults to
``'newest'``

* **php_unit_expectation** [@PHPUnit52Migration:risky, @PHPUnit54Migration:risky, @PHPUnit55Migration:risky, @PHPUnit56Migration:risky, @PHPUnit57Migration:risky, @PHPUnit60Migration:risky, @PHPUnit75Migration:risky]

Usages of ``->setExpectedException*`` methods MUST be replaced by
``->expectException*`` methods.
Expand Down Expand Up @@ -1319,7 +1331,7 @@ Choose from the list of available rules:
- ``case`` (``'camel_case'``, ``'snake_case'``): apply camel or snake case to test
methods; defaults to ``'camel_case'``

* **php_unit_mock** [@PHPUnit54Migration:risky, @PHPUnit55Migration:risky, @PHPUnit56Migration:risky, @PHPUnit57Migration:risky, @PHPUnit60Migration:risky]
* **php_unit_mock** [@PHPUnit54Migration:risky, @PHPUnit55Migration:risky, @PHPUnit56Migration:risky, @PHPUnit57Migration:risky, @PHPUnit60Migration:risky, @PHPUnit75Migration:risky]

Usages of ``->getMock`` and
``->getMockWithoutInvokingTheOriginalConstructor`` methods MUST be
Expand All @@ -1332,7 +1344,7 @@ Choose from the list of available rules:
- ``target`` (``'5.4'``, ``'5.5'``, ``'newest'``): target version of PHPUnit; defaults to
``'newest'``

* **php_unit_namespaced** [@PHPUnit48Migration:risky, @PHPUnit50Migration:risky, @PHPUnit52Migration:risky, @PHPUnit54Migration:risky, @PHPUnit55Migration:risky, @PHPUnit56Migration:risky, @PHPUnit57Migration:risky, @PHPUnit60Migration:risky]
* **php_unit_namespaced** [@PHPUnit48Migration:risky, @PHPUnit50Migration:risky, @PHPUnit52Migration:risky, @PHPUnit54Migration:risky, @PHPUnit55Migration:risky, @PHPUnit56Migration:risky, @PHPUnit57Migration:risky, @PHPUnit60Migration:risky, @PHPUnit75Migration:risky]

PHPUnit classes MUST be used in namespaced version, e.g.
``\PHPUnit\Framework\TestCase`` instead of ``\PHPUnit_Framework_TestCase``.
Expand All @@ -1344,7 +1356,7 @@ Choose from the list of available rules:
- ``target`` (``'4.8'``, ``'5.7'``, ``'6.0'``, ``'newest'``): target version of PHPUnit;
defaults to ``'newest'``

* **php_unit_no_expectation_annotation** [@PHPUnit32Migration:risky, @PHPUnit35Migration:risky, @PHPUnit43Migration:risky, @PHPUnit48Migration:risky, @PHPUnit50Migration:risky, @PHPUnit52Migration:risky, @PHPUnit54Migration:risky, @PHPUnit55Migration:risky, @PHPUnit56Migration:risky, @PHPUnit57Migration:risky, @PHPUnit60Migration:risky]
* **php_unit_no_expectation_annotation** [@PHPUnit32Migration:risky, @PHPUnit35Migration:risky, @PHPUnit43Migration:risky, @PHPUnit48Migration:risky, @PHPUnit50Migration:risky, @PHPUnit52Migration:risky, @PHPUnit54Migration:risky, @PHPUnit55Migration:risky, @PHPUnit56Migration:risky, @PHPUnit57Migration:risky, @PHPUnit60Migration:risky, @PHPUnit75Migration:risky]

Usages of ``@expectedException*`` annotations MUST be replaced by
``->setExpectedException*`` methods.
Expand Down
200 changes: 200 additions & 0 deletions src/Fixer/PhpUnit/PhpUnitDedicateAssertInternalTypeFixer.php
@@ -0,0 +1,200 @@
<?php

/*
* 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\Fixer\PhpUnit;

use PhpCsFixer\AbstractFixer;
use PhpCsFixer\Fixer\ConfigurationDefinitionFixerInterface;
use PhpCsFixer\FixerConfiguration\FixerConfigurationResolver;
use PhpCsFixer\FixerConfiguration\FixerOptionBuilder;
use PhpCsFixer\FixerDefinition\CodeSample;
use PhpCsFixer\FixerDefinition\FixerDefinition;
use PhpCsFixer\Indicator\PhpUnitTestCaseIndicator;
use PhpCsFixer\Tokenizer\Token;
use PhpCsFixer\Tokenizer\Tokens;
use PhpCsFixer\Tokenizer\TokensAnalyzer;

/**
* @author Filippo Tessarotto <zoeslam@gmail.com>
*/
final class PhpUnitDedicateAssertInternalTypeFixer extends AbstractFixer implements ConfigurationDefinitionFixerInterface
{
/**
* @var array
*/
private $typeToDedicatedAssertMap = [
'array' => 'assertIsArray',
'boolean' => 'assertIsBool',
'bool' => 'assertIsBool',
'double' => 'assertIsFloat',
'float' => 'assertIsFloat',
'integer' => 'assertIsInt',
'int' => 'assertIsInt',
'null' => 'assertNull',
'numeric' => 'assertIsNumeric',
'object' => 'assertIsObject',
'real' => 'assertIsFloat',
'resource' => 'assertIsResource',
'string' => 'assertIsString',
'scalar' => 'assertIsScalar',
'callable' => 'assertIsCallable',
'iterable' => 'assertIsIterable',
];

/**
* {@inheritdoc}
*/
public function getDefinition()
{
return new FixerDefinition(
'PHPUnit assertions like `assertIsArray` should be used over `assertInternalType`.',
[
new CodeSample(
'<?php
final class MyTest extends \PHPUnit\Framework\TestCase
{
public function testMe()
{
$this->assertInternalType("array", $var);
$this->assertInternalType("boolean", $var);
}
}
'
),
],
null,
'Risky when PHPUnit methods are overridden or when project has PHPUnit incompatibilities.'
);
}

/**
* {@inheritdoc}
*/
public function isCandidate(Tokens $tokens)
{
return $tokens->isAllTokenKindsFound([T_CLASS, T_FUNCTION]);
}

/**
* {@inheritdoc}
*/
public function isRisky()
{
return true;
}

/**
* {@inheritdoc}
*/
public function getPriority()
{
// should be run after the PhpUnitDedicateAssertFixer
return -16;
}

/**
* {@inheritdoc}
*/
protected function applyFix(\SplFileInfo $file, Tokens $tokens)
{
$phpUnitTestCaseIndicator = new PhpUnitTestCaseIndicator();
foreach ($phpUnitTestCaseIndicator->findPhpUnitClasses($tokens) as $indexes) {
$this->updateAssertInternalTypeMethods($tokens, $indexes[0], $indexes[1]);
}
}

/**
* {@inheritdoc}
*/
protected function createConfigurationDefinition()
{
return new FixerConfigurationResolver([
(new FixerOptionBuilder('target', 'Target version of PHPUnit.'))
->setAllowedTypes(['string'])
->setAllowedValues([PhpUnitTargetVersion::VERSION_7_5, PhpUnitTargetVersion::VERSION_NEWEST])
->setDefault(PhpUnitTargetVersion::VERSION_NEWEST)
->getOption(),
]);
}

/**
* @param Tokens $tokens
* @param int $startIndex
* @param int $endIndex
*/
private function updateAssertInternalTypeMethods(Tokens $tokens, $startIndex, $endIndex)
{
$anonymousClassIndexes = [];
$tokenAnalyzer = new TokensAnalyzer($tokens);
for ($index = $startIndex; $index < $endIndex; ++$index) {
if (!$tokens[$index]->isClassy() || !$tokenAnalyzer->isAnonymousClass($index)) {
continue;
}

$openingBraceIndex = $tokens->getNextTokenOfKind($index, ['{']);
$closingBraceIndex = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_CURLY_BRACE, $openingBraceIndex);

$anonymousClassIndexes[$closingBraceIndex] = $openingBraceIndex;
}

for ($index = $endIndex - 1; $index > $startIndex; --$index) {
if (isset($anonymousClassIndexes[$index])) {
$index = $anonymousClassIndexes[$index];

continue;
}

if (!$tokens[$index]->isGivenKind(T_STRING)) {
continue;
}

$functionName = strtolower($tokens[$index]->getContent());
if ('assertinternaltype' !== $functionName && 'assertnotinternaltype' !== $functionName) {
continue;
}

$bracketTokenIndex = $tokens->getNextMeaningfulToken($index);
if (!$tokens[$bracketTokenIndex]->equals('(')) {
continue;
}

$expectedTypeTokenIndex = $tokens->getNextMeaningfulToken($bracketTokenIndex);
$expectedTypeToken = $tokens[$expectedTypeTokenIndex];
if (!$expectedTypeToken->equals([T_CONSTANT_ENCAPSED_STRING])) {
continue;
}

$expectedType = trim($expectedTypeToken->getContent(), '\'"');
if (!isset($this->typeToDedicatedAssertMap[$expectedType])) {
continue;
}

$commaTokenIndex = $tokens->getNextMeaningfulToken($expectedTypeTokenIndex);
if (!$tokens[$commaTokenIndex]->equals(',')) {
continue;
}

$newAssertion = $this->typeToDedicatedAssertMap[$expectedType];
if ('assertnotinternaltype' === $functionName) {
$newAssertion = str_replace('Is', 'IsNot', $newAssertion);
$newAssertion = str_replace('Null', 'NotNull', $newAssertion);
}

$nextMeaningfulTokenIndex = $tokens->getNextMeaningfulToken($commaTokenIndex);

$tokens->overrideRange($index, $nextMeaningfulTokenIndex - 1, [
new Token([T_STRING, $newAssertion]),
new Token('('),
]);
}
}
}
1 change: 1 addition & 0 deletions src/Fixer/PhpUnit/PhpUnitTargetVersion.php
Expand Up @@ -33,6 +33,7 @@ final class PhpUnitTargetVersion
const VERSION_5_6 = '5.6';
const VERSION_5_7 = '5.7';
const VERSION_6_0 = '6.0';
const VERSION_7_5 = '7.5';
const VERSION_NEWEST = 'newest';

private function __construct()
Expand Down
4 changes: 4 additions & 0 deletions src/RuleSet.php
Expand Up @@ -368,6 +368,10 @@ final class RuleSet implements RuleSetInterface
'@PHPUnit57Migration:risky' => true,
'php_unit_namespaced' => ['target' => PhpUnitTargetVersion::VERSION_6_0],
],
'@PHPUnit75Migration:risky' => [
'@PHPUnit60Migration:risky' => true,
'php_unit_dedicate_assert_internal_type' => ['target' => PhpUnitTargetVersion::VERSION_7_5],
],
];

/**
Expand Down
1 change: 1 addition & 0 deletions tests/AutoReview/FixerFactoryTest.php
Expand Up @@ -165,6 +165,7 @@ public function provideFixersPriorityCases()
[$fixers['php_unit_fqcn_annotation'], $fixers['php_unit_ordered_covers']],
[$fixers['php_unit_no_expectation_annotation'], $fixers['no_empty_phpdoc']],
[$fixers['php_unit_no_expectation_annotation'], $fixers['php_unit_expectation']],
[$fixers['php_unit_dedicate_assert'], $fixers['php_unit_dedicate_assert_internal_type']],
[$fixers['phpdoc_add_missing_param_annotation'], $fixers['no_empty_phpdoc']],
[$fixers['phpdoc_add_missing_param_annotation'], $fixers['phpdoc_align']],
[$fixers['phpdoc_add_missing_param_annotation'], $fixers['phpdoc_order']],
Expand Down

0 comments on commit bb4651c

Please sign in to comment.