Ignore unused parameters when function in abstract class is empty (sirbrillig#270)

* Add test for ignoring unused parameters in empty abstract methods

* Ignore unused parameters when function in abstract class is empty

* Fix lint errors

Author: sirbrillig

Tests/VariableAnalysisSniff/VariableAnalysisTest.php

@@ -209,6 +209,7 @@ public function testClassWithMembersWarnings()
 			66,
 			115,
 			116,
+			174,
 		];
 		$this->assertSame($expectedWarnings, $lines);
 	}

Tests/VariableAnalysisSniff/fixtures/ClassWithMembersFixture.php

@@ -154,9 +154,24 @@ abstract class AbstractClassWithStaticProperties {
   public static int $static_with_visibility_and_type;
   public static ?int $static_with_visibility_and_nullable_type;
 
-  public function use_vars();
+  abstract public function use_vars();
 
-  public static function getIntOrNull($value);
+  abstract public static function getIntOrNull($value);
 
-  static function getIntOrNull2($value);
+  abstract static function getIntOrNull2($value);
+}
+
+abstract class AbstractClassWithEmptyMethodBodies {
+  abstract public function bar($param);
+
+  public function baz($param) {
+  }
+
+  public function baz2($param) {
+    return;
+  }
+
+  public function baz3($param) { // Unused variable $param
+    return 'foobar';
+  }
 }

VariableAnalysis/Lib/Helpers.php

@@ -91,14 +91,15 @@ public static function areAnyConditionsAClass(array $conditions)
 	}
 
 	/**
+	 * Return true if the token conditions are within a function before they are
+	 * within a class.
+	 *
 	 * @param (int|string)[] $conditions
 	 *
 	 * @return bool
 	 */
 	public static function areConditionsWithinFunctionBeforeClass(array $conditions)
 	{
-		// Return true if the token conditions are within a function before
-		// they are within a class.
 		$classTypes = [T_CLASS, T_ANON_CLASS, T_TRAIT];
 		foreach (array_reverse($conditions, true) as $scopeCode) {
 			if (in_array($scopeCode, $classTypes)) {
@@ -112,14 +113,15 @@ public static function areConditionsWithinFunctionBeforeClass(array $conditions)
 	}
 
 	/**
+	 * Return true if the token conditions are within an if block before they are
+	 * within a class or function.
+	 *
 	 * @param (int|string)[] $conditions
 	 *
 	 * @return int|string|null
 	 */
 	public static function getClosestIfPositionIfBeforeOtherConditions(array $conditions)
 	{
-		// Return true if the token conditions are within an if block before
-		// they are within a class or function.
 		$conditionsInsideOut = array_reverse($conditions, true);
 		if (empty($conditions)) {
 			return null;
@@ -1396,4 +1398,55 @@ public static function isConstructorPromotion(File $phpcsFile, $stackPtr)
 
 		return false;
 	}
+
+	/**
+	 * Return true if the token is inside an abstract class.
+	 *
+	 * @param File $phpcsFile
+	 * @param int  $stackPtr
+	 *
+	 * @return bool
+	 */
+	public static function isInAbstractClass(File $phpcsFile, $stackPtr)
+	{
+		$classIndex = $phpcsFile->getCondition($stackPtr, T_CLASS);
+		if (! is_int($classIndex)) {
+			return false;
+		}
+		$classProperties = $phpcsFile->getClassProperties($classIndex);
+		return $classProperties['is_abstract'];
+	}
+
+	/**
+	 * Return true if the function body is empty or contains only `return;`
+	 *
+	 * @param File $phpcsFile
+	 * @param int  $stackPtr  The index of the function keyword.
+	 *
+	 * @return bool
+	 */
+	public static function isFunctionBodyEmpty(File $phpcsFile, $stackPtr)
+	{
+		$tokens = $phpcsFile->getTokens();
+		if ($tokens[$stackPtr]['code'] !== T_FUNCTION) {
+			return false;
+		}
+		$functionScopeStart = $tokens[$stackPtr]['scope_opener'];
+		$functionScopeEnd = $tokens[$stackPtr]['scope_closer'];
+		$tokensToIgnore = array_merge(
+			Tokens::$emptyTokens,
+			[
+				T_RETURN,
+				T_SEMICOLON,
+				T_OPEN_CURLY_BRACKET,
+				T_CLOSE_CURLY_BRACKET,
+			]
+		);
+		for ($i = $functionScopeStart; $i < $functionScopeEnd; $i++) {
+			if (! in_array($tokens[$i]['code'], $tokensToIgnore, true)) {
+				return false;
+			}
+		}
+		return true;
+	}
 }

VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

@@ -1948,6 +1948,24 @@ protected function processScopeCloseForVariable(File $phpcsFile, VariableInfo $v
 		if ($scopeInfo->scopeStartIndex === 0 && $this->allowUnusedVariablesInFileScope) {
 			return;
 		}
+		if (
+			! empty($varInfo->firstDeclared)
+			&& $varInfo->scopeType === ScopeType::PARAM
+			&& Helpers::isInAbstractClass(
+				$phpcsFile,
+				Helpers::getFunctionIndexForFunctionParameter($phpcsFile, $varInfo->firstDeclared) ?: 0
+			)
+			&& Helpers::isFunctionBodyEmpty(
+				$phpcsFile,
+				Helpers::getFunctionIndexForFunctionParameter($phpcsFile, $varInfo->firstDeclared) ?: 0
+			)
+		) {
+			// Allow non-abstract methods inside an abstract class to have unused
+			// parameters if the method body does nothing. Such methods are
+			// effectively optional abstract methods so their unused parameters
+			// should be ignored as we do with abstract method parameters.
+			return;
+		}
 
 		$this->warnAboutUnusedVariable($phpcsFile, $varInfo);
 	}