From 6dbc0f711cb50620a1fca147496aff4d15b8dddc Mon Sep 17 00:00:00 2001 From: Guite Date: Wed, 22 Jan 2020 18:26:00 +0100 Subject: [PATCH 1/4] added extractor for form field titles --- src/Visitor/Php/Symfony/FormTypeTitle.php | 78 +++++++++++++++++++ .../Visitor/Php/Symfony/FormTypeLabelTest.php | 2 + .../Visitor/Php/Symfony/FormTypeTitleTest.php | 62 +++++++++++++++ .../Php/Symfony/TitleFormErrorType.php | 24 ++++++ tests/Resources/Php/Symfony/TitleFormType.php | 23 ++++++ tests/Smoke/AllExtractorsTest.php | 4 +- 6 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 src/Visitor/Php/Symfony/FormTypeTitle.php create mode 100644 tests/Functional/Visitor/Php/Symfony/FormTypeTitleTest.php create mode 100644 tests/Resources/Php/Symfony/TitleFormErrorType.php create mode 100644 tests/Resources/Php/Symfony/TitleFormType.php diff --git a/src/Visitor/Php/Symfony/FormTypeTitle.php b/src/Visitor/Php/Symfony/FormTypeTitle.php new file mode 100644 index 0000000..73d27bb --- /dev/null +++ b/src/Visitor/Php/Symfony/FormTypeTitle.php @@ -0,0 +1,78 @@ + + * + * 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\NodeVisitor; + +/** + * @author Tobias Nyholm + */ +final class FormTypeTitle extends AbstractFormType implements NodeVisitor +{ + use FormTrait; + + /** + * {@inheritdoc} + */ + public function enterNode(Node $node): ?Node + { + if (!$this->isFormType($node)) { + return null; + } + + parent::enterNode($node); + + if (!$node instanceof Node\Expr\Array_) { + return null; + } + + $titleNode = null; + $domain = null; + foreach ($node->items as $item) { + if (!$item->key instanceof Node\Scalar\String_) { + continue; + } + if ('translation_domain' === $item->key->value) { + // Try to find translation domain + if ($item->value instanceof Node\Scalar\String_) { + $domain = $item->value->value; + } elseif ($item->value instanceof Node\Expr\ConstFetch && 'false' === $item->value->name->toString()) { + $domain = false; + } + } elseif ('attr' === $item->key->value && $item->value instanceof Node\Expr\Array_) { + foreach ($item->value->items as $attrValue) { + if ('title' === $attrValue->key->value) { + $titleNode = $attrValue; + + break; + } + } + } + } + + if (null === $titleNode) { + return null; + } + + if ($titleNode->value instanceof Node\Scalar\String_) { + $line = $titleNode->value->getAttribute('startLine'); + if (null !== $location = $this->getLocation($titleNode->value->value, $line, $titleNode, ['domain' => $domain])) { + $this->lateCollect($location); + } + } else { + $this->addError($titleNode, 'Form field title is not a scalar string'); + } + + return null; + } +} diff --git a/tests/Functional/Visitor/Php/Symfony/FormTypeLabelTest.php b/tests/Functional/Visitor/Php/Symfony/FormTypeLabelTest.php index 1f1997f..5f19419 100644 --- a/tests/Functional/Visitor/Php/Symfony/FormTypeLabelTest.php +++ b/tests/Functional/Visitor/Php/Symfony/FormTypeLabelTest.php @@ -19,6 +19,7 @@ use Translation\Extractor\Visitor\Php\Symfony\FormTypeLabelExplicit; use Translation\Extractor\Visitor\Php\Symfony\FormTypeLabelImplicit; use Translation\Extractor\Visitor\Php\Symfony\FormTypePlaceholder; +use Translation\Extractor\Visitor\Php\Symfony\FormTypeTitle; /** * @author Rein Baarsma @@ -36,6 +37,7 @@ public function __construct() new FormTypeLabelExplicit(), new FormTypeLabelImplicit(), new FormTypePlaceholder(), + new FormTypeTitle(), ]; parent::__construct(); diff --git a/tests/Functional/Visitor/Php/Symfony/FormTypeTitleTest.php b/tests/Functional/Visitor/Php/Symfony/FormTypeTitleTest.php new file mode 100644 index 0000000..4d5e2b7 --- /dev/null +++ b/tests/Functional/Visitor/Php/Symfony/FormTypeTitleTest.php @@ -0,0 +1,62 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Translation\Extractor\Tests\Functional\Visitor\Php\Symfony; + +use Translation\Extractor\Tests\Functional\Visitor\Php\BasePHPVisitorTest; +use Translation\Extractor\Tests\Resources; +use Translation\Extractor\Visitor\Php\Symfony\ContainerAwareTrans; +use Translation\Extractor\Visitor\Php\Symfony\FormTypeTitle; + +/** + * @author Tobias Nyholm + */ +final class FormTypeTitleTest extends BasePHPVisitorTest +{ + public function testExtract() + { + $collection = $this->getSourceLocations(new FormTypeTitle(), + Resources\Php\Symfony\TitleFormType::class); + + $this->assertCount(2, $collection); + $this->assertEquals('form.title.text', $collection->get(0)->getMessage()); + $this->assertEquals('form.title.text.but.no.label', $collection->get(1)->getMessage()); + } + + public function testExtractError() + { + $collection = $this->getSourceLocations(new FormTypeTitle(), + Resources\Php\Symfony\TitleFormErrorType::class); + + $errors = $collection->getErrors(); + $this->assertCount(2, $errors); + } + + public function testChildVisitationNotBlocked() + { + $collection = $this->getSourceLocations( + [ + new FormTypeTitle(), + new ContainerAwareTrans(), + ], + Resources\Php\Symfony\ContainerAwareTrans::class + ); + + $this->assertCount(6, $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()); + $this->assertEquals('my.pdf', $collection->get(4)->getMessage()); + $this->assertEquals('bar', $collection->get(5)->getMessage()); + } +} diff --git a/tests/Resources/Php/Symfony/TitleFormErrorType.php b/tests/Resources/Php/Symfony/TitleFormErrorType.php new file mode 100644 index 0000000..8088304 --- /dev/null +++ b/tests/Resources/Php/Symfony/TitleFormErrorType.php @@ -0,0 +1,24 @@ +add('field_with_attr_title', 'text', array( + 'label' => 'field.with.title', + 'attr' => array('title' => $string) + )) + ->add('field_without_label_with_attr_title', 'text', array( + 'label' => false, + 'attr' => array('title' => $string) + )) + ; + } +} diff --git a/tests/Resources/Php/Symfony/TitleFormType.php b/tests/Resources/Php/Symfony/TitleFormType.php new file mode 100644 index 0000000..80e7395 --- /dev/null +++ b/tests/Resources/Php/Symfony/TitleFormType.php @@ -0,0 +1,23 @@ +add('field_with_attr_title', 'text', array( + 'label' => 'field.with.title', + 'attr' => array('title' => 'form.title.text') + )) + ->add('field_without_label_with_attr_title', 'text', array( + 'label' => false, + 'attr' => array('title' => 'form.title.text.but.no.label') + )) + ; + } +} diff --git a/tests/Smoke/AllExtractorsTest.php b/tests/Smoke/AllExtractorsTest.php index 986ddcc..ae8e7a7 100644 --- a/tests/Smoke/AllExtractorsTest.php +++ b/tests/Smoke/AllExtractorsTest.php @@ -30,6 +30,7 @@ use Translation\Extractor\Visitor\Php\Symfony\FormTypeLabelExplicit; use Translation\Extractor\Visitor\Php\Symfony\FormTypeLabelImplicit; use Translation\Extractor\Visitor\Php\Symfony\FormTypePlaceholder; +use Translation\Extractor\Visitor\Php\Symfony\FormTypeTitle; use Translation\Extractor\Visitor\Php\TranslateAnnotationVisitor; use Translation\Extractor\Visitor\Twig\TwigVisitor; @@ -89,7 +90,7 @@ public function testNoException() * 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(12, $sc->getErrors(), 'There was an unexpected number of errors. Please investigate.'); + $this->assertCount(14, $sc->getErrors(), 'There was an unexpected number of errors. Please investigate.'); } /** @@ -149,6 +150,7 @@ private function getPHPFileExtractor(): PHPFileExtractor $file->addVisitor(new FormTypeLabelExplicit()); $file->addVisitor(new FormTypeLabelImplicit()); $file->addVisitor(new FormTypePlaceholder()); + $file->addVisitor(new FormTypeTitle()); $file->addVisitor(new SourceLocationContainerVisitor()); $file->addVisitor(new TranslateAnnotationVisitor()); From bac75be6cf9340fa27885e392f4f03b78be767ca Mon Sep 17 00:00:00 2001 From: Guite Date: Wed, 22 Jan 2020 18:32:30 +0100 Subject: [PATCH 2/4] added safety check --- src/Visitor/Php/Symfony/FormTypePlaceholder.php | 3 +++ src/Visitor/Php/Symfony/FormTypeTitle.php | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/Visitor/Php/Symfony/FormTypePlaceholder.php b/src/Visitor/Php/Symfony/FormTypePlaceholder.php index 58998d6..2311baf 100644 --- a/src/Visitor/Php/Symfony/FormTypePlaceholder.php +++ b/src/Visitor/Php/Symfony/FormTypePlaceholder.php @@ -55,6 +55,9 @@ public function enterNode(Node $node): ?Node $placeholderNode = $item; } elseif ('attr' === $item->key->value && $item->value instanceof Node\Expr\Array_) { foreach ($item->value->items as $attrValue) { + if (!$attrValue->key instanceof Node\Scalar\String_) { + continue; + } if ('placeholder' === $attrValue->key->value) { $placeholderNode = $attrValue; diff --git a/src/Visitor/Php/Symfony/FormTypeTitle.php b/src/Visitor/Php/Symfony/FormTypeTitle.php index 73d27bb..571f0af 100644 --- a/src/Visitor/Php/Symfony/FormTypeTitle.php +++ b/src/Visitor/Php/Symfony/FormTypeTitle.php @@ -51,6 +51,9 @@ public function enterNode(Node $node): ?Node } } elseif ('attr' === $item->key->value && $item->value instanceof Node\Expr\Array_) { foreach ($item->value->items as $attrValue) { + if (!$attrValue->key instanceof Node\Scalar\String_) { + continue; + } if ('title' === $attrValue->key->value) { $titleNode = $attrValue; From 7a9fe381b88c6904a315404d1e0096f4b0b93e4b Mon Sep 17 00:00:00 2001 From: Guite Date: Wed, 22 Jan 2020 18:34:49 +0100 Subject: [PATCH 3/4] do not ignore pattern anymore --- phpstan-baseline.neon | 5 ----- 1 file changed, 5 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index bf649e6..4def910 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -14,8 +14,3 @@ parameters: message: "#^Access to an undefined property PhpParser\\\\Node\\:\\:\\$args\\.$#" count: 1 path: src/Visitor/Php/Symfony/AbstractFormType.php - - - - message: "#^Access to an undefined property PhpParser\\\\Node\\\\Expr\\:\\:\\$value\\.$#" - count: 1 - path: src/Visitor/Php/Symfony/FormTypePlaceholder.php From 48d1c9bb54eb6edfa05230bcc2495c150d3ef565 Mon Sep 17 00:00:00 2001 From: Guite Date: Wed, 22 Jan 2020 18:37:24 +0100 Subject: [PATCH 4/4] simplified test --- tests/Functional/Visitor/Php/Symfony/FormTypeTitleTest.php | 5 ++--- tests/Resources/Php/Symfony/TitleFormErrorType.php | 4 ---- tests/Resources/Php/Symfony/TitleFormType.php | 4 ---- tests/Smoke/AllExtractorsTest.php | 2 +- 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/tests/Functional/Visitor/Php/Symfony/FormTypeTitleTest.php b/tests/Functional/Visitor/Php/Symfony/FormTypeTitleTest.php index 4d5e2b7..c69d416 100644 --- a/tests/Functional/Visitor/Php/Symfony/FormTypeTitleTest.php +++ b/tests/Functional/Visitor/Php/Symfony/FormTypeTitleTest.php @@ -26,9 +26,8 @@ public function testExtract() $collection = $this->getSourceLocations(new FormTypeTitle(), Resources\Php\Symfony\TitleFormType::class); - $this->assertCount(2, $collection); + $this->assertCount(1, $collection); $this->assertEquals('form.title.text', $collection->get(0)->getMessage()); - $this->assertEquals('form.title.text.but.no.label', $collection->get(1)->getMessage()); } public function testExtractError() @@ -37,7 +36,7 @@ public function testExtractError() Resources\Php\Symfony\TitleFormErrorType::class); $errors = $collection->getErrors(); - $this->assertCount(2, $errors); + $this->assertCount(1, $errors); } public function testChildVisitationNotBlocked() diff --git a/tests/Resources/Php/Symfony/TitleFormErrorType.php b/tests/Resources/Php/Symfony/TitleFormErrorType.php index 8088304..f2327d7 100644 --- a/tests/Resources/Php/Symfony/TitleFormErrorType.php +++ b/tests/Resources/Php/Symfony/TitleFormErrorType.php @@ -15,10 +15,6 @@ public function buildForm(FormBuilderInterface $builder, array $options) 'label' => 'field.with.title', 'attr' => array('title' => $string) )) - ->add('field_without_label_with_attr_title', 'text', array( - 'label' => false, - 'attr' => array('title' => $string) - )) ; } } diff --git a/tests/Resources/Php/Symfony/TitleFormType.php b/tests/Resources/Php/Symfony/TitleFormType.php index 80e7395..c760c8a 100644 --- a/tests/Resources/Php/Symfony/TitleFormType.php +++ b/tests/Resources/Php/Symfony/TitleFormType.php @@ -14,10 +14,6 @@ public function buildForm(FormBuilderInterface $builder, array $options) 'label' => 'field.with.title', 'attr' => array('title' => 'form.title.text') )) - ->add('field_without_label_with_attr_title', 'text', array( - 'label' => false, - 'attr' => array('title' => 'form.title.text.but.no.label') - )) ; } } diff --git a/tests/Smoke/AllExtractorsTest.php b/tests/Smoke/AllExtractorsTest.php index ae8e7a7..fc565a7 100644 --- a/tests/Smoke/AllExtractorsTest.php +++ b/tests/Smoke/AllExtractorsTest.php @@ -90,7 +90,7 @@ public function testNoException() * 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(14, $sc->getErrors(), 'There was an unexpected number of errors. Please investigate.'); + $this->assertCount(13, $sc->getErrors(), 'There was an unexpected number of errors. Please investigate.'); } /**