diff --git a/src/Rules/PHPUnit/AssertEmptyIsDiscouragedRule.php b/src/Rules/PHPUnit/AssertEmptyIsDiscouragedRule.php index 5459e26..b0ebf23 100644 --- a/src/Rules/PHPUnit/AssertEmptyIsDiscouragedRule.php +++ b/src/Rules/PHPUnit/AssertEmptyIsDiscouragedRule.php @@ -2,14 +2,21 @@ namespace PHPStan\Rules\PHPUnit; +use Countable; use PhpParser\Node; use PhpParser\Node\Expr\CallLike; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Identifier; +use PhpParser\Node\Scalar\Int_; +use PhpParser\Node\Scalar\String_; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\Type\Type; +use PHPStan\Type\TypeCombinator; +use PHPStan\Type\UnionType; +use function array_merge; use function count; use function in_array; use function sprintf; @@ -43,11 +50,83 @@ public function processNode(Node $node, Scope $scope): array return []; } + $errorBuilder = RuleErrorBuilder::message(sprintf('%s() is not allowed. Use more strict assertion.', $node->name->toString())) + ->identifier('phpunit.assertEmpty'); + + if (AssertRuleHelper::hasNamedOrUnpackedArguments($node)) { + return [$errorBuilder->build()]; + } + + $replacement = $this->getReplacement($scope->getNativeType($node->getArgs()[0]->value), $node->name->toLowerString() === 'assertnotempty'); + if ($replacement !== null) { + [$correctName, $expectedValue] = $replacement; + $errorBuilder->fixNode($node, static function (CallLike $node) use ($correctName, $expectedValue) { + $node->name = new Identifier($correctName); + if ($expectedValue !== null) { + $node->args = array_merge([new Node\Arg($expectedValue)], $node->args); + } + + return $node; + }); + } + return [ - RuleErrorBuilder::message(sprintf('%s() is not allowed. Use more strict assertion.', $node->name->toString())) - ->identifier('phpunit.assertEmpty') - ->build(), + $errorBuilder->build(), ]; } + /** + * @return array{string, Node\Expr|null}|null + */ + private function getReplacement(Type $type, bool $negated): ?array + { + if ($type instanceof UnionType) { + if (TypeCombinator::containsNull($type)) { + $typeWithoutNull = TypeCombinator::removeNull($type); + + $classReflections = $typeWithoutNull->getObjectClassReflections(); + if (count($classReflections) === 0) { + return null; + } + foreach ($classReflections as $classReflection) { + if ( + $classReflection->isBuiltin() + || !$classReflection->isFinalByKeyword() + || $classReflection->implementsInterface(Countable::class) + ) { + return null; + } + + $parentClass = $classReflection->getParentClass(); + while ($parentClass !== null) { + // Builtin parents can define different empty() semantics. + if ($parentClass->isBuiltin()) { + return null; + } + $parentClass = $parentClass->getParentClass(); + } + } + + return [$negated ? 'assertNotNull' : 'assertNull', null]; + } + + return null; + } + + if ($type->isBoolean()->yes()) { + return [$negated ? 'assertTrue' : 'assertFalse', null]; + } + if ($type->isArray()->yes()) { + return [$negated ? 'assertNotCount' : 'assertCount', new Int_(0)]; + } + if ($type->isInteger()->yes()) { + return [$negated ? 'assertNotSame' : 'assertSame', new Int_(0)]; + } + if ($type->isNonFalsyString()->yes()) { + return [$negated ? 'assertNotSame' : 'assertSame', new String_('')]; + } + + return null; + } + } diff --git a/tests/Rules/PHPUnit/AssertEmptyIsDiscouragedRuleTest.php b/tests/Rules/PHPUnit/AssertEmptyIsDiscouragedRuleTest.php index 156a6e8..304d7c4 100644 --- a/tests/Rules/PHPUnit/AssertEmptyIsDiscouragedRuleTest.php +++ b/tests/Rules/PHPUnit/AssertEmptyIsDiscouragedRuleTest.php @@ -4,6 +4,7 @@ use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; +use const PHP_VERSION_ID; /** * @extends RuleTestCase @@ -21,6 +22,20 @@ public function testRule(): void ]); } + public function testFix(): void + { + $this->fix(__DIR__ . '/data/assert-empty-is-discouraged-fixable.php', __DIR__ . '/data/assert-empty-is-discouraged-fixable.php.fixed'); + } + + public function testNativeUnionTypeIsNotFixed(): void + { + if (PHP_VERSION_ID < 80000) { + self::markTestSkipped('Native union types require PHP 8.0.'); + } + + $this->fix(__DIR__ . '/data/assert-empty-is-discouraged-native-union.php', __DIR__ . '/data/assert-empty-is-discouraged-native-union.php.fixed'); + } + protected function getRule(): Rule { return new AssertEmptyIsDiscouragedRule(); diff --git a/tests/Rules/PHPUnit/data/assert-empty-is-discouraged-fixable.php b/tests/Rules/PHPUnit/data/assert-empty-is-discouraged-fixable.php new file mode 100644 index 0000000..96cd551 --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-empty-is-discouraged-fixable.php @@ -0,0 +1,143 @@ +assertEmpty(...$arguments); + $this->assertEmpty($boolean); + $this->assertNotEmpty($boolean); + $this->assertEmpty($array, 'message'); + $this->assertNotEmpty($array); + $this->assertEmpty($integer); + $this->assertNotEmpty($integer); + Assert::assertEmpty($float); + Assert::assertNotEmpty($float); + $this->assertEmpty($nonFalsyString); + $this->assertNotEmpty($otherNonFalsyString); + $this->assertEmpty($string); + $this->assertNotEmpty($string); + $this->assertEmpty($union); + $this->assertNotEmpty($union); + $this->assertEmpty($nullableObject); + $this->assertNotEmpty($otherNullableObject); + $this->assertEmpty($nullableSimpleXml); + $this->assertEmpty($nullableUserDefinedObject); + $this->assertNotEmpty($otherNullableUserDefinedObject); + $this->assertEmpty($nullablePhpDocFinalObject); + $this->assertNotEmpty($nullablePhpDocFinalObject); + $this->assertEmpty($nullableCountable); + $this->assertNotEmpty($nullableCountable); + $this->assertEmpty($nullableOpenParent); + $this->assertNotEmpty($nullableOpenParent); + $this->assertEmpty($nullableThing); + $this->assertNotEmpty($nullableThing); + $this->assertEmpty($nullableEmptyIterator); + $this->assertNotEmpty($nullableEmptyIterator); + $this->assertEmpty($nullableFinalCountable); + $this->assertNotEmpty($nullableFinalCountable); + $this->assertNotEmpty($phpDocInteger); + $this->assertEmpty($mixed); + } + + public function testNativeNonFalsyString(string $value): void + { + if ($value === '' || $value === '0') { + return; + } + + $this->assertNotEmpty($value); + } + +} diff --git a/tests/Rules/PHPUnit/data/assert-empty-is-discouraged-fixable.php.fixed b/tests/Rules/PHPUnit/data/assert-empty-is-discouraged-fixable.php.fixed new file mode 100644 index 0000000..70954d8 --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-empty-is-discouraged-fixable.php.fixed @@ -0,0 +1,143 @@ +assertEmpty(...$arguments); + $this->assertFalse($boolean); + $this->assertTrue($boolean); + $this->assertCount(0, $array, 'message'); + $this->assertNotCount(0, $array); + $this->assertSame(0, $integer); + $this->assertNotSame(0, $integer); + Assert::assertEmpty($float); + Assert::assertNotEmpty($float); + $this->assertEmpty($nonFalsyString); + $this->assertNotEmpty($otherNonFalsyString); + $this->assertEmpty($string); + $this->assertNotEmpty($string); + $this->assertEmpty($union); + $this->assertNotEmpty($union); + $this->assertEmpty($nullableObject); + $this->assertNotEmpty($otherNullableObject); + $this->assertEmpty($nullableSimpleXml); + $this->assertNull($nullableUserDefinedObject); + $this->assertNotNull($otherNullableUserDefinedObject); + $this->assertEmpty($nullablePhpDocFinalObject); + $this->assertNotEmpty($nullablePhpDocFinalObject); + $this->assertEmpty($nullableCountable); + $this->assertNotEmpty($nullableCountable); + $this->assertEmpty($nullableOpenParent); + $this->assertNotEmpty($nullableOpenParent); + $this->assertEmpty($nullableThing); + $this->assertNotEmpty($nullableThing); + $this->assertEmpty($nullableEmptyIterator); + $this->assertNotEmpty($nullableEmptyIterator); + $this->assertEmpty($nullableFinalCountable); + $this->assertNotEmpty($nullableFinalCountable); + $this->assertNotEmpty($phpDocInteger); + $this->assertEmpty($mixed); + } + + public function testNativeNonFalsyString(string $value): void + { + if ($value === '' || $value === '0') { + return; + } + + $this->assertNotSame('', $value); + } + +} diff --git a/tests/Rules/PHPUnit/data/assert-empty-is-discouraged-native-union.php b/tests/Rules/PHPUnit/data/assert-empty-is-discouraged-native-union.php new file mode 100644 index 0000000..b6ec90d --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-empty-is-discouraged-native-union.php @@ -0,0 +1,33 @@ += 8.0 + +namespace AssertEmptyIsDiscouragedNativeUnionTest; + +use PHPUnit\Framework\TestCase; + +final class UserDefinedObject +{ + +} + +final class AssertEmptyTest extends TestCase +{ + + public function test( + string|int $value, + array|bool $otherValue, + \stdClass|int|null $nullableUnion, + UserDefinedObject|int|null $userDefinedNullableUnion, + bool $boolean, + int $integer + ): void + { + $this->assertEmpty($value); + $this->assertNotEmpty($otherValue); + $this->assertEmpty($nullableUnion); + $this->assertEmpty($userDefinedNullableUnion); + $this->assertNotEmpty($userDefinedNullableUnion); + $this->assertEmpty(actual: $boolean); + $this->assertEmpty(message: 'message', actual: $integer); + } + +} diff --git a/tests/Rules/PHPUnit/data/assert-empty-is-discouraged-native-union.php.fixed b/tests/Rules/PHPUnit/data/assert-empty-is-discouraged-native-union.php.fixed new file mode 100644 index 0000000..b6ec90d --- /dev/null +++ b/tests/Rules/PHPUnit/data/assert-empty-is-discouraged-native-union.php.fixed @@ -0,0 +1,33 @@ += 8.0 + +namespace AssertEmptyIsDiscouragedNativeUnionTest; + +use PHPUnit\Framework\TestCase; + +final class UserDefinedObject +{ + +} + +final class AssertEmptyTest extends TestCase +{ + + public function test( + string|int $value, + array|bool $otherValue, + \stdClass|int|null $nullableUnion, + UserDefinedObject|int|null $userDefinedNullableUnion, + bool $boolean, + int $integer + ): void + { + $this->assertEmpty($value); + $this->assertNotEmpty($otherValue); + $this->assertEmpty($nullableUnion); + $this->assertEmpty($userDefinedNullableUnion); + $this->assertNotEmpty($userDefinedNullableUnion); + $this->assertEmpty(actual: $boolean); + $this->assertEmpty(message: 'message', actual: $integer); + } + +}