Skip to content

Add missing types to the codebase#433

Merged
greg0ire merged 1 commit intodoctrine:3.0.xfrom
greg0ire:restore-types
Oct 16, 2024
Merged

Add missing types to the codebase#433
greg0ire merged 1 commit intodoctrine:3.0.xfrom
greg0ire:restore-types

Conversation

@greg0ire
Copy link
Member

Some of the types were wrongly reverted in f741a20, some others were never added.

@greg0ire greg0ire marked this pull request as draft October 14, 2024 21:13
@greg0ire greg0ire marked this pull request as ready for review October 14, 2024 21:15
Comparison::GT => static fn ($object): bool => self::getObjectFieldValue($object, $field) > $value,
Comparison::GTE => static fn ($object): bool => self::getObjectFieldValue($object, $field) >= $value,
Comparison::IN => static function ($object) use ($field, $value): bool {
Comparison::EQ => static fn (object|array $object): bool => self::getObjectFieldValue($object, $field) === $value,
Copy link
Member Author

Choose a reason for hiding this comment

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

This test is the reason why I used array|object here and below:

public function testArrayComparison(): void
{
$closure = $this->visitor->walkComparison($this->builder->eq('foo', 42));
self::assertTrue($closure(['foo' => 42]));
}
}

private function andExpressions(array $expressions): Closure
{
return static fn ($object): bool => array_all(
return static fn (object $object): bool => array_all(
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't use object|array here though. Running the ORM test suite with that type hint causes no issues.

SenseException
SenseException previously approved these changes Oct 14, 2024
@greg0ire greg0ire added this to the 3.0.0 milestone Oct 16, 2024
@greg0ire greg0ire merged commit 471407e into doctrine:3.0.x Oct 16, 2024
@greg0ire greg0ire deleted the restore-types branch October 16, 2024 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants