fix: Process variable before the end of scope (#324)

* test: add test for variable use in short open echo When a variable is used in a short php open echo tag, it should be marked as used. * fix: process variable before the end of scope fixes #323 When the last php code is a short php open echo tag using a variable, process the variable before processing the end of scope.
sirbrillig · Jun 26, 2024 · bc8d7e3 · bc8d7e3
1 parent bb070b8
commit bc8d7e3
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 4 deletions.
diff --git a/Tests/VariableAnalysisSniff/ShortPhpTagsTest.php b/Tests/VariableAnalysisSniff/ShortPhpTagsTest.php
@@ -0,0 +1,31 @@
+<?php
+
+namespace VariableAnalysis\Tests\VariableAnalysisSniff;
+
+use VariableAnalysis\Tests\BaseTestCase;
+
+class ShortPhpTagsTest extends BaseTestCase
+{
+    public function testVariableWarningsWhenShortEchoTagsAreUsed()
+    {
+        $fixtureFile = $this->getFixture('ShortPhpTagsFixture.php');
+		$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
+		$phpcsFile->process();
+		$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+        $expectedWarnings = [
+            4,
+            7,
+        ];
+        $this->assertSame($expectedWarnings, $lines);
+    }
+
+    public function testVariableWarningsHaveCorrectSniffCodesWhenShortEchoTagsAreUsed()
+    {
+        $fixtureFile = $this->getFixture('ShortPhpTagsFixture.php');
+		$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
+		$phpcsFile->process();
+		$warnings = $phpcsFile->getWarnings();
+        $this->assertSame(self::UNUSED_ERROR_CODE, $warnings[4][1][0]['source']);
+        $this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[7][5][0]['source']);
+    }
+}
diff --git a/Tests/VariableAnalysisSniff/fixtures/ShortPhpTagsFixture.php b/Tests/VariableAnalysisSniff/fixtures/ShortPhpTagsFixture.php
@@ -0,0 +1,10 @@
+<?php
+$foo = 'hello';
+$usedLast = 'hello';
+$bar = 'bye'; // unused variable
+?>
+<html>
+<?= $baz ?> undefined variable
+<?= $foo ?>
+<?= $usedLast ?> used as last php statement without semicolon
+</html>
diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
@@ -245,6 +245,13 @@ public function process(File $phpcsFile, $stackPtr)
 			$this->scopeManager->recordScopeStartAndEnd($phpcsFile, 0);
 		}
 
+		// Find and process variables to perform two jobs: to record variable
+		// definition or use, and to report variables as undefined if they are used
+		// without having been first defined.
+		if ($token['code'] === T_VARIABLE) {
+			$this->processVariable($phpcsFile, $stackPtr);
+		}
+
 		// Report variables defined but not used in the current scope as unused
 		// variables if the current token closes scopes.
 		$this->searchForAndProcessClosingScopesAt($phpcsFile, $stackPtr);
@@ -253,13 +260,10 @@ public function process(File $phpcsFile, $stackPtr)
 		// expression of a for loop if the current token closes a loop.
 		$this->processClosingForLoopsAt($phpcsFile, $stackPtr);
 
-		// Find and process variables to perform two jobs: to record variable
-		// definition or use, and to report variables as undefined if they are used
-		// without having been first defined.
 		if ($token['code'] === T_VARIABLE) {
-			$this->processVariable($phpcsFile, $stackPtr);
 			return;
 		}
+
 		if (($token['code'] === T_DOUBLE_QUOTED_STRING) || ($token['code'] === T_HEREDOC)) {
 			$this->processVariableInString($phpcsFile, $stackPtr);
 			return;