From 0d14e067da1237e00ad8378f5904aaaa992001e6 Mon Sep 17 00:00:00 2001
From: Vincent Langlet <VincentLanglet@users.noreply.github.com>
Date: Sat, 3 Feb 2024 15:58:04 +0100
Subject: [PATCH] Add missing for loop support in VariableNameRule (#187)

---
 src/Rules/Variable/VariableNameRule.php       | 26 ++++++++++++++-----
 tests/Report/ReporterFactoryTest.php          |  2 ++
 .../VariableName/VariableNameRuleTest.php     | 20 +++++++++-----
 .../VariableName/VariableNameRuleTest.twig    |  3 +++
 4 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/src/Rules/Variable/VariableNameRule.php b/src/Rules/Variable/VariableNameRule.php
index 8bb852c1..45c39326 100644
--- a/src/Rules/Variable/VariableNameRule.php
+++ b/src/Rules/Variable/VariableNameRule.php
@@ -38,14 +38,28 @@ protected function process(int $tokenPosition, array $tokens): void
     {
         $token = $tokens[$tokenPosition];
 
-        if (!$this->isTokenMatching($token, Token::BLOCK_NAME_TYPE, 'set')) {
-            return;
-        }
+        if ($this->isTokenMatching($token, Token::BLOCK_NAME_TYPE, 'set')) {
+            $nameTokenPosition = $this->findNext(Token::NAME_TYPE, $tokens, $tokenPosition);
+            Assert::notFalse($nameTokenPosition, 'A BLOCK_NAME_TYPE "set" must be followed by a name');
+
+            $this->validateVariable($tokens[$nameTokenPosition]);
+        } elseif ($this->isTokenMatching($token, Token::BLOCK_NAME_TYPE, 'for')) {
+            $nameTokenPosition = $this->findNext(Token::NAME_TYPE, $tokens, $tokenPosition);
+            Assert::notFalse($nameTokenPosition, 'A BLOCK_NAME_TYPE "for" must be followed by a name');
+
+            $secondNameTokenPosition = $this->findNext([Token::NAME_TYPE, Token::OPERATOR_TYPE], $tokens, $nameTokenPosition + 1);
+            Assert::notFalse($secondNameTokenPosition, 'A BLOCK_NAME_TYPE "for" must use the "in" operator');
 
-        $nameTokenPosition = $this->findNext(Token::NAME_TYPE, $tokens, $tokenPosition);
-        Assert::notFalse($nameTokenPosition, 'A BLOCK_NAME_TYPE set must be followed by a name');
-        $name = $tokens[$nameTokenPosition]->getValue();
+            $this->validateVariable($tokens[$nameTokenPosition]);
+            if ($this->isTokenMatching($tokens[$secondNameTokenPosition], Token::NAME_TYPE)) {
+                $this->validateVariable($tokens[$secondNameTokenPosition]);
+            }
+        }
+    }
 
+    private function validateVariable(Token $token): void
+    {
+        $name = $token->getValue();
         $expected = match ($this->case) {
             self::SNAKE_CASE => (new UnicodeString($name))->snake()->toString(),
             self::CAMEL_CASE => (new UnicodeString($name))->camel()->toString(),
diff --git a/tests/Report/ReporterFactoryTest.php b/tests/Report/ReporterFactoryTest.php
index b150314c..2b04730c 100644
--- a/tests/Report/ReporterFactoryTest.php
+++ b/tests/Report/ReporterFactoryTest.php
@@ -6,6 +6,7 @@
 
 use PHPUnit\Framework\TestCase;
 use TwigCsFixer\Report\Reporter\CheckstyleReporter;
+use TwigCsFixer\Report\Reporter\GithubReporter;
 use TwigCsFixer\Report\Reporter\JUnitReporter;
 use TwigCsFixer\Report\Reporter\NullReporter;
 use TwigCsFixer\Report\Reporter\TextReporter;
@@ -22,6 +23,7 @@ public function testGetReporter(): void
         static::assertInstanceOf(NullReporter::class, $reporterFactory->getReporter(NullReporter::NAME));
         static::assertInstanceOf(CheckstyleReporter::class, $reporterFactory->getReporter(CheckstyleReporter::NAME));
         static::assertInstanceOf(JUnitReporter::class, $reporterFactory->getReporter(JUnitReporter::NAME));
+        static::assertInstanceOf(GithubReporter::class, $reporterFactory->getReporter(GithubReporter::NAME));
     }
 
     public function testGetMessageForAnotherFile(): void
diff --git a/tests/Rules/Variable/VariableName/VariableNameRuleTest.php b/tests/Rules/Variable/VariableName/VariableNameRuleTest.php
index 68d68020..b682b77a 100644
--- a/tests/Rules/Variable/VariableName/VariableNameRuleTest.php
+++ b/tests/Rules/Variable/VariableName/VariableNameRuleTest.php
@@ -25,25 +25,31 @@ public function testConfiguration(): void
     public function testRule(): void
     {
         $this->checkRule(new VariableNameRule(), [
-            'VariableName.Error:2:4',
-            'VariableName.Error:4:4',
+            'VariableName.Error:2:8',
+            'VariableName.Error:4:8',
+            'VariableName.Error:6:8',
+            'VariableName.Error:7:8',
+            'VariableName.Error:7:16',
         ]);
     }
 
     public function testRuleCamelCase(): void
     {
         $this->checkRule(new VariableNameRule(VariableNameRule::CAMEL_CASE), [
-            'VariableName.Error:3:4',
-            'VariableName.Error:4:4',
+            'VariableName.Error:3:8',
+            'VariableName.Error:4:8',
         ]);
     }
 
     public function testRulePascalCase(): void
     {
         $this->checkRule(new VariableNameRule(VariableNameRule::PASCAL_CASE), [
-            'VariableName.Error:1:4',
-            'VariableName.Error:2:4',
-            'VariableName.Error:3:4',
+            'VariableName.Error:1:8',
+            'VariableName.Error:2:8',
+            'VariableName.Error:3:8',
+            'VariableName.Error:6:8',
+            'VariableName.Error:7:8',
+            'VariableName.Error:7:16',
         ]);
     }
 }
diff --git a/tests/Rules/Variable/VariableName/VariableNameRuleTest.twig b/tests/Rules/Variable/VariableName/VariableNameRuleTest.twig
index 38bf67a2..6c68fdd7 100644
--- a/tests/Rules/Variable/VariableName/VariableNameRuleTest.twig
+++ b/tests/Rules/Variable/VariableName/VariableNameRuleTest.twig
@@ -2,3 +2,6 @@
 {% set fooBar = 'foo' %}
 {% set foo_bar = 'foo' %}
 {% set FooBar = 'foo' %}
+
+{% for userFoo in users %}{% endfor %}
+{% for keyFoo, userFoo in users %}{% endfor %}