[WIP] Orm querybuilder set parameters to collection v2 #346
Conversation
…ection for the setParameters ORM QueryBuilder method
…ters if the variable is used in a ORM QueryBuilder
There was a problem hiding this comment.
Hi @marcelthole.
Thought I could chime in and comment on your code a bit.
| $arrayValueType = $this->nodeTypeResolver->getType($value->value); | ||
| if (! $arrayValueType instanceof ObjectType || ! $arrayValueType->isInstanceOf( | ||
| 'Doctrine\\ORM\\Query\\Parameter' | ||
| )->yes()) { |
There was a problem hiding this comment.
Same here
$arrayValueType->isInstanceOf('Doctrine\\ORM\\Query\\Parameter')->no()| return null; | ||
| } | ||
|
|
||
| if (! $varType->isInstanceOf('Doctrine\\ORM\\QueryBuilder')->yes()) { |
There was a problem hiding this comment.
Same here:
$varType->isInstanceOf('Doctrine\\ORM\\QueryBuilder')->no()| && $stmt->expr->name->name === 'setParameters' | ||
| ) { | ||
| $varType = $this->nodeTypeResolver->getType($stmt->expr->var); | ||
| if (! $varType->isInstanceOf('Doctrine\\ORM\\QueryBuilder')->yes()) { |
There was a problem hiding this comment.
Instead of not yes you could just use no so
! $varType->isInstanceOf('Doctrine\\ORM\\QueryBuilder')->yes()can be changed to:
$varType->isInstanceOf('Doctrine\\ORM\\QueryBuilder')->no()There was a problem hiding this comment.
overall i would say the same but the varType as also the type MAYBE - And in that cases i do not want to modify the values. But i will try to test this also with an fixture
| @@ -0,0 +1,27 @@ | |||
| <?php | |||
|
|
|||
There was a problem hiding this comment.
like other PR, please add namespace if possible
There was a problem hiding this comment.
i will rebase this PR and apply all the fixes after the other one is done if this is fine for you
| @@ -0,0 +1,27 @@ | |||
| <?php | |||
There was a problem hiding this comment.
use underscored fixture file name instead of dash
| foreach ($variables as $variable) { | ||
| $this->iterateThroughStmts($node->getStmts(), $variable); | ||
| } | ||
|
|
There was a problem hiding this comment.
verify to return null if no change
Additional checks to #326