Strict and opinionated rules for PHPUnit

README.md

@@ -18,6 +18,13 @@ It also contains this framework-specific rule (can be enabled separately):
 
 * Check that both values passed to `assertSame()` method are of the same type.
 
+It also contains this strict framework-specific rules (can be enabled separately):
+
+* Check that you are not using `assertSame()` with `true` as expected value. `assertTrue()` should be used instead.
+* 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.
+
 ## How to document mock objects in phpDocs?
 
 If you need to configure the mock even after you assign it to a property or return it from a method, you should add `PHPUnit_Framework_MockObject_MockObject` to the phpDoc:
@@ -81,3 +88,9 @@ To perform framework-specific checks, include also this file:
 ```
 	- vendor/phpstan/phpstan-phpunit/rules.neon
 ```
+
+To perform addition strict PHPUnit checks, include also this file:
+
+```
+	- vendor/phpstan/phpstan-phpunit/strictRules.neon
+```

build.xml

@@ -42,7 +42,6 @@
 			<arg value="--extensions=php"/>
 			<arg value="--encoding=utf-8"/>
 			<arg value="--tab-width=4"/>
-			<arg value="--ignore=tests/*/data"/>
 			<arg value="-sp"/>
 			<arg path="src"/>
 			<arg path="tests"/>
@@ -59,7 +58,6 @@
 			<arg value="--extensions=php"/>
 			<arg value="--encoding=utf-8"/>
 			<arg value="--tab-width=4"/>
-			<arg value="--ignore=tests/*/data"/>
 			<arg value="-sp"/>
 			<arg path="src"/>
 			<arg path="tests"/>

phpcs.xml

@@ -36,4 +36,5 @@
 			<property name="enableVoidTypeHint" type="false" />
 		</properties>
 	</rule>
+	<exclude-pattern>tests/*/data</exclude-pattern>
 </ruleset>

src/Rules/PHPUnit/AssertRuleHelper.php

@@ -0,0 +1,52 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\PHPUnit;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Type\ObjectType;
+
+class AssertRuleHelper
+{
+
+	/**
+	 * @param \PhpParser\Node\Expr\MethodCall|\PhpParser\Node\Expr\StaticCall $node
+	 * @param \PHPStan\Analyser\Scope $scope
+	 * @return bool
+	 */
+	public static function isMethodOrStaticCallOnTestCase(Node $node, Scope $scope): bool
+	{
+		$testCaseType = new ObjectType(\PHPUnit\Framework\TestCase::class);
+		if ($node instanceof Node\Expr\MethodCall) {
+			$calledOnType = $scope->getType($node->var);
+		} elseif ($node instanceof Node\Expr\StaticCall) {
+			if ($node->class instanceof Node\Name) {
+				$class = (string) $node->class;
+				if (in_array(
+					strtolower($class),
+					[
+						'self',
+						'static',
+						'parent',
+					],
+					true
+				)) {
+					$calledOnType = new ObjectType($scope->getClassReflection()->getName());
+				} else {
+					$calledOnType = new ObjectType($class);
+				}
+			} else {
+				$calledOnType = $scope->getType($node->class);
+			}
+		} else {
+			return false;
+		}
+
+		if (!$testCaseType->isSuperTypeOf($calledOnType)->yes()) {
+			return false;
+		}
+
+		return true;
+	}
+
+}

src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php

@@ -0,0 +1,53 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\PHPUnit;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Type\FalseBooleanType;
+use PHPStan\Type\TrueBooleanType;
+
+class AssertSameBooleanExpectedRule 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) !== 'assertsame') {
+			return [];
+		}
+
+		$leftType = $scope->getType($node->args[0]->value);
+
+		if ($leftType instanceof TrueBooleanType) {
+			return [
+				'You should use assertTrue() instead of assertSame() when expecting "true"',
+			];
+		}
+
+		if ($leftType instanceof FalseBooleanType) {
+			return [
+				'You should use assertFalse() instead of assertSame() when expecting "false"',
+			];
+		}
+
+		return [];
+	}
+
+}

src/Rules/PHPUnit/AssertSameDifferentTypesRule.php

@@ -4,7 +4,6 @@
 
 use PhpParser\Node;
 use PHPStan\Analyser\Scope;
-use PHPStan\Type\ObjectType;
 
 class AssertSameDifferentTypesRule implements \PHPStan\Rules\Rule
 {
@@ -21,33 +20,7 @@ public function getNodeType(): string
 	 */
 	public function processNode(Node $node, Scope $scope): array
 	{
-		$testCaseType = new ObjectType(\PHPUnit\Framework\TestCase::class);
-		if ($node instanceof Node\Expr\MethodCall) {
-			$calledOnType = $scope->getType($node->var);
-		} elseif ($node instanceof Node\Expr\StaticCall) {
-			if ($node->class instanceof Node\Name) {
-				$class = (string) $node->class;
-				if (in_array(
-					strtolower($class),
-					[
-						'self',
-						'static',
-						'parent',
-					],
-					true
-				)) {
-					$calledOnType = new ObjectType($scope->getClassReflection()->getName());
-				} else {
-					$calledOnType = new ObjectType($class);
-				}
-			} else {
-				$calledOnType = $scope->getType($node->class);
-			}
-		} else {
-			return [];
-		}
-
-		if (!$testCaseType->isSuperTypeOf($calledOnType)->yes()) {
+		if (!AssertRuleHelper::isMethodOrStaticCallOnTestCase($node, $scope)) {
 			return [];
 		}
 

src/Rules/PHPUnit/AssertSameNullExpectedRule.php

@@ -0,0 +1,46 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\PHPUnit;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Type\NullType;
+
+class AssertSameNullExpectedRule 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) !== 'assertsame') {
+			return [];
+		}
+
+		$leftType = $scope->getType($node->args[0]->value);
+
+		if ($leftType instanceof NullType) {
+			return [
+				'You should use assertNull() instead of assertSame(null, $actual).',
+			];
+		}
+
+		return [];
+	}
+
+}

src/Rules/PHPUnit/AssertSameWithCountRule.php

@@ -0,0 +1,49 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\PHPUnit;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+
+class AssertSameWithCountRule 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) !== 'assertsame') {
+			return [];
+		}
+
+		$right = $node->args[1]->value;
+
+		if (
+			$right instanceof Node\Expr\FuncCall
+			&& $right->name instanceof Node\Name
+			&& strtolower($right->name->toString()) === 'count'
+		) {
+			return [
+				'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).',
+			];
+		}
+
+		return [];
+	}
+
+}

strictRules.neon

@@ -0,0 +1,13 @@
+services:
+	-
+		class: PHPStan\Rules\PHPUnit\AssertSameBooleanExpectedRule
+		tags:
+			- phpstan.rules.rule
+	-
+		class: PHPStan\Rules\PHPUnit\AssertSameNullExpectedRule
+		tags:
+			- phpstan.rules.rule
+	-
+		class: PHPStan\Rules\PHPUnit\AssertSameWithCountRule
+		tags:
+			- phpstan.rules.rule

tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php

@@ -0,0 +1,37 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\PHPUnit;
+
+use PHPStan\Rules\Rule;
+
+class AssertSameBooleanExpectedRuleTest extends \PHPStan\Testing\RuleTestCase
+{
+
+	protected function getRule(): Rule
+	{
+		return new AssertSameBooleanExpectedRule();
+	}
+
+	public function testRule()
+	{
+		$this->analyse([__DIR__ . '/data/assert-same-boolean-expected.php'], [
+			[
+				'You should use assertTrue() instead of assertSame() when expecting "true"',
+				10,
+			],
+			[
+				'You should use assertFalse() instead of assertSame() when expecting "false"',
+				11,
+			],
+			[
+				'You should use assertTrue() instead of assertSame() when expecting "true"',
+				14,
+			],
+			[
+				'You should use assertFalse() instead of assertSame() when expecting "false"',
+				17,
+			],
+		]);
+	}
+
+}

tests/Rules/PHPUnit/AssertSameDifferentTypesRuleTest.php

@@ -35,9 +35,21 @@ public function testRule()
 				'Call to assertSame() with different types array<int, string> and array<int, int> will always result in test failure.',
 				14,
 			],
+			[
+				'Call to assertSame() with different types string and int will always result in test failure.',
+				16,
+			],
+			[
+				'Call to assertSame() with different types string and int will always result in test failure.',
+				17,
+			],
+			[
+				'Call to assertSame() with different types string and int will always result in test failure.',
+				18,
+			],
 			[
 				'Call to assertSame() with different types array<string> and array<int> will always result in test failure.',
-				35,
+				39,
 			],
 		]);
 	}

tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php

@@ -0,0 +1,29 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\PHPUnit;
+
+use PHPStan\Rules\Rule;
+
+class AssertSameNullExpectedRuleTest extends \PHPStan\Testing\RuleTestCase
+{
+
+	protected function getRule(): Rule
+	{
+		return new AssertSameNullExpectedRule();
+	}
+
+	public function testRule()
+	{
+		$this->analyse([__DIR__ . '/data/assert-same-null-expected.php'], [
+			[
+				'You should use assertNull() instead of assertSame(null, $actual).',
+				10,
+			],
+			[
+				'You should use assertNull() instead of assertSame(null, $actual).',
+				13,
+			],
+		]);
+	}
+
+}