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.

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

Allow using Money, Currency and Context in Psalm's immutable contexts #75

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 2 additions & 0 deletions src/AbstractMoney.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*
* Please consider this class sealed: extending this class yourself is not supported, and breaking changes (such as
* adding new abstract methods) can happen at any time, even in a minor version.
*
* @psalm-immutable
*/
abstract class AbstractMoney implements MoneyContainer, Stringable, JsonSerializable
{
Expand Down
2 changes: 2 additions & 0 deletions src/Context.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

/**
* Adjusts a rational number to a decimal amount.
*
* @psalm-immutable
*/
interface Context
{
Expand Down
2 changes: 2 additions & 0 deletions src/Context/AutoContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

/**
* Automatically adjusts the scale of a number to the strict minimum.
*
* @psalm-immutable
*/
final class AutoContext implements Context
{
Expand Down
2 changes: 2 additions & 0 deletions src/Context/CashContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

/**
* Adjusts a number to the default scale for the currency, respecting a cash rounding.
*
* @psalm-immutable
*/
final class CashContext implements Context
{
Expand Down
2 changes: 2 additions & 0 deletions src/Context/CustomContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

/**
* Adjusts a number to a custom scale, and optionally step.
*
* @psalm-immutable
*/
final class CustomContext implements Context
{
Expand Down
2 changes: 2 additions & 0 deletions src/Context/DefaultContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

/**
* Adjusts a number to the default scale for the currency.
*
* @psalm-immutable
*/
final class DefaultContext implements Context
{
Expand Down
2 changes: 2 additions & 0 deletions src/Currency.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

/**
* A currency. This class is immutable.
*
* @psalm-immutable
*/
final class Currency implements Stringable
{
Expand Down
22 changes: 22 additions & 0 deletions src/Money.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
* - CashContext is similar to DefaultContext, but supports a cash rounding step.
* - CustomContext handles monies with a custom scale, and optionally step.
* - AutoContext automatically adjusts the scale of the money to the minimum required.
*
* @psalm-immutable
*/
final class Money extends AbstractMoney
{
Expand Down Expand Up @@ -71,6 +73,8 @@ private function __construct(BigDecimal $amount, Currency $currency, Context $co
* @return Money
*
* @throws MoneyMismatchException If all the monies are not in the same currency.
*
* @psalm-pure
*/
public static function min(Money $money, Money ...$monies) : Money
{
Expand All @@ -96,6 +100,8 @@ public static function min(Money $money, Money ...$monies) : Money
* @return Money
*
* @throws MoneyMismatchException If all the monies are not in the same currency.
*
* @psalm-pure
*/
public static function max(Money $money, Money ...$monies) : Money
{
Expand All @@ -121,6 +127,8 @@ public static function max(Money $money, Money ...$monies) : Money
* @return Money
*
* @throws MoneyMismatchException If all the monies are not in the same currency and context.
*
* @psalm-pure
*/
public static function total(Money $money, Money ...$monies) : Money
{
Expand All @@ -146,6 +154,8 @@ public static function total(Money $money, Money ...$monies) : Money
* @return Money
*
* @throws RoundingNecessaryException If RoundingMode::UNNECESSARY is used but rounding is necessary.
*
* @psalm-pure
*/
public static function create(BigNumber $amount, Currency $currency, Context $context, int $roundingMode = RoundingMode::UNNECESSARY) : Money
{
Expand Down Expand Up @@ -641,6 +651,10 @@ private function gcdOfMultipleInt(array $values): int
{
$values = array_map(fn (int $value) => BigInteger::of($value), $values);

/**
* @psalm-suppress ImpureMethodCall
* FIXME: remove the suppression after BigInteger::gcdMultiple() is marked @psalm-pure
*/
Copy link
Member

Choose a reason for hiding this comment

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

What prevents BigInteger::gcdMultiple() from being marked as pure?

Copy link
Author

@someniatko someniatko Sep 28, 2023

Choose a reason for hiding this comment

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

this is another library, hence out of scope of this PR

return BigInteger::gcdMultiple(...$values)->toInt();
}

Expand Down Expand Up @@ -745,6 +759,10 @@ public function convertedTo(
int $roundingMode = RoundingMode::UNNECESSARY,
) : Money {
if (! $currency instanceof Currency) {
/**
* FIXME: dunno what to do here, Currency::of() uses ISOCurrencyProvider which is heavily "impure".
* @psalm-suppress ImpureMethodCall
*/
$currency = Currency::of($currency);
}

Expand All @@ -769,6 +787,7 @@ public function convertedTo(
*/
public function formatWith(\NumberFormatter $formatter) : string
{
/** @psalm-suppress ImpureMethodCall */
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to suppress this?

As I understand it, this method is not pure as you could pass the same NumberFormatter instance twice but configured differently, and the method would return different results?

Copy link
Author

Choose a reason for hiding this comment

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

this is bad news, because formatWith() will prevent Money from being a @psalm-immutable object otherwise :(

return $formatter->formatCurrency(
$this->amount->toFloat(),
$this->currency->getCurrencyCode()
Expand All @@ -785,6 +804,9 @@ public function formatWith(\NumberFormatter $formatter) : string
* @param bool $allowWholeNumber Whether to allow formatting as a whole number if the amount has no fraction.
*
* @return string
*
* @psalm-suppress ImpureMethodCall - \NumberFormatter is impure, but we use it here in a side effect free way
* @psalm-suppress ImpureStaticVariable - static variables are used for optimization reasons
*/
public function formatTo(string $locale, bool $allowWholeNumber = false) : string
{
Expand Down
2 changes: 2 additions & 0 deletions src/RationalMoney.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*
* This is used to represent intermediate calculation results, and may not be exactly convertible to a decimal amount
* with a finite number of digits. The final conversion to a Money may require rounding.
*
* @psalm-immutable
*/
final class RationalMoney extends AbstractMoney
{
Expand Down