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

diff --git a/docs/hooks.txt b/docs/hooks.txt
index 41cf488..ca5da14 100644 (file)
--- 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 0925406..4afeb0e 100644 (file)
--- 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();