Skip to content

fix ControllerMethodInjectionToConstructorRector - update call sites when params are moved to constructor#934

Merged
TomasVotruba merged 2 commits into
mainfrom
claude/fix-rector-symfony-issue-0dQg6
May 16, 2026
Merged

fix ControllerMethodInjectionToConstructorRector - update call sites when params are moved to constructor#934
TomasVotruba merged 2 commits into
mainfrom
claude/fix-rector-symfony-issue-0dQg6

Conversation

@TomasVotruba
Copy link
Copy Markdown
Member

Problem

Fixes rectorphp/rector#9695

When a controller has a helper method that takes both a service parameter and optional scalar parameters:

final class SomeController extends AbstractController
{
    #[Route('/some-action')]
    public function someAction(LoggerInterface $logger)
    {
        $this->problematic($logger);
    }

    public function problematic(LoggerInterface $logger, ?string $optional = 'test')
    {
        $logger->log('level', 'value');
        dump($optional);
    }
}

The rule correctly moved LoggerInterface $logger out of both method signatures and into the constructor, and replaced $logger usages inside each method body with $this->logger. However, it did not update the call site $this->problematic($logger) in someAction — after $logger was replaced with $this->logger by the variable-replacement step, the call became $this->problematic($this->logger), even though problematic no longer accepts that argument. This produced invalid code.

Fix

After all param removals and variable replacements, a new updateCallSitesForRemovedParams step traverses every method body in the class. For any $this->methodName(...) call whose method had parameters removed, the corresponding positional arguments are removed from the call.

Changes

  • rules/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector.php — track removed param positions per method; add updateCallSitesForRemovedParams() that strips the stale arguments from internal call sites
  • rules-tests/.../Fixture/internal_method_call_arg_removal.php.inc — new fixture reproducing the exact scenario from the issue

Generated by Claude Code

claude and others added 2 commits May 16, 2026 20:49
…rgs when method params are moved to constructor

When a helper method (e.g. problematic(LoggerInterface $logger, ?string $optional)) had a
service param moved to the constructor, any $this->problematic($logger) call sites in other
methods were not updated to drop the now-removed argument, producing invalid code.

The fix tracks which parameter positions are removed per method and, after all existing
transforms run, traverses every method body to remove the corresponding args from
$this->methodName(...) call sites.

Fixes rectorphp/rector#9695
@TomasVotruba TomasVotruba merged commit 55ece61 into main May 16, 2026
7 checks passed
@TomasVotruba TomasVotruba deleted the claude/fix-rector-symfony-issue-0dQg6 branch May 16, 2026 20:52
@TomasVotruba
Copy link
Copy Markdown
Member Author

Good job :)

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.

ControllerMethodInjectionToConstructorRector removing parameters from definitions but not calls

3 participants