Skip to content

Commit 8d16b84

Browse files
staabmstaabm
authored
AssertEmptyIsDiscouragedRule is auto-fixable (#342)
1 parent b73700a commit 8d16b84

6 files changed

Lines changed: 449 additions & 3 deletions

‎src/Rules/PHPUnit/AssertEmptyIsDiscouragedRule.php‎

Lines changed: 82 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,21 @@
22

33
namespace PHPStan\Rules\PHPUnit;
44

5+
use Countable;
56
use PhpParser\Node;
67
use PhpParser\Node\Expr\CallLike;
78
use PhpParser\Node\Expr\MethodCall;
89
use PhpParser\Node\Expr\StaticCall;
910
use PhpParser\Node\Identifier;
11+
use PhpParser\Node\Scalar\Int_;
12+
use PhpParser\Node\Scalar\String_;
1013
use PHPStan\Analyser\Scope;
1114
use PHPStan\Rules\Rule;
1215
use PHPStan\Rules\RuleErrorBuilder;
16+
use PHPStan\Type\Type;
17+
use PHPStan\Type\TypeCombinator;
18+
use PHPStan\Type\UnionType;
19+
use function array_merge;
1320
use function count;
1421
use function in_array;
1522
use function sprintf;
@@ -43,11 +50,83 @@ public function processNode(Node $node, Scope $scope): array
4350
return [];
4451
}
4552

53+
$errorBuilder = RuleErrorBuilder::message(sprintf('%s() is not allowed. Use more strict assertion.', $node->name->toString()))
54+
->identifier('phpunit.assertEmpty');
55+
56+
if (AssertRuleHelper::hasNamedOrUnpackedArguments($node)) {
57+
return [$errorBuilder->build()];
58+
}
59+
60+
$replacement = $this->getReplacement($scope->getNativeType($node->getArgs()[0]->value), $node->name->toLowerString() === 'assertnotempty');
61+
if ($replacement !== null) {
62+
[$correctName, $expectedValue] = $replacement;
63+
$errorBuilder->fixNode($node, static function (CallLike $node) use ($correctName, $expectedValue) {
64+
$node->name = new Identifier($correctName);
65+
if ($expectedValue !== null) {
66+
$node->args = array_merge([new Node\Arg($expectedValue)], $node->args);
67+
}
68+
69+
return $node;
70+
});
71+
}
72+
4673
return [
47-
RuleErrorBuilder::message(sprintf('%s() is not allowed. Use more strict assertion.', $node->name->toString()))
48-
->identifier('phpunit.assertEmpty')
49-
->build(),
74+
$errorBuilder->build(),
5075
];
5176
}
5277

78+
/**
79+
* @return array{string, Node\Expr|null}|null
80+
*/
81+
private function getReplacement(Type $type, bool $negated): ?array
82+
{
83+
if ($type instanceof UnionType) {
84+
if (TypeCombinator::containsNull($type)) {
85+
$typeWithoutNull = TypeCombinator::removeNull($type);
86+
87+
$classReflections = $typeWithoutNull->getObjectClassReflections();
88+
if (count($classReflections) === 0) {
89+
return null;
90+
}
91+
foreach ($classReflections as $classReflection) {
92+
if (
93+
$classReflection->isBuiltin()
94+
|| !$classReflection->isFinalByKeyword()
95+
|| $classReflection->implementsInterface(Countable::class)
96+
) {
97+
return null;
98+
}
99+
100+
$parentClass = $classReflection->getParentClass();
101+
while ($parentClass !== null) {
102+
// Builtin parents can define different empty() semantics.
103+
if ($parentClass->isBuiltin()) {
104+
return null;
105+
}
106+
$parentClass = $parentClass->getParentClass();
107+
}
108+
}
109+
110+
return [$negated ? 'assertNotNull' : 'assertNull', null];
111+
}
112+
113+
return null;
114+
}
115+
116+
if ($type->isBoolean()->yes()) {
117+
return [$negated ? 'assertTrue' : 'assertFalse', null];
118+
}
119+
if ($type->isArray()->yes()) {
120+
return [$negated ? 'assertNotCount' : 'assertCount', new Int_(0)];
121+
}
122+
if ($type->isInteger()->yes()) {
123+
return [$negated ? 'assertNotSame' : 'assertSame', new Int_(0)];
124+
}
125+
if ($type->isNonFalsyString()->yes()) {
126+
return [$negated ? 'assertNotSame' : 'assertSame', new String_('')];
127+
}
128+
129+
return null;
130+
}
131+
53132
}

‎tests/Rules/PHPUnit/AssertEmptyIsDiscouragedRuleTest.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use PHPStan\Rules\Rule;
66
use PHPStan\Testing\RuleTestCase;
7+
use const PHP_VERSION_ID;
78

89
/**
910
* @extends RuleTestCase<AssertEmptyIsDiscouragedRule>
@@ -21,6 +22,20 @@ public function testRule(): void
2122
]);
2223
}
2324

25+
public function testFix(): void
26+
{
27+
$this->fix(__DIR__ . '/data/assert-empty-is-discouraged-fixable.php', __DIR__ . '/data/assert-empty-is-discouraged-fixable.php.fixed');
28+
}
29+
30+
public function testNativeUnionTypeIsNotFixed(): void
31+
{
32+
if (PHP_VERSION_ID < 80000) {
33+
self::markTestSkipped('Native union types require PHP 8.0.');
34+
}
35+
36+
$this->fix(__DIR__ . '/data/assert-empty-is-discouraged-native-union.php', __DIR__ . '/data/assert-empty-is-discouraged-native-union.php.fixed');
37+
}
38+
2439
protected function getRule(): Rule
2540
{
2641
return new AssertEmptyIsDiscouragedRule();

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

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace AssertEmptyIsDiscouragedFixTest;
4+
5+
use PHPUnit\Framework\Assert;
6+
use PHPUnit\Framework\TestCase;
7+
8+
final class UserDefinedObject
9+
{
10+
11+
}
12+
13+
/** @final */
14+
class PhpDocFinalObject
15+
{
16+
17+
}
18+
19+
class UserDefinedCountable implements \Countable
20+
{
21+
22+
public function count(): int
23+
{
24+
return 0;
25+
}
26+
27+
}
28+
29+
class OpenParent
30+
{
31+
32+
}
33+
34+
interface Thing
35+
{
36+
37+
}
38+
39+
class ThingImplementation implements Thing, \Countable
40+
{
41+
42+
public function count(): int
43+
{
44+
return 0;
45+
}
46+
47+
}
48+
49+
final class FinalEmptyIterator extends \EmptyIterator
50+
{
51+
52+
}
53+
54+
final class FinalCountable implements \Countable
55+
{
56+
57+
public function count(): int
58+
{
59+
return 0;
60+
}
61+
62+
}
63+
64+
final class AssertEmptyTest extends TestCase
65+
{
66+
67+
/**
68+
* @param non-falsy-string $nonFalsyString
69+
* @param non-falsy-string $otherNonFalsyString
70+
* @param int $phpDocInteger
71+
* @param string|int $union
72+
*/
73+
public function test(
74+
array $arguments,
75+
bool $boolean,
76+
array $array,
77+
int $integer,
78+
float $float,
79+
string $string,
80+
$union,
81+
string $nonFalsyString,
82+
string $otherNonFalsyString,
83+
?\stdClass $nullableObject,
84+
?object $otherNullableObject,
85+
?\SimpleXMLElement $nullableSimpleXml,
86+
?UserDefinedObject $nullableUserDefinedObject,
87+
?UserDefinedObject $otherNullableUserDefinedObject,
88+
?PhpDocFinalObject $nullablePhpDocFinalObject,
89+
?UserDefinedCountable $nullableCountable,
90+
?OpenParent $nullableOpenParent,
91+
?Thing $nullableThing,
92+
?FinalEmptyIterator $nullableEmptyIterator,
93+
?FinalCountable $nullableFinalCountable,
94+
$phpDocInteger,
95+
$mixed
96+
): void
97+
{
98+
$this->assertEmpty(...$arguments);
99+
$this->assertEmpty($boolean);
100+
$this->assertNotEmpty($boolean);
101+
$this->assertEmpty($array, 'message');
102+
$this->assertNotEmpty($array);
103+
$this->assertEmpty($integer);
104+
$this->assertNotEmpty($integer);
105+
Assert::assertEmpty($float);
106+
Assert::assertNotEmpty($float);
107+
$this->assertEmpty($nonFalsyString);
108+
$this->assertNotEmpty($otherNonFalsyString);
109+
$this->assertEmpty($string);
110+
$this->assertNotEmpty($string);
111+
$this->assertEmpty($union);
112+
$this->assertNotEmpty($union);
113+
$this->assertEmpty($nullableObject);
114+
$this->assertNotEmpty($otherNullableObject);
115+
$this->assertEmpty($nullableSimpleXml);
116+
$this->assertEmpty($nullableUserDefinedObject);
117+
$this->assertNotEmpty($otherNullableUserDefinedObject);
118+
$this->assertEmpty($nullablePhpDocFinalObject);
119+
$this->assertNotEmpty($nullablePhpDocFinalObject);
120+
$this->assertEmpty($nullableCountable);
121+
$this->assertNotEmpty($nullableCountable);
122+
$this->assertEmpty($nullableOpenParent);
123+
$this->assertNotEmpty($nullableOpenParent);
124+
$this->assertEmpty($nullableThing);
125+
$this->assertNotEmpty($nullableThing);
126+
$this->assertEmpty($nullableEmptyIterator);
127+
$this->assertNotEmpty($nullableEmptyIterator);
128+
$this->assertEmpty($nullableFinalCountable);
129+
$this->assertNotEmpty($nullableFinalCountable);
130+
$this->assertNotEmpty($phpDocInteger);
131+
$this->assertEmpty($mixed);
132+
}
133+
134+
public function testNativeNonFalsyString(string $value): void
135+
{
136+
if ($value === '' || $value === '0') {
137+
return;
138+
}
139+
140+
$this->assertNotEmpty($value);
141+
}
142+
143+
}

0 commit comments

Comments
 (0)