Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

namespace Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Fixture;

use Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Source\SomeMultiArg;

class NamedArgAddNew
{
public function run()
{
$obj = new SomeMultiArg();
// Named arguments exist, but not the one we're adding (c)
$obj->run(b: 5);
}
}

?>
-----
<?php

namespace Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Fixture;

use Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Source\SomeMultiArg;

class NamedArgAddNew
{
public function run()
{
$obj = new SomeMultiArg();
// Named arguments exist, but not the one we're adding (c)
$obj->run(b: 5, c: 4);
}
}

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

namespace Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Fixture;

use Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Source\SomeMultiArg;

class NamedArgMixedPositionalNamed
{
public function run()
{
$obj = new SomeMultiArg();
// Mix of positional and named arguments
$obj->run(0, c: 6);
$obj->run(0, b: 6);
}
}

?>
-----
<?php

namespace Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Fixture;

use Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Source\SomeMultiArg;

class NamedArgMixedPositionalNamed
{
public function run()
{
$obj = new SomeMultiArg();
// Mix of positional and named arguments
$obj->run(0, c: 6);
$obj->run(0, b: 6, c: 4);
}
}

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

namespace Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Fixture;

use Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Source\SomeMultiArg;

class SkipNamedArgAlreadyPresent
{
public function run()
{
$obj = new SomeMultiArg();
// Parameter 'c' is already provided as named argument
$obj->run(b: 5, c: 999);
$obj->run(c: 999, b: 5);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

namespace Rector\Tests\Arguments\Rector\ClassMethod\ReplaceArgumentDefaultValueRector\Fixture;

use Rector\Tests\Arguments\Rector\ClassMethod\ReplaceArgumentDefaultValueRector\Fixture\ReplaceMethodArgumentWithConstant;

class SkipNamedArguments
{
public function run()
{
$obj = new ReplaceMethodArgumentWithConstant();
$obj->setSomeMethod(someValue: 'some value');
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

namespace Rector\Tests\Arguments\Rector\FuncCall\FunctionArgumentDefaultValueReplacerRector\Fixture;

class SkipNamedArguments
{
public function run()
{
version_compare(version1: '5.6', version2: PHP_VERSION, operator: 'lte');
version_compare('5.6', PHP_VERSION, operator: 'lte');
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

namespace Rector\Tests\Arguments\Rector\MethodCall\RemoveMethodCallParamRector\Fixture;

use Rector\Tests\Arguments\Rector\MethodCall\RemoveMethodCallParamRector\Source\MethodCaller;

final class SkipNamedArguments
{
public function run(MethodCaller $caller)
{
$caller->process(first: 1, second: 2);
$caller->process(1, second: 2);
}
}
8 changes: 7 additions & 1 deletion rules/Arguments/ArgumentDefaultValueReplacer.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,16 @@
use PhpParser\Node\Stmt\ClassMethod;
use Rector\Arguments\Contract\ReplaceArgumentDefaultValueInterface;
use Rector\Arguments\ValueObject\ReplaceArgumentDefaultValue;
use Rector\NodeAnalyzer\ArgsAnalyzer;
use Rector\PhpParser\Node\NodeFactory;
use Rector\PhpParser\Node\Value\ValueResolver;

final readonly class ArgumentDefaultValueReplacer
{
public function __construct(
private NodeFactory $nodeFactory,
private ValueResolver $valueResolver
private ValueResolver $valueResolver,
private ArgsAnalyzer $argsAnalyzer
) {
}

Expand Down Expand Up @@ -101,6 +103,10 @@ private function processArgs(
return null;
}

if ($this->argsAnalyzer->hasNamedArg($expr->getArgs())) {
return null;
}

$position = $replaceArgumentDefaultValue->getPosition();
$particularArg = $expr->getArgs()[$position] ?? null;
if (! $particularArg instanceof Arg) {
Expand Down
44 changes: 34 additions & 10 deletions rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\Node\Param;
use PhpParser\Node\Stmt\Class_;
Expand All @@ -24,6 +25,7 @@
use Rector\Contract\Rector\ConfigurableRectorInterface;
use Rector\Enum\ObjectReference;
use Rector\Exception\ShouldNotHappenException;
use Rector\NodeAnalyzer\ArgsAnalyzer;
use Rector\PhpParser\AstResolver;
use Rector\PHPStanStaticTypeMapper\Enum\TypeKind;
use Rector\Rector\AbstractRector;
Expand All @@ -48,7 +50,8 @@ public function __construct(
private readonly ArgumentAddingScope $argumentAddingScope,
private readonly ChangedArgumentsDetector $changedArgumentsDetector,
private readonly AstResolver $astResolver,
private readonly StaticTypeMapper $staticTypeMapper
private readonly StaticTypeMapper $staticTypeMapper,
private readonly ArgsAnalyzer $argsAnalyzer
) {
}

Expand Down Expand Up @@ -181,13 +184,18 @@ private function processMethodCall(

$defaultValue = $argumentAdder->getArgumentDefaultValue();
$arg = new Arg(BuilderHelpers::normalizeValue($defaultValue));
if (isset($methodCall->args[$position])) {
return;
}

$this->fillGapBetweenWithDefaultValue($methodCall, $position);
if ($this->argsAnalyzer->hasNamedArg($methodCall->getArgs())) {
$argumentName = $argumentAdder->getArgumentName();
if ($argumentName === null) {
return;
}
$arg->name = new Identifier($argumentName);
} else {
$this->fillGapBetweenWithDefaultValue($methodCall, $position);
}

$methodCall->args[$position] = $arg;
$methodCall->args[] = $arg;
$this->hasChanged = true;
}

Expand Down Expand Up @@ -249,8 +257,18 @@ private function shouldSkipParameter(
return $this->changedArgumentsDetector->isTypeChanged($param, $argumentAdder->getArgumentType());
}

if (isset($node->args[$position])) {
return true;
// If named arguments exist
Copy link
Member

@samsonasik samsonasik Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think to get correct position, you can combine it with argumentName from:

$position = $this->argsAnalyzer->resolveArgPosition($args, $argumentName, $position);

see

public function resolveArgPosition(array $args, string $name, int $defaultPosition): int
{
foreach ($args as $position => $arg) {
if (! $arg->name instanceof Identifier) {
continue;
}
if (! $this->nodeNameResolver->isName($arg->name, $name)) {
continue;
}
return $position;
}
return $defaultPosition;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, but not sure how that would be useful here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is to compare parameter name vs arg name, if equal, use its position whenever they match, otherwise, use default position originally provided.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That can be useful in the rule that tries to replace an argument, but I don't think we need this in this rule which just tries to add a new argument, if the argument already exists we need to skip the rule and if it doesn't, trying to add it positionally can be problematic, so better just add it as a new named argument

if ($this->argsAnalyzer->hasNamedArg($node->getArgs())) {
// Check if the parameter we're trying to add is already present as a named argument
foreach ($node->getArgs() as $arg) {
if ($arg->name instanceof Identifier && $this->isName($arg->name, $argumentName)) {
return true; // Skip - parameter already provided with name
}
}
} else {
if (isset($node->args[$position])) {
return true;
}
}

// Check if default value is the same
Expand Down Expand Up @@ -333,9 +351,15 @@ private function processStaticCall(
return;
}

$this->fillGapBetweenWithDefaultValue($staticCall, $position);
// Handle named arguments case - add as named argument at the end
$arg = new Arg(new Variable($argumentName));
if ($this->argsAnalyzer->hasNamedArg($staticCall->getArgs())) {
$arg->name = new Identifier($argumentName);
} else {
$this->fillGapBetweenWithDefaultValue($staticCall, $position);
}

$staticCall->args[$position] = new Arg(new Variable($argumentName));
$staticCall->args[] = $arg;
$this->hasChanged = true;
}

Expand Down
10 changes: 10 additions & 0 deletions rules/Arguments/Rector/MethodCall/RemoveMethodCallParamRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use PhpParser\Node\Expr\StaticCall;
use Rector\Arguments\ValueObject\RemoveMethodCallParam;
use Rector\Contract\Rector\ConfigurableRectorInterface;
use Rector\NodeAnalyzer\ArgsAnalyzer;
use Rector\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
Expand All @@ -25,6 +26,11 @@ final class RemoveMethodCallParamRector extends AbstractRector implements Config
*/
private array $removeMethodCallParams = [];

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

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Remove parameter of method call', [
Expand Down Expand Up @@ -73,6 +79,10 @@ public function refactor(Node $node): ?Node
return null;
}

if ($this->argsAnalyzer->hasNamedArg($node->getArgs())) {
return null;
}

foreach ($this->removeMethodCallParams as $removeMethodCallParam) {
if (! $this->isName($node->name, $removeMethodCallParam->getMethodName())) {
continue;
Expand Down