AssertSameBooleanExpectedRule, AssertSameNullExpectedRule - report on…

…ly ConstFetch * Add more tests for AssertSameBooleanExpectedRule * Report only direct true|false for assertTrue()|asertFalse() * Add more tests for AssertSameNullExpectedRule * Report only direct null for assertNull()
phpstan · Apr 20, 2022 · 4a3c437 · 4a3c437
1 parent 2832aad
commit 4a3c437
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 20 deletions.
diff --git a/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php b/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php
@@ -3,12 +3,12 @@
 namespace PHPStan\Rules\PHPUnit;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr\ConstFetch;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\StaticCall;
 use PhpParser\NodeAbstract;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
-use PHPStan\Type\Constant\ConstantBooleanType;
 use function count;
 use function strtolower;
 
@@ -39,20 +39,24 @@ public function processNode(Node $node, Scope $scope): array
 			return [];
 		}
 
-		$leftType = $scope->getType($node->getArgs()[0]->value);
-		if (!$leftType instanceof ConstantBooleanType) {
+		$expectedArgumentValue = $node->getArgs()[0]->value;
+		if (!($expectedArgumentValue instanceof ConstFetch)) {
 			return [];
 		}
 
-		if ($leftType->getValue()) {
+		if ($expectedArgumentValue->name->toLowerString() === 'true') {
 			return [
 				'You should use assertTrue() instead of assertSame() when expecting "true"',
 			];
 		}
 
-		return [
-			'You should use assertFalse() instead of assertSame() when expecting "false"',
-		];
+		if ($expectedArgumentValue->name->toLowerString() === 'false') {
+			return [
+				'You should use assertFalse() instead of assertSame() when expecting "false"',
+			];
+		}
+
+		return [];
 	}
 
 }
diff --git a/src/Rules/PHPUnit/AssertSameNullExpectedRule.php b/src/Rules/PHPUnit/AssertSameNullExpectedRule.php
@@ -3,12 +3,12 @@
 namespace PHPStan\Rules\PHPUnit;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr\ConstFetch;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\StaticCall;
 use PhpParser\NodeAbstract;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
-use PHPStan\Type\NullType;
 use function count;
 use function strtolower;
 
@@ -39,9 +39,12 @@ public function processNode(Node $node, Scope $scope): array
 			return [];
 		}
 
-		$leftType = $scope->getType($node->getArgs()[0]->value);
+		$expectedArgumentValue = $node->getArgs()[0]->value;
+		if (!($expectedArgumentValue instanceof ConstFetch)) {
+			return [];
+		}
 
-		if ($leftType instanceof NullType) {
+		if ($expectedArgumentValue->name->toLowerString() === 'null') {
 			return [
 				'You should use assertNull() instead of assertSame(null, $actual).',
 			];

diff --git a/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php b/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php
@@ -29,15 +29,15 @@ public function testRule(): void
 			],
 			[
 				'You should use assertTrue() instead of assertSame() when expecting "true"',
-				14,
+				26,
 			],
 			[
-				'You should use assertFalse() instead of assertSame() when expecting "false"',
-				17,
+				'You should use assertTrue() instead of assertSame() when expecting "true"',
+				74,
 			],
 			[
-				'You should use assertTrue() instead of assertSame() when expecting "true"',
-				26,
+				'You should use assertFalse() instead of assertSame() when expecting "false"',
+				75,
 			],
 		]);
 	}

diff --git a/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php b/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php
@@ -25,11 +25,11 @@ public function testRule(): void
 			],
 			[
 				'You should use assertNull() instead of assertSame(null, $actual).',
-				13,
+				24,
 			],
 			[
 				'You should use assertNull() instead of assertSame(null, $actual).',
-				24,
+				60,
 			],
 		]);
 	}

diff --git a/tests/Rules/PHPUnit/data/assert-same-boolean-expected.php b/tests/Rules/PHPUnit/data/assert-same-boolean-expected.php
@@ -11,10 +11,10 @@ public function testAssertSameWithBooleanAsExpected()
 		$this->assertSame(false, 'a');
 
 		$truish = true;
-		$this->assertSame($truish, true);
+		$this->assertSame($truish, true); // using variable is OK
 
 		$falsish = false;
-		$this->assertSame($falsish, false);
+		$this->assertSame($falsish, false); // using variable is OK
 
 		/** @var bool $a */
 		$a = null;
@@ -26,4 +26,56 @@ public function testAssertSameIsDetectedWithDirectAssertAccess()
 		\PHPUnit\Framework\Assert::assertSame(true, 'foo');
 	}
 
+	public function testConstants(): void
+	{
+		\PHPUnit\Framework\Assert::assertSame(PHPSTAN_PHPUNIT_TRUE, 'foo');
+		\PHPUnit\Framework\Assert::assertSame(PHPSTAN_PHPUNIT_FALSE, 'foo');
+	}
+
+	private const TRUE = true;
+	private const FALSE = false;
+
+	public function testClassConstants(): void
+	{
+		\PHPUnit\Framework\Assert::assertSame(self::TRUE, 'foo');
+		\PHPUnit\Framework\Assert::assertSame(self::FALSE, 'foo');
+	}
+
+	public function returnBool(): bool
+	{
+		return true;
+	}
+
+	/**
+	 * @return true
+	 */
+	public function returnTrue(): bool
+	{
+		return true;
+	}
+
+	/**
+	 * @return false
+	 */
+	public function returnFalse(): bool
+	{
+		return false;
+	}
+
+	public function testMethodCalls(): void
+	{
+		\PHPUnit\Framework\Assert::assertSame($this->returnTrue(), 'foo');
+		\PHPUnit\Framework\Assert::assertSame($this->returnFalse(), 'foo');
+		\PHPUnit\Framework\Assert::assertSame($this->returnBool(), 'foo');
+	}
+
+	public function testNonLowercase(): void
+	{
+		\PHPUnit\Framework\Assert::assertSame(True, 'foo');
+		\PHPUnit\Framework\Assert::assertSame(False, 'foo');
+	}
+
 }
+
+const PHPSTAN_PHPUNIT_TRUE = true;
+const PHPSTAN_PHPUNIT_FALSE = false;
diff --git a/tests/Rules/PHPUnit/data/assert-same-null-expected.php b/tests/Rules/PHPUnit/data/assert-same-null-expected.php
@@ -10,7 +10,7 @@ public function testAssertSameWithNullAsExpected()
 		$this->assertSame(null, 'a');
 
 		$a = null;
-		$this->assertSame($a, 'b');
+		$this->assertSame($a, 'b'); // using variable is OK
 
 		$this->assertSame('a', 'b'); // OK
 
@@ -24,4 +24,42 @@ public function testAssertSameIsDetectedWithDirectAssertAccess()
 		\PHPUnit\Framework\Assert::assertSame(null, 'foo');
 	}
 
+	public function testConstant(): void
+	{
+		\PHPUnit\Framework\Assert::assertSame(PHPSTAN_PHPUNIT_NULL, 'foo');
+	}
+
+	private const NULL = null;
+
+	public function testClassConstant(): void
+	{
+		\PHPUnit\Framework\Assert::assertSame(self::NULL, 'foo');
+	}
+
+	public function returnNullable(): ?string
+	{
+
+	}
+
+	/**
+	 * @return null
+	 */
+	public function returnNull()
+	{
+		return null;
+	}
+
+	public function testMethodCalls(): void
+	{
+		\PHPUnit\Framework\Assert::assertSame($this->returnNull(), 'foo');
+		\PHPUnit\Framework\Assert::assertSame($this->returnNullable(), 'foo');
+	}
+
+	public function testNonLowercase(): void
+	{
+		\PHPUnit\Framework\Assert::assertSame(Null, 'foo');
+	}
+
 }
+
+const PHPSTAN_PHPUNIT_NULL = null;
-Original file line number
+Diff line change
@@ Expand Up / @@ -25,11 +25,11 @@ public function testRule(): void @@
     			],
     			[
     				'You should use assertNull() instead of assertSame(null, $actual).',
-,
+,
     			],
     			[
     				'You should use assertNull() instead of assertSame(null, $actual).',
-,
+,
     			],
     		]);
     	}
@@ Expand Down @@