Skip to content

Commit

Permalink
Allow superglobal assign and access at root level
Browse files Browse the repository at this point in the history
  • Loading branch information
paulbalandan committed Nov 5, 2023
1 parent ff38e66 commit 7c76a17
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 21 deletions.
7 changes: 7 additions & 0 deletions src/Rules/Superglobals/SuperglobalAccessRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\VerbosityLevel;

Expand All @@ -35,6 +36,8 @@ public function getNodeType(): string

/**
* @param Node\Expr\ArrayDimFetch $node
*
* @return list<RuleError>
*/
public function processNode(Node $node, Scope $scope): array
{
Expand All @@ -56,6 +59,10 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

if ($scope->getFunction() === null) {
return []; // ignore uses in root level (not inside function or method)
}

if ($node->dim === null) {
return [];
}
Expand Down
8 changes: 8 additions & 0 deletions src/Rules/Superglobals/SuperglobalAssignRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ private function processArrayDimFetch(Node $node, Scope $scope): array
return [];
}

if ($scope->getFunction() === null) {
return []; // ignore uses in root level (not inside function or method)
}

if ($scope->isInClass() && $scope->getClassReflection()->getName() === Superglobals::class) {
return [];
}
Expand Down Expand Up @@ -159,6 +163,10 @@ private function processVariableExpr(Node $node, Scope $scope): array
];
}

if ($scope->getFunction() === null) {
return []; // ignore uses in root level (not inside function or method)
}

return [
RuleErrorBuilder::message('Re-assigning arrays to $_GET directly is discouraged.')
->tip('Use \\Config\\Services::superglobals()->setGetArray() instead.')
Expand Down
14 changes: 11 additions & 3 deletions tests/Fixtures/Rules/Superglobals/superglobal-access-cases.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,18 @@

namespace SuperglobalAccess;

$foo = $_SERVER['foo'] ?? null;
/**
* @return list<mixed>
*/
function access(): array
{
$foo = $_SERVER['foo'] ?? null;

$a = (static fn (): string => mt_rand(0, 1) ? 'a' : 'b')();
$b = $_GET[$a] ?? null;
$a = (static fn (): string => mt_rand(0, 1) ? 'a' : 'b')();
$b = $_GET[$a] ?? null;

return [$foo, $b];
}

function bar(string $c): ?string
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,14 @@

namespace SuperglobalAssign;

$_SERVER['HTTP_HOST'] = 'https://localhost';
function assigns(): void
{
$_SERVER['HTTP_HOST'] = 'https://localhost';

$_GET['first_name'] = 'John Doe';
$_GET['first_name'] = 'John Doe';

$_SERVER[0] = 'hello';
$_SERVER[0] = 'hello';
}

function bar(string $key, string $value): void
{
Expand Down
8 changes: 4 additions & 4 deletions tests/Rules/Superglobals/SuperglobalAccessRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,22 @@ public function testRule(): void
$this->analyse([__DIR__ . '/../../Fixtures/Rules/Superglobals/superglobal-access-cases.php'], [
[
'Accessing offset \'foo\' directly on $_SERVER is discouraged.',
16,
21,
'Use \\Config\\Services::superglobals()->server(\'foo\') instead.',
],
[
'Accessing offset \'a\' directly on $_GET is discouraged.',
19,
24,
'Use \\Config\\Services::superglobals()->get(\'a\') instead.',
],
[
'Accessing offset \'b\' directly on $_GET is discouraged.',
19,
24,
'Use \\Config\\Services::superglobals()->get(\'b\') instead.',
],
[
'Accessing offset string directly on $_SERVER is discouraged.',
23,
31,
'Use \\Config\\Services::superglobals()->server() instead.',
],
]);
Expand Down
17 changes: 6 additions & 11 deletions tests/Rules/Superglobals/SuperglobalAssignRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,36 +40,31 @@ public function testRule(): void
$this->analyse([__DIR__ . '/../../Fixtures/Rules/Superglobals/superglobal-assign-cases.php'], [
[
'Assigning \'https://localhost\' directly on offset \'HTTP_HOST\' of $_SERVER is discouraged.',
16,
18,
'Use \\Config\\Services::superglobals()->setServer(\'HTTP_HOST\', \'https://localhost\') instead.',
],
[
'Assigning \'John Doe\' directly on offset \'first_name\' of $_GET is discouraged.',
18,
20,
'Use \\Config\\Services::superglobals()->setGet(\'first_name\', \'John Doe\') instead.',
],
[
'Assigning string directly on offset string of $_SERVER is discouraged.',
24,
27,
'Use \\Config\\Services::superglobals()->setServer() instead.',
],
[
'Assigning string directly on offset string of $_GET is discouraged.',
26,
29,
'Use \Config\Services::superglobals()->setGet() instead.',
],
[
'Cannot re-assign non-arrays to $_GET, got string.',
29,
32,
],
[
'Cannot re-assign non-arrays to $_GET, got int.',
30,
],
[
'Re-assigning arrays to $_GET directly is discouraged.',
32,
'Use \\Config\\Services::superglobals()->setGetArray() instead.',
33,
],
]);
}
Expand Down

0 comments on commit 7c76a17

Please sign in to comment.