-
-
Notifications
You must be signed in to change notification settings - Fork 143
Fix class_uses polyfill #31
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
Conversation
Could you add a test please ? |
@nicolas-grekas done |
I've completely changed the function now btw, as it had to return an array and not a boolean as it did before. |
Great! Some comments:
|
831f72a
to
1224378
Compare
I've fixed the first 2 comments. The test with |
function class_uses($class, $autoload = true) { return $autoload && class_exists($class, $autoload) && false; } | ||
function class_uses($class, $autoload = true) | ||
{ | ||
if (!is_object($class) && $autoload && !class_exists($class, true)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when autoload === false, we should still return false when the class does not exist. But when $class is an interface, we shoudld return array()... Could be done with:
if (is_object($class) || class_exists($class, $autoload) || interface_exists($class, false)) {
return array();
}
return false;
Updated again |
function class_uses($class, $autoload = true) { return $autoload && class_exists($class, $autoload) && false; } | ||
function class_uses($class, $autoload = true) | ||
{ | ||
if (is_object($class) || class_exists($class, $autoload) || interface_exists($class, $autoload)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interface_exists($class, false) because the autoloader has already been triggered, if needed, by class_exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
👍 (the missing warning is a bonus to me, nobody should rely on it) |
👍 (if you wanted to add the warning too that would be nice though) |
Can we please merge this? |
Thank you @wouterj. |
This PR was merged into the 1.1-dev branch. Discussion ---------- Fix class_uses polyfill `class_uses` accepts both the class name and object instance as first argument, `class_exists` only accepts a class name. If an object instance is giving, the class is already autoloaded, so the other logic can be skipped. Commits ------- b2c4e67 Fix class_uses polyfill
This PR was merged into the 1.1-dev branch. Discussion ---------- Add CHANGELOG Fixes #32 * v1.1.1 * bug #40 [Apcu] Load APCUIterator only when APCIterator exists (nicolas-grekas) * bug #37 [Iconv] Fix wrong use in bootstrap.php (tucksaun) * bug #31 Fix class_uses polyfill (WouterJ) * v1.1.0 * feature #22 [APCu] A new polyfill for the legacy APC users (nicolas-grekas) * bug #28 [Php70] Workaround https://bugs.php.net/63206 (nicolas-grekas) * v1.0.1 * bug #14 ldap_escape does not encode leading/trailing spaces. (ChadSikorra) * bug #17 Fix #16 - gzopen() / gzopen64() - 32 bit builds of Ubuntu 14.04 (fisharebest) * v1.0.0 * Hello symfony/polyfill Commits ------- 1781007 Add CHANGELOG
class_uses
accepts both the class name and object instance as first argument,class_exists
only accepts a class name. If an object instance is giving, the class is already autoloaded, so the other logic can be skipped.