diff --git a/src/__tests__/PhabricatorInfrastructureTestCase.php b/src/__tests__/PhabricatorInfrastructureTestCase.php --- a/src/__tests__/PhabricatorInfrastructureTestCase.php +++ b/src/__tests__/PhabricatorInfrastructureTestCase.php @@ -12,10 +12,71 @@ * This is more of an acceptance test case instead of a unit test. It verifies * that all symbols can be loaded correctly. It can catch problems like * missing methods in descendants of abstract base classes. + * + * We then perform some additional reflection-based checks. */ public function testEverythingImplemented() { - id(new PhutilSymbolLoader())->selectAndLoadSymbols(); - $this->assertTrue(true); + $symbols = id(new PhutilSymbolLoader()) + ->selectAndLoadSymbols(); + + $classes = array(); + foreach ($symbols as $symbol) { + if ($symbol['type'] == 'class') { + $classes[$symbol['name']] = new ReflectionClass($symbol['name']); + } + } + + foreach ($classes as $class_name => $class) { + $parents = array(); + $parent = $class; + while ($parent = $parent->getParentClass()) { + $parents[] = $parent; + } + + $interfaces = $class->getInterfaces(); + + foreach ($class->getMethods() as $method) { + $visibility = $this->getVisibility($method); + $method_name = $method->getName(); + + foreach (array_merge($parents, $interfaces) as $extends) { + if ($extends->hasMethod($method_name)) { + $xmethod = $extends->getMethod($method_name); + $xvisibility = $this->getVisibility($xmethod); + if ($visibility != $xvisibility) { + $this->assertEqual( + $xvisibility, + $visibility, + pht( + 'Class "%s" implements method "%s" with the wrong '. + 'visibility. The method has visibility "%s", but it is '. + 'defined in parent "%s" with visibility "%s". In '. + 'Phabricator, a method which overrides another must '. + 'always have the same visibility.', + $class_name, + $method_name, + $visibility, + $extends->getName(), + $xvisibility)); + } + + // We found a declaration somewhere, so stop looking. + break; + } + } + } + } + + } + + private function getVisibility(ReflectionMethod $method) { + if ($method->isPrivate()) { + return 'private'; + } else if ($method->isProtected()) { + return 'protected'; + } else { + return 'public'; + } } /**