Skip to content

Commit 5620b43

Browse files
sirbrilligsirbrillig
authored
Support global scope (#190)
* Fix indentation in VariableAnalysisTest * Add global scope tests * Add global scope to processScopeClose * Fix AnonymousClassWithPropertiesFixture test * Add getScopeCloseForScopeOpen helper * Remove findWhereAssignExecuted * Fix ThisWithinNestedClosedScopeFixture test * Enable test for foreach in global scope * Disable allowUnusedForeachVariables in FunctionWithForeachFixture test * Allow a token to close more than one scope * Fix FunctionWithGlobalVarFixture test
1 parent 275cb81 commit 5620b43

File tree

7 files changed

+104
-73
lines changed

7 files changed

+104
-73
lines changed

‎Tests/VariableAnalysisSniff/GlobalScopeTest.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
namespace VariableAnalysis\Tests\VariableAnalysisSniff;
3+
4+
use VariableAnalysis\Tests\BaseTestCase;
5+
6+
class GlobalScopeTest extends BaseTestCase {
7+
public function testGlobalScopeWarnings() {
8+
$fixtureFile = $this->getFixture('GlobalScopeFixture.php');
9+
$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
10+
$phpcsFile->process();
11+
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
12+
$expectedErrors = [
13+
4,
14+
7,
15+
];
16+
$this->assertEquals($expectedErrors, $lines);
17+
}
18+
}

‎Tests/VariableAnalysisSniff/VariableAnalysisTest.php‎

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,11 @@ public function testFunctionWithForeachErrors() {
114114
public function testFunctionWithForeachWarnings() {
115115
$fixtureFile = $this->getFixture('FunctionWithForeachFixture.php');
116116
$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
117+
$phpcsFile->ruleset->setSniffProperty(
118+
'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
119+
'allowUnusedForeachVariables',
120+
'false'
121+
);
117122
$phpcsFile->process();
118123
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
119124
$expectedWarnings = [
@@ -129,10 +134,7 @@ public function testFunctionWithForeachWarnings() {
129134
50,
130135
52,
131136
54,
132-
// FIXME: this is an unused variable that needs to be fixed but for now
133-
// we will ignore it. See
134-
// https://github.com/sirbrillig/phpcs-variable-analysis/pull/36
135-
// 67,
137+
67,
136138
];
137139
$this->assertEquals($expectedWarnings, $lines);
138140
}
@@ -585,6 +587,7 @@ public function testAnonymousClassAllowPropertyDefinitions() {
585587
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
586588
$expectedWarnings = [
587589
17,
590+
26,
588591
38,
589592
];
590593
$this->assertEquals($expectedWarnings, $lines);
@@ -767,7 +770,7 @@ public function testValidUndefinedVariableNamesIgnoresVarsInGlobalScope() {
767770
7,
768771
23,
769772
39,
770-
54,
773+
54,
771774
];
772775
$this->assertEquals($expectedWarnings, $lines);
773776
}
@@ -939,10 +942,10 @@ public function testGetDefinedVarsCountsAsRead() {
939942
$phpcsFile->process();
940943
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
941944
$expectedWarnings = [
942-
6,
943-
18,
944-
22,
945-
29,
945+
6,
946+
18,
947+
22,
948+
29,
946949
];
947950
$this->assertEquals($expectedWarnings, $lines);
948951
}
@@ -953,12 +956,14 @@ public function testThisWithinNestedClosedScope() {
953956
$phpcsFile->process();
954957
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
955958
$expectedWarnings = [
956-
5,
957-
8,
958-
20,
959-
33,
960-
47,
961-
61,
959+
5,
960+
8,
961+
15,
962+
20,
963+
33,
964+
41,
965+
47,
966+
61,
962967
];
963968
$this->assertEquals($expectedWarnings, $lines);
964969
}
@@ -969,12 +974,12 @@ public function testUnusedVarWithValueChange() {
969974
$phpcsFile->process();
970975
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
971976
$expectedWarnings = [
972-
5,
973-
6,
974-
8,
975-
9,
976-
11,
977-
12,
977+
5,
978+
6,
979+
8,
980+
9,
981+
11,
982+
12,
978983
];
979984
$this->assertEquals($expectedWarnings, $lines);
980985
}

‎Tests/VariableAnalysisSniff/fixtures/AnonymousClassWithPropertiesFixture.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public function methodWithStaticVar() {
2323
}
2424
}
2525

26-
$anonClass = new class() {
26+
$anonClass = new class() { // should trigger unused warning
2727
protected $storedHello;
2828
private static $storedHello2;
2929
private $storedHello3;

‎Tests/VariableAnalysisSniff/fixtures/FunctionWithGlobalVarFixture.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,5 @@ function globalWithUnusedFunctionArg($user_type, $text, $testvar) { // should wa
5555
global $config;
5656
return $config . $text . $user_type;
5757
}
58+
59+
echo $sunday;

‎Tests/VariableAnalysisSniff/fixtures/GlobalScopeFixture.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
$name = 'friend';
4+
$place = 'faerie'; // unused variable $place
5+
6+
echo $name;
7+
echo $activity; // undefined variable $activity

‎VariableAnalysis/Lib/Helpers.php‎

Lines changed: 37 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -237,41 +237,6 @@ public static function findFunctionCallArguments(File $phpcsFile, $stackPtr) {
237237
return $argPtrs;
238238
}
239239

240-
/**
241-
* @param File $phpcsFile
242-
* @param int $stackPtr
243-
*
244-
* @return int
245-
*/
246-
public static function findWhereAssignExecuted(File $phpcsFile, $stackPtr) {
247-
$tokens = $phpcsFile->getTokens();
248-
249-
// Write should be recorded at the next statement to ensure we treat the
250-
// assign as happening after the RHS execution.
251-
// eg: $var = $var + 1; -> RHS could still be undef.
252-
// However, if we're within a bracketed expression, we take place at the
253-
// closing bracket, if that's first.
254-
// eg: echo (($var = 12) && ($var == 12));
255-
$semicolonPtr = $phpcsFile->findNext([T_SEMICOLON], $stackPtr + 1, null, false, null, true);
256-
$commaPtr = $phpcsFile->findNext([T_COMMA], $stackPtr + 1, null, false, null, true);
257-
$closePtr = false;
258-
$openPtr = Helpers::findContainingOpeningBracket($phpcsFile, $stackPtr);
259-
if ($openPtr !== null) {
260-
if (isset($tokens[$openPtr]['parenthesis_closer'])) {
261-
$closePtr = $tokens[$openPtr]['parenthesis_closer'];
262-
}
263-
}
264-
265-
// Return the first thing: comma, semicolon, close-bracket, or stackPtr if nothing else
266-
$assignEndTokens = [$commaPtr, $semicolonPtr, $closePtr];
267-
$assignEndTokens = array_filter($assignEndTokens); // remove false values
268-
sort($assignEndTokens);
269-
if (empty($assignEndTokens)) {
270-
return $stackPtr;
271-
}
272-
return $assignEndTokens[0];
273-
}
274-
275240
/**
276241
* @param File $phpcsFile
277242
* @param int $stackPtr
@@ -649,4 +614,41 @@ public static function getAttachedBlockIndicesForElse(File $phpcsFile, $stackPtr
649614
public static function isIndexInsideScope($needle, $scopeStart, $scopeEnd) {
650615
return ($needle > $scopeStart && $needle < $scopeEnd);
651616
}
617+
618+
/**
619+
* @param File $phpcsFile
620+
* @param int $scopeStartIndex
621+
*
622+
* @return int
623+
*/
624+
public static function getScopeCloseForScopeOpen(File $phpcsFile, $scopeStartIndex) {
625+
$tokens = $phpcsFile->getTokens();
626+
$scopeCloserIndex = isset($tokens[$scopeStartIndex]['scope_closer']) ? $tokens[$scopeStartIndex]['scope_closer'] : null;
627+
628+
if (FunctionDeclarations::isArrowFunction($phpcsFile, $scopeStartIndex)) {
629+
$arrowFunctionInfo = FunctionDeclarations::getArrowFunctionOpenClose($phpcsFile, $scopeStartIndex);
630+
$scopeCloserIndex = $arrowFunctionInfo ? $arrowFunctionInfo['scope_closer'] : $scopeCloserIndex;
631+
}
632+
633+
if ($scopeStartIndex === 0) {
634+
$scopeCloserIndex = Helpers::getLastNonEmptyTokenIndexInFile($phpcsFile);
635+
}
636+
return $scopeCloserIndex;
637+
}
638+
639+
/**
640+
* @param File $phpcsFile
641+
*
642+
* @return int
643+
*/
644+
public static function getLastNonEmptyTokenIndexInFile(File $phpcsFile) {
645+
$tokens = $phpcsFile->getTokens();
646+
foreach (array_reverse($tokens, true) as $index => $token) {
647+
if (! in_array($token['code'], Tokens::$emptyTokens, true)) {
648+
return $index;
649+
}
650+
}
651+
self::debug('no non-empty token found for end of file');
652+
return 0;
653+
}
652654
}

‎VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class VariableAnalysisSniff implements Sniff {
3333
*
3434
* @var int[]
3535
*/
36-
private $scopeStartIndices = [];
36+
private $scopeStartIndices = [0];
3737

3838
/**
3939
* A list of custom functions which pass in variables to be initialized by
@@ -180,21 +180,20 @@ public function process(File $phpcsFile, $stackPtr) {
180180
T_CLOSURE,
181181
];
182182

183-
$scopeIndexThisCloses = array_reduce($this->scopeStartIndices, function ($found, $index) use ($phpcsFile, $stackPtr, $tokens) {
184-
$scopeCloserIndex = isset($tokens[$index]['scope_closer']) ? $tokens[$index]['scope_closer'] : null;
185-
if (FunctionDeclarations::isArrowFunction($phpcsFile, $index)) {
186-
$arrowFunctionInfo = FunctionDeclarations::getArrowFunctionOpenClose($phpcsFile, $index);
187-
$scopeCloserIndex = $arrowFunctionInfo ? $arrowFunctionInfo['scope_closer'] : $scopeCloserIndex;
188-
}
183+
$scopeIndicesThisCloses = array_reduce($this->scopeStartIndices, function ($found, $scopeStartIndex) use ($phpcsFile, $stackPtr) {
184+
$scopeCloserIndex = Helpers::getScopeCloseForScopeOpen($phpcsFile, $scopeStartIndex);
185+
189186
if (!$scopeCloserIndex) {
190-
Helpers::debug('No scope closer found for scope start', $index);
187+
Helpers::debug('No scope closer found for scope start', $scopeStartIndex);
191188
}
189+
192190
if ($stackPtr === $scopeCloserIndex) {
193-
return $index;
191+
$found[] = $scopeStartIndex;
194192
}
195193
return $found;
196-
}, null);
197-
if ($scopeIndexThisCloses) {
194+
}, []);
195+
196+
foreach ($scopeIndicesThisCloses as $scopeIndexThisCloses) {
198197
Helpers::debug('found closing scope at', $stackPtr, 'for scope', $scopeIndexThisCloses);
199198
$this->processScopeClose($phpcsFile, $scopeIndexThisCloses);
200199
}
@@ -856,8 +855,6 @@ protected function processVariableAsAssignment(File $phpcsFile, $stackPtr, $varN
856855
return false;
857856
}
858857

859-
$writtenPtr = Helpers::findWhereAssignExecuted($phpcsFile, $assignPtr);
860-
861858
// If the right-hand-side of the assignment to this variable is a reference
862859
// variable, then this variable is a reference to that one, and as such any
863860
// assignment to this variable (except another assignment by reference,
@@ -877,14 +874,14 @@ protected function processVariableAsAssignment(File $phpcsFile, $stackPtr, $varN
877874
// actually need to mark it as used in this case because the act of this
878875
// assignment will mark it used on the next token.
879876
$varInfo->referencedVariableScope = $currScope;
880-
$this->markVariableDeclaration($varName, ScopeType::LOCAL, null, $writtenPtr, $currScope, true);
877+
$this->markVariableDeclaration($varName, ScopeType::LOCAL, null, $stackPtr, $currScope, true);
881878
// An assignment to a reference is a binding and should not count as
882879
// initialization since it doesn't change any values.
883-
$this->markVariableAssignmentWithoutInitialization($varName, $writtenPtr, $currScope);
880+
$this->markVariableAssignmentWithoutInitialization($varName, $stackPtr, $currScope);
884881
return true;
885882
}
886883

887-
$this->markVariableAssignment($varName, $writtenPtr, $currScope);
884+
$this->markVariableAssignment($varName, $stackPtr, $currScope);
888885

889886
return true;
890887
}

0 commit comments

Comments
 (0)