Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[Contracts/Service] add LazyString with new fromStringable(), isStringable() and resolve() methods #34190

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

Closed

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Oct 30, 2019

Q A
Branch? 4.4
Bug fix? no
New feature? yes
Deprecations? -
Tickets -
License MIT
Doc PR -

Its purpose is so generic that I'd like to move LazyString from DI to the Service contracts.

This PR also adds three new methods to the class, for very common boilerplate we have to write again and again otherwise:

  • LazyString::isStringable($value) -> ensure a value can be safely cast to string, considering __toString() too
  • LazyString::fromStringable($value) -> turns any object with __toString() into a LazyString
  • LazyString::resolve($value) -> casts a value to a string or throw a TypeError if not possible (strval() only throws a notice, that's why we need this one.)

@nicolas-grekas nicolas-grekas added this to the 4.4 milestone Oct 30, 2019
@nicolas-grekas nicolas-grekas force-pushed the contracts-lazy-string branch 3 times, most recently from ffa91fe to cd1dcf3 Compare October 30, 2019 14:49
src/Symfony/Contracts/Service/LazyString.php Show resolved Hide resolved
Copy link
Member

@stof stof left a comment

Choose a reason for hiding this comment

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

Tests should be added for the new utilities.

Btw, does isStringable deal properly with objects having a non-public __toString method ?

/**
* Casts scalars and stringable objects to strings.
*
* @param object|string|int|float|bool $value
Copy link
Member

Choose a reason for hiding this comment

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

should null be supported too ?

Copy link
Member 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 so - if one wants to deal with null, one can always do resolve($foo ?? 'bar')

@nicolas-grekas
Copy link
Member Author

Tests should be added for the new utilities.

tests added

does isStringable deal properly with objects having a non-public __toString method ?

now yes! - another boilerplate that is missing from similar checks in the code btw.

@nicolas-grekas
Copy link
Member Author

Status: needs review

nicolas-grekas added a commit that referenced this pull request Nov 8, 2019
…he String component (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[DI] Remove LazyString from 4.4, before adding back to the String component

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

In #34190 I'm proposing to move LazyString to the Service contracts, but String might be a better fit actually. Let's remove the class from 4.4 where it's not really needed, and add it back on 5.0 in the String component.

Commits
-------

b1a3ee7 [DI] Remove LazyString from 4.4, before adding back to the String component
@nicolas-grekas nicolas-grekas deleted the contracts-lazy-string branch November 8, 2019 16:27
fabpot added a commit that referenced this pull request Feb 3, 2020
…e objects (nicolas-grekas)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[String] add LazyString to provide memoizing stringable objects

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Replaces #34190

The proposed `LazyString` class is a value object that can be used in type declarations of our libraries/apps.

Right now, when a method accepts or returns "strings|stringable-objects", the most accurate type declaration one can use is `string|object` (either in docblocks or in union types in PHP 8). The goal of `LazyString` is to allow one to use `string|LazyString` instead and gain type-accuracy, thus type-safety while doing so.

Another defining property of the proposed class is also that it memoizes the computed string value so that the computation happens only once.

Two factories are provided to create a `LazyString` instance:
- `LazyString::fromStringable($value): self` -> turns any object with `__toString()` into a `LazyString`
- `LazyString::fromCallable($callback, ...$arguments): self` -> delegates the computation of the string value to a callback (optionally calling it with arguments).

Two generic helpers are also provided to help deal with stringables:
- `LazyString::isStringable($value): bool` -> checks whether a value can be safely cast to string, considering `__toString()` too. This replaces the boilerplate we all have to write currently (`is_string($value) || is_scalar($value) || is_callable([$value, '__toString'])`)
- `LazyString::resolve($value): string` -> casts a stringable value into a string. This is similar to the casting `(string)` operator or to `strval()`, but it throws a `TypeError` instead of a PHP notice when a non stringable is passed. This helps e.g. with code that enabled strict types and want to maintain compatibility with stringable objects.

An additional feature of `LazyString` instances is that they allow exceptions thrown from the wrapped `__toString()` methods or callbacks to be propagated. This requires having the `ErrorHandler` class from the `Debug` or `ErrorHandler` components registered as a PHP error handler (already the case for any Symfony apps by default). As a reminder, throwing from `__toString()` is not possible natively before PHP 7.4.

Commits
-------

4bb19c6 [String] add LazyString to provide generic stringable objects
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.