Skip to content

Commit

Permalink
[CodeQuality] Handle possibly has '0' value on string on ExplicitBool…
Browse files Browse the repository at this point in the history
…CompareRector (#709)

* [CodeQuality] Skip explode() on ExplicitBoolCompareRector

* Fixed 🎉

* clean up

* update failing fixture

* Fixed 🎉

* better check

* [ci-review] Rector Rectify

* fix

* naming

* cs fix

* clean up

Co-authored-by: GitHub Action <action@github.com>
  • Loading branch information
samsonasik and actions-user committed Aug 18, 2021
1 parent 582d1d3 commit 25f7193
Show file tree
Hide file tree
Showing 6 changed files with 250 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\If_\ExplicitBoolCompareRector\Fixture;

class SomeExplode
{
public function run()
{
$values = explode('_', 'a_0_b');
foreach ($values as $value) {
if ($value) {
echo $value . PHP_EOL;
}
}
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\If_\ExplicitBoolCompareRector\Fixture;

class SomeExplode
{
public function run()
{
$values = explode('_', 'a_0_b');
foreach ($values as $value) {
if ($value !== '' && $value !== '0') {
echo $value . PHP_EOL;
}
}
}
}

?>
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ final class ExplicitString
{
public function run(string $item)
{
if ($item === '') {
if ($item === '' || $item === '0') {
return 'empty';
}

if ($item !== '') {
if ($item !== '' && $item !== '0') {
return 'not empty';
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\If_\ExplicitBoolCompareRector\Fixture;

final class StringKnownvalue
{
public function run()
{
$item = 'a value';

if (!$item) {
return 'empty';
}
}

public function run2()
{
$item = 'a value';

if ($item) {
return 'not empty';
}
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\If_\ExplicitBoolCompareRector\Fixture;

final class StringKnownvalue
{
public function run()
{
$item = 'a value';

if ($item === '') {
return 'empty';
}
}

public function run2()
{
$item = 'a value';

if ($item !== '') {
return 'not empty';
}
}
}

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

namespace Rector\Tests\CodeQuality\Rector\If_\ExplicitBoolCompareRector\Fixture;

final class StringKnownvalue2
{
public function run()
{
$item = '';

if (!$item) {
return 'empty';
}
}

public function run2()
{
$item = '';

if ($item) {
return 'not empty';
}
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\If_\ExplicitBoolCompareRector\Fixture;

final class StringKnownvalue2
{
public function run()
{
$item = '';

if ($item === '') {
return 'empty';
}
}

public function run2()
{
$item = '';

if ($item !== '') {
return 'not empty';
}
}
}

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

namespace Rector\Tests\CodeQuality\Rector\If_\ExplicitBoolCompareRector\Fixture;

final class StringSingleChar
{
public function run()
{
$item = 'a';

if (!$item) {
return 'empty';
}
}

public function run2()
{
$item = 'a';

if ($item) {
return 'not empty';
}
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\If_\ExplicitBoolCompareRector\Fixture;

final class StringSingleChar
{
public function run()
{
$item = 'a';

if ($item === '0') {
return 'empty';
}
}

public function run2()
{
$item = 'a';

if ($item !== '0') {
return 'not empty';
}
}
}

?>
57 changes: 52 additions & 5 deletions rules/CodeQuality/Rector/If_/ExplicitBoolCompareRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\BinaryOp;
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\BinaryOp\BooleanOr;
use PhpParser\Node\Expr\BinaryOp\Greater;
use PhpParser\Node\Expr\BinaryOp\Identical;
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
Expand Down Expand Up @@ -182,16 +184,61 @@ private function resolveArray(bool $isNegated, Expr $expr): ?BinaryOp
return new NotIdentical($expr, $array);
}

private function resolveString(bool $isNegated, Expr $expr): Identical | NotIdentical
private function resolveString(bool $isNegated, Expr $expr): Identical | NotIdentical | BooleanAnd | BooleanOr
{
$string = new String_('');

// compare === ''
if ($isNegated) {
return new Identical($expr, $string);
$identical = $this->resolveIdentical($expr, $isNegated, $string);

$value = $this->valueResolver->getValue($expr);

// unknown value. may be from parameter
if ($value === null) {
return $this->resolveZeroIdenticalstring($identical, $isNegated, $expr);
}

$length = strlen($value);

if ($length === 1) {
$string = new String_('0');
return $this->resolveIdentical($expr, $isNegated, $string);
}

return new NotIdentical($expr, $string);
return $identical;
}

private function resolveIdentical(Expr $expr, bool $isNegated, String_ $string): Identical | NotIdentical
{
/**
* // compare === ''
*
* @var Identical|NotIdentical $identical
*/
$identical = $isNegated
? new Identical($expr, $string)
: new NotIdentical($expr, $string);

return $identical;
}

private function resolveZeroIdenticalstring(
Identical | NotIdentical $identical,
bool $isNegated,
Expr $expr
): BooleanAnd | BooleanOr {
$string = new String_('0');
$zeroIdentical = $isNegated
? new Identical($expr, $string)
: new NotIdentical($expr, $string);

/**
* @var BooleanAnd|BooleanOr $result
*/
$result = $isNegated
? new BooleanOr($identical, $zeroIdentical)
: new BooleanAnd($identical, $zeroIdentical);

return $result;
}

private function resolveInteger(bool $isNegated, Expr $expr): Identical | NotIdentical
Expand Down

0 comments on commit 25f7193

Please sign in to comment.