Skip to content

Fix: Str::lower() expects string, Stringable given #148

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Aluisio-Pires
Copy link

At the start of every project, phpstan complains that $this->string('email') is not a string but a Illuminate\Support\Stringable.
This PR fixes that issue.

…this->string('email')->value(). Illuminate\Support\Stringable -> string
@@ -80,6 +80,6 @@ public function ensureIsNotRateLimited(): void
*/
public function throttleKey(): string
{
return Str::transliterate(Str::lower($this->string('email')).'|'.$this->ip());
return Str::transliterate(Str::lower($this->string('email')->value()).'|'.$this->ip());
Copy link
Contributor

Choose a reason for hiding this comment

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

You can directly call $this->input('email')

Copy link
Author

Choose a reason for hiding this comment

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

$this->input('email') returns a mixed type. To ensure string type we need to use $this->string('email')->value()

Copy link
Author

Choose a reason for hiding this comment

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

image

Copy link
Contributor

@firebed firebed Jun 10, 2025

Choose a reason for hiding this comment

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

Oh I see, you're right. However, I am still convinced we should not create a Stringable object just to get the string value of the email. Maybe casting to string is a better way. What do you think? Is phpstan still complaining?

(string)$this->input('email')

Copy link
Author

Choose a reason for hiding this comment

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

Since the type of $this->input('email') is mixed, we can’t cast it directly to a string.
image

@firebed
Copy link
Contributor

firebed commented Jun 10, 2025

Stringable under the hood does casting mixed type into string and to me $this->string('email')->value() feels like a hack into tricking phpstan because it accomplises the same result with extra steps just to make phpstan happy 😕

@Plytas
Copy link

Plytas commented Jun 10, 2025

I think a proper fix here would be to PR a change to the framework Str methods to accept an instance of Stringable.

@Aluisio-Pires
Copy link
Author

Stringable under the hood does casting mixed type into string and to me $this->string('email')->value() feels like a hack into tricking phpstan because it accomplises the same result with extra steps just to make phpstan happy 😕

You’re partly correct: the returned Stringable does convert the mixed value to string under the hood, but it remains a Stringable instance, not a native string. To extract a pure string, you must call value(), which returns the object’s underlying string.
You can use this video of Nuno Maduro as reference.

@Aluisio-Pires
Copy link
Author

I think a proper fix here would be to PR a change to the framework Str methods to accept an instance of Stringable.

This doesn’t only occur in Str methods but in any methods that work with the string type. Using ->value() is a best practice to convert to PHP’s native string type in these cases.
Here are some examples of PHP functions that take a string as a parameter: strlen(), substr(), strpos(), trim(), str_replace(), explode(), preg_match(), json_encode()

@Plytas
Copy link

Plytas commented Jun 10, 2025

Aware of that. But as a framework it should provide convenience wherever possible.

@firebed
Copy link
Contributor

firebed commented Jun 10, 2025

You can tell that even the author's original intention wasn't to create a stringable object, because he could chain the ->lower() method instead of wrapping the entire thing into Str::lower. In any case; your pr makes sense, but the underlying issue remains. Honestly I would just ignore this error in phpstan.

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