Skip to content

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Aug 13, 2024

}
}

$result[] = $innerType;
Copy link
Member

Choose a reason for hiding this comment

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

I'd like some DRY around this, we already have the same code elsewhere. We could have a method that just accepts the narrowed array size as Type, and return SpecifiedTypes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if ($orEqual) {
$constantType = IntegerRangeType::createAllGreaterThanOrEqualTo($leftType->getValue());
} else {
$constantType = IntegerRangeType::createAllGreaterThan($leftType->getValue());
Copy link
Member

Choose a reason for hiding this comment

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

We already have this elsewhere:

					$context->true() && (IntegerRangeType::createAllGreaterThanOrEqualTo(1 - $offset)->isSuperTypeOf($leftType)->yes())
					|| ($context->false() && (new ConstantIntegerType(1 - $offset))->isSuperTypeOf($leftType)->yes())

Would be nice to reuse that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think its the same thing. in the first block we work on the concrete constant integer, used in the comparison while in the second block we just check for the comparison value beeing >0 or >1.

@staabm staabm marked this pull request as ready for review August 13, 2024 08:40
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@ondrejmirtes ondrejmirtes merged commit e9c60a2 into phpstan:1.11.x Aug 13, 2024
452 of 462 checks passed
@ondrejmirtes
Copy link
Member

Awesome, 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.

3 participants