Skip to content

Conversation

@yoeunes
Copy link
Contributor

@yoeunes yoeunes commented Nov 4, 2025

Q A
Branch? 7.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Issues -
License MIT

This PR fixes a return type error in WrappedTemplatedEmail::getReturnPath().

The method is type-hinted to return a string, but it was directly returning the result of $this->message->getReturnPath(), which is ?Address.

@carsonbot carsonbot added this to the 7.3 milestone Nov 4, 2025
@yoeunes yoeunes force-pushed the twig-wrapped-email-return-type branch 3 times, most recently from 6f2b3a3 to 8eed792 Compare November 4, 2025 09:42
@stof
Copy link
Member

stof commented Nov 4, 2025

This fix is still incomplete, as you now return string|null

@yoeunes yoeunes force-pushed the twig-wrapped-email-return-type branch from 8eed792 to d5d59e0 Compare November 4, 2025 09:50
@yoeunes
Copy link
Contributor Author

yoeunes commented Nov 4, 2025

Hi @stof thank you, fixed now

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Nov 4, 2025

Let's add a test case? I suppose the current code (before this change) throws an exception?

@yoeunes yoeunes force-pushed the twig-wrapped-email-return-type branch from d5d59e0 to f3a763e Compare November 4, 2025 15:36
@yoeunes yoeunes force-pushed the twig-wrapped-email-return-type branch from f3a763e to 39c70bb Compare November 4, 2025 15:37
@yoeunes
Copy link
Contributor Author

yoeunes commented Nov 4, 2025

Hi @nicolas-grekas, thanks for the review.

I didn't test this at runtime. I found it because PhpStorm flagged it as a return type violation.

You're right, it would likely cause a fatal Error (Object of class Symfony\Component\Mime\Address could not be converted to string) because the method is type-hinted as string

@nicolas-grekas
Copy link
Member

Thank you @yoeunes.

@nicolas-grekas nicolas-grekas merged commit 3f5c522 into symfony:7.3 Nov 5, 2025
11 checks passed
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