diff --git a/src/__tests__/PhutilLibraryTestCase.php b/src/__tests__/PhutilLibraryTestCase.php --- a/src/__tests__/PhutilLibraryTestCase.php +++ b/src/__tests__/PhutilLibraryTestCase.php @@ -40,6 +40,67 @@ } /** + * This is more of an acceptance test case instead of a unit test. It verifies + * that methods in subclasses have the same visibility as the method in the + * parent class. + */ + public function testMethodVisibility() { + $symbols = id(new PhutilSymbolLoader()) + ->selectSymbolsWithoutLoading(); + + $classes = array(); + foreach ($symbols as $symbol) { + if ($symbol['type'] == 'class') { + $classes[$symbol['name']] = new ReflectionClass($symbol['name']); + } + } + + $failures = array(); + + 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 ($xvisibility != $visibility) { + $failures[] = 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; + } + } + } + } + + $this->assertTrue( + empty($failures), + "\n\n".implode("\n\n", $failures)); + } + + /** * Get the root directory for the library currently being tested. */ protected function getLibraryRoot() { @@ -47,4 +108,14 @@ return phutil_get_library_root_for_path($caller); } + private function getVisibility(ReflectionMethod $method) { + if ($method->isPrivate()) { + return 'private'; + } else if ($method->isProtected()) { + return 'protected'; + } else { + return 'public'; + } + } + }