Skip to content

Commit

Permalink
AssertEqualsIsDiscouragedRule
Browse files Browse the repository at this point in the history
  • Loading branch information
mhujer committed Jan 21, 2018
1 parent 1187ed5 commit ac956fe
Show file tree
Hide file tree
Showing 5 changed files with 184 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ It also contains this strict framework-specific rules (can be enabled separately
* Check that you are not using `assertSame()` with `false` as expected value. `assertFalse()` should be used instead.
* Check that you are not using `assertSame()` with `null` as expected value. `assertNull()` should be used instead.
* Check that you are not using `assertSame()` with `count($variable)` as second parameter. `assertCount($variable)` should be used instead.
* Check that you are not using `assertEquals()` with same types (`assertSame()` should be used) or without explanatory comment above.

## How to document mock objects in phpDocs?

Expand Down
92 changes: 92 additions & 0 deletions src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php
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())) {
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 [];
}

}
4 changes: 4 additions & 0 deletions strictRules.neon
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ services:
class: PHPStan\Rules\PHPUnit\AssertSameWithCountRule
tags:
- phpstan.rules.rule
-
class: PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule
tags:
- phpstan.rules.rule
53 changes: 53 additions & 0 deletions tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php
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"',
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,
],
]);
}

}
34 changes: 34 additions & 0 deletions tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php
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');
}

}

0 comments on commit ac956fe

Please sign in to comment.