feat: Discourage assert(Not)Empty if "empty" usage is disallowed (#325)

Author: mitelg

.github/workflows/e2e-tests.yml

@@ -28,10 +28,10 @@ jobs:
           - script: |
               cd e2e/composer-version
               composer install
-              OUTPUT=$(../bashunit -a exit_code "1" "vendor/bin/phpstan analyze test.php --error-format=raw")
+              OUTPUT=$(../bashunit assert exit_code "1" "vendor/bin/phpstan analyze test.php --error-format=raw")
               echo "$OUTPUT"
-              ../bashunit -a contains 'test.php:12:Version requirement <=8.0.0 does not match 8.1.0...8.5.99.' "$OUTPUT"
-              ../bashunit -a contains 'test.php:32:Version requirement ^11.0.0 does not match 12.5.0...12.5.99.' "$OUTPUT"
+              ../bashunit assert contains 'test.php:12:Version requirement <=8.0.0 does not match 8.1.0...8.6.99.' "$OUTPUT"
+              ../bashunit assert contains 'test.php:32:Version requirement ^11.0.0 does not match 12.5.0...12.5.99.' "$OUTPUT"
 
     steps:
       - name: Harden the runner (Audit all outbound calls)

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 `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)
 * Check that you are not using `assertNotEquals()` with same types (`assertNotSame()` should be used)
+* If PHPStan Strict Rules are enabled, PHPUnit's `assertEmpty()` and `assertNotEmpty()` assertions are disallowed as well.
 
 ## How to document mock objects in phpDocs?
 

rules.neon

@@ -12,6 +12,8 @@ rules:
 conditionalTags:
 	PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule:
 		phpstan.rules.rule: [%strictRulesInstalled%, %featureToggles.bleedingEdge%]
+	PHPStan\Rules\PHPUnit\AssertEmptyIsDiscouragedRule:
+		phpstan.rules.rule: [%strictRulesInstalled%, %featureToggles.bleedingEdge%]
 
 	PHPStan\Rules\PHPUnit\DataProviderDataRule:
 		phpstan.rules.rule: %featureToggles.bleedingEdge%
@@ -39,5 +41,8 @@ services:
 	-
 		class: PHPStan\Rules\PHPUnit\AssertEqualsIsDiscouragedRule
 
+	-
+		class: PHPStan\Rules\PHPUnit\AssertEmptyIsDiscouragedRule
+
 	-
 		class: PHPStan\Rules\PHPUnit\DataProviderDataRule

src/Rules/PHPUnit/AssertEmptyIsDiscouragedRule.php

@@ -0,0 +1,53 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\PHPUnit;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\CallLike;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\StaticCall;
+use PhpParser\Node\Identifier;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use function count;
+use function in_array;
+use function sprintf;
+
+/**
+ * @implements Rule<CallLike>
+ */
+class AssertEmptyIsDiscouragedRule implements Rule
+{
+
+	public function getNodeType(): string
+	{
+		return CallLike::class;
+	}
+
+	public function processNode(Node $node, Scope $scope): array
+	{
+		if (!($node instanceof MethodCall) && !($node instanceof StaticCall)) {
+			return [];
+		}
+
+		if ($node->isFirstClassCallable() || count($node->getArgs()) < 1) {
+			return [];
+		}
+
+		if (!$node->name instanceof Identifier || !in_array($node->name->toLowerString(), ['assertempty', 'assertnotempty'], true)) {
+			return [];
+		}
+
+		if (!AssertRuleHelper::isMethodOrStaticCallOnAssert($node, $scope)) {
+			return [];
+		}
+
+		return [
+			RuleErrorBuilder::message(sprintf('%s() is not allowed. Use more strict assertion.', $node->name->toString()))
+				->identifier('empty.notAllowed')
+				->build(),
+		];
+	}
+
+}

tests/Rules/PHPUnit/AssertEmptyIsDiscouragedRuleTest.php

@@ -0,0 +1,29 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\PHPUnit;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+
+/**
+ * @extends RuleTestCase<AssertEmptyIsDiscouragedRule>
+ */
+final class AssertEmptyIsDiscouragedRuleTest extends RuleTestCase
+{
+
+	public function testRule(): void
+	{
+		$this->analyse([__DIR__ . '/data/assert-empty-is-discouraged.php'], [
+			['assertEmpty() is not allowed. Use more strict assertion.', 13],
+			['assertNotEmpty() is not allowed. Use more strict assertion.', 14],
+			['assertEmpty() is not allowed. Use more strict assertion.', 15],
+			['assertNotEmpty() is not allowed. Use more strict assertion.', 16],
+		]);
+	}
+
+	protected function getRule(): Rule
+	{
+		return new AssertEmptyIsDiscouragedRule();
+	}
+
+}

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

@@ -0,0 +1,21 @@
+<?php declare(strict_types = 1);
+
+namespace AssertEmptyIsDiscouragedTest;
+
+use PHPUnit\Framework\Assert;
+use PHPUnit\Framework\TestCase;
+
+final class AssertEmptyTest extends TestCase
+{
+
+	public function test(string $bar): void
+	{
+		$this->assertEmpty([]);
+		$this->assertNotEmpty([1]);
+		Assert::assertEmpty([]);
+		static::assertNotEmpty([1]);
+		static::assertSame('foo', $bar);
+		static::assertEquals(1, '1');
+	}
+
+}