Skip to content

feat: add CallLike::getArg() method#1089

Merged
nikic merged 1 commit intonikic:masterfrom
calebdw:calebdw/push-tqntykytzkvo
Jul 19, 2025
Merged

feat: add CallLike::getArg() method#1089
nikic merged 1 commit intonikic:masterfrom
calebdw:calebdw/push-tqntykytzkvo

Conversation

@calebdw
Copy link
Contributor

@calebdw calebdw commented Jun 13, 2025

Hello!

This method returns the named argument that matches the given $name, or the positional (unnamed) argument that exists at the given $position, otherwise, returns null for first-class callables or if no match is found.

Motivation

I found myself duplicating this logic over and over again, so thought it would be a good idea to just PR it.

For a given CallLike, I often need to target a particular parameter. Take for example this function signature:

function test(int $foo = 24, int $bar = 42);

and let's say that I need to do something with $bar (which has name bar and is at position 1) if it's provided in the call. Now I can simply use:

$arg = $node->getArg('bar', 1);

which would return the following:

test(); // null
test(...); // null
test(24, 42); // Arg(42)
test(24); // null
test(foo: 24); // null
test(bar: 42); // Arg(42, 'bar')
test(foo: 24, bar: 42); // Arg(42, 'bar')
test(bar: 42, foo: 24); // Arg(42, 'bar')

Thanks!

@nikic
Copy link
Owner

nikic commented Jun 13, 2025

I don't think this logic is correct if you have something like test(...$x, 42).

@calebdw calebdw force-pushed the calebdw/push-tqntykytzkvo branch from b12eeb9 to 1ee4b02 Compare June 13, 2025 20:00
@calebdw
Copy link
Contributor Author

calebdw commented Jun 13, 2025

@nikic, that is illegal: https://3v4l.org/Prr6W

@calebdw calebdw force-pushed the calebdw/push-tqntykytzkvo branch from 1ee4b02 to d4f040e Compare June 13, 2025 20:13
@nikic
Copy link
Owner

nikic commented Jun 13, 2025

@calebdw True, but I still think you have to consider unpacks in general. Let's turn it around and say test(42, ...$x). You probably wouldn't want $x to count as argument 1?

@calebdw calebdw force-pushed the calebdw/push-tqntykytzkvo branch from d4f040e to adc049c Compare June 13, 2025 21:45
@calebdw
Copy link
Contributor Author

calebdw commented Jun 13, 2025

Ah that makes sense, I've updated the logic to skip over unpacks.

For the $position parameter, do you think this should be 1-based or 0-based?

$arg = $node->getArg('bar', 1); // 0 based
$arg = $node->getArg('bar', 2); // 1 based

Also, it looks like the last ci failure wasn't caused by my changes?

This method returns the named argument that matches the given `$name`,
or the positional (unnamed) argument that exists at the given `$position`,
otherwise, returns `null` for first-class callables or if no match is found.
@calebdw calebdw force-pushed the calebdw/push-tqntykytzkvo branch from adc049c to a1dd70e Compare June 14, 2025 20:19
@calebdw
Copy link
Contributor Author

calebdw commented Jun 21, 2025

@nikic, just wanted to touch base---do you think this will suffice?

@nikic nikic merged commit 66d5018 into nikic:master Jul 19, 2025
10 checks passed
@calebdw calebdw deleted the calebdw/push-tqntykytzkvo branch July 19, 2025 19:43
@calebdw
Copy link
Contributor Author

calebdw commented Jul 19, 2025

Thanks!

Comment on lines +43 to +50
public function getArg(string $name, int $position): ?Arg {
if ($this->isFirstClassCallable()) {
return null;
}
foreach ($this->getRawArgs() as $i => $arg) {
if ($arg->unpack) {
continue;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@calebdw this seems cause error when using array_map flipped arg:

$result = array_map(array: $items, callback: fn ($item) => $item * 2);

Tested on rector, got error:

assert($itemStartPos >= 0 && $itemEndPos >= 0 && $itemStartPos >= $pos)

/Users/samsonasik/www/rector-src/vendor/nikic/php-parser/lib/PhpParser/PrettyPrinterAbstract.php:842

seems because array is unpack, array

The signature is:

function array_map(?callable $callback, array $array, array ...$arrays): array { }

/cc @TomasVotruba

Copy link
Contributor

Choose a reason for hiding this comment

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

PHPStan has a ArgumentsNormalizer for args reordering

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@staabm I will check if this can be handled on nikic/php-parser itself without re-order argument, as on specific rule I included above, it nothing todo with ordering argument task.

Copy link
Contributor

@samsonasik samsonasik Oct 2, 2025

Choose a reason for hiding this comment

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

The issue seems probably on Printer, as it shown on the stack trace above when run phpunit, while on rector demo, it printed without error, just show repetitive that cause invalid result

https://getrector.com/demo/104834e1-c8bd-4266-99f5-361581e44448

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked even without getArg() call, the error still exists on Printer, so it nothing todo with the getArg() method.

So the bug seems somewhere in the Printer.

Copy link
Contributor

@samsonasik samsonasik Oct 2, 2025

Choose a reason for hiding this comment

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

If it know about the information on named argument and its unpacked on print, it seems the issue on Internal PHPStan printer...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants