Add Psalm immutability annotations

[nederdirk]

Hi, I just found out about this library by searching for JSR310-inspired libraries for PHP.

This patch helps to ensure that classes marked as immutable will actually be
immutable¹. Downstream projects using both brick/date-time and
Psalm will have the benefit of allowing for more thorough analysis.

Footnotes

1. https://psalm.dev/docs/annotating_code/supported_annotations/#psalm-immutable ↩

[nederdirk]

By the way, the psalm CI job rightly fails, because

1. there are some calls to *::now() (which can never be pure, since it can, and is usually, tied to the wall clock) in immutable classes.
2. The DateTimeParseResult seems to depend on its state mutating, I am not knowledgable enough about brick/date-time to figure out a way to improve this class

These problems can either be fixed by adding them into a psalm-baseline file, or by adding @psalm-suppress docblock lines in strategic places.

• The PHPunit job may fail because of the 2 DateTime ➡️ DateTimeImmutable changes I included, I'm looking into that 😄

[BenMorel]

Hi, thanks for your PR. I would love to annotate immutable classes as such, however it looks like your attempt outlined a number of issues that may prevent us from doing so:

• I'm a bit worried by the fact that you have to eagerly calculate things in the constructor in 24986dc; IMO, even if the class guarantees immutability from an external perspective, nothing prevents it from lazy-initializing internal properties, especially when these calculations are not always needed and may be costly;
• immutable should not imply pure IMO (and if I understand correctly The way Psalm treats the concepts of pure and immutable is conceptually wrong and misleading vimeo/psalm#3135, this is not only my opinion); as you've discovered, isFuture() cannot be pure, as it relies on the current time, so @psalm-immutable cannot be the correct annotation for it.

[nederdirk]

Hey @BenMorel,
I just pushed and updated (and rebased on top of current master). As you can see in https://github.com/brick/date-time/pull/32/checks?check_run_id=2397952389, it's only the DateTimeParseResult that taints the immutable/mutation-free annotations. Could you elaborate a bit on why the stack-based design is necessary for that class? From reading the other code in this package, that doesn't seem to be necessary, and I think DateTimeParseResult could be refactored to be properly immutable, while retaining the interface, or by creating a new immutable interface ($dateTimeParseResult->withField($name, $value), $dataTimeParseResult->getFieldIterator($name)) and deprecating the current functions.

[BenMorel]

Hi, thanks for round 2!

I'm not sure to keep DateTimeParseResult as is TBH, I simply inherited it from the Java version. I'm not particularly happy with this mechanism and may replace it entirely at some point.

I'm wondering though: how is it a problem to call getters on a mutable class from a factory method of an immutable class?