Add missing for loop support in VariableNameRule (#187)

VincentLanglet · Feb 3, 2024 · 0d14e06 · 0d14e06
1 parent 0d207f4
commit 0d14e06
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 %}