From e981fd5d69a970ddf215c2a62854ed3534eea3da Mon Sep 17 00:00:00 2001 From: Michael Moravec Date: Fri, 10 May 2019 15:04:08 +0200 Subject: [PATCH] Fix detection of unspecific assert*() with direct Assert::assert*() calls --- src/Rules/PHPUnit/AssertRuleHelper.php | 4 ++-- src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php | 2 +- src/Rules/PHPUnit/AssertSameNullExpectedRule.php | 2 +- src/Rules/PHPUnit/AssertSameWithCountRule.php | 2 +- tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php | 4 ++++ tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php | 4 ++++ tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php | 4 ++++ tests/Rules/PHPUnit/data/assert-same-boolean-expected.php | 5 +++++ tests/Rules/PHPUnit/data/assert-same-count.php | 5 +++++ tests/Rules/PHPUnit/data/assert-same-null-expected.php | 5 +++++ 10 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/Rules/PHPUnit/AssertRuleHelper.php b/src/Rules/PHPUnit/AssertRuleHelper.php index e29f86c..e8edc50 100644 --- a/src/Rules/PHPUnit/AssertRuleHelper.php +++ b/src/Rules/PHPUnit/AssertRuleHelper.php @@ -9,9 +9,9 @@ class AssertRuleHelper { - public static function isMethodOrStaticCallOnTestCase(Node $node, Scope $scope): bool + public static function isMethodOrStaticCallOnAssert(Node $node, Scope $scope): bool { - $testCaseType = new ObjectType('PHPUnit\Framework\TestCase'); + $testCaseType = new ObjectType('PHPUnit\Framework\Assert'); if ($node instanceof Node\Expr\MethodCall) { $calledOnType = $scope->getType($node->var); } elseif ($node instanceof Node\Expr\StaticCall) { diff --git a/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php b/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php index e4ad350..cb06ab6 100644 --- a/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php +++ b/src/Rules/PHPUnit/AssertSameBooleanExpectedRule.php @@ -21,7 +21,7 @@ public function getNodeType(): string */ public function processNode(Node $node, Scope $scope): array { - if (!AssertRuleHelper::isMethodOrStaticCallOnTestCase($node, $scope)) { + if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { return []; } diff --git a/src/Rules/PHPUnit/AssertSameNullExpectedRule.php b/src/Rules/PHPUnit/AssertSameNullExpectedRule.php index 9d2d9a2..39f8a99 100644 --- a/src/Rules/PHPUnit/AssertSameNullExpectedRule.php +++ b/src/Rules/PHPUnit/AssertSameNullExpectedRule.php @@ -21,7 +21,7 @@ public function getNodeType(): string */ public function processNode(Node $node, Scope $scope): array { - if (!AssertRuleHelper::isMethodOrStaticCallOnTestCase($node, $scope)) { + if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { return []; } diff --git a/src/Rules/PHPUnit/AssertSameWithCountRule.php b/src/Rules/PHPUnit/AssertSameWithCountRule.php index 9327b8b..5d0225a 100644 --- a/src/Rules/PHPUnit/AssertSameWithCountRule.php +++ b/src/Rules/PHPUnit/AssertSameWithCountRule.php @@ -20,7 +20,7 @@ public function getNodeType(): string */ public function processNode(Node $node, Scope $scope): array { - if (!AssertRuleHelper::isMethodOrStaticCallOnTestCase($node, $scope)) { + if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) { return []; } diff --git a/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php b/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php index 63c608e..ac95c35 100644 --- a/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameBooleanExpectedRuleTest.php @@ -31,6 +31,10 @@ public function testRule(): void 'You should use assertFalse() instead of assertSame() when expecting "false"', 17, ], + [ + 'You should use assertTrue() instead of assertSame() when expecting "true"', + 26, + ], ]); } diff --git a/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php b/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php index 53bcf8c..1c43625 100644 --- a/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameNullExpectedRuleTest.php @@ -23,6 +23,10 @@ public function testRule(): void 'You should use assertNull() instead of assertSame(null, $actual).', 13, ], + [ + 'You should use assertNull() instead of assertSame(null, $actual).', + 24, + ], ]); } diff --git a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php index eee5e0f..cc17fa7 100644 --- a/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php +++ b/tests/Rules/PHPUnit/AssertSameWithCountRuleTest.php @@ -19,6 +19,10 @@ public function testRule(): void 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).', 10, ], + [ + 'You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).', + 20, + ], ]); } diff --git a/tests/Rules/PHPUnit/data/assert-same-boolean-expected.php b/tests/Rules/PHPUnit/data/assert-same-boolean-expected.php index 22d913e..1158a57 100644 --- a/tests/Rules/PHPUnit/data/assert-same-boolean-expected.php +++ b/tests/Rules/PHPUnit/data/assert-same-boolean-expected.php @@ -21,4 +21,9 @@ public function testAssertSameWithBooleanAsExpected() $this->assertSame($a, 'b'); // OK } + public function testAssertSameIsDetectedWithDirectAssertAccess() + { + \PHPUnit\Framework\Assert::assertSame(true, 'foo'); + } + } diff --git a/tests/Rules/PHPUnit/data/assert-same-count.php b/tests/Rules/PHPUnit/data/assert-same-count.php index 0716ec7..ab6204f 100644 --- a/tests/Rules/PHPUnit/data/assert-same-count.php +++ b/tests/Rules/PHPUnit/data/assert-same-count.php @@ -15,4 +15,9 @@ public function testAssertSameWithCountMethodIsOK() $this->assertSame(5, $this->count()); // OK } + public function testAssertSameIsDetectedWithDirectAssertAccess() + { + \PHPUnit\Framework\Assert::assertSame(5, count([1, 2, 3])); + } + } diff --git a/tests/Rules/PHPUnit/data/assert-same-null-expected.php b/tests/Rules/PHPUnit/data/assert-same-null-expected.php index e4cec93..8c7be33 100644 --- a/tests/Rules/PHPUnit/data/assert-same-null-expected.php +++ b/tests/Rules/PHPUnit/data/assert-same-null-expected.php @@ -19,4 +19,9 @@ public function testAssertSameWithNullAsExpected() $this->assertSame($c, 'foo'); // nullable is OK } + public function testAssertSameIsDetectedWithDirectAssertAccess() + { + \PHPUnit\Framework\Assert::assertSame(null, 'foo'); + } + }