Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix types on ResultSetMapping #9621

Merged
merged 1 commit into from
Apr 4, 2022
Merged

Fix types on ResultSetMapping #9621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 15 additions & 12 deletions lib/Doctrine/ORM/Query/ResultSetMapping.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class ResultSetMapping
* Maps column names in the result set to the alias/field name to use in the mapped result.
*
* @ignore
* @psalm-var array<string, string>
* @psalm-var array<string, string|int>
*/
public $scalarMappings = [];

Expand Down Expand Up @@ -169,6 +169,7 @@ class ResultSetMapping
* results or joined entity results within this ResultSetMapping.
* @param string|null $resultAlias The result alias with which the entity result should be
* placed in the result structure.
* @psalm-param class-string $class
*
* @return $this
*
Expand Down Expand Up @@ -315,6 +316,7 @@ public function isFieldResult($columnName)
* the field $fieldName is defined on a subclass, specify that here.
* If not specified, the field is assumed to belong to the class
* designated by $alias.
* @psalm-param class-string|null $declaringClass
*
* @return $this
*
Expand Down Expand Up @@ -344,6 +346,7 @@ public function addFieldResult($alias, $columnName, $fieldName, $declaringClass
* @param string $parentAlias The alias of the entity result that is the parent of this joined result.
* @param string $relation The association field that connects the parent entity result
* with the joined entity result.
* @psalm-param class-string $class
*
* @return $this
*
Expand All @@ -361,9 +364,9 @@ public function addJoinedEntityResult($class, $alias, $parentAlias, $relation)
/**
* Adds a scalar result mapping.
*
* @param string $columnName The name of the column in the SQL result set.
* @param string $alias The result alias with which the scalar result should be placed in the result structure.
* @param string $type The column type
* @param string $columnName The name of the column in the SQL result set.
* @param string|int $alias The result alias with which the scalar result should be placed in the result structure.
Copy link
Member Author

@derrabus derrabus Mar 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm widening the type here because we pass an integer in this line:

orm/lib/Doctrine/ORM/Query/SqlWalker.php

Line 1637 in 18d6bc3

$this->rsm->addScalarResult($columnAlias, $resultAlias, $fieldType);

The $resultAlias is definitely an int as we can see here:

orm/lib/Doctrine/ORM/Query/SqlWalker.php

Line 1587 in 18d6bc3

$resultAlias = $this->scalarResultCounter++;

An alternative approach would be to cast the integer to a string.

* @param string $type The column type
*
* @return $this
*
Expand All @@ -384,8 +387,8 @@ public function addScalarResult($columnName, $alias, $type = 'string')
/**
* Adds a metadata parameter mappings.
*
* @param mixed $parameter The parameter name in the SQL result set.
* @param string $attribute The metadata attribute.
* @param string|int $parameter The parameter name in the SQL result set.
* @param string $attribute The metadata attribute.
*
* @return void
*/
Expand Down Expand Up @@ -426,7 +429,7 @@ public function getClassName($alias)
*
* @param string $columnName The name of the column in the SQL result set.
*
* @return string
* @return string|int
*/
public function getScalarAlias($columnName)
{
Expand Down Expand Up @@ -549,11 +552,11 @@ public function isMixedResult()
/**
* Adds a meta column (foreign key or discriminator column) to the result set.
*
* @param string $alias The result alias with which the meta result should be placed in the result structure.
* @param string $columnName The name of the column in the SQL result set.
* @param string $fieldName The name of the field on the declaring class.
* @param bool $isIdentifierColumn
* @param string $type The column type
* @param string $alias The result alias with which the meta result should be placed in the result structure.
* @param string $columnName The name of the column in the SQL result set.
* @param string $fieldName The name of the field on the declaring class.
* @param bool $isIdentifierColumn
* @param string|null $type The column type
*
* @return $this
*
Expand Down
23 changes: 20 additions & 3 deletions lib/Doctrine/ORM/Query/ResultSetMappingBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
use Doctrine\ORM\Mapping\MappingException;
use Doctrine\ORM\Utility\PersisterHelper;
use InvalidArgumentException;
use LogicException;

use function assert;
use function explode;
use function in_array;
use function sprintf;
Expand Down Expand Up @@ -57,11 +59,13 @@ class ResultSetMappingBuilder extends ResultSetMapping
* Default column renaming mode.
*
* @var int
* @psalm-var self::COLUMN_RENAMING_*
*/
private $defaultRenameMode;

/**
* @param int $defaultRenameMode
* @psalm-param self::COLUMN_RENAMING_* $defaultRenameMode
*/
public function __construct(EntityManagerInterface $em, $defaultRenameMode = self::COLUMN_RENAMING_NONE)
{
Expand All @@ -76,7 +80,9 @@ public function __construct(EntityManagerInterface $em, $defaultRenameMode = sel
* @param string $alias The unique alias to use for the root entity.
* @param string[] $renamedColumns Columns that have been renamed (tableColumnName => queryColumnName).
* @param int|null $renameMode One of the COLUMN_RENAMING_* constants or array for BC reasons (CUSTOM).
* @psalm-param class-string $class
* @psalm-param array<string, string> $renamedColumns
* @psalm-param self::COLUMN_RENAMING_*|null $renameMode
*
* @return void
*/
Expand All @@ -99,7 +105,9 @@ public function addRootEntityFromClassMetadata($class, $alias, $renamedColumns =
* with the joined entity result.
* @param string[] $renamedColumns Columns that have been renamed (tableColumnName => queryColumnName).
* @param int|null $renameMode One of the COLUMN_RENAMING_* constants or array for BC reasons (CUSTOM).
* @psalm-param class-string $class
* @psalm-param array<string, string> $renamedColumns
* @psalm-param self::COLUMN_RENAMING_*|null $renameMode
*
* @return void
*/
Expand Down Expand Up @@ -186,6 +194,8 @@ private function isInheritanceSupported(ClassMetadataInfo $classMetadata): bool
* Gets column alias for a given column.
*
* @psalm-param array<string, string> $customRenameColumns
*
* @psalm-assert self::COLUMN_RENAMING_* $mode
*/
private function getColumnAlias(string $columnName, int $mode, array $customRenameColumns): string
{
Expand Down Expand Up @@ -273,8 +283,10 @@ public function addNamedNativeQueryMapping(ClassMetadataInfo $class, array $quer
public function addNamedNativeQueryResultClassMapping(ClassMetadataInfo $class, $resultClassName)
{
$classMetadata = $this->em->getClassMetadata($resultClassName);
$shortName = $classMetadata->reflClass->getShortName();
$alias = strtolower($shortName[0]) . '0';
assert($classMetadata->reflClass !== null);

$shortName = $classMetadata->reflClass->getShortName();
$alias = strtolower($shortName[0]) . '0';

$this->addEntityResult($class->name, $alias);

Expand Down Expand Up @@ -316,14 +328,19 @@ public function addNamedNativeQueryResultClassMapping(ClassMetadataInfo $class,
*/
public function addNamedNativeQueryResultSetMapping(ClassMetadataInfo $class, $resultSetMappingName)
{
if ($class->reflClass === null) {
throw new LogicException('Given class metadata has now class reflector.');
}

$counter = 0;
$resultMapping = $class->getSqlResultSetMapping($resultSetMappingName);
$rootShortName = $class->reflClass->getShortName();
$rootAlias = strtolower($rootShortName[0]) . $counter;

if (isset($resultMapping['entities'])) {
foreach ($resultMapping['entities'] as $key => $entityMapping) {
foreach ($resultMapping['entities'] as $entityMapping) {
$classMetadata = $this->em->getClassMetadata($entityMapping['entityClass']);
assert($classMetadata->reflClass !== null);

if ($class->reflClass->name === $classMetadata->reflClass->name) {
$this->addEntityResult($classMetadata->name, $rootAlias);
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/ORM/Query/SqlWalker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class SqlWalker implements TreeWalker
/**
* Map from result variable names to their SQL column alias names.
*
* @psalm-var array<string, string|list<string>>
* @psalm-var array<string|int, string|list<string>>
*/
private $scalarResultAliasMap = [];

Expand Down
5 changes: 0 additions & 5 deletions phpstan-baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -960,11 +960,6 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Query/SqlWalker.php

-
message: "#^Parameter \\#2 \\$alias of method Doctrine\\\\ORM\\\\Query\\\\ResultSetMapping\\:\\:addScalarResult\\(\\) expects string, int given\\.$#"
count: 1
path: lib/Doctrine/ORM/Query/SqlWalker.php

-
message: "#^Result of && is always false\\.$#"
count: 2
Expand Down
36 changes: 2 additions & 34 deletions psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2367,29 +2367,10 @@
<code>Comparison::EQ</code>
</RedundantConditionGivenDocblockType>
</file>
<file src="lib/Doctrine/ORM/Query/ResultSetMapping.php">
<PropertyTypeCoercion occurrences="3">
<code>$this-&gt;aliasMap</code>
<code>$this-&gt;aliasMap</code>
<code>$this-&gt;declaringClasses</code>
</PropertyTypeCoercion>
</file>
<file src="lib/Doctrine/ORM/Query/ResultSetMappingBuilder.php">
<ArgumentTypeCoercion occurrences="5">
<code>$class</code>
<code>$class</code>
<ArgumentTypeCoercion occurrences="1">
<code>$class</code>
<code>$renameMode</code>
<code>$renameMode</code>
</ArgumentTypeCoercion>
<PossiblyNullPropertyFetch occurrences="1">
<code>$classMetadata-&gt;reflClass-&gt;name</code>
</PossiblyNullPropertyFetch>
<PossiblyNullReference occurrences="3">
<code>getShortName</code>
<code>getShortName</code>
<code>getShortName</code>
</PossiblyNullReference>
</file>
<file src="lib/Doctrine/ORM/Query/SqlWalker.php">
<DocblockTypeContradiction occurrences="5">
Expand All @@ -2411,20 +2392,7 @@
<InvalidNullableReturnType occurrences="1">
<code>walkConditionalPrimary</code>
</InvalidNullableReturnType>
<InvalidPropertyAssignmentValue occurrences="7">
<code>$this-&gt;scalarResultAliasMap</code>
<code>$this-&gt;scalarResultAliasMap</code>
<code>$this-&gt;scalarResultAliasMap</code>
<code>$this-&gt;scalarResultAliasMap</code>
<code>$this-&gt;scalarResultAliasMap</code>
<code>$this-&gt;scalarResultAliasMap</code>
<code>$this-&gt;scalarResultAliasMap</code>
</InvalidPropertyAssignmentValue>
<InvalidScalarArgument occurrences="7">
<code>$resultAlias</code>
<code>$resultAlias</code>
<code>$resultAlias</code>
<code>$resultAlias</code>
<InvalidScalarArgument occurrences="3">
<code>$this-&gt;queryComponents[$expr]['token']['value']</code>
<code>$this-&gt;queryComponents[$factor]['token']['value']</code>
<code>$this-&gt;queryComponents[$term]['token']['value']</code>
Expand Down