AssertEqualsIsDiscouragedRule

Author: mhujer

README.md

@@ -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?
 

src/Rules/PHPUnit/AssertEqualsIsDiscouragedRule.php

@@ -0,0 +1,79 @@
+<?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',
+			];
+		}
+
+		$fileContents = explode("\n", file_get_contents($scope->getFile()));
+		$previousLine = $fileContents[$node->getLine() - 2];
+
+		if (!preg_match('~^\s+//\s+assertEquals because(.*)~', $previousLine)) {
+			return [
+				'You should use assertSame instead of assertEquals. Or it should have a comment above with explanation: // assertEquals because ...',
+			];
+		}
+
+		return [];
+	}
+
+}

strictRules.neon

@@ -11,3 +11,7 @@ services:
 		class: PHPStan\Rules\PHPUnit\AssertSameWithCountRule
 		tags:
 			- phpstan.rules.rule
+	-
+		class: PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule
+		tags:
+			- phpstan.rules.rule

tests/Rules/PHPUnit/AssertEqualsIsDiscouragedRuleTest.php

@@ -0,0 +1,49 @@
+<?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()
+	{
+		$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 ...',
+				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 ...',
+				24,
+			],
+		]);
+	}
+
+}

tests/Rules/PHPUnit/data/assert-equals-is-discouraged.php

@@ -0,0 +1,30 @@
+<?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!
+		$this->assertEquals(1, '1');
+	}
+
+}