Skip to content

[Translation] Fix docblock and ensure that preg_match isn't called on null #33247

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
wants to merge 1 commit into from

Conversation

derrabus
Copy link
Member

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR N/A

#33154 (comment)

@@ -74,7 +74,7 @@ class Translator implements TranslatorInterface, TranslatorBagInterface
private $configCacheFactory;

/**
* @param string $locale The locale
* @param string|null $locale The locale
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look right to me. If we were to make this change, we would also have to change the return type of getLocale() which would break the contract of the TranslatorInterface.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the interface is lying and needs to be fixed. That is what I wrote in #33154 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

I don't see a use case for a null locale. My suggestion is to deprecate support for this instead (see #33272).

Copy link
Member

Choose a reason for hiding this comment

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

Now that support for passing null is to be removed in Symfony 5.0 I would revert the docblock changes and only keep the check for null below.

@nicolas-grekas
Copy link
Member

Closing, no need to do anything on 3.4, this has been dealt with on 4.4. Thanks.

@derrabus derrabus deleted the bugfix/preg-match-null branch August 21, 2019 15:03
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.

5 participants