Skip to content

Commit

Permalink
Add test fixture for sprintf number + make use of getArgs() to get al…
Browse files Browse the repository at this point in the history
…ways an Arg type (#3822)
  • Loading branch information
TomasVotruba committed May 13, 2023
1 parent 6dea82d commit 645071a
Show file tree
Hide file tree
Showing 29 changed files with 209 additions and 347 deletions.
2 changes: 1 addition & 1 deletion build/target-repository/composer.json
Expand Up @@ -8,7 +8,7 @@
],
"require": {
"php": "^7.2|^8.0",
"phpstan/phpstan": "^1.10.14"
"phpstan/phpstan": "^1.10.15"
},
"autoload": {
"files": [
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Expand Up @@ -20,7 +20,7 @@
"nikic/php-parser": "^4.15.4",
"ondram/ci-detector": "^4.1",
"phpstan/phpdoc-parser": "^1.20.3",
"phpstan/phpstan": "^1.10.14",
"phpstan/phpstan": "^1.10.15",
"react/event-loop": "^1.3",
"react/socket": "^1.12",
"rector/extension-installer": "^0.11.2",
Expand Down
Expand Up @@ -12,7 +12,6 @@
use PHPStan\Type\ArrayType;
use PHPStan\Type\MixedType;
use PHPStan\Type\Type;
use Rector\Core\NodeAnalyzer\ArgsAnalyzer;
use Rector\Core\PhpParser\Comparing\NodeComparator;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\NodeNameResolver\NodeNameResolver;
Expand All @@ -26,7 +25,6 @@ public function __construct(
private readonly NodeNameResolver $nodeNameResolver,
private readonly ParentScopeFinder $parentScopeFinder,
private readonly NodeComparator $nodeComparator,
private readonly ArgsAnalyzer $argsAnalyzer
) {
}

Expand Down Expand Up @@ -57,17 +55,15 @@ public function correct(Expr $expr, Type $originalType): Type
continue;
}

if (! $this->nodeNameResolver->isNames($funcCallNode, ['preg_match', 'preg_match_all'])) {
$thirdArg = $funcCallNode->getArgs()[2] ?? null;
if (! $thirdArg instanceof Arg) {
continue;
}

if (! $this->argsAnalyzer->isArgInstanceInArgsPosition($funcCallNode->args, 2)) {
if (! $this->nodeNameResolver->isNames($funcCallNode, ['preg_match', 'preg_match_all'])) {
continue;
}

/** @var Arg $thirdArg */
$thirdArg = $funcCallNode->args[2];

// are the same variables
if (! $this->nodeComparator->areNodesEqual($thirdArg->value, $expr)) {
continue;
Expand Down
@@ -0,0 +1,11 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\FuncCall\RemoveSoleValueSprintfRector\Fixture;

final class SkipNonString
{
public function run()
{
$value = sprintf('%s', 1000);
}
}
@@ -0,0 +1,19 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\CodeQuality\Rector\FuncCall\SimplifyStrposLowerRector\Fixture;

final class Strpos
{
public function run()
{
$string = 'hey';
strpos(strtolower($string), 'find-me');

$funcName = 'strpos';
$funcName(strtolower($string), 'find-me');
}
}

?>
Expand Up @@ -5,7 +5,6 @@
namespace Rector\CodeQuality\Rector\FuncCall;

use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\ArrayDimFetch;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\FuncCall;
Expand Down Expand Up @@ -73,14 +72,11 @@ public function refactor(Node $node): ?array
}

$args = $funcCall->getArgs();

if ($args === []) {
return null;
}

/** @var Arg $firstArg */
$firstArg = array_shift($args);

if ($args === []) {
return null;
}
Expand Down
23 changes: 3 additions & 20 deletions rules/CodeQuality/Rector/FuncCall/RemoveSoleValueSprintfRector.php
Expand Up @@ -5,10 +5,8 @@
namespace Rector\CodeQuality\Rector\FuncCall;

use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Scalar\String_;
use Rector\Core\NodeAnalyzer\ArgsAnalyzer;
use Rector\Core\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
Expand All @@ -18,11 +16,6 @@
*/
final class RemoveSoleValueSprintfRector extends AbstractRector
{
public function __construct(
private readonly ArgsAnalyzer $argsAnalyzer
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Remove sprintf() wrapper if not needed', [
Expand All @@ -32,8 +25,6 @@ class SomeClass
{
public function run()
{
$value = sprintf('%s', 'hi');
$welcome = 'hello';
$value = sprintf('%s', $welcome);
}
Expand All @@ -45,8 +36,6 @@ class SomeClass
{
public function run()
{
$value = 'hi';
$welcome = 'hello';
$value = $welcome;
}
Expand All @@ -73,16 +62,11 @@ public function refactor(Node $node): ?Node
return null;
}

if (count($node->args) !== 2) {
return null;
}

if (! $this->argsAnalyzer->isArgsInstanceInArgsPositions($node->args, [0, 1])) {
if (count($node->getArgs()) !== 2) {
return null;
}

/** @var Arg $firstArg */
$firstArg = $node->args[0];
$firstArg = $node->getArgs()[0];
$maskArgument = $firstArg->value;
if (! $maskArgument instanceof String_) {
return null;
Expand All @@ -92,8 +76,7 @@ public function refactor(Node $node): ?Node
return null;
}

/** @var Arg $secondArg */
$secondArg = $node->args[1];
$secondArg = $node->getArgs()[1];
$valueArgument = $secondArg->value;

$valueType = $this->getType($valueArgument);
Expand Down
37 changes: 12 additions & 25 deletions rules/CodeQuality/Rector/FuncCall/SetTypeToCastRector.php
Expand Up @@ -17,7 +17,6 @@
use PhpParser\Node\Expr\Cast\String_;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Stmt\Expression;
use Rector\Core\NodeAnalyzer\ArgsAnalyzer;
use Rector\Core\Rector\AbstractRector;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
Expand Down Expand Up @@ -45,11 +44,6 @@ final class SetTypeToCastRector extends AbstractRector
'string' => String_::class,
];

public function __construct(
private readonly ArgsAnalyzer $argsAnalyzer
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Changes settype() to (type) where possible', [
Expand Down Expand Up @@ -98,47 +92,40 @@ public function refactor(Node $node): ?Node
return null;
}

if (! $this->argsAnalyzer->isArgInstanceInArgsPosition($node->args, 1)) {
if ($node->isFirstClassCallable()) {
return null;
}

/** @var Arg $secondArg */
$secondArg = $node->args[1];
$typeNode = $this->valueResolver->getValue($secondArg->value);
if (! is_string($typeNode)) {
$typeValue = $this->valueResolver->getValue($node->getArgs()[1]->value);
if (! is_string($typeValue)) {
return null;
}

$typeNode = strtolower($typeNode);

if (! $this->argsAnalyzer->isArgInstanceInArgsPosition($node->args, 0)) {
return null;
}
$typeValue = strtolower($typeValue);

/** @var Arg $firstArg */
$firstArg = $node->args[0];
$varNode = $firstArg->value;
$variable = $node->getArgs()[0]
->value;
$parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);

// result of function or probably used
if ($parentNode instanceof Expr || $parentNode instanceof Arg) {
return null;
}

if (isset(self::TYPE_TO_CAST[$typeNode])) {
$castClass = self::TYPE_TO_CAST[$typeNode];
$castNode = new $castClass($varNode);
if (isset(self::TYPE_TO_CAST[$typeValue])) {
$castClass = self::TYPE_TO_CAST[$typeValue];
$castNode = new $castClass($variable);

if ($parentNode instanceof Expression) {
// bare expression? → assign
return new Assign($varNode, $castNode);
return new Assign($variable, $castNode);
}

return $castNode;
}

if ($typeNode === 'null') {
return new Assign($varNode, $this->nodeFactory->createNull());
if ($typeValue === 'null') {
return new Assign($variable, $this->nodeFactory->createNull());
}

return $node;
Expand Down
Expand Up @@ -5,9 +5,7 @@
namespace Rector\CodeQuality\Rector\FuncCall;

use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\FuncCall;
use Rector\Core\NodeAnalyzer\ArgsAnalyzer;
use Rector\Core\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
Expand All @@ -18,11 +16,6 @@
*/
final class UnwrapSprintfOneArgumentRector extends AbstractRector
{
public function __construct(
private readonly ArgsAnalyzer $argsAnalyzer
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('unwrap sprintf() with one argument', [
Expand Down Expand Up @@ -51,20 +44,19 @@ public function getNodeTypes(): array
*/
public function refactor(Node $node): ?Node
{
if (! $this->isName($node, 'sprintf')) {
if ($node->isFirstClassCallable()) {
return null;
}

if (count($node->args) > 1) {
if (! $this->isName($node, 'sprintf')) {
return null;
}

if (! $this->argsAnalyzer->isArgInstanceInArgsPosition($node->args, 0)) {
if (count($node->getArgs()) > 1) {
return null;
}

/** @var Arg $firstArg */
$firstArg = $node->args[0];
$firstArg = $node->getArgs()[0];
if ($firstArg->unpack) {
return null;
}
Expand Down
Expand Up @@ -5,12 +5,10 @@
namespace Rector\CodeQuality\Rector\Identical;

use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\BinaryOp\Identical;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Scalar\String_;
use Rector\Core\NodeAnalyzer\ArgsAnalyzer;
use Rector\Core\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
Expand All @@ -20,11 +18,6 @@
*/
final class StrlenZeroToIdenticalEmptyStringRector extends AbstractRector
{
public function __construct(
private readonly ArgsAnalyzer $argsAnalyzer
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
Expand Down Expand Up @@ -85,24 +78,23 @@ private function processIdentical(Expr $expr, FuncCall $funcCall): ?Identical
return null;
}

if (! $this->valueResolver->isValue($expr, 0)) {
if ($funcCall->isFirstClassCallable()) {
return null;
}

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

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

// Needs string cast if variable type is not string
// see https://github.com/rectorphp/rector/issues/6700
$isStringType = $this->nodeTypeResolver->getNativeType($variable)
->isString()
->yes();

if (! $isStringType) {
return new Identical(new Expr\Cast\String_($variable), new String_(''));
}
Expand Down
Expand Up @@ -262,17 +262,17 @@ private function replaceNextUsageVariable(
return;
}

if (! isset($stmts[$key+1])) {
if (! isset($stmts[$key + 1])) {
return;
}

$currentNode = $stmts[$key+1];
$currentNode = $stmts[$key + 1];

if (! isset($stmts[$key+2])) {
if (! isset($stmts[$key + 2])) {
return;
}

$nextNode = $stmts[$key+2];
$nextNode = $stmts[$key + 2];
$key += 2;

$this->replaceNextUsageVariable($currentNode, $oldVariableName, $newVariableName, $key, $stmts, $nextNode);
Expand Down

0 comments on commit 645071a

Please sign in to comment.