-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CodeQuality] Skip call parent::__construct() indirect parent on ConstructClassMethodToSetUpTestCaseRector #251
Changes from all commits
8ecfd95
95018d8
3d533e2
fe88653
e700d52
1972906
b9c5aca
e3b0e1c
9ca5e41
719d28f
33e7329
5e4999d
8c182f2
7e389a7
b6321ff
5044935
ed9184e
00a4489
e39796b
48e1b75
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<?php | ||
|
||
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\ConstructClassMethodToSetUpTestCaseRector\Fixture; | ||
|
||
use Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\ConstructClassMethodToSetUpTestCaseRector\Source\TestResponse; | ||
|
||
final class SkipCallParentConstructExists extends TestResponse | ||
{ | ||
private $someValue; | ||
|
||
public function __construct() | ||
{ | ||
$this->someValue = 1000; | ||
|
||
parent::__construct(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
<?php | ||
|
||
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\ConstructClassMethodToSetUpTestCaseRector\Fixture; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
|
||
final class SkipParameterUsed extends TestCase | ||
{ | ||
public function __construct($param) | ||
{ | ||
$this->initEarly($param); | ||
} | ||
|
||
private function initEarly($param) | ||
{ | ||
echo 'init'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\ConstructClassMethodToSetUpTestCaseRector\Source; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
|
||
abstract class TestResponse extends TestCase | ||
{ | ||
public function __construct() | ||
{ | ||
echo "init"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
use Rector\PHPUnit\Tests\ConfigList; | ||
|
||
use Rector\Config\RectorConfig; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,7 +71,7 @@ public function refactor(Node $node): ?Node | |
return null; | ||
} | ||
|
||
if ($this->isName($node->name, 'test*')) { | ||
if (fnmatch('test*', $node->name->toString(), FNM_NOESCAPE)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TomasVotruba @staabm using |
||
return null; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,7 +93,8 @@ public function refactor(Node $node): ?Node | |
return null; | ||
} | ||
|
||
$firstArgumentName = $this->getName($firstArgumentValue); | ||
$firstArgumentName = $this->resolveFirstArgument($firstArgumentValue); | ||
|
||
if ($firstArgumentName === null || ! array_key_exists( | ||
$firstArgumentName, | ||
self::FUNCTION_NAME_WITH_ASSERT_METHOD_NAMES | ||
|
@@ -132,6 +133,13 @@ public function refactor(Node $node): ?Node | |
return $node; | ||
} | ||
|
||
private function resolveFirstArgument(FuncCall|Empty_ $firstArgumentValue): ?string | ||
{ | ||
return $firstArgumentValue instanceof Empty_ | ||
? 'empty' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TomasVotruba this is due to removal of EmptyNameResolver PR: detected after rules-tests registered to phpunit.xml |
||
: $this->getName($firstArgumentValue); | ||
} | ||
|
||
private function renameMethod( | ||
MethodCall|StaticCall $node, | ||
FunctionNameWithAssertMethods $functionNameWithAssertMethods | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
use PhpParser\Node; | ||
use PhpParser\Node\Expr\MethodCall; | ||
use PhpParser\Node\Expr\StaticCall; | ||
use PhpParser\Node\Identifier; | ||
use PhpParser\Node\Stmt\ClassMethod; | ||
use PHPStan\Type\TypeWithClassName; | ||
use Rector\Core\PhpParser\AstResolver; | ||
|
@@ -161,16 +162,31 @@ private function resolveClassMethodFromCall(StaticCall | MethodCall $call): ?Cla | |
|
||
private function isAssertMethodName(MethodCall|StaticCall $call): bool | ||
{ | ||
return $this->nodeNameResolver->isNames($call->name, [ | ||
// phpunit | ||
if (! $call->name instanceof Identifier) { | ||
return false; | ||
} | ||
|
||
$callname = $call->name->toString(); | ||
|
||
foreach (['doTestFileInfo', 'expectNotToPerformAssertions'] as $methodCallName) { | ||
if ($callname === $methodCallName) { | ||
return true; | ||
} | ||
} | ||
|
||
foreach ([ | ||
'*assert', | ||
'assert*', | ||
'expectException*', | ||
'setExpectedException*', | ||
'expectOutput*', | ||
'should*', | ||
'doTestFileInfo', | ||
'expectNotToPerformAssertions', | ||
]); | ||
] as $methodCallNamePattern) { | ||
if (fnmatch($methodCallNamePattern, $callname, FNM_NOESCAPE)) { | ||
return true; | ||
} | ||
} | ||
Comment on lines
+184
to
+188
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TomasVotruba @staabm here the tweak fnmatch in loop :) |
||
|
||
return false; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TomasVotruba here
rules-tests
need to be registered to show the PHPUnit error, it was never executed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn, that's a big miss from my side 🤦 😅