From 01eb216c5f3640ae020df362ff62dd91ae61be3c Mon Sep 17 00:00:00 2001 From: umherirrender Date: Sun, 20 Sep 2015 11:40:53 +0200 Subject: [PATCH] Allow findHooks.php to compare parameter count of hooks All wrong hook documentation gets fixed with own patch sets: https://gerrit.wikimedia.org/r/#/q/project:mediawiki/core+branch:master+topic:hooksdoc,n,z and Iae0285b1a39cf851aaaa735cb22e95c839813997 Change-Id: I5991c4f0ff3ed1fbad18a38169a62d406bf4d105 --- docs/hooks.txt | 7 +-- maintenance/findHooks.php | 102 ++++++++++++++++++++++++++++++++------ 2 files changed, 90 insertions(+), 19 deletions(-) diff --git a/docs/hooks.txt b/docs/hooks.txt index 41cf4884e7..ca5da148ff 100644 --- a/docs/hooks.txt +++ b/docs/hooks.txt @@ -1961,14 +1961,14 @@ $rcid: ID of the revision to be marked patrolled $user: the user (object) marking the revision as patrolled $wcOnlySysopsCanPatrol: config setting indicating whether the user needs to be a sysop in order to mark an edit patrolled. -$auto true if the edit is being marked as patrolled automatically +$auto: true if the edit is being marked as patrolled automatically 'MarkPatrolledComplete': After an edit is marked patrolled. $rcid: ID of the revision marked as patrolled $user: user (object) who marked the edit patrolled $wcOnlySysopsCanPatrol: config setting indicating whether the user must be a sysop to patrol the edit. -$auto true if the edit is being marked as patrolled automatically +$auto: true if the edit is being marked as patrolled automatically 'MediaWikiPerformAction': Override MediaWiki::performAction(). Use this to do something completely different, after the basic globals have been set up, but @@ -2598,7 +2598,8 @@ $terms: Search terms, for highlighting &$text: Text to use for the link $result: The search result $terms: The search terms entered -$page: The SpecialSearch object. +$page: The SpecialSearch object +&$query: Query string to be appended to the link 'SidebarBeforeOutput': Allows to edit sidebar just before it is output by skins. Warning: This hook is run on each display. You should consider to use diff --git a/maintenance/findHooks.php b/maintenance/findHooks.php index 09254065ef..4afeb0e872 100644 --- a/maintenance/findHooks.php +++ b/maintenance/findHooks.php @@ -60,8 +60,8 @@ class FindHooks extends Maintenance { public function execute() { global $IP; - $documented = $this->getHooksFromDoc( $IP . '/docs/hooks.txt' ); - $potential = array(); + $documentedHooks = $this->getHooksFromDoc( $IP . '/docs/hooks.txt' ); + $potentialHooks = array(); $bad = array(); // TODO: Don't hardcode the list of directories @@ -117,21 +117,43 @@ class FindHooks extends Maintenance { ); foreach ( $pathinc as $dir ) { - $potential = array_merge( $potential, $this->getHooksFromPath( $dir ) ); + $potentialHooks = array_merge( $potentialHooks, $this->getHooksFromPath( $dir ) ); $bad = array_merge( $bad, $this->getBadHooksFromPath( $dir ) ); } + $documented = array_keys( $documentedHooks ); + $potential = array_keys( $potentialHooks ); $potential = array_unique( $potential ); $bad = array_diff( array_unique( $bad ), self::$ignore ); $todo = array_diff( $potential, $documented, self::$ignore ); $deprecated = array_diff( $documented, $potential, self::$ignore ); + // Check parameter count + $badParameter = array(); + foreach ( $potentialHooks as $hook => $args ) { + if ( !isset( $documentedHooks[$hook] ) ) { + // Not documented, but that will also be in $todo + continue; + } + $argsDoc = $documentedHooks[$hook]; + if ( $args === 'unknown' || $argsDoc === 'unknown' ) { + // Could not get parameter information + continue; + } + if ( count( $argsDoc ) !== count( $args ) ) { + $badParameter[] = $hook . ': Doc: ' . count( $argsDoc ) . ' vs. Code: ' . count( $args ); + } + } + // let's show the results: $this->printArray( 'Undocumented', $todo ); $this->printArray( 'Documented and not found', $deprecated ); $this->printArray( 'Unclear hook calls', $bad ); + $this->printArray( 'Different parameter count', $badParameter ); - if ( count( $todo ) == 0 && count( $deprecated ) == 0 && count( $bad ) == 0 ) { + if ( count( $todo ) == 0 && count( $deprecated ) == 0 && count( $bad ) == 0 + && count( $badParameter ) == 0 + ) { $this->output( "Looks good!\n" ); } else { $this->error( 'The script finished with errors.', 1 ); @@ -141,7 +163,7 @@ class FindHooks extends Maintenance { /** * Get the hook documentation, either locally or from MediaWiki.org * @param string $doc - * @return array Array of documented hooks + * @return array Array: key => hook name; value => array of arguments or string 'unknown' */ private function getHooksFromDoc( $doc ) { if ( $this->hasOption( 'online' ) ) { @@ -154,24 +176,41 @@ class FindHooks extends Maintenance { /** * Get hooks from a local file (for example docs/hooks.txt) * @param string $doc Filename to look in - * @return array Array of documented hooks + * @return array Array: key => hook name; value => array of arguments or string 'unknown' */ private function getHooksFromLocalDoc( $doc ) { $m = array(); $content = file_get_contents( $doc ); - preg_match_all( "/\n'(.*?)':/", $content, $m ); + preg_match_all( + "/\n'(.*?)':.*((?:\n.+)*)/", + $content, + $m, + PREG_SET_ORDER + ); - return array_unique( $m[1] ); + // Extract the documented parameter + $hooks = array(); + foreach ( $m as $match ) { + $args = array(); + if ( isset( $match[2] ) ) { + $n = array(); + if ( preg_match_all( "/\n(&?\\$\w+):.+/", $match[2], $n ) ) { + $args = $n[1]; + } + } + $hooks[$match[1]] = $args; + } + return $hooks; } /** * Get hooks from www.mediawiki.org using the API - * @return array Array of documented hooks + * @return array Array: key => hook name; value => string 'unknown' */ private function getHooksFromOnlineDoc() { $allhooks = $this->getHooksFromOnlineDocCategory( 'MediaWiki_hooks' ); $removed = $this->getHooksFromOnlineDocCategory( 'Removed_hooks' ); - return array_diff( $allhooks, $removed ); + return array_diff_key( $allhooks, $removed ); } /** @@ -198,7 +237,8 @@ class FindHooks extends Maintenance { $data = FormatJson::decode( $json, true ); foreach ( $data['query']['categorymembers'] as $page ) { if ( preg_match( '/Manual\:Hooks\/([a-zA-Z0-9- :]+)/', $page['title'], $m ) ) { - $retval[] = str_replace( ' ', '_', $m[1] ); + // parameters are unknown, because that needs parsing of wikitext + $retval[str_replace( ' ', '_', $m[1] )] = 'unknown'; } } if ( !isset( $data['continue'] ) ) { @@ -211,24 +251,54 @@ class FindHooks extends Maintenance { /** * Get hooks from a PHP file * @param string $file Full filename to the PHP file. - * @return array Array of hooks found + * @return array Array: key => hook name; value => array of arguments or string 'unknown' */ private function getHooksFromFile( $file ) { $content = file_get_contents( $file ); $m = array(); preg_match_all( - '/(?:wfRunHooks|Hooks\:\:run|ContentHandler\:\:runLegacyHooks)\(\s*([\'"])(.*?)\1/', + // All functions which runs hooks + '/(?:wfRunHooks|Hooks\:\:run|ContentHandler\:\:runLegacyHooks)\s*\(\s*' . + // First argument is the hook name as string + '([\'"])(.*?)\1' . + // Comma for second argument + '(?:\s*(,))?' . + // Second argument must start with array to be processed + '(?:\s*array\s*\(' . + // Matching inside array - allows one deep of brackets + '((?:[^\(\)]|\([^\(\)]*\))*)' . + // End + '\))?/', $content, - $m + $m, + PREG_SET_ORDER ); - return $m[2]; + // Extract parameter + $hooks = array(); + foreach ( $m as $match ) { + $args = array(); + if ( isset( $match[4] ) ) { + $n = array(); + if ( preg_match_all( '/((?:[^,\(\)]|\([^\(\)]*\))+)/', $match[4], $n ) ) { + $args = array_map( 'trim', $n[1] ); + } + } elseif ( isset( $match[3] ) ) { + // Found a parameter for Hooks::run, + // but could not extract the hooks argument, + // because there are given by a variable + $args = 'unknown'; + } + $hooks[$match[2]] = $args; + } + + return $hooks; } /** * Get hooks from the source code. * @param string $path Directory where the include files can be found - * @return array Array of hooks found + * @return array Array: key => hook name; value => array of arguments or string 'unknown' */ private function getHooksFromPath( $path ) { $hooks = array(); -- 2.20.1