Skip to content

[DeadCode] Skip private methods used as callables on RemoveUnusedPrivateMethodParameterRector - #8923

Merged
samsonasik merged 2 commits into
rectorphp:mainfrom
phinor:fix-remove-unused-private-method-param-callable
Nov 5, 2026
Merged

samsonasik merged 2 commits into
rectorphp:mainfrom
phinor:fix-remove-unused-private-method-param-callable

Conversation

@phinor

@phinor phinor commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Fixes rectorphp/rector#9960

RemoveUnusedPrivateMethodParameterRector removed parameters from private methods used as callables. The code that receives a callable decides how many arguments it passes, so the callable then received arguments in the wrong positions. This PR fixes two separate causes.

self::method(...) and static::method(...)

#8793 moved the isFirstClassCallable() early return to the top of PHPStanNodeScopeResolver::processCallLike(). The class name of a first-class callable then got no scope, and NameTypeResolver needs the scope to resolve self and static. It returned an ObjectType for a class literally named self, so PrivateMethodParamRemover::resolveCallers() did not match the call and the isFirstClassCallable() guard never ran.

The early return only has to protect getArgs(), so this PR moves it back below the scope assignments. This is an engine change, not a rule change, so any rule that reads the type of such a class name now gets the real class. The full test suite passes with it.

Array callables

PrivateMethodParamRemover did not look at arrays at all. It now uses ArrayCallableMethodMatcher, as IsClassMethodUsedAnalyzer does, and keeps the parameter for [self::class, 'method'], [static::class, 'method'] and [$this, 'method'].

It also keeps the parameter when the matcher returns ArrayCallableDynamicMethod, for example [$this, $method]. This is deliberately cautious: the rule skips that class's method rather than risk removing a parameter in use.

The guard searches the whole class, so it also covers a callable in a property default.

Not fixed here

ArrayCallableMethodMatcher returns null for arrays passed to register_shutdown_function() and forward_static_call(), so this rule still removes the parameter in that case. The same exclusion makes RemoveUnusedPrivateMethodRector delete a private method that is only registered through register_shutdown_function([$this, 'method']). Changing the matcher also affects other rules, so I left it out of this PR.

Verification

  • All nine new skip_* fixtures failed before the fix and pass now. remove_with_array_callable_of_other_class passes both ways: it stops the guard matching a same-named method on another class.
  • composer complete-check equivalents pass: fastunit, ECS and PHPStan. bin/rector process --dry-run on this repository reports no changes. I did not run the e2e tests.
  • On a real codebase, Rector 2.8.4 with only this rule flagged the file that originally broke at runtime. This branch flags nothing.

🤖 Generated with Claude Code

phinor and others added 2 commits November 5, 2026 10:43
processCallLike() returned early for every first-class callable, which
left the class name of self::method(...) and static::method(...) without
a scope. NameTypeResolver needs that scope to resolve self and static, so
it returned an ObjectType for a class literally named "self" or "static".

RemoveUnusedPrivateMethodParameterRector could then not match the callable
to its class, missed the isFirstClassCallable() guard and removed a
parameter from a method whose arity the receiving code controls.

The early return only has to protect getArgs(), so it moves back below the
scope assignments, where it was before rectorphp#8793.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sed parameter

The code receiving [self::class, 'method'], [static::class, 'method'] or
[$this, 'method'] decides how many arguments it passes, the same as with a
first-class callable. PrivateMethodParamRemover never looked at arrays, so
it removed the parameter.

It now keeps the parameter when an array in the class names the method, or
when ArrayCallableMethodMatcher cannot tell which method the array names.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@samsonasik

Copy link
Copy Markdown
Member

Thank you @phinor

@samsonasik
samsonasik merged commit 5a82213 into rectorphp:main Nov 5, 2026
51 checks passed
@phinor
phinor deleted the fix-remove-unused-private-method-param-callable branch November 5, 2026 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

RemoveUnusedPrivateMethodParameterRector removes parameters of a method passed as self::method (...) or static::method (...)

2 participants