Fix #197 Reset form type detection for each file and class

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');

src/Visitor/Php/Symfony/FormTrait.php

@@ -16,31 +16,60 @@
 use PhpParser\NodeTraverser;
 use PhpParser\NodeVisitor\NameResolver;
 use PhpParser\ParserFactory;
+use Symfony\Component\Finder\SplFileInfo;
+use Translation\Extractor\Model\SourceCollection;
 
 trait FormTrait
 {
-    private bool $isFormType = false;
+    /**
+     * Classes being traversed, innermost last: [end position in file, is form type].
+     *
+     * @var list<array{int, bool}>
+     */
+    private array $classStack = [];
     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->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);
         }
 
-        $allInterfaces = $this->getAllInterfacesFromNode($node);
+        if ($node instanceof Class_) {
+            $this->classStack[] = [$node->getEndFilePos(), $this->implementsFormInterface($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

tests/Functional/Visitor/Php/Symfony/IsFormTypeTest.php

@@ -41,4 +41,54 @@ 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());
+    }
+
+    /**
+     * 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);
+    }
 }

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()],
         ];
     }

tests/Resources/Php/Symfony/AnonymousClasses.php

@@ -0,0 +1,50 @@
+<?php
+
+namespace Translation\Extractor\Tests\Resources\Php\Symfony;
+
+use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\DataTransformerInterface;
+use Symfony\Component\Form\FormBuilderInterface;
+
+class AnonymousClasses extends AbstractType
+{
+    public function buildForm(FormBuilderInterface $builder, array $options): void
+    {
+        $builder->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;
+    }
+}

tests/Resources/Php/Symfony/FormTypeFollowedByClass.php

@@ -0,0 +1,26 @@
+<?php
+
+namespace Translation\Extractor\Tests\Resources\Php\Symfony;
+
+use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\FormBuilderInterface;
+
+class FormTypeFollowedByClass extends AbstractType
+{
+    public function buildForm(FormBuilderInterface $builder, array $options): void
+    {
+        $builder->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'
+        ]);
+    }
+}

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.');
     }
 
     /**