From bb585ff9dcfa2c8753ea44ad90c2c25b14bc112e Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Fri, 26 Feb 2016 18:51:40 +0000 Subject: [PATCH] findHooks.php: Use recursive directory iterator The following dirs were not previously included: * includes/compat/ * includes/compat/normal/ * includes/composer/ * includes/config/ * includes/db/loadbalancer/ * includes/debug/logger/ * includes/filebackend/filejournal/ * includes/filebackend/lockmanager/ * includes/jobqueue/aggregator/ * includes/jobqueue/jobs/ * includes/jobqueue/utils/ * includes/libs/ * includes/objectcache/ * includes/pager/ * includes/poolcounter/ * includes/profiler/output/ * includes/profiler/ * includes/registration/ * includes/templates/ * includes/tidy/ * includes/title/ * includes/utils/iterators/ * includes/widget/ * maintenance/ subdirectories Change-Id: I6c01b8768f9f70152e320d254f8025a0ffde8c62 --- maintenance/findHooks.php | 171 +++++++++++++++----------------------- 1 file changed, 65 insertions(+), 106 deletions(-) diff --git a/maintenance/findHooks.php b/maintenance/findHooks.php index 60bdf29c66..d44581cd34 100644 --- a/maintenance/findHooks.php +++ b/maintenance/findHooks.php @@ -42,10 +42,13 @@ require_once __DIR__ . '/Maintenance.php'; * @ingroup Maintenance */ class FindHooks extends Maintenance { + const FIND_NON_RECURSIVE = 0; + const FIND_RECURSIVE = 1; + /* * Hooks that are ignored */ - protected static $ignore = [ 'testRunLegacyHooks' ]; + protected static $ignore = [ 'testRunLegacyHooks', 'Test' ]; public function __construct() { parent::__construct(); @@ -62,73 +65,36 @@ class FindHooks extends Maintenance { $documentedHooks = $this->getHooksFromDoc( $IP . '/docs/hooks.txt' ); $potentialHooks = []; - $bad = []; + $badHooks = []; - // TODO: Don't hardcode the list of directories - $pathinc = [ - $IP . '/', - $IP . '/includes/', - $IP . '/includes/actions/', - $IP . '/includes/api/', - $IP . '/includes/cache/', - $IP . '/includes/changes/', - $IP . '/includes/changetags/', - $IP . '/includes/clientpool/', - $IP . '/includes/content/', - $IP . '/includes/context/', - $IP . '/includes/dao/', - $IP . '/includes/db/', - $IP . '/includes/debug/', - $IP . '/includes/deferred/', - $IP . '/includes/diff/', - $IP . '/includes/exception/', - $IP . '/includes/export/', - $IP . '/includes/externalstore/', - $IP . '/includes/filebackend/', - $IP . '/includes/filerepo/', - $IP . '/includes/filerepo/file/', - $IP . '/includes/gallery/', - $IP . '/includes/htmlform/', - $IP . '/includes/import/', - $IP . '/includes/installer/', - $IP . '/includes/interwiki/', - $IP . '/includes/jobqueue/', - $IP . '/includes/json/', - $IP . '/includes/logging/', - $IP . '/includes/mail/', - $IP . '/includes/media/', - $IP . '/includes/page/', - $IP . '/includes/parser/', - $IP . '/includes/password/', - $IP . '/includes/rcfeed/', - $IP . '/includes/resourceloader/', - $IP . '/includes/revisiondelete/', - $IP . '/includes/search/', - $IP . '/includes/session/', - $IP . '/includes/site/', - $IP . '/includes/skins/', - $IP . '/includes/specialpage/', - $IP . '/includes/specials/', - $IP . '/includes/upload/', - $IP . '/includes/user/', - $IP . '/includes/utils/', - $IP . '/languages/', - $IP . '/maintenance/', - $IP . '/maintenance/language/', - $IP . '/tests/', - $IP . '/tests/parser/', - $IP . '/tests/phpunit/suites/', + $recurseDirs = [ + "$IP/includes/", + "$IP/mw-config/", + "$IP/languages/", + "$IP/maintenance/", + // Omit $IP/tests/phpunit as it contains hook tests that shouldn't be documented + "$IP/tests/parser", + "$IP/tests/phpunit/suites", + ]; + $nonRecurseDirs = [ + "$IP/", ]; - foreach ( $pathinc as $dir ) { - $potentialHooks = array_merge( $potentialHooks, $this->getHooksFromPath( $dir ) ); - $bad = array_merge( $bad, $this->getBadHooksFromPath( $dir ) ); + foreach ( $recurseDirs as $dir ) { + $ret = $this->getHooksFromDir( $dir, self::FIND_RECURSIVE ); + $potentialHooks = array_merge( $potentialHooks, $ret['good'] ); + $badHooks = array_merge( $badHooks, $ret['bad'] ); + } + foreach ( $nonRecurseDirs as $dir ) { + $ret = $this->getHooksFromDir( $dir ); + $potentialHooks = array_merge( $potentialHooks, $ret['good'] ); + $badHooks = array_merge( $badHooks, $ret['bad'] ); } $documented = array_keys( $documentedHooks ); $potential = array_keys( $potentialHooks ); $potential = array_unique( $potential ); - $bad = array_diff( array_unique( $bad ), self::$ignore ); + $badHooks = array_diff( array_unique( $badHooks ), self::$ignore ); $todo = array_diff( $potential, $documented, self::$ignore ); $deprecated = array_diff( $documented, $potential, self::$ignore ); @@ -158,15 +124,15 @@ class FindHooks extends Maintenance { } } - // let's show the results: + // Print the results $this->printArray( 'Undocumented', $todo ); $this->printArray( 'Documented and not found', $deprecated ); - $this->printArray( 'Unclear hook calls', $bad ); + $this->printArray( 'Unclear hook calls', $badHooks ); $this->printArray( 'Different parameter count', $badParameterCount ); $this->printArray( 'Different parameter reference', $badParameterReference ); - if ( count( $todo ) == 0 && count( $deprecated ) == 0 && count( $bad ) == 0 - && count( $badParameterCount ) == 0 && count( $badParameterReference ) == 0 + if ( !$todo && !$deprecated && !$badHooks + && !$badParameterCount && !$badParameterReference ) { $this->output( "Looks good!\n" ); } else { @@ -264,11 +230,11 @@ class FindHooks extends Maintenance { /** * Get hooks from a PHP file - * @param string $file Full filename to the PHP file. + * @param string $filePath Full file path to the PHP file. * @return array Array: key => hook name; value => array of arguments or string 'unknown' */ - private function getHooksFromFile( $file ) { - $content = file_get_contents( $file ); + private function getHooksFromFile( $filePath ) { + $content = file_get_contents( $filePath ); $m = []; preg_match_all( // All functions which runs hooks @@ -309,63 +275,56 @@ class FindHooks extends Maintenance { return $hooks; } - /** - * Get hooks from the source code. - * @param string $path Directory where the include files can be found - * @return array Array: key => hook name; value => array of arguments or string 'unknown' - */ - private function getHooksFromPath( $path ) { - $hooks = []; - $dh = opendir( $path ); - if ( $dh ) { - while ( ( $file = readdir( $dh ) ) !== false ) { - if ( filetype( $path . $file ) == 'file' ) { - $hooks = array_merge( $hooks, $this->getHooksFromFile( $path . $file ) ); - } - } - closedir( $dh ); - } - - return $hooks; - } - /** * Get bad hooks (where the hook name could not be determined) from a PHP file - * @param string $file Full filename to the PHP file. + * @param string $filePath Full filename to the PHP file. * @return array Array of bad wfRunHooks() lines */ - private function getBadHooksFromFile( $file ) { - $content = file_get_contents( $file ); + private function getBadHooksFromFile( $filePath ) { + $content = file_get_contents( $filePath ); $m = []; - # We want to skip the "function wfRunHooks()" one. :) + // We want to skip the "function wfRunHooks()" one. :) preg_match_all( '/(? hook name; value => array of arguments or string 'unknown' */ - private function getBadHooksFromPath( $path ) { - $hooks = []; - $dh = opendir( $path ); - if ( $dh ) { - while ( ( $file = readdir( $dh ) ) !== false ) { - # We don't want to read this file as it contains bad calls to wfRunHooks() - if ( filetype( $path . $file ) == 'file' && !$path . $file == __FILE__ ) { - $hooks = array_merge( $hooks, $this->getBadHooksFromFile( $path . $file ) ); - } + private function getHooksFromDir( $dir, $recurse = 0 ) { + $good = []; + $bad = []; + + if ( $recurse === self::FIND_RECURSIVE ) { + $iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator( $dir ), + RecursiveIteratorIterator::SELF_FIRST | RecursiveDirectoryIterator::SKIP_DOTS + ); + } else { + $iterator = new DirectoryIterator( $dir ); + } + + foreach ( $iterator as $info ) { + // Ignore directories, ignore json (installer and api i18n), + // ignore extension-less files like HISTORY + if ( $info->isFile() && $info->getExtension() !== 'json' && $info->getExtension() + // Skip this file as it contains text that looks like a bad wfRunHooks() call + && $info->getRealPath() !== __FILE__ + ) { + $good = array_merge( $good, $this->getHooksFromFile( $info->getRealPath() ) ); + $bad = array_merge( $bad, $this->getBadHooksFromFile( $info->getRealPath() ) ); } - closedir( $dh ); } - return $hooks; + return [ 'good' => $good, 'bad' => $bad ]; } /** -- 2.20.1