-
Notifications
You must be signed in to change notification settings - Fork 46
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
5 changed files
with
181 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
<?php declare(strict_types = 1); | ||
|
||
namespace PHPStan\Rules\PHPUnit; | ||
|
||
use PhpParser\Node; | ||
use PHPStan\Analyser\Scope; | ||
use PHPStan\Type\BooleanType; | ||
use PHPStan\Type\FloatType; | ||
use PHPStan\Type\IntegerType; | ||
use PHPStan\Type\StringType; | ||
|
||
class AssertEqualsIsDiscouragedRule implements \PHPStan\Rules\Rule | ||
{ | ||
|
||
public function getNodeType(): string | ||
{ | ||
return \PhpParser\NodeAbstract::class; | ||
} | ||
|
||
/** | ||
* @param \PhpParser\Node\Expr\MethodCall|\PhpParser\Node\Expr\StaticCall $node | ||
* @param \PHPStan\Analyser\Scope $scope | ||
* @return string[] errors | ||
*/ | ||
public function processNode(Node $node, Scope $scope): array | ||
{ | ||
if (!AssertRuleHelper::isMethodOrStaticCallOnTestCase($node, $scope)) { | ||
return []; | ||
} | ||
|
||
if (count($node->args) < 2) { | ||
return []; | ||
} | ||
if (!is_string($node->name) || strtolower($node->name) !== 'assertequals') { | ||
return []; | ||
} | ||
|
||
$leftType = $scope->getType($node->args[0]->value); | ||
$rightType = $scope->getType($node->args[1]->value); | ||
|
||
if ( | ||
($leftType instanceof BooleanType && $rightType instanceof BooleanType) | ||
|| ($leftType instanceof IntegerType && $rightType instanceof IntegerType) | ||
|| ($leftType instanceof StringType && $rightType instanceof StringType) | ||
) { | ||
$typeDescription = $leftType->describe(); | ||
if ($leftType instanceof BooleanType) { | ||
$typeDescription = 'bool'; | ||
} | ||
return [ | ||
sprintf( | ||
'You should use assertSame instead of assertEquals, because both values are of the same type "%s"', | ||
$typeDescription | ||
), | ||
]; | ||
} | ||
if ( | ||
($leftType instanceof FloatType && $rightType instanceof FloatType) | ||
&& count($node->args) < 4 // is not using delta for comparing floats | ||
) { | ||
return [ | ||
'You should use assertSame instead of assertEquals, because both values are of the same type "float" and you are not using $delta argument', | ||
]; | ||
} | ||
|
||
if (!$node->hasAttribute('comments')) { | ||
return [ | ||
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ...', | ||
]; | ||
} | ||
|
||
/** @var \PhpParser\Comment[] $comments */ | ||
$comments = $node->getAttribute('comments'); | ||
$comment = $comments[count($comments) - 1]; | ||
|
||
// the comment should be on the line above the assertEquals() | ||
if ($comment->getLine() !== ($node->getLine() - 1)) { | ||
return [ | ||
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (The comment is not directly above the assertEquals)', | ||
]; | ||
} | ||
|
||
if (preg_match('~^//\s+assertEquals because(.*)~', $comment->getReformattedText()) === 0) { | ||
return [ | ||
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (There is a different comment)', | ||
]; | ||
} | ||
|
||
return []; | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
<?php declare(strict_types = 1); | ||
|
||
namespace PHPStan\Rules\PHPUnit; | ||
|
||
use PHPStan\Rules\Rule; | ||
|
||
class AssertEqualsIsDiscouragedRuleTest extends \PHPStan\Testing\RuleTestCase | ||
{ | ||
|
||
protected function getRule(): Rule | ||
{ | ||
return new AssertEqualsIsDiscouragedRule(); | ||
} | ||
|
||
public function testRule(): void | ||
{ | ||
$this->analyse([__DIR__ . '/data/assert-equals-is-discouraged.php'], [ | ||
[ | ||
'You should use assertSame instead of assertEquals, because both values are of the same type "string"', | ||
11, | ||
], | ||
[ | ||
'You should use assertSame instead of assertEquals, because both values are of the same type "int(1)"', | ||
12, | ||
], | ||
[ | ||
'You should use assertSame instead of assertEquals, because both values are of the same type "bool"', | ||
13, | ||
], | ||
[ | ||
'You should use assertSame instead of assertEquals, because both values are of the same type "float" and you are not using $delta argument', | ||
16, | ||
], | ||
[ | ||
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (There is a different comment)', | ||
19, | ||
], | ||
[ | ||
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ...', | ||
21, | ||
], | ||
[ | ||
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (There is a different comment)', | ||
24, | ||
], | ||
[ | ||
'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ... (The comment is not directly above the assertEquals)', | ||
28, | ||
], | ||
]); | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
<?php declare(strict_types = 1); | ||
|
||
namespace ExampleTestCase; | ||
|
||
class AssertEqualsIsDiscouragedTestCase extends \PHPUnit\Framework\TestCase | ||
{ | ||
|
||
public function testAssertEqualsIsDiscouraged() | ||
{ | ||
// assertSame can be used as both are of same type | ||
$this->assertEquals('a', 'b'); | ||
$this->assertEquals(1, 2); | ||
$this->assertEquals(true, false); | ||
|
||
// comparing floats without delta | ||
$this->assertEquals(1.0, 2.0); | ||
|
||
// comparing floats with delta | ||
$this->assertEquals(1.0, 2.0, '', 0.01); | ||
|
||
$this->assertEquals(1, '1'); // assertEquals without comment on previous line | ||
|
||
// with incorrect comment | ||
$this->assertEquals(1, '1'); | ||
|
||
// assertEquals because I want it! But sadly, the comment is not just above the assert. | ||
|
||
$this->assertEquals(1, '1'); | ||
|
||
// assertEquals because I want it! | ||
$this->assertEquals(1, '1'); | ||
} | ||
|
||
} |