From a28f4be392ed10d317b8d85cd573e0493445aa69 Mon Sep 17 00:00:00 2001 From: Sam Van der Borght Date: Thu, 15 Jun 2017 16:26:39 +0200 Subject: [PATCH 1/3] Add unit test to demonstrate incorrect child traversal behaviour --- .../Visitor/Php/BasePHPVisitorTest.php | 9 ++++++- .../Php/Symfony/FormTypePlaceholderTest.php | 25 +++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/tests/Functional/Visitor/Php/BasePHPVisitorTest.php b/tests/Functional/Visitor/Php/BasePHPVisitorTest.php index b2e89ee..370f27a 100644 --- a/tests/Functional/Visitor/Php/BasePHPVisitorTest.php +++ b/tests/Functional/Visitor/Php/BasePHPVisitorTest.php @@ -31,7 +31,14 @@ abstract class BasePHPVisitorTest extends \PHPUnit_Framework_TestCase protected function getSourceLocations($visitor, $namespaceForTestFile) { $extractor = new PHPFileExtractor(); - $extractor->addVisitor($visitor); + + if (is_array($visitor)) { + foreach($visitor as $nodeVisitor) { + $extractor->addVisitor($nodeVisitor); + } + } else { + $extractor->addVisitor($visitor); + } $currentNamespace = explode('\\', __NAMESPACE__); $fileNamespace = explode('\\', $namespaceForTestFile); diff --git a/tests/Functional/Visitor/Php/Symfony/FormTypePlaceholderTest.php b/tests/Functional/Visitor/Php/Symfony/FormTypePlaceholderTest.php index cf97b8d..f9a2cab 100644 --- a/tests/Functional/Visitor/Php/Symfony/FormTypePlaceholderTest.php +++ b/tests/Functional/Visitor/Php/Symfony/FormTypePlaceholderTest.php @@ -14,6 +14,7 @@ use Translation\Extractor\Tests\Functional\Visitor\Php\BasePHPVisitorTest; use Translation\Extractor\Tests\Resources; use Translation\Extractor\Visitor\Php\Symfony\FormTypePlaceholder; +use Translation\Extractor\Visitor\Php\Symfony\ContainerAwareTrans; /** * @author Tobias Nyholm @@ -22,7 +23,8 @@ final class FormTypePlaceholderTest extends BasePHPVisitorTest { public function testExtract() { - $collection = $this->getSourceLocations(new FormTypePlaceholder(), Resources\Php\Symfony\PlaceholderFormType::class); + $collection = $this->getSourceLocations(new FormTypePlaceholder(), + Resources\Php\Symfony\PlaceholderFormType::class); $this->assertCount(3, $collection); $this->assertEquals('form.placeholder.text', $collection->get(0)->getMessage()); @@ -32,9 +34,28 @@ public function testExtract() public function testExtractError() { - $collection = $this->getSourceLocations(new FormTypePlaceholder(), Resources\Php\Symfony\PlaceholderFormErrorType::class); + $collection = $this->getSourceLocations(new FormTypePlaceholder(), + Resources\Php\Symfony\PlaceholderFormErrorType::class); $errors = $collection->getErrors(); $this->assertCount(3, $errors); } + + public function testChildVisitationNotBlocked() + { + $collection = $this->getSourceLocations( + [ + new FormTypePlaceholder(), + new ContainerAwareTrans(), + ], + Resources\Php\Symfony\ContainerAwareTrans::class + ); + + $this->assertCount(4, $collection); + + $this->assertEquals('trans0', $collection->get(0)->getMessage()); + $this->assertEquals('trans1', $collection->get(1)->getMessage()); + $this->assertEquals('trans_line', $collection->get(2)->getMessage()); + $this->assertEquals('variable', $collection->get(3)->getMessage()); + } } From 45d24eba27a1cbc38996d418a17bd1c1a566d36f Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sat, 12 Aug 2017 18:09:18 +0200 Subject: [PATCH 2/3] Do not stop all visitors if FormVisitor fails --- src/Visitor/Php/Symfony/FormTypeChoices.php | 2 +- src/Visitor/Php/Symfony/FormTypeLabelExplicit.php | 2 +- src/Visitor/Php/Symfony/FormTypeLabelImplicit.php | 2 +- src/Visitor/Php/Symfony/FormTypePlaceholder.php | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Visitor/Php/Symfony/FormTypeChoices.php b/src/Visitor/Php/Symfony/FormTypeChoices.php index 378b272..89d2159 100644 --- a/src/Visitor/Php/Symfony/FormTypeChoices.php +++ b/src/Visitor/Php/Symfony/FormTypeChoices.php @@ -41,7 +41,7 @@ public function enterNode(Node $node) // only Traverse *Type if ($node instanceof Stmt\Class_) { if (substr($node->name, -4) !== 'Type') { - return NodeTraverser::DONT_TRAVERSE_CHILDREN; + return; } } diff --git a/src/Visitor/Php/Symfony/FormTypeLabelExplicit.php b/src/Visitor/Php/Symfony/FormTypeLabelExplicit.php index ffdd8b8..fe71719 100644 --- a/src/Visitor/Php/Symfony/FormTypeLabelExplicit.php +++ b/src/Visitor/Php/Symfony/FormTypeLabelExplicit.php @@ -28,7 +28,7 @@ public function enterNode(Node $node) // only Traverse *Type if ($node instanceof Stmt\Class_) { if (substr($node->name, -4) !== 'Type') { - return NodeTraverser::DONT_TRAVERSE_CHILDREN; + return; } } diff --git a/src/Visitor/Php/Symfony/FormTypeLabelImplicit.php b/src/Visitor/Php/Symfony/FormTypeLabelImplicit.php index 126824f..9b63489 100644 --- a/src/Visitor/Php/Symfony/FormTypeLabelImplicit.php +++ b/src/Visitor/Php/Symfony/FormTypeLabelImplicit.php @@ -28,7 +28,7 @@ public function enterNode(Node $node) // only Traverse *Type if ($node instanceof Stmt\Class_) { if (substr($node->name, -4) !== 'Type') { - return NodeTraverser::DONT_TRAVERSE_CHILDREN; + return; } } diff --git a/src/Visitor/Php/Symfony/FormTypePlaceholder.php b/src/Visitor/Php/Symfony/FormTypePlaceholder.php index f5c46e4..d92fbd3 100644 --- a/src/Visitor/Php/Symfony/FormTypePlaceholder.php +++ b/src/Visitor/Php/Symfony/FormTypePlaceholder.php @@ -28,7 +28,7 @@ public function enterNode(Node $node) // only Traverse *Type if ($node instanceof Stmt\Class_) { if (substr($node->name, -4) !== 'Type') { - return NodeTraverser::DONT_TRAVERSE_CHILDREN; + return; } } From 19dde6e525186c585be2a7f2eb5de196d1ad7bad Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sat, 12 Aug 2017 18:11:05 +0200 Subject: [PATCH 3/3] Applied changes from StyleCI --- src/Visitor/Php/Symfony/FormTypeChoices.php | 2 +- src/Visitor/Php/Symfony/FormTypeLabelExplicit.php | 1 - src/Visitor/Php/Symfony/FormTypeLabelImplicit.php | 1 - src/Visitor/Php/Symfony/FormTypePlaceholder.php | 1 - tests/Functional/Visitor/Php/BasePHPVisitorTest.php | 3 ++- 5 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Visitor/Php/Symfony/FormTypeChoices.php b/src/Visitor/Php/Symfony/FormTypeChoices.php index 89d2159..bb2682e 100644 --- a/src/Visitor/Php/Symfony/FormTypeChoices.php +++ b/src/Visitor/Php/Symfony/FormTypeChoices.php @@ -13,7 +13,6 @@ use PhpParser\Node; use PhpParser\Node\Stmt; -use PhpParser\NodeTraverser; use PhpParser\NodeVisitor; use Translation\Extractor\Model\SourceLocation; use Translation\Extractor\Visitor\Php\BasePHPVisitor; @@ -63,6 +62,7 @@ public function enterNode(Node $node) if ($item->key->value === 'choices_as_values') { $useKey = true; + continue; } diff --git a/src/Visitor/Php/Symfony/FormTypeLabelExplicit.php b/src/Visitor/Php/Symfony/FormTypeLabelExplicit.php index fe71719..0b503ca 100644 --- a/src/Visitor/Php/Symfony/FormTypeLabelExplicit.php +++ b/src/Visitor/Php/Symfony/FormTypeLabelExplicit.php @@ -13,7 +13,6 @@ use PhpParser\Node; use PhpParser\Node\Stmt; -use PhpParser\NodeTraverser; use PhpParser\NodeVisitor; use Translation\Extractor\Model\SourceLocation; use Translation\Extractor\Visitor\Php\BasePHPVisitor; diff --git a/src/Visitor/Php/Symfony/FormTypeLabelImplicit.php b/src/Visitor/Php/Symfony/FormTypeLabelImplicit.php index 9b63489..41319b9 100644 --- a/src/Visitor/Php/Symfony/FormTypeLabelImplicit.php +++ b/src/Visitor/Php/Symfony/FormTypeLabelImplicit.php @@ -13,7 +13,6 @@ use PhpParser\Node; use PhpParser\Node\Stmt; -use PhpParser\NodeTraverser; use PhpParser\NodeVisitor; use Translation\Extractor\Model\SourceLocation; use Translation\Extractor\Visitor\Php\BasePHPVisitor; diff --git a/src/Visitor/Php/Symfony/FormTypePlaceholder.php b/src/Visitor/Php/Symfony/FormTypePlaceholder.php index d92fbd3..47bd49e 100644 --- a/src/Visitor/Php/Symfony/FormTypePlaceholder.php +++ b/src/Visitor/Php/Symfony/FormTypePlaceholder.php @@ -13,7 +13,6 @@ use PhpParser\Node; use PhpParser\Node\Stmt; -use PhpParser\NodeTraverser; use PhpParser\NodeVisitor; use Translation\Extractor\Model\SourceLocation; use Translation\Extractor\Visitor\Php\BasePHPVisitor; diff --git a/tests/Functional/Visitor/Php/BasePHPVisitorTest.php b/tests/Functional/Visitor/Php/BasePHPVisitorTest.php index 370f27a..ed96787 100644 --- a/tests/Functional/Visitor/Php/BasePHPVisitorTest.php +++ b/tests/Functional/Visitor/Php/BasePHPVisitorTest.php @@ -33,7 +33,7 @@ protected function getSourceLocations($visitor, $namespaceForTestFile) $extractor = new PHPFileExtractor(); if (is_array($visitor)) { - foreach($visitor as $nodeVisitor) { + foreach ($visitor as $nodeVisitor) { $extractor->addVisitor($nodeVisitor); } } else { @@ -51,6 +51,7 @@ protected function getSourceLocations($visitor, $namespaceForTestFile) for ($j = $i; $j < count($fileNamespace); ++$j) { $path .= '/'.$fileNamespace[$j]; } + break; } }