Skip to content

Conversation

@janedbal
Copy link
Contributor

Separated from #506

$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 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

@ondrejmirtes ondrejmirtes merged commit 25e200b into phpstan:1.4.x Jun 25, 2024
@ondrejmirtes
Copy link
Member

Thank you.

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