Skip to content

Commit

Permalink
Prevent detecting static closures when looking for static variables (#…
Browse files Browse the repository at this point in the history
…281)

* Add test for static function inside array_map

* Prevent detecting a static closure as a static variable

* Improve documentation of processVariableAsStaticDeclaration

* Treat open paren as start of statement for static detection

* Add tests for static arrow functions followed by variable defs

* Also ignore static anonymous arrow functions
  • Loading branch information
sirbrillig authored Oct 5, 2022
1 parent aaf9022 commit 6273088
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,31 @@ function function_with_static_variable_inside_anonymous_function_inside_argument
echo $providerId;
});
}

function function_with_static_closure_inside_array_map($account, $getTeamsByUserQuery) {
$developer_team_ids = array_map(
static function (GroupInterface $group) {
return (int) $group->id();
},
($getTeamsByUserQuery)((int) $account->id())
);
$developer_team_ids = array_map(
static function (GroupInterface $group) {
return (int) $group->id();
},
$getTeamsByUserQuery($account)
);
return $developer_team_ids;
}

function function_with_static_arrow_closure_inside_array_map($account, $getTeamsByUserQuery) {
$developer_team_ids = array_map(
static fn (GroupInterface $group) => (int) $group->id(),
($getTeamsByUserQuery)((int) $account->id())
);
$developer_team_ids = array_map(
static fn (GroupInterface $group) => (int) $group->id(),
$getTeamsByUserQuery($account)
);
return $developer_team_ids;
}
38 changes: 27 additions & 11 deletions VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -1276,16 +1276,20 @@ protected function processVariableAsGlobalDeclaration(File $phpcsFile, $stackPtr
/**
* Process a variable as a static declaration within a function.
*
* This will not operate on variables that are written a class definition
* like `static $foo;` or `public static ?int $foo = 'bar';` because class
* properties (static or instance) are currently not tracked by this sniff.
* This is because a class property might be unused inside the class, but
* used outside the class (we cannot easily know if it is unused); this is
* also because it's common and legal to define class properties when they
* are assigned and that assignment can happen outside a class (we cannot
* easily know if the use of a property is undefined). These sorts of checks
* are better performed by static analysis tools that can see a whole project
* rather than a linter which can only easily see a file or some lines.
* Specifically, this looks for variable definitions of the form `static
* $foo = 'hello';` or `static int $foo;` inside a function definition.
*
* This will not operate on variables that are written in a class definition
* outside of a function like `static $foo;` or `public static ?int $foo =
* 'bar';` because class properties (static or instance) are currently not
* tracked by this sniff. This is because a class property might be unused
* inside the class, but used outside the class (we cannot easily know if it
* is unused); this is also because it's common and legal to define class
* properties when they are assigned and that assignment can happen outside a
* class (we cannot easily know if the use of a property is undefined). These
* sorts of checks are better performed by static analysis tools that can see
* a whole project rather than a linter which can only easily see a file or
* some lines.
*
* If found, such a variable will be marked as declared (and possibly
* assigned, if it includes an initial value) within the scope of the
Expand Down Expand Up @@ -1313,7 +1317,7 @@ protected function processVariableAsStaticDeclaration(File $phpcsFile, $stackPtr
$tokens = $phpcsFile->getTokens();

// Search backwards for a `static` keyword that occurs before the start of the statement.
$startOfStatement = $phpcsFile->findPrevious([T_SEMICOLON, T_OPEN_CURLY_BRACKET, T_FN_ARROW], $stackPtr - 1, null, false, null, true);
$startOfStatement = $phpcsFile->findPrevious([T_SEMICOLON, T_OPEN_CURLY_BRACKET, T_FN_ARROW, T_OPEN_PARENTHESIS], $stackPtr - 1, null, false, null, true);
$staticPtr = $phpcsFile->findPrevious([T_STATIC], $stackPtr - 1, null, false, null, true);
if (! is_int($startOfStatement)) {
$startOfStatement = 1;
Expand All @@ -1326,6 +1330,18 @@ protected function processVariableAsStaticDeclaration(File $phpcsFile, $stackPtr
return false;
}

// Is the 'static' keyword an anonymous static function declaration? If so,
// this is not a static variable declaration.
$tokenAfterStatic = $phpcsFile->findNext(Tokens::$emptyTokens, $staticPtr + 1, null, true, null, true);
$functionTokenTypes = [
T_FUNCTION,
T_CLOSURE,
T_FN,
];
if (is_int($tokenAfterStatic) && in_array($tokens[$tokenAfterStatic]['code'], $functionTokenTypes, true)) {
return false;
}

// Is the token inside function parameters? If so, this is not a static
// declaration because we must be inside a function body.
if (Helpers::isTokenFunctionParameter($phpcsFile, $stackPtr)) {
Expand Down

0 comments on commit 6273088

Please sign in to comment.