diff --git a/README.rst b/README.rst index 7147795f1fb..1d5020fbe1f 100644 --- a/README.rst +++ b/README.rst @@ -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``. @@ -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. @@ -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 @@ -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``. @@ -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. diff --git a/src/Fixer/PhpUnit/PhpUnitDedicateAssertInternalTypeFixer.php b/src/Fixer/PhpUnit/PhpUnitDedicateAssertInternalTypeFixer.php new file mode 100644 index 00000000000..6ad127be927 --- /dev/null +++ b/src/Fixer/PhpUnit/PhpUnitDedicateAssertInternalTypeFixer.php @@ -0,0 +1,200 @@ + + * Dariusz Rumiński + * + * 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 + */ +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( + '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('('), + ]); + } + } +} diff --git a/src/Fixer/PhpUnit/PhpUnitTargetVersion.php b/src/Fixer/PhpUnit/PhpUnitTargetVersion.php index bb3840ca9ea..7e5bdf802d8 100644 --- a/src/Fixer/PhpUnit/PhpUnitTargetVersion.php +++ b/src/Fixer/PhpUnit/PhpUnitTargetVersion.php @@ -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() diff --git a/src/RuleSet.php b/src/RuleSet.php index ee70472923b..b127804d0a5 100644 --- a/src/RuleSet.php +++ b/src/RuleSet.php @@ -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], + ], ]; /** diff --git a/tests/AutoReview/FixerFactoryTest.php b/tests/AutoReview/FixerFactoryTest.php index a1a7339e5e8..800716acac6 100644 --- a/tests/AutoReview/FixerFactoryTest.php +++ b/tests/AutoReview/FixerFactoryTest.php @@ -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']], diff --git a/tests/Fixer/PhpUnit/PhpUnitDedicateAssertInternalTypeFixerTest.php b/tests/Fixer/PhpUnit/PhpUnitDedicateAssertInternalTypeFixerTest.php new file mode 100644 index 00000000000..334f2bca6cd --- /dev/null +++ b/tests/Fixer/PhpUnit/PhpUnitDedicateAssertInternalTypeFixerTest.php @@ -0,0 +1,194 @@ + + * Dariusz Rumiński + * + * This source file is subject to the MIT license that is bundled + * with this source code in the file LICENSE. + */ + +namespace PhpCsFixer\Tests\Fixer\PhpUnit; + +use PhpCsFixer\Tests\Test\AbstractFixerTestCase; + +/** + * @author Filippo Tessarotto + * + * @internal + * + * @covers \PhpCsFixer\Fixer\PhpUnit\PhpUnitDedicateAssertInternalTypeFixer + */ +final class PhpUnitDedicateAssertInternalTypeFixerTest extends AbstractFixerTestCase +{ + /** + * @param string $expected + * @param null|string $input + * + * @dataProvider provideTestFixInternalTypeCases + */ + public function testFixInternalType($expected, $input = null) + { + $this->doTest($expected, $input); + } + + public function provideTestFixInternalTypeCases() + { + return [ + 'skip cases' => [ + 'assertInternalType(gettype($expectedVar), $var); + $this->assertNotInternalType(gettype($expectedVar), $var); + + $this->assertInternalType("foo", $var); + $this->assertNotInternalType("bar", $var); + + $this->assertInternalType(); + $this->assertNotInternalType(); + + $this->assertInternalType("array" . "foo", $var); + $this->assertNotInternalType(\'bool\' . "bar", $var); + } +} +', + ], + 'expected normal cases' => [ + 'assertIsArray($var); + $this->assertIsBool($var); + $this->assertIsBool($var); + $this->assertIsFloat($var); + $this->assertIsFloat($var); + $this->assertIsInt($var); + $this->assertIsInt($var); + $this->assertNull($var); + $this->assertIsNumeric($var); + $this->assertIsObject($var); + $this->assertIsFloat($var); + $this->assertIsResource($var); + $this->assertIsString($var); + $this->assertIsScalar($var); + $this->assertIsCallable($var); + $this->assertIsIterable($var); + + $this->assertIsNotArray($var); + $this->assertIsNotBool($var); + $this->assertIsNotBool($var); + $this->assertIsNotFloat($var); + $this->assertIsNotFloat($var); + $this->assertIsNotInt($var); + $this->assertIsNotInt($var); + $this->assertNotNull($var); + $this->assertIsNotNumeric($var); + $this->assertIsNotObject($var); + $this->assertIsNotFloat($var); + $this->assertIsNotResource($var); + $this->assertIsNotString($var); + $this->assertIsNotScalar($var); + $this->assertIsNotCallable($var); + $this->assertIsNotIterable($var); + } +} +', + 'assertInternalType(\'array\', $var); + $this->assertInternalType("boolean", $var); + $this->assertInternalType("bool", $var); + $this->assertInternalType("double", $var); + $this->assertInternalType("float", $var); + $this->assertInternalType("integer", $var); + $this->assertInternalType("int", $var); + $this->assertInternalType("null", $var); + $this->assertInternalType("numeric", $var); + $this->assertInternalType("object", $var); + $this->assertInternalType("real", $var); + $this->assertInternalType("resource", $var); + $this->assertInternalType("string", $var); + $this->assertInternalType("scalar", $var); + $this->assertInternalType("callable", $var); + $this->assertInternalType("iterable", $var); + + $this->assertNotInternalType("array", $var); + $this->assertNotInternalType("boolean", $var); + $this->assertNotInternalType("bool", $var); + $this->assertNotInternalType("double", $var); + $this->assertNotInternalType("float", $var); + $this->assertNotInternalType("integer", $var); + $this->assertNotInternalType("int", $var); + $this->assertNotInternalType("null", $var); + $this->assertNotInternalType("numeric", $var); + $this->assertNotInternalType("object", $var); + $this->assertNotInternalType("real", $var); + $this->assertNotInternalType("resource", $var); + $this->assertNotInternalType("string", $var); + $this->assertNotInternalType("scalar", $var); + $this->assertNotInternalType("callable", $var); + $this->assertNotInternalType("iterable", $var); + } +} +', + ], + 'false positive cases' => [ + 'assertInternalType = 42; + $this->assertNotInternalType = 43; + } +} +', + ], + ]; + } + + /** + * @param string $expected + * @param null|string $input + * + * @dataProvider provideFix70Cases + * @requires PHP 7.0 + */ + public function testFix70($expected, $input = null) + { + $this->doTest($expected, $input); + } + + public function provideFix70Cases() + { + return [ + 'anonymous class false positive case' => [ + 'assertInternalType("array", $var); + } + }; + } +} +', + ], + ]; + } +} diff --git a/tests/Fixtures/Integration/priority/php_unit_dedicate_assert,php_unit_dedicate_assert_internal_type.test b/tests/Fixtures/Integration/priority/php_unit_dedicate_assert,php_unit_dedicate_assert_internal_type.test new file mode 100644 index 00000000000..12d8f1a59f9 --- /dev/null +++ b/tests/Fixtures/Integration/priority/php_unit_dedicate_assert,php_unit_dedicate_assert_internal_type.test @@ -0,0 +1,21 @@ +--TEST-- +Integration of fixers: php_unit_dedicate_assert,php_unit_dedicate_assert_internal_type. +--RULESET-- +{"php_unit_dedicate_assert": true, "php_unit_dedicate_assert_internal_type": true} +--EXPECT-- +assertIsFloat($a); + } +} + +--INPUT-- +assertTrue(is_float($a)); + } +}