From 97e66cf1f94efb196b9ac3d105001ae94dc38cac Mon Sep 17 00:00:00 2001 From: Guillaume Luchet Date: Thu, 29 Oct 2026 15:45:28 +0100 Subject: [PATCH 1/2] Fix #197 Reset form type detection for each file and class --- src/Visitor/BaseVisitor.php | 3 +- src/Visitor/Php/Symfony/FormTrait.php | 16 +++++++++++ .../Visitor/Php/Symfony/IsFormTypeTest.php | 28 +++++++++++++++++++ tests/Resources/Github/Issue_71.php | 4 +-- .../Php/Symfony/FormTypeFollowedByClass.php | 26 +++++++++++++++++ tests/Smoke/AllExtractorsTest.php | 2 +- 6 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 tests/Resources/Php/Symfony/FormTypeFollowedByClass.php diff --git a/src/Visitor/BaseVisitor.php b/src/Visitor/BaseVisitor.php index 8ad0102..40ca1ad 100644 --- a/src/Visitor/BaseVisitor.php +++ b/src/Visitor/BaseVisitor.php @@ -54,7 +54,8 @@ protected function addError(Node $node, string $errorMessage): void $docComment = $node->getDocComment(); $file = $this->getAbsoluteFilePath(); - if (property_exists($node, 'value')) { + // Scalar nodes also have a "value" property, holding the raw value + if (property_exists($node, 'value') && $node->value instanceof Node) { $line = $node->value->getAttribute('startLine'); } else { $line = $node->getAttribute('startLine'); diff --git a/src/Visitor/Php/Symfony/FormTrait.php b/src/Visitor/Php/Symfony/FormTrait.php index 546c4ce..e26b83d 100644 --- a/src/Visitor/Php/Symfony/FormTrait.php +++ b/src/Visitor/Php/Symfony/FormTrait.php @@ -16,6 +16,8 @@ use PhpParser\NodeTraverser; use PhpParser\NodeVisitor\NameResolver; use PhpParser\ParserFactory; +use Symfony\Component\Finder\SplFileInfo; +use Translation\Extractor\Model\SourceCollection; trait FormTrait { @@ -23,12 +25,26 @@ trait FormTrait private array $classMap = []; private array $symfonyInterfaces = ['FormTypeInterface', 'FormTypeExtensionInterface']; + /** + * The visitor is reused for every file: the form type detection of the + * previous file must not leak to the next one. + */ + public function init(SourceCollection $collection, SplFileInfo $file): void + { + $this->isFormType = false; + + parent::init($collection, $file); + } + protected function isFormType(Node $node): bool { if (!$node instanceof Class_) { return $this->isFormType; } + // Each class must be checked on its own + $this->isFormType = false; + $allInterfaces = $this->getAllInterfacesFromNode($node); foreach ($allInterfaces as $interface) { diff --git a/tests/Functional/Visitor/Php/Symfony/IsFormTypeTest.php b/tests/Functional/Visitor/Php/Symfony/IsFormTypeTest.php index 7e8695d..e9ad969 100644 --- a/tests/Functional/Visitor/Php/Symfony/IsFormTypeTest.php +++ b/tests/Functional/Visitor/Php/Symfony/IsFormTypeTest.php @@ -41,4 +41,32 @@ public function testNonFormTypeDoesNotExtractTranslations(): void $this->assertCount(0, $collection); } + + /** + * @see https://github.com/php-translation/extractor/issues/197 + */ + public function testFormTypeDetectionDoesNotLeakToNextFile(): void + { + $visitor = new FormTypeLabelExplicit(); + + $collection = $this->getSourceLocations($visitor, Resources\Php\Symfony\ParentType::class); + $this->assertCount(1, $collection); + + $collection = $this->getSourceLocations($visitor, Resources\Php\Symfony\NotAFormType::class); + $this->assertCount(0, $collection); + } + + /** + * @see https://github.com/php-translation/extractor/issues/197 + */ + public function testFormTypeDetectionDoesNotLeakToNextClass(): void + { + $collection = $this->getSourceLocations( + new FormTypeLabelExplicit(), + Resources\Php\Symfony\FormTypeFollowedByClass::class + ); + + $this->assertCount(1, $collection); + $this->assertEquals('form.field.label', $collection->first()->getMessage()); + } } diff --git a/tests/Resources/Github/Issue_71.php b/tests/Resources/Github/Issue_71.php index 905e743..8d19f63 100644 --- a/tests/Resources/Github/Issue_71.php +++ b/tests/Resources/Github/Issue_71.php @@ -11,10 +11,10 @@ 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 + ['label' => $this->get('translator')->trans('this.should.be.translated.without.errors')], [/** @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 + ['label' => $this->shouldBeIgnored()], [/** @Ignore */'label' => $this->shouldBeIgnoredAndGoesWithoutError()], ]; } diff --git a/tests/Resources/Php/Symfony/FormTypeFollowedByClass.php b/tests/Resources/Php/Symfony/FormTypeFollowedByClass.php new file mode 100644 index 0000000..218ca3e --- /dev/null +++ b/tests/Resources/Php/Symfony/FormTypeFollowedByClass.php @@ -0,0 +1,26 @@ +add('field', null, [ + 'label' => 'form.field.label' + ]); + } +} + +class ClassFollowingFormType +{ + public function buildForm(FormBuilderInterface $builder, array $options): void + { + $builder->add('field', null, [ + 'label' => 'should.not.be.extracted' + ]); + } +} diff --git a/tests/Smoke/AllExtractorsTest.php b/tests/Smoke/AllExtractorsTest.php index a2b82bc..e4c6dac 100644 --- a/tests/Smoke/AllExtractorsTest.php +++ b/tests/Smoke/AllExtractorsTest.php @@ -94,7 +94,7 @@ public function testNoException(): void * 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(15, $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.'); } /** From cbd05c96fccbd05617bfda7ba3516871911e42a6 Mon Sep 17 00:00:00 2001 From: Guillaume Luchet Date: Fri, 30 Oct 2026 17:44:53 +0100 Subject: [PATCH 2/2] Fix #197 Restore form type detection after anonymous classes Resetting the detection on every Class_ node broke form types containing an anonymous class (e.g. a data transformer): the fields declared after it were no longer extracted. Track the enclosing classes in a stack keyed on their end position instead, as the visitors define their own leaveNode(). Co-Authored-By: Claude Opus 5.5 --- src/Visitor/Php/Symfony/FormTrait.php | 35 +++++++++---- .../Visitor/Php/Symfony/IsFormTypeTest.php | 22 ++++++++ .../Php/Symfony/AnonymousClasses.php | 50 +++++++++++++++++++ 3 files changed, 96 insertions(+), 11 deletions(-) create mode 100644 tests/Resources/Php/Symfony/AnonymousClasses.php diff --git a/src/Visitor/Php/Symfony/FormTrait.php b/src/Visitor/Php/Symfony/FormTrait.php index e26b83d..f5c998f 100644 --- a/src/Visitor/Php/Symfony/FormTrait.php +++ b/src/Visitor/Php/Symfony/FormTrait.php @@ -21,7 +21,12 @@ trait FormTrait { - private bool $isFormType = false; + /** + * Classes being traversed, innermost last: [end position in file, is form type]. + * + * @var list + */ + private array $classStack = []; private array $classMap = []; private array $symfonyInterfaces = ['FormTypeInterface', 'FormTypeExtensionInterface']; @@ -31,32 +36,40 @@ trait FormTrait */ public function init(SourceCollection $collection, SplFileInfo $file): void { - $this->isFormType = false; + $this->classStack = []; parent::init($collection, $file); } + /** + * Must be called for every node, in traversal order. The enclosing classes + * are tracked by position because the visitors define their own leaveNode(). + */ protected function isFormType(Node $node): bool { - if (!$node instanceof Class_) { - return $this->isFormType; + // Leave the classes (named or anonymous) that end before this node + while ($this->classStack && end($this->classStack)[0] < $node->getStartFilePos()) { + array_pop($this->classStack); } - // Each class must be checked on its own - $this->isFormType = false; + if ($node instanceof Class_) { + $this->classStack[] = [$node->getEndFilePos(), $this->implementsFormInterface($node)]; + } - $allInterfaces = $this->getAllInterfacesFromNode($node); + return $this->classStack ? end($this->classStack)[1] : false; + } - foreach ($allInterfaces as $interface) { + private function implementsFormInterface(Class_ $node): bool + { + foreach ($this->getAllInterfacesFromNode($node) as $interface) { foreach ($this->symfonyInterfaces as $symfonyInterface) { if ($interface === $symfonyInterface || str_ends_with($interface, '\\'.$symfonyInterface)) { - $this->isFormType = true; - break; + return true; } } } - return $this->isFormType; + return false; } protected function getAllInterfacesFromNode(Class_ $node): array diff --git a/tests/Functional/Visitor/Php/Symfony/IsFormTypeTest.php b/tests/Functional/Visitor/Php/Symfony/IsFormTypeTest.php index e9ad969..7171db1 100644 --- a/tests/Functional/Visitor/Php/Symfony/IsFormTypeTest.php +++ b/tests/Functional/Visitor/Php/Symfony/IsFormTypeTest.php @@ -69,4 +69,26 @@ public function testFormTypeDetectionDoesNotLeakToNextClass(): void $this->assertCount(1, $collection); $this->assertEquals('form.field.label', $collection->first()->getMessage()); } + + /** + * The form type detection is restored once an anonymous class ends, in both directions. + */ + public function testAnonymousClasses(): void + { + $collection = $this->getSourceLocations( + new FormTypeLabelExplicit(), + Resources\Php\Symfony\AnonymousClasses::class + ); + + $messages = []; + foreach ($collection as $source) { + $messages[] = $source->getMessage(); + } + + $this->assertEquals([ + 'before.anonymous.class', + 'after.anonymous.class', + 'inside.anonymous.form.type', + ], $messages); + } } diff --git a/tests/Resources/Php/Symfony/AnonymousClasses.php b/tests/Resources/Php/Symfony/AnonymousClasses.php new file mode 100644 index 0000000..9e5f8ab --- /dev/null +++ b/tests/Resources/Php/Symfony/AnonymousClasses.php @@ -0,0 +1,50 @@ +add('before', null, [ + 'label' => 'before.anonymous.class' + ]); + $builder->get('before')->addModelTransformer(new class implements DataTransformerInterface { + public function transform($value): mixed + { + return ['label' => 'inside.anonymous.class']; + } + + public function reverseTransform($value): mixed + { + return $value; + } + }); + $builder->add('after', null, [ + 'label' => 'after.anonymous.class' + ]); + } +} + +class ClassWithAnonymousFormType +{ + public function createType(): AbstractType + { + $type = new class extends AbstractType { + public function buildForm(FormBuilderInterface $builder, array $options): void + { + $builder->add('field', null, [ + 'label' => 'inside.anonymous.form.type' + ]); + } + }; + + $notAForm = ['label' => 'after.anonymous.form.type']; + + return $type; + } +}