Add disallowedSuperglobals rule

README.md

@@ -88,6 +88,7 @@ There are several different types (and configuration keys) that can be disallowe
 3. `disallowedFunctionCalls` - for functions like `function()`
 4. `disallowedConstants` - for constants like `DATE_ISO8601` or `DateTime::ISO8601` (which needs to be split to `class: DateTime` & `constant: ISO8601` in the configuration, see notes below)
 5. `disallowedNamespaces` - for usages of classes from a namespace
+6. `disallowedSuperglobals` - for usages of superglobal variables like `$GLOBALS` or `$_POST`
 
 Use them to add rules to your `phpstan.neon` config file. I like to use a separate file (`disallowed-calls.neon`) for these which I'll include later on in the main `phpstan.neon` config file. Here's an example, update to your needs:
 
@@ -133,6 +134,11 @@ parameters:
         -
             namespace: 'Assert\*'
             message: 'use Webmozart\Assert instead'
+
+    disallowedSuperglobals:
+        -
+            superglobal: '$_GET'
+            message: 'use the Request methods instead'
 ```
 
 The `message` key is optional. Functions and methods can be specified with or without `()`. Omitting `()` is not recommended though to avoid confusing method calls with class constants.

extension.neon

@@ -5,6 +5,7 @@ parameters:
 	disallowedStaticCalls: []
 	disallowedFunctionCalls: []
 	disallowedConstants: []
+	disallowedSuperglobals: []
 
 parametersSchema:
 	allowInRootDir: schema(string(), nullable())
@@ -55,6 +56,15 @@ parametersSchema:
 			)
 		)
 	)
+	disallowedSuperglobals: listOf(
+		arrayOf(
+			anyOf(
+				string(),
+				listOf(string()),
+				arrayOf(anyOf(int(), string(), bool()))
+			)
+		)
+	)
 
 services:
 	- Spaze\PHPStan\Rules\Disallowed\IsAllowedFileHelper(allowInRootDir: %allowInRootDir%)
@@ -63,6 +73,8 @@ services:
 	- Spaze\PHPStan\Rules\Disallowed\DisallowedHelper
 	- Spaze\PHPStan\Rules\Disallowed\DisallowedNamespaceFactory
 	- Spaze\PHPStan\Rules\Disallowed\DisallowedNamespaceHelper
+	- Spaze\PHPStan\Rules\Disallowed\DisallowedSuperglobalFactory
+	- Spaze\PHPStan\Rules\Disallowed\DisallowedSuperglobalHelper
 	-
 		factory: Spaze\PHPStan\Rules\Disallowed\Usages\NamespaceUsages(forbiddenNamespaces: %disallowedNamespaces%)
 		tags:
@@ -115,3 +127,7 @@ services:
 		factory: Spaze\PHPStan\Rules\Disallowed\Usages\ClassConstantUsages(disallowedConstants: %disallowedConstants%)
 		tags:
 			- phpstan.rules.rule
+	-
+		factory: Spaze\PHPStan\Rules\Disallowed\Usages\SuperglobalUsages(disallowedSuperglobals: %disallowedSuperglobals%)
+		tags:
+			- phpstan.rules.rule

src/DisallowedSuperglobal.php

@@ -0,0 +1,63 @@
+<?php
+declare(strict_types = 1);
+
+namespace Spaze\PHPStan\Rules\Disallowed;
+
+class DisallowedSuperglobal
+{
+
+	/** @var string */
+	private $superglobal;
+
+	/** @var string|null */
+	private $message;
+
+	/** @var string[] */
+	private $allowIn;
+
+	/** @var string */
+	private $errorIdentifier;
+
+
+	/**
+	 * @param string $superglobal
+	 * @param string|null $message
+	 * @param string[] $allowIn
+	 * @param string $errorIdentifier
+	 */
+	public function __construct(string $superglobal, ?string $message, array $allowIn, string $errorIdentifier)
+	{
+		$this->superglobal = $superglobal;
+		$this->message = $message;
+		$this->allowIn = $allowIn;
+		$this->errorIdentifier = $errorIdentifier;
+	}
+
+
+	public function getSuperglobal(): string
+	{
+		return $this->superglobal;
+	}
+
+
+	public function getMessage(): ?string
+	{
+		return $this->message;
+	}
+
+
+	/**
+	 * @return string[]
+	 */
+	public function getAllowIn(): array
+	{
+		return $this->allowIn;
+	}
+
+
+	public function getErrorIdentifier(): string
+	{
+		return $this->errorIdentifier;
+	}
+
+}

src/DisallowedSuperglobalFactory.php

@@ -0,0 +1,35 @@
+<?php
+declare(strict_types = 1);
+
+namespace Spaze\PHPStan\Rules\Disallowed;
+
+use PHPStan\ShouldNotHappenException;
+
+class DisallowedSuperglobalFactory
+{
+
+	/**
+	 * @param array<array{superglobal?:string, message?:string, allowIn?:string[], errorIdentifier?:string}> $config
+	 * @return DisallowedSuperglobal[]
+	 * @throws ShouldNotHappenException
+	 */
+	public function createFromConfig(array $config): array
+	{
+		$superglobals = [];
+		foreach ($config as $disallowedSuperglobal) {
+			$superglobal = $disallowedSuperglobal['superglobal'] ?? null;
+			if (!$superglobal) {
+				throw new ShouldNotHappenException("'superglobal' must be set in configuration items");
+			}
+			$disallowedSuperglobal = new DisallowedSuperglobal(
+				$superglobal,
+				$disallowedSuperglobal['message'] ?? null,
+				$disallowedSuperglobal['allowIn'] ?? [],
+				$disallowedSuperglobal['errorIdentifier'] ?? ''
+			);
+			$superglobals[$disallowedSuperglobal->getSuperglobal()] = $disallowedSuperglobal;
+		}
+		return array_values($superglobals);
+	}
+
+}

src/DisallowedSuperglobalHelper.php

@@ -0,0 +1,59 @@
+<?php
+declare(strict_types = 1);
+
+namespace Spaze\PHPStan\Rules\Disallowed;
+
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\RuleError;
+use PHPStan\Rules\RuleErrorBuilder;
+
+class DisallowedSuperglobalHelper
+{
+
+	/** @var IsAllowedFileHelper */
+	private $isAllowedFileHelper;
+
+
+	public function __construct(IsAllowedFileHelper $isAllowedFileHelper)
+	{
+		$this->isAllowedFileHelper = $isAllowedFileHelper;
+	}
+
+
+	private function isAllowedPath(Scope $scope, DisallowedSuperglobal $disallowedSuperglobal): bool
+	{
+		foreach ($disallowedSuperglobal->getAllowIn() as $allowedPath) {
+			if (fnmatch($this->isAllowedFileHelper->absolutizePath($allowedPath), $scope->getFile())) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+
+	/**
+	 * @param string $superglobal
+	 * @param Scope $scope
+	 * @param DisallowedSuperglobal[] $disallowedSuperglobals
+	 * @return RuleError[]
+	 */
+	public function getDisallowedMessage(string $superglobal, Scope $scope, array $disallowedSuperglobals): array
+	{
+		foreach ($disallowedSuperglobals as $disallowedSuperglobal) {
+			if ($disallowedSuperglobal->getSuperglobal() === $superglobal && !$this->isAllowedPath($scope, $disallowedSuperglobal)) {
+				return [
+					RuleErrorBuilder::message(sprintf(
+						'Using %s is forbidden, %s',
+						$disallowedSuperglobal->getSuperglobal(),
+						$disallowedSuperglobal->getMessage()
+					))
+						->identifier($disallowedSuperglobal->getErrorIdentifier())
+						->build(),
+				];
+			}
+		}
+
+		return [];
+	}
+
+}

src/Usages/SuperglobalUsages.php

@@ -0,0 +1,93 @@
+<?php
+declare(strict_types = 1);
+
+namespace Spaze\PHPStan\Rules\Disallowed\Usages;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\Variable;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
+use PHPStan\ShouldNotHappenException;
+use Spaze\PHPStan\Rules\Disallowed\DisallowedSuperglobal;
+use Spaze\PHPStan\Rules\Disallowed\DisallowedSuperglobalFactory;
+use Spaze\PHPStan\Rules\Disallowed\DisallowedSuperglobalHelper;
+
+/**
+ * Reports on global usage.
+ *
+ * @package Spaze\PHPStan\Rules\Disallowed
+ * @implements Rule<Variable>
+ */
+class SuperglobalUsages implements Rule
+{
+
+	/** @var DisallowedSuperglobalHelper */
+	private $disallowedHelper;
+
+	/** @var DisallowedSuperglobal[] */
+	private $disallowedSuperglobals;
+
+
+	/**
+	 * @param DisallowedSuperglobalHelper $disallowedSuperglobalHelper
+	 * @param DisallowedSuperglobalFactory $disallowedSuperglobalFactory
+	 * @param array<array{superglobal?:string, message?:string}> $disallowedSuperglobals
+	 * @throws ShouldNotHappenException
+	 */
+	public function __construct(DisallowedSuperglobalHelper $disallowedSuperglobalHelper, DisallowedSuperglobalFactory $disallowedSuperglobalFactory, array $disallowedSuperglobals)
+	{
+		$this->disallowedHelper = $disallowedSuperglobalHelper;
+		$this->disallowedSuperglobals = $disallowedSuperglobalFactory->createFromConfig($disallowedSuperglobals);
+	}
+
+
+	public function getNodeType(): string
+	{
+		return Variable::class;
+	}
+
+
+	private function isVariableBeingAssigned(Variable $variable): bool
+	{
+		$parentNode = $variable->getAttribute('parent');
+		if (!($parentNode instanceof Assign)) {
+			return false;
+		}
+
+		return $variable === $parentNode->var;
+	}
+
+
+	/**
+	 * @param Variable $node
+	 * @param Scope $scope
+	 * @return RuleError[]
+	 * @throws ShouldNotHappenException
+	 */
+	public function processNode(Node $node, Scope $scope): array
+	{
+		if (!($node instanceof Variable)) {
+			throw new ShouldNotHappenException(sprintf('$node should be %s but is %s', Variable::class, get_class($node)));
+		}
+
+		// If it's an assignment, allow it since it might define the variable in the current scope.
+		if ($this->isVariableBeingAssigned($node)) {
+			return [];
+		}
+
+		$variableName = $node->name;
+		if (!is_string($variableName)) {
+			return [];
+		}
+
+		$definedVariables = $scope->getDefinedVariables();
+		if (in_array($variableName, $definedVariables, true)) {
+			return [];
+		}
+
+		return $this->disallowedHelper->getDisallowedMessage('$' . $variableName, $scope, $this->disallowedSuperglobals);
+	}
+
+}

tests/Usages/SuperglobalUsagesTest.php

@@ -0,0 +1,77 @@
+<?php
+declare(strict_types = 1);
+
+namespace Spaze\PHPStan\Rules\Disallowed\Usages;
+
+use PHPStan\File\FileHelper;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use Spaze\PHPStan\Rules\Disallowed\DisallowedSuperglobalFactory;
+use Spaze\PHPStan\Rules\Disallowed\DisallowedSuperglobalHelper;
+use Spaze\PHPStan\Rules\Disallowed\IsAllowedFileHelper;
+
+class SuperglobalUsagesTest extends RuleTestCase
+{
+
+	protected function getRule(): Rule
+	{
+		return new SuperglobalUsages(
+			new DisallowedSuperglobalHelper(new IsAllowedFileHelper(new FileHelper(__DIR__))),
+			new DisallowedSuperglobalFactory(),
+			[
+				[
+					'superglobal' => '$GLOBALS',
+					'message' => 'the cake is a lie',
+					'allowIn' => [
+						'../src/disallowed-allowed/*.php',
+						'../src/*-allow/*.*',
+					],
+				],
+				[
+					'superglobal' => '$_GET',
+					'message' => 'the cake is a lie',
+					'allowIn' => [
+						'../src/disallowed-allowed/*.php',
+						'../src/*-allow/*.*',
+					],
+				],
+				[
+					'superglobal' => '$TEST_GLOBAL_VARIABLE',
+					'message' => 'the cake is a lie',
+					'allowIn' => [
+						'../src/disallowed-allowed/*.php',
+						'../src/*-allow/*.*',
+					],
+				],
+			]
+		);
+	}
+
+
+	public function testRule(): void
+	{
+		// Based on the configuration above, in this file:
+		$this->analyse([__DIR__ . '/../src/disallowed/superglobalUsages.php'], [
+			[
+				// expect this error message:
+				'Using $GLOBALS is forbidden, the cake is a lie',
+				// on this line:
+				8,
+			],
+			[
+				'Using $_GET is forbidden, the cake is a lie',
+				9,
+			],
+			[
+				'Using $_GET is forbidden, the cake is a lie',
+				12,
+			],
+			[
+				'Using $TEST_GLOBAL_VARIABLE is forbidden, the cake is a lie',
+				19,
+			],
+		]);
+		$this->analyse([__DIR__ . '/../src/disallowed-allow/superglobalUsages.php'], []);
+	}
+
+}