Skip to content
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

fix/static method getCanonicalID should not be called dynamically #4544

Merged
merged 1 commit into from
Jan 29, 2024
Merged

Conversation

apphp
Copy link
Contributor

@apphp apphp commented Jan 29, 2024

Q A
Is bugfix? ✔️/❌
New feature? ✔️/❌
Breaks BC? ✔️/❌
Tests pass? ✔️/❌
Fixed issues comma-separated list of tickets # fixed by the PR, if any

Copy link

what-the-diff bot commented Jan 29, 2024

PR Summary

  • Modification of function calls within CLocale.php
    The manner in which specific functions (getLanguageID, getScriptID, getTerritoryID, and getLocaleDisplayName) within the CLocale.php are calling the getCanonicalID method has been altered. Instead of using $this->, they are now using self::. This change makes the code's method calls more consistent and can potentially improve the overall performance of these functions.

@marcovtwout
Copy link
Member

Thanks!

@marcovtwout marcovtwout merged commit 51615ee into yiisoft:master Jan 29, 2024
12 checks passed
@rob006
Copy link
Contributor

rob006 commented Jan 29, 2024

This change breaks BC: previously if you extend this class and override getCanonicalID(), new implementation was used everywhere. After this change new implementation will be ignored and always CLocale::getCanonicalID() will be used.

@marcovtwout
Copy link
Member

@rob006 Thank you for checking in detail, but that is not exactly the case. The method itself was not updated and was already static, and there are more function calls within the framework that were already calling it correctly using ::getCanonicalID()

@rob006
Copy link
Contributor

rob006 commented Jan 30, 2024

@marcovtwout

Before change: https://3v4l.org/ZAIWn
After change: https://3v4l.org/bM5Ka

I'm not sure if overwriting getCanonicalID() is even supported, but $this-> could be used here to emulate static:: which was not available in older versions of PHP.

@marcovtwout
Copy link
Member

The old behavior (to my surprise) doesn't cause any PHP warning. Looking at the history, it doesn't look like $this was used on purpose, so I would say this change is fairly safe.

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