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

ReflectionParameter::isDefaultValueConstant() is missing #3812

Closed
BenMorel opened this issue Sep 22, 2014 · 21 comments
Closed

ReflectionParameter::isDefaultValueConstant() is missing #3812

BenMorel opened this issue Sep 22, 2014 · 21 comments

Comments

@BenMorel
Copy link

As mentioned in #1600, this method is missing. (#1600 was addressing this issue, but was closed because too broad.)

@ghost
Copy link

ghost commented Oct 11, 2014

I believe I'm able to fix this one.
Already have a possibly working solution here.
I'll just write a few more tests and send a pull request later.

@Majkl578
Copy link
Contributor

@lucastadeu: This is what I have locally for this. (Postponed for some reason, which I don't remember.) Sadly there is no native way to get the info, so detection is extremely hackish.

diff --git a/hphp/runtime/ext/reflection/ext_reflection-classes.php b/hphp/runtime/ext/reflection/ext_reflection-classes.php
index 9d4a680..617ae1a 100644
--- a/hphp/runtime/ext/reflection/ext_reflection-classes.php
+++ b/hphp/runtime/ext/reflection/ext_reflection-classes.php
@@ -413,11 +413,18 @@ class ReflectionParameter implements Reflector {
    * @return     mixed   Returns string on success or NULL on failure.
    */
   public function getDefaultValueConstantName() {
-    if (array_key_exists('defaultText', $this->info)) {
-      return $this->info['defaultText'];
+    if (!array_key_exists('defaultText', $this->info)) {
+      return null;
     }

-    return '';
+    $val = $this->info['defaultText'];
+    if ($val[0] === '"' || $val[0] === "'" || $val[0] === '['
+        || $val === 'NULL' || $val === 'true' || $val === 'false'
+        || substr($val, 0, 5) === 'array' || strpbrk($val[0], '-0123456789') !== FALSE) {
+      return null;
+    }
+
+    return ltrim($val, '\\');
   }

   // This doc comment block generated by idl/sysdoc.php
@@ -495,6 +502,20 @@ class ReflectionParameter implements Reflector {
         $index);
     }
   }
+
+  /**
+   * ( excerpt from
+   * http://php.net/manual/en/reflectionparameter.isdefaultvalueconstant.php )
+   *
+   * Warning: This function is currently not documented; only its argument
+   * list is available.
+   *
+   * @return     mixed   Returns TRUE if the default value is constant, FALSE
+   *                     if it is not or NULL on failure.
+   */
+  public function isDefaultValueConstant() {
+    return $this->getDefaultValueConstantName() !== null;
+  }
 }

 ///////////////////////////////////////////////////////////////////////////////

@ghost
Copy link

ghost commented Oct 11, 2014

@Majkl578 mine was equally hackish :-P

Nevermind it then.

@Majkl578
Copy link
Contributor

@lucastadeu: Feel free to send PR if you want, if not, I'll try. :)

@ghost
Copy link

ghost commented Oct 12, 2014

@Majkl578: Okay then! I'll send a PR soon enough :)

@paulbiss
Copy link
Contributor

@lucastadeu Are you planning to submit that PR?

@BenMorel
Copy link
Author

Any update on this one?

BenMorel added a commit to brick/reflection that referenced this issue Jan 21, 2015
bobthecow added a commit to bobthecow/psysh that referenced this issue Feb 15, 2015
@BenMorel
Copy link
Author

Up!

@Orvid
Copy link
Contributor

Orvid commented Nov 19, 2015

This sounds like it was implemented.

@paulbiss
Copy link
Contributor

Doesn't appear to have been merged though

@JoelMarcey
Copy link
Contributor

Implemented now.

BenMorel added a commit to brick/reflection that referenced this issue Jan 25, 2016
@BenMorel
Copy link
Author

@JoelMarcey Thank you, what's the fix version for this bug? The latest release, 3.11.1 is still affected.

@SiebelsTim
Copy link
Contributor

@BenMorel This was just three days ago
80d73f5

You can expect it to be in 3.11.2

@fredemmott
Copy link
Contributor

Is there a PR for the 3.11 branch? At the moment it's not in there

@SiebelsTim
Copy link
Contributor

@fredemmott Is 3.11 going to be 3.11.2, without a new cut?
Then it's not going to be in 3.11.2 :D

@fredemmott
Copy link
Contributor

We re-cut for y releases, z releases are cherry-picks on top of the HHVM-x.y branch

@JoelMarcey
Copy link
Contributor

I didn't think this was 100% necessary for a z-release cherry-pick. I was thinking this would go into 3.12. Has 3.12 been cut?

@fredemmott
Copy link
Contributor

Yep, cut on the 18th: https://docs.hhvm.com/hhvm/installation/release-schedule

@JoelMarcey
Copy link
Contributor

So, I would be in favor of either cherry-picking into 3.12 or waiting until 3.13. I think we have enough to deal with in 3.11.x at this point.

@bobthecow
Copy link

So if I'm matching against a version number for this, what version should I be using?

@JoelMarcey
Copy link
Contributor

Right now there is nor version number to match. You need to download the nightlies (http://dl.hhvm.com/) or build from master.

BenMorel added a commit to brick/reflection that referenced this issue Mar 29, 2016
The fix for ReflectionParameter::isDefaultValueConstant() is still not present in any version.
@see facebook/hhvm#3812
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants