Movatterモバイル変換


[0]ホーム

URL:


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

Sign up
Appearance settings

[String] Check if function exists before declaring it#40203

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:5.2fromNyholm:string-function-exists
Feb 16, 2021

Conversation

@Nyholm
Copy link
Member

QA
Branch?5.2
Bug fix?yes
New feature?no
Deprecations?no
Tickets
LicenseMIT
Doc PR

If you installed a command line tool likepsalm with composer and then try to run it on a project that included the String component you will get an error like:

Fatal error: Cannot redeclare Symfony\Component\String\u() (previously declared in /Workspace/symfony/src/Symfony/Component/String/Resources/functions.php:14) in /user/.composer/vendor/symfony/string/Resources/functions.php on line 14

That is because we are loading two installations of the string component.

@Nyholm
Copy link
MemberAuthor

I've updated the code to use::class, I've also fixed the same issue in Translation component

nicolas-grekas reacted with thumbs up emoji

@nicolas-grekas
Copy link
Member

Thank you@Nyholm.

@nicolas-grekasnicolas-grekas merged commit7dcf156 intosymfony:5.2Feb 16, 2021
@Nyholm
Copy link
MemberAuthor

Thank you for merging.

@NyholmNyholm deleted the string-function-exists branchFebruary 16, 2021 10:21
@fabpotfabpot mentioned this pull requestMar 4, 2021
@weirdan
Copy link

If you installed a command line tool like psalm
[...]
I've updated the code to use::class

Ironically, this broke Psalm tests:https://github.com/vimeo/psalm/runs/2037304361#step:7:42

I would suggest to replace

if (!\function_exists(u::class)) {

with

if (!\function_exists(__NAMESPACE__ .'\\u')) {

@Nyholm
Copy link
MemberAuthor

The current implementation is valid PHP code. I’m not sure what the benefits are to implement to your suggestion.

I don’t want to be disrespectful, but this is a problem psalm should fix, isn’t it?

@weirdan
Copy link

I’m not sure what the benefits are to implement to your suggestion.

Clearer code, mostly. I had this little 'huh?' moment looking at it, thinking 'What are those weirdly named classes? And why are they used infunction_exists()? Oh, those arenot classes...'

this is a problem psalm should fix, isn’t it?

Perhaps. Hardly a high-priority issue though, as in practice it will only happen when people run Psalm with badly written custom autoloader.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@derrabusderrabusderrabus approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

5 participants

@Nyholm@nicolas-grekas@weirdan@derrabus@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp