From 7c76a17c31026dd9abef2457e27d289570f23fda Mon Sep 17 00:00:00 2001 From: "John Paul E. Balandan, CPA" Date: Sun, 5 Nov 2023 18:37:27 +0800 Subject: [PATCH] Allow superglobal assign and access at root level --- .../Superglobals/SuperglobalAccessRule.php | 7 +++++++ .../Superglobals/SuperglobalAssignRule.php | 8 ++++++++ .../Superglobals/superglobal-access-cases.php | 14 +++++++++++--- .../Superglobals/superglobal-assign-cases.php | 9 ++++++--- .../Superglobals/SuperglobalAccessRuleTest.php | 8 ++++---- .../Superglobals/SuperglobalAssignRuleTest.php | 17 ++++++----------- 6 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/Rules/Superglobals/SuperglobalAccessRule.php b/src/Rules/Superglobals/SuperglobalAccessRule.php index 185ccf0..18b1d22 100644 --- a/src/Rules/Superglobals/SuperglobalAccessRule.php +++ b/src/Rules/Superglobals/SuperglobalAccessRule.php @@ -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; @@ -35,6 +36,8 @@ public function getNodeType(): string /** * @param Node\Expr\ArrayDimFetch $node + * + * @return list */ public function processNode(Node $node, Scope $scope): array { @@ -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 []; } diff --git a/src/Rules/Superglobals/SuperglobalAssignRule.php b/src/Rules/Superglobals/SuperglobalAssignRule.php index 997a58b..119ea76 100644 --- a/src/Rules/Superglobals/SuperglobalAssignRule.php +++ b/src/Rules/Superglobals/SuperglobalAssignRule.php @@ -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 []; } @@ -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.') diff --git a/tests/Fixtures/Rules/Superglobals/superglobal-access-cases.php b/tests/Fixtures/Rules/Superglobals/superglobal-access-cases.php index 43c92b7..3dc1c3b 100644 --- a/tests/Fixtures/Rules/Superglobals/superglobal-access-cases.php +++ b/tests/Fixtures/Rules/Superglobals/superglobal-access-cases.php @@ -13,10 +13,18 @@ namespace SuperglobalAccess; -$foo = $_SERVER['foo'] ?? null; +/** + * @return list + */ +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 { diff --git a/tests/Fixtures/Rules/Superglobals/superglobal-assign-cases.php b/tests/Fixtures/Rules/Superglobals/superglobal-assign-cases.php index e551ad7..766a369 100644 --- a/tests/Fixtures/Rules/Superglobals/superglobal-assign-cases.php +++ b/tests/Fixtures/Rules/Superglobals/superglobal-assign-cases.php @@ -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 { diff --git a/tests/Rules/Superglobals/SuperglobalAccessRuleTest.php b/tests/Rules/Superglobals/SuperglobalAccessRuleTest.php index 60615cd..77dc1c2 100644 --- a/tests/Rules/Superglobals/SuperglobalAccessRuleTest.php +++ b/tests/Rules/Superglobals/SuperglobalAccessRuleTest.php @@ -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.', ], ]); diff --git a/tests/Rules/Superglobals/SuperglobalAssignRuleTest.php b/tests/Rules/Superglobals/SuperglobalAssignRuleTest.php index 3670e74..2e62ca6 100644 --- a/tests/Rules/Superglobals/SuperglobalAssignRuleTest.php +++ b/tests/Rules/Superglobals/SuperglobalAssignRuleTest.php @@ -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, ], ]); }