Skip to content

More precise type inference with unary plus and minus #580

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

Merged
merged 2 commits into from
Jun 25, 2024
Merged

More precise type inference with unary plus and minus #580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
efa39f1
More precise type inference with unary plus and minus
janedbal Jun 25, 2024
cb04d4b
Add unary minus test
janedbal Jun 25, 2024
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
14 changes: 13 additions & 1 deletion src/Type/Doctrine/Query/QueryResultTypeWalker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1261,7 +1261,19 @@ public function walkArithmeticFactor($factor): string
$primary = $factor->arithmeticPrimary;

$type = $this->unmarshalType($this->walkArithmeticPrimary($primary));
$type = TypeUtils::generalizeType($type, GeneralizePrecision::lessSpecific());

if ($type instanceof ConstantIntegerType && $factor->sign === false) {
$type = new ConstantIntegerType($type->getValue() * -1);
Copy link
Member

Choose a reason for hiding this comment

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

  1. What about sign=true?
  2. Looks like this doesn't have tests, at least from the diff

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Sign true is +1, sign null is 1, those dont change value of constant type
  2. Tests are present in origin: #506 commit. Mostly I didnt add testcases to the old test as that one is MUCH worse. But I can add simple one to this separated PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test I'm trying to write produces wrong results as those two are not merged yet:

DQL from the test:

SELECT		-o.intColumn as minusInt,
		-o.floatColumn as minusFloat,
		-COUNT(o.intColumn) as minusIntRange
FROM		QueryResult\Entities\One o

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test added, it will fail once rebased on those PRs.


} elseif ($type instanceof IntegerRangeType && $factor->sign === false) {
$type = IntegerRangeType::fromInterval(
$type->getMax() === null ? null : $type->getMax() * -1,
$type->getMin() === null ? null : $type->getMin() * -1
);

} elseif ($type instanceof ConstantFloatType && $factor->sign === false) {
$type = new ConstantFloatType($type->getValue() * -1);
Copy link
Member

Choose a reason for hiding this comment

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

Same questions as above

}

return $this->marshalType($type);
}
Expand Down
20 changes: 18 additions & 2 deletions tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public static function setUpBeforeClass(): void

$dataOne = [
'intColumn' => [1, 2],
'floatColumn' => [0.1, 2.0],
'stringColumn' => ['A', 'B'],
'stringNullColumn' => ['A', null],
];
Expand All @@ -108,10 +109,11 @@ public static function setUpBeforeClass(): void
$id = 1;

foreach (self::combinations($dataOne) as $combination) {
[$intColumn, $stringColumn, $stringNullColumn] = $combination;
[$intColumn, $floatColumn, $stringColumn, $stringNullColumn] = $combination;
$one = new One();
$one->id = (string) $id++;
$one->intColumn = $intColumn;
$one->floatColumn = $floatColumn;
$one->stringColumn = $stringColumn;
$one->stringNullColumn = $stringNullColumn;
$embedded = new Embedded();
Expand Down Expand Up @@ -1189,7 +1191,7 @@ public function getTestData(): iterable
yield 'arithmetic' => [
$this->constantArray([
[new ConstantStringType('intColumn'), new IntegerType()],
[new ConstantIntegerType(1), $this->intStringified()],
[new ConstantIntegerType(1), TypeCombinator::union(new ConstantIntegerType(1), new ConstantStringType('1'))],
[new ConstantIntegerType(2), $this->intStringified()],
[new ConstantIntegerType(3), TypeCombinator::addNull($this->intStringified())],
[new ConstantIntegerType(4), $this->intStringified()],
Expand Down Expand Up @@ -1614,6 +1616,20 @@ public function getTestData(): iterable
FROM QueryResult\Entities\One o
',
];

yield 'unary minus' => [
$this->constantArray([
[new ConstantStringType('minusInt'), $this->numericStringOrInt()], // should be nullable
[new ConstantStringType('minusFloat'), TypeCombinator::union(new FloatType(), $this->numericStringOrInt())], // should be nullable && should not include int
[new ConstantStringType('minusIntRange'), TypeCombinator::union(IntegerRangeType::fromInterval(null, 0), $this->numericString())],
]),
'
SELECT -o.intColumn as minusInt,
-o.floatColumn as minusFloat,
-COUNT(o.intColumn) as minusIntRange
FROM QueryResult\Entities\One o
',
];
}

/**
Expand Down
7 changes: 7 additions & 0 deletions tests/Type/Doctrine/data/QueryResult/Entities/One.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ class One
*/
public $intColumn;

/**
* @Column(type="float")
*
* @var float
*/
public $floatColumn;

/**
* @Column(type="string")
*
Expand Down