Skip to content
/ cache Public

Add return types#24

Merged
Crell merged 1 commit intophp-fig:masterfrom
Crell:v3
Feb 3, 2021
Merged

Add return types#24
Crell merged 1 commit intophp-fig:masterfrom
Crell:v3

Conversation

@Crell
Copy link
Collaborator

@Crell Crell commented Dec 14, 2020

Follow on to #23

* The invoked object.
*/
public function set($value);
public function set(mixed $value): static;
Copy link
Member

Choose a reason for hiding this comment

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

Does that mean that immutability is required?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

CacheItem explicitly cannot be an immutable object. It's a builder object. set() must return "the invoked object", so it's going to be of the same type (static) no matter what. If it's not, it's an error.

* Sets the expiration time for this cache item.
*
* @param \DateTimeInterface|null $expiration
* @param ?\DateTimeInterface $expiration
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Doesn't that make it inconsistent with the native types, then?

(PHPDoc's own spec is well over a decade old at this point, no? That's the reason PSR-5 and friends, er, tried to exist...)

Copy link
Member

Choose a reason for hiding this comment

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

No. It's not inconsistent with PHP 8 union types and it's current PHPDocumentor 3 syntax that has wide support among IDEs and static code analysis tools.

@Crell Crell merged commit aa5030c into php-fig:master Feb 3, 2021
@Crell Crell deleted the v3 branch February 3, 2021 23:27
@nicolas-grekas
Copy link
Contributor

This just broke projects that tests with the dev-version.
Please update the branch-alias asap.

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.

6 participants