QueryCheckerTreeWalker: improve error message

JanTvrdik · JanTvrdik · commit a0a98fd39a60 · 2025-02-21T09:44:36.000+01:00
diff --git a/src/QueryCheckerTreeWalker.php b/src/QueryCheckerTreeWalker.php
@@ -21,6 +21,7 @@
 use Doctrine\ORM\Query\AST\Phase2OptimizableConditional;
 use Doctrine\ORM\Query\AST\SelectStatement;
 use Doctrine\ORM\Query\AST\WhereClause;
+use Doctrine\ORM\Query\Parameter;
 use Doctrine\ORM\Query\TreeWalkerAdapter;
 use Psr\Log\LoggerInterface;
 use ShipMonk\DoctrineQueryChecker\Exception\LogicException;
@@ -157,9 +158,15 @@ protected function verifyInputParameterType(
         InputParameter $inputParameter,
     ): void
     {
-        $inputParameterType = $this->getInputParameterType($inputParameter);
+        $parameter = $this->_getQuery()->getParameter($inputParameter->name);
 
-        if ($inputParameterType === null) {
+        if ($parameter === null) {
+            return; // happens when the query is analyzed by PHPStan
+        }
+
+        $parameterType = $this->getParameterType($parameter);
+
+        if ($parameterType === null) {
             return;
         }
 
@@ -173,27 +180,26 @@ protected function verifyInputParameterType(
             }
         }
 
-        if (in_array($inputParameterType, $compatibleTypesExtended, strict: true)) {
+        if (in_array($parameterType, $compatibleTypesExtended, strict: true)) {
             return;
         }
 
-        if (count($compatibleTypes) === 1) {
-            $expectedDescription = sprintf('"%s"', self::typeToName($compatibleTypes[0]));
+        $parameterTypeName = self::typeToName($parameterType);
 
-        } else {
-            $compatibleTypeNames = array_map(self::typeToName(...), $compatibleTypes);
-            $expectedDescription = sprintf('one of: ["%s"]', implode('", "', $compatibleTypeNames));
-        }
+        $expressionName = $pathExpression->field !== null
+            ? "{$pathExpression->identificationVariable}.{$pathExpression->field}"
+            : $pathExpression->identificationVariable;
 
-        $this->processException(
-            new LogicException(sprintf(
-                'QueryCheckerTreeWalker: Parameter "%s" is of type "%s", but expected %s (because it\'s used in expression with %s)',
-                $inputParameter->name,
-                self::typeToName($inputParameterType),
-                $expectedDescription,
-                $pathExpression->field !== null ? "{$pathExpression->identificationVariable}.{$pathExpression->field}" : $pathExpression->identificationVariable,
-            )),
-        );
+        $expectedTypeName = count($compatibleTypes) === 1
+            ? sprintf("'%s'", self::typeToName($compatibleTypes[0]))
+            : sprintf("one of: ['%s']", implode("', '", array_map(self::typeToName(...), $compatibleTypes)));
+
+        $message = $parameter->typeWasSpecified()
+            ? "Parameter '{$inputParameter->name}' is using '{$parameterTypeName}' type in 3rd argument of setParameter()"
+            : "Parameter '{$inputParameter->name}' has no type specified in 3rd argument of setParameter(). Thus it is inferred as '{$parameterTypeName}'";
+
+        $message .= ", but it is compared with '{$expressionName}' which can only be compared with {$expectedTypeName}.";
+        $this->processException(new LogicException("QueryCheckerTreeWalker: $message"));
     }
 
     private static function typeToName(string|Type|ParameterType|ArrayParameterType|null $type): string
@@ -259,14 +265,8 @@ protected function getFieldCompatibleTypes(
         return $types;
     }
 
-    protected function getInputParameterType(InputParameter $node): string|Type|ParameterType|ArrayParameterType|null
+    protected function getParameterType(Parameter $parameter): string|Type|ParameterType|ArrayParameterType|null
     {
-        $parameter = $this->_getQuery()->getParameter($node->name);
-
-        if ($parameter === null) {
-            return null; // happens when the query is analyzed by PHPStan
-        }
-
         if ($parameter->typeWasSpecified()) {
             return $this->normalizeType($parameter->getType());
         }
diff --git a/tests/QueryCheckerTreeWalkerTest.php b/tests/QueryCheckerTreeWalkerTest.php
@@ -27,7 +27,7 @@ public function testWrongParameterTypeInVariousPlaces(mixed $expr): void
     {
         self::assertException(
             LogicException::class,
-            'QueryCheckerTreeWalker: Parameter "stringField" is of type "float", but expected "string" (because it\'s used in expression with e.stringField)',
+            'QueryCheckerTreeWalker: Parameter \'stringField\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'float\', but it is compared with \'e.stringField\' which can only be compared with \'string\'.',
             function () use ($expr): void {
                 $this->getEntityManager()->createQueryBuilder()
                     ->select('e')
@@ -42,7 +42,7 @@ function () use ($expr): void {
 
         self::assertException(
             LogicException::class,
-            'QueryCheckerTreeWalker: Parameter "stringField" is of type "float", but expected "string" (because it\'s used in expression with e.stringField)',
+            'QueryCheckerTreeWalker: Parameter \'stringField\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'float\', but it is compared with \'e.stringField\' which can only be compared with \'string\'.',
             function () use ($expr): void {
                 $this->getEntityManager()->createQueryBuilder()
                     ->select('e')
@@ -201,113 +201,113 @@ public static function provideWrongParameterTypesData(): iterable
             'e.stringField',
             123,
             null,
-            'Parameter "e_stringField" is of type "integer", but expected "string" (because it\'s used in expression with e.stringField)',
+            'Parameter \'e_stringField\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'integer\', but it is compared with \'e.stringField\' which can only be compared with \'string\'.',
         ];
 
         yield [
             'e.stringField',
             123.4,
             null,
-            'Parameter "e_stringField" is of type "float", but expected "string" (because it\'s used in expression with e.stringField)',
+            'Parameter \'e_stringField\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'float\', but it is compared with \'e.stringField\' which can only be compared with \'string\'.',
         ];
 
         yield [
             'e.stringField',
             TestEntityWithManyFieldTypesStringEnum::A,
             null,
-            'Parameter "e_stringField" is of type "ShipMonkTests\DoctrineQueryChecker\Fixture\Enum\TestEntityWithManyFieldTypesStringEnum", but expected "string" (because it\'s used in expression with e.stringField)',
+            'Parameter \'e_stringField\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'ShipMonkTests\DoctrineQueryChecker\Fixture\Enum\TestEntityWithManyFieldTypesStringEnum\', but it is compared with \'e.stringField\' which can only be compared with \'string\'.',
         ];
 
         yield [
             'e.textField',
             123,
             null,
-            'Parameter "e_textField" is of type "integer", but expected "string" (because it\'s used in expression with e.textField)',
+            'Parameter \'e_textField\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'integer\', but it is compared with \'e.textField\' which can only be compared with \'string\'.',
         ];
 
         yield [
             'e.textField',
             123.4,
             null,
-            'Parameter "e_textField" is of type "float", but expected "string" (because it\'s used in expression with e.textField)',
+            'Parameter \'e_textField\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'float\', but it is compared with \'e.textField\' which can only be compared with \'string\'.',
         ];
 
         yield [
             'e.booleanField',
             'ABC',
             null,
-            'Parameter "e_booleanField" is of type "string", but expected "boolean" (because it\'s used in expression with e.booleanField)',
+            'Parameter \'e_booleanField\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'string\', but it is compared with \'e.booleanField\' which can only be compared with \'boolean\'.',
         ];
 
         yield [
             'e.stringEnumField',
             TestEntityWithManyFieldTypesIntEnum::A,
             null,
-            'Parameter "e_stringEnumField" is of type "ShipMonkTests\DoctrineQueryChecker\Fixture\Enum\TestEntityWithManyFieldTypesIntEnum", but expected one of: ["ShipMonkTests\DoctrineQueryChecker\Fixture\Enum\TestEntityWithManyFieldTypesStringEnum", "string"] (because it\'s used in expression with e.stringEnumField)',
+            'Parameter \'e_stringEnumField\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'ShipMonkTests\DoctrineQueryChecker\Fixture\Enum\TestEntityWithManyFieldTypesIntEnum\', but it is compared with \'e.stringEnumField\' which can only be compared with one of: [\'ShipMonkTests\DoctrineQueryChecker\Fixture\Enum\TestEntityWithManyFieldTypesStringEnum\', \'string\'].',
         ];
 
         yield [
             'e.dateTimeImmutableField',
             '2021-01-01',
             Types::STRING,
-            'Parameter "e_dateTimeImmutableField" is of type "string", but expected "datetime_immutable" (because it\'s used in expression with e.dateTimeImmutableField)',
+            'Parameter \'e_dateTimeImmutableField\' is using \'string\' type in 3rd argument of setParameter(), but it is compared with \'e.dateTimeImmutableField\' which can only be compared with \'datetime_immutable\'.',
         ];
 
         $simpleTestEntityWithUuid = new SimpleTestEntityWithUuid();
         yield [
             'e.simpleTestEntity',
             $simpleTestEntityWithUuid,
             null,
-            'Parameter "e_simpleTestEntity" is of type "ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid", but expected one of: ["ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntity", "integer"] (because it\'s used in expression with e.simpleTestEntity)',
+            'Parameter \'e_simpleTestEntity\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid\', but it is compared with \'e.simpleTestEntity\' which can only be compared with one of: [\'ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntity\', \'integer\'].',
         ];
 
         yield [
             'se.id',
             $simpleTestEntityWithUuid,
             null,
-            'Parameter "se_id" is of type "ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid", but expected one of: ["ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntity", "integer"] (because it\'s used in expression with se.id)',
+            'Parameter \'se_id\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid\', but it is compared with \'se.id\' which can only be compared with one of: [\'ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntity\', \'integer\'].',
         ];
 
         yield [
             'e.simpleTestEntityWithUuid',
             $simpleTestEntityWithUuid->getUuid(),
             null,
-            'Parameter "e_simpleTestEntityWithUuid" is of type "string", but expected one of: ["ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid", "uuid"] (because it\'s used in expression with e.simpleTestEntityWithUuid)',
+            'Parameter \'e_simpleTestEntityWithUuid\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'string\', but it is compared with \'e.simpleTestEntityWithUuid\' which can only be compared with one of: [\'ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid\', \'uuid\'].',
         ];
 
         yield [
             'e.simpleTestEntityWithUuid',
             $simpleTestEntityWithUuid->getUuid(),
             Types::STRING,
-            'Parameter "e_simpleTestEntityWithUuid" is of type "string", but expected one of: ["ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid", "uuid"] (because it\'s used in expression with e.simpleTestEntityWithUuid)',
+            'Parameter \'e_simpleTestEntityWithUuid\' is using \'string\' type in 3rd argument of setParameter(), but it is compared with \'e.simpleTestEntityWithUuid\' which can only be compared with one of: [\'ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid\', \'uuid\'].',
         ];
 
         yield [
             'sewu',
             $simpleTestEntityWithUuid->getUuid(),
             null,
-            'Parameter "sewu" is of type "string", but expected one of: ["ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid", "uuid"] (because it\'s used in expression with sewu.uuid)',
+            'Parameter \'sewu\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'string\', but it is compared with \'sewu.uuid\' which can only be compared with one of: [\'ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid\', \'uuid\'].',
         ];
 
         yield [
             'sewu',
             $simpleTestEntityWithUuid->getUuid(),
             Types::STRING,
-            'Parameter "sewu" is of type "string", but expected one of: ["ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid", "uuid"] (because it\'s used in expression with sewu.uuid)',
+            'Parameter \'sewu\' is using \'string\' type in 3rd argument of setParameter(), but it is compared with \'sewu.uuid\' which can only be compared with one of: [\'ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid\', \'uuid\'].',
         ];
 
         yield [
             'sewu.uuid',
             $simpleTestEntityWithUuid->getUuid(),
             null,
-            'Parameter "sewu_uuid" is of type "string", but expected one of: ["ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid", "uuid"] (because it\'s used in expression with sewu.uuid)',
+            'Parameter \'sewu_uuid\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'string\', but it is compared with \'sewu.uuid\' which can only be compared with one of: [\'ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid\', \'uuid\'].',
         ];
 
         yield [
             'sewu.uuid',
             $simpleTestEntityWithUuid->getUuid(),
             Types::STRING,
-            'Parameter "sewu_uuid" is of type "string", but expected one of: ["ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid", "uuid"] (because it\'s used in expression with sewu.uuid)',
+            'Parameter \'sewu_uuid\' is using \'string\' type in 3rd argument of setParameter(), but it is compared with \'sewu.uuid\' which can only be compared with one of: [\'ShipMonkTests\DoctrineQueryChecker\Fixture\Entity\SimpleTestEntityWithUuid\', \'uuid\'].',
         ];
     }
 
@@ -317,7 +317,7 @@ public function testWillUseLoggerIfAvailable(): void
         $logger->expects(self::once())
             ->method('error')
             ->with(
-                'QueryCheckerTreeWalker: Parameter "value" is of type "float", but expected "boolean" (because it\'s used in expression with e.booleanField)',
+                'QueryCheckerTreeWalker: Parameter \'value\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'float\', but it is compared with \'e.booleanField\' which can only be compared with \'boolean\'.',
                 self::arrayHasKey('exception'),
             );
 
@@ -336,7 +336,7 @@ public function testWillUseLoggerIfAvailable(): void
 
         self::assertException(
             LogicException::class,
-            'QueryCheckerTreeWalker: Parameter "value" is of type "float", but expected "boolean" (because it\'s used in expression with e.booleanField)',
+            'QueryCheckerTreeWalker: Parameter \'value\' has no type specified in 3rd argument of setParameter(). Thus it is inferred as \'float\', but it is compared with \'e.booleanField\' which can only be compared with \'boolean\'.',
             function (): void {
                 $this->getEntityManager()->createQueryBuilder()
                     ->select('e')
