Merge pull request #832 from magento-mpi/MAGETWO-64523

MAGETWO-64523: Add static test on forbidden "final" keyword and eliminate it usage in code

app/code/Magento/Braintree/Model/Ui/ConfigProvider.php

@@ -13,7 +13,7 @@
 /**
  * Class ConfigProvider
  */
-final class ConfigProvider implements ConfigProviderInterface
+class ConfigProvider implements ConfigProviderInterface
 {
     const CODE = 'braintree';
 

app/code/Magento/Vault/Model/Method/Vault.php

@@ -28,7 +28,7 @@
  * @SuppressWarnings(PHPMD.ExcessivePublicCount)
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
-final class Vault implements VaultPaymentInterface
+class Vault implements VaultPaymentInterface
 {
     /**
      * @deprecated

app/code/Magento/Vault/Model/Ui/Adminhtml/TokensConfigProvider.php

@@ -27,7 +27,7 @@
  * @api
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
-final class TokensConfigProvider
+class TokensConfigProvider
 {
     /**
      * @var PaymentTokenRepositoryInterface

app/code/Magento/Vault/Model/Ui/TokensConfigProvider.php

@@ -15,7 +15,7 @@
  * Class ConfigProvider
  * @api
  */
-final class TokensConfigProvider implements ConfigProviderInterface
+class TokensConfigProvider implements ConfigProviderInterface
 {
     /**
      * @var string

dev/tests/integration/testsuite/Magento/Framework/Code/GeneratorTest/ParentClassWithNamespace.php

@@ -78,6 +78,9 @@ public static function publicParentStatic()
     {
     }
 
+    /**
+     * @SuppressWarnings(PHPMD.FinalImplementation) Suppressed as is a fixture but not a real code
+     */
     final public function publicParentFinal()
     {
     }

dev/tests/integration/testsuite/Magento/Framework/Code/GeneratorTest/SourceClassWithNamespace.php

@@ -104,6 +104,9 @@ public static function publicChildStatic()
     {
     }
 
+    /**
+     * @SuppressWarnings(PHPMD.FinalImplementation) Suppressed as is a fixture but not a real code
+     */
     final public function publicChildFinal()
     {
     }

dev/tests/integration/testsuite/Magento/Framework/Interception/Fixture/Intercepted.php

@@ -48,6 +48,7 @@ public function D($param1)
 
     /**
      * @SuppressWarnings(PHPMD.ShortMethodName)
+     * @SuppressWarnings(PHPMD.FinalImplementation) Suppressed as is a fixture but not a real code
      */
     final public function E($param1)
     {

dev/tests/static/framework/Magento/CodeMessDetector/Rule/Design/FinalImplementation.php

@@ -0,0 +1,30 @@
+<?php
+/**
+ * Copyright © 2013-2017 Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\CodeMessDetector\Rule\Design;
+
+use PHPMD\AbstractNode;
+use PHPMD\AbstractRule;
+use PHPMD\Rule\ClassAware;
+use PHPMD\Rule\MethodAware;
+
+/**
+ * Magento is a highly extensible and customizable platform.
+ * Usage of final classes and methods is prohibited.
+ */
+class FinalImplementation extends AbstractRule implements ClassAware, MethodAware
+{
+
+    /**
+     * @inheritdoc
+     */
+    public function apply(AbstractNode $node)
+    {
+        if ($node->isFinal()) {
+            $this->addViolation($node, [$node->getType(), $node->getFullQualifiedName()]);
+        }
+    }
+}

dev/tests/static/framework/Magento/CodeMessDetector/Test/Unit/Rule/Design/FinalImplementationTest.php

@@ -0,0 +1,128 @@
+<?php
+/**
+ * Copyright © 2013-2017 Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\CodeMessDetector\Test\Unit\Rule\Design;
+
+use PHPUnit_Framework_TestCase as TestCase;
+use PHPUnit_Framework_MockObject_MockObject as MockObject;
+use PHPUnit_Framework_MockObject_Matcher_InvokedRecorder as InvokedRecorder;
+use PHPUnit_Framework_MockObject_Builder_InvocationMocker as InvocationMocker;
+use Magento\CodeMessDetector\Rule\Design\FinalImplementation;
+use PHPMD\Report;
+use PHPMD\AbstractNode;
+use PHPMD\Node\ClassNode;
+use PHPMD\Node\MethodNode;
+use BadMethodCallException;
+
+class FinalImplementationTest extends TestCase
+{
+    /**
+     * @param string $nodeType
+     *
+     * @dataProvider finalizableNodeTypesProvider
+     */
+    public function testRuleNotAppliesToNotFinalFinalizable($nodeType)
+    {
+        $finalizableNode = $this->createFinalizableNodeMock($nodeType);
+        $finalizableNode->method('isFinal')->willReturn(false);
+
+        $rule = new FinalImplementation();
+        $this->expectsRuleViolation($rule, $this->never());
+        $rule->apply($finalizableNode);
+    }
+
+    /**
+     * @param string $nodeType
+     *
+     * @dataProvider finalizableNodeTypesProvider
+     */
+    public function testRuleAppliesToFinalFinalizable($nodeType)
+    {
+        $finalizableNode = $this->createFinalizableNodeMock($nodeType);
+        $finalizableNode->method('isFinal')->willReturn(true);
+
+        $rule = new FinalImplementation();
+        $this->expectsRuleViolation($rule, $this->once());
+        $rule->apply($finalizableNode);
+    }
+
+    /**
+     * @param string $nodeType
+     *
+     * @dataProvider finalizableNodeTypesProvider
+     */
+    public function testRuleVerifiesFinalizableNodes($nodeType)
+    {
+        $finalizableNode = $this->createFinalizableNodeMock($nodeType);
+
+        $finalizableNode->expects($this->atLeastOnce())
+            ->method('isFinal');
+
+        $rule = new FinalImplementation();
+        $rule->apply($finalizableNode);
+    }
+
+    /**
+     * @expectedException BadMethodCallException
+     */
+    public function testRuleFailsOnNotFinalizableNodes()
+    {
+        $someNode = $this->getMockBuilder(AbstractNode::class)
+            ->disableOriginalConstructor()
+            ->getMockForAbstractClass();
+
+        $rule = new FinalImplementation();
+        $rule->apply($someNode);
+    }
+
+    /**
+     * "final" keyword may be applied only to classes and methods
+     *
+     * @return array
+     */
+    public function finalizableNodeTypesProvider()
+    {
+        return [
+            [ClassNode::class],
+            [MethodNode::class],
+        ];
+    }
+
+    /**
+     * If node is finalizable it has "isFinal" magic PHP method
+     *
+     * @param string $nodeType
+     * @return ClassNode|MethodNode|MockObject
+     */
+    private function createFinalizableNodeMock($nodeType)
+    {
+        $finalizableNode = $this->getMockBuilder($nodeType)
+            ->disableOriginalConstructor()
+            ->disableProxyingToOriginalMethods()
+            ->setMethods([
+                'isFinal',
+                // disable name lookup from AST artifact
+                'getNamespaceName',
+                'getParentName',
+                'getName',
+            ])
+            ->getMock();
+        return $finalizableNode;
+    }
+
+    /**
+     * @param FinalImplementation $rule
+     * @param InvokedRecorder $violationExpectation
+     * @return InvocationMocker
+     */
+    private function expectsRuleViolation(FinalImplementation $rule, InvokedRecorder $violationExpectation)
+    {
+        $report = $this->getMockBuilder(Report::class)->getMock();
+        $invokation = $report->expects($violationExpectation)->method('addRuleViolation');
+        $rule->setReport($report);
+        return $invokation;
+    }
+}

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

@@ -0,0 +1,36 @@
+<?xml version="1.0"?>
+<!--
+/**
+ * Copyright © 2013-2017 Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+-->
+<ruleset name="Magento Specific Design 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="FinalImplementation"
+          class="Magento\CodeMessDetector\Rule\Design\FinalImplementation"
+          message= "The {0} {1} declared as final.">
+        <description>
+            <![CDATA[
+Final keyword is prohibited in Magento as this decreases extensibility and customizability.
+Final classes and method are not compatible with plugins and proxies.
+            ]]>
+        </description>
+        <priority>1</priority>
+        <properties />
+        <example>
+            <![CDATA[
+final class Foo
+{
+    public function bar() {}
+}
+class Baz {
+    final public function bad() {}
+}
+            ]]>
+        </example>
+    </rule>
+</ruleset>

dev/tests/static/framework/autoload.php

@@ -18,6 +18,7 @@
         $testsBaseDir . '/../integration/framework/Magento/TestFramework/',
     ]
 );
+$autoloadWrapper->addPsr4('Magento\\CodeMessDetector\\', $testsBaseDir . '/framework/Magento/CodeMessDetector');
 
 $generatedCode = DirectoryList::getDefaultConfig()[DirectoryList::GENERATED_CODE][DirectoryList::PATH];
 $autoloadWrapper->addPsr4('Magento\\', $baseDir . '/' . $generatedCode . '/Magento/');