Skip to content

Commit

Permalink
handle cases where strlen arg is a Stringable object (#981)
Browse files Browse the repository at this point in the history
* handle cases where strlen arg is a Stringable object

* handle in processLeftIdentical as well

* fix to use getNativeType

* add test fixtures

* stick on strict comparison by using string cast

* fix return type

* refactor processIdentical

* refactor processEqual

* fix

* separate expr to variable
  • Loading branch information
rajyan committed Oct 11, 2021
1 parent 65c3a44 commit 5f915d8
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

namespace Rector\Tests\CodeQuality\Rector\Identical\StrlenZeroToIdenticalEmptyStringRector\Fixture;

class Fixture
class DefinitelyString
{
public function run($value)
public function run(string $value)
{
$empty = strlen($value) === 0;

Expand All @@ -18,9 +18,9 @@ class Fixture

namespace Rector\Tests\CodeQuality\Rector\Identical\StrlenZeroToIdenticalEmptyStringRector\Fixture;

class Fixture
class DefinitelyString
{
public function run($value)
public function run(string $value)
{
$empty = $value === '';

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\Identical\StrlenZeroToIdenticalEmptyStringRector\Fixture;

class MightNotBeString
{
public function run($value)
{
$empty = strlen($value) === 0;

$empty = 0 === strlen($value);
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\Identical\StrlenZeroToIdenticalEmptyStringRector\Fixture;

class MightNotBeString
{
public function run($value)
{
$empty = (string) $value === '';

$empty = (string) $value === '';
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\Identical\StrlenZeroToIdenticalEmptyStringRector\Fixture;

class NonStringValue
{
public function run()
{
$value = null;

$empty = strlen($value) === 0;

$empty = 0 === strlen($value);
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\Identical\StrlenZeroToIdenticalEmptyStringRector\Fixture;

class NonStringValue
{
public function run()
{
$value = null;

$empty = (string) $value === '';

$empty = (string) $value === '';
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\Identical\StrlenZeroToIdenticalEmptyStringRector\Fixture;

class Stringable {
private string $string = '';

public function __toString() : string {
return $this->string;
}
}

class StringableObject
{
public function run()
{
$value = new Stringable();

$empty = strlen($value) === 0;

$empty = 0 === strlen($value);
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\Identical\StrlenZeroToIdenticalEmptyStringRector\Fixture;

class Stringable {
private string $string = '';

public function __toString() : string {
return $this->string;
}
}

class StringableObject
{
public function run()
{
$value = new Stringable();

$empty = (string) $value === '';

$empty = (string) $value === '';
}
}

?>
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Scalar\String_;
use Rector\Core\NodeAnalyzer\ArgsAnalyzer;
use PhpParser\Node\Expr\BinaryOp\Equal;
use PHPStan\Type\StringType;
use Rector\Core\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
Expand All @@ -34,7 +36,7 @@ public function getRuleDefinition(): RuleDefinition
<<<'CODE_SAMPLE'
class SomeClass
{
public function run($value)
public function run(string $value)
{
$empty = strlen($value) === 0;
}
Expand All @@ -44,7 +46,7 @@ public function run($value)
<<<'CODE_SAMPLE'
class SomeClass
{
public function run($value)
public function run(string $value)
{
$empty = $value === '';
}
Expand All @@ -69,23 +71,23 @@ public function getNodeTypes(): array
public function refactor(Node $node): ?Node
{
if ($node->left instanceof FuncCall) {
return $this->processLeftIdentical($node, $node->left);
return $this->processIdentical($node->right, $node->left);
}

if ($node->right instanceof FuncCall) {
return $this->processRightIdentical($node, $node->right);
return $this->processIdentical($node->left, $node->right);
}

return null;
}

private function processLeftIdentical(Identical $identical, FuncCall $funcCall): ?Identical
private function processIdentical(Expr $value, FuncCall $funcCall): ?Identical
{
if (! $this->isName($funcCall, 'strlen')) {
return null;
}

if (! $this->valueResolver->isValue($identical->right, 0)) {
if (! $this->valueResolver->isValue($value, 0)) {
return null;
}

Expand All @@ -98,28 +100,13 @@ private function processLeftIdentical(Identical $identical, FuncCall $funcCall):
/** @var Expr $variable */
$variable = $firstArg->value;

return new Identical($variable, new String_(''));
}

private function processRightIdentical(Identical $identical, FuncCall $funcCall): ?Identical
{
if (! $this->isName($funcCall, 'strlen')) {
return null;
}

if (! $this->valueResolver->isValue($identical->left, 0)) {
return null;
// Needs string cast if variable type is not string
// see https://github.com/rectorphp/rector/issues/6700
$isStringType = $this->nodeTypeResolver->getNativeType($variable) instanceof StringType;
if (!$isStringType) {
return new Identical(new Expr\Cast\String_($variable), new String_(''));
}

if (! $this->argsAnalyzer->isArgInstanceInArgsPosition($funcCall->args, 0)) {
return null;
}

/** @var Arg $firstArg */
$firstArg = $funcCall->args[0];
/** @var Expr $variable */
$variable = $firstArg->value;

return new Identical($variable, new String_(''));
}
}

0 comments on commit 5f915d8

Please sign in to comment.