Skip to content

Commit

Permalink
PHPMD - limit CyclomaticComplexity (#160)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kubawerlos
kubawerlos committed Oct 20, 2019
1 parent 38f15e9 commit f62f798
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 81 deletions.
15 changes: 14 additions & 1 deletion phpmd.xml
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
<?xml version='1.0' encoding='UTF-8'?>

<ruleset xmlns='http://pmd.sf.net/ruleset/1.0.0'
<ruleset name='kubawerlos/php-cs-fixer-custom-fixers'
xmlns='http://pmd.sf.net/ruleset/1.0.0'
xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance'
xsi:schemaLocation='http://pmd.sf.net/ruleset/1.0.0 http://pmd.sf.net/ruleset_xml_schema.xsd'
xsi:noNamespaceSchemaLocation='http://pmd.sf.net/ruleset_xml_schema.xsd'
>

<description>Ruleset used in the project</description>

<rule ref='rulesets/codesize.xml/CyclomaticComplexity'>
<properties>
<property name='reportLevel' value='13' />
</properties>
</rule>
<rule ref='rulesets/codesize.xml/ExcessiveMethodLength' />
<rule ref='rulesets/codesize.xml/ExcessiveClassLength' />
<rule ref='rulesets/codesize.xml/ExcessiveParameterList' />
<rule ref='rulesets/codesize.xml/ExcessivePublicCount' />

<rule ref='rulesets/controversial.xml/CamelCaseClassName' />
<rule ref='rulesets/controversial.xml/CamelCaseMethodName' />
<rule ref='rulesets/controversial.xml/CamelCaseParameterName' />
Expand Down
120 changes: 63 additions & 57 deletions src/Fixer/DataProviderNameFixer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,91 +57,97 @@ public function isRisky(): bool
public function fix(\SplFileInfo $file, Tokens $tokens): void
{
$phpUnitTestCaseIndicator = new PhpUnitTestCaseIndicator();
foreach ($phpUnitTestCaseIndicator->findPhpUnitClasses($tokens) as $indexes) {
$this->fixNames($tokens, $indexes[0], $indexes[1]);
foreach ($phpUnitTestCaseIndicator->findPhpUnitClasses($tokens) as $indices) {
$this->fixNames($tokens, $indices[0], $indices[1]);
}
}

private function fixNames(Tokens $tokens, int $startIndex, int $endIndex): void
{
$dataProviderCallIndices = [];
$dataProviderUsagesCounts = [];
$dataProviderUsingFunctionNames = [];
$functionDefinitionIndices = [];
for ($index = $startIndex; $index < $endIndex; $index++) {
// if it's the function and string follows then it's function's definition
if ($tokens[$index]->isGivenKind(T_FUNCTION)) {
$functionNameIndex = $tokens->getNextNonWhitespace($index);
if ($tokens[$functionNameIndex]->isGivenKind(T_STRING)) {
$functionDefinitionIndices[$tokens[$functionNameIndex]->getContent()] = $functionNameIndex;
}
continue;
}
$functions = $this->getFunctions($tokens, $startIndex, $endIndex);

// as it's not function's definition we search for data provider usage
$dataProvidersToRename = $this->getDataProvidersToRename($functions);

if (!$tokens[$index]->isGivenKind(T_DOC_COMMENT)) {
foreach ($dataProvidersToRename as $dataProviderName => $testName) {
if (!\array_key_exists($dataProviderName, $functions)) {
continue;
}

/** @var int $functionIndex */
$functionIndex = $tokens->getTokenNotOfKindSibling(
$index,
1,
[[T_ABSTRACT], [T_COMMENT], [T_FINAL], [T_PRIVATE], [T_PROTECTED], [T_PUBLIC], [T_STATIC], [T_WHITESPACE]]
);
if (!$tokens[$functionIndex]->isGivenKind(T_FUNCTION)) {
continue;
}

$functionNameIndex = $tokens->getNextNonWhitespace($functionIndex);
if (!$tokens[$functionNameIndex]->isGivenKind(T_STRING)) {
$dataProviderNewName = $this->getProviderNameForTestName($testName);
if (\array_key_exists($dataProviderNewName, $functions)) {
continue;
}

Preg::matchAll('/@dataProvider\s+([a-zA-Z0-9._:-\\\\x7f-\xff]+)/', $tokens[$index]->getContent(), $matches);
$tokens[$functions[$dataProviderName]['name_index']] = new Token([T_STRING, $dataProviderNewName]);
$functions[$dataProviderNewName] = [];

/** @var string[] $matches */
$matches = $matches[1];

foreach ($matches as $match) {
if (!isset($dataProviderUsagesCounts[$match])) {
$dataProviderUsagesCounts[$match] = 0;
}
$dataProviderUsagesCounts[$match]++;

$dataProviderCallIndices[$match] = $index;
/** @var string $newCommentContent */
$newCommentContent = Preg::replace(
\sprintf('/(@dataProvider\s+)%s/', $dataProviderName),
\sprintf('$1%s', $dataProviderNewName),
$tokens[$functions[$testName]['doc_comment_index']]->getContent()
);

$dataProviderUsingFunctionNames[$match] = $tokens[$functionNameIndex]->getContent();
}
$tokens[$functions[$testName]['doc_comment_index']] = new Token([T_DOC_COMMENT, $newCommentContent]);
}
}

foreach ($dataProviderUsagesCounts as $dataProviderName => $numberOfCalls) {
if ($numberOfCalls > 1) {
private function getFunctions(Tokens $tokens, int $startIndex, int $endIndex): array
{
$functions = [];
for ($index = $startIndex; $index < $endIndex; $index++) {
if (!$tokens[$index]->isGivenKind(T_FUNCTION)) {
continue;
}

if (!isset($functionDefinitionIndices[$dataProviderName])) {
/** @var int $functionNameIndex */
$functionNameIndex = $tokens->getNextNonWhitespace($index);
if (!$tokens[$functionNameIndex]->isGivenKind(T_STRING)) {
continue;
}
$indices = ['name_index' => $functionNameIndex];

$dataProviderNewName = $this->getProviderNameForTestName($dataProviderUsingFunctionNames[$dataProviderName]);
if (isset($functionDefinitionIndices[$dataProviderNewName])) {
continue;
/** @var int $docCommentIndex */
$docCommentIndex = $tokens->getTokenNotOfKindSibling(
$index,
-1,
[[T_ABSTRACT], [T_COMMENT], [T_FINAL], [T_PRIVATE], [T_PROTECTED], [T_PUBLIC], [T_STATIC], [T_WHITESPACE]]
);
if ($tokens[$docCommentIndex]->isGivenKind(T_DOC_COMMENT)) {
Preg::matchAll('/@dataProvider\s+([a-zA-Z0-9._:-\\\\x7f-\xff]+)/', $tokens[$docCommentIndex]->getContent(), $matches);

$indices['doc_comment_index'] = $docCommentIndex;
$indices['data_provider_names'] = $matches[1];
}

$tokens[$functionDefinitionIndices[$dataProviderName]] = new Token([T_STRING, $dataProviderNewName]);
$functions[$tokens[$functionNameIndex]->getContent()] = $indices;
}

/** @var string $newCommentContent */
$newCommentContent = Preg::replace(
\sprintf('/(@dataProvider\s+)%s/', $dataProviderName),
\sprintf('$1%s', $dataProviderNewName),
$tokens[$dataProviderCallIndices[$dataProviderName]]->getContent()
);
return $functions;
}

$tokens[$dataProviderCallIndices[$dataProviderName]] = new Token([T_DOC_COMMENT, $newCommentContent]);
$functionDefinitionIndices[$dataProviderNewName] = $dataProviderCallIndices[$dataProviderName];
private function getDataProvidersToRename(array $functions): array
{
$dataProvidersUses = [];
foreach ($functions as $name => $indices) {
if (!\array_key_exists('data_provider_names', $indices)) {
continue;
}
foreach ($indices['data_provider_names'] as $provider) {
if (\array_key_exists($provider, $dataProvidersUses)) {
$dataProvidersUses[$provider] = '';
continue;
}
$dataProvidersUses[$provider] = $name;
}
}

return \array_filter(
$dataProvidersUses,
static function (string $name): bool {
return $name !== '';
}
);
}

private function getProviderNameForTestName(string $name): string
Expand Down
4 changes: 1 addition & 3 deletions src/Fixer/NoImportFromGlobalNamespaceFixer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ private function fixImports(Tokens $tokens, int $startIndex, int $endIndex, bool
$semicolonIndex = $tokens->getNextMeaningfulToken($classNameIndex);
if ($tokens[$semicolonIndex]->getContent() === ';') {
$imports[] = $tokens[$classNameIndex]->getContent();
for ($i = $index; $i < $semicolonIndex; $i++) {
$tokens->clearTokenAndMergeSurroundingWhitespace($i);
}
$tokens->clearRange($index, $semicolonIndex);
TokenRemover::removeWithLinesIfPossible($tokens, $semicolonIndex);
$index = $semicolonIndex + 1;
}
Expand Down
24 changes: 14 additions & 10 deletions src/Fixer/PhpUnitNoUselessReturnFixer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,7 @@ private function removeUselessReturns(Tokens $tokens, int $startIndex, int $endI
continue;
}

/** @var int $operatorIndex */
$operatorIndex = $tokens->getPrevMeaningfulToken($index);

/** @var int $referenceIndex */
$referenceIndex = $tokens->getPrevMeaningfulToken($operatorIndex);

if (!($tokens[$operatorIndex]->isGivenKind(T_OBJECT_OPERATOR) && $tokens[$referenceIndex]->equals([T_VARIABLE, '$this'], false))
&& !($tokens[$operatorIndex]->isGivenKind(T_DOUBLE_COLON) && $tokens[$referenceIndex]->equals([T_STRING, 'self'], false))
&& !($tokens[$operatorIndex]->isGivenKind(T_DOUBLE_COLON) && $tokens[$referenceIndex]->isGivenKind(T_STATIC))
) {
if (!$this->isTheSameClassCall($tokens, $index)) {
continue;
}

Expand Down Expand Up @@ -117,4 +108,17 @@ private function removeUselessReturns(Tokens $tokens, int $startIndex, int $endI
TokenRemover::removeWithLinesIfPossible($tokens, $semicolonAfterReturnIndex);
}
}

private function isTheSameClassCall(Tokens $tokens, int $index): bool
{
/** @var int $operatorIndex */
$operatorIndex = $tokens->getPrevMeaningfulToken($index);

/** @var int $referenceIndex */
$referenceIndex = $tokens->getPrevMeaningfulToken($operatorIndex);

return $tokens[$operatorIndex]->isGivenKind(T_OBJECT_OPERATOR) && $tokens[$referenceIndex]->equals([T_VARIABLE, '$this'], false)
|| $tokens[$operatorIndex]->isGivenKind(T_DOUBLE_COLON) && $tokens[$referenceIndex]->equals([T_STRING, 'self'], false)
|| $tokens[$operatorIndex]->isGivenKind(T_DOUBLE_COLON) && $tokens[$referenceIndex]->isGivenKind(T_STATIC);
}
}
28 changes: 24 additions & 4 deletions tests/Fixer/DataProviderNameFixerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,18 @@ public function testClosure()
function () { return true; };
}
/** @dataProvider reusedDataProvider */
/**
* @dataProvider reusedDataProvider
* @dataProvider provideFooCases
*/
public function testFoo() {}
/** @dataProvider reusedDataProvider */
/**
* @dataProvider reusedDataProvider
* @dataProvider provideBarCases
*/
public function testBar() {}
public function reusedDataProvider() {}
/** @dataProvider provideBazCases */
Expand All @@ -194,6 +202,8 @@ public function provideBazCases() {}
/** @dataProvider provideSomethingCases */
public function testSomething() {}
public function provideSomethingCases() {}
public function provideFooCases() {}
public function provideBarCases() {}
}',
'<?php
class FooTest extends TestCase {
Expand All @@ -206,10 +216,18 @@ public function testClosure()
function () { return true; };
}
/** @dataProvider reusedDataProvider */
/**
* @dataProvider reusedDataProvider
* @dataProvider testFooProvider
*/
public function testFoo() {}
/** @dataProvider reusedDataProvider */
/**
* @dataProvider reusedDataProvider
* @dataProvider testBarProvider
*/
public function testBar() {}
public function reusedDataProvider() {}
/** @dataProvider provideBazCases */
Expand All @@ -219,6 +237,8 @@ public function provideBazCases() {}
/** @dataProvider someDataProvider */
public function testSomething() {}
public function someDataProvider() {}
public function testFooProvider() {}
public function testBarProvider() {}
}',
];

Expand Down
12 changes: 6 additions & 6 deletions tests/Fixer/NoImportFromGlobalNamespaceFixerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,12 +277,12 @@ public function __construct($a) {}

yield [
'<?php
namespace N1; new \DateTime();
namespace N2; new \DateTime();
namespace N3; new \DateTime();
namespace N4; new \DateTime();
namespace N5; new \DateTime();
namespace N6; new \DateTime();
namespace N1; new \DateTime();
namespace N2; new \DateTime();
namespace N3; new \DateTime();
namespace N4; new \DateTime();
namespace N5; new \DateTime();
namespace N6; new \DateTime();
',
'<?php
namespace N1; use DateTime; new DateTime();
Expand Down

0 comments on commit f62f798

Please sign in to comment.