facebook / hhvm

A virtual machine for executing programs written in Hack.
https://hhvm.com
Other
18.13k stars 2.99k forks source link

Reflection API does not match PHP's Reflection API #5890

Open sebastianbergmann opened 9 years ago

sebastianbergmann commented 9 years ago

Discovered via https://github.com/sebastianbergmann/phpunit-mock-objects/issues/228 which is discussed in https://github.com/sebastianbergmann/phpunit-mock-objects/pull/229.

This is just one example of where the Reflection API differs between HHVM and PHP:

<?php
ReflectionClass::export(ReflectionParameter::class);
$ php-7 test.php > php-7.txt
$ /usr/local/src/hhvm/src/hphp/hhvm/hhvm test.php > hhvm.txt
$ diff -u php-7.txt hhvm.txt
--- php-7.txt   2015-08-09 05:47:15.317145284 +0200
+++ hhvm.txt    2015-08-09 05:47:20.830186176 +0200
@@ -1,4 +1,10 @@
-Class [ <internal:Reflection> class ReflectionParameter implements Reflector ] {
+
+Notice: Constant SUNFUNCS_RET_DOUBLE already defined
+
+Notice: Constant SUNFUNCS_RET_STRING already defined
+
+Notice: Constant SUNFUNCS_RET_TIMESTAMP already defined
+Class [ <internal:> class ReflectionParameter implements Reflector, Stringish ] {

   - Constants [0] {
   }
@@ -6,149 +12,94 @@
   - Static properties [0] {
   }

-  - Static methods [1] {
-    Method [ <internal:Reflection, prototype Reflector> static public method export ] {
+  - Static methods [2] {
+    Method [ <internal> static public method export ] {

       - Parameters [3] {
-        Parameter #0 [ <required> $function ]
-        Parameter #1 [ <required> $parameter ]
-        Parameter #2 [ <optional> $return ]
+        Parameter #0 [ <required> $func ]
+        Parameter #1 [ <required> $param ]
+        Parameter #2 [ <optional> $ret = false ]
+      }
+    }
+    Method [ <internal> static private method collectAttributes ] {
+
+      - Parameters [4] {
+        Parameter #0 [ <required> &$attrs ]
+        Parameter #1 [ <required> $class ]
+        Parameter #2 [ <required> $function_name ]
+        Parameter #3 [ <required> $index ]
       }
     }
   }

-  - Properties [1] {
+  - Properties [2] {
+    Property [ <default> public $info ]
     Property [ <default> public $name ]
   }

-  - Methods [21] {
-    Method [ <internal:Reflection> final private method __clone ] {
-
-      - Parameters [0] {
-      }
-    }
-
-    Method [ <internal:Reflection, ctor> public method __construct ] {
+  - Methods [25] {
+    Method [ <internal, ctor> public method __construct ] {

       - Parameters [2] {
-        Parameter #0 [ <required> $function ]
-        Parameter #1 [ <required> $parameter ]
+        Parameter #0 [ <required> $func ]
+        Parameter #1 [ <required> $param ]
       }
     }
-
-    Method [ <internal:Reflection, prototype Reflector> public method __toString ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal, implements Reflector> public method __toString ] {
     }
-
-    Method [ <internal:Reflection> public method getName ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> final public method __clone ] {
     }
-
-    Method [ <internal:Reflection> public method isPassedByReference ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method getName ] {
     }
-
-    Method [ <internal:Reflection> public method canBePassedByValue ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method isPassedByReference ] {
     }
-
-    Method [ <internal:Reflection> public method getDeclaringFunction ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method canBePassedByValue ] {
     }
-
-    Method [ <internal:Reflection> public method getDeclaringClass ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method getDeclaringClass ] {
     }
-
-    Method [ <internal:Reflection> public method getClass ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method getDeclaringFunction ] {
     }
-
-    Method [ <internal:Reflection> public method hasType ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method getClass ] {
     }
-
-    Method [ <internal:Reflection> public method getType ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method getTypehintText ] {
     }
-
-    Method [ <internal:Reflection> public method isArray ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method getTypeText ] {
     }
-
-    Method [ <internal:Reflection> public method isCallable ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method isArray ] {
     }
-
-    Method [ <internal:Reflection> public method allowsNull ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method allowsNull ] {
     }
-
-    Method [ <internal:Reflection> public method getPosition ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method isOptional ] {
     }
-
-    Method [ <internal:Reflection> public method isOptional ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method isVariadic ] {
     }
-
-    Method [ <internal:Reflection> public method isDefaultValueAvailable ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method isDefaultValueAvailable ] {
     }
-
-    Method [ <internal:Reflection> public method getDefaultValue ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method getDefaultValue ] {
     }
-
-    Method [ <internal:Reflection> public method isDefaultValueConstant ] {
-
-      - Parameters [0] {
-      }
+    Method [ <internal> public method getDefaultValueText ] {
     }
+    Method [ <internal> public method getDefaultValueConstantName ] {
+    }
+    Method [ <internal> public method getPosition ] {
+    }
+    Method [ <internal> public method getAttribute ] {

-    Method [ <internal:Reflection> public method getDefaultValueConstantName ] {
-
-      - Parameters [0] {
+      - Parameters [1] {
+        Parameter #0 [ <required> $name ]
       }
     }
+    Method [ <internal> public method getAttributes ] {
+    }
+    Method [ <internal> public method getAttributeRecursive ] {

-    Method [ <internal:Reflection> public method isVariadic ] {
-
-      - Parameters [0] {
+      - Parameters [1] {
+        Parameter #0 [ <required> $name ]
       }
     }
+    Method [ <internal> public method getAttributesRecursive ] {
+    }
+    Method [ <internal> public method isCallable ] {
+    }
   }
 }
-

The notices have been reported in #5888.

paulbiss commented 9 years ago

I'm not seeing the notices when I run the code snippet, I suspect they're unrelated. The rest seems rather implementation specific (you're reflecting on systemlib and many of the differences are naming related and otherwise invisible). I think at best this is low-pri, but I don't think supporting parity with systemlib parameter names is realistic. (http://3v4l.org/2JkWA)

sebastianbergmann commented 9 years ago

Not sure what you think this is about, but HHVM's implementation of ReflectionParameter is missing the hasType() and getType() methods which were introduced in PHP 7.

paulbiss commented 9 years ago

Ah, that was not clear from the diff...

sebastianbergmann commented 9 years ago

Not from the diff, no. Sorry about that. But from https://github.com/sebastianbergmann/phpunit-mock-objects/pull/229#issuecomment-111694588 linked from the first line of the issue :-)

SiebelsTim commented 9 years ago

Is this equivalent to getTypehintText() and might be just an alias?

stof commented 9 years ago

@SiebelsTim it is not equivalent, hasType should return true for any type-hinted argument, and getType should return a ReflectionType object (or null when there is no type). Note that ReflectionType is also used for return types. See http://3v4l.org/4hj3X

Orvid commented 9 years ago

After a bit of checking, hasType should be able to be implemented as quite simply:

public function hasType() {
  return $this->getTypeText() !== '';
}

As I don't see a ReflectionType class defined in HHVM currently, I'll assume that's a bit more complicated to implement.

stof commented 9 years ago

@Orvid of course it does not exist currently. It is part of the new PHP 7 API

JoelMarcey commented 8 years ago

A big crux of this looks to be implementing ReflectionType and ReflectionGenerator if I understand what was added to PHP 7. And a couple of new methods to existing types...

https://github.com/tpunt/PHP7-Reference#reflection-additions