Skip to content

specific ignore for variadics #21

@savinmikhail

Description

@savinmikhail

I ran into a case where AddNamedArgumentsRector produces valid PHP syntax, but the resulting code breaks at runtime for libraries that explicitly reject named arguments passed into variadic parameters.

Problem

With the default behavior, the rule can transform variadic method calls like this:

$qb->addSelect('item');                                                                                                                                                                                                           
                                                                                                                                                                                                                                  
into:                                                                                                                                                                                                                             
                                                                                                                                                                                                                                  
$qb->addSelect(select1: 'item');                                                                                                                                                                                                  
                                                                                                                                                                                                                                  
This is valid PHP, and for some userland variadic methods it works fine because named variadic arguments become associative array entries.                                                                                        
                                                                                                                                                                                                                                  
However, some libraries explicitly reject this pattern. A concrete example is Doctrine ORM QueryBuilder:                                                                                                                          
                                                                                                                                                                                                                                  
- Doctrine\ORM\QueryBuilder::addSelect(mixed ...$select)                                                                                                                                                                          
- Doctrine\ORM\QueryBuilder::where(mixed ...$predicates)                                                                                                                                                                          
- Doctrine\ORM\QueryBuilder::andWhere(mixed ...$where)                                                                                                                                                                            
                                                                                                                                                                                                                                  
Doctrine internally validates variadic parameters and throws if they contain unknown named arguments.                                                                                                                             
                                                                                                                                                                                                                                  
So code transformed by the rector can fail at runtime with errors like:                                                                                                                                                           
                                                                                                                                                                                                                                  
Invalid call to Doctrine\ORM\QueryBuilder::addSelect(), unknown named arguments: select1                                                                                                                                          
                                                                                                                                                                                                                                  
## Minimal example                                                                                                                                                                                                                
                                                                                                                                                                                                                                  
Before Rector:                                                                                                                                                                                                                    
                                                                                                                                                                                                                                  
$qb = $repository->createQueryBuilder('category')                                                                                                                                                                                 
    ->leftJoin('category.items', 'item')                                                                                                                                                                                          
    ->addSelect('item')                                                                                                                                                                                                           
    ->where('category.id = :id')                                                                                                                                                                                                  
    ->setParameter('id', $id);                                                                                                                                                                                                    
                                                                                                                                                                                                                                  
After Rector:                                                                                                                                                                                                                     
                                                                                                                                                                                                                                  
$qb = $repository->createQueryBuilder(alias: 'category')                                                                                                                                                                          
    ->leftJoin(join: 'category.items', alias: 'item')                                                                                                                                                                             
    ->addSelect(select1: 'item')                                                                                                                                                                                                  
    ->where(predicates1: 'category.id = :id')                                                                                                                                                                                     
    ->setParameter(key: 'id', value: $id);                                                                                                                                                                                        
                                                                                                                                                                                                                                  
The addSelect() / where() calls then break at runtime.                                                                                                                                                                            
                                                                                                                                                                                                                                  
## Current workarounds                                                                                                                                                                                                            
                                                                                                                                                                                                                                  
I see two current options:                                                                                                                                                                                                        
                                                                                                                                                                                                                                  
1. Set ALLOW_NAMED_VARIADIC_ARGUMENTS => false                                                                                                                                                                                    
2. Implement a custom ConfigStrategy                                                                                                                                                                                              

Both work, but they are a bit too coarse / heavy:                                                                                                                                                                                 
                                                                                                                                                                                                                                  
- option 1 disables named variadic arguments globally                                                                                                                                                                             
- option 2 requires custom PHP code even for a small denylist of known problematic methods                                                                                                                                        
                                                                                                                                                                                                                                  
## Feature request                                                                                                                                                                                                                
                                                                                                                                                                                                                                  
Would it make sense to add a config-level skip list for specific calls/methods, especially variadic ones?                                                                                                                         
                                                                                                                                                                                                                                  
Something along these lines:                                                                                                                                                                                                      
                                                                                                                                                                                                                                  
$rectorConfig->ruleWithConfiguration(                                                                                                                                                                                             
    AddNamedArgumentsRector::class,                                                                                                                                                                                               
    [                                                                                                                                                                                                                             
        AddNamedArgumentsRector::SKIP_VARIADIC_CALLS => [                                                                                                                                                                         
            'Doctrine\ORM\QueryBuilder::addSelect',                                                                                                                                                                               
            'Doctrine\ORM\QueryBuilder::where',                                                                                                                                                                                   
            'Doctrine\ORM\QueryBuilder::andWhere',                                                                                                                                                                                
            'Doctrine\ORM\QueryBuilder::orWhere',                                                                                                                                                                                 
            'Doctrine\ORM\QueryBuilder::groupBy',                                                                                                                                                                                 
            'Doctrine\ORM\QueryBuilder::addGroupBy',                                                                                                                                                                              
            'Doctrine\ORM\QueryBuilder::having',                                                                                                                                                                                  
            'Doctrine\ORM\QueryBuilder::andHaving',                                                                                                                                                                               
            'Doctrine\ORM\QueryBuilder::orHaving',                                                                                                                                                                                
        ],                                                                                                                                                                                                                        
    ]                                                                                                                                                                                                                             
);                                                                                                                                                                                                                                
                                                                                                                                                                                                                                  
Or maybe a more general option like:                                                                                                                                                                                              
                                                                                                                                                                                                                                  
AddNamedArgumentsRector::SKIP_CALLS => [                                                                                                                                                                                          
    'Doctrine\ORM\QueryBuilder::*',                                                                                                                                                                                               
]                                                                                                                                                                                                                                 
                                                                                                                                                                                                                                  
## Why this would help                                                                                                                                                                                                            
                                                                                                                                                                                                                                  
This would allow keeping named arguments enabled for most code, including safe userland variadics, while excluding known runtime-incompatible APIs without having to write a custom strategy.                                     
                                                                                                                                                                                                                                  
## Note                                                                                                                                                                                                                           
                                                                                                                                                                                                                                  
I know this can already be solved with a custom ConfigStrategy, so this is mainly a DX/configurability request, not a claim that the package is unusable right now.                                                               
                                                                                                                                                                                                                                  
If you think this should be solved via strategy only, that’s fine too, but a built-in skip list would be much more convenient for real projects.                                                                                  
                                 

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions