LowerCamelFunctionsNameSniff: Ignore hook methods

For function names that have an underscore, check if the class
implements a hook interface associated with that method, using
the logic that a hook `on(Something)` comes from a hook interface
named `(Something)Hook`

Bug: T273482
Change-Id: I4c5d7fe7a533a2de885a5d9e6d3fb2a3470ea924

HISTORY.md

@@ -19,6 +19,7 @@
 * `FunctionCommentSniff`: Actually check if a method returns anything (thiemowmde)
 * `FunctionAnnotationsSniff`: Add `@phan-type` as an allowed annotation (Umherirrender)
 * `FunctionAnnotationsSniff`: Add `@phan-side-effect-free` as an allowed annotation (Bartosz Dziewoński)
+* `LowerCamelFunctionsNameSniff`: Ignore hook methods (DannyS712)
 
 ### Removed sniffs ###
 * `OneSpaceInlineArraySniff`: Superseded by `Universal.WhiteSpace.CommaSpacing`

MediaWiki/Sniffs/NamingConventions/LowerCamelFunctionsNameSniff.php

@@ -8,7 +8,15 @@
 use PHP_CodeSniffer\Util\Tokens;
 
 /**
- * Make sure lower camel method name.
+ * Make sure function names follow lower camel case
+ *
+ * This ignores methods in the form on(Something) where the class
+ * implements an interface with the name (Something)Hook, to avoid
+ * sending warnings for code in MediaWiki extensions and skins for
+ * hook handlers where the method cannot be renamed because it is
+ * inherited from the hook interface
+ *
+ * @author DannyS712
  */
 class LowerCamelFunctionsNameSniff implements Sniff {
 
@@ -76,14 +84,24 @@ public function process( File $phpcsFile, $stackPtr ) {
 			return;
 		}
 
+		$containsUnderscores = str_contains( $originalFunctionName, '_' );
 		if ( $originalFunctionName[0] === $lowerFunctionName[0] &&
-			( !str_contains( $originalFunctionName, '_' ) || $this->isTestFunction( $phpcsFile, $stackPtr ) )
+			( !$containsUnderscores || $this->isTestFunction( $phpcsFile, $stackPtr ) )
 		) {
 			// Everything is ok when the first letter is lowercase and there are no underscores
 			// (except in tests where they are allowed)
 			return;
 		}
 
+		if ( $containsUnderscores ) {
+			// Check for MediaWiki hooks
+			// Only matters if there is an underscore, all hook handlers have methods beginning
+			// with "on" and so start with lowercase
+			if ( $this->shouldIgnoreHookHandler( $phpcsFile, $stackPtr, $originalFunctionName ) ) {
+				return;
+			}
+		}
+
 		$tokens = $phpcsFile->getTokens();
 		foreach ( $tokens[$stackPtr]['conditions'] as $code ) {
 			if ( !isset( Tokens::$ooScopeTokens[$code] ) ) {
@@ -98,4 +116,56 @@ public function process( File $phpcsFile, $stackPtr ) {
 			);
 		}
 	}
+
+	/**
+	 * Check if the method should be ignored because it is a hook handler and the method
+	 * name is inherited from an interface
+	 *
+	 * @param File $phpcsFile
+	 * @param int $stackPtr
+	 * @param string $functionName
+	 * @return bool
+	 */
+	private function shouldIgnoreHookHandler(
+		File $phpcsFile,
+		int $stackPtr,
+		string $functionName
+	): bool {
+		$matches = [];
+		if ( !( preg_match( '/^on([A-Z]\S+)$/', $functionName, $matches ) ) ) {
+			return false;
+		}
+
+		// Method name looks like a hook handler, check if the class implements
+		// a hook by that name
+
+		$classToken = $this->getClassToken( $phpcsFile, $stackPtr );
+		if ( !$classToken ) {
+			// Not within a class, don't skip
+			return false;
+		}
+
+		$implementedInterfaces = $phpcsFile->findImplementedInterfaceNames( $classToken );
+		if ( !$implementedInterfaces ) {
+			// Not implementing the hook interface
+			return false;
+		}
+
+		$hookMethodName = $matches[1];
+		$hookInterfaceName = $hookMethodName . 'Hook';
+
+		// We need to account for the interface name in both the fully qualified form,
+		// and just the interface name. If we have the fully qualified form, explode()
+		// will return an array of the different namespaces and sub namespaces, with the
+		// last entry being the actual interface name, and if we just have the interface
+		// name, explode() will return an array of just that string
+		foreach ( $implementedInterfaces as $interface ) {
+			$parts = explode( '\\', $interface );
+			if ( end( $parts ) === $hookInterfaceName ) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 }

MediaWiki/Tests/files/NamingConventions/lower_camel_functions_name_hooks.php

@@ -0,0 +1,54 @@
+<?php
+
+use MediaWiki\Hook\RecentChange_saveHook;
+
+class HookHandler implements
+	RecentChange_saveHook,
+	\MediaWiki\SpecialPage\Hook\SpecialPage_initListHook
+{
+	/**
+	 * This method name comes from the hook and cannot be changed, phpcs should not complain
+	 * (the interface for this hook is imported with a `use` statement, and the `implements`
+	 * only has the final interface name
+	 *
+	 * @param RecentChange $recentChange
+	 * @return bool|void True or no return value to continue or false to abort
+	 */
+	public function onRecentChange_save( $recentChange ) {
+		return false;
+	}
+
+	/**
+	 * This method name comes from the hook and cannot be changed, phpcs should not complain
+	 * (the interface for this hook is not imported with a `use` statement, and the `implements`
+	 * has the fully qualified name
+	 *
+	 * @param array &$list List of core special pages
+	 * @return bool|void True or no return value to continue or false to abort
+	 */
+	public function onSpecialPage_initList( &$list ) {
+		return false;
+	}
+
+	/**
+	 * This method is not inherited from an interface, and can be changed, phpcs should complain,
+	 * even though it looks like a hook handler
+	 *
+	 * @param SpecialUpload $upload
+	 * @return bool|void True or no return value to continue or false to abort
+	 */
+	public function onUploadForm_initial( $upload ) {
+		return false;
+	}
+
+	/**
+	 * This method is not inherited from an interface and does not look like a hook handler,
+	 * phpcs should complain
+	 *
+	 * @return bool
+	 */
+	public function getThe_thing() {
+		return false;
+	}
+
+}

MediaWiki/Tests/files/NamingConventions/lower_camel_functions_name_hooks.php.expect

@@ -0,0 +1,4 @@
+ 40 | ERROR | Method name "onUploadForm_initial" should use lower camel case.
+    |       | (MediaWiki.NamingConventions.LowerCamelFunctionsName.FunctionName)
+ 50 | ERROR | Method name "getThe_thing" should use lower camel case.
+    |       | (MediaWiki.NamingConventions.LowerCamelFunctionsName.FunctionName)