[PHPMD] Add phpmd rule with ignoring of unused required parameters in the plugins

app/code/Magento/Catalog/Plugin/CategoryAuthorization.php

@@ -38,7 +38,6 @@ public function __construct(Authorization $authorization)
  * @param CategoryInterface $category
  * @throws LocalizedException
  * @return array
- * @SuppressWarnings(PHPMD.UnusedFormalParameter)
  */
  public function beforeSave(CategoryRepositoryInterface $subject, CategoryInterface $category): array
  {

app/code/Magento/Catalog/Plugin/ProductAuthorization.php

@@ -39,7 +39,6 @@ public function __construct(Authorization $authorization)
  * @param bool $saveOptions
  * @throws LocalizedException
  * @return array
- * @SuppressWarnings(PHPMD.UnusedFormalParameter)
  */
  public function beforeSave(
  ProductRepositoryInterface $subject,

app/code/Magento/Catalog/Plugin/RemoveImagesFromGalleryAfterRemovingProduct.php

@@ -45,7 +45,6 @@ public function __construct(Gallery $galleryResource, ReadHandler $mediaGalleryR
  * @param callable $proceed
  * @param ProductInterface $product
  * @return bool
- * @SuppressWarnings(PHPMD.UnusedFormalParameter)
  */
  public function aroundDelete(
  ProductRepositoryInterface $subject,

dev/tests/static/framework/Magento/CodeMessDetector/Rule/UnusedCode/UnusedFormalParameter.php

@@ -0,0 +1,103 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\CodeMessDetector\Rule\UnusedCode;
+
+use PHPMD\AbstractNode;
+use PHPMD\Node\ClassNode;
+use PHPMD\Node\MethodNode;
+use PHPMD\Rule\UnusedFormalParameter as PhpmdUnusedFormalParameter;
+
+class UnusedFormalParameter extends PhpmdUnusedFormalParameter
+{
+ /**
+ * This method collects all local variables in the body of the currently
+ * analyzed method or function and removes those parameters that are
+ * referenced by one of the collected variables.
+ *
+ * @param AbstractNode $node
+ * @return void
+ */
+ protected function removeUsedParameters(AbstractNode $node)
+ {
+ parent::removeUsedParameters($node);
+ $this->removeVariablesUsedInPlugins($node);
+ }
+
+ /**
+ * Remove required method variables used in plugins from given node
+ *
+ * @param AbstractNode $node
+ */
+ private function removeVariablesUsedInPlugins(AbstractNode $node)
+ {
+ if (!$node instanceof MethodNode) {
+ return;
+ }
+
+ /** @var ClassNode $classNode */
+ $classNode = $node->getParentType();
+ if (!$this->isPluginClass($classNode->getNamespaceName())) {
+ return;
+ }
+
+ /**
+ * Around and After plugins has 2 required params $subject and $proceed or $result
+ * that should be ignored
+ */
+ foreach (['around', 'after'] as $pluginMethodPrefix) {
+ if ($this->isFunctionNameStartingWith($node, $pluginMethodPrefix)) {
+ $this->removeVariablesByCount(2);
+
+ break;
+ }
+ }
+
+ /**
+ * Before plugins has 1 required params $subject
+ * that should be ignored
+ */
+ if ($this->isFunctionNameStartingWith($node, 'before')) {
+ $this->removeVariablesByCount(1);
+ }
+ }
+
+ /**
+ * Check if the first part of function fully qualified name is equal to $name
+ *
+ * Methods getImage and getName are equal. getImage used prior to usage in phpmd source
+ *
+ * @param MethodNode $node
+ * @param string $name
+ * @return boolean
+ */
+ private function isFunctionNameStartingWith(MethodNode $node, string $name): bool
+ {
+ return (0 === strpos($node->getImage(), $name));
+ }
+
+ /**
+ * Remove first $countOfRemovingVariables from given node
+ *
+ * @param int $countOfRemovingVariables
+ */
+ private function removeVariablesByCount(int $countOfRemovingVariables)
+ {
+ array_splice($this->nodes, 0, $countOfRemovingVariables);
+ }
+
+ /**
+ * Check if namespace contain "Plugin". Case-sensitive ignored
+ *
+ * @param string $class
+ * @return bool
+ */
+ private function isPluginClass(string $class): bool
+ {
+ return (stripos($class, 'plugin') !== false);
+ }
+}

dev/tests/static/framework/Magento/CodeMessDetector/Test/Unit/Rule/UnusedCode/UnusedFormalParameterTest.php

@@ -0,0 +1,178 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\CodeMessDetector\Test\Unit\Rule\UnusedCode;
+
+use Magento\CodeMessDetector\Rule\UnusedCode\UnusedFormalParameter;
+use PHPMD\Node\ASTNode;
+use PHPMD\Node\MethodNode;
+use PHPMD\Report;
+use PHPUnit\Framework\MockObject\MockObject as MockObject;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @SuppressWarnings(PHPMD.UnusedFormalParameter)
+ */
+class UnusedFormalParameterTest extends TestCase
+{
+ private const FAKE_PLUGIN_NAMESPACE = 'Magento\CodeMessDetector\Test\UnusedCode\Plugin';
+ private const FAKE_NAMESPACE = 'Magento\CodeMessDetector\Test\UnusedCode';
+
+ /**
+ *
+ * @dataProvider getCases
+ */
+ public function testApply($methodName, $methodParams, $namespace, $expectViolation)
+ {
+ $node = $this->createMethodNodeMock($methodName, $methodParams, $namespace);
+ $rule = new UnusedFormalParameter();
+ $this->expectsRuleViolation($rule, $expectViolation);
+ $rule->apply($node);
+ }
+
+ /**
+ * Prepare method node mock
+ *
+ * @param $methodName
+ * @param $methodParams
+ * @param $namespace
+ * @return MethodNode|MockObject
+ */
+ private function createMethodNodeMock($methodName, $methodParams, $namespace)
+ {
+ $methodNode = $this->createConfiguredMock(
+ MethodNode::class,
+ [
+ 'getName' => $methodName,
+ 'getImage' => $methodName,
+ 'isAbstract' => false,
+ 'isDeclaration' => true
+ ]
+ );
+
+ $variableDeclarators = [];
+ foreach ($methodParams as $methodParam) {
+ $variableDeclarator = $this->createASTNodeMock();
+ $variableDeclarator->method('getImage')
+ ->willReturn($methodParam);
+
+ $variableDeclarators[] = $variableDeclarator;
+ }
+ $parametersMock = $this->createASTNodeMock();
+ $parametersMock->expects($this->once())
+ ->method('findChildrenOfType')
+ ->with('VariableDeclarator')
+ ->willReturn($variableDeclarators);
+
+ /**
+ * Declare mock result for findChildrenOfType
+ * with Dummy for removeCompoundVariables and removeVariablesUsedByFuncGetArgs
+ */
+ $methodNode->expects($this->atLeastOnce())
+ ->method('findChildrenOfType')
+ ->withConsecutive(['FormalParameters'], ['CompoundVariable'], ['FunctionPostfix'])
+ ->willReturnOnConsecutiveCalls([$parametersMock], [], []);
+
+ // Dummy result for removeRegularVariables
+ $methodNode->expects($this->once())
+ ->method('findChildrenOfTypeVariable')
+ ->willReturn([]);
+
+ $classNode = $this->createASTNodeMock();
+ $classNode->expects($this->once())
+ ->method('getNamespaceName')
+ ->willReturn($namespace);
+ $methodNode->expects($this->once())
+ ->method('getParentType')
+ ->willReturn($classNode);
+
+ return $methodNode;
+ }
+
+ /**
+ * Create ASTNode mock
+ *
+ * @return ASTNode|MockObject
+ */
+ private function createASTNodeMock()
+ {
+ return $this->createMock(ASTNode::class);
+ }
+
+ /**
+ * @param UnusedFormalParameter $rule
+ * @param bool $expects
+ */
+ private function expectsRuleViolation(UnusedFormalParameter $rule, bool $expects)
+ {
+ /** @var Report|MockObject $reportMock */
+ $reportMock = $this->createMock(Report::class);
+ if ($expects) {
+ $violationExpectation = $this->atLeastOnce();
+ } else {
+ $violationExpectation = $this->never();
+ }
+ $reportMock->expects($violationExpectation)
+ ->method('addRuleViolation');
+ $rule->setReport($reportMock);
+ }
+
+ /**
+ * @return array
+ */
+ public function getCases(): array
+ {
+ return [
+ // Plugin methods
+ [
+ 'beforePluginMethod',
+ [
+ 'subject'
+ ],
+ self::FAKE_PLUGIN_NAMESPACE,
+ false
+ ],
+ [
+ 'aroundPluginMethod',
+ [
+ 'subject',
+ 'proceed'
+ ],
+ self::FAKE_PLUGIN_NAMESPACE,
+ false
+ ],
+ [
+ 'aroundPluginMethod',
+ [
+ 'subject',
+ 'result'
+ ],
+ self::FAKE_PLUGIN_NAMESPACE,
+ false
+ ],
+ // Plugin method that contain unused parameter
+ [
+ 'someMethod',
+ [
+ 'unusedParameter'
+ ],
+ self::FAKE_PLUGIN_NAMESPACE,
+ true
+ ],
+ // Non plugin method
+ [
+ 'someMethod',
+ [
+ 'subject',
+ 'result'
+ ],
+ self::FAKE_NAMESPACE,
+ true
+ ]
+ ];
+ }
+}

dev/tests/static/framework/Magento/CodeMessDetector/resources/rulesets/unusedcode.xml

@@ -0,0 +1,31 @@
+<?xml version="1.0"?>
+<!--
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+-->
+<ruleset name="Unused Code Rules"
+ xmlns="http://pmd.sf.net/ruleset/1.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://pmd.sf.net/ruleset/1.0.0 http://pmd.sf.net/ruleset_xml_schema.xsd"
+ xsi:noNamespaceSchemaLocation="http://pmd.sf.net/ruleset_xml_schema.xsd">
+
+ <rule name="UnusedFormalParameter"
+ message="Avoid unused parameters such as '{0}'."
+ class="Magento\CodeMessDetector\Rule\UnusedCode\UnusedFormalParameter">
+ <description><![CDATA[Avoid passing parameters to methods or constructors and then not using those parameters except on plugins]]></description>
+ <example>
+<![CDATA[
+class Foo
+{
+ private function bar($howdy)
+ {
+ // $howdy is not used
+ }
+}
+]]>
+ </example>
+ </rule>
+
+</ruleset>

dev/tests/static/testsuite/Magento/Test/Php/_files/phpmd/ruleset.xml

@@ -26,7 +26,9 @@
  </rule>
 
  <!-- Unused code rules -->
- <rule ref="rulesets/unusedcode.xml" />
+ <rule ref="rulesets/unusedcode.xml">
+ <exclude name="UnusedFormalParameter"/>
+ </rule>
 
  <!-- Code design rules -->
  <rule ref="rulesets/design.xml/NumberOfChildren" />
@@ -45,5 +47,6 @@
  <!-- Magento Specific Rules -->
  <rule ref="Magento/CodeMessDetector/resources/rulesets/design.xml/AllPurposeAction" />
  <rule ref="Magento/CodeMessDetector/resources/rulesets/design.xml/CookieAndSessionMisuse" />
+ <rule ref="Magento/CodeMessDetector/resources/rulesets/unusedcode.xml/UnusedFormalParameter" />
 
 </ruleset>