Make sure FormExtracotrs are disabled unless we extract from a form type (#74)

Nyholm · web-flow · commit 06557d460a05 · 2018-01-06T13:26:52.000+01:00
* Make sure FormExtracotrs are disabled unless we extract from a form type

* cs

* Check for sf 2

* using a FormTrait

* Update composer alias

* cs

* Remove PHP 7 code

* Remove more PHP 7 code

* cs
diff --git a/composer.json b/composer.json
@@ -37,7 +37,7 @@
     },
     "extra": {
         "branch-alias": {
-            "dev-master": "1.2-dev"
+            "dev-master": "1.4-dev"
         }
     }
 }
diff --git a/src/Visitor/Php/Symfony/FormTrait.php b/src/Visitor/Php/Symfony/FormTrait.php
@@ -0,0 +1,37 @@
+<?php
+
+/*
+ * This file is part of the PHP Translation package.
+ *
+ * (c) PHP Translation team <tobias.nyholm@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Translation\Extractor\Visitor\Php\Symfony;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt;
+
+trait FormTrait
+{
+    private $isFormType = false;
+
+    /**
+     * Check if this node is a form type.
+     *
+     * @param Node $node
+     *
+     * @return bool
+     */
+    private function isFormType(Node $node)
+    {
+        // only Traverse *Type
+        if ($node instanceof Stmt\Class_) {
+            $this->isFormType = 'Type' === substr($node->name, -4);
+        }
+
+        return $this->isFormType;
+    }
+}
diff --git a/src/Visitor/Php/Symfony/FormTypeChoices.php b/src/Visitor/Php/Symfony/FormTypeChoices.php
@@ -12,7 +12,6 @@
 namespace Translation\Extractor\Visitor\Php\Symfony;
 
 use PhpParser\Node;
-use PhpParser\Node\Stmt;
 use PhpParser\NodeVisitor;
 use Translation\Extractor\Model\SourceLocation;
 use Translation\Extractor\Visitor\Php\BasePHPVisitor;
@@ -22,6 +21,8 @@
  */
 final class FormTypeChoices extends BasePHPVisitor implements NodeVisitor
 {
+    use FormTrait;
+
     /**
      * @var int defaults to major version 3
      */
@@ -41,11 +42,8 @@ public function setSymfonyMajorVersion($sfMajorVersion)
 
     public function enterNode(Node $node)
     {
-        // only Traverse *Type
-        if ($node instanceof Stmt\Class_) {
-            if ('Type' !== substr($node->name, -4)) {
-                return;
-            }
+        if (!$this->isFormType($node)) {
+            return;
         }
 
         if (null === $this->state && $node instanceof Node\Expr\Assign) {
@@ -60,8 +58,8 @@ public function enterNode(Node $node)
             $this->state = null;
         }
 
-        // symfony 3 displays key by default, where symfony 2 displays value
-        $useKey = 3 === $this->symfonyMajorVersion;
+        // symfony 3 or 4 displays key by default, where symfony 2 displays value
+        $useKey = 2 !== $this->symfonyMajorVersion;
 
         // remember choices in this node
         $choicesNodes = [];
diff --git a/src/Visitor/Php/Symfony/FormTypeEmptyValue.php b/src/Visitor/Php/Symfony/FormTypeEmptyValue.php
@@ -12,7 +12,6 @@
 namespace Translation\Extractor\Visitor\Php\Symfony;
 
 use PhpParser\Node;
-use PhpParser\Node\Stmt;
 use PhpParser\NodeVisitor;
 use Translation\Extractor\Visitor\Php\BasePHPVisitor;
 
@@ -21,13 +20,12 @@
  */
 final class FormTypeEmptyValue extends BasePHPVisitor implements NodeVisitor
 {
+    use FormTrait;
+
     public function enterNode(Node $node)
     {
-        // only Traverse *Type
-        if ($node instanceof Stmt\Class_) {
-            if ('Type' !== substr($node->name, -4)) {
-                return;
-            }
+        if (!$this->isFormType($node)) {
+            return;
         }
 
         if (!$node instanceof Node\Expr\Array_) {
diff --git a/src/Visitor/Php/Symfony/FormTypeInvalidMessage.php b/src/Visitor/Php/Symfony/FormTypeInvalidMessage.php
@@ -12,7 +12,6 @@
 namespace Translation\Extractor\Visitor\Php\Symfony;
 
 use PhpParser\Node;
-use PhpParser\Node\Stmt;
 use PhpParser\NodeVisitor;
 use Translation\Extractor\Visitor\Php\BasePHPVisitor;
 
@@ -21,13 +20,12 @@
  */
 final class FormTypeInvalidMessage extends BasePHPVisitor implements NodeVisitor
 {
+    use FormTrait;
+
     public function enterNode(Node $node)
     {
-        // only Traverse *Type
-        if ($node instanceof Stmt\Class_) {
-            if ('Type' !== substr($node->name, -4)) {
-                return;
-            }
+        if (!$this->isFormType($node)) {
+            return;
         }
 
         if (!$node instanceof Node\Expr\Array_) {
diff --git a/src/Visitor/Php/Symfony/FormTypeLabelExplicit.php b/src/Visitor/Php/Symfony/FormTypeLabelExplicit.php
@@ -12,7 +12,6 @@
 namespace Translation\Extractor\Visitor\Php\Symfony;
 
 use PhpParser\Node;
-use PhpParser\Node\Stmt;
 use PhpParser\NodeVisitor;
 use Translation\Extractor\Visitor\Php\BasePHPVisitor;
 
@@ -21,13 +20,12 @@
  */
 final class FormTypeLabelExplicit extends BasePHPVisitor implements NodeVisitor
 {
+    use FormTrait;
+
     public function enterNode(Node $node)
     {
-        // only Traverse *Type
-        if ($node instanceof Stmt\Class_) {
-            if ('Type' !== substr($node->name, -4)) {
-                return;
-            }
+        if (!$this->isFormType($node)) {
+            return;
         }
 
         // we could have chosen to traverse specifically the buildForm function or ->add()
diff --git a/src/Visitor/Php/Symfony/FormTypeLabelImplicit.php b/src/Visitor/Php/Symfony/FormTypeLabelImplicit.php
@@ -12,7 +12,6 @@
 namespace Translation\Extractor\Visitor\Php\Symfony;
 
 use PhpParser\Node;
-use PhpParser\Node\Stmt;
 use PhpParser\NodeVisitor;
 use Translation\Extractor\Visitor\Php\BasePHPVisitor;
 
@@ -21,13 +20,12 @@
  */
 final class FormTypeLabelImplicit extends BasePHPVisitor implements NodeVisitor
 {
+    use FormTrait;
+
     public function enterNode(Node $node)
     {
-        // only Traverse *Type
-        if ($node instanceof Stmt\Class_) {
-            if ('Type' !== substr($node->name, -4)) {
-                return;
-            }
+        if (!$this->isFormType($node)) {
+            return;
         }
 
         $domain = null;
diff --git a/src/Visitor/Php/Symfony/FormTypePlaceholder.php b/src/Visitor/Php/Symfony/FormTypePlaceholder.php
@@ -12,7 +12,6 @@
 namespace Translation\Extractor\Visitor\Php\Symfony;
 
 use PhpParser\Node;
-use PhpParser\Node\Stmt;
 use PhpParser\NodeVisitor;
 use Translation\Extractor\Visitor\Php\BasePHPVisitor;
 
@@ -21,13 +20,12 @@
  */
 final class FormTypePlaceholder extends BasePHPVisitor implements NodeVisitor
 {
+    use FormTrait;
+
     public function enterNode(Node $node)
     {
-        // only Traverse *Type
-        if ($node instanceof Stmt\Class_) {
-            if ('Type' !== substr($node->name, -4)) {
-                return;
-            }
+        if (!$this->isFormType($node)) {
+            return;
         }
 
         if (!$node instanceof Node\Expr\Array_) {
diff --git a/tests/Functional/Visitor/Php/Symfony/FormEmptyValueTest.php b/tests/Functional/Visitor/Php/Symfony/FormEmptyValueTest.php
@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Translation\extractor\tests\Functional\Visitor\Php\Symfony;
+namespace Translation\Extractor\Tests\Functional\Visitor\Php\Symfony;
 
 use Translation\Extractor\Tests\Functional\Visitor\Php\BasePHPVisitorTest;
 use Translation\Extractor\Tests\Resources;
diff --git a/tests/Functional/Visitor/Php/Symfony/FormTypeInvalidMessageTest.php b/tests/Functional/Visitor/Php/Symfony/FormTypeInvalidMessageTest.php
@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Translation\extractor\tests\Functional\Visitor\Php\Symfony;
+namespace Translation\Extractor\Tests\Functional\Visitor\Php\Symfony;
 
 use Translation\Extractor\Tests\Functional\Visitor\Php\BasePHPVisitorTest;
 use Translation\Extractor\Tests\Resources;
diff --git a/tests/Functional/Visitor/Php/Symfony/FormTypePlaceholderTest.php b/tests/Functional/Visitor/Php/Symfony/FormTypePlaceholderTest.php
@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Translation\Extractor\Tests\Resources\Php\Symfony;
+namespace Translation\Extractor\Tests\Functional\Visitor\Php\Symfony;
 
 use Translation\Extractor\Tests\Functional\Visitor\Php\BasePHPVisitorTest;
 use Translation\Extractor\Tests\Resources;
diff --git a/tests/Resources/Github/Issue_71.php b/tests/Resources/Github/Issue_71.php
@@ -0,0 +1,29 @@
+<?php
+
+
+namespace App\Controller\Test;
+
+
+use Symfony\Bundle\FrameworkBundle\Controller\Controller;
+
+class TestController extends Controller
+{
+    public function foo()
+    {
+        $foo = [
+            ['label' => $this->get('translator')->trans('this.should.be.translated.without.errors')], //ERROR: Form label is not a scalar string
+            [/** @Ignore */'label' => $this->get('translator')->trans('this.should.be.translated.without.errors.and.now.it.works')],
+            ['label_' => $this->get('translator')->trans('this.works.correctly')],
+            ['label' => $this->shouldBeIgnored()], //ERROR: Form label is not a scalar string
+            [/** @Ignore */'label' => $this->shouldBeIgnoredAndGoesWithoutError()],
+        ];
+    }
+
+    private function shouldBeIgnored()
+    {
+    }
+
+    private function shouldBeIgnoredAndGoesWithoutError()
+    {
+    }
+}
diff --git a/tests/Resources/Php/Symfony/FormInvalidMessageType.php b/tests/Resources/Php/Symfony/FormInvalidMessageType.php
@@ -6,7 +6,7 @@
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\FormBuilderInterface;
 
-class FormInvalidMessage extends AbstractType
+class FormInvalidMessageType extends AbstractType
 {
     public function buildForm(FormBuilderInterface $builder, array $options)
     {
diff --git a/tests/Smoke/AllExtractorsTest.php b/tests/Smoke/AllExtractorsTest.php
@@ -45,6 +45,12 @@ public function testNoException()
         $sc = $extractor->extract($finder);
         $this->translationExists($sc, 'trans.issue_34');
         $this->translationMissing($sc, 'trans.issue_62');
+
+        /*
+         * It is okey to increase the error count if you adding more fixtures/code.
+         * We just need to be aware that it changes.
+         */
+        $this->assertCount(9, $sc->getErrors(), 'There was an unexpected number of errors. Please investigate.');
     }
 
     /**

Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@`
`37`	`37`	`},`
`38`	`38`	`"extra": {`
`39`	`39`	`"branch-alias": {`
`40`		`- "dev-master": "1.2-dev"`
	`40`	`+ "dev-master": "1.4-dev"`
`41`	`41`	`}`
`42`	`42`	`}`
`43`	`43`	`}`